-
Notifications
You must be signed in to change notification settings - Fork 884
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]Add "All use case" option to workspace form #7318
[Workspace]Add "All use case" option to workspace form #7318
Conversation
Signed-off-by: Lin Wang <wonglam@amazon.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7318 +/- ##
==========================================
- Coverage 67.53% 67.53% -0.01%
==========================================
Files 3504 3504
Lines 69412 69408 -4
Branches 11324 11325 +1
==========================================
- Hits 46879 46872 -7
- Misses 19773 19776 +3
Partials 2760 2760
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
useCaseId === ALL_USE_CASE_ID | ||
? useCases.reduce<string[]>((previous, { features }) => previous.concat(features ?? []), []) | ||
: useCases.find(({ id }) => id === useCaseId)?.features ?? []; | ||
return availableFeatures.includes(featureId); |
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.
if an app have not register to any use case, will it be visible/accesable in all use case?
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.
It will.
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.
Based current implementation, the all use case will match all features inside registered use cases. If an app not register to any use case, it still can be accessible if it match below conditions:
workspaceAvailability
isoutsideWorkspace
navLinkStatus
equalhidden
chromeless
equaltrue
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 example, security analytics plugin have not finished the nav change, based on above conditions, it will be inaccessible when workspace is all use case. will that be an issue?
@@ -122,6 +124,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> | |||
if ( | |||
navGroup.type !== NavGroupType.SYSTEM && | |||
currentWorkspace.features && | |||
getFirstUseCaseOfFeatureConfigs(currentWorkspace.features) !== ALL_USE_CASE_ID && |
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.
Would be nice to have a comment on this logic
import { WorkspaceUseCase as WorkspaceUseCaseObject } from '../../types'; | ||
import { WorkspaceFormErrors } from './types'; | ||
import './workspace_use_case.scss'; | ||
|
||
import './workspace_use_case.scss'; |
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.
Duplicate I suppose?
useCaseId === ALL_USE_CASE_ID | ||
? useCases.reduce<string[]>((previous, { features }) => previous.concat(features ?? []), []) | ||
: useCases.find(({ id }) => id === useCaseId)?.features ?? []; | ||
return availableFeatures.includes(featureId); |
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.
It will.
return useCase.features.includes(featureId); | ||
} | ||
return false; | ||
const availableFeatures = |
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 thought we will just bypass all the features in all use case. Why do we change that?
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 you mean we don't need to check whether the application id exists in registered use cases? We add skip the feature verification logic outside for all use case workspace.
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
The backport to
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-7318-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5741fd7c262f4ace7d63dbc18c9549f8d35a92f7
# Push it to GitHub
git push --set-upstream origin backport/backport-7318-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 |
* Add all use case option Signed-off-by: Lin Wang <wonglam@amazon.com> * Changeset file for PR #7318 created/updated * Changeset file for PR #7318 created/updated * Changeset file for PR #7318 created/updated * Fix use case selection Signed-off-by: Lin Wang <wonglam@amazon.com> * All use case can accessible any app and add comment Signed-off-by: Lin Wang <wonglam@amazon.com> * Remove duplicate style import Signed-off-by: Lin Wang <wonglam@amazon.com> --------- Signed-off-by: Lin Wang <wonglam@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 5741fd7) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Add all use case option * Changeset file for PR #7318 created/updated * Changeset file for PR #7318 created/updated * Changeset file for PR #7318 created/updated * Fix use case selection * All use case can accessible any app and add comment * Remove duplicate style import --------- (cherry picked from commit 5741fd7) Signed-off-by: Lin Wang <wonglam@amazon.com> 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
The use case of workspace can only be one specific use case or all use case. This PR mainly includes below changes:
Issues Resolved
#7319
Screenshot
Testing the changes
yarn osd boostrap
opensearch_dashboards.yml
yarn start --no-base-path
and login with admin user(The "all use case" will be selected and the left navigation will changed to all style)
Changelog
Check List
yarn test:jest
yarn test:jest_integration