-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add paging user list #328
Add paging user list #328
Conversation
820df07
to
2f6ea43
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.
Great work Tim, nice job on the icontains filter especially.
The only major thing is the debounce of the user filter, otherwise it looks good. We should make a separate issue to display a spinner in the user search bar while results are loading, otherwise it might look like the search isn't doing anything.
backend/LexBoxApi/GraphQL/CustomFilters/CustomFilterOperations.cs
Outdated
Show resolved
Hide resolved
backend/LexBoxApi/GraphQL/CustomFilters/QueryableStringDeterministicInvariantContainsHandler.cs
Outdated
Show resolved
Hide resolved
backend/LexBoxApi/GraphQL/CustomFilters/QueryableStringDeterministicInvariantContainsHandler.cs
Outdated
Show resolved
Hide resolved
backend/LexBoxApi/GraphQL/CustomFilters/QueryableStringDeterministicInvariantContainsHandler.cs
Outdated
Show resolved
Hide resolved
8a873ac
to
728a667
Compare
@hahn-kev ready for another review 😁 |
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.
Looks good! Merge when you're ready
728a667
to
18638be
Compare
Resolves #319
Adds server-side paging to the admin user-table to prevent memory problems.
Additionally filtering projects by user-membership is now done in the DB with GQL, so we don't have an N-projects*M-users on the server or client.
Only return explicitly requested query-params prevented the new user-search query-param from landing in the project filter bar. In other scenarios we could pick the required params/props higher up, but this seems like a good default behaviour anyway and fixes the problem.