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 to the GetBannedPersons fetch. #2847

Open
dessalines opened this issue May 9, 2023 · 6 comments · May be fixed by #5428
Open

Add paging to the GetBannedPersons fetch. #2847

dessalines opened this issue May 9, 2023 · 6 comments · May be fixed by #5428
Labels
area: api enhancement New feature or request extra: good first issue Good for newcomers
Milestone

Comments

@dessalines
Copy link
Member

Currently GetBannedPersons fetches all banned users. It would be more performant if paging were added to this endpoint.

Not a huge deal, since this is only available to admins anyway.

@Nutomic Nutomic added the extra: good first issue Good for newcomers label Sep 20, 2024
@Nutomic
Copy link
Member

Nutomic commented Sep 20, 2024

See #4580 for an example how to implement this.

@dessalines
Copy link
Member Author

Also, we'd like to convert every existing page and limit to cursor pagination, like list_posts . But I'd need @dullbananas expertise on how to do that correctly.

@dessalines dessalines added this to the 1.0.0 milestone Feb 12, 2025
manuc66 added a commit to manuc66/lemmy that referenced this issue Feb 12, 2025
@Nutomic
Copy link
Member

Nutomic commented Feb 13, 2025

When doing this I would change the endpoint to /api/v4/person/list with parameter banned_only. It only requires slightly more work, fulfills the same purpose and can later be extended with more optional parameters.

@dessalines
Copy link
Member Author

dessalines commented Feb 14, 2025

Hrm. So far the only two person fetches are listing admins, and listing banned persons, and only banned_persons is its own endpoint. Search can also list persons, but it already has pagination.

I think this is one case where I'd prefer specific endpoints, rather than a multi-purpose generic one, because we only have one case of person fetching, and /admin/banned is pretty clear.

I will make person_view.rs a bit more generic tho now, by adding a PersonQuery, with admin_only, banned_only, and pagination.

dessalines added a commit that referenced this issue Feb 14, 2025
@dessalines dessalines linked a pull request Feb 14, 2025 that will close this issue
@Nutomic
Copy link
Member

Nutomic commented Feb 17, 2025

I dont see any reason why banned users should be a separate endpoint, its better to do like GetPosts with various filters (banned_only, local_only etc). This has been frequently requested (eg #4419) and would be very useful for small instances. Other frontends could already implement a full user list display with that basic endpoint, and later it could be extended with more filters and sort options. Afaik lemmy-ui only shows the banned users list under /admin which would work just as well with a general user list and banned_only filter.

@dessalines
Copy link
Member Author

Alright, as long as its under the admin heading, that'll probably be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api enhancement New feature or request extra: good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants