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

Remove pagination for departments #2501

Merged
merged 3 commits into from
Dec 23, 2024
Merged

Remove pagination for departments #2501

merged 3 commits into from
Dec 23, 2024

Conversation

annagav
Copy link
Contributor

@annagav annagav commented Dec 23, 2024

What are the relevant tickets?

Fix https://github.com/mitodl/hq/issues/6308

Description (What does it do?)

Remove pagination for departments api

How can this be tested?

Create over 12 departments with courses. Go to http://mitxonline.odl.local:8013/api/v2/departments/
You should see all departments on one page.

@pdpinch
Copy link
Member

pdpinch commented Dec 23, 2024

Oh, does this mean that Urban Studies is coming on the 2nd page?

Is removing the pagination the best solution? What about changing the page size?

Granted, there is a practical limit to the number of departments. The largest possible list is this one, in Learn: https://learn.mit.edu/departments

@annagav
Copy link
Contributor Author

annagav commented Dec 23, 2024

@pdpinch What is the use value for having pagination for departments? It seems redundant. We want the whole list when rendering the Catalog page.

@annagav
Copy link
Contributor Author

annagav commented Dec 23, 2024

Oh, does this mean that Urban Studies is coming on the 2nd page?

Yes, and another department "Political Science". Both are missing from the Catalog Page.

@annagav
Copy link
Contributor Author

annagav commented Dec 23, 2024

If 31 is largest number of Departments and we need all of the departments on first rendering I don't see a reason to have pagination.

@pdpinch
Copy link
Member

pdpinch commented Dec 23, 2024

Ok, if you and Collin are in agreement that's good enough for me.

What do you think about releasing this today? It's a pretty serious bug.

@annagav annagav merged commit 1e33dc4 into main Dec 23, 2024
7 checks passed
@odlbot odlbot mentioned this pull request Dec 23, 2024
1 task
@annagav annagav deleted the ag/department_pagination branch December 23, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants