Release 1.168.0 - #4009
Merged
Merged
Release 1.168.0#4009
Conversation
* fix(flexiblepricing): make status read-only on the applicant serializer FlexiblePriceSerializer (used by the applicant-facing FlexiblePriceViewSet, IsAuthenticated-only) included `status` as a plain writable field. Since the checkout discount gate (is_courseware_flexible_price_approved / determine_courseware_flexible_price_discount) trusts flexible_price.status directly, a learner could PATCH their own application to status: "approved" and receive a financial-aid discount without any staff review. Status changes remain available through the staff-gated FlexiblePriceAdminViewSet/FlexiblePriceAdminSerializer, which is unaffected. Fixes GHSA-2pxf-9249-jf4q. * Make the affected view read only. Nothing POST/PATCHes to this anyway. Form submission is via the Wagtail form (which is a traditional form post, since people have to fill that out) and the staff-dashboard stuff uses a separate admin-only viewset. * Added `courseware_object` but reconsidered so removing it. No sense in changing this API's data at this point. * Fix status code assert on the new test; change status codes to use the constants rather than bare ints * Remove these viewsets entirely, since nothing really uses them --------- Co-authored-by: James Kachel <jkachel@mit.edu>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: CP <collinp@mit.edu> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
OpenAPI ChangesShow/hide changesUnexpected changes? Ensure your branch is up-to-date with |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+49
to
+55
| class Pagination(PageNumberPagination): | ||
| """Paginator class for infinite loading.""" | ||
|
|
||
| django_paginator_class = CountOptimizedPaginator | ||
| page_size = 12 | ||
| page_size_query_param = "page_size" | ||
| max_page_size = 100 |
Contributor
There was a problem hiding this comment.
Bug: The new shared Pagination class is missing a default ordering attribute, causing non-deterministic pagination results for v1 API endpoints that do not explicitly order their querysets.
Severity: MEDIUM
Suggested Fix
Add a default ordering to the shared Pagination class in main/pagination.py, such as ordering = "-created_on". This will restore the previous behavior and ensure consistent, predictable pagination for all viewsets that use this class without specifying their own ordering.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: main/pagination.py#L49-L55
Potential issue: The new shared `Pagination` class in `main/pagination.py` omits the
`ordering = "-created_on"` attribute that was present in the old, local pagination
classes it replaced. This attribute served as a fallback ordering mechanism.
Consequently, `ProgramViewSet` and `CourseViewSet` in the v1 API, which do not define an
explicit order for their querysets, will now return paginated results in a
non-deterministic database order. This can lead to inconsistent page results for API
consumers, where items might appear on different pages or be missed entirely between
requests.
Also affects:
courses/views/v1/__init__.py:76~82courses/views/v2/__init__.py:81~87
Did we get this right? 👍 / 👎 to inform future reviews.
odlbot
added
waiting for checkboxes
all checkboxes checked
and removed
deploying to rc
waiting for checkboxes
labels
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
James Kachel
Nathan Levesque
Sar