Skip to content

Add pagination for admin interface #3808

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

Merged
merged 2 commits into from
May 28, 2025
Merged

Add pagination for admin interface #3808

merged 2 commits into from
May 28, 2025

Conversation

provokateurin
Copy link
Member

No description provided.

@provokateurin provokateurin added this to the Nextcloud 32 milestone May 26, 2025
@provokateurin provokateurin added the 2. developing Items that are currently under development label May 26, 2025
@susnux susnux added 3. to review Items that need to be reviewed and removed 2. developing Items that are currently under development labels May 26, 2025
@susnux susnux marked this pull request as ready for review May 26, 2025 11:44
@provokateurin
Copy link
Member Author

@susnux can you please pull all occurrences of the number of items per page into a constant so we can easily adjust it later instead of having to change 20 lines and potentially miss some.

@susnux susnux force-pushed the feat/pagination branch from adcda70 to a431390 Compare May 28, 2025 07:23
@susnux
Copy link
Contributor

susnux commented May 28, 2025

@provokateurin done

@zak39
Copy link
Contributor

zak39 commented May 28, 2025

Hi everyone,

I would like to share my feedback.

Currently, you are paginating the results related to the getAllFoldersWithSize function without passing a limit as a parameter to this function.

As a result, the query time to display 30 folders will be the same as that to display 3000 folders, in my opinion.

It might be beneficial to define both the offset and limit as parameters in the getAllFoldersWithSize  function, don’t you think? 🤔

@provokateurin
Copy link
Member Author

@zak39 thanks for your feedback. I agree it would be much more efficient to let the DB handle the pagination, but the problem is that the controller method has some more logic to filter entries and I'm not sure if it is even possible or at least feasible to move all that into the query (which could also hurt performance a lot).

The reason we are adding this is that the frontend is not capable handling so many Team folders at once. I originally suggested that we might do pagination frontend-only, as the backend size and network requests are not known to cause problems when there are many Team folders.

I think this solution is good enough for now and if there are more performance concerns in the future then we can look at it again. As long as the frontend is unusable with many Team folders, it's hard to say if the backend is any problem at all.

provokateurin and others added 2 commits May 28, 2025 11:24
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the feat/pagination branch from ab3844b to 31647c4 Compare May 28, 2025 09:24
@susnux susnux self-requested a review May 28, 2025 09:24
@provokateurin provokateurin enabled auto-merge May 28, 2025 09:36
@provokateurin provokateurin merged commit bf96030 into master May 28, 2025
50 checks passed
@provokateurin provokateurin deleted the feat/pagination branch May 28, 2025 09:47
@provokateurin
Copy link
Member Author

/backport to stable31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Items that need to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants