[Workspace]feat: add collaborator table to workspace detail page #8501

raintygao · 2024-10-05T14:35:45Z

Description

add collaborator table to workspace detail page

Screenshot

Testing the changes

Go to workspace detail page, then test the related changes.

Changelog

feat: add collaborator table to workspace detail 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

codecov · 2024-10-06T07:50:55Z

Codecov Report

Attention: Patch coverage is 69.09091% with 34 lines in your changes missing coverage. Please review.

Project coverage is 60.93%. Comparing base (61eedf7) to head (fd17483).
Report is 93 commits behind head on main.

Files with missing lines	Patch %	Lines
...ts/workspace_form/workspace_collaborator_table.tsx	62.33%	24 Missing and 5 partials ⚠️
...ic/components/workspace_form/use_workspace_form.ts	50.00%	3 Missing ⚠️
...ponents/workspace_form/add_collaborator_button.tsx	88.88%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8501      +/-   ##
==========================================
- Coverage   60.93%   60.93%   -0.01%     
==========================================
  Files        3767     3769       +2     
  Lines       89406    89498      +92     
  Branches    13994    14008      +14     
==========================================
+ Hits        54478    54534      +56     
- Misses      31526    31556      +30     
- Partials     3402     3408       +6

Flag	Coverage Δ
Linux_1	`29.00% <69.09%> (+0.04%)`	⬆️
Linux_2	`56.25% <ø> (ø)`
Linux_3	`37.75% <ø> (+<0.01%)`	⬆️
Linux_4	`29.91% <ø> (ø)`
Windows_1	`29.02% <69.09%> (+0.04%)`	⬆️
Windows_2	`56.20% <ø> (ø)`
Windows_3	`37.75% <ø> (ø)`
Windows_4	`29.91% <ø> (ø)`

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.

SuZhou-Joe · 2024-10-07T02:13:47Z

src/plugins/workspace/public/components/workspace_form/add_collaborator_button.tsx

+            group: collaboratorId,
+          }),
+    }));
+    const newPermissionSettings = [...permissionSettings, ...addedSettings];


Shall we check duplicate records here?

Yes, we change to use modal and the check interaction is to be confirmed with at least one owner, we could ignore this for now.

SuZhou-Joe · 2024-10-07T02:15:19Z

src/plugins/workspace/public/components/workspace_form/add_collaborator_button.tsx

+    onChange(newPermissionSettings);
+    handleSubmitPermissionSettings(newPermissionSettings as WorkspacePermissionSetting[]);


It seems we are telling parent component we are changing the permission settings through 2 callbacks. May I know the reason?

The original reason is that we will split change and submit into two steps for now, one is changing data flow, one is submitting and refreshing. For now we won't refresh after submitting, let me combine them.

SuZhou-Joe · 2024-10-07T02:19:18Z

src/plugins/workspace/public/components/workspace_form/utils.ts

+  // const userPermissionExists = finalPermissionSettings.find(
+  //   (setting) => setting.type === WorkspacePermissionItemType.User
+  // );
+  // const groupPermissionExists = finalPermissionSettings.find(
+  //   (setting) => setting.type === WorkspacePermissionItemType.Group
+  // );
+  // if (!userPermissionExists) {
+  //   finalPermissionSettings.push({
+  //     ...emptyUserPermission,
+  //     id: generateNextPermissionSettingsId(finalPermissionSettings),
+  //   } as typeof finalPermissionSettings[0]);
+  // }
+  // if (!groupPermissionExists) {
+  //   finalPermissionSettings.push({
+  //     ...emptyUserGroupPermission,
+  //     id: generateNextPermissionSettingsId(finalPermissionSettings),
+  //   } as typeof finalPermissionSettings[0]);
+  // }


Suggested change

// const userPermissionExists = finalPermissionSettings.find(

// (setting) => setting.type === WorkspacePermissionItemType.User

// );

// const groupPermissionExists = finalPermissionSettings.find(

// (setting) => setting.type === WorkspacePermissionItemType.Group

// );

// if (!userPermissionExists) {

// finalPermissionSettings.push({

// ...emptyUserPermission,

// id: generateNextPermissionSettingsId(finalPermissionSettings),

// } as typeof finalPermissionSettings[0]);

// }

// if (!groupPermissionExists) {

// finalPermissionSettings.push({

// ...emptyUserGroupPermission,

// id: generateNextPermissionSettingsId(finalPermissionSettings),

// } as typeof finalPermissionSettings[0]);

// }

Better to remove the code instead of comment out.

Oh sorry, I missed this change. 😂

SuZhou-Joe · 2024-10-07T02:23:15Z

src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx

+          cancelButtonText="Cancel"
+          confirmButtonText="Confirm"
+        >
+          <EuiText>
+            <p>Delete collaborator? The collaborators will not have access to the workspace.</p>


SuZhou-Joe · 2024-10-07T02:33:44Z

src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx

+      const basicSettings = {
+        ...setting,
+        // This is used for table display and search match.
+        displayedType:


https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8486/files#diff-974bcd16c33ef28e4be36bfb140026ab9cb437f8b85bdd717ba7ecf58d38bd0eR17

Should we use this method to get the displayedType? I do not think it is collaborator_table's duty to generate the displayedType.

Yes, but this method not been implemented. It only provides a virtual type. cc @wanglam

I think we should call all displayedCollaboratorTypes.getDisplayedType and use the first not undefined result and display a — if it's undefined. Forget to register default getDisplayedType in my previous PR. We can call getDisplayedType here first.

wanglam · 2024-10-08T03:45:14Z

src/plugins/workspace/public/components/workspace_form/constants.ts

@@ -138,7 +138,7 @@ export const permissionModeOptions = [
    inputDisplay: i18n.translate(
      'workspace.form.permissionSettingPanel.permissionModeOptions.readAndWrite',
      {
-        defaultMessage: 'Read & Write',
+        defaultMessage: 'Read and write',


Nit: can we use WORKSPACE_ACCESS_LEVEL_NAMES in src/plugins/workspace/public/constants.ts instead?

ruanyl · 2024-10-08T05:14:59Z

src/plugins/workspace/public/components/workspace_detail/workspace_detail.test.tsx

@@ -16,7 +16,7 @@ import { WorkspaceFormProvider, WorkspaceOperationType } from '../workspace_form
 import { DataSourceConnectionType } from '../../../common/types';
 import * as utilsExports from '../../utils';
 import { IntlProvider } from 'react-intl';
-
+import { of } from 'rxjs';


Nit: let's keep the empty line

Suggested change

import { of } from 'rxjs';

import { of } from 'rxjs';

ruanyl · 2024-10-08T05:44:56Z

src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx

+          <EuiConfirmModal
+            title="Change access level"
+            onCancel={() => modal.close()}
+            onConfirm={async () => {


Is this async necessary?

ruanyl · 2024-10-08T05:48:46Z

src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx

+      if (selection) {
+        const modal = overlays.openModal(
+          <EuiConfirmModal
+            title="Change access level"


wanglam · 2024-10-08T06:47:03Z

src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx

+  const [isPopoverOpen, setIsPopoverOpen] = useState(false);
+  const { overlays } = useOpenSearchDashboards();
+
+  const accessLevelOptions = Object.keys(WORKSPACE_ACCESS_LEVEL_NAMES).map((level) => ({


Nit: We can use Object.keys(WORKSPACE_ACCESS_LEVEL_NAMES) as WorkspaceCollaboratorAccessLevel[]
, then we don't need too many as WorkspaceCollaboratorAccessLevel below.

wanglam · 2024-10-08T06:47:49Z

src/plugins/workspace/public/components/workspace_form/workspace_collaborator_table.tsx

+            confirmButtonText="Confirm"
+          >
+            <EuiText>
+              <p>


Missing i18n here.

Signed-off-by: tygao <[email protected]>

* feat: add collaborator table Signed-off-by: tygao <[email protected]> * Changeset file for PR #8501 created/updated * Changeset file for PR #8501 created/updated * remove extra comment and add test for button Signed-off-by: tygao <[email protected]> * combine onChange Signed-off-by: tygao <[email protected]> * update table Signed-off-by: tygao <[email protected]> * test: add tests for table Signed-off-by: tygao <[email protected]> * address comments Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 98df4dd) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…) (#8528) * feat: add collaborator table * Changeset file for PR #8501 created/updated * Changeset file for PR #8501 created/updated * remove extra comment and add test for button * combine onChange * update table * test: add tests for table * address comments --------- (cherry picked from commit 98df4dd) Signed-off-by: tygao <[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>

github-actions bot added the all-star-contributor label Oct 5, 2024

opensearch-changeset-bot bot added a commit to raintygao/OpenSearch-Dashboards that referenced this pull request Oct 5, 2024

Changeset file for PR opensearch-project#8501 created/updated

800a841

raintygao changed the title ~~[Workspace]feat: add collaborator table to workspace list page~~ [Workspace]feat: add collaborator table to workspace detail page Oct 5, 2024

opensearch-changeset-bot bot added a commit to raintygao/OpenSearch-Dashboards that referenced this pull request Oct 5, 2024

Changeset file for PR opensearch-project#8501 created/updated

7ecdbad

SuZhou-Joe reviewed Oct 7, 2024

View reviewed changes

wanglam reviewed Oct 8, 2024

View reviewed changes

ruanyl added backport 2.x workspace labels Oct 8, 2024

ruanyl reviewed Oct 8, 2024

View reviewed changes

wanglam reviewed Oct 8, 2024

View reviewed changes

raintygao and others added 8 commits October 8, 2024 17:46

feat: add collaborator table

1c34a83

Signed-off-by: tygao <[email protected]>

Changeset file for PR opensearch-project#8501 created/updated

d0da576

Changeset file for PR opensearch-project#8501 created/updated

42b2b82

remove extra comment and add test for button

5ecf839

Signed-off-by: tygao <[email protected]>

combine onChange

38e6716

Signed-off-by: tygao <[email protected]>

update table

5770158

Signed-off-by: tygao <[email protected]>

test: add tests for table

1efb722

Signed-off-by: tygao <[email protected]>

address comments

fd17483

Signed-off-by: tygao <[email protected]>

raintygao force-pushed the coll branch from 5d53cc2 to fd17483 Compare October 8, 2024 09:46

SuZhou-Joe approved these changes Oct 9, 2024

View reviewed changes

Hailong-am approved these changes Oct 9, 2024

View reviewed changes

Hailong-am merged commit 98df4dd into opensearch-project:main Oct 9, 2024
68 checks passed

opensearch-trigger-bot bot mentioned this pull request Oct 9, 2024

[Backport 2.x] [Workspace]feat: add collaborator table to workspace detail page #8528

Merged

ananzh added the v2.18.0 label Oct 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Workspace]feat: add collaborator table to workspace detail page #8501

[Workspace]feat: add collaborator table to workspace detail page #8501

raintygao commented Oct 5, 2024 •

edited

Loading

codecov bot commented Oct 6, 2024 •

edited

Loading

SuZhou-Joe Oct 7, 2024

raintygao Oct 7, 2024 •

edited

Loading

SuZhou-Joe Oct 7, 2024

raintygao Oct 7, 2024

SuZhou-Joe Oct 7, 2024

raintygao Oct 7, 2024

SuZhou-Joe Oct 7, 2024

SuZhou-Joe Oct 7, 2024

raintygao Oct 7, 2024

wanglam Oct 8, 2024 •

edited

Loading

wanglam Oct 8, 2024

ruanyl Oct 8, 2024

ruanyl Oct 8, 2024

ruanyl Oct 8, 2024

wanglam Oct 8, 2024

wanglam Oct 8, 2024

		onChange(newPermissionSettings);
		handleSubmitPermissionSettings(newPermissionSettings as WorkspacePermissionSetting[]);

[Workspace]feat: add collaborator table to workspace detail page #8501

[Workspace]feat: add collaborator table to workspace detail page #8501

Conversation

raintygao commented Oct 5, 2024 • edited Loading

Description

Screenshot

Testing the changes

Changelog

Check List

codecov bot commented Oct 6, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

raintygao Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wanglam Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raintygao commented Oct 5, 2024 •

edited

Loading

codecov bot commented Oct 6, 2024 •

edited

Loading

raintygao Oct 7, 2024 •

edited

Loading

wanglam Oct 8, 2024 •

edited

Loading