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

[CPDNPQ-2076] Admin Management #1781

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[CPDNPQ-2076] Admin Management #1781

wants to merge 3 commits into from

Conversation

rwrrll
Copy link
Contributor

@rwrrll rwrrll commented Oct 4, 2024

Context

Ticket: https://dfedigital.atlassian.net/browse/CPDNPQ-2076

Implement admin management currently at /admin/admins as new version at /npq-separation/admin/admins

Copy link

github-actions bot commented Oct 4, 2024

@rwrrll rwrrll changed the title 2076 Admin Management [CPDNPQ-2076] Admin Management Oct 4, 2024
Copy link

sonarcloud bot commented Oct 4, 2024

@rwrrll rwrrll marked this pull request as ready for review October 4, 2024 15:24
@rwrrll rwrrll requested a review from a team as a code owner October 4, 2024 15:24
@@ -244,7 +244,7 @@
end

resources :lead_providers, only: %i[index show], path: "lead-providers"
resources :admins, only: %i[index]
resources :admins, except: :edit
Copy link
Member

@peteryates peteryates Oct 4, 2024

Choose a reason for hiding this comment

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

I think referring to 'admins' inside the admin section could possibly be a bit confusing. As these people will be DfE staff, maybe /admin/staff?

Suggested change
resources :admins, except: :edit
resources :admins, except: :edit, path: "staff"

@peteryates
Copy link
Member

Looking good so far. I left a suggestion on the naming as having an 'admins' section inside the admin section feels like it could be a bit confusing. Perhaps referring to DfE people as 'staff' would make things easier? (there's a fair chance that could just make everything more confusing - worth having a chat with the team).

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

Successfully merging this pull request may close these issues.

2 participants