-
Notifications
You must be signed in to change notification settings - Fork 214
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
(feat) Allow registering workspace groups via routes.json #1256
Conversation
Size Change: -137 kB (-2.17%) Total Size: 6.19 MB
ℹ️ View Unchanged
|
569801e
to
378e4f6
Compare
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.
Basically LGTM, but a couple of questions / comments.
@@ -28,6 +29,14 @@ const workspaceRegistrationStore = createGlobalStore<WorkspaceRegistrationStore> | |||
workspaces: {}, | |||
}); | |||
|
|||
interface WorkspaceGroupRegistrationStore { | |||
workspaceGroups: Record<string, { name: string; members: Array<string> }>; |
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.
Why do we have both a key and a name property?
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.
Yes, the key and the name will be the same, but name is kept to get the whole workspace group definition whenever intended, instead of defining the name as the key again.
Hi @ibacher, |
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.
LGTM
Thanks @vasharma05
* Registering workspace groups similar to registration of individual workspaces * Update docs * Update tests * Using the workspace name instead of workspace group name * Allow workspace to define groups it can be a part of * Review changes
Requirements
feat
,fix
, orchore
, among others). See existing PR titles for inspiration.For changes to apps
If applicable
Summary
This PR allows workspace groups to be registered via routes.json, which is similar to how workspaces are registered. In the previous implementation of
launchWorkspaceGroup
in #1185, there was a design flaw, wherein, for a workspace to be part of a workspace group, it should define the respective group under its definition in theroutes.json
. Now, the workspace group registration will define the individual workspaces that will be themembers
of the respective workspace group, hence the individual workspace will not need to know what all workspace groups it is a part of.Screenshots
Yet to test out changes, since dev3 is down.
Related Issue
https://issues.openmrs.org/browse/O3-4077
Other