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

Add paging user list #328

Closed
wants to merge 0 commits into from
Closed

Add paging user list #328

wants to merge 0 commits into from

Conversation

myieye
Copy link
Contributor

@myieye myieye commented Oct 16, 2023

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.

@myieye myieye linked an issue Oct 16, 2023 that may be closed by this pull request
@myieye myieye requested a review from hahn-kev October 16, 2023 10:19
@github-actions
Copy link

github-actions bot commented Oct 16, 2023

UI unit Tests

1 tests  ±0   1 ✔️ ±0   0s ⏱️ ±0s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ±0 

Results for commit 728a667. ± Comparison against base commit 524de9f.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 16, 2023

C# Unit Tests

28 tests   28 ✔️  14s ⏱️
  6 suites    0 💤
  1 files      0

Results for commit 728a667.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@hahn-kev hahn-kev left a 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.

@myieye
Copy link
Contributor Author

myieye commented Oct 18, 2023

@hahn-kev ready for another review 😁

Copy link
Collaborator

@hahn-kev hahn-kev left a 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

frontend/src/routes/(authenticated)/admin/+page.ts Outdated Show resolved Hide resolved
@myieye myieye closed this Oct 19, 2023
@myieye myieye deleted the add-paging-user-list branch October 19, 2023 10:13
@myieye myieye temporarily deployed to staging October 19, 2023 10:14 — with GitHub Actions Inactive
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.

admin dashboard crashes server due to memory use with prod data loaded
2 participants