[Workspace] Support feature quantities in workspace list page #6627

raintygao · 2024-04-24T03:22:45Z

Description

Support feature selection quantities in workspace list page

Screenshot

Changelog

feat: [Workspace] Support feature quantities in workspace list 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

SuZhou-Joe · 2024-04-24T03:27:13Z

src/plugins/workspace/public/components/workspace_list/index.test.tsx

+const defaultProps = ({
+  workspaceConfigurableApps$: of(undefined),
+} as unknown) as WorkspaceListProps;


Suggested change

const defaultProps = ({

workspaceConfigurableApps$: of(undefined),

} as unknown) as WorkspaceListProps;

const workspaceConfigurableApps: PublicAppInfo[] = [];

const defaultProps: WorkspaceListProps = {

workspaceConfigurableApps$: new BehaviorSubject(workspaceConfigurableApps),

};

Can we do this? The cast as WorksapaceListProps is not necessary and safe here.

The suggested change will also throw type error. I'm open to if we use type assertion in test case.

It seems unnecessary, we can just mock an object that matches the WorkspaceListProps.

WorkspaceListProps may have more field in the future and if we use type assertion, the type error will be swallowed.

So do you mean if there is type error in test file we can ignore? Seems your suggested change will throw type error.

No, I mean we should not use type assertion here. Could you please try the latest version of the suggestions? It should work without type error as long as you import type PublicAppInfo.

Sure, that's fine.

SuZhou-Joe · 2024-04-24T03:29:58Z

src/plugins/workspace/public/components/workspace_list/index.tsx

  const [queryInput, setQueryInput] = useState<string>('');
  const [pagination, setPagination] = useState({
    pageIndex: 0,
    pageSize: 5,
    pageSizeOptions: [5, 10, 20],
  });
  const [deletedWorkspace, setDeletedWorkspace] = useState<WorkspaceAttribute | null>(null);
+  const configurableApps = useObservable(props.workspaceConfigurableApps$ ?? of(undefined));


curious why don't we use of([]) as default value.

This is a missed change, updated.

codecov · 2024-04-24T03:37:33Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.49%. Comparing base (460428c) to head (a89f35c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6627      +/-   ##
==========================================
- Coverage   67.79%   67.49%   -0.30%     
==========================================
  Files        3413     2655     -758     
  Lines       66755    50623   -16132     
  Branches    10861     9170    -1691     
==========================================
- Hits        45254    34170   -11084     
+ Misses      18857    14206    -4651     
+ Partials     2644     2247     -397

Flag	Coverage Δ
Linux_1	`?`
Linux_2	`55.63% <ø> (ø)`
Linux_3	`45.26% <ø> (+0.01%)`	⬆️
Linux_4	`34.91% <ø> (ø)`
Windows_1	`?`
Windows_2	`55.59% <ø> (ø)`
Windows_3	`?`
Windows_4	`34.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.

github-actions · 2024-04-24T05:22:50Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "feat". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

SuZhou-Joe · 2024-04-24T05:24:12Z

@raintygao https://github.com/apps/opensearch-changeset-bot could you please install the app into your forked repo? I think this PR deserves a changelog.

github-actions · 2024-04-24T05:41:04Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "feat". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

wanglam · 2024-04-24T05:37:44Z

src/plugins/workspace/public/utils.ts

+  const featureFilter = featureMatchesConfig(featuresConfig);
+  const selectedApplications = configurableApps.filter((app) => featureFilter(app));
+  return {
+    total: configurableApps.length,


The DEFAULT_SELECTED_FEATURES_IDS is not exists in configurable apps. Shall we add them to the total and selected list?

No. Here we need to make features consistent with create page.

I think the featureConfig will includes these two feature ids, which means it may return {total: 0, selected: 2}. Do we need to filter out these two feature ids?

SelectedFeatures are filtered from workspaceConfigurableApps, workspaceConfigurableApps will filter out default selected features.

wanglam · 2024-04-24T05:41:04Z

src/plugins/workspace/public/components/workspace_list/index.tsx

@@ -106,6 +117,10 @@ export const WorkspaceList = () => {
      name: 'Features',
      isExpander: true,
      hasActions: true,
+      render: (features: string[]) => {
+        const { total, selected } = getSelectedFeatureQuantities(features, configurableApps || []);


Shall we render "-" for configurableApps is undefined or empty? 0/0 may looks strange.

I don't think so. Usually - represents null or missed data, 0/0 represent 0 features selected, I think that's how it should be.

kavilla · 2024-04-27T00:21:30Z

src/plugins/workspace/public/components/workspace_list/index.tsx

+  workspaceConfigurableApps$?: BehaviorSubject<PublicAppInfo[]>;
+}
+
+const WORKSPACE_LIST_PAGE_DESCRIPTION = i18n.translate('workspace.list.description', {
  defaultMessage:
    'Workspace allow you to save and organize library items, such as index patterns, visualizations, dashboards, saved searches, and share them with other OpenSearch Dashboards users. You can control which features are visible in each workspace, and which users and groups have read and write access to the library items in the workspace.',


nit: is it A workspace allows or Workspaces allow

kavilla

i think it would be cool to make the #/# a tool tip, and it displays the names of the selected apps

Signed-off-by: tygao <tygao@amazon.com>

raintygao · 2024-04-28T08:19:23Z

i think it would be cool to make the #/# a tool tip, and it displays the names of the selected apps

Thanks for your suggestion. That's cool. I will discuss this with designer.

raintygao · 2024-04-29T02:30:57Z

Hi @kavilla , could you help approve again? Seems your approval was dismissed because of my commit to update description.

BionIT · 2024-06-05T03:18:57Z

Hi @raintygao, is this PR targeting 2.15 or 2.16?

raintygao · 2024-06-05T09:29:40Z

Hi @raintygao, is this PR targeting 2.15 or 2.16?

Hi @BionIT , there will be some new UX design updates in this part, we convert this PR to draft for now.

raintygao requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, curq, bandinib-amzn, SuZhou-Joe, ruanyl, BionIT and xinruiba as code owners April 24, 2024 03:22

github-actions bot added valued-contributor Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Apr 24, 2024

SuZhou-Joe force-pushed the feat-list-quantity branch from 07c8db0 to fa9affc Compare April 24, 2024 03:24

SuZhou-Joe reviewed Apr 24, 2024

View reviewed changes

SuZhou-Joe changed the title ~~feat: Support feature quantities in workspace list page~~ [Workspace] Support feature quantities in workspace list page Apr 24, 2024

SuZhou-Joe added the workspace label Apr 24, 2024

github-actions bot added failed changeset and removed Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Apr 24, 2024

raintygao closed this Apr 24, 2024

raintygao reopened this Apr 24, 2024

wanglam reviewed Apr 24, 2024

View reviewed changes

raintygao force-pushed the feat-list-quantity branch from efa079a to 1007e63 Compare April 26, 2024 02:38

raintygao requested a review from SuZhou-Joe April 26, 2024 02:50

kavilla reviewed Apr 27, 2024

View reviewed changes

kavilla previously approved these changes Apr 27, 2024

View reviewed changes

raintygao dismissed kavilla’s stale review via e8ee342 April 28, 2024 08:15

raintygao added 3 commits April 28, 2024 16:16

feat: support feature quantities in workspace list page

18ec754

Signed-off-by: tygao <tygao@amazon.com>

update

d6f773a

Signed-off-by: tygao <tygao@amazon.com>

update workspace description

a89f35c

Signed-off-by: tygao <tygao@amazon.com>

raintygao force-pushed the feat-list-quantity branch from e8ee342 to a89f35c Compare April 28, 2024 08:16

SuZhou-Joe approved these changes Apr 29, 2024

View reviewed changes

yujin-emma assigned SuZhou-Joe May 28, 2024

SuZhou-Joe marked this pull request as draft June 5, 2024 02:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Workspace] Support feature quantities in workspace list page #6627

[Workspace] Support feature quantities in workspace list page #6627

raintygao commented Apr 24, 2024 •

edited by SuZhou-Joe

SuZhou-Joe Apr 24, 2024 •

edited

raintygao Apr 24, 2024

SuZhou-Joe Apr 24, 2024 •

edited

raintygao Apr 24, 2024

SuZhou-Joe Apr 24, 2024

raintygao Apr 24, 2024

SuZhou-Joe Apr 24, 2024 •

edited

raintygao Apr 24, 2024

codecov bot commented Apr 24, 2024 •

edited

github-actions bot commented Apr 24, 2024

SuZhou-Joe commented Apr 24, 2024

github-actions bot commented Apr 24, 2024

wanglam Apr 24, 2024

raintygao Apr 24, 2024

wanglam Apr 24, 2024

raintygao Apr 24, 2024

wanglam Apr 24, 2024

raintygao Apr 24, 2024

kavilla Apr 27, 2024

kavilla left a comment

raintygao commented Apr 28, 2024

raintygao commented Apr 29, 2024

BionIT commented Jun 5, 2024

raintygao commented Jun 5, 2024

[Workspace] Support feature quantities in workspace list page #6627

Are you sure you want to change the base?

[Workspace] Support feature quantities in workspace list page #6627

Conversation

raintygao commented Apr 24, 2024 • edited by SuZhou-Joe

Description

Screenshot

Changelog

Check List

SuZhou-Joe Apr 24, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuZhou-Joe Apr 24, 2024 • edited

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

SuZhou-Joe Apr 24, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 24, 2024 • edited

Codecov Report

github-actions bot commented Apr 24, 2024

❌ Invalid Prefix For Manual Changeset Creation

SuZhou-Joe commented Apr 24, 2024

github-actions bot commented Apr 24, 2024

❌ Invalid Prefix For Manual Changeset Creation

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

kavilla left a comment

Choose a reason for hiding this comment

raintygao commented Apr 28, 2024

raintygao commented Apr 29, 2024

BionIT commented Jun 5, 2024

raintygao commented Jun 5, 2024

raintygao commented Apr 24, 2024 •

edited by SuZhou-Joe

SuZhou-Joe Apr 24, 2024 •

edited

SuZhou-Joe Apr 24, 2024 •

edited

SuZhou-Joe Apr 24, 2024 •

edited

codecov bot commented Apr 24, 2024 •

edited