Skip to content

Next page token presence does not behave as documented #188

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

Open
radiozradioz opened this issue Mar 14, 2025 · 1 comment
Open

Next page token presence does not behave as documented #188

radiozradioz opened this issue Mar 14, 2025 · 1 comment
Labels
bug Something isn't working

Comments

@radiozradioz
Copy link

Describe the bug

The documentation says this:

| nextPageToken | string | A token, which can be sent as page_token to retrieve the next page. If this field is omitted, there are no subsequent pages. | No |

When I make a ListMemos request that will have no next page, e.g. curl https://memos.example.com/api/v2/memos?pageSize=50&filter=row_status%20%3D%3D%20'NORMAL' , the returned page token is an empty string rather than being absent:

{..."resources":[], "relations":[], "reactions":[]}], "nextPageToken":""}

The documentation implies that the field will be absent. This is incongruent.

Personally I think the fix should either be ommitting the field or setting it to null when there is no extra page. Empty string is not an acceptable stand-in for null.

Steps to reproduce

Run a ListMemos query with a page size that is higher than the number of memos in your instance.

The version of Memos you're using

v0.24.1

Screenshots or additional context

No response

@radiozradioz radiozradioz added the bug Something isn't working label Mar 14, 2025
@johnnyjoygh johnnyjoygh transferred this issue from usememos/memos Mar 16, 2025
@radiozradioz
Copy link
Author

In usememos/memos#4514 , commenters noted that the field should not be optional according to Google's AIP. The PR was then closed under the assumption that the nullability field was irrelevant. I see that this issue has been moved to usememos/dotcom, implying that the intended fix for this situation is to change the documentation.

Personally I think discussing the nullability of the field is relevant. Indeed the documentation should be updated, but I also think the current behavior of the API is undesirable. I think that an empty string is not a good substitute/representation for null, and that the API should be updated to return null here instead of an empty string.

Unless it's an express "no" for changing the field behavior, I suggest we put the issue back into usememos/memos because it encompasses a functional change to the server, not just a change to the documentation.

Here is my comment in the PR with my reasoning: usememos/memos#4514 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant