Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken link in the Overview page #314

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

vchrombie
Copy link
Contributor

@vchrombie vchrombie commented Apr 24, 2021

Description

The 'Try our sample data' link in the opensearch dashboards overview page is broken. It has to be pointed to the path /app/home#/tutorial_directory/sampleData. This commit fixes this bug. The snapshots are updated accordingly.

Issues Resolved

Fixes #193

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

@odfe-release-bot
Copy link

✅   DCO Check Passed 6339a91

Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vchrombie for the PR, I have few small comments.

@vchrombie vchrombie changed the title Fix broken link of the sample data path in the Overview page Fix broken link in the Overview page Apr 26, 2021
@odfe-release-bot
Copy link

✅   DCO Check Passed 32177ce

@odfe-release-bot
Copy link

✅   DCO Check Passed 286ff0c

@odfe-release-bot
Copy link

✅   DCO Check Passed c36c78c

@boktorbb boktorbb self-requested a review April 26, 2021 18:23
Copy link
Contributor

@boktorbb boktorbb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After pulling the PR down, I notice that the link works now but it doesn't seamlessly route to the sample data path like on the home screen but actually reloads the whole dashboards app. While this works, it isn't fully the experience we want. Could you take a look there?

The 'Try our sample data' link in the opensearch dashboards
overview page is broken. It has to be pointed to the path
`/app/home#/tutorial_directory/sampleData`. This commit fixes
this bug. The snapshots are updated accordingly.

Signed-off-by: Venu Vardhan Reddy Tekula <[email protected]>
@vchrombie
Copy link
Contributor Author

vchrombie commented Apr 27, 2021

Hi boktorbb-amzn, thanks for the review.

After pulling the PR down, I notice that the link works now but it doesn't seamlessly route to the sample data path like on the home screen but actually reloads the whole dashboards app. While this works, it isn't fully the experience we want. Could you take a look there?

Nice catch, you are right. I could fix the issue and updated the PR too. Can you look at this again?

@vchrombie vchrombie requested review from boktorbb and mihirsoni April 27, 2021 09:45
@odfe-release-bot
Copy link

✅   DCO Check Passed f81948c

Copy link
Contributor

@boktorbb boktorbb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The link is redirecting properly now. Looks good to me and thank you for your contribution!

Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! One integ test failing but unrelated to this and currently being fixed. Otherwise unit and functional tests are passing.

Thank you!

@mihirsoni mihirsoni merged commit 126c2b8 into opensearch-project:main Apr 28, 2021
kavilla pushed a commit that referenced this pull request May 21, 2021
The 'Try our sample data' link in the opensearch dashboards
overview page is broken. It has to be pointed to the path
`/app/home#/tutorial_directory/sampleData`. This commit fixes
this bug. The snapshots are updated accordingly.

Fixes #193

Signed-off-by: Venu Vardhan Reddy Tekula <[email protected]>
SuZhou-Joe added a commit to SuZhou-Joe/OpenSearch-Dashboards that referenced this pull request Apr 7, 2024
* [Workspace]Add permission control logic for workspace (opensearch-project#6052)

* Add permission control for workspace

Signed-off-by: Lin Wang <[email protected]>

* Add changelog for permission control in workspace

Signed-off-by: Lin Wang <[email protected]>

* Fix integration tests and remove no need type

Signed-off-by: Lin Wang <[email protected]>

* Update permission enabled for workspace CRUD integration tests

Signed-off-by: Lin Wang <[email protected]>

* Change back to config schema

Signed-off-by: Lin Wang <[email protected]>

* feat: do not append workspaces field when no workspaces present (#6)

* feat: do not append workspaces field when no workspaces present

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: do not append workspaces field when no workspaces present

Signed-off-by: SuZhou-Joe <[email protected]>

---------

Signed-off-by: SuZhou-Joe <[email protected]>

* fix: authInfo destructure (#7)

* fix: authInfo destructure

Signed-off-by: SuZhou-Joe <[email protected]>

* fix: unit test error

Signed-off-by: SuZhou-Joe <[email protected]>

---------

Signed-off-by: SuZhou-Joe <[email protected]>

* Fix permissions assign in attributes

Signed-off-by: Lin Wang <[email protected]>

* Remove deleteByWorkspace since not exists

Signed-off-by: Lin Wang <[email protected]>

* refactor: remove formatWorkspacePermissionModeToStringArray

Signed-off-by: Lin Wang <[email protected]>

* Remove current not used code

Signed-off-by: Lin Wang <[email protected]>

* Add missing unit tests for permission control

Signed-off-by: Lin Wang <[email protected]>

* Update workspaces API test describe

Signed-off-by: Lin Wang <[email protected]>

* Fix workspace CRUD API integration tests failed

Signed-off-by: Lin Wang <[email protected]>

* Address PR comments

Signed-off-by: Lin Wang <[email protected]>

* Store permissions when savedObjects.permissions.enabled

Signed-off-by: Lin Wang <[email protected]>

* Add permission control for deleteByWorkspace

Signed-off-by: Lin Wang <[email protected]>

* Update src/plugins/workspace/server/permission_control/client.ts

Signed-off-by: SuZhou-Joe <[email protected]>

* Update src/plugins/workspace/server/permission_control/client.ts

Signed-off-by: SuZhou-Joe <[email protected]>

* Refactor permissions field in workspace create and update API

Signed-off-by: Lin Wang <[email protected]>

* Fix workspace CRUD API integration tests

Signed-off-by: Lin Wang <[email protected]>

---------

Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
Signed-off-by: Lin Wang <[email protected]>

* Convert permission settings in client side

Signed-off-by: Lin Wang <[email protected]>

* Fix workspace list always render

Signed-off-by: Lin Wang <[email protected]>

---------

Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Sample data path from Overview page is broken
5 participants