-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Fix errors encountered in feature testing. #19100
Conversation
@kshitij01042002 Could you please provide more detail in point 3 of the description and also in the proof of changes? Thanks. |
Hi @seanlip I have updated the description PTAL. Thanks. |
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.
Thanks! Just one nit.
core/controllers/acl_decorators.py
Outdated
if new_dashboard_enabled and ( | ||
role_services | ||
.ACTION_ACCESS_NEW_CONTRIBUTOR_DASHBOARD_ADMIN_PAGE in ( | ||
self.user.actions | ||
)): |
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.
if new_dashboard_enabled and ( | |
role_services | |
.ACTION_ACCESS_NEW_CONTRIBUTOR_DASHBOARD_ADMIN_PAGE in ( | |
self.user.actions | |
)): | |
if new_dashboard_enabled and ( | |
role_services.ACTION_ACCESS_NEW_CONTRIBUTOR_DASHBOARD_ADMIN_PAGE | |
in self.user.actions | |
): |
@kshitij01042002 FYI the changes in #19108 might help fix the backend test failure in your PR. (See the description of that PR for details.) |
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.
Thanks for the changes @kshitij01042002 LGTM for the code owner file.
Hi @kshitij01042002, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks! |
I am merging this to unbreak develop. |
Overview
This PR fixes or fixes part of #[fill_in_number_here].
This PR does the following: This PR fixes the API access for the new contributor dashboard.
This error was recognixed during feature testing.
(For bug-fixing PRs only) The original bug occurred because: Used the API which had access to only super admins, so made the get API access open.
Because of introduction of new roles and new feature needed to update the access according to the feature flag.
Essential Checklist
followed by a short, clear summary of the changes.
comment with the phrase "@{{reviewer_username}} PTAL" if I don't have
permissions to assign reviewers directly).
Proof that changes are correct
Proof that question admins can access the old contributor admin dashboard when feature flag is disabled/ can't access the new contributor admin dashboard when feature flag is enabled and question coordinators can access the new contributor admin dashboard when feature flag is enabled.
Screencast.from.03-11-23.11.08.04.AM.IST.webm
No console errors in new contributor admin dashboard and only languages assigned to the coordinator are shown in the dropdown.
Screencast.from.03-11-23.11.12.14.AM.IST.webm
Proof of changes on desktop with slow/throttled network
Proof of changes on mobile phone
Proof of changes in Arabic language
PR Pointers