NotificationsYou must be signed in to change notification settings
Fork1.1k
Star11.6k

feat: manage groups from deployment settings for single-org deployments#14016

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

aslilac merged 8 commits intomainfromnon-org-groups

Jul 26, 2024

Merged

feat: manage groups from deployment settings for single-org deployments#14016

aslilac merged 8 commits intomainfromnon-org-groups

Jul 26, 2024

Conversation

Copy link

Member

aslilac commentedJul 25, 2024•
edited
Loading

Closes#13957

aslilac added2 commits

July 25, 2024 18:26

feat: manage non-multi-org groups from deployment settings

4c840c7

🧹

5346d8e

github-actionsbot assignedaslilac

Jul 25, 2024

aslilac changed the title~~feat: manage non-multi-org groups from deployment settings~~feat: manage groups from deployment settings for single-org deployments

Jul 25, 2024

of course

29dffc5

aslilac requested review fromBrunoQuaresma andcode-asher

July 25, 2024 21:04

code-asher reviewed

Jul 26, 2024

View reviewed changes

Copy link

Member

code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Just to make sure I understand "single-org" in this context, that is with the multi-org experiment enabled but not licensed, right?

Should we redirect/groups to/deployment/groups (or/organizations/default/groups/ for multi-org?) similar to the users page? Probably not very important, but if someone has a bookmark or tries typing it manually maybe.

I think the create group page needs to take me to/deployment/groups/groupname rather than/organizations/default/groups/groupname with single-org since the latter gives me a blank sidebar (maybe a good place for another relative redirect).

Looks good so far!

BrunoQuaresma reviewed

Jul 26, 2024

View reviewed changes

site/src/api/queries/groups.ts OutdatedShow resolvedHide resolved

BrunoQuaresma reviewed

Jul 26, 2024

View reviewed changes

site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.tsx


		exportconstGroupPage:FC=()=>{
		const{ organization, groupName}=useParams()as{
		const{ organization="default", groupName}=useParams()as{

Copy link

Contributor

BrunoQuaresmaJul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I see the constant "default" used in multiple places. We should define it in aconst.ts file for consistency. Makes sense?

Copy link

MemberAuthor

aslilacJul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

personally, I really dread adding imports like that which end up being more typing that the thing you could just say directly

Copy link

Contributor

BrunoQuaresma commentedJul 26, 2024•
edited
Loading

Just to make sure I understand "single-org" in this context, that is with the multi-org experiment enabled but not licensed, right?

@code-asher I think it is also true when there is only one org created.

Copy link

Contributor

BrunoQuaresma commentedJul 26, 2024

When I go to Deployment > Organizations, the sidebar is hidden. It looks strange to me; I would prefer to see it open.

It is not related to this particular PR, but I've been finding it hard to determine the currently active menu in the sidebar due to the numerous options available. Maybe we could style the active option differently?

I have a license and multi-org enabled, but I can't create new orgs. Perhaps it's related to 1?

BrunoQuaresma reviewed

Jul 26, 2024

View reviewed changes

Copy link

Contributor

BrunoQuaresma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm missing tests for the changes so we can be sure we are not introducing bugs while moving forward.

Copy link

MemberAuthor

aslilac commentedJul 26, 2024

I think it is also true when there is only one org created.

No, a deployment which only has one org created but has a multi-org license is still a multi-org deployment. We need to show the "New organization" button, for example which is only present in the multi-org version of the UI. (and maintaining a 3rd version of the UI that's half single-org and half multi-org would be a gigantic pain)

Copy link

MemberAuthor

aslilac commentedJul 26, 2024

I have a license and multi-org enabled, but I can't create new orgs.

there are two license types now, and old/enterprise licenses no longer have access to org features. you can get a "premium" one from notion to test with. :)