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 keyset pagination for users ordered by username / display_name #851

Merged
merged 4 commits into from Jan 20, 2023

Conversation

andreaskoepf
Copy link
Collaborator

@andreaskoepf andreaskoepf commented Jan 19, 2023

for keyset pagination pass lowest or highest values of last result-set:
gt_id & gte_username or
lt_id & lte_username

/api/v1/users/by_username

all parameters are optional:

api_client_id: str
gte_username: str
gt_id: uuid
lte_username: str
lt_id: uuid
auth_method: str
search_text: str
max_count: int

/api/v1/users/by_display_name

all parameters are optional:

api_client_id: str
gte_display_name: str
gt_id: uuid
lte_display_name: str
lt_id: uuid
auth_method: str
search_text: str
max_count: int

@andreaskoepf andreaskoepf requested a review from yk as a code owner January 19, 2023 18:35
@notmd
Copy link
Collaborator

notmd commented Jan 20, 2023

Could this simplify to 1 endpoint and add an extra sort_by column and make default to username or display_name? Also, unify all gt_* and lt_* param into gt and lt too. The back end should return 2 extra fields, next and previous, which are the cursors encoded or encrypted by the backend itself. I'm feeling the front currently needs to specify the param and endpoint just to get users which is too much work here. Let's push more work to the backend instead.

Copy link
Collaborator

@yk yk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

@andreaskoepf andreaskoepf merged commit 70fc80a into main Jan 20, 2023
@andreaskoepf andreaskoepf deleted the user_keyset_pagination branch January 20, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants