feat(authz): implement filters and sorting on audit user page#110
feat(authz): implement filters and sorting on audit user page#110jacobo-dominguez-wgu merged 28 commits intoopenedx:masterfrom
Conversation
|
Thanks for the pull request, @jacobo-dominguez-wgu! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
3be3af0 to
798b19d
Compare
92a07c5 to
3ba621c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #110 +/- ##
==========================================
+ Coverage 96.20% 96.70% +0.50%
==========================================
Files 81 81
Lines 1869 1880 +11
Branches 413 418 +5
==========================================
+ Hits 1798 1818 +20
+ Misses 68 59 -9
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
57714f2 to
2e03479
Compare
|
I have updated this PR, @jacobo-dominguez-wgu could you please remove the draft status on this PR so Diana can review it? |
| path={ROUTES.AUDIT_USER_PATH} | ||
| element={<AuditUserPage />} | ||
| /> | ||
| <Route path="*" element={<NotFoundError />} /> |
There was a problem hiding this comment.
This is a duplication, is'n it?
I see the same at line 37
There was a problem hiding this comment.
This is a duplication, is'n it? I see the same at line 37
ay yeah you are right, I've updated this already. Thanks
There was a problem hiding this comment.
Same comment as previous PRs about
There was a problem hiding this comment.
Same comment as previous PRs about
you mean, this one. right?
I'm not against introducing this test but I don't think it will introduce much value either.
I explain better, the functions usually call getAuthenticatedHttpClient and return a camel cased value, the logic is simple.
In addition, the hooks test follows the same pattern as here, mocking getAuthenticatedHttpClient, with the idea to cover the api during the test coverage. because of that, I prefer modify them if we find improvements than creating a solo api test suite.
rodmgwgu
left a comment
There was a problem hiding this comment.
Did a high level test on my local and functionality looks good, there is just one wrong filter in the Roles filters, that also appears in the Team Members page:
The "Library Collaborator" should be "Library Contributor" (also the filter sent to the query is wrong for this one)
Thank you Rodrigo, for your feedback. this has been fixed! |
rodmgwgu
left a comment
There was a problem hiding this comment.
Thanks for this work, just found some things that need changes.
| const roleData = permissionsList.find(item => item.role === role); | ||
| return roleData ? roleData.permissions.length : 0; | ||
| */ | ||
| const count = Math.floor(Math.random() * 50); |
There was a problem hiding this comment.
Actually I see that this is a placeholder, can we remove it?
There was a problem hiding this comment.
Removed.
| variant="reduced" | ||
| currentPage={pageIndex + 1} | ||
| pageCount={pageCount} | ||
| paginationLabel="Table pagination" |
There was a problem hiding this comment.
Should we use formatMessage here for i18n?
5b72c81 to
b754ce1
Compare
@jacobo-dominguez-wgu @jesusbalderramawgu This issue seems to be related to breakpoints, I only see it on smaller screens:
|
…vements on the header
66a8195 to
82211dc
Compare
|
I have solved the conflicts, made the rebase and fix a minor things. |
rodmgwgu
left a comment
There was a problem hiding this comment.
Thanks! I verified and the filter menu overflow issue is fixed




Description
Adding filtering and sorting to the roles table on audit user page.
Adds the functionality to filter by Organization and Role and sorting by Role, Organization, and Scope A to Z and Z to A.
It closes #88.
Unit tests are pending and also some TODOs to be solved when the API is integrated.✅Warning
Will be on draft status until the required endpoints are completed.This PR requires the endpoints proposed in the section
"Proposed Endpoint for M2"How to test it
Pre - requisites
1 - Go to the new audit user page ->
http://apps.local.openedx.io:2025/admin-console/authz/user/:username(select a user with many roles assigned to get a more populated table) or go tohttp://apps.local.openedx.io:2025/admin-console/authz/and click on the Eye Icon button on one of the rows of the team members table.2 - Verify filters for orgs and roles and sorting works properly.
3 - Validate all the data loads properly, the UI matches the design and acceptance criteria is fulfilled.
NOTE: Expand all permissions and delete functionality are on different prs.
NOTE 2: The "Assign Role" button redirects to the new role assignation wizard (in development here #109 and #111).