-
Notifications
You must be signed in to change notification settings - Fork 894
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] use registered nav groups in workspace form #7221
[Workspace] use registered nav groups in workspace form #7221
Conversation
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
❌ Empty Changelog SectionThe Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7221 +/- ##
==========================================
+ Coverage 67.52% 67.53% +0.01%
==========================================
Files 3501 3502 +1
Lines 69340 69372 +32
Branches 11303 11311 +8
==========================================
+ Hits 46824 46853 +29
+ Misses 19764 19762 -2
- Partials 2752 2757 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
…red-nav-groups Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
…red-nav-groups Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
order, | ||
}); | ||
|
||
export const isEqualWorkspaceUseCase = (a: WorkspaceUseCase, b: WorkspaceUseCase) => { |
There was a problem hiding this comment.
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 if the use case from constant equal to the one from navGroup? I thought match on id should be good enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, the isEqualWorkspaceUseCase
only been used in UseCaseService
. It's just for avoid duplicate changes fired. The appUpdater$
in filterNavLinks
will generate new nav links. Then the getNavGroupsMap$
will emit new nav groups map. It will generate an infinite loop. So we add distinctUntilChanged
after getNavGroupsMap$
to avoid these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I c, thanks.
…red-nav-groups Signed-off-by: Lin Wang <[email protected]>
* Add registered use cases Signed-off-by: Lin Wang <[email protected]> * Separate use case service and fix UTs Signed-off-by: Lin Wang <[email protected]> * Add test case for workspace plugin Signed-off-by: Lin Wang <[email protected]> * Fix workspace unit tests Signed-off-by: Lin Wang <[email protected]> * Changeset file for PR #7221 created/updated * Remove workspaceConfigurableApp$ in component Signed-off-by: Lin Wang <[email protected]> * Remove no need workspaceConfigurableApps$ and add navGroupUpdater ut Signed-off-by: Lin Wang <[email protected]> * Fix type error Signed-off-by: Lin Wang <[email protected]> * Remove centered horizontal position Signed-off-by: Lin Wang <[email protected]> * Fix isDashboardAdmin in workspace creator unit tests Signed-off-by: Lin Wang <[email protected]> * Fix dynamic nav groups missing in workspace list 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 fceba91) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Add registered use cases * Separate use case service and fix UTs * Add test case for workspace plugin * Fix workspace unit tests * Changeset file for PR #7221 created/updated * Remove workspaceConfigurableApp$ in component * Remove no need workspaceConfigurableApps$ and add navGroupUpdater ut * Fix type error * Remove centered horizontal position * Fix isDashboardAdmin in workspace creator unit tests * Fix dynamic nav groups missing in workspace list --------- (cherry picked from commit fceba91) 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>
Description
In 2.15, we add the workspace use case selector in the workspace form. It used a fixed use cases list. In #7060 , we added a new concept nav group. The workspace form should use these registered nav groups as use cases and displayed in the workspace use case selector. This PR mainly includes below changes:
workspaceConfigurableApps$
Issues Resolved
#7222
Screenshot
No UI changes
Testing the changes
The nav groups have not been displayed in the navigation, so we need to use unit tests to verify all changes.
getRegisteredUseCases$
inside use case serviceCan refer src/plugins/workspace/public/services/use_case_service.test.ts
Can refer src/plugins/workspace/public/plugin.test.ts
Can refer below files:
src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx
src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx
src/plugins/workspace/public/components/workspace_overview/workspace_overview.test.tsx
Changelog
Check List
yarn test:jest
yarn test:jest_integration