feat: rely on DefaultPagination from edx_rest_framework_extensions

[adamstankiewicz]

Rather than relying on from rest_framework.pagination import PageNumberPagination for the base PaginationWithPageCount that exists in enterprise-access today and is consumed by viewsets including browse & request, subsidy access policy, and assignments, this PR proposed we rely on from edx_rest_framework_extensions.paginators import DefaultPagination instead, so we don't need to manually set the num_pages and current_page properties ourselves. Additionally, it gives us a currentPage property which is useful for working with the DataTable in consuming MFE(s).

Secondly, this PR proposes the introduction of an optional ?no_page query parameter for any viewset that utilizes a new OptionalPaginationWithPageCount pagination such that callers can disable pagination for any given request should the use case deem it necessary. The only viewset that utilizes the new OptionalPaginationWithPageCount is assignments.

For example, currently, listing all assignments for a learner on the Dashboard page would only return 10 assignments (unless the frontend naively overrides ?page_size=500) and force the frontend to traverse the pagination which is not performant for users. In such cases, the ?no_page query parameter could be used to return all assignments as a single list instead of segmented by page.

[adamstankiewicz]

[inform] By returning None here, DRF never calls self.get_paginated_response(), so we wouldn't be able to conditionally modify self.get_paginated_response() to behave different with the presence of a ?no_page=1 query parameter.

It feels preferable to rely on this logic living in the paginator than the viewset itself even though it means we have to return a top-level list, e.g.:

[
    {
        "uuid": "db2372f3-c25c-4c38-8de0-60d4d105e0e3",
        "assignment_configuration": "942410d4-ef20-4e4a-9a5f-fa2da07cbb39",
        "learner_email": "[[email protected]](mailto:[email protected])",
        "lms_user_id": null,
        "content_key": "edX+DemoX",
        "content_quantity": -19900,
        "state": "allocated",
        "transaction_uuid": null,
        "last_notification_at": null
    },
    {
        "uuid": "f9c7c4c8-11a7-45f7-ae3e-d81029ec5541",
        "assignment_configuration": "942410d4-ef20-4e4a-9a5f-fa2da07cbb39",
        "learner_email": "[[email protected]](mailto:[email protected])",
        "lms_user_id": null,
        "content_key": "HarvardX+CS50x",
        "content_quantity": -500000,
        "state": "allocated",
        "transaction_uuid": null,
        "last_notification_at": null
    }
]

FWIW, the top-level list for non-paginated responses does seem to be the current convention for several (most?) non-paginated list APIs in Enterprise (e.g., enterprise_course_enrollments) and the broader Open edX platform.

[adamstankiewicz]

Opting to treat uuid as the clickable cell to go into the detail page in Django Admin from the list page of assignments. It felt a bit odd clicking on an enterprise customer UUID to go into a specific content assignment.

[adamstankiewicz]

By migrating from PageNumberPagination (DRF standard) -> DefaultPagination (Open edX standard), list APIs throughout enterprise-access will have additional properties such as num_pages and current_page without extra effort.

However, it does mean we have to override DefaultPagination's page_size to keep the original page_size for PaginationWithPageCount in place (i.e., relies on the REST_FRAMEWORK.PAGE_SIZE Django settings).

[pwnage101]

LGTM! Not gonna block, but I too am leaning on beggs' side of the fence with regards to wanting pagination even if we just set a large page size. Why can't we just make unreasonably large requests take >1 request to fetch the full list, and 99% of the time the normal experience is 1 request?

[iloveagent57]

code-golf nit: you could default the get() to return an empty string, or use or '' to do something like e.g.

param_value = request.query_params.get(self.no_page_query_param) or ''
return param_value.lower()

or if we believe no_page will never be set explicitly as null in the query params, simply

return request.query_params.get(self.no_page_query_param, '').lower()

[adamstankiewicz]

@pwnage101 @iloveagent57 I've decided to remove the ?no_page update from this PR (more details why below); the only changes in this PR now are to ensure we rely on the DefaultPagination from edx_rest_framework_extensions to get the default num_pages and current_page fields returned.

The rationale to no longer include support for ?no_page is the realization that the learner-facing assignments list API can't be used as it exists today to support the learner portal dashboard UX, given it requires additional metadata for display on the assignment cards. Additionally, the assignments list API currently assumes a single request per budget as the list API is for a given assignment configuration UUID. As is, the frontend would need to resolve all budgets available to the learner through the credits_available API, then make another call to the assignments list API for each assignable budget type. Ideally, we'd have a single API endpoint that returns all assignments and their associated course metadata for display in the UI. Markhors filed this ticket to track any API work necessary to surface assignments to the learner UX.

Given the uncertainty around which API endpoint(s) will be used to support the learner-facing UX of content assignments (a different assignments list API? extending credits_available? etc.), I've decided to punt on supporting ?no_page on the existing list API since there's a good chance it won't be used for the learner UX as is anyways.