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

Implement pagination with 'continue' to load all linked pages #6068

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bhushan354
Copy link
Contributor

@bhushan354 bhushan354 commented Dec 18, 2024

NOTE: Please review the pull request process before opening your first PR: https://github.com/WikiEducationFoundation/WikiEduDashboard/blob/master/CONTRIBUTING.md#pull-request-process

What this PR does

fixes #5588

Screenshots

Before:

After:
< add a screenshot of the UI after your change >

Open questions and concerns

< anything you learned that you want to share, or questions you're wondering about related to this PR >

@bhushan354
Copy link
Contributor Author

bhushan354 commented Dec 18, 2024

Screenshot from 2024-12-18 14-08-58

Screenshot from 2024-12-18 14-10-52

  • only have one slug editing-basics but this is the message i am getting.

also i have a few questions regarding the Wiki API and pagination:

1 - does the Wiki API always return a continue key when there are more than 500 linked pages? If not, how is pagination handled? This PR assumes that theres an api key continue.
2 - i used Postman to check for the presence of a continue key in the API response but was not able to get the output. how can I better inspect the API response or debug it? Are there any tools or methods you'd recommend for troubleshooting?
3 - Can you suggest any improvements for error handling or pagination logic in the current implementation?

Thanks !

@bhushan354
Copy link
Contributor Author

@ragesoss can you review this ?

@ragesoss
Copy link
Member

Thanks for the ping. It's on my todo list.

@ragesoss
Copy link
Member

ragesoss commented Jan 7, 2025

You'll definitely want to implement a test for this. I think the API always includes a continue param of some sort, if not all the results in included in the response. The limit won't necessarily be 500; that depends on a mediawiki setting that might change, as well as the identify of the client making the request (as making a request while logged in with some accounts will allow for larger limits). But I think 500 is the current limit for a logged-0ut request (which is what the dashboard uses).

I think the slides index currently has more than 500 links, so you can write test around that: https://meta.wikimedia.org/wiki/Training_modules/dashboard/slides

I suggest making a dedicated method to do the link fetching, so that you can test that in isolation, without needing to do anything with training records in the test.

@ragesoss ragesoss marked this pull request as draft January 7, 2025 22:28
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.

Fix 500-link limit for loading training slides from wiki
2 participants