-
Notifications
You must be signed in to change notification settings - Fork 4
feat: [FC-0099] add filters and sorting functionality to the team table #8
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
feat: [FC-0099] add filters and sorting functionality to the team table #8
Conversation
|
Thanks for the pull request, @bra-i-am! 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. Where 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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8 +/- ##
==========================================
+ Coverage 92.44% 93.55% +1.11%
==========================================
Files 31 37 +6
Lines 463 605 +142
Branches 81 119 +38
==========================================
+ Hits 428 566 +138
- Misses 34 38 +4
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2e16508 to
6d9adf8
Compare
| manualSortBy | ||
| fetchData={debounce(handleTableFetch, 1000)} | ||
| data={rows} | ||
| itemCount={rows?.length} |
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.
Needs to be updated with the backend
| itemCount={rows?.length} | |
| itemCount={teamMembers?.count || 0} |
src/authz-module/data/api.ts
Outdated
| export interface QuerySettings { | ||
| roles: string | null; | ||
| search: string | null; | ||
| ordering: string | null; |
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.
| ordering: string | null; | |
| order: string | null; | |
| sortBy: string | null; | |
src/authz-module/data/api.ts
Outdated
| if (querySettings.ordering) { | ||
| url.searchParams.set('ordering', querySettings.ordering); | ||
| } |
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 (querySettings.ordering) { | |
| url.searchParams.set('ordering', querySettings.ordering); | |
| } | |
| if (querySettings.sortBy) { | |
| url.searchParams.set('sort_by', querySettings.sortBy); | |
| url.searchParams.set('order', querySettings.order); | |
| } |
src/authz-module/data/api.ts
Outdated
| url.searchParams.set('page', (querySettings.pageIndex + 1).toString()); | ||
|
|
||
| const { data } = await getAuthenticatedHttpClient().get(url); | ||
| return camelCaseObject(data.results); |
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.
We need to update the response we can access the total number of users
| return camelCaseObject(data.results); | |
| return camelCaseObject(data); |
|
I think is better if you group all the filters over a TeamTable folder, all of them are related to the same component and it is quite large : |
| > | ||
| <TableControlBar /> | ||
| <DataTable.Table /> | ||
| </DataTable> |
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.
The table controls are not visible, so we needo add the footer component
| > | |
| <TableControlBar /> | |
| <DataTable.Table /> | |
| </DataTable> | |
| > | |
| <TableControlBar /> | |
| <DataTable.Table /> | |
| <TableFooter /> | |
| </DataTable> |
467f8df to
9450e16
Compare
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.
I tested the filters by username/email, role sorting, and alphabetical sorting — everything seems to be working well!
That said, I noticed a small opportunity to improve the user experience. There’s a slight delay before the skeleton appears when changing the alphabetical sorting, filtering by role or username/email, and when clicking the “Clear filters” button. Occasionally, the skeleton doesn’t appear at all. I’ve attached a short video showing what I observed:
Grabacion.de.pantalla.2025-10-21.a.la.s.3.42.09.p.m.mov
Also, I have a quick question about a potential edge case:
if the admin wants to find and remove all the collaborator roles from an specific email domain so he set the filtering for that, after filters are applied he clicks on edit button for the first row and then removes the team member role, then returns to the table and the filters seem to reset. Is that the expected behavior, or should the filtering persist after returning?
98e07b5 to
5dcd6aa
Compare
Yes, that happen because the information is loaded from the cache instead of a request for the backend, but not sure how to improve that delay after the data is cached and printed in the table again.
there is not specification about persistence in the query cross views, I think for the MVP is not a priority but will be considered in future stages. |
5dcd6aa to
96225f4
Compare
brian-smith-tcril
left a comment
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.
Overall this looks great!
The components being added here look like they'd likely be useful to have in more places than just this MFE. It'd be awesome to see what it'd take to get them added to Paragon.
I left a few comments with questions.
Thank you so much for this PR!
| if (querySettings.sortBy && querySettings.order) { | ||
| url.searchParams.set('sort_by', querySettings.sortBy); | ||
| url.searchParams.set('order', querySettings.order); | ||
| } |
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.
Would there be a possible scenario where we'd have sortBy but not order? I know for the current sort options (Name A-Z, Name Z-A) we'll always have both so this isn't a blocking comment.
| queryKey: authzQueryKeys.teamMembers(scope, querySettings), | ||
| queryFn: () => getTeamMembers(scope, querySettings), | ||
| staleTime: 1000 * 60 * 30, // refetch after 30 minutes | ||
| refetchOnWindowFocus: false, |
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.
Just curious as to why this is being added. It doesn't seem like something that would be a sort/filter/search specific change.
src/authz-module/libraries-manager/components/TeamTable/components/SortDropdown.tsx
Outdated
Show resolved
Hide resolved
| [key: string]: Omit<SortOption, 'label'>; | ||
| } | ||
|
|
||
| const SORT_BY_OPTIONS: SortByOptions = { |
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.
Not blocking, but it'd be great to update this component to take in the SortByOptions and SORT_LABELS as props so it can be reused.
src/authz-module/libraries-manager/components/TeamTable/components/SortDropdown.tsx
Outdated
Show resolved
Hide resolved
src/authz-module/libraries-manager/components/TeamTable/index.tsx
Outdated
Show resolved
Hide resolved
… filtering and pagination
…utton functionality
57a7c3c to
0962368
Compare
brian-smith-tcril
left a comment
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.
LGTM!
I'm still curious about #8 (comment), but I don't feel like it's hurting anything (I'd really only be opposed to having refetchOnWindowFocus set to false for a page where we expect content to be updated rather frequently.
Warning
This PR depends on #5 for getting the roles and permissions metadata.
This pull request introduces filtering and sorting capabilities to the Team Members table.
Evidence
Screencast.from.15-10-25.16.32.43.webm
Testing instructions
npm run devGo to the Studio and create a library
Install the plugin https://github.com/openedx/openedx-authz by running
tutor dev exec lms pip install git+https://github.com/openedx/openedx-authzRun the migrations
tutor dev exec lms python manage.py lms migrate openedx_authzLoad the default policies by running
tutor dev exec lms python manage.py lms load_policiesGo to http://local.openedx.io:8000/api-docs/#/authz/authz_v1_roles_users_update and add roles to some users, following the structure
Additional Information