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]Remove collaborators in workspace creation page and refactor with getDisplayedType #8520

Conversation

wanglam
Copy link
Contributor

@wanglam wanglam commented Oct 8, 2024

Description

This PR updates below for collaborators in workspace creation page and workspace setting page.

  1. Remove collaborators tab in workspace creation page
  2. Redirect to collaborators tab in workspace setting page after workspace created
  3. Use getDisplayedType generate displayed types in collaborators table

Screenshot

image
image

Testing the changes

  • Clone branch code and run yarn osd bootstrap --single-version loose
  • Add below configs in config/opensearch_dashboards.yml
savedObjects.permission.enabled: true
workspace.enabled: true
uiSettings:
  overrides:
    'home:useNewHomePage': true
opensearchDashboards.dashboardAdmin.users: ['admin']
  • Run yarn start --no-base-path
  • Login with admin user and go to workspace create page by bottom left menu
  • The "Add collaborators" section should bd removed
  • Fill the workspace name and data sources, click "Create workspace" button
  • It should redirect to collaborators tab in workspace setting page
  • Add collaborators the types should be displayed as expected

Changelog

  • feat: [Workspace]Remove collaborators in workspace creation page

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 Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.96%. Comparing base (794757d) to head (8e852af).
Report is 81 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8520   +/-   ##
=======================================
  Coverage   60.95%   60.96%           
=======================================
  Files        3777     3777           
  Lines       89641    89639    -2     
  Branches    14046    14049    +3     
=======================================
+ Hits        54643    54646    +3     
+ Misses      31584    31583    -1     
+ Partials     3414     3410    -4     
Flag Coverage Δ
Linux_1 29.09% <100.00%> (+<0.01%) ⬆️
Linux_2 56.27% <ø> (ø)
Linux_3 37.75% <ø> (+<0.01%) ⬆️
Linux_4 29.92% <ø> (ø)
Windows_1 29.10% <100.00%> (+<0.01%) ⬆️
Windows_2 56.23% <ø> (ø)
Windows_3 37.75% <ø> (ø)
Windows_4 29.92% <ø> (ø)

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.

@@ -177,7 +167,7 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => {
savedObjects={savedObjects}
onSubmit={handleWorkspaceFormSubmit}
operationType={WorkspaceOperationType.Create}
permissionEnabled={isPermissionEnabled}
permissionEnabled={false}
Copy link
Member

@SuZhou-Joe SuZhou-Joe Oct 9, 2024

Choose a reason for hiding this comment

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

Can we remove this props instead of giving false. I think this permissionEnabled props is not needed any more right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, let's remove the permissionEnabled property. I met a unit tests failed without this property before. But seems all the unit tests can be passed now. Let me remove this property.

SuZhou-Joe
SuZhou-Joe previously approved these changes Oct 9, 2024
SuZhou-Joe
SuZhou-Joe previously approved these changes Oct 9, 2024
newWorkspaceId,
http.basePath
DetailTab.Collaborators
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need to check permission enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I think we should check this. Let me add a fallback logic here.

@wanglam wanglam changed the title [Workspace]Remove collaborators in workspace creation page [Workspace]Remove collaborators in workspace creation page and refactor with getDisplayedType Oct 9, 2024
@wanglam
Copy link
Contributor Author

wanglam commented Oct 10, 2024

Hi @SuZhou-Joe @Hailong-am , I've refactor the type column in the collaborator table. Feel free to help me take a look. Thank you.

SuZhou-Joe
SuZhou-Joe previously approved these changes Oct 11, 2024
@Hailong-am Hailong-am merged commit fcc18fb into opensearch-project:main Oct 11, 2024
67 of 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-8520-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 fcc18fb2033f7216ca51854900946339d53bc772
# Push it to GitHub
git push --set-upstream origin backport/backport-8520-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-8520-to-2.x.

opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 11, 2024
…or with getDisplayedType (#8520)

* Remove workspace collaborators section in workspace creation

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

* Redirect to workspace detail collaborators tab after workspace created

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

* Changeset file for PR #8520 created/updated

* Add getDisplayedType in default collaborator types

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

* Update detail type to ReactNode

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

* Add learn more link

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

* Remove permissionEnabled passed in WorkspaceCreatorForm

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

* Redirect to use case landing page if permission not enabled

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

* Use getDisplayedType and render &mdash; for empty

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

* Fix incorrect unit tests

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

* Assign current user as workspace admin

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

---------

Signed-off-by: Lin Wang <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit fcc18fb)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
SuZhou-Joe pushed a commit that referenced this pull request Oct 12, 2024
…or with getDisplayedType (#8520) (#8560)

* Remove workspace collaborators section in workspace creation



* Redirect to workspace detail collaborators tab after workspace created



* Changeset file for PR #8520 created/updated

* Add getDisplayedType in default collaborator types



* Update detail type to ReactNode



* Add learn more link



* Remove permissionEnabled passed in WorkspaceCreatorForm



* Redirect to use case landing page if permission not enabled



* Use getDisplayedType and render &mdash; for empty



* Fix incorrect unit tests



* Assign current user as workspace admin



---------



(cherry picked from commit fcc18fb)

Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Qxisylolo pushed a commit to Qxisylolo/OpenSearch-Dashboards that referenced this pull request Oct 30, 2024
…or with getDisplayedType (opensearch-project#8520)

* Remove workspace collaborators section in workspace creation

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

* Redirect to workspace detail collaborators tab after workspace created

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

* Changeset file for PR opensearch-project#8520 created/updated

* Add getDisplayedType in default collaborator types

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

* Update detail type to ReactNode

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

* Add learn more link

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

* Remove permissionEnabled passed in WorkspaceCreatorForm

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

* Redirect to use case landing page if permission not enabled

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

* Use getDisplayedType and render &mdash; for empty

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

* Fix incorrect unit tests

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

* Assign current user as workspace admin

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

---------

Signed-off-by: Lin Wang <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
@ananzh ananzh added the v2.18.0 label Oct 30, 2024
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.

5 participants