NotificationsYou must be signed in to change notification settings
Fork907
Star10k

fix: fix deployment settings navigation issues#16780

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 3 commits intomainfromlilac/patches

Mar 3, 2025

Merged

fix: fix deployment settings navigation issues#16780

aslilac merged 3 commits intomainfromlilac/patches

Mar 3, 2025

Conversation

Copy link

Member

aslilac commentedMar 3, 2025

As discussed, a minimal patch that fixes some issues discovered at the last hour of the Orgs GA work:

Admin settings > Healthcheck button not always visible when it should be
Admin settings > Deployment button not always visible when it should be
Admin settings > Deployment button can bring you to a page you don't have permissions to view when you do have permissions to view another page
Some pages under the /deployment settings would break because of a false dependency on the deploymentConfig query, which requires a high level of permissions
DeploymentSettingsLayout component may lock you out entirely, incorrectly, because of a bad permission check on the front end

aslilac added2 commits

March 3, 2025 18:47

fixin those bugs mhm

32ce6fb

conditional default page

aeb6711

aslilac requested a review fromjaaydenh

March 3, 2025 19:25

github-actionsbot assignedaslilac

Mar 3, 2025

Parkreiner self-requested a review

March 3, 2025 19:36

Parkreiner reviewed

Mar 3, 2025

View reviewed changes

Copy link

Member

Parkreiner left a comment

Choose a reason for hiding this comment

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

Did the initial review – going to spin up the branch and verify everything else

site/src/contexts/auth/permissions.tsx OutdatedShow resolvedHide resolved

site/src/modules/management/DeploymentSettingsLayout.tsx OutdatedShow resolvedHide resolved

site/e2e/tests/roles.spec.ts OutdatedShow resolvedHide resolved

feedback

88e3720

aslilac requested a review fromParkreiner

March 3, 2025 21:40

Copy link

Member

Parkreiner commentedMar 3, 2025•
edited
Loading

Notes from trying to kick the tires (tested using dev frontend and production backend):

When I try to select the Organization option in the dropdown as a template admin, I'm not directed to the Coder organization, but instead get moved to Danny's test org. This doesn't happen with an owner or user admin, and while it's not a huge deal, as the Coder org is still accessible, it does seem odd
I never got deep enough into permissions to know this, but there might still be a bug if a user who has access to all the deployment roles except Owner is expected to see the Healthcheck. They can't see the tab at all, but Owners still can
Trying to navigate directly to pages that aren't available in the dropdown seems to stop the user correctly. It seems like we just need to make sure that we apply the same UI approach to everything
Caught some nasty stying that creeped in somehow (the borders of the table randomly change color when near the roles)

Aside from the potential owner issue, these definitely aren't in scope. I think it should be safe for me to approve things, and then I can make tickets later