[Workspace] Jump to non-workspace url when clicking home icon

[SuZhou-Joe]

Description

Home page, which is supposed to be only visited without workspace, now is enabled to be accessed by clicking the home icon or enter the url manually in the browser.

This PR introduces a new field workspaceAccessibility inside App, which indicates the accessibility of the application based on workspace. This field can bring two benefits:

1. When navigate between apps, plugins and core application are using the navigateToApp method. By using the workspaceAccessibility field, workspace is able to do the hard navigation in a central place.
2. The workspaceAccessibility field can be used in workspace create page to filter applications that should not be visible inside workspace. For now we filter out home overview page and all the pages under management section by hard code, which is not elegant and not extensible.

Issues Resolved

opensearch-project#6362

Screenshot

Testing the changes

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-commenter]

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 44.41%. Comparing base (0f34d69) to head (00682b2).
Report is 9 commits behind head on workspace-pr-integr.

Files	Patch %	Lines
...rc/core/public/application/application_service.tsx	0.00%	7 Missing ⚠️
src/core/public/chrome/nav_links/to_nav_link.ts	0.00%	3 Missing ⚠️
src/core/public/core_system.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@                   Coverage Diff                   @@
##           workspace-pr-integr     #316      +/-   ##
=======================================================
+ Coverage                35.17%   44.41%   +9.24%     
=======================================================
  Files                     1885     2741     +856     
  Lines                    36421    54278   +17857     
  Branches                  6672     8543    +1871     
=======================================================
+ Hits                     12810    24109   +11299     
- Misses                   22761    28796    +6035     
- Partials                   850     1373     +523     

Flag	Coverage Δ
_1	32.80% <0.00%> (?)
_4	35.15% <0.00%> (-0.02%)	⬇️

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.

[ruanyl]

@SuZhou-Joe I'm getting "Not Found" page when server base path is enabled

[ruanyl]

I can see the anchor of home logo is still /mzt/w/management/app/home

I think toNavLink function should also be updated

[SuZhou-Joe]

@SuZhou-Joe I'm getting "Not Found" page when server base path is enabled

Sorry for the mistake. Fix for that.

[SuZhou-Joe]

How do you like the idea of adding a new field visibility on App? If it makes sense, I will add more unit test for this PR to make it prepared for merge.

@wanglam @ruanyl @gaobinlong @Hailong-am @raintygao @xluo-aws

[SuZhou-Joe]

I can see the anchor of home logo is still /mzt/w/management/app/home

I think toNavLink function should also be updated

Sure, done for that. But actually the home icon's link is coming from another place and fix for that as well.

[wanglam]

Since this property only be used for workspace. Shall we change to a boolean property, and renaming to something like workspaceVisible?

[SuZhou-Joe]

There should be 3 states: both, homeOnly and workspaceOnly. I have some concern that regarding undefined as both would introduce some confusion.

[SuZhou-Joe]

Though actually a link should be either homeOnly or workspaceOnly, I am afraid we can not force plugins to change the registration by 2.14 .

[wanglam]

Just a little concerned about there are too many visibility related property (status, navLinkStatus, chromeless) in the interface. Or we can just renaming to workspaceLess, then we just mark home plugin to true.

[SuZhou-Joe]

Yes, agree on that there are too many similar property. So I think maybe we could extend the status field by adding inaccessible-in-workspace and inaccessible-in-home, what do you think?

[ruanyl]

an app can only be accessed within a workspace

workspace update/overview page are two examples

I mean only two state is not enough, if using workspaceless which is boolean, it can only represent two state above.

[wanglam]

Is it necessary to mark the application as "an app can only be accessed within a workspace"? It will a little bit confused when workspace not enabled. For these pages, I think we still need to be displayed when workspace is turn off.

[Hailong-am]

I mean only two state is not enough

I see. do you think undefined/null, true, false will be three states and enough for our case? undefined/null same as current.

[SuZhou-Joe]

If we need to represent 3 states in a single field, an enum would be better instead of regarding undefined/null as a different state to false. This is a draft PR and we can adjust that.
What about:

• workspaceAccessibility.withinWorkspace -> only accessible in workspace, should be filter out from home in the future when workspace enabled, which means user won't be enable to create visualizations and dashboards out of workspace.
• workspaceAccessibility.outOfWorkspace -> only accessible out of workspace, can be used for pages like home/workspace list/workspace create page. Should be used in workspace-create/workspace-update pages to filter out inaccessible features.
• undefined -> as current to keep backward capability.

[ruanyl]

Is it necessary to mark the application as "an app can only be accessed within a workspace"? It will a little bit confused when workspace not enabled. For these pages, I think we still need to be displayed when workspace is turn off.

That's a good point. One way that I can think of is to check the workspace flag when plugin register the app, for example, via capabilities service. If workspace is turned off, plugin should not use this flag

[wanglam]

How do you like the idea of adding a new field visibility on App? If it makes sense, I will add more unit test for this PR to make it prepared for merge.

@wanglam @ruanyl @gaobinlong @Hailong-am @raintygao @xluo-aws

Shall we filter out features in workspace create and update page by the new field in this PR?

[SuZhou-Joe]

How do you like the idea of adding a new field visibility on App? If it makes sense, I will add more unit test for this PR to make it prepared for merge.
@wanglam @ruanyl @gaobinlong @Hailong-am @raintygao @xluo-aws

Shall we filter out features in workspace create and update page by the new field in this PR?

Will be raised in a following PR.

[Hailong-am]

do we need to add workspaceless flag to workspace create/list application?