diff --git a/RELEASE.rst b/RELEASE.rst index b7614094b1..99184549c0 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,14 @@ Release Notes ============= +Version 1.166.3 +--------------- + +- feat: GET /api/v0/products/{id}/user_pricing/ per-user price quote (#3959) +- Return 409 when a new organization's name reuses a page slug (#3955) +- Restrict B2B page access to admins (#3957) +- fix(sentry): cap request bodies at 1KB and scrub Postgres DETAIL rows (#3936) + Version 1.166.2 --------------- diff --git a/b2b/exceptions.py b/b2b/exceptions.py index 0921646a78..375c4c95cd 100644 --- a/b2b/exceptions.py +++ b/b2b/exceptions.py @@ -29,6 +29,16 @@ class AliasCollisionError(Exception): """ +class OrganizationNameCollisionError(Exception): + """ + Raised when a new organization's name would reuse an existing page slug. + + The name becomes the OrganizationPage slug, which Wagtail requires to be + unique under the organization index. Names that differ only in case or + punctuation slugify to the same thing, so they collide too. + """ + + class InvalidLifecycleTransitionError(Exception): """Raised when an identity provider is asked to skip a lifecycle state.""" diff --git a/b2b/models.py b/b2b/models.py index cd16a282d5..d3c6f4ea33 100644 --- a/b2b/models.py +++ b/b2b/models.py @@ -147,13 +147,19 @@ class OrganizationPage(Page): # Use default promote_panels from Page to allow manual slug editing + @staticmethod + def slug_for_name(name): + """Return the slug a new organization with this name is saved under.""" + + return slugify(f"org-{name}") + def save(self, clean=True, user=None, log_action=False, **kwargs): # noqa: FBT002 """Save the page, and update the slug and title appropriately.""" self.title = str(self.name) if not self.slug: - self.slug = slugify(f"org-{self.name}") + self.slug = self.slug_for_name(self.name) Page.save(self, clean=clean, user=user, log_action=log_action, **kwargs) def get_learners(self): diff --git a/b2b/provisioning.py b/b2b/provisioning.py index d7532b79bb..4430ff33cb 100644 --- a/b2b/provisioning.py +++ b/b2b/provisioning.py @@ -33,6 +33,7 @@ from b2b.exceptions import ( AliasCollisionError, InvalidLifecycleTransitionError, + OrganizationNameCollisionError, OrganizationNotProvisionedError, OrphanedKeycloakOrganizationError, ) @@ -190,6 +191,7 @@ def create_organization( # noqa: PLR0913 - OrganizationPage: the new organization Raises: - AliasCollisionError: org_key is taken here or in the realm + - OrganizationNameCollisionError: the name's page slug is already taken - OrphanedKeycloakOrganizationError: the MITx Online write and its compensating delete both failed - requests.HTTPError: Keycloak rejected the create @@ -220,6 +222,20 @@ def create_organization( # noqa: PLR0913 ) raise ImproperlyConfigured(msg) + # add_child() enforces sibling slug uniqueness with a ValidationError, after + # the Keycloak write. Checking here keeps a duplicate name from creating and + # then compensating a Keycloak organization, and gives the caller a 409. + if ( + organization_index.get_children() + .filter(slug=OrganizationPage.slug_for_name(name)) + .exists() + ): + msg = ( + f"An organization named '{name}', or one whose name produces the same " + "page slug, already exists." + ) + raise OrganizationNameCollisionError(msg) + # Domains are written verified, with no verification having occurred: staff # are asserting them. That is defensible only while the asserting party is # MIT staff, and stops being so the moment the partner-facing wizard (C2) diff --git a/b2b/provisioning_test.py b/b2b/provisioning_test.py index 5d56f8945e..78514ded2c 100644 --- a/b2b/provisioning_test.py +++ b/b2b/provisioning_test.py @@ -18,6 +18,7 @@ from b2b.exceptions import ( AliasCollisionError, InvalidLifecycleTransitionError, + OrganizationNameCollisionError, OrganizationNotProvisionedError, OrphanedKeycloakOrganizationError, ) @@ -148,6 +149,30 @@ def test_create_organization_rejects_an_alias_taken_in_the_realm(connection): connection.organizations.create.assert_not_called() +@pytest.mark.parametrize("name", ["Example University", "example university!"]) +def test_create_organization_rejects_a_name_that_reuses_a_page_slug(connection, name): + """ + A name whose slug is taken under the index fails before Keycloak is touched. + + Wagtail's own sibling-slug check in add_child() raises a plain + ValidationError, which the API turned into a 500 (MITXONLINE-73J), and only + after a Keycloak organization had been created and had to be deleted again. + """ + + create_organization(connection=connection, **_organization_kwargs()) + connection.organizations.create.reset_mock() + + with pytest.raises(OrganizationNameCollisionError): + create_organization( + connection=connection, + **_organization_kwargs(name=name, org_key="EXAMPLEU2"), + ) + + connection.organizations.create.assert_not_called() + connection.organizations.delete.assert_not_called() + assert not OrganizationPage.objects.filter(org_key="EXAMPLEU2").exists() + + def test_create_organization_compensates_a_failed_local_write(connection, mocker): """ A failed MITx Online write deletes the Keycloak organization it made. diff --git a/b2b/views/v0/__init__.py b/b2b/views/v0/__init__.py index 6be06ca858..640ca29734 100644 --- a/b2b/views/v0/__init__.py +++ b/b2b/views/v0/__init__.py @@ -14,7 +14,7 @@ from mitol.common.utils.datetime import now_in_utc from rest_framework import serializers, status, viewsets from rest_framework.decorators import action -from rest_framework.permissions import IsAuthenticated +from rest_framework.permissions import IsAdminUser, IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView from rest_framework_api_key.permissions import HasAPIKey @@ -38,7 +38,6 @@ from ecommerce.models import Discount, Product from main.authentication import CsrfExemptSessionAuthentication from main.constants import USER_MSG_TYPE_B2B_ENROLL_SUCCESS -from main.permissions import IsAdminOrReadOnly log = logging.getLogger(__name__) @@ -62,7 +61,7 @@ class OrganizationPageViewSet(viewsets.ReadOnlyModelViewSet): ) ) serializer_class = OrganizationPageSerializer - permission_classes = [IsAdminOrReadOnly | HasAPIKey] + permission_classes = [IsAdminUser | HasAPIKey] lookup_field = "slug" lookup_url_kwarg = "organization_slug" @@ -73,7 +72,7 @@ class ContractPageViewSet(viewsets.ReadOnlyModelViewSet): """ serializer_class = ContractPageSerializer - permission_classes = [IsAdminOrReadOnly | HasAPIKey] + permission_classes = [IsAdminUser | HasAPIKey] lookup_field = "slug" lookup_url_kwarg = "contract_slug" diff --git a/b2b/views/v0/provisioning.py b/b2b/views/v0/provisioning.py index 07c16f2eb4..b7ede3984d 100644 --- a/b2b/views/v0/provisioning.py +++ b/b2b/views/v0/provisioning.py @@ -26,6 +26,7 @@ from b2b.exceptions import ( AliasCollisionError, InvalidLifecycleTransitionError, + OrganizationNameCollisionError, OrganizationNotProvisionedError, OrphanedKeycloakOrganizationError, ) @@ -77,7 +78,12 @@ class ProvisioningExceptionMixin: def handle_exception(self, exc): """Map provisioning exceptions onto HTTP responses.""" - if isinstance(exc, AliasCollisionError | OrganizationNotProvisionedError): + if isinstance( + exc, + AliasCollisionError + | OrganizationNameCollisionError + | OrganizationNotProvisionedError, + ): return Response({"detail": str(exc)}, status=status.HTTP_409_CONFLICT) if isinstance(exc, InvalidLifecycleTransitionError): return Response({"detail": str(exc)}, status=status.HTTP_400_BAD_REQUEST) diff --git a/b2b/views/v0/provisioning_test.py b/b2b/views/v0/provisioning_test.py index b3ba79250f..a49385b142 100644 --- a/b2b/views/v0/provisioning_test.py +++ b/b2b/views/v0/provisioning_test.py @@ -14,7 +14,7 @@ IDP_STATE_TESTING, ONBOARDING_STATE_LIVE, ) -from b2b.exceptions import AliasCollisionError +from b2b.exceptions import AliasCollisionError, OrganizationNameCollisionError from b2b.factories import OrganizationIndexPageFactory, OrganizationPageFactory from b2b.keycloak_admin_dataclasses import ( OrganizationDomainRepresentation, @@ -136,6 +136,20 @@ def test_create_organization_alias_collision_is_a_conflict(admin_drf_client, moc assert response.json()["detail"] == "taken" +def test_create_organization_name_collision_is_a_conflict(admin_drf_client, mocker): + """A name whose page slug is taken is 409, not the 500 in MITXONLINE-73J.""" + + mocker.patch( + "b2b.views.v0.provisioning.create_organization", + side_effect=OrganizationNameCollisionError("name taken"), + ) + + response = admin_drf_client.post(_organizations_url(), CREATE_BODY, format="json") + + assert response.status_code == status.HTTP_409_CONFLICT + assert response.json()["detail"] == "name taken" + + def test_keycloak_failure_is_a_bad_gateway(admin_drf_client, mocker): """ A failed Keycloak call is 502, not 500. diff --git a/drf_lint_baseline.json b/drf_lint_baseline.json index 65c1ba54e7..2a222b4328 100644 --- a/drf_lint_baseline.json +++ b/drf_lint_baseline.json @@ -95,36 +95,36 @@ "ecommerce/serializers/__init__.py:658:26:ORM004", "ecommerce/serializers/__init__.py:796:22:ORM002", "ecommerce/serializers/__init__.py:978:28:ORM002", - "ecommerce/serializers/v0/__init__.py:289:17:ORM001", - "ecommerce/serializers/v0/__init__.py:291:18:ORM001", - "ecommerce/serializers/v0/__init__.py:292:18:ORM001", - "ecommerce/serializers/v0/__init__.py:314:26:ORM002", - "ecommerce/serializers/v0/__init__.py:352:42:ORM006", - "ecommerce/serializers/v0/__init__.py:402:26:ORM002", - "ecommerce/serializers/v0/__init__.py:411:35:ORM002", - "ecommerce/serializers/v0/__init__.py:418:20:ORM002", - "ecommerce/serializers/v0/__init__.py:420:19:ORM004", - "ecommerce/serializers/v0/__init__.py:424:35:ORM002", - "ecommerce/serializers/v0/__init__.py:441:35:ORM002", - "ecommerce/serializers/v0/__init__.py:463:4:ORM005", - "ecommerce/serializers/v0/__init__.py:464:4:ORM005", - "ecommerce/serializers/v0/__init__.py:465:4:ORM005", - "ecommerce/serializers/v0/__init__.py:495:15:ORM003", - "ecommerce/serializers/v0/__init__.py:499:15:ORM003", - "ecommerce/serializers/v0/__init__.py:503:15:ORM003", - "ecommerce/serializers/v0/__init__.py:507:18:ORM003", - "ecommerce/serializers/v0/__init__.py:512:15:ORM003", - "ecommerce/serializers/v0/__init__.py:522:24:ORM002", - "ecommerce/serializers/v0/__init__.py:541:12:ORM001", - "ecommerce/serializers/v0/__init__.py:565:22:ORM002", - "ecommerce/serializers/v0/__init__.py:612:22:ORM002", - "ecommerce/serializers/v0/__init__.py:649:46:ORM006", - "ecommerce/serializers/v0/__init__.py:66:12:ORM005", - "ecommerce/serializers/v0/__init__.py:681:15:ORM003", - "ecommerce/serializers/v0/__init__.py:687:20:ORM002", - "ecommerce/serializers/v0/__init__.py:724:26:ORM004", - "ecommerce/serializers/v0/__init__.py:819:22:ORM002", - "ecommerce/serializers/v0/__init__.py:963: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", + "ecommerce/serializers/v0/__init__.py:297:18:ORM001", + "ecommerce/serializers/v0/__init__.py:319:26:ORM002", + "ecommerce/serializers/v0/__init__.py:357:42:ORM006", + "ecommerce/serializers/v0/__init__.py:407:26:ORM002", + "ecommerce/serializers/v0/__init__.py:416:35:ORM002", + "ecommerce/serializers/v0/__init__.py:423:20:ORM002", + "ecommerce/serializers/v0/__init__.py:425:19:ORM004", + "ecommerce/serializers/v0/__init__.py:429:35:ORM002", + "ecommerce/serializers/v0/__init__.py:446:35:ORM002", + "ecommerce/serializers/v0/__init__.py:468:4:ORM005", + "ecommerce/serializers/v0/__init__.py:469:4:ORM005", + "ecommerce/serializers/v0/__init__.py:470:4:ORM005", + "ecommerce/serializers/v0/__init__.py:500:15:ORM003", + "ecommerce/serializers/v0/__init__.py:504:15:ORM003", + "ecommerce/serializers/v0/__init__.py:508:15:ORM003", + "ecommerce/serializers/v0/__init__.py:512:18:ORM003", + "ecommerce/serializers/v0/__init__.py:517:15:ORM003", + "ecommerce/serializers/v0/__init__.py:527:24:ORM002", + "ecommerce/serializers/v0/__init__.py:546:12:ORM001", + "ecommerce/serializers/v0/__init__.py:570:22:ORM002", + "ecommerce/serializers/v0/__init__.py:617:22:ORM002", + "ecommerce/serializers/v0/__init__.py:654:46:ORM006", + "ecommerce/serializers/v0/__init__.py:686:15:ORM003", + "ecommerce/serializers/v0/__init__.py:692:20:ORM002", + "ecommerce/serializers/v0/__init__.py:71:12:ORM005", + "ecommerce/serializers/v0/__init__.py:729:26:ORM004", + "ecommerce/serializers/v0/__init__.py:961:22:ORM002", "flexiblepricing/serializers.py:147:34:ORM001", "flexiblepricing/serializers.py:163:43:ORM006", "flexiblepricing/serializers.py:170:34:ORM001", diff --git a/ecommerce/api.py b/ecommerce/api.py index 8fa4ebfe2a..88baceaa30 100644 --- a/ecommerce/api.py +++ b/ecommerce/api.py @@ -2,6 +2,7 @@ import logging import uuid +from dataclasses import dataclass from datetime import datetime, timedelta from decimal import Decimal from urllib.parse import urljoin @@ -66,6 +67,7 @@ from ecommerce.discount_sources import ( double_spent_source_line_ids, fulfilled_paid_amount_off_redemptions, + source_line_for, ) from ecommerce.exceptions import ( VerifiedProgramInvalidBasketError, @@ -80,6 +82,7 @@ DiscountProduct, DiscountRedemption, FulfilledOrder, + Line, Order, OrderStatus, PendingOrder, @@ -1187,6 +1190,140 @@ def generate_discount_code(**kwargs): # noqa: C901 return generated_codes +def _active_discounts() -> QuerySet[Discount]: + """Every discount inside its activation and expiration window right now.""" + now = now_in_utc() + return Discount.objects.filter( + Q(activation_date__lte=now) | Q(activation_date=None), + Q(expiration_date__gt=now) | Q(expiration_date=None), + ) + + +def _discounts_offered_to(user, flexible_price_discounts) -> QuerySet[Discount]: + """ + Every active discount on offer to ``user``: the automatic ones, the ones + tied to this learner, and ``flexible_price_discounts``, the + financial-assistance tier discounts already determined for the products in + question. + + A tier discount carries no product links, so naming its id is what offers + it for the product it was determined for and no other. + + Offered is not the same as applicable: an automatic discount may also carry + UserDiscount rows naming other learners, and product scope, redemption + limits and the program-child-purchase source are all still open. Callers + narrow from here. + """ + return _active_discounts().filter( + Q(automatic=True) + | Q(user_discount_discount__user=user) + | Q(pk__in=[discount.id for discount in flexible_price_discounts]) + ) + + +@dataclass(frozen=True) +class UserPriceQuote: + """ + What this user pays for one product, and why. + + ``flexible_price_discount`` is the learner's approved financial assistance + discount whether or not it won the price -- a tier whose redemptions are + spent, one another candidate undercuts, and the top tier that prices at + list all still answer "is this learner approved for aid", which is a + different question from what checkout charges. + + ``source_line`` is the prior purchase a winning paid-amount-off discount + spends, so the caller can name the credit without resolving it again. + """ + + discount: Discount | None + price: Decimal + flexible_price_discount: Discount | None + source_line: Line | None + + +def quote_user_price(product, user) -> UserPriceQuote: + """ + What checkout charges ``user`` for ``product``. + + The cheapest applicable discount wins, which is the rule + apply_discount_to_basket applies: it keeps a candidate only when the + candidate prices an item at or below the applied price, so no class of + discount outranks another. + + The price therefore agrees with what the basket charges for the + single-item baskets checkout builds; only the discount named can differ, + in two ways. On an exact tie the basket keeps whichever discount it applied + last while this names the lowest id. And a discount that beats no other + candidate but still quotes the list price is recorded on the basket, while + this reports no discount, because the basket's price is the discount's + price capped at the list price. + + Applicability restates is_valid_for_basket for a single product: the + discount is inside its window, in scope for the product, offered to this + learner rather than tied to another, inside its redemption limits, and -- + for a program-child-purchase discount -- backed by an unconsumed + qualifying prior purchase. The first three are the candidate query; + discount_product enforces the rest. + + The work is bounded by the discounts that can price this product, not by + anything about the request. Checkout's own bound is looser: it checks every + discount on offer to the learner, in scope for the basket or not. + + Args: + product (Product): the product to price + user (User or None): the learner, or None/anonymous for list price + Returns: + UserPriceQuote + """ + if user is None or user.is_anonymous: + return UserPriceQuote( + discount=None, + price=product.price, + flexible_price_discount=None, + source_line=None, + ) + + finaid = determine_courseware_flexible_price_discount(product, user) + candidates = ( + _discounts_offered_to(user, [finaid] if finaid else []) + .filter( + # A discount carrying DiscountProduct rows applies only to the + # products named by them; one carrying none applies to everything. + # Scoping in SQL rather than per candidate is what keeps the cost + # independent of how many discounts are live for other products. + Q(products__isnull=True) | Q(products__product=product) + ) + .filter( + # A discount carrying UserDiscount rows is offered only to the + # users named by them. The two filters join the rows separately, so + # this one excludes a discount tied to another learner even where + # _discounts_offered_to admitted it for being automatic. + Q(user_discount_discount__isnull=True) + | Q(user_discount_discount__user=user) + ) + .distinct() + # Ordering is what makes a price tie resolve on the lowest id rather + # than on however the database returned the rows. + .order_by("id") + ) + + best, best_price = None, product.price + for discount in candidates: + price = discount.discount_product(product, user) + if price is not None and price < best_price: + best, best_price = discount, price + return UserPriceQuote( + discount=best, + price=best_price, + flexible_price_discount=finaid, + # discount_product resolves a paid-amount-off winner's source to price + # it and discards the line; resolving that one discount a second time + # is cheaper than threading the line out through every pricing caller. + source_line=source_line_for(best, user, [product]) if best else None, + ) + + def get_auto_apply_discounts_for_basket(basket_id: int) -> QuerySet[Discount]: """ Get the auto-apply discounts that can be applied to a basket. @@ -1204,42 +1341,45 @@ def get_auto_apply_discounts_for_basket(basket_id: int) -> QuerySet[Discount]: QuerySet: The auto-apply discounts that can be applied to the basket. """ basket = Basket.objects.get(pk=basket_id) - products = basket.get_products() - - finaid_discounts = [] - - for product in products: - finaid_discount = determine_courseware_flexible_price_discount( - product, basket.user + flexible_price_discounts = [ + discount + for product in basket.get_products() + if ( + discount := determine_courseware_flexible_price_discount( + product, basket.user + ) ) - - if finaid_discount: - finaid_discounts.append(finaid_discount.id) - - return Discount.objects.filter( - Q(activation_date__lte=now_in_utc()) | Q(activation_date=None), - Q(expiration_date__gt=now_in_utc()) | Q(expiration_date=None), - ).filter( - Q(user_discount_discount__user=basket.user) - | Q(pk__in=finaid_discounts) - | Q(automatic=True) - ) + ] + return _discounts_offered_to(basket.user, flexible_price_discounts) -def apply_discount_to_basket(basket: Basket, discount: Discount, *, allow_finaid=False): # noqa: C901 +def apply_discount_to_basket(basket: Basket, discount: Discount, *, allow_finaid=False): """ Apply a discount to a basket. Discount application is subject to rules: - - The discount itself must be valid on its face (not inactive, applies to products, etc.) - - The discount is not a financial assistance tier discount, unless allow_finaid is set - - The discount provides a better price to the learner than any other applied discount - - The discount is not overriding a user discount - - If a user discount is supplied to this function, then that discount will be - applied _unless_ a financial assistance discount is also applied. User - discounts take precedence over any other discount, other than financial - assistance discounts. + - The discount itself must be valid on its face (inside its + activation/expiration window, applies to products, tied to no user or to + this one, within its redemption limits, and -- for a + program-child-purchase discount -- the learner still holds an unconsumed + qualifying prior purchase among the basket's products) + - The discount is not a discount marked as financial assistance + (``payment_type``), unless allow_finaid is set + - The discount prices some basket item at or below that item's current + discounted price + + For the single-item baskets checkout builds -- ``_create_basket_from_product`` + and ``create_basket_with_products`` in ecommerce/views/v0 empty the basket + before adding, unless ENABLE_MULTIPLE_CART_ITEMS is on, which it is not by + default -- the cheapest applicable discount wins whatever order the + candidates arrive in: no class of discount outranks another, so a user-tied + discount, a financial assistance tier discount, an automatic discount and a + typed-in code all compete on price alone, and a candidate that ties the + applied price replaces it, which is why the basket view applies the code the + learner typed in last. With several items the rule is bullet 3 exactly: a + candidate is kept when it prices *some* item at or below the applied price, + so which candidates survive, and the basket total, depend on the order they + arrive in. This function is not for use with B2B or verified program enrollment code redemption. Those use cases have their own redemption code paths because @@ -1257,65 +1397,16 @@ def apply_discount_to_basket(basket: Basket, discount: Discount, *, allow_finaid } if basket.discounts.count() > 0 and basket.basket_items.count() > 0: - # Check to make sure the supplied discount can be applied. This means - # that it should not override any user discounts that are applied, - # and it should be better than the other discounts in the basket. - - if discount.user_discount_discount.filter(user=basket.user).exists(): - # This is a user discount. - # Check for an existing tier discount - user discount shouldn't override that - finaid_discounts = [ - basket_discount - for basket_discount in basket.discounts.all() - if basket_discount.redeemed_discount.flexible_price_tiers.count() - > 0 - ] - - if len(finaid_discounts) > 0: - # There is a finaid discount, so don't apply this user one. - return - else: - is_finaid_discount = discount.flexible_price_tiers.exists() - has_user_discount = ( - basket.discounts.filter( - redeemed_discount__user_discount_discount__user=basket.user - ).count() - > 0 - ) - - if is_finaid_discount and not allow_finaid: - # Financial assistance discount; bail unless the flag is set - return - - if has_user_discount and is_finaid_discount and allow_finaid: - # Basket has a user discount applied; this is a finaid - # discount (and we're allowed to apply it); apply the - # discount without further evaluation. - - BasketDiscount.objects.update_or_create( - redeemed_by=basket.user, - redeemed_basket=basket, - defaults=defaults, - create_defaults=defaults, - ) - return - - if has_user_discount: - # This basket has a user discount applied; this isn't a - # finaid discount that we're permitting to be applied; so - # skip this one. - return - - found_better = False + found_better = False - for item in basket.basket_items.all(): - test_price = discount.discount_product(item.product, basket.user) - if test_price is not None and item.discounted_price >= test_price: - found_better = True - break + for item in basket.basket_items.all(): + test_price = discount.discount_product(item.product, basket.user) + if test_price is not None and item.discounted_price >= test_price: + found_better = True + break - if not found_better: - return + if not found_better: + return BasketDiscount.objects.update_or_create( redeemed_by=basket.user, diff --git a/ecommerce/api_test.py b/ecommerce/api_test.py index 5d706da5d7..8663e8d9c1 100644 --- a/ecommerce/api_test.py +++ b/ecommerce/api_test.py @@ -1,5 +1,6 @@ """Tests for Ecommerce api""" +import itertools import logging import random import uuid @@ -14,7 +15,9 @@ from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.contrib.contenttypes.models import ContentType +from django.db import connection from django.test import RequestFactory +from django.test.utils import CaptureQueriesContext from django.urls import reverse from factory import Faker, fuzzy from mitol.common.utils.datetime import now_in_utc @@ -22,6 +25,7 @@ from mitol.payment_gateway.constants import MITOL_PAYMENT_GATEWAY_STRIPE from reversion.models import Version from stripe import convert_to_stripe_object +from zeal import zeal_context from courses.factories import ( CourseRunEnrollmentFactory, @@ -53,11 +57,14 @@ process_cybersource_payment_response, process_stripe_checkout_completed, process_stripe_checkout_expired, + quote_user_price, refund_order, unenroll_learner_from_order, ) from ecommerce.constants import ( DISCOUNT_TYPE_FIXED_PRICE, + DISCOUNT_TYPE_PERCENT_OFF, + PAYMENT_TYPE_FINANCIAL_ASSISTANCE, STRIPE_CHECKOUT_SESSION_STATUS_COMPLETE, STRIPE_CHECKOUT_SESSION_STATUS_EXPIRED, STRIPE_CHECKOUT_SESSION_STATUS_OPEN, @@ -108,13 +115,18 @@ FulfilledOrder, Order, OrderStatus, + PendingOrder, Product, StripeEventLog, Transaction, UserDiscount, ) from flexiblepricing.constants import FlexiblePriceStatus -from flexiblepricing.factories import FlexiblePriceFactory, FlexiblePriceTierFactory +from flexiblepricing.factories import ( + FlexiblePriceFactory, + FlexiblePriceTierFactory, + approve_flexible_price, +) from openedx.constants import EDX_ENROLLMENT_AUDIT_MODE, EDX_ENROLLMENT_VERIFIED_MODE from openedx.factories import OpenEdxUserFactory from users.factories import UserFactory @@ -1174,22 +1186,25 @@ def test_apply_discount_to_basket_prefers_a_full_credit_discount(user): @pytest.mark.parametrize( - "is_better", + ("candidate_amount", "candidate_wins"), [ - True, - False, + (50, True), + (300, False), + (200, True), ], ) -def test_apply_discount_to_basket_with_user_discount(user, is_better): - """ - Test that apply_discount_to_basket function works properly with a user discount applied. - - User discounts should take precedence over anything that the learner is - applying, whether or not it's a better discount. +def test_apply_discount_to_basket_replaces_a_user_discount_only_when_cheaper( + user, candidate_amount, candidate_wins +): + """A user-tied discount competes on price like any other: the cheaper of the + applied user-tied discount and the candidate ends up applied, and a candidate + pricing the item at exactly the applied price replaces it. """ run = CourseRunFactory.create() - product = ProductFactory.create(purchasable_object=run) + # A fixed-price discount never raises the price, so the product has to cost + # more than either amount for the two to price the item differently. + product = ProductFactory.create(purchasable_object=run, price=500) basket, _ = Basket.objects.get_or_create(user=user) BasketItem.objects.create(basket=basket, product=product, quantity=1) @@ -1198,7 +1213,7 @@ def test_apply_discount_to_basket_with_user_discount(user, is_better): amount=200, discount_type=DISCOUNT_TYPE_FIXED_PRICE ) apply_discount = UnlimitedUseDiscountFactory.create( - amount=(50 if is_better else 300), discount_type=DISCOUNT_TYPE_FIXED_PRICE + amount=candidate_amount, discount_type=DISCOUNT_TYPE_FIXED_PRICE ) UserDiscount.objects.create(user=user, discount=user_discount) @@ -1215,24 +1230,18 @@ def test_apply_discount_to_basket_with_user_discount(user, is_better): apply_discount_to_basket(basket, apply_discount) assert basket.discounts.count() == 1 - assert basket.discounts.filter(redeemed_discount=user_discount).exists() - + assert basket.discounts.get().redeemed_discount == ( + apply_discount if candidate_wins else user_discount + ) -@pytest.mark.parametrize("apply_finaid_first", [True, False]) -def test_apply_discount_to_basket_with_user_discount_and_finaid( - user, apply_finaid_first -): - """ - Test that apply_discount_to_basket function works properly with a finaid discount - and user discount applied. - User discounts should take precedence over anything that the learner is - applying, whether or not it's a better discount, unless there's a financial - assistance discount applied. +def test_apply_discount_to_basket_is_order_independent(user): + """The basket lands on the cheapest of a financial assistance, a user-tied + and an automatic discount whatever order they are applied in. """ run = CourseRunFactory.create() - product = ProductFactory.create(purchasable_object=run) + product = ProductFactory.create(purchasable_object=run, price=100) basket, _ = Basket.objects.get_or_create(user=user) finaid_tier = FlexiblePriceTierFactory(courseware_object=run.course) FlexiblePriceFactory( @@ -1242,41 +1251,52 @@ def test_apply_discount_to_basket_with_user_discount_and_finaid( status=FlexiblePriceStatus.APPROVED, ) finaid_tier.discount.discount_type = DISCOUNT_TYPE_FIXED_PRICE - finaid_tier.discount.amount = 100 + finaid_tier.discount.amount = 80 + finaid_tier.discount.payment_type = PAYMENT_TYPE_FINANCIAL_ASSISTANCE finaid_tier.discount.save() BasketItem.objects.create(basket=basket, product=product, quantity=1) user_discount = UnlimitedUseDiscountFactory.create( - amount=200, discount_type=DISCOUNT_TYPE_FIXED_PRICE + amount=90, discount_type=DISCOUNT_TYPE_FIXED_PRICE ) UserDiscount.objects.create(user=user, discount=user_discount) - BasketDiscount.objects.create( - redeemed_by=user, - redemption_date=now_in_utc(), - redeemed_discount=finaid_tier.discount if apply_finaid_first else user_discount, - redeemed_basket=basket, + automatic_discount = UnlimitedUseDiscountFactory.create( + amount=40, discount_type=DISCOUNT_TYPE_FIXED_PRICE, automatic=True ) - apply_discount_to_basket( - basket, - user_discount if apply_finaid_first else finaid_tier.discount, - allow_finaid=True, - ) + for order in itertools.permutations( + [finaid_tier.discount, user_discount, automatic_discount] + ): + BasketDiscount.objects.filter(redeemed_basket=basket).delete() - assert basket.discounts.count() == 1 - assert basket.discounts.filter(redeemed_discount=finaid_tier.discount).exists() + for discount in order: + apply_discount_to_basket(basket, discount, allow_finaid=True) + + assert basket.discounts.get().redeemed_discount == automatic_discount + # discounted_price is a cached_property reading the basket's discounts, + # so it has to be read off an item fetched after this permutation ran. + assert BasketItem.objects.get(basket=basket).discounted_price == Decimal( + "40.00" + ) + + +def test_apply_discount_to_basket_refuses_finaid_without_the_flag(user): + """A financial assistance discount stays unapplied unless allow_finaid is set.""" - regular_discount = UnlimitedUseDiscountFactory.create( - amount=50, discount_type=DISCOUNT_TYPE_FIXED_PRICE + run = CourseRunFactory.create() + product = ProductFactory.create(purchasable_object=run) + basket, _ = Basket.objects.get_or_create(user=user) + BasketItem.objects.create(basket=basket, product=product, quantity=1) + + finaid_discount = UnlimitedUseDiscountFactory.create( + payment_type=PAYMENT_TYPE_FINANCIAL_ASSISTANCE, ) - apply_discount_to_basket(basket, regular_discount) - assert basket.discounts.count() == 1 - # The finaid discount should override the user discount, so we should now - # have the regular discount, because it's better. - assert basket.discounts.filter(redeemed_discount=regular_discount).exists() + apply_discount_to_basket(basket, finaid_discount) + + assert basket.discounts.count() == 0 def test_get_auto_apply_discounts(user): # noqa: PLR0915 @@ -1442,6 +1462,251 @@ def test_get_auto_apply_discounts_respects_dates(user): assert discounts.count() == 0 +def test_quote_user_price_picks_the_cheapest_across_discount_classes(user): + """ + Financial assistance, a user-tied discount and an automatic one compete on + price alone, so the cheapest of the three wins whatever class it belongs + to -- and checkout charges the quoted price. + """ + product = ProductFactory.create(price=Decimal("100.00")) + automatic = UnlimitedUseDiscountFactory.create( + automatic=True, amount=90, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + user_tied = UnlimitedUseDiscountFactory.create( + amount=10, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + UserDiscount.objects.create(discount=user_tied, user=user) + finaid = approve_flexible_price(user, product.purchasable_object.course, 20) + + quote = quote_user_price(product, user) + + assert quote.discount == automatic + assert quote.price == Decimal("10.00") + + basket = Basket.objects.create(user=user) + BasketItem.objects.create(basket=basket, product=product, quantity=1) + for discount in (finaid, user_tied, automatic): + apply_discount_to_basket(basket, discount, allow_finaid=True) + + assert basket.basket_items.first().discounted_price == quote.price + + +def test_quote_user_price_reports_no_discount_for_a_full_price_finaid_tier(user): + """ + A candidate that quotes the list price is not worth reporting, so the + 0%-off top tier leaves the learner at list price with no discount -- and + is still reported as the aid they hold. + """ + product = ProductFactory.create(price=Decimal("100.00")) + finaid = approve_flexible_price(user, product.purchasable_object.course, 0) + + quote = quote_user_price(product, user) + + assert quote.discount is None + assert quote.price == product.price + assert quote.flexible_price_discount == finaid + + +def test_quote_user_price_quotes_list_price_without_a_user(django_assert_num_queries): + """An anonymous or absent user is quoted list price, sale or no sale, + without reading the database at all. + """ + product = ProductFactory.create() + UnlimitedUseDiscountFactory.create( + automatic=True, amount=50, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + + for caller in (AnonymousUser(), None): + with django_assert_num_queries(0): + quote = quote_user_price(product, caller) + assert quote.discount is None + assert quote.price == product.price + assert quote.flexible_price_discount is None + assert quote.source_line is None + + +def test_quote_user_price_breaks_a_price_tie_on_the_lowest_discount_id(user): + """ + Two automatic discounts quoting the same price are separated by id, so the + quote names one discount rather than depending on iteration order. + """ + product = ProductFactory.create(price=Decimal("100.00")) + first = UnlimitedUseDiscountFactory.create( + automatic=True, amount=30, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + UnlimitedUseDiscountFactory.create( + automatic=True, amount=30, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + + quote = quote_user_price(product, user) + + assert quote.discount == first + assert quote.price == Decimal("70.00") + + +def test_quote_user_price_considers_every_user_tied_discount(user): + """ + Checkout's auto-apply queryset offers every user-tied discount the learner + holds, so a second, cheaper UserDiscount row beats the first. + """ + product = ProductFactory.create(price=Decimal("100.00")) + dearer = UnlimitedUseDiscountFactory.create( + amount=10, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + cheaper = UnlimitedUseDiscountFactory.create( + amount=40, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + UserDiscount.objects.create(discount=dearer, user=user) + UserDiscount.objects.create(discount=cheaper, user=user) + + quote = quote_user_price(product, user) + + assert quote.discount == cheaper + assert quote.price == Decimal("60.00") + + +def test_quote_user_price_skips_an_automatic_tied_to_another_learner(user): + """ + An automatic discount carrying a UserDiscount for someone else is refused + at checkout, so it is not quoted to this learner either. + """ + product = ProductFactory.create() + automatic = UnlimitedUseDiscountFactory.create( + automatic=True, amount=50, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + UserDiscount.objects.create(discount=automatic, user=UserFactory.create()) + + 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 + those links name, so it does not price a product it is not linked to. + """ + product = ProductFactory.create() + linked = UnlimitedUseDiscountFactory.create( + automatic=True, amount=50, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + DiscountProduct.objects.create(discount=linked, product=ProductFactory.create()) + + quote = quote_user_price(product, user) + + assert quote.discount is None + assert quote.price == product.price + + +def test_quote_user_price_matches_checkout_for_linked_purchase(paid_amount_off_source): + """ + The quoted price is the price PendingOrder charges for that discount, and + the quote names the prior purchase the credit is spent from. + """ + program_product = paid_amount_off_source.program_product + user = paid_amount_off_source.user + + quote = quote_user_price(program_product, user) + order = PendingOrder.create_from_product(program_product, user, quote.discount) + + assert quote.discount == paid_amount_off_source.discount + assert quote.price == Decimal("899.00") + assert quote.source_line == paid_amount_off_source.source_line + assert order.total_price_paid == quote.price + + +def test_quote_user_price_confines_financial_assistance_to_its_courseware(user): + """ + A tier discount carries no product links, so nothing but the aid lookup + confines it: a product the learner was not approved for is quoted list + price and reports no aid. + """ + other = ProductFactory.create() + approve_flexible_price(user, CourseRunFactory.create().course, 25) + + quote = quote_user_price(other, user) + + assert quote.discount is None + assert quote.price == other.price + assert quote.flexible_price_discount is None + + +def test_quote_user_price_keeps_an_automatic_that_is_also_a_tier_discount(user): + """ + A discount that qualifies on its own -- here an automatic sale a tier also + points at -- prices a product the learner holds no aid for, so being + someone's tier discount does not narrow it to that courseware. + """ + plain = ProductFactory.create(price=Decimal("100.00")) + shared = UnlimitedUseDiscountFactory.create( + automatic=True, amount=25, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + aided_course = CourseRunFactory.create().course + tier = FlexiblePriceTierFactory.create( + courseware_object=aided_course, discount=shared + ) + FlexiblePriceFactory.create( + user=user, + courseware_object=aided_course, + tier=tier, + status=FlexiblePriceStatus.APPROVED, + ) + + quote = quote_user_price(plain, user) + + assert quote.discount == shared + assert quote.price == Decimal("75.00") + + +def test_quote_user_price_query_count_does_not_grow_with_unrelated_discounts( + paid_amount_off_source, +): + """ + One quote costs what the learner's own applicable discounts cost and + nothing more. Product scope is a filter on the candidate query rather than + a check per candidate, so five automatic discounts on sale elsewhere leave + the count untouched -- without that, each one costs queries whether or not + it can price this product. + """ + user = paid_amount_off_source.user + program_product = paid_amount_off_source.program_product + approve_flexible_price(user, CourseRunFactory.create().course, 25) + UserDiscount.objects.create( + discount=UnlimitedUseDiscountFactory.create( + amount=10, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ), + user=user, + ) + + def reload_product(): + return Product.objects.get(id=program_product.id) + + # ContentType.objects.get_for_model caches per process, so the first quote + # pays for the lookups behind the source resolve and the second does not. + quote_user_price(reload_product(), user) + + alone = reload_product() + with zeal_context(), CaptureQueriesContext(connection) as before: + quote_user_price(alone, user) + + for _ in range(5): + elsewhere = OneTimePerUserDiscountFactory.create( + automatic=True, amount=5, discount_type=DISCOUNT_TYPE_PERCENT_OFF + ) + DiscountProduct.objects.create( + discount=elsewhere, product=ProductFactory.create() + ) + + crowded = reload_product() + with zeal_context(), CaptureQueriesContext(connection) as after: + quote = quote_user_price(crowded, user) + + assert quote.discount == paid_amount_off_source.discount + assert quote.price == Decimal("899.00") + assert len(after) == len(before) + + @pytest.mark.parametrize( "no_delay", [ diff --git a/ecommerce/conftest.py b/ecommerce/conftest.py index 422c3f5e72..f49d936db2 100644 --- a/ecommerce/conftest.py +++ b/ecommerce/conftest.py @@ -1,19 +1,9 @@ """Common fixtures for ecommerce tests""" -from decimal import Decimal -from types import SimpleNamespace - import pytest -import reversion -from reversion.models import Version -from courses.factories import CourseRunFactory, ProgramFactory -from ecommerce.factories import ( - PaidAmountOffDiscountFactory, - ProgramProductFactory, - make_purchase, -) -from ecommerce.models import DiscountProduct +from courses.factories import CourseRunFactory +from ecommerce.factories import make_paid_amount_off_offer @pytest.fixture(autouse=True) @@ -23,26 +13,5 @@ def mocked_hubspot_deal_sync(mocker): @pytest.fixture def paid_amount_off_source(user): - """ - One learner holding exactly one qualifying source: a $999 program product, - a $100 paid run of a direct child course, and a paid-amount-off discount - linked to the program product. Resolving it returns a 100.00 credit, so the - program prices at 899.00. - """ - program = ProgramFactory.create() - run = CourseRunFactory.create() - program.add_requirement(run.course) - source_line = make_purchase(user, run, Decimal("100.00")) - with reversion.create_revision(): - program_product = ProgramProductFactory.create( - purchasable_object=program, price=Decimal("999.00") - ) - discount = PaidAmountOffDiscountFactory.create() - DiscountProduct.objects.create(discount=discount, product=program_product) - return SimpleNamespace( - user=user, - program_product=program_product, - program_product_version=Version.objects.get_for_object(program_product).first(), - source_line=source_line, - discount=discount, - ) + """One learner holding a run purchase that funds a paid-amount-off credit.""" + return make_paid_amount_off_offer(user, CourseRunFactory.create()) diff --git a/ecommerce/constants.py b/ecommerce/constants.py index ae2a514564..2e7d8e57f5 100644 --- a/ecommerce/constants.py +++ b/ecommerce/constants.py @@ -6,6 +6,8 @@ MITOL_PAYMENT_GATEWAY_STRIPE, ) +from courses.constants import CONTENT_TYPE_MODEL_COURSE, CONTENT_TYPE_MODEL_PROGRAM + REFERENCE_NUMBER_PREFIX = "mitxonline-" # Standard self-service refund window, per the terms of service: learners may @@ -38,6 +40,16 @@ zip(STANDARD_DISCOUNT_TYPES, STANDARD_DISCOUNT_TYPES) ) +# The courseware a paid-amount-off discount credits a prior purchase of. A run +# purchase is credited to its course, so a run never appears here. These are +# the same two tokens the course and program serializers publish as `type`, +# which is what lets a client key both off one vocabulary. +ALL_DISCOUNT_SOURCE_TYPES = [ + CONTENT_TYPE_MODEL_COURSE, + CONTENT_TYPE_MODEL_PROGRAM, +] +DISCOUNT_SOURCE_TYPES = list(zip(ALL_DISCOUNT_SOURCE_TYPES, ALL_DISCOUNT_SOURCE_TYPES)) + REDEMPTION_TYPE_ONE_TIME = "one-time" REDEMPTION_TYPE_ONE_TIME_PER_USER = "one-time-per-user" REDEMPTION_TYPE_UNLIMITED = "unlimited" diff --git a/ecommerce/discount_sources.py b/ecommerce/discount_sources.py index e6b5e19104..c9ae4d9aab 100644 --- a/ecommerce/discount_sources.py +++ b/ecommerce/discount_sources.py @@ -235,6 +235,19 @@ def source_line_for(discount, user, products) -> Line | None: return resolution.source_line if resolution else None +def credited_courseware(source_line): + """ + The course or program ``source_line`` credits a paid-amount-off discount + with. A run purchase credits its course: that is the title the learner + recognizes and the id Learn links by. + + resolve_program_child_purchase only ever matches a run or a program, so + those are the only two shapes a source line arrives in. + """ + purchased = source_line.purchased_object + return purchased.course if isinstance(purchased, CourseRun) else purchased + + def has_paid_amount_off(discounts) -> bool: """Whether any of ``discounts`` spends a source, i.e. needs resolving.""" return any(spends_source(discount) for discount in discounts) diff --git a/ecommerce/factories.py b/ecommerce/factories.py index aa27c4a032..464145ec00 100644 --- a/ecommerce/factories.py +++ b/ecommerce/factories.py @@ -1,3 +1,6 @@ +from decimal import Decimal +from types import SimpleNamespace + import faker import reversion from factory import LazyAttribute, SubFactory, fuzzy @@ -5,6 +8,7 @@ from reversion.models import Version from courses.factories import CourseRunFactory, ProgramFactory +from courses.models import CourseRun from ecommerce import models from ecommerce.constants import ( DISCOUNT_TYPE_PAID_AMOUNT_OFF, @@ -171,3 +175,31 @@ def make_purchase( quantity=1, discounted_unit_price=charged, ) + + +def make_paid_amount_off_offer(user, purchased): + """ + One learner holding exactly one qualifying source for a paid-amount-off + discount: a $999 program product whose requirement tree contains + ``purchased`` -- a course run or a sub-program -- and a $100 fulfilled + purchase of it. Resolving the discount returns a 100.00 credit, so the + program prices at 899.00. + """ + program = ProgramFactory.create() + program.add_requirement( + purchased.course if isinstance(purchased, CourseRun) else purchased + ) + source_line = make_purchase(user, purchased, Decimal("100.00")) + with reversion.create_revision(): + program_product = ProgramProductFactory.create( + purchasable_object=program, price=Decimal("999.00") + ) + discount = PaidAmountOffDiscountFactory.create() + models.DiscountProduct.objects.create(discount=discount, product=program_product) + return SimpleNamespace( + user=user, + program_product=program_product, + program_product_version=Version.objects.get_for_object(program_product).first(), + source_line=source_line, + discount=discount, + ) diff --git a/ecommerce/serializers/v0/__init__.py b/ecommerce/serializers/v0/__init__.py index 2c06a79733..beb6ab707f 100644 --- a/ecommerce/serializers/v0/__init__.py +++ b/ecommerce/serializers/v0/__init__.py @@ -9,12 +9,17 @@ from rest_framework import serializers from cms.serializers import CoursePageSerializer, ProgramPageSerializer +from courses.constants import CONTENT_TYPE_MODEL_COURSE, CONTENT_TYPE_MODEL_PROGRAM from courses.models import Course, CourseRun, Program, ProgramRun from ecommerce import models from ecommerce.constants import ( CYBERSOURCE_CARD_TYPES, + DISCOUNT_SOURCE_TYPES, + DISCOUNT_TYPES, + PAYMENT_TYPES, TRANSACTION_TYPE_REFUND, ) +from ecommerce.discount_sources import credited_courseware from ecommerce.models import ( Basket, BasketItem, @@ -737,6 +742,143 @@ class Meta: model = models.Product +class DiscountSourceSerializer(serializers.Serializer): + """The prior purchase a discount credits.""" + + type = serializers.ChoiceField(choices=DISCOUNT_SOURCE_TYPES) + readable_id = serializers.CharField() + title = serializers.CharField() + + +class UserPricingDiscountSerializer(serializers.Serializer): + """The discount checkout would apply to a product for this user.""" + + id = serializers.IntegerField() + discount_code = serializers.CharField() + discount_type = serializers.ChoiceField(choices=DISCOUNT_TYPES) + # required=False matches V0Discount's derivation from the nullable model + # column; openapi-generator types a required nullable enum as non-null. + payment_type = serializers.ChoiceField( + choices=PAYMENT_TYPES, + allow_null=True, + required=False, + help_text=( + "What kind of discount won: `financial-assistance` is the learner's " + "approved aid tier; the other values say how a discount was funded. " + "Null on discounts created without one." + ), + ) + amount_off = serializers.DecimalField( + max_digits=7, + decimal_places=2, + help_text=( + "Dollars taken off `price` for this user. For paid-amount-off " + "discounts this is the prior purchase's paid price, capped at " + "`price`, never the stored amount." + ), + ) + source = DiscountSourceSerializer( + allow_null=True, + help_text=( + "Set only for paid-amount-off discounts (`discount_type` is the " + "discriminator): the prior purchase being credited." + ), + ) + + def get_attribute(self, instance): + """ + Shape the winning discount out of the request's quote. The quote is the + only place the resolved amount and the credited purchase exist; neither + is stored on the Discount row. + """ + quote = self.context["quote"] + if quote.discount is None: + return None + return { + "id": quote.discount.id, + "discount_code": quote.discount.discount_code, + "discount_type": quote.discount.discount_type, + "payment_type": quote.discount.payment_type, + "amount_off": instance.price - quote.price, + "source": self._source(quote.source_line), + } + + @staticmethod + def _source(source_line): + if source_line is None: + return None + courseware = credited_courseware(source_line) + return { + "type": CONTENT_TYPE_MODEL_PROGRAM + if isinstance(courseware, Program) + else CONTENT_TYPE_MODEL_COURSE, + "readable_id": courseware.readable_id, + "title": courseware.title, + } + + +class _QuotedPriceField(serializers.DecimalField): + """ + The quoted price, which belongs to the request rather than to the product, + so it is read from serializer context. Declaring it as a real DecimalField + rather than a method field is what keeps it a decimal string on the wire, + like every other price the API publishes. + """ + + def get_attribute(self, instance): # noqa: ARG002 + return self.context["quote"].price + + +# Subclassing the deprecated endpoint's serializer is what makes this payload a +# strict superset of that one rather than a copy of it: every field a caller +# reads from user_flexible_price is inherited here, so moving off it +# (https://github.com/mitodl/hq/issues/12799) cannot lose one. The docstring is +# the public schema description, so the rationale lives here instead. +class UserPricingProductSerializer(ProductFlexiblePriceSerializer): + """A product priced for one user.""" + + # The quote already determined the learner's aid while gathering its + # candidates, so this reads it from context instead of repeating the + # lookup the inherited method makes. + @extend_schema_field( + V0DiscountSerializer( + allow_null=True, + help_text=( + "The learner's approved financial-assistance tier discount, or " + "null: whether they are approved and at which tier, even when " + "that tier is 0% or another discount wins. Read `user_price` " + "for the price, not this field's `amount`." + ), + ) + ) + def get_product_flexible_price(self, instance): # noqa: ARG002 + finaid = self.context["quote"].flexible_price_discount + if finaid is None: + return None + return V0DiscountSerializer(finaid, context=self.context).data + + user_price = _QuotedPriceField( + max_digits=7, + decimal_places=2, + help_text="What this user pays at checkout today.", + ) + discount = UserPricingDiscountSerializer( + allow_null=True, + help_text=( + "The discount checkout applies to this product for this user, or " + "null at list price." + ), + ) + + class Meta: + fields = [ + *ProductFlexiblePriceSerializer.Meta.fields, + "user_price", + "discount", + ] + model = models.Product + + class DiscountRedemptionSerializer(serializers.ModelSerializer): """Serializes a discount redemption.""" diff --git a/ecommerce/views/v0/__init__.py b/ecommerce/views/v0/__init__.py index d64fddc8b0..9e52a61da2 100644 --- a/ecommerce/views/v0/__init__.py +++ b/ecommerce/views/v0/__init__.py @@ -35,6 +35,7 @@ from rest_framework_extensions.mixins import NestedViewSetMixin from b2b.api import is_product_courserun, is_product_program +from b2b.serializers.v0.manager import DetailErrorSerializer from courses.models import ( Course, CourseRun, @@ -51,6 +52,7 @@ generate_checkout_payload, generate_discount_code, get_auto_apply_discounts_for_basket, + quote_user_price, ) from ecommerce.discount_sources import funds_fulfilled_redemption_exists from ecommerce.exceptions import ProductBlockedError @@ -82,6 +84,7 @@ RefundRequestSerializer, UserDiscountMetaSerializer, UserDiscountSerializer, + UserPricingProductSerializer, V0DiscountSerializer, requests, ) @@ -650,7 +653,11 @@ def get_queryset(self): @extend_schema( operation_id="products_user_flexible_price_retrieve", - description="Retrieve a product with user-specific flexible price information", + description=( + "Retrieve a product with user-specific flexible price information. " + "Use `user_pricing` instead." + ), + deprecated=True, responses={ 200: ProductFlexiblePriceSerializer, }, @@ -669,6 +676,42 @@ def user_flexible_price(self, request, **kwargs): # noqa: ARG002 ) return Response(serializer.data) + @extend_schema( + operation_id="products_user_pricing_retrieve", + description=( + "The price this user pays for this product, computed the way " + "checkout computes it (financial assistance, user-tied and " + "automatic discounts, including paid-amount-off credit for a " + "qualifying prior purchase). The response also carries " + "product_flexible_price exactly as the deprecated " + "user_flexible_price endpoint returns it, so a caller moves over " + "field for field. Anonymous requests are a 403; an unknown or " + "no-longer-purchasable product is a 404." + ), + responses={ + 200: UserPricingProductSerializer, + 403: DetailErrorSerializer, + 404: DetailErrorSerializer, + }, + ) + @action( + detail=True, + methods=["get"], + permission_classes=[IsAuthenticated], + url_path="user_pricing", + ) + def user_pricing(self, request, **kwargs): # noqa: ARG002 + """Quote the per-user price of a product.""" + product = self.get_object() + serializer = UserPricingProductSerializer( + product, + context={ + "request": request, + "quote": quote_user_price(product, request.user), + }, + ) + return Response(serializer.data) + class DiscountFilterSet(django_filters.FilterSet): """Custom filtering for discounts.""" diff --git a/ecommerce/views/v0/views_test.py b/ecommerce/views/v0/views_test.py index 57b26c45c0..148db7967f 100644 --- a/ecommerce/views/v0/views_test.py +++ b/ecommerce/views/v0/views_test.py @@ -21,6 +21,7 @@ from b2b.constants import CONTRACT_MEMBERSHIP_CODE, CONTRACT_MEMBERSHIP_MANAGED from b2b.factories import ContractPageFactory +from courses.constants import CONTENT_TYPE_MODEL_COURSE, CONTENT_TYPE_MODEL_PROGRAM from courses.factories import ( BlockedCountryFactory, CourseRunEnrollmentFactory, @@ -53,6 +54,7 @@ ProgramProductFactory, TransactionFactory, UnlimitedUseDiscountFactory, + make_paid_amount_off_offer, make_purchase, ) from ecommerce.models import ( @@ -76,7 +78,11 @@ ProductSerializer, ) from flexiblepricing.constants import FlexiblePriceStatus -from flexiblepricing.factories import FlexiblePriceFactory, FlexiblePriceTierFactory +from flexiblepricing.factories import ( + FlexiblePriceFactory, + FlexiblePriceTierFactory, + approve_flexible_price, +) from main.constants import ( USER_MSG_TYPE_B2B_ERROR_MISSING_ENROLLMENT_CODE, USER_MSG_TYPE_BASKET_EMPTY, @@ -264,6 +270,90 @@ def test_product_user_flexible_price_unauthenticated(client, products): assert resp_data["product_flexible_price"] is None +@pytest.mark.parametrize("purchased_a_program", [False, True]) +def test_user_pricing_quotes_the_paid_amount_off_credit( + user_client, user, purchased_a_program +): + """ + An eligible learner sees the program at price minus their child purchase, + and the credit names the courseware they bought: a run purchase names its + course, a sub-program purchase names the program. + """ + purchased = ( + ProgramFactory.create() if purchased_a_program else CourseRunFactory.create() + ) + offer = make_paid_amount_off_offer(user, purchased) + credited = purchased if purchased_a_program else purchased.course + + resp = user_client.get( + reverse( + "v0:products_api-user-pricing", + kwargs={"pk": offer.program_product.id}, + ) + ) + + assert resp.status_code == 200 + quoted = resp.json() + assert quoted["user_price"] == "899.00" + assert quoted["discount"]["discount_type"] == DISCOUNT_TYPE_PAID_AMOUNT_OFF + assert quoted["discount"]["amount_off"] == "100.00" + assert quoted["discount"]["source"] == { + "type": CONTENT_TYPE_MODEL_PROGRAM + if purchased_a_program + else CONTENT_TYPE_MODEL_COURSE, + "readable_id": credited.readable_id, + "title": credited.title, + } + + +def test_user_pricing_requires_a_signed_in_user(client): + """ + An anonymous request is a 403, not a list-price quote: the answer is + per-user, and a silent anonymous fallback would hide a caller whose + session did not reach this host. + """ + product = ProductFactory.create() + + resp = client.get( + reverse("v0:products_api-user-pricing", kwargs={"pk": product.id}) + ) + + assert resp.status_code == 403 + + +def test_user_pricing_returns_the_single_product_flexible_price_data(user_client, user): + """hq#12799: the response carries exactly what user_flexible_price returns.""" + product = ProductFactory.create() + finaid = approve_flexible_price(user, product.purchasable_object.course, 50) + + single = user_client.get( + reverse("v0:products_api-user-flexible-price", kwargs={"pk": product.id}) + ).json() + quoted = user_client.get( + reverse("v0:products_api-user-pricing", kwargs={"pk": product.id}) + ).json() + + assert single["product_flexible_price"]["id"] == finaid.id + assert {key: quoted[key] for key in single} == single + assert quoted["discount"]["id"] == finaid.id + assert quoted["discount"]["payment_type"] == PAYMENT_TYPE_FINANCIAL_ASSISTANCE + assert quoted["discount"]["source"] is None + + +def test_user_pricing_404s_for_a_product_the_queryset_excludes(user_client): + """A product whose run closed enrollment is a 404, like an unknown id.""" + closed_run = CourseRunFactory.create( + enrollment_end=now_in_utc() - timedelta(days=1) + ) + closed = ProductFactory.create(purchasable_object=closed_run) + + resp = user_client.get( + reverse("v0:products_api-user-pricing", kwargs={"pk": closed.id}) + ) + + assert resp.status_code == 404 + + def test_get_basket(user_drf_client, user): """Test the view that returns a state of Basket""" basket = BasketFactory.create(user=user) diff --git a/flexiblepricing/factories.py b/flexiblepricing/factories.py index 96c47d50ba..81626f053b 100644 --- a/flexiblepricing/factories.py +++ b/flexiblepricing/factories.py @@ -6,7 +6,11 @@ from mitol.common.utils import now_in_utc from courses.factories import CourseFactory -from ecommerce.factories import DiscountFactory +from ecommerce.constants import ( + DISCOUNT_TYPE_PERCENT_OFF, + PAYMENT_TYPE_FINANCIAL_ASSISTANCE, +) +from ecommerce.factories import DiscountFactory, UnlimitedUseDiscountFactory from flexiblepricing import models from flexiblepricing.constants import FlexiblePriceStatus from users.factories import UserFactory @@ -57,3 +61,26 @@ class FlexiblePriceFactory(DjangoModelFactory): class Meta: model = models.FlexiblePrice + + +def approve_flexible_price(user, courseware, amount): + """ + An approved percent-off financial-assistance tier on ``courseware`` for + ``user``, marked financial-assistance the way configure_tiers marks it. + Returns the tier's discount, which is what pricing applies. + """ + tier = FlexiblePriceTierFactory.create( + courseware_object=courseware, + discount=UnlimitedUseDiscountFactory.create( + amount=amount, + discount_type=DISCOUNT_TYPE_PERCENT_OFF, + payment_type=PAYMENT_TYPE_FINANCIAL_ASSISTANCE, + ), + ) + FlexiblePriceFactory.create( + user=user, + courseware_object=courseware, + tier=tier, + status=FlexiblePriceStatus.APPROVED, + ) + return tier.discount diff --git a/main/sentry.py b/main/sentry.py index e030cdfb76..6fa1148bab 100644 --- a/main/sentry.py +++ b/main/sentry.py @@ -1,6 +1,7 @@ """Sentry setup and configuration""" import logging +import re import sentry_sdk from celery.exceptions import WorkerLostError @@ -15,6 +16,62 @@ log = logging.getLogger() +# Postgres appends a DETAIL line to constraint violations that echoes the whole +# offending row -- on a users table that is the learner's name, email and +# external UUID. psycopg puts it in str(exc), so it ships inside the exception +# value, where no SDK privacy setting reaches it: send_default_pii governs +# user/cookie/header capture and max_request_body_size governs request bodies, +# and neither touches exception text. +# +# The newline is matched both raw and as a literal backslash-n: the SDK repr()s +# frame locals and non-string logging params during serialization, so there the +# DETAIL line arrives as "...constraint\\nDETAIL: ..." inside a repr string. +PG_DETAIL_RE = re.compile(r"(\n|\\n)DETAIL:.*", re.DOTALL) + + +def scrub_pg_detail(text): + """Truncate a Postgres error string at its DETAIL line. + + Keeps the primary message, which is what identifies the failure, and drops + the row echo plus any HINT/CONTEXT Postgres appends after it. + """ + return PG_DETAIL_RE.sub( + lambda match: match.group(1) + "DETAIL: [scrubbed]", text, count=1 + ) + + +def scrub_pg_details(event): + """Truncate Postgres DETAIL lines everywhere in a Sentry event. + + The row echo reaches Sentry through more fields than the exception value: + LoggingIntegration puts the log message in a breadcrumb + (BreadcrumbHandler._breadcrumb_from_record), logger.error("...: %s", exc) + puts it in logentry.params (EventHandler._emit), and captured stack-frame + locals carry it in frame vars because include_local_variables defaults to + True (serialize_frame). Walking the whole event covers those without + enumerating them, and does not go stale when the SDK adds another. + + Safe to walk naively because Client._prepare_event serializes the event + before calling before_send, so every leaf here is already a JSON + primitive -- no live exception objects to coerce. + """ + return _scrub_node(event) + + +def _scrub_node(node): + """Recurse through the serialized event, rewriting strings in place.""" + if isinstance(node, str): + return scrub_pg_detail(node) + if isinstance(node, dict): + for key, value in node.items(): + node[key] = _scrub_node(value) + return node + if isinstance(node, list): + node[:] = [_scrub_node(item) for item in node] + return node + return node + + def before_send(event, hint): """ Filter or transform events before they're sent to Sentry @@ -31,7 +88,7 @@ def before_send(event, hint): if isinstance(exc_value, SHUTDOWN_ERRORS): # so we don't want to report expected shutdown errors to sentry return None - return event + return scrub_pg_details(event) def init_sentry( # noqa: PLR0913 @@ -74,6 +131,13 @@ def init_sentry( # noqa: PLR0913 environment=environment, release=version, before_send=before_send, + # Request bodies are NOT gated on send_default_pii: the SDK sets + # request.data unconditionally (RequestExtractor.extract_into_event) + # and this is the only control (request_body_within_bounds). Left + # unset it defaults to "medium", i.e. 10,000-byte bodies -- enrollment, + # checkout, profile and SCIM payloads. Set explicitly so the choice is + # findable here rather than in a dependency's defaults. + max_request_body_size="small", send_default_pii=send_default_pii, traces_sample_rate=traces_sample_rate, profiles_sample_rate=profiles_sample_rate, diff --git a/main/sentry_test.py b/main/sentry_test.py new file mode 100644 index 0000000000..96cbe47e5c --- /dev/null +++ b/main/sentry_test.py @@ -0,0 +1,198 @@ +"""Tests for Sentry event scrubbing.""" + +import json +import logging + +import pytest +import sentry_sdk +from sentry_sdk.integrations.logging import LoggingIntegration +from sentry_sdk.transport import Transport + +from main.sentry import ( + before_send, + scrub_pg_detail, + scrub_pg_details, +) + +# A real MITXONLINE-6PK exception value, with the learner identifiers replaced. +PG_INTEGRITY_ERROR = ( + 'null value in column "name" of relation "users_user" violates not-null ' + "constraint\n" + "DETAIL: Failing row contains (1863408, , 2026-08-07 18:38:14.503726+00, f, " + "learner@example.invalid, learner@example.invalid, null, f, t, " + "12d7dfc5-6f84-46db-9383-2d7079434173, 1863408, learner@example.invalid, f)." +) +PG_PRIMARY_MESSAGE = ( + 'null value in column "name" of relation "users_user" violates not-null constraint' +) + + +class FakeTransport(Transport): + """Collect outgoing events instead of sending them.""" + + def __init__(self): + super().__init__() + self.events = [] + + def capture_envelope(self, envelope): + self.events.extend( + item.payload.json for item in envelope.items if item.type == "event" + ) + + +@pytest.fixture +def sentry_transport(): + """Initialize the real SDK with before_send, and detach it afterwards.""" + transport = FakeTransport() + sentry_sdk.init( + dsn="https://k@o0.ingest.sentry.io/0", + transport=transport, + before_send=before_send, + default_integrations=False, + integrations=[ + LoggingIntegration(level=logging.INFO, event_level=logging.ERROR) + ], + ) + yield transport + sentry_sdk.get_global_scope().set_client(None) + + +def test_detail_line_is_truncated(): + """The row echo goes; the primary error that names the failure stays.""" + scrubbed = scrub_pg_detail(PG_INTEGRITY_ERROR) + assert scrubbed.startswith(PG_PRIMARY_MESSAGE) + assert "learner@example.invalid" not in scrubbed + assert "12d7dfc5-6f84-46db-9383-2d7079434173" not in scrubbed + + +def test_escaped_detail_line_is_truncated(): + """repr() turns the newline into a literal backslash-n; that form goes too.""" + scrubbed = scrub_pg_detail(repr(Exception(PG_INTEGRITY_ERROR))) + assert PG_PRIMARY_MESSAGE in scrubbed + assert "learner@example.invalid" not in scrubbed + + +def test_message_without_detail_is_unchanged(): + """A message with no DETAIL line passes through untouched.""" + message = "connection to server failed" + assert scrub_pg_detail(message) == message + + +def test_hint_and_context_after_detail_are_dropped(): + """HINT and CONTEXT follow DETAIL and can quote row data too.""" + text = "boom\nDETAIL: row data\nHINT: try again\nCONTEXT: SQL statement" + scrubbed = scrub_pg_detail(text) + assert "row data" not in scrubbed + assert "try again" not in scrubbed + assert "SQL statement" not in scrubbed + + +def test_scrubs_exception_values_logentry_and_message(): + """Every place the SDK can put an error string is covered.""" + event = { + "exception": {"values": [{"value": PG_INTEGRITY_ERROR}]}, + "logentry": { + "message": PG_INTEGRITY_ERROR, + "formatted": PG_INTEGRITY_ERROR, + }, + "message": PG_INTEGRITY_ERROR, + } + scrub_pg_details(event) + assert "learner@example.invalid" not in repr(event) + + +def test_before_send_scrubs_the_event(): + """The scrub is wired into the before_send hook, not just callable.""" + event = {"exception": {"values": [{"value": PG_INTEGRITY_ERROR}]}} + assert "learner@example.invalid" not in repr(before_send(event, {})) + + +def test_scrubs_breadcrumb_messages(): + """LoggingIntegration records the log message as a breadcrumb.""" + event = { + "breadcrumbs": { + "values": [ + { + "type": "log", + "category": "django_scim.views", + "message": PG_INTEGRITY_ERROR, + } + ] + } + } + scrub_pg_details(event) + assert "learner@example.invalid" not in repr(event) + + +def test_scrubs_logentry_params(): + """logger.error("...: %s", exc) puts the repr'd exception in logentry.params.""" + event = { + "logentry": { + "message": "Unable to complete SCIM call: %s", + "formatted": "Unable to complete SCIM call: " + PG_INTEGRITY_ERROR, + "params": [repr(Exception(PG_INTEGRITY_ERROR))], + } + } + scrub_pg_details(event) + assert "learner@example.invalid" not in repr(event) + + +def test_scrubs_captured_frame_locals(): + """include_local_variables defaults to True, so repr'd frame vars carry it.""" + event = { + "exception": { + "values": [ + { + "value": "boom", + "stacktrace": { + "frames": [ + { + "function": "save", + "vars": { + "exc": repr(Exception(PG_INTEGRITY_ERROR)), + "retries": 3, + }, + } + ] + }, + } + ] + } + } + scrub_pg_details(event) + assert "learner@example.invalid" not in repr(event) + frame = event["exception"]["values"][0]["stacktrace"]["frames"][0] + assert frame["vars"]["retries"] == 3 + + +def test_walk_preserves_non_string_leaves(): + """The walk must not coerce timestamps, ints or None into strings.""" + event = { + "timestamp": 1757345533.179, + "level": "error", + "extra": {"count": 42, "missing": None, "flag": True}, + "message": PG_INTEGRITY_ERROR, + } + scrub_pg_details(event) + assert event["timestamp"] == 1757345533.179 + assert event["extra"] == {"count": 42, "missing": None, "flag": True} + assert "learner@example.invalid" not in event["message"] + + +def test_real_sdk_scrubs_params_and_local_variables(sentry_transport): + """Go through the real SDK, which repr()s params and locals before before_send.""" + + def save(): + exc = Exception(PG_INTEGRITY_ERROR) + raise exc + + try: + save() + except Exception as e: # noqa: BLE001 + logging.getLogger("x").error("Unable to save: %s", e) # noqa: TRY400 + sentry_sdk.capture_exception(e) + sentry_sdk.flush() + + assert len(sentry_transport.events) == 2 + for event in sentry_transport.events: + assert "learner@example.invalid" not in json.dumps(event) diff --git a/main/settings.py b/main/settings.py index 5c4933f7ba..b4fbb0797f 100644 --- a/main/settings.py +++ b/main/settings.py @@ -39,7 +39,7 @@ from main.sentry import init_sentry from openapi.settings_spectacular import open_spectacular_settings -VERSION = "1.166.2" +VERSION = "1.166.3" log = logging.getLogger() diff --git a/openapi/settings_spectacular.py b/openapi/settings_spectacular.py index a25faffe94..c06d9c7244 100644 --- a/openapi/settings_spectacular.py +++ b/openapi/settings_spectacular.py @@ -40,6 +40,7 @@ "OnboardingStateEnum": "b2b.constants.ONBOARDING_STATE_CHOICES", "IdentityProviderLifecycleStateEnum": "b2b.constants.IDP_LIFECYCLE_CHOICES", "IdentityProviderProtocolEnum": "b2b.constants.IDP_PROTOCOL_CHOICES", + "DiscountSourceTypeEnum": "ecommerce.constants.DISCOUNT_SOURCE_TYPES", }, "POSTPROCESSING_HOOKS": [ "drf_spectacular.hooks.postprocess_schema_enums", diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 47f43fef57..d4bcb31d17 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -2849,7 +2849,8 @@ paths: /api/v0/products/{id}/user_flexible_price/: get: operationId: products_user_flexible_price_retrieve - description: Retrieve a product with user-specific flexible price information + description: Retrieve a product with user-specific flexible price information. + Use `user_pricing` instead. parameters: - in: path name: id @@ -2859,6 +2860,7 @@ paths: required: true tags: - products + deprecated: true responses: '200': content: @@ -2866,6 +2868,43 @@ paths: schema: $ref: '#/components/schemas/ProductFlexiblePrice' description: '' + /api/v0/products/{id}/user_pricing/: + get: + operationId: products_user_pricing_retrieve + description: The price this user pays for this product, computed the way checkout + computes it (financial assistance, user-tied and automatic discounts, including + paid-amount-off credit for a qualifying prior purchase). The response also + carries product_flexible_price exactly as the deprecated user_flexible_price + endpoint returns it, so a caller moves over field for field. Anonymous requests + are a 403; an unknown or no-longer-purchasable product is a 404. + parameters: + - in: path + name: id + schema: + type: integer + description: A unique integer value identifying this product. + required: true + tags: + - products + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/UserPricingProduct' + description: '' + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/DetailError' + description: '' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/DetailError' + description: '' /api/v0/products/all/: get: operationId: products_all_list @@ -7097,6 +7136,31 @@ components: - redeemed_by - redeemed_discount - redeemed_order + DiscountSource: + type: object + description: The prior purchase a discount credits. + properties: + type: + $ref: '#/components/schemas/DiscountSourceTypeEnum' + readable_id: + type: string + title: + type: string + required: + - readable_id + - title + - type + DiscountSourceTypeEnum: + enum: + - course + - program + type: string + description: |- + * `course` - course + * `program` - program + x-enum-descriptions: + - course + - program DiscountTypeEnum: enum: - percent-off @@ -10311,6 +10375,92 @@ components: required: - discount - user + UserPricingDiscount: + type: object + description: The discount checkout would apply to a product for this user. + properties: + id: + type: integer + discount_code: + type: string + discount_type: + $ref: '#/components/schemas/DiscountTypeEnum' + payment_type: + nullable: true + description: |- + What kind of discount won: `financial-assistance` is the learner's approved aid tier; the other values say how a discount was funded. Null on discounts created without one. + + * `marketing` - marketing + * `sales` - sales + * `financial-assistance` - financial-assistance + * `customer-support` - customer-support + * `staff` - staff + * `legacy` - legacy + oneOf: + - $ref: '#/components/schemas/PaymentTypeEnum' + - $ref: '#/components/schemas/NullEnum' + amount_off: + type: string + format: decimal + pattern: ^-?\d{0,5}(?:\.\d{0,2})?$ + description: Dollars taken off `price` for this user. For paid-amount-off + discounts this is the prior purchase's paid price, capped at `price`, + never the stored amount. + source: + allOf: + - $ref: '#/components/schemas/DiscountSource' + nullable: true + description: 'Set only for paid-amount-off discounts (`discount_type` is + the discriminator): the prior purchase being credited.' + required: + - amount_off + - discount_code + - discount_type + - id + - source + UserPricingProduct: + type: object + description: A product priced for one user. + properties: + id: + type: integer + readOnly: true + price: + type: string + format: decimal + pattern: ^-?\d{0,5}(?:\.\d{0,2})?$ + description: + type: string + is_active: + type: boolean + description: Controls visibility of the product in the app. + product_flexible_price: + allOf: + - $ref: '#/components/schemas/V0Discount' + nullable: true + description: 'The learner''s approved financial-assistance tier discount, + or null: whether they are approved and at which tier, even when that tier + is 0% or another discount wins. Read `user_price` for the price, not this + field''s `amount`.' + readOnly: true + user_price: + type: string + format: decimal + pattern: ^-?\d{0,5}(?:\.\d{0,2})?$ + description: What this user pays at checkout today. + discount: + allOf: + - $ref: '#/components/schemas/UserPricingDiscount' + nullable: true + description: The discount checkout applies to this product for this user, + or null at list price. + required: + - description + - discount + - id + - price + - product_flexible_price + - user_price UserProfile: type: object description: Serializer for profile diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 83e90a6fb9..8854f49ff1 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -2849,7 +2849,8 @@ paths: /api/v0/products/{id}/user_flexible_price/: get: operationId: products_user_flexible_price_retrieve - description: Retrieve a product with user-specific flexible price information + description: Retrieve a product with user-specific flexible price information. + Use `user_pricing` instead. parameters: - in: path name: id @@ -2859,6 +2860,7 @@ paths: required: true tags: - products + deprecated: true responses: '200': content: @@ -2866,6 +2868,43 @@ paths: schema: $ref: '#/components/schemas/ProductFlexiblePrice' description: '' + /api/v0/products/{id}/user_pricing/: + get: + operationId: products_user_pricing_retrieve + description: The price this user pays for this product, computed the way checkout + computes it (financial assistance, user-tied and automatic discounts, including + paid-amount-off credit for a qualifying prior purchase). The response also + carries product_flexible_price exactly as the deprecated user_flexible_price + endpoint returns it, so a caller moves over field for field. Anonymous requests + are a 403; an unknown or no-longer-purchasable product is a 404. + parameters: + - in: path + name: id + schema: + type: integer + description: A unique integer value identifying this product. + required: true + tags: + - products + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/UserPricingProduct' + description: '' + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/DetailError' + description: '' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/DetailError' + description: '' /api/v0/products/all/: get: operationId: products_all_list @@ -7097,6 +7136,31 @@ components: - redeemed_by - redeemed_discount - redeemed_order + DiscountSource: + type: object + description: The prior purchase a discount credits. + properties: + type: + $ref: '#/components/schemas/DiscountSourceTypeEnum' + readable_id: + type: string + title: + type: string + required: + - readable_id + - title + - type + DiscountSourceTypeEnum: + enum: + - course + - program + type: string + description: |- + * `course` - course + * `program` - program + x-enum-descriptions: + - course + - program DiscountTypeEnum: enum: - percent-off @@ -10311,6 +10375,92 @@ components: required: - discount - user + UserPricingDiscount: + type: object + description: The discount checkout would apply to a product for this user. + properties: + id: + type: integer + discount_code: + type: string + discount_type: + $ref: '#/components/schemas/DiscountTypeEnum' + payment_type: + nullable: true + description: |- + What kind of discount won: `financial-assistance` is the learner's approved aid tier; the other values say how a discount was funded. Null on discounts created without one. + + * `marketing` - marketing + * `sales` - sales + * `financial-assistance` - financial-assistance + * `customer-support` - customer-support + * `staff` - staff + * `legacy` - legacy + oneOf: + - $ref: '#/components/schemas/PaymentTypeEnum' + - $ref: '#/components/schemas/NullEnum' + amount_off: + type: string + format: decimal + pattern: ^-?\d{0,5}(?:\.\d{0,2})?$ + description: Dollars taken off `price` for this user. For paid-amount-off + discounts this is the prior purchase's paid price, capped at `price`, + never the stored amount. + source: + allOf: + - $ref: '#/components/schemas/DiscountSource' + nullable: true + description: 'Set only for paid-amount-off discounts (`discount_type` is + the discriminator): the prior purchase being credited.' + required: + - amount_off + - discount_code + - discount_type + - id + - source + UserPricingProduct: + type: object + description: A product priced for one user. + properties: + id: + type: integer + readOnly: true + price: + type: string + format: decimal + pattern: ^-?\d{0,5}(?:\.\d{0,2})?$ + description: + type: string + is_active: + type: boolean + description: Controls visibility of the product in the app. + product_flexible_price: + allOf: + - $ref: '#/components/schemas/V0Discount' + nullable: true + description: 'The learner''s approved financial-assistance tier discount, + or null: whether they are approved and at which tier, even when that tier + is 0% or another discount wins. Read `user_price` for the price, not this + field''s `amount`.' + readOnly: true + user_price: + type: string + format: decimal + pattern: ^-?\d{0,5}(?:\.\d{0,2})?$ + description: What this user pays at checkout today. + discount: + allOf: + - $ref: '#/components/schemas/UserPricingDiscount' + nullable: true + description: The discount checkout applies to this product for this user, + or null at list price. + required: + - description + - discount + - id + - price + - product_flexible_price + - user_price UserProfile: type: object description: Serializer for profile diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index 04374cce66..9b96ac1699 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -2849,7 +2849,8 @@ paths: /api/v0/products/{id}/user_flexible_price/: get: operationId: products_user_flexible_price_retrieve - description: Retrieve a product with user-specific flexible price information + description: Retrieve a product with user-specific flexible price information. + Use `user_pricing` instead. parameters: - in: path name: id @@ -2859,6 +2860,7 @@ paths: required: true tags: - products + deprecated: true responses: '200': content: @@ -2866,6 +2868,43 @@ paths: schema: $ref: '#/components/schemas/ProductFlexiblePrice' description: '' + /api/v0/products/{id}/user_pricing/: + get: + operationId: products_user_pricing_retrieve + description: The price this user pays for this product, computed the way checkout + computes it (financial assistance, user-tied and automatic discounts, including + paid-amount-off credit for a qualifying prior purchase). The response also + carries product_flexible_price exactly as the deprecated user_flexible_price + endpoint returns it, so a caller moves over field for field. Anonymous requests + are a 403; an unknown or no-longer-purchasable product is a 404. + parameters: + - in: path + name: id + schema: + type: integer + description: A unique integer value identifying this product. + required: true + tags: + - products + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/UserPricingProduct' + description: '' + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/DetailError' + description: '' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/DetailError' + description: '' /api/v0/products/all/: get: operationId: products_all_list @@ -7097,6 +7136,31 @@ components: - redeemed_by - redeemed_discount - redeemed_order + DiscountSource: + type: object + description: The prior purchase a discount credits. + properties: + type: + $ref: '#/components/schemas/DiscountSourceTypeEnum' + readable_id: + type: string + title: + type: string + required: + - readable_id + - title + - type + DiscountSourceTypeEnum: + enum: + - course + - program + type: string + description: |- + * `course` - course + * `program` - program + x-enum-descriptions: + - course + - program DiscountTypeEnum: enum: - percent-off @@ -10311,6 +10375,92 @@ components: required: - discount - user + UserPricingDiscount: + type: object + description: The discount checkout would apply to a product for this user. + properties: + id: + type: integer + discount_code: + type: string + discount_type: + $ref: '#/components/schemas/DiscountTypeEnum' + payment_type: + nullable: true + description: |- + What kind of discount won: `financial-assistance` is the learner's approved aid tier; the other values say how a discount was funded. Null on discounts created without one. + + * `marketing` - marketing + * `sales` - sales + * `financial-assistance` - financial-assistance + * `customer-support` - customer-support + * `staff` - staff + * `legacy` - legacy + oneOf: + - $ref: '#/components/schemas/PaymentTypeEnum' + - $ref: '#/components/schemas/NullEnum' + amount_off: + type: string + format: decimal + pattern: ^-?\d{0,5}(?:\.\d{0,2})?$ + description: Dollars taken off `price` for this user. For paid-amount-off + discounts this is the prior purchase's paid price, capped at `price`, + never the stored amount. + source: + allOf: + - $ref: '#/components/schemas/DiscountSource' + nullable: true + description: 'Set only for paid-amount-off discounts (`discount_type` is + the discriminator): the prior purchase being credited.' + required: + - amount_off + - discount_code + - discount_type + - id + - source + UserPricingProduct: + type: object + description: A product priced for one user. + properties: + id: + type: integer + readOnly: true + price: + type: string + format: decimal + pattern: ^-?\d{0,5}(?:\.\d{0,2})?$ + description: + type: string + is_active: + type: boolean + description: Controls visibility of the product in the app. + product_flexible_price: + allOf: + - $ref: '#/components/schemas/V0Discount' + nullable: true + description: 'The learner''s approved financial-assistance tier discount, + or null: whether they are approved and at which tier, even when that tier + is 0% or another discount wins. Read `user_price` for the price, not this + field''s `amount`.' + readOnly: true + user_price: + type: string + format: decimal + pattern: ^-?\d{0,5}(?:\.\d{0,2})?$ + description: What this user pays at checkout today. + discount: + allOf: + - $ref: '#/components/schemas/UserPricingDiscount' + nullable: true + description: The discount checkout applies to this product for this user, + or null at list price. + required: + - description + - discount + - id + - price + - product_flexible_price + - user_price UserProfile: type: object description: Serializer for profile