Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 3, 2025
Merged

fix: fix deployment settings navigation issues #16780

merged 3 commits into from
Mar 3, 2025

Conversation

aslilac
Copy link
Member

@aslilac aslilac commented Mar 3, 2025

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

  1. Admin settings > Healthcheck button not always visible when it should be
  2. Admin settings > Deployment button not always visible when it should be
  3. 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
  4. Some pages under the /deployment settings would break because of a false dependency on the deploymentConfig query, which requires a high level of permissions
  5. DeploymentSettingsLayout component may lock you out entirely, incorrectly, because of a bad permission check on the front end

@aslilac aslilac requested a review from jaaydenh March 3, 2025 19:25
@Parkreiner Parkreiner self-requested a review March 3, 2025 19:36
Copy link
Member

@Parkreiner 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

@aslilac aslilac requested a review from Parkreiner March 3, 2025 21:40
@Parkreiner
Copy link
Member

Parkreiner commented Mar 3, 2025

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)
Screenshot 2025-03-03 at 5 03 16 PM

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

Copy link
Member

@Parkreiner Parkreiner left a comment

Choose a reason for hiding this comment

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

Approving tentatively, since I didn't find anything major

@aslilac aslilac merged commit 17ad284 into main Mar 3, 2025
30 checks passed
@aslilac aslilac deleted the lilac/patches branch March 3, 2025 22:48
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2025
@stirby
Copy link
Collaborator

stirby commented Mar 3, 2025

Thank you @aslilac @Parkreiner

@stirby
Copy link
Collaborator

stirby commented Mar 3, 2025

/cherry-pick release/2.20

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants