-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat: Use default pagination class for v2 lib views #34879
feat: Use default pagination class for v2 lib views #34879
Conversation
Thanks for the pull request, @yusuf-musleh! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
cad4475
to
b7aed5c
Compare
num_pages
to v2 lib paginated responseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Works great @yusuf-musleh .
I have one question about whether or not we need to set a default page size value, but everything is working as described.
- I tested this on my tutor dev stack
- I read through the code
-
I checked for accessibility issuesN/A -
Includes documentationN/A -
User-facing strings are extracted for translationN/A
This ensures `num_pages` is included in paginated response.
b7aed5c
to
53c8a39
Compare
@yusuf-musleh @pomegranited Are we waiting for anything before we merge this? Is it backwards compatible with the code in the Library Authoring MFE? |
@bradenmacdonald Its was just awaiting CC review, it should be good to go. It is backwards compatible with the library authoring mfe, I tested it out locally, the libraries show up as expected. This technically just added an extra field to the response |
It has CC review already though :) Going to merge soon. |
Oh didn't realize @pomegranited was a CC on this repo 😅 |
@yusuf-musleh 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
2U Release Notice: This PR has been deployed to the edX production environment. |
Description
This PR removes the custom pagination added to the V2 Library views and uses the default one, as it already contains
num_pages
field. Which is needed for pagination of v2 Libraries in the course authoring MFE.Supporting information
Related Tickets:
Testing instructions
Follow the instructions in openedx/frontend-app-authoring#1050
Private-ref: FAL-3751