Skip to content

Release 1.166.7 #3985

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.

Sign up for GitHub

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

Merged
odlbot merged 3 commits into from
Sep 17, 2026
Merged

Release 1.166.7 #3985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a548efa
Make verified-program discounts an "internal" redemption type (#3972)
ChristopherChudzicki Sep 17, 2026
b8f3edc
Scope the CSRF cookie domain by Origin and re-issue it when missing (…
ChristopherChudzicki Sep 17, 2026
4f3b6c2
Release 1.166.7
odlbot Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions RELEASE.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
Release Notes
=============

Version 1.166.7
---------------

- Scope the CSRF cookie domain by Origin and re-issue it when missing (#3978)
- Make verified-program discounts an "internal" redemption type (#3972)

Version 1.166.5
---------------

Expand Down
54 changes: 27 additions & 27 deletions drf_lint_baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,33 +68,33 @@
"courses/serializers/v3/courses.py:62:18:ORM004",
"courses/serializers/v3/courses.py:69:18:ORM004",
"courses/serializers/v3/courses.py:74:18:ORM004",
"ecommerce/serializers/__init__.py:243:12:ORM005",
"ecommerce/serializers/__init__.py:254:17:ORM001",
"ecommerce/serializers/__init__.py:256:18:ORM001",
"ecommerce/serializers/__init__.py:257:18:ORM001",
"ecommerce/serializers/__init__.py:279:26:ORM002",
"ecommerce/serializers/__init__.py:317:42:ORM006",
"ecommerce/serializers/__init__.py:367:26:ORM002",
"ecommerce/serializers/__init__.py:375:31:ORM002",
"ecommerce/serializers/__init__.py:381:20:ORM002",
"ecommerce/serializers/__init__.py:383:19:ORM004",
"ecommerce/serializers/__init__.py:386:31:ORM002",
"ecommerce/serializers/__init__.py:402:35:ORM002",
"ecommerce/serializers/__init__.py:422:4:ORM005",
"ecommerce/serializers/__init__.py:423:4:ORM005",
"ecommerce/serializers/__init__.py:424:4:ORM005",
"ecommerce/serializers/__init__.py:431:12:ORM005",
"ecommerce/serializers/__init__.py:451:15:ORM003",
"ecommerce/serializers/__init__.py:461:24:ORM002",
"ecommerce/serializers/__init__.py:480:12:ORM001",
"ecommerce/serializers/__init__.py:504:22:ORM002",
"ecommerce/serializers/__init__.py:551:22:ORM002",
"ecommerce/serializers/__init__.py:588:46:ORM006",
"ecommerce/serializers/__init__.py:615:15:ORM003",
"ecommerce/serializers/__init__.py:621:20:ORM002",
"ecommerce/serializers/__init__.py:658:26:ORM004",
"ecommerce/serializers/__init__.py:796:22:ORM002",
"ecommerce/serializers/__init__.py:978:28:ORM002",
"ecommerce/serializers/__init__.py:258:12:ORM005",
"ecommerce/serializers/__init__.py:269:17:ORM001",
"ecommerce/serializers/__init__.py:271:18:ORM001",
"ecommerce/serializers/__init__.py:272:18:ORM001",
"ecommerce/serializers/__init__.py:294:26:ORM002",
"ecommerce/serializers/__init__.py:332:42:ORM006",
"ecommerce/serializers/__init__.py:382:26:ORM002",
"ecommerce/serializers/__init__.py:390:31:ORM002",
"ecommerce/serializers/__init__.py:396:20:ORM002",
"ecommerce/serializers/__init__.py:398:19:ORM004",
"ecommerce/serializers/__init__.py:401:31:ORM002",
"ecommerce/serializers/__init__.py:417:35:ORM002",
"ecommerce/serializers/__init__.py:437:4:ORM005",
"ecommerce/serializers/__init__.py:438:4:ORM005",
"ecommerce/serializers/__init__.py:439:4:ORM005",
"ecommerce/serializers/__init__.py:446:12:ORM005",
"ecommerce/serializers/__init__.py:466:15:ORM003",
"ecommerce/serializers/__init__.py:476:24:ORM002",
"ecommerce/serializers/__init__.py:495:12:ORM001",
"ecommerce/serializers/__init__.py:519:22:ORM002",
"ecommerce/serializers/__init__.py:566:22:ORM002",
"ecommerce/serializers/__init__.py:603:46:ORM006",
"ecommerce/serializers/__init__.py:630:15:ORM003",
"ecommerce/serializers/__init__.py:636:20:ORM002",
"ecommerce/serializers/__init__.py:673:26:ORM004",
"ecommerce/serializers/__init__.py:811:22:ORM002",
"ecommerce/serializers/__init__.py:993:28:ORM002",
"ecommerce/serializers/v0/__init__.py:1105:28:ORM002",
"ecommerce/serializers/v0/__init__.py:294:17:ORM001",
"ecommerce/serializers/v0/__init__.py:296:18:ORM001",
Expand Down
10 changes: 10 additions & 0 deletions ecommerce/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from viewflow import fsm

from ecommerce.api import refund_order
from ecommerce.constants import REDEMPTION_TYPE_INTERNAL
from ecommerce.discount_sources import fulfilled_redemptions_funded_by
from ecommerce.forms import AdminRefundOrderForm
from ecommerce.models import (
Expand Down Expand Up @@ -154,6 +155,7 @@ class BasketItemAdmin(VersionAdmin):
@admin.register(Discount)
class DiscountAdmin(admin.ModelAdmin):
model = Discount
exclude = ["is_program_discount"]
search_fields = ["discount_type", "redemption_type", "discount_code"]
list_display = [
"id",
Expand All @@ -165,6 +167,14 @@ class DiscountAdmin(admin.ModelAdmin):
]
list_filter = ["discount_type", "redemption_type", "payment_type"]

def get_readonly_fields(self, request, obj=None): # noqa: ARG002
# An internal discount's code is visible on receipts, so any other
# redemption type would make that code live. DiscountShapeMixin refuses
# the same change over the API.
if obj is not None and obj.redemption_type == REDEMPTION_TYPE_INTERNAL:
return ("redemption_type",)
return ()


@admin.register(DiscountProduct)
class DiscountProductAdmin(admin.ModelAdmin):
Expand Down
26 changes: 25 additions & 1 deletion ecommerce/admin_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
"""Tests for ecommerce admin views"""

import pytest
from django.contrib import admin
from django.contrib.contenttypes.models import ContentType
from django.contrib.messages import get_messages
from django.test import RequestFactory
from django.urls import NoReverseMatch, reverse
from reversion.models import Version

from courses.factories import CourseRunFactory
from ecommerce.admin import DiscountAdmin
from ecommerce.factories import (
DiscountRedemptionFactory,
InternalDiscountFactory,
OrderFactory,
UnlimitedUseDiscountFactory,
)
from ecommerce.models import OrderStatus, Product
from ecommerce.models import Discount, OrderStatus, Product

pytestmark = [pytest.mark.django_db]

Expand Down Expand Up @@ -336,3 +341,22 @@ def test_admin_refund_view_keeps_the_credit_warning_on_a_rejected_form(
assert response.context["form_valid"] is False
assert list(response.context["used_source_redemptions"]) == [redemption]
assert redemption.redeemed_order.reference_number in response.content.decode()


@pytest.mark.parametrize(
("factory", "locked"),
[(InternalDiscountFactory, True), (UnlimitedUseDiscountFactory, False)],
)
def test_discount_admin_locks_redemption_type_for_internal_discounts(
admin_user, factory, locked
):
"""Re-typing an internal discount would turn it into a live 100%-off code."""
discount = factory.create()
request = RequestFactory().get("/")
request.user = admin_user

readonly = DiscountAdmin(Discount, admin.site).get_readonly_fields(
request, discount
)

assert ("redemption_type" in readonly) is locked
23 changes: 18 additions & 5 deletions ecommerce/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
DISCOUNT_TYPE_PERCENT_OFF,
PAYMENT_TYPE_FINANCIAL_ASSISTANCE,
PAYMENT_TYPE_SALES,
REDEMPTION_TYPE_INTERNAL,
REDEMPTION_TYPE_ONE_TIME,
REDEMPTION_TYPE_ONE_TIME_PER_USER,
REDEMPTION_TYPE_UNLIMITED,
Expand Down Expand Up @@ -70,6 +71,7 @@
source_line_for,
)
from ecommerce.exceptions import (
VerifiedProgramCourseNotInProgramError,
VerifiedProgramInvalidBasketError,
VerifiedProgramInvalidOrderError,
VerifiedProgramNoEnrollmentError,
Expand Down Expand Up @@ -1425,8 +1427,10 @@ def create_verified_program_discount(program):
codes - this creates one for the program that is set up to make the order
zero-value, so the learner doesn't have to pay for upgraded enrollments.

This will create a single discount, with the "verified program" flag set,
with unlimited redemptions, set to 100% off.
This creates a single 100%-off discount with the "internal" redemption type
and no redemption cap: learners cannot redeem it, and it prices whatever the
verified-enrollment flow attaches it to. Callers are responsible for
checking the run belongs to the program before attaching it.

If a discount already exists for this purpose, this will return it.

Expand All @@ -1445,7 +1449,9 @@ def create_verified_program_discount(program):
Q(activation_date__isnull=True) | Q(activation_date__lte=now_in_utc()),
Q(expiration_date__isnull=True) | Q(expiration_date__gte=now_in_utc()),
products__product=product,
is_program_discount=True,
redemption_type=REDEMPTION_TYPE_INTERNAL,
discount_type=DISCOUNT_TYPE_PERCENT_OFF,
amount=100,
)

if existing_discount_qs.exists():
Comment on lines +1454 to 1457

@sentry sentry Bot Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The new, more restrictive filter for program discounts may fail to find pre-existing, manually-edited discounts, leading to the creation of duplicates.
Severity: LOW

Suggested Fix

To prevent creating duplicates, the lookup for existing program discounts should be less restrictive. Consider only filtering by a unique, immutable identifier for program discounts, such as redemption_type=REDEMPTION_TYPE_INTERNAL. Alternatively, if the discount_type and amount must be fixed, add a data migration to normalize any existing, manually-edited program discounts to match the new expected values.

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: ecommerce/api.py#L1454-L1457

Potential issue: The logic to find an existing program discount was changed to filter on
`redemption_type=INTERNAL`, `discount_type=DISCOUNT_TYPE_PERCENT_OFF`, and `amount=100`.
Previously, the lookup was less restrictive. If a program discount was manually edited
in the admin panel to have a different `discount_type` or `amount` before this change,
the new, more restrictive filter will fail to find it. This will cause the system to
create a duplicate discount for the program, which could cause issues in the critical
enrollment flow.

Did we get this right? 👍 / 👎 to inform future reviews.

Expand All @@ -1455,11 +1461,10 @@ def create_verified_program_discount(program):
amount=Decimal(100),
automatic=False,
discount_type=DISCOUNT_TYPE_PERCENT_OFF,
redemption_type=REDEMPTION_TYPE_UNLIMITED,
redemption_type=REDEMPTION_TYPE_INTERNAL,
payment_type=PAYMENT_TYPE_SALES,
discount_code=f"{program.readable_id}-{uuid.uuid4()}",
is_bulk=True,
is_program_discount=True,
)

DiscountProduct.objects.create(discount=discount, product=product)
Expand Down Expand Up @@ -1496,6 +1501,8 @@ def create_verified_program_course_run_enrollment(request, courserun, program):
Raises:
- VerifiedProgramNoEnrollmentError if the learner doesn't have a program
enrollment
- VerifiedProgramCourseNotInProgramError if the run's course is not in the
program's requirements
- VerifiedProgramInvalidBasketError if the basket isn't zero value
- VerifiedProgramInvalidOrderError if the order doesn't get processed through
"""
Expand All @@ -1506,6 +1513,12 @@ def create_verified_program_course_run_enrollment(request, courserun, program):
msg = f"No verified enrollment for {request.user} for program {program}"
raise VerifiedProgramNoEnrollmentError(msg)

# The program's internal discount prices whatever it is attached to, so
# membership is decided here, against the current requirements tree.
if not program.courses_qset.filter(courseruns=courserun).exists():
msg = f"Course run {courserun} is not in program {program}"
raise VerifiedProgramCourseNotInProgramError(msg)

discount = create_verified_program_discount(program)

cr_ctype = ContentType.objects.get_for_model(courserun)
Expand Down
43 changes: 42 additions & 1 deletion ecommerce/api_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
DISCOUNT_TYPE_FIXED_PRICE,
DISCOUNT_TYPE_PERCENT_OFF,
PAYMENT_TYPE_FINANCIAL_ASSISTANCE,
REDEMPTION_TYPE_INTERNAL,
STRIPE_CHECKOUT_SESSION_STATUS_COMPLETE,
STRIPE_CHECKOUT_SESSION_STATUS_EXPIRED,
STRIPE_CHECKOUT_SESSION_STATUS_OPEN,
Expand All @@ -87,10 +88,12 @@
ZERO_PAYMENT_DATA,
)
from ecommerce.exceptions import (
VerifiedProgramCourseNotInProgramError,
VerifiedProgramNoEnrollmentError,
)
from ecommerce.factories import (
DiscountRedemptionFactory,
InternalDiscountFactory,
LineFactory,
OneTimeDiscountFactory,
OneTimePerUserDiscountFactory,
Expand Down Expand Up @@ -978,10 +981,11 @@ def test_create_verified_program_discount():
discount = create_verified_program_discount(program)

assert discount
assert discount.is_program_discount
assert discount.redemption_type == REDEMPTION_TYPE_INTERNAL
assert discount.products.filter(
product__content_type=content_type, product__object_id=program.id
).exists()
assert create_verified_program_discount(program) == discount


def test_create_verified_program_course_run_enrollment(
Expand Down Expand Up @@ -1047,6 +1051,27 @@ def test_create_vpcre_no_program(bootstrapped_verified_program, user):
assert "No verified enrollment" in str(exc.value)


def test_create_vpcre_run_not_in_program(bootstrapped_verified_program, user):
"""
The program's discount prices anything it is attached to, so a run whose
course is outside the program's requirements is refused before a basket
exists.
"""
(program, _, _, _, _) = bootstrapped_verified_program
ProgramEnrollmentFactory.create(
program=program, user=user, enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE
)
other_run = CourseRunFactory.create()

request = RequestFactory().get("/")
request.user = user

with pytest.raises(VerifiedProgramCourseNotInProgramError):
create_verified_program_course_run_enrollment(request, other_run, program)

assert not BasketDiscount.objects.filter(redeemed_by=user).exists()


def test_create_vpcre_bad_basket(
mocker,
mock_hubspot_order,
Expand Down Expand Up @@ -1582,6 +1607,22 @@ def test_quote_user_price_skips_an_automatic_tied_to_another_learner(user):
assert quote.price == product.price


def test_quote_user_price_skips_an_internal_discount(user):
"""
Checkout refuses an internal discount, so a UserDiscount row tying one to
this learner must not quote a price the cart will not honor.
"""
product = ProductFactory.create()
internal = InternalDiscountFactory.create()
DiscountProduct.objects.create(discount=internal, product=product)
UserDiscount.objects.create(discount=internal, user=user)

quote = quote_user_price(product, user)

assert quote.discount is None
assert quote.price == product.price


def test_quote_user_price_skips_a_discount_linked_to_another_product(user):
"""
A discount carrying DiscountProduct links is in scope only for the products
Expand Down
15 changes: 12 additions & 3 deletions ecommerce/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,32 @@
REDEMPTION_TYPE_ONE_TIME_PER_USER = "one-time-per-user"
REDEMPTION_TYPE_UNLIMITED = "unlimited"
REDEMPTION_TYPE_PROGRAM_CHILD_PURCHASE = "program-child-purchase"
# An internal discount reaches a basket only through application code that has
# already decided the learner is entitled to it — the one such caller is
# ecommerce.api.create_verified_program_course_run_enrollment. Every
# learner-facing route refuses it (Discount._within_redemption_limits), and
# pricing does not re-check eligibility: its product links say what it is for,
# not where it applies.
REDEMPTION_TYPE_INTERNAL = "internal"

ALL_REDEMPTION_TYPES = [
REDEMPTION_TYPE_ONE_TIME,
REDEMPTION_TYPE_ONE_TIME_PER_USER,
REDEMPTION_TYPE_UNLIMITED,
REDEMPTION_TYPE_PROGRAM_CHILD_PURCHASE,
REDEMPTION_TYPE_INTERNAL,
]

REDEMPTION_TYPES = list(zip(ALL_REDEMPTION_TYPES, ALL_REDEMPTION_TYPES))

# program-child-purchase forces automatic=True and program-only product
# links even when paired with a standard calculation, so the random draw
# skips it too.
# links even when paired with a standard calculation, and internal is never
# learner-redeemable, so the random draw and bulk generation skip both.
STANDARD_REDEMPTION_TYPES = [
redemption_type
for redemption_type in ALL_REDEMPTION_TYPES
if redemption_type != REDEMPTION_TYPE_PROGRAM_CHILD_PURCHASE
if redemption_type
not in (REDEMPTION_TYPE_PROGRAM_CHILD_PURCHASE, REDEMPTION_TYPE_INTERNAL)
]
BULK_GENERATION_REDEMPTION_TYPES = list(
zip(STANDARD_REDEMPTION_TYPES, STANDARD_REDEMPTION_TYPES)
Expand Down
8 changes: 5 additions & 3 deletions ecommerce/discounts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
DISCOUNT_TYPE_FIXED_PRICE,
DISCOUNT_TYPE_PAID_AMOUNT_OFF,
DISCOUNT_TYPE_PERCENT_OFF,
REDEMPTION_TYPE_INTERNAL,
)
from ecommerce.models import Discount, Product

Expand Down Expand Up @@ -59,10 +60,11 @@ def get_discounted_price(
return price

def get_product_price(self, product: Product):
# Program discounts are linked to a program product for identification only;
# they must still apply to the course-run products placed in the basket.
# An internal discount's product links say what it is for, not what it
# prices; the code that attached it decided eligibility (see
# REDEMPTION_TYPE_INTERNAL).
if (
not self.discount.is_program_discount
self.discount.redemption_type != REDEMPTION_TYPE_INTERNAL
and not self.discount.applies_to_products([product])
):
return product.price
Expand Down
Loading