Skip to content
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

[RFC][Workspace]Add workspace id into url to indicate current visiting workspace #5243

Closed
SuZhou-Joe opened this issue Oct 7, 2023 · 4 comments
Labels
enhancement New feature or request workspace

Comments

@SuZhou-Joe
Copy link
Member

SuZhou-Joe commented Oct 7, 2023

Is your feature request related to a problem? Please describe.

In Trinity project, we are gonna to introduce a new concept: Workspace as a place to organize saved objects and features. And workspace will be a new type of saved object which has its own id and attributes. As workspace will be a new place to separate saved objects and features, we need to store current workspaceId in a place. And this issue will used mainly to discuss the options we can take and their pros and cons.

Describe the solution you'd like

Option 1: store current workspaceId into cookie or sessionStorage.

  • description:
    • We can store a key-value pair inside cookie or sessionStorage to indicate current workspaceId, in cookie it looks like document.cookie = '&CURRENT_WORKSPACE_ID={currentWorkspaceId}&' while in sessionStorage in looks like { CURRENT_WORKSPACE_ID: {currentWorkspaceId} }.
  • pros
      1. Easy to implement.
      1. Has little effect on current system.
  • cons
      1. The url is not shareable. Imaging that a user is visiting a dashboard page under workspaceA, when user tries to copy the url and paste the url to another user, current workspaceId information will be missed as the workspace information is stored in a url-non-related place.
      1. If we storage the current workspaceId info into cookie, a user can not visit multiple workspaces in a single browser, it is not user-friendly.

Option 2: store current workspaceId into query or hash. (Use Option 4 if we choose option 2)

  • description:
    • We can store a key-value pair inside hash to indicate current workspaceId. Let's use dashboard for example: /app/dashboards/#/view/{dashboardId}?CURRENT_WORKSPACE_ID={currentWorkspaceId}.
  • pros
      1. Has little effect on current system.
      1. The url is shareable as current workspaceId is a part of url.
  • cons
      1. If we storage the current workspaceId info into query, when user jump from a page with workspace information to a page, the redirect will refresh the page as the query inside target url(without current workspace id) is not the same as the query inside original url(with current workspace), it will totally change the redirect behavior on OSD.
      1. If we storage the current workspaceId info into hash, many plugins are using hash as their routes and the hash will be overwritten by some plugins, in order to keep the current workspaceId in url consistent with the one in memory, we need to setup many listeners and rewrite the hash when we detect there is a overwrite happens. The link contains essential code changes. And you will find that the code change has many listeners and patch to keep url and memory consistent, which is difficult to maintain in the long term.
      1. Many edge cases we need to care about:

Option 3: store current workspaceId into path(preferred)

  • description:
    • We can store a key-value pair inside path to indicate current workspaceId. Let's use dashboard for example: /w/{currentWorkspaceId}/app/dashboards/#/view/{dashboardId}.
  • pros
      1. Because the workspace will manage saved objects and features in the future, placing the current workspaceId at the beginning of the application path is also more natural and semantically correct.
      1. The url is shareable as current workspaceId is a part of url.
      1. We can get rid of all the hacky listeners in option 2.
      1. workspace info is intuitive as it is in path level.
  • cons
      1. The implementation will have some impact on basePath and core module.
  • implementation

Option 4: store current workspaceId by using data_persistence service

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

Additional context
#5075

@SuZhou-Joe SuZhou-Joe added the enhancement New feature or request label Oct 7, 2023
@xluo-aws
Copy link
Member

xluo-aws commented Oct 7, 2023

#5075 for additional context @kavilla

@SuZhou-Joe
Copy link
Member Author

#4944

@SuZhou-Joe
Copy link
Member Author

@kavilla , here are answers to your questions.

OSD starts up and discovers what plugins that are installed. It then takes the bundles for plugins and pushes under url/app/pluginName. So the current navigation system will have to be reworked. Because it will depend on url/app/pluginName to make the appropriate HTTP calls to download the bundles.

For the bundles, bundles' path are consist of /{serverBasePath}/{bundleHash}/bundles/plugins/xxx.bundle.js, our solution won't add prefix on serverBasePath so bundles' path won't be affected.

History will have to be considered as well. Parts of the application and external plugins can be relying on the URL to handle history so if only parts of the application have the workspace route then they can get in a bad state.

Original url can be visited without workspaceId inside path. The workspaceId only affect pages that are related to saved objects. For example, saved objects management page. User can only see objects inside workspaceA when the url is like /w/${currentWorkspaceId}/app/objects while can see all the objects he/she has permission to when visiting /app/objects.

State management is managed by the URL via _a (for app) and _g so this might be impacted and you will have to modify existing OSD URL storage functions as some functions might be slicing up the route based on url/{base_path}/app/plugin

In browser side, /w/{currentWorkspaceId} is part of {base_path}, so there is no difference for state management.

APIs get hosted under url/api/pluginName

Indeed, so we will have a rewrite in workspace plugins to redirect all the traffic from {basePath}/w/{workspaceId}{osdPath*} paths to {basePath}{osdPath*}

@kavilla
Copy link
Member

kavilla commented Nov 1, 2023

Apologies on the delay will check on the example implementation later. One of the biggest worries I have is that the current ecosystem is incredibly flexible.

Would the second point be similar to adding a new state? That we can use _w? Seems like it would be similar to app state and global state. Or consider adding to the workspace id to those states?

For the sake of time would you be able to showcase screen shots of your implementation and playing with the application? Adding the sample data sets and navigating to each of these dashboards. Do filters properly work? Do editting and expanding work fine? Saving work fine? Workspace to workspace? State management and rendering within the application on the client side can be easy to break functionality not tracked in tests as we see with security fixes that impacted rendering.

With 3, we might end up implementation support of this but then end up impacting downstream dependencies. It could also fundamentally change designs if it ends up being a breaking change that can't be backported to 2.x if it needed to be. I know some users do redirects. Technically, I could set my base path to "w" so that the app starts like "localhost:5601/w/app". But now I'm just being the devil's advocate. However, for me it seems like option 3 is a one way door and might be a slower approval and debate whereas the other options are 2 way doors that can lead to implementing 3 eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request workspace
Projects
None yet
Development

No branches or pull requests

4 participants