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

[Workspace] Validate if workspace exists when setup inside a workspace #6154

Conversation

SuZhou-Joe
Copy link
Member

@SuZhou-Joe SuZhou-Joe commented Mar 15, 2024

Description

When user enters a url with workspaceId pattern inside url, workspace plugin should validate if the id exists, or should show an friendly error page to ask user to go back to home page without a pattern in url.

Issues Resolved

closes #6155

Screenshot

image

Testing the changes

  • Using the branch to bootstrap
  • Enable workspace feature flag in yml file by set workspace.enabled: true
  • Start OSD by using yarn start --no-base-path to make sure no random base path will be appended
  • Navigate to devTools
  • Insert test workspaces by calling
PUT .kibana/_doc/workspace:foo
{
  "type": "workspace",
  "workspace": {
    "name": "foo"
  }
}
  • Visit devTools under the test workspace: http://localhost:5601/w/foo/app/dev_tools, the page should render successfully.
  • Visit devTools under a unexisting workspace: http://localhost:5601/w/not_existing/app/dev_tools, should see a page with error just like the screenshot above.
  • Visit the error page under a existing worksapce: http://localhost:5601/w/foo/app/workspace_fatal_error, user should be redirect to http://localhost:5601/w/foo/app/workspace_overview page, though the page has not been implement yet.

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

Attention: Patch coverage is 76.92308% with 6 lines in your changes missing coverage. Please review.

Project coverage is 67.31%. Comparing base (f103b7e) to head (fc570f3).
Report is 577 commits behind head on main.

Files with missing lines Patch % Lines
src/plugins/workspace/public/plugin.ts 68.42% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6154   +/-   ##
=======================================
  Coverage   67.31%   67.31%           
=======================================
  Files        3348     3350    +2     
  Lines       64966    64993   +27     
  Branches    10465    10468    +3     
=======================================
+ Hits        43729    43752   +23     
- Misses      18690    18694    +4     
  Partials     2547     2547           
Flag Coverage Δ
Linux_1 31.75% <76.92%> (+0.03%) ⬆️
Linux_2 55.55% <ø> (ø)
Linux_3 44.68% <ø> (+0.01%) ⬆️
Linux_4 35.02% <ø> (ø)
Windows_1 31.77% <76.92%> (+0.03%) ⬆️
Windows_2 55.52% <ø> (ø)
Windows_3 44.68% <ø> (ø)
Windows_4 35.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

application.navigateToApp(WORKSPACE_FATAL_ERROR_APP_ID, {
replace: true,
state: {
error: result.error,
Copy link
Member

Choose a reason for hiding this comment

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

do we consider to to check result.error exists before access it. like as const error = result.error || "Unknown error"; so ensures that error always have value

Copy link
Member Author

Choose a reason for hiding this comment

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

If the error is empty or null, we will hide the callout component to not display the detailed error message, but update to result?.error to protect the code.

Flyingliuhub
Flyingliuhub previously approved these changes Mar 15, 2024
@SuZhou-Joe SuZhou-Joe force-pushed the feature/validate-workspace-id-when-setup branch from cf4c84c to 8350bef Compare March 20, 2024 03:08
Co-authored-by: Yulong Ruan <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
@SuZhou-Joe SuZhou-Joe merged commit 0cc91ab into opensearch-project:main Mar 20, 2024
68 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-6154-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 0cc91ab3ed89ed72552a29e503cab2dab324a073
# Push it to GitHub
git push --set-upstream origin backport/backport-6154-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-6154-to-2.x.

SuZhou-Joe added a commit to SuZhou-Joe/OpenSearch-Dashboards that referenced this pull request Apr 14, 2024
opensearch-project#6154)

* feat: validate if workspace exists when setup inside a workspace

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

* feat: add CHANGELOG

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

* fix: unit test

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

* feat: optimize import order

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

* feat: add protection

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

* Apply suggestions from code review

Co-authored-by: Yulong Ruan <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>

* feat: jump to landing page

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

---------

Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Yulong Ruan <[email protected]>
(cherry picked from commit 0cc91ab)
SuZhou-Joe added a commit that referenced this pull request Apr 15, 2024
#6154) (#6444)

* feat: validate if workspace exists when setup inside a workspace

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

* feat: add CHANGELOG

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

* fix: unit test

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

* feat: optimize import order

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

* feat: add protection

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

* Apply suggestions from code review

Co-authored-by: Yulong Ruan <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>

* feat: jump to landing page

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

---------

Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Yulong Ruan <[email protected]>
(cherry picked from commit 0cc91ab)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Workspace] Validate the workspace id in the url
5 participants