diff --git a/RELEASE.rst b/RELEASE.rst index 9aa7d08c1f..d9bc080b1c 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,17 @@ Release Notes ============= +Version 1.164.2 +--------------- + +- Email context updates (#3911) +- fix: basket api discount-related bugfixes (#3898) +- fix: OpenAPI / Serializer improvements for BulkDiscountSerializer, CreateBasketWithProductsSerializer (#3902) +- fix: unbreak python-checks — ecommerce migration leaf conflict + stale Keycloak dataclasses (#3908) +- Skip downgrade for unenrolled learners on refund (#3901) +- fix: downgrade stale course run 404 errors from error to warning (#3904) +- Look up orders by reference_number on the CyberSource callback (#3899) + Version 1.164.1 (Released August 31, 2026) --------------- diff --git a/b2b/keycloak_admin_dataclasses.py b/b2b/keycloak_admin_dataclasses.py index d6d2350c63..724a933ddc 100644 --- a/b2b/keycloak_admin_dataclasses.py +++ b/b2b/keycloak_admin_dataclasses.py @@ -1211,7 +1211,9 @@ class OrganizationInvitationRepresentation(BaseModel): sent_date: Annotated[int | None, Field(alias='sentDate')] = None expires_at: Annotated[int | None, Field(alias='expiresAt')] = None status: Status | None = None - invite_link: Annotated[str | None, Field(alias='inviteLink')] = None + invite_link: Annotated[str | None, Field(alias='inviteLink', deprecated=True)] = ( + None + ) class OrganizationRepresentation(BaseModel): diff --git a/b2b/mail.py b/b2b/mail.py index 9acd02a651..b2332ea2a9 100644 --- a/b2b/mail.py +++ b/b2b/mail.py @@ -4,16 +4,16 @@ from django.conf import settings from mitol.common.utils.datetime import now_in_utc from mitol.mail.api import get_message_sender -from mitol.mail.messages import TemplatedMessage from b2b.models import ContractPage, DiscountContractAttachmentRedemption +from mail.messages import SiteTemplatedMessage log = logging.getLogger(__name__) ENROLLMENT_CODE_ASSINGMENT_TAG = "enrollment-code-assignment" -class BaseEnrollmentCodeAssignmentMessage(TemplatedMessage): +class BaseEnrollmentCodeAssignmentMessage(SiteTemplatedMessage): template_name = "mail/enrollment_code_assignment" name = "Enrollment Code Assignment" @@ -21,7 +21,7 @@ class BaseEnrollmentCodeAssignmentMessage(TemplatedMessage): class EnrollmentCodeAssignmentMessage(BaseEnrollmentCodeAssignmentMessage): @staticmethod def get_default_headers() -> dict: - base_headers = TemplatedMessage.get_default_headers() + base_headers = SiteTemplatedMessage.get_default_headers() headers = base_headers.copy() headers["X-Mailgun-Tag"] = ENROLLMENT_CODE_ASSINGMENT_TAG return headers diff --git a/cms/models.py b/cms/models.py index 2a2454db10..d96f4e4895 100644 --- a/cms/models.py +++ b/cms/models.py @@ -1506,7 +1506,7 @@ def _get_current_finaid(self, request): ecommerce_product, request.user ) - if discount and discount.check_validity(request.user): + if discount and discount.is_redeemable_by(request.user): log.debug( f"price is {ecommerce_product.price}, discount is {discount.discount_product(ecommerce_product)}" # noqa: G004 ) diff --git a/courses/management/commands/create_verified_enrollment.py b/courses/management/commands/create_verified_enrollment.py index d65f12b023..a3a33561b1 100644 --- a/courses/management/commands/create_verified_enrollment.py +++ b/courses/management/commands/create_verified_enrollment.py @@ -119,7 +119,7 @@ def handle(self, *args, **options): # noqa: ARG002, C901 ) ) - if not discount.check_validity(user): + if not discount.is_redeemable_by(user): raise CommandError( "That enrollment code {} for course with courseware_id={} is invalid for user {}".format( # noqa: EM103 options["code"], options["run"], options["user"] diff --git a/courses/messages.py b/courses/messages.py index 529b0b054f..a31d5e0385 100644 --- a/courses/messages.py +++ b/courses/messages.py @@ -1,9 +1,9 @@ """Course email messages""" from django.conf import settings -from mitol.mail.messages import TemplatedMessage from courses.utils import is_uai_course_run +from mail.messages import SiteTemplatedMessage class UAIEmailMixin: @@ -25,28 +25,28 @@ def create(cls, **kwargs): return super().create(**kwargs) -class CourseRunEnrollmentMessage(UAIEmailMixin, TemplatedMessage): +class CourseRunEnrollmentMessage(UAIEmailMixin, SiteTemplatedMessage): """Email message for course enrollment""" name = "Course Run Enrollment" template_name = "mail/course_run_enrollment" -class CourseRunUnenrollmentMessage(UAIEmailMixin, TemplatedMessage): +class CourseRunUnenrollmentMessage(UAIEmailMixin, SiteTemplatedMessage): """Email message for course unenrollment""" name = "Course Run Unenrollment" template_name = "mail/course_run_unenrollment" -class EnrollmentFailureMessage(TemplatedMessage): +class EnrollmentFailureMessage(SiteTemplatedMessage): """Email message for enrollment failures""" name = "Enrollment Failure" template_name = "mail/enrollment_failure" -class PartnerSchoolSharingMessage(TemplatedMessage): +class PartnerSchoolSharingMessage(SiteTemplatedMessage): """Email message for sharing learner records to partner schools""" name = "Shared Learner Record" diff --git a/courses/utils.py b/courses/utils.py index 4eb827baf1..d99a649d52 100644 --- a/courses/utils.py +++ b/courses/utils.py @@ -2,6 +2,7 @@ import logging import re +from http import HTTPStatus from urllib.parse import urljoin from django.conf import settings @@ -51,7 +52,19 @@ def exception_logging_generator(generator): except StopIteration: # noqa: PERF203 return except HTTPError as exc: - log.exception("EdX API error for fetching user grades %s:", exc) # noqa: TRY401 + if ( + exc.response is not None + and exc.response.status_code == HTTPStatus.NOT_FOUND + ): + # Course run no longer exists in edX (e.g. an old run that was + # removed). This is expected for stale runs and shouldn't page + # Sentry every hour - see mitodl/hq#12729. + log.warning( + "EdX API 404 fetching user grades, course run may no longer exist in edX: %s", + exc, + ) + else: + log.exception("EdX API error for fetching user grades %s:", exc) # noqa: TRY401 except Exception as exp: # pylint: disable=broad-except log.exception("Error fetching user grades from edX %s:", exp) # noqa: TRY401 diff --git a/courses/utils_test.py b/courses/utils_test.py index bf89cafe7d..44b4e9b1c9 100644 --- a/courses/utils_test.py +++ b/courses/utils_test.py @@ -8,6 +8,7 @@ import pytest from mitol.common.utils import now_in_utc +from requests.exceptions import HTTPError from courses.factories import ( CourseFactory, @@ -20,6 +21,7 @@ ) from courses.models import Course, CourseRun from courses.utils import ( + exception_logging_generator, get_dated_courseruns, get_enrollable_courseruns_qs, get_enrollable_courses, @@ -365,3 +367,33 @@ def test_is_uai_order_uses_purchased_object_when_available(): order = SimpleNamespace(lines=SimpleNamespace(all=lambda: [line])) assert is_uai_order(order) is True + + +def _make_http_error(status_code): + error = HTTPError(f"{status_code} error") + error.response = SimpleNamespace(status_code=status_code) + return error + + +@pytest.mark.parametrize( + ("status_code", "expected_level"), + [ + (404, "WARNING"), # stale/deleted course run - shouldn't page Sentry + (500, "ERROR"), # real edX API failure - should still page Sentry + ], +) +def test_exception_logging_generator_http_error_log_level( + caplog, status_code, expected_level +): + """HTTPErrors should log at WARNING for 404s and ERROR otherwise, without stopping iteration.""" + + def gen(): + yield 1 + raise _make_http_error(status_code) + + with caplog.at_level("WARNING"): + results = list(exception_logging_generator(gen())) + + assert results == [1] + assert len(caplog.records) == 1 + assert caplog.records[0].levelname == expected_level diff --git a/drf_lint_baseline.json b/drf_lint_baseline.json index 374dbaeb56..f514e3168b 100644 --- a/drf_lint_baseline.json +++ b/drf_lint_baseline.json @@ -22,38 +22,38 @@ "courses/serializers/v2/programs.py:387:50:ORM002", "courses/serializers/v2/programs.py:500:12:ORM002", "courses/serializers/v3/courses.py:57:12:ORM002", - "ecommerce/serializers/__init__.py:205:17:ORM001", - "ecommerce/serializers/__init__.py:207:18:ORM001", + "ecommerce/serializers/__init__.py:206:17:ORM001", "ecommerce/serializers/__init__.py:208:18:ORM001", - "ecommerce/serializers/__init__.py:230:26:ORM002", - "ecommerce/serializers/__init__.py:318:26:ORM002", - "ecommerce/serializers/__init__.py:326:31:ORM002", - "ecommerce/serializers/__init__.py:332:20:ORM002", - "ecommerce/serializers/__init__.py:337:31:ORM002", - "ecommerce/serializers/__init__.py:352:31:ORM002", - "ecommerce/serializers/__init__.py:427:24:ORM002", - "ecommerce/serializers/__init__.py:446:12:ORM001", - "ecommerce/serializers/__init__.py:470:22:ORM002", - "ecommerce/serializers/__init__.py:517:22:ORM002", - "ecommerce/serializers/__init__.py:587:20:ORM002", - "ecommerce/serializers/__init__.py:719:22:ORM002", - "ecommerce/serializers/__init__.py:901:28:ORM002", - "ecommerce/serializers/v0/__init__.py:292:17:ORM001", - "ecommerce/serializers/v0/__init__.py:294:18:ORM001", - "ecommerce/serializers/v0/__init__.py:295:18:ORM001", - "ecommerce/serializers/v0/__init__.py:317:26:ORM002", - "ecommerce/serializers/v0/__init__.py:405:26:ORM002", - "ecommerce/serializers/v0/__init__.py:414:35:ORM002", - "ecommerce/serializers/v0/__init__.py:421:20:ORM002", - "ecommerce/serializers/v0/__init__.py:427:35:ORM002", - "ecommerce/serializers/v0/__init__.py:443:31:ORM002", - "ecommerce/serializers/v0/__init__.py:540:24:ORM002", - "ecommerce/serializers/v0/__init__.py:559:12:ORM001", - "ecommerce/serializers/v0/__init__.py:583:22:ORM002", - "ecommerce/serializers/v0/__init__.py:630:22:ORM002", - "ecommerce/serializers/v0/__init__.py:705:20:ORM002", - "ecommerce/serializers/v0/__init__.py:854:22:ORM002", - "ecommerce/serializers/v0/__init__.py:998:28:ORM002", + "ecommerce/serializers/__init__.py:209:18:ORM001", + "ecommerce/serializers/__init__.py:231:26:ORM002", + "ecommerce/serializers/__init__.py:319:26:ORM002", + "ecommerce/serializers/__init__.py:327:31:ORM002", + "ecommerce/serializers/__init__.py:333:20:ORM002", + "ecommerce/serializers/__init__.py:338:31:ORM002", + "ecommerce/serializers/__init__.py:353:31:ORM002", + "ecommerce/serializers/__init__.py:428:24:ORM002", + "ecommerce/serializers/__init__.py:447:12:ORM001", + "ecommerce/serializers/__init__.py:471:22:ORM002", + "ecommerce/serializers/__init__.py:518:22:ORM002", + "ecommerce/serializers/__init__.py:588:20:ORM002", + "ecommerce/serializers/__init__.py:760:22:ORM002", + "ecommerce/serializers/__init__.py:942:28:ORM002", + "ecommerce/serializers/v0/__init__.py:288:17:ORM001", + "ecommerce/serializers/v0/__init__.py:290:18:ORM001", + "ecommerce/serializers/v0/__init__.py:291:18:ORM001", + "ecommerce/serializers/v0/__init__.py:313:26:ORM002", + "ecommerce/serializers/v0/__init__.py:401:26:ORM002", + "ecommerce/serializers/v0/__init__.py:410:35:ORM002", + "ecommerce/serializers/v0/__init__.py:417:20:ORM002", + "ecommerce/serializers/v0/__init__.py:423:35:ORM002", + "ecommerce/serializers/v0/__init__.py:439:31:ORM002", + "ecommerce/serializers/v0/__init__.py:536:24:ORM002", + "ecommerce/serializers/v0/__init__.py:555:12:ORM001", + "ecommerce/serializers/v0/__init__.py:579:22:ORM002", + "ecommerce/serializers/v0/__init__.py:626:22:ORM002", + "ecommerce/serializers/v0/__init__.py:701:20:ORM002", + "ecommerce/serializers/v0/__init__.py:832:22:ORM002", + "ecommerce/serializers/v0/__init__.py:976:28:ORM002", "flexiblepricing/serializers.py:147:34:ORM001", "flexiblepricing/serializers.py:170:34:ORM001", "flexiblepricing/serializers.py:173:20:ORM001", diff --git a/ecommerce/api.py b/ecommerce/api.py index 7048f71549..6607525d13 100644 --- a/ecommerce/api.py +++ b/ecommerce/api.py @@ -2,7 +2,7 @@ import logging import uuid -from datetime import timedelta +from datetime import datetime, timedelta from decimal import Decimal from urllib.parse import urljoin @@ -30,6 +30,7 @@ ) from courses.api import create_run_enrollments, deactivate_run_enrollment from courses.constants import ENROLL_CHANGE_STATUS_REFUNDED +from courses.models import CourseRunEnrollment from courses.utils import is_uai_course_run from ecommerce.constants import ( ADMIN_FULFILLED_PAYMENT_DATA, @@ -307,7 +308,7 @@ def check_basket_discounts_for_validity(request): basket = establish_basket(request) for basket_discount in basket.discounts.all(): - if not basket_discount.redeemed_discount.check_validity( + if not basket_discount.redeemed_discount.is_redeemable_by( basket.user ) or not check_discount_for_products(basket_discount.redeemed_discount, basket): return False @@ -360,7 +361,7 @@ def apply_user_discounts(request): # check for product specificity in the discount if not check_discount_for_products( discount, basket - ) or not discount.check_validity(user): + ) or not discount.is_redeemable_by(user): return bd = BasketDiscount( @@ -396,11 +397,16 @@ def get_order_from_cybersource_payment_response(request): converted_order = PaymentGateway.get_gateway_class( settings.ECOMMERCE_DEFAULT_PAYMENT_GATEWAY ).convert_to_order(payment_data) - order_id = Order.decode_reference_number(converted_order.reference) - try: - order = Order.objects.select_for_update().get(pk=order_id) + order = Order.objects.select_for_update().get( + reference_number=converted_order.reference + ) except ObjectDoesNotExist: + log.warning( + "get_order_from_cybersource_payment_response: no order found for " + "reference number %s", + converted_order.reference, + ) order = None return order @@ -712,11 +718,24 @@ def downgrade_learner_from_order(order_id): order = Order.objects.get(pk=order_id) + # Only downgrade runs the learner still has an active enrollment in. If they + # unenrolled themselves entirely (e.g. via the dashboard) before the refund + # was processed, we leave their enrollment status alone rather than forcing + # them back into the course as an audit learner. (See ticket #3696.) + active_runs = [ + run + for run in order.purchased_runs + if CourseRunEnrollment.objects.filter(user=order.purchaser, run=run).exists() + ] + + if not active_runs: + return + # Forcing the enrollment here - if the refund comes after the end date # for the course for whatever reason, we still want to revert the mode. create_run_enrollments( user=order.purchaser, - runs=order.purchased_runs, + runs=active_runs, keep_failed_enrollments=True, mode=EDX_ENROLLMENT_AUDIT_MODE, ) @@ -1013,6 +1032,17 @@ def check_for_duplicate_discount_redemptions(): return seen +def _coerce_supplied_date(value): + """ + Normalize a date reaching generate_discount_code either as a management + command's raw string or as a datetime BulkDiscountSerializer already parsed. + """ + if value is None or isinstance(value, datetime): + return value + + return parse_supplied_date(value) + + def generate_discount_code(**kwargs): # noqa: C901 """ Generates a discount code (or a batch of discount codes) as specified by the @@ -1022,7 +1052,7 @@ def generate_discount_code(**kwargs): # noqa: C901 UUID - if you want one (the convention is a -), you need to ensure it's there in the prefix (and that counts against the limit) - If you specify redemption_type, specifying one_time or one_time_per_user will not be + If you specify redemption_type, specifying one_time or once_per_user will not be honored. Keyword Args: @@ -1031,18 +1061,18 @@ def generate_discount_code(**kwargs): # noqa: C901 * redemption_type - one of the valid redemption types (overrules use of the flags) * amount - the value of the discount * one_time - boolean; discount can only be redeemed once - * one_time_per_user - boolean; discount can only be redeemed once per user + * once_per_user - boolean; discount can only be redeemed once per user * activates - date to activate * expires - date to expire the code - * count - number of codes to create (requires prefix) - * prefix - prefix to append to the codes (max 63 characters) + * prefix - prefix to append to generated codes (max 63 characters) + * count - how many codes to generate from prefix; defaults to 1 + * codes - the exact codes to create, instead of generating from a prefix Returns: * List of generated codes, with the following fields: code, type, amount, expiration_date """ - codes_to_generate = [] discount_type = kwargs["discount_type"] redemption_type = REDEMPTION_TYPE_UNLIMITED payment_type = kwargs["payment_type"] @@ -1059,25 +1089,36 @@ def generate_discount_code(**kwargs): # noqa: C901 f"Discount amount {amount} not valid for discount type {DISCOUNT_TYPE_PERCENT_OFF}." # noqa: EM102 ) - if kwargs["count"] > 1 and "prefix" not in kwargs: - raise Exception("You must specify a prefix to create a batch of codes.") # noqa: EM101, TRY002 + count = kwargs.get("count") + prefix = kwargs.get("prefix") + codes = kwargs.get("codes") - if kwargs["count"] > 1: - prefix = kwargs["prefix"] + # A caller either names the codes to create or asks for some to be generated + # from a prefix. Honoring both at once has no unambiguous reading, so they are + # mutually exclusive, and count only means anything in the generated case. + if bool(codes) == bool(prefix): + msg = "Supply either codes, or a prefix to generate them from." + raise Exception(msg) # noqa: TRY002 + + if codes: + if count is not None: + msg = "count applies only when generating codes from a prefix." + raise Exception(msg) # noqa: TRY002 + + codes_to_generate = codes + else: + count = 1 if count is None else count + + if count < 1: + msg = f"count must be at least 1, got {count}." + raise Exception(msg) # noqa: TRY002 # upped the discount code limit to 100 characters - this used to be 13 (50 - 37 for the UUID) if len(prefix) > 63: # noqa: PLR2004 - raise Exception( # noqa: TRY002 - f"Prefix {prefix} is {len(prefix)} - prefixes must be 63 characters or less." # noqa: EM102 - ) + msg = f"Prefix {prefix} is {len(prefix)} - prefixes must be 63 characters or less." + raise Exception(msg) # noqa: TRY002 - for i in range(kwargs["count"]): # noqa: B007 - generated_uuid = uuid.uuid4() - code = f"{prefix}{generated_uuid}" - - codes_to_generate.append(code) - else: - codes_to_generate = kwargs["codes"] + codes_to_generate = [f"{prefix}{uuid.uuid4()}" for _ in range(count)] if kwargs.get("one_time"): redemption_type = REDEMPTION_TYPE_ONE_TIME @@ -1091,15 +1132,8 @@ def generate_discount_code(**kwargs): # noqa: C901 ): redemption_type = kwargs["redemption_type"] - if "expires" in kwargs and kwargs["expires"] is not None: - expiration_date = parse_supplied_date(kwargs["expires"]) - else: - expiration_date = None - - if "activates" in kwargs and kwargs["activates"] is not None: - activation_date = parse_supplied_date(kwargs["activates"]) - else: - activation_date = None + expiration_date = _coerce_supplied_date(kwargs.get("expires")) + activation_date = _coerce_supplied_date(kwargs.get("activates")) generated_codes = [] @@ -1183,7 +1217,7 @@ def apply_discount_to_basket(basket: Basket, discount: Discount, *, allow_finaid Keyword Args: allow_finaid (bool): Allow a financial assistance discount through. """ - if discount.is_valid(basket, allow_finaid=allow_finaid): + if discount.is_valid_for_basket(basket, allow_finaid=allow_finaid): defaults = { "redeemed_discount": discount, "redemption_date": now_in_utc(), @@ -1243,7 +1277,7 @@ def apply_discount_to_basket(basket: Basket, discount: Discount, *, allow_finaid for item in basket.basket_items.all(): test_price = discount.discount_product(item.product, basket.user) - if test_price and item.discounted_price >= test_price: + if test_price is not None and item.discounted_price >= test_price: found_better = True break diff --git a/ecommerce/api_test.py b/ecommerce/api_test.py index 6390d02084..48fbb3df45 100644 --- a/ecommerce/api_test.py +++ b/ecommerce/api_test.py @@ -38,11 +38,13 @@ create_verified_program_course_run_enrollment, create_verified_program_discount, cull_anonymous_baskets, + downgrade_learner_from_order, establish_basket, establish_basket_for_request, generate_checkout_payload, get_anonymous_basket_id, get_auto_apply_discounts_for_basket, + get_order_from_cybersource_payment_response, log_stripe_event, process_cybersource_payment_response, process_stripe_checkout_completed, @@ -106,7 +108,7 @@ ) from flexiblepricing.constants import FlexiblePriceStatus from flexiblepricing.factories import FlexiblePriceFactory, FlexiblePriceTierFactory -from openedx.constants import EDX_ENROLLMENT_VERIFIED_MODE +from openedx.constants import EDX_ENROLLMENT_AUDIT_MODE, EDX_ENROLLMENT_VERIFIED_MODE from openedx.factories import OpenEdxUserFactory from users.factories import UserFactory @@ -538,6 +540,56 @@ def test_unenrollment_unenrolls_learner(mocker, user): unenroll_mock.assert_called() +def test_downgrade_learner_from_order_downgrades_active_enrollment(mocker, user): + """ + downgrade_learner_from_order should force an audit enrollment for runs the + learner is still actively enrolled in. + """ + order = OrderFactory.create(purchaser=user, state=OrderStatus.FULFILLED) + with reversion.create_revision(): + product = ProductFactory.create() + version = Version.objects.get_for_object(product).first() + enrollment = CourseRunEnrollmentFactory.create(user=user, active=True) + LineFactory.create( + order=order, purchased_object=enrollment.run, product_version=version + ) + + create_run_enrollments_mock = mocker.patch( + "ecommerce.api.create_run_enrollments", + ) + downgrade_learner_from_order(order_id=order.id) + + create_run_enrollments_mock.assert_called_once() + _, kwargs = create_run_enrollments_mock.call_args + assert kwargs["runs"] == [enrollment.run] + assert kwargs["mode"] == EDX_ENROLLMENT_AUDIT_MODE + + +def test_downgrade_learner_from_order_skips_unenrolled_learner(mocker, user): + """ + If the learner has unenrolled entirely (inactive enrollment) before the + refund is processed, downgrade_learner_from_order should leave their + enrollment alone rather than re-enrolling them as audit. + + Regression test for ticket #3696. + """ + order = OrderFactory.create(purchaser=user, state=OrderStatus.FULFILLED) + with reversion.create_revision(): + product = ProductFactory.create() + version = Version.objects.get_for_object(product).first() + enrollment = CourseRunEnrollmentFactory.create(user=user, active=False) + LineFactory.create( + order=order, purchased_object=enrollment.run, product_version=version + ) + + create_run_enrollments_mock = mocker.patch( + "ecommerce.api.create_run_enrollments", + ) + downgrade_learner_from_order(order_id=order.id) + + create_run_enrollments_mock.assert_not_called() + + @pytest.mark.skip_nplusone_check def test_process_cybersource_payment_response(settings, rf, mocker, user, products): """Test that ensures the response from Cybersource for an ACCEPTed payment updates the orders state""" @@ -573,6 +625,53 @@ def test_process_cybersource_payment_response(settings, rf, mocker, user, produc assert result == OrderStatus.FULFILLED +@pytest.mark.skip_nplusone_check +@pytest.mark.parametrize("stored_reference_number", [None, "someotherprefix-dev-9999"]) +def test_get_order_from_cybersource_payment_response( + rf, user, products, stored_reference_number +): + """ + The CyberSource callback should resolve the order by its stored reference + number, whatever scheme that reference number was generated under - the + prefix can change, but the stored value never does. + """ + create_basket(user, products) + create_pending_order(user) + + order = Order.objects.get(state=OrderStatus.PENDING, purchaser=user) + + if stored_reference_number is not None: + # simulate an order created under a different reference number scheme + Order.objects.filter(pk=order.pk).update( + reference_number=stored_reference_number + ) + order.refresh_from_db() + + payload = { + "req_reference_number": order.reference_number, + "req_consumer_id": user.edx_username, + "req_customer_ip_address": "127.0.0.1", + "req_line_item_count": 0, + } + request = rf.post(reverse("checkout_result_api"), payload) + + assert get_order_from_cybersource_payment_response(request) == order + + +@pytest.mark.skip_nplusone_check +def test_get_order_from_cybersource_payment_response_unknown_reference(rf): + """An unrecognized reference number should return None, not raise.""" + payload = { + "req_reference_number": "mitxonline-dev-does-not-exist", + "req_consumer_id": "someone", + "req_customer_ip_address": "127.0.0.1", + "req_line_item_count": 0, + } + request = rf.post(reverse("checkout_result_api"), payload) + + assert get_order_from_cybersource_payment_response(request) is None + + @pytest.mark.skip_nplusone_check @pytest.mark.parametrize("include_discount", [True, False]) def test_process_cybersource_payment_decline_response( @@ -990,6 +1089,31 @@ def test_apply_discount_to_basket(user, better_discount, is_valid, _count): assert basket.discounts.filter(redeemed_discount=existing_discount).exists() +def test_apply_discount_to_basket_prefers_a_full_credit_discount(user): + """A discount that prices the item at $0.00 must win the best-price comparison.""" + 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) + + existing_discount = UnlimitedUseDiscountFactory.create( + amount=50, discount_type="percent-off" + ) + BasketDiscount.objects.create( + redeemed_by=user, + redemption_date=now_in_utc(), + redeemed_discount=existing_discount, + redeemed_basket=basket, + ) + full_credit = UnlimitedUseDiscountFactory.create( + amount=100, discount_type="percent-off" + ) + + apply_discount_to_basket(basket, full_credit) + + assert basket.discounts.get().redeemed_discount == full_credit + + @pytest.mark.parametrize( "is_better", [ diff --git a/ecommerce/management/commands/generate_discount_code.py b/ecommerce/management/commands/generate_discount_code.py index f80b99874e..2d07db6b5b 100644 --- a/ecommerce/management/commands/generate_discount_code.py +++ b/ecommerce/management/commands/generate_discount_code.py @@ -91,8 +91,7 @@ def add_arguments(self, parser) -> None: "--count", type=int, nargs="?", - help="Number of codes to produce", - default=1, + help="Number of codes to generate from --prefix (default 1)", ) parser.add_argument( @@ -111,14 +110,15 @@ def add_arguments(self, parser) -> None: "codes", nargs="*", type=str, - help="Discount codes to generate (ignored if --count is specified)", + help="The exact codes to create; mutually exclusive with --prefix", ) def handle(self, *args, **kwargs): # pylint: disable=unused-argument # noqa: ARG002 try: generated_codes = generate_discount_code(**kwargs) except Exception as e: # noqa: BLE001 - self.stderr.write(self.style.ERROR(e)) + self.stderr.write(self.style.ERROR(str(e))) + return with open("generated-codes.csv", mode="w") as output_file: # noqa: PTH123 writer = csv.DictWriter( diff --git a/ecommerce/messages.py b/ecommerce/messages.py index ed660327b7..f116a1d0df 100644 --- a/ecommerce/messages.py +++ b/ecommerce/messages.py @@ -1,18 +1,18 @@ """Ecommerce email messages""" -from mitol.mail.messages import TemplatedMessage +from mail.messages import SiteTemplatedMessage -class OrderReceiptMessage(TemplatedMessage): +class OrderReceiptMessage(SiteTemplatedMessage): template_name = "mail/product_order_receipt" name = "Order Receipt" -class OrderRefundMessage(TemplatedMessage): +class OrderRefundMessage(SiteTemplatedMessage): template_name = "mail/order_refund_message" name = "Refund of MITx Online Order" -class RefundRequestNotificationMessage(TemplatedMessage): +class RefundRequestNotificationMessage(SiteTemplatedMessage): template_name = "mail/refund_request_notification" name = "Refund Request Submitted" diff --git a/ecommerce/migrations/0052_order_reference_number_unique_concurrently.py b/ecommerce/migrations/0052_order_reference_number_unique_concurrently.py new file mode 100644 index 0000000000..78000427c8 --- /dev/null +++ b/ecommerce/migrations/0052_order_reference_number_unique_concurrently.py @@ -0,0 +1,79 @@ +""" +Add the unique constraint on Order.reference_number, building it concurrently. + +AddConstraint for a plain UniqueConstraint emits + + ALTER TABLE ecommerce_order ADD CONSTRAINT ... UNIQUE (reference_number); + +which holds ACCESS EXCLUSIVE on ecommerce_order for the whole index build - +measured at ~3s over 537k rows. An ACCESS EXCLUSIVE request also queues behind +any in-flight transaction and blocks everything arriving after it, so one slow +query at deploy time turns that into an outage. + +Build the index with CONCURRENTLY instead and then attach it to the constraint. +ADD CONSTRAINT ... USING INDEX is a catalog-only operation: it adopts the index +that already exists rather than rebuilding it, which drops the ACCESS EXCLUSIVE +window from ~3s to ~3ms at the same row count. + +The end state is byte-for-byte what AddConstraint would have produced - a +UNIQUE constraint backed by a unique btree index of the same name - so a future +migration that alters or drops it lines up with what is actually here. + +Recovery: CREATE INDEX CONCURRENTLY leaves an INVALID index behind if it fails +(most likely on a duplicate reference_number). Re-running this migration will +not fix that - IF NOT EXISTS sees the invalid index and skips the build, and the +ADD CONSTRAINT then fails because the index is not valid. Drop it first: + + DROP INDEX CONCURRENTLY unique_order_reference_number; + +then resolve the duplicates and re-run. +""" + +from django.db import migrations, models + +CONSTRAINT_NAME = "unique_order_reference_number" + + +class Migration(migrations.Migration): + # CREATE INDEX CONCURRENTLY cannot run inside a transaction block. + atomic = False + + dependencies = [ + ("ecommerce", "0051_refund_reason_choices_from_design"), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + database_operations=[ + # Each statement is its own RunSQL so that neither gets bundled + # into an implicit transaction block. + migrations.RunSQL( + sql=( + f"CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS {CONSTRAINT_NAME} " + f"ON ecommerce_order (reference_number)" + ), + reverse_sql=f"DROP INDEX CONCURRENTLY IF EXISTS {CONSTRAINT_NAME}", + ), + migrations.RunSQL( + sql=( + f"ALTER TABLE ecommerce_order ADD CONSTRAINT {CONSTRAINT_NAME} " + f"UNIQUE USING INDEX {CONSTRAINT_NAME}" + ), + # Dropping the constraint drops the index it adopted, so the + # reverse of the statement above becomes a no-op. + reverse_sql=( + f"ALTER TABLE ecommerce_order " + f"DROP CONSTRAINT IF EXISTS {CONSTRAINT_NAME}" + ), + ), + ], + state_operations=[ + migrations.AddConstraint( + model_name="order", + constraint=models.UniqueConstraint( + fields=["reference_number"], name=CONSTRAINT_NAME + ), + ), + ], + ), + ] diff --git a/ecommerce/models.py b/ecommerce/models.py index b5f4cc23c5..ba156cd803 100644 --- a/ecommerce/models.py +++ b/ecommerce/models.py @@ -336,7 +336,7 @@ def is_redeemed(self) -> bool: """Returns True if the discount has been redeemed""" return DiscountRedemption.objects.filter(redeemed_discount=self).exists() - def check_validity(self, user: User): + def is_redeemable_by(self, user: User): """ Enforces the redemption rules for a given discount. @@ -405,10 +405,14 @@ def valid_now(self): return True - def is_valid(self, basket, *, allow_finaid=False) -> bool: + def is_valid_for_basket(self, basket, *, allow_finaid=False) -> bool: """ Check if the discount is valid for the basket. + Performs the finaid gate and user-tied-discount checks, then delegates + product scope to check_validity_with_products and the redemption-limit + and date-window rules to is_redeemable_by. + Financial assistance discounts are excluded by default, because this check is used for discount codes that are submitted by the user, and those discounts can't be applied manually. When this is used to check @@ -424,19 +428,6 @@ def is_valid(self, basket, *, allow_finaid=False) -> bool: """ - def _discount_product_in_basket() -> bool: - """ - Check if the discount is associated to the product in the basket. - - Returns: - bool: True if the discount is associated to the product in the basket, - or not associated with any product. - """ - return ( - self.products.count() == 0 - or self.products.filter(product__in=basket.get_products()).count() > 0 - ) - def _discount_user_has_discount() -> bool: """ Check if the discount is associated with the basket's user. @@ -450,47 +441,11 @@ def _discount_user_has_discount() -> bool: or self.user_discount_discount.filter(user=basket.user).count() > 0 ) - def _discount_redemption_limit_valid() -> bool: - """ - Check if the discount has been redeemed less than the maximum number - of times. - - Returns: - bool: True if the discount has been redeemed less than the maximum - number of times, or the maximum number of redemptions is 0. - """ - return ( - self.max_redemptions == 0 - or self.order_redemptions.count() < self.max_redemptions - ) - - def _discount_activation_date_valid() -> bool: - """ - Check if the discount's activation date is in the past. - - Returns: - bool: True if the discount's activation date is in the past, or the - activation date is None. - """ - return self.activation_date is None or now_in_utc() >= self.activation_date - - def _discount_expiration_date_valid() -> bool: - """ - Check if the discount's expiration date is in the future. - - Returns: - bool: True if the discount's expiration date is in the future, or the - expiration date is None. - """ - return self.expiration_date is None or now_in_utc() <= self.expiration_date - return ( (allow_finaid or self.payment_type != PAYMENT_TYPE_FINANCIAL_ASSISTANCE) - and _discount_product_in_basket() + and self.check_validity_with_products(basket.get_products()) and _discount_user_has_discount() - and _discount_redemption_limit_valid() - and _discount_activation_date_valid() - and _discount_expiration_date_valid() + and self.is_redeemable_by(basket.user) ) def friendly_format(self): @@ -517,7 +472,7 @@ def discount_product(self, product, user=None): """ from ecommerce.discounts import DiscountType # noqa: PLC0415 - if (user is None and self.valid_now()) or self.check_validity(user): + if (user is None and self.valid_now()) or self.is_redeemable_by(user): return DiscountType.get_discounted_price([self], product).quantize( Decimal("0.01") ) @@ -815,6 +770,13 @@ class Order(TimestampedModel): decimal_places=5, max_digits=20, ) + # Immutable identifier for this order, shared with the payment gateway. + # Generated once on first save (see save/_generate_reference_number) and + # never regenerated, so it stays valid even if the generation scheme + # changes. Uniqueness is enforced by the constraint in Meta rather than + # unique=True on the field: a field-level unique also pulls in a + # varchar_pattern_ops index for LIKE queries that nothing here issues. + # Postgres allows duplicate NULLs, so the initial insert in save() still works. reference_number = models.CharField(max_length=255, null=True, blank=True) # noqa: DJ001 gateway_type = models.CharField( max_length=32, @@ -823,6 +785,14 @@ class Order(TimestampedModel): help_text="The payment gateway used for this order. Must match one of the supported payment gateways in ol-django.", ) + class Meta: + constraints = [ + models.UniqueConstraint( + fields=["reference_number"], + name="unique_order_reference_number", + ) + ] + def get_object_flow(self): """Instantiate the flow without default constructor""" return OrderFlow(self, user=self.purchaser) @@ -967,10 +937,6 @@ def purchased_runs(self): def __str__(self): return f"{self.state.capitalize()} Order for {self.purchaser.name} ({self.purchaser.email})" - @staticmethod - def decode_reference_number(refno): - return refno.replace(f"{REFERENCE_NUMBER_PREFIX}{settings.ENVIRONMENT}-", "") - def send_ecommerce_order_receipt(self): send_ecommerce_order_receipt.delay(self.id) diff --git a/ecommerce/models_test.py b/ecommerce/models_test.py index f3bf7ca026..ea8800a91b 100644 --- a/ecommerce/models_test.py +++ b/ecommerce/models_test.py @@ -109,11 +109,11 @@ def test_one_time_discount(user, onetime_discount): again. """ - assert onetime_discount.check_validity(user) is True + assert onetime_discount.is_redeemable_by(user) is True perform_discount_redemption(user, onetime_discount) - assert onetime_discount.check_validity(user) is False + assert onetime_discount.is_redeemable_by(user) is False def test_one_time_per_user_discount(users, onetime_per_user_discount): @@ -123,11 +123,11 @@ def test_one_time_per_user_discount(users, onetime_per_user_discount): """ for user in users: - assert onetime_per_user_discount.check_validity(user) is True + assert onetime_per_user_discount.is_redeemable_by(user) is True perform_discount_redemption(user, onetime_per_user_discount) - assert onetime_per_user_discount.check_validity(user) is False + assert onetime_per_user_discount.is_redeemable_by(user) is False def test_unlimited_discounts(users, unlimited_discount): @@ -139,11 +139,11 @@ def test_unlimited_discounts(users, unlimited_discount): for user in users: for i in range(random.randrange(1, 15, 1)): # noqa: S311, B007 - assert unlimited_discount.check_validity(user) is True + assert unlimited_discount.is_redeemable_by(user) is True perform_discount_redemption(user, unlimited_discount) - assert unlimited_discount.check_validity(user) is True + assert unlimited_discount.is_redeemable_by(user) is True def test_set_limit_discount_single_user(user, set_limited_use_discount): @@ -154,11 +154,11 @@ def test_set_limit_discount_single_user(user, set_limited_use_discount): """ for i in range(set_limited_use_discount.max_redemptions): # noqa: B007 - assert set_limited_use_discount.check_validity(user) is True + assert set_limited_use_discount.is_redeemable_by(user) is True perform_discount_redemption(user, set_limited_use_discount) - assert set_limited_use_discount.check_validity(user) is False + assert set_limited_use_discount.is_redeemable_by(user) is False def test_set_limit_discount_multiple_users(users, set_limited_use_discount): @@ -170,14 +170,14 @@ def test_set_limit_discount_multiple_users(users, set_limited_use_discount): for user in users: for i in range(int(set_limited_use_discount.max_redemptions / 2)): # noqa: B007 - assert set_limited_use_discount.check_validity(user) is True + assert set_limited_use_discount.is_redeemable_by(user) is True perform_discount_redemption(user, set_limited_use_discount) if set_limited_use_discount.max_redemptions % 2: perform_discount_redemption(user, set_limited_use_discount) - assert set_limited_use_discount.check_validity(user) is False + assert set_limited_use_discount.is_redeemable_by(user) is False def test_basket_discount_conversion(user, unlimited_discount): @@ -378,6 +378,25 @@ def test_order_update_reference_number(user): ) +def test_order_reference_number_is_immutable(settings, user): + """ + Once set, an order's reference_number should survive later saves even if the + generation scheme changes - it's the identifier we share with the payment + gateway and look the order up by on the callback. + """ + order = Order(purchaser=user, total_price_paid=10) + order.save() + + original_reference_number = order.reference_number + + settings.ENVIRONMENT = "some-other-environment" + order.total_price_paid = 20 + order.save() + order.refresh_from_db() + + assert order.reference_number == original_reference_number + + def test_duplicated_product_lines_validation(basket): """Test that creating multiple lines for the same product in the same order are deduped automatically""" @@ -744,7 +763,7 @@ def test_discount_with_product_value_is_valid_for_basket(is_none): discount = UnlimitedUseDiscountFactory.create(amount=10) if product: DiscountProduct.objects.create(discount=discount, product=product) - assert discount.is_valid(basket_item.basket) + assert discount.is_valid_for_basket(basket_item.basket) @pytest.mark.parametrize("is_none", [True, False]) @@ -759,7 +778,32 @@ def test_discount_with_user_value_is_valid_for_basket(is_none): user=basket_item.basket.user, ) basket_item.basket.user.user_discount_user.add(user_discount) - assert discount.is_valid(basket_item.basket) + assert discount.is_valid_for_basket(basket_item.basket) + + +def test_discount_with_null_max_redemptions_is_valid_for_basket(): + """A NULL max_redemptions means unlimited, not a TypeError.""" + basket_item = BasketItemFactory.create() + discount = UnlimitedUseDiscountFactory.create(max_redemptions=None) + + assert discount.is_valid_for_basket(basket_item.basket) + + +def test_discount_with_unfulfilled_redemption_is_valid_for_basket(): + """Redemptions on unfulfilled orders don't consume the limit, matching is_redeemable_by.""" + basket_item = BasketItemFactory.create() + discount = UnlimitedUseDiscountFactory.create(max_redemptions=1) + order = OrderFactory.create( + purchaser=basket_item.basket.user, state=OrderStatus.PENDING + ) + DiscountRedemption.objects.create( + redeemed_discount=discount, + redeemed_by=basket_item.basket.user, + redeemed_order=order, + redemption_date=now_in_utc(), + ) + + assert discount.is_valid_for_basket(basket_item.basket) @pytest.mark.parametrize("is_none", [True, False]) @@ -779,7 +823,7 @@ def test_discount_with_max_redemptions_is_valid_for_basket(is_none): redeemed_order=order, redemption_date=now_in_utc(), ) - assert discount.is_valid(basket_item.basket) + assert discount.is_valid_for_basket(basket_item.basket) @pytest.mark.parametrize("is_none", [True, False]) @@ -791,7 +835,7 @@ def test_discount_with_activation_date_in_past_is_valid_for_basket(is_none): activation_date=activation_date, amount=10, ) - assert discount.is_valid(basket_item.basket) + assert discount.is_valid_for_basket(basket_item.basket) @pytest.mark.parametrize("is_none", [True, False]) @@ -803,7 +847,7 @@ def test_discount_with_expiration_date_in_future_is_valid_for_basket(is_none): expiration_date=expiration_date, amount=10, ) - assert discount.is_valid(basket_item.basket) + assert discount.is_valid_for_basket(basket_item.basket) def test_discount_with_unmatched_product_value_is_not_valid_for_basket(): @@ -818,7 +862,7 @@ def test_discount_with_unmatched_product_value_is_not_valid_for_basket(): discount=discount, product=product, ) - assert not discount.is_valid(basket_item.basket) + assert not discount.is_valid_for_basket(basket_item.basket) def test_discount_with_unmatched_user_value_is_not_valid_for_basket(): @@ -830,7 +874,7 @@ def test_discount_with_unmatched_user_value_is_not_valid_for_basket(): ) user = UserFactory.create() UserDiscount.objects.create(discount=discount, user=user) - assert not discount.is_valid(basket_item.basket) + assert not discount.is_valid_for_basket(basket_item.basket) def test_discount_with_max_redemptions_is_not_valid_for_basket(): @@ -842,14 +886,16 @@ def test_discount_with_max_redemptions_is_not_valid_for_basket(): amount=10, ) - order = OrderFactory.create(purchaser=basket_item.basket.user) + order = OrderFactory.create( + purchaser=basket_item.basket.user, state=OrderStatus.FULFILLED + ) DiscountRedemption.objects.create( redeemed_discount=discount, redeemed_by=basket_item.basket.user, redeemed_order=order, redemption_date=now_in_utc(), ) - assert not discount.is_valid(basket_item.basket) + assert not discount.is_valid_for_basket(basket_item.basket) def test_discount_with_activation_date_in_future_is_not_valid_for_basket(): @@ -860,7 +906,7 @@ def test_discount_with_activation_date_in_future_is_not_valid_for_basket(): activation_date=activation_date, amount=10, ) - assert not discount.is_valid(basket_item.basket) + assert not discount.is_valid_for_basket(basket_item.basket) def test_discount_with_expiration_date_in_past_is_not_valid_for_basket(): @@ -874,7 +920,70 @@ def test_discount_with_expiration_date_in_past_is_not_valid_for_basket(): amount=10, ) - assert not discount.is_valid(basket_item.basket) + assert not discount.is_valid_for_basket(basket_item.basket) + + +def test_one_time_discount_with_fulfilled_redemption_is_not_valid_for_basket(): + """A one-time discount already redeemed on a fulfilled order can't be reused.""" + basket_item = BasketItemFactory.create() + discount = OneTimeDiscountFactory.create() + order = OrderFactory.create( + purchaser=basket_item.basket.user, state=OrderStatus.FULFILLED + ) + DiscountRedemption.objects.create( + redeemed_discount=discount, + redeemed_by=basket_item.basket.user, + redeemed_order=order, + redemption_date=now_in_utc(), + ) + assert not discount.is_valid_for_basket(basket_item.basket) + + +def test_one_time_discount_with_pending_redemption_is_valid_for_basket(): + """A one-time discount redeemed only on a pending order hasn't been consumed yet.""" + basket_item = BasketItemFactory.create() + discount = OneTimeDiscountFactory.create() + order = OrderFactory.create( + purchaser=basket_item.basket.user, state=OrderStatus.PENDING + ) + DiscountRedemption.objects.create( + redeemed_discount=discount, + redeemed_by=basket_item.basket.user, + redeemed_order=order, + redemption_date=now_in_utc(), + ) + assert discount.is_valid_for_basket(basket_item.basket) + + +def test_one_time_per_user_discount_redeemed_by_basket_user_is_not_valid_for_basket(): + """A one-time-per-user discount already redeemed by this user can't be reused by them.""" + basket_item = BasketItemFactory.create() + discount = OneTimePerUserDiscountFactory.create() + order = OrderFactory.create( + purchaser=basket_item.basket.user, state=OrderStatus.FULFILLED + ) + DiscountRedemption.objects.create( + redeemed_discount=discount, + redeemed_by=basket_item.basket.user, + redeemed_order=order, + redemption_date=now_in_utc(), + ) + assert not discount.is_valid_for_basket(basket_item.basket) + + +def test_one_time_per_user_discount_redeemed_by_other_user_is_valid_for_basket(): + """A one-time-per-user discount redeemed by someone else is still available to this user.""" + basket_item = BasketItemFactory.create() + discount = OneTimePerUserDiscountFactory.create() + other_user = UserFactory.create() + order = OrderFactory.create(purchaser=other_user, state=OrderStatus.FULFILLED) + DiscountRedemption.objects.create( + redeemed_discount=discount, + redeemed_by=other_user, + redeemed_order=order, + redemption_date=now_in_utc(), + ) + assert discount.is_valid_for_basket(basket_item.basket) @pytest.mark.skip_nplusone_check diff --git a/ecommerce/serializers/__init__.py b/ecommerce/serializers/__init__.py index 47df274a20..a6c5113a50 100644 --- a/ecommerce/serializers/__init__.py +++ b/ecommerce/serializers/__init__.py @@ -17,6 +17,7 @@ DISCOUNT_TYPE_PERCENT_OFF, DISCOUNT_TYPES, PAYMENT_TYPES, + REDEMPTION_TYPES, TRANSACTION_TYPE_REFUND, ) from ecommerce.discounts import product_from_version @@ -667,22 +668,62 @@ class Meta: ] +MAX_PERCENT_OFF = Decimal(100) + + class BulkDiscountSerializer(serializers.Serializer): """For validating bulk discount requests.""" + # Every field generate_discount_code honors has to be declared here: the + # views pass validated_data, so an undeclared key is dropped rather than + # reaching the function. redemption_type in particular is load-bearing — + # the staff dashboard's bulk form sends it. discount_type = serializers.ChoiceField(choices=DISCOUNT_TYPES) + redemption_type = serializers.ChoiceField(choices=REDEMPTION_TYPES, required=False) payment_type = serializers.ChoiceField(choices=PAYMENT_TYPES) amount = serializers.DecimalField(max_digits=9, decimal_places=2) one_time = serializers.BooleanField(default=False) - one_time_per_user = serializers.BooleanField(default=False) + once_per_user = serializers.BooleanField(default=False) activates = serializers.DateTimeField( required=False, default_timezone=ZoneInfo(TIME_ZONE) ) expires = serializers.DateTimeField( required=False, default_timezone=ZoneInfo(TIME_ZONE) ) - count = serializers.IntegerField(required=False) - prefix = serializers.CharField(max_length=63, required=False) + prefix = serializers.CharField( + max_length=63, + required=False, + help_text="Generate codes from this prefix plus a UUID.", + ) + count = serializers.IntegerField( + required=False, + min_value=1, + help_text="How many codes to generate from prefix. Defaults to 1.", + ) + codes = serializers.ListField( + child=serializers.CharField(max_length=100), + required=False, + help_text="The exact codes to create, instead of generating from a prefix.", + ) + + def validate(self, attrs): + """Reject the input shapes generate_discount_code raises a bare Exception on.""" + if ( + attrs["discount_type"] == DISCOUNT_TYPE_PERCENT_OFF + and attrs["amount"] > MAX_PERCENT_OFF + ): + msg = f"A percent-off discount cannot exceed {MAX_PERCENT_OFF}%." + raise serializers.ValidationError({"amount": msg}) + + if bool(attrs.get("codes")) == bool(attrs.get("prefix")): + msg = "Supply either codes, or a prefix to generate them from." + raise serializers.ValidationError({"codes": msg}) + + if attrs.get("codes") and "count" in attrs: + msg = "count applies only when generating codes from a prefix." + raise serializers.ValidationError({"count": msg}) + + return attrs class UserDiscountSerializer(serializers.ModelSerializer): diff --git a/ecommerce/serializers/v0/__init__.py b/ecommerce/serializers/v0/__init__.py index d7ee954299..00b0899b57 100644 --- a/ecommerce/serializers/v0/__init__.py +++ b/ecommerce/serializers/v0/__init__.py @@ -3,7 +3,6 @@ """ from decimal import Decimal -from zoneinfo import ZoneInfo from django.contrib.auth import get_user_model from drf_spectacular.utils import extend_schema_field, extend_schema_serializer @@ -16,8 +15,6 @@ CYBERSOURCE_CARD_TYPES, DISCOUNT_TYPE_DOLLARS_OFF, DISCOUNT_TYPE_PERCENT_OFF, - DISCOUNT_TYPES, - PAYMENT_TYPES, TRANSACTION_TYPE_REFUND, ) from ecommerce.discounts import product_from_version @@ -42,7 +39,6 @@ USER_MSG_TYPE_ENROLL_BLOCKED, USER_MSG_TYPE_ENROLL_DUPLICATED, ) -from main.settings import TIME_ZONE from users.serializers import ( ExtendedLegalAddressSerializer, PublicUserSerializer, @@ -792,24 +788,6 @@ class Meta: ] -class BulkDiscountSerializer(serializers.Serializer): - """For validating bulk discount requests.""" - - discount_type = serializers.ChoiceField(choices=DISCOUNT_TYPES) - payment_type = serializers.ChoiceField(choices=PAYMENT_TYPES) - amount = serializers.DecimalField(max_digits=9, decimal_places=2) - one_time = serializers.BooleanField(default=False) - one_time_per_user = serializers.BooleanField(default=False) - activates = serializers.DateTimeField( - required=False, default_timezone=ZoneInfo(TIME_ZONE) - ) - expires = serializers.DateTimeField( - required=False, default_timezone=ZoneInfo(TIME_ZONE) - ) - count = serializers.IntegerField(required=False) - prefix = serializers.CharField(max_length=63, required=False) - - class UserDiscountSerializer(serializers.ModelSerializer): """Serializes UserDiscount (many-to-many FK for users and discounts)""" diff --git a/ecommerce/serializers/v0/requests.py b/ecommerce/serializers/v0/requests.py index eb4cc260b7..9df400275e 100644 --- a/ecommerce/serializers/v0/requests.py +++ b/ecommerce/serializers/v0/requests.py @@ -1,8 +1,7 @@ """ Request serializers for MITx Online ecommerce. -Request serializers are used for OpenAPI schema generation - so separating them -out so they don't get used for responses. +These validate incoming request bodies and describe them in the OpenAPI schema. """ from rest_framework import serializers @@ -16,9 +15,12 @@ class CreateBasketWithProductIDSerializer(serializers.Serializer): class CreateBasketWithProductsSerializer(serializers.Serializer): - """Serializer for creating a basket with products. (For OpenAPI spec.)""" + """Serializer for creating a basket with products.""" - system_slug = serializers.CharField() product_ids = CreateBasketWithProductIDSerializer(many=True) - checkout = serializers.BooleanField() - discount_code = serializers.CharField() + checkout = serializers.BooleanField(required=False, default=False) + # `null` and `""` both mean "no discount": the view treats any falsy code as + # absent. CharField rejects both without allow_null/allow_blank. + discount_code = serializers.CharField( + required=False, default=None, allow_null=True, allow_blank=True + ) diff --git a/ecommerce/views/legacy/__init__.py b/ecommerce/views/legacy/__init__.py index 5dcdac5b48..f24aad98a6 100644 --- a/ecommerce/views/legacy/__init__.py +++ b/ecommerce/views/legacy/__init__.py @@ -400,7 +400,9 @@ def create_batch(self, request): otherSerializer = BulkDiscountSerializer(data=request.data) if otherSerializer.is_valid(): - generated_codes = api.generate_discount_code(**request.data) + generated_codes = api.generate_discount_code( + **otherSerializer.validated_data + ) discounts = DiscountSerializer(generated_codes, many=True) @@ -637,7 +639,7 @@ def redeem_discount(self, request): if not api.check_discount_for_products(discount, basket): raise ObjectDoesNotExist() # noqa: RSE102, TRY301 - if not discount.check_validity(request.user): + if not discount.is_redeemable_by(request.user): log.error( f"Discount code {request.data['discount']} has already been redeemed" # noqa: G004 ) diff --git a/ecommerce/views/v0/__init__.py b/ecommerce/views/v0/__init__.py index 64b8c4b391..7a336d5f70 100644 --- a/ecommerce/views/v0/__init__.py +++ b/ecommerce/views/v0/__init__.py @@ -64,10 +64,10 @@ Product, UserDiscount, ) +from ecommerce.serializers import BulkDiscountSerializer from ecommerce.serializers.v0 import ( BasketItemSerializer, BasketWithProductSerializer, - BulkDiscountSerializer, CheckoutPayloadSerializer, DiscountProductSerializer, DiscountRedemptionSerializer, @@ -371,27 +371,35 @@ def create_basket_from_product_with_discount( def create_basket_with_products(request): """ Create new basket items for the currently logged in user. Reuse the existing - basket object if it exists. Optionally apply the specified discount. + basket object if it exists. Optionally apply the supplied discount code. + + Eligible auto-apply and financial assistance discounts are applied whether + or not a discount code is supplied. If the checkout flag is set in the POST data, then this will create the basket, then immediately flip the user to the checkout interstitial (which then redirects to the payment gateway). - If any of the products aren't found, this will return a 404 error. If - the discount code is invalid, the discount won't be applied and an error - will be logged, but the basket will still be updated. + If any of the products aren't found, this will return a 404 error. A + discount code that isn't found, isn't redeemable, or doesn't beat a + discount already on the basket is dropped silently, and the basket is + still updated. POST Args: + product_ids (list[dict]): products to add to the basket, each + ``{"product_id": int, "quantity": int}`` (required) checkout (bool): redirect to checkout interstitial (defaults to False) - product_ids (list[(str, int)]): list of product SKUs to add to the basket with quantity - discount_code (str): discount code to apply to the basket + discount_code (str): discount code to apply to the basket (optional) Returns: Response: HTTP response """ - checkout = request.data.get("checkout", False) - discount_code = request.data.get("discount_code", None) - product_ids = request.data.get("product_ids", []) + serializer = requests.CreateBasketWithProductsSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + checkout = serializer.validated_data["checkout"] + discount_code = serializer.validated_data["discount_code"] + product_ids = serializer.validated_data["product_ids"] basket = establish_basket(request) products = [] @@ -399,10 +407,10 @@ def create_basket_with_products(request): try: products = [ ( - Product.objects.get(id=product_id["product_id"]), - product_id["quantity"], + Product.objects.get(id=item["product_id"]), + item["quantity"], ) - for product_id in product_ids + for item in product_ids ] except Product.DoesNotExist: return Response( @@ -468,9 +476,6 @@ def clear_basket(request): """ Clear the basket for the current user. - Args: - system_slug (str): system slug - Returns: Response: HTTP response """ @@ -681,7 +686,20 @@ class DiscountViewSet(ModelViewSet): filter_backends = (django_filters.rest_framework.DjangoFilterBackend,) filterset_class = DiscountFilterSet - @action(url_name="create_batch", detail=False, methods=["post"]) + # filters=False and pagination_class=None because the schema otherwise + # inherits the viewset's filter and pagination query params, which this + # POST action never reads. + @extend_schema( + request=BulkDiscountSerializer, + responses={201: V0DiscountSerializer(many=True)}, + filters=False, + ) + @action( + url_name="create_batch", + detail=False, + methods=["post"], + pagination_class=None, + ) def create_batch(self, request): """ Create a batch of codes. This is used in the staff-dashboard. @@ -691,7 +709,7 @@ def create_batch(self, request): otherSerializer = BulkDiscountSerializer(data=request.data) if otherSerializer.is_valid(): - generated_codes = generate_discount_code(**request.data) + generated_codes = generate_discount_code(**otherSerializer.validated_data) discounts = V0DiscountSerializer(generated_codes, many=True) diff --git a/ecommerce/views/v0/views_test.py b/ecommerce/views/v0/views_test.py index fc9d6ebb35..04aee98474 100644 --- a/ecommerce/views/v0/views_test.py +++ b/ecommerce/views/v0/views_test.py @@ -387,6 +387,66 @@ def test_create_basket_with_products( ) +@pytest.mark.parametrize( + "payload", + [ + pytest.param({}, id="product_ids-missing"), + pytest.param({"product_ids": [5]}, id="product_ids-not-objects"), + pytest.param( + {"product_ids": [{"product_id": 5, "quantity": 0}]}, + id="quantity-below-minimum", + ), + ], +) +def test_create_basket_with_products_rejects_malformed_body(user_client, payload): + """A body that doesn't match the request serializer is rejected with a 400.""" + + response = user_client.post( + reverse("v0:baskets_api-create_with_products"), + data=payload, + content_type="application/json", + ) + + assert response.status_code == 400 + + +@pytest.mark.parametrize("discount_code", ["", None], ids=["blank", "null"]) +def test_create_basket_with_products_empty_discount_code(user_client, discount_code): + """An empty discount code means "no discount", not a validation error.""" + + product = ProductFactory.create() + + response = user_client.post( + reverse("v0:baskets_api-create_with_products"), + data={ + "product_ids": [{"product_id": product.id, "quantity": 1}], + "discount_code": discount_code, + }, + content_type="application/json", + ) + + assert response.status_code == 200 + assert Basket.objects.get(id=response.data["id"]).discounts.count() == 0 + + +def test_create_basket_with_products_checkout_redirects(user_client): + """The checkout flag sends the user to the checkout interstitial.""" + + product = ProductFactory.create() + + response = user_client.post( + reverse("v0:baskets_api-create_with_products"), + data={ + "product_ids": [{"product_id": product.id, "quantity": 1}], + "checkout": True, + }, + content_type="application/json", + ) + + assert response.status_code == 302 + assert response.url == reverse("checkout_interstitial_page") + + @pytest.mark.parametrize( ( "existing_basket", @@ -454,7 +514,7 @@ def test_create_basket_with_product( # noqa: PLR0913 max_redemptions=1, redemption_type=REDEMPTION_TYPE_UNLIMITED, ) - order = OrderFactory.create() + order = OrderFactory.create(state=OrderStatus.FULFILLED) DiscountRedemption.objects.create( redeemed_by=order.purchaser, redemption_date=order.created_on, @@ -905,6 +965,102 @@ def test_bulk_discount_create(admin_drf_client, use_redemption_type_flags): assert discounts[0].is_bulk +def test_bulk_discount_create_rejects_a_percent_off_above_100(admin_drf_client): + """ + generate_discount_code enforces this ceiling with a bare Exception, so the + serializer has to answer 400 before the view reaches it. + """ + resp = admin_drf_client.post( + reverse("v0:discounts_api-create_batch"), + { + "discount_type": DISCOUNT_TYPE_PERCENT_OFF, + "payment_type": PAYMENT_TYPE_CUSTOMER_SUPPORT, + "count": 5, + "amount": 101, + "prefix": "Generated-Code-", + }, + ) + + assert resp.status_code == 400 + assert not Discount.objects.filter( + discount_code__startswith="Generated-Code-" + ).exists() + + +@pytest.mark.parametrize("count", [None, 1], ids=["count-omitted", "count-1"]) +def test_bulk_discount_create_generates_one_code_from_a_prefix(admin_drf_client, count): + """A prefix generates codes whether or not the caller asks for more than one.""" + payload = { + "discount_type": DISCOUNT_TYPE_PERCENT_OFF, + "payment_type": PAYMENT_TYPE_CUSTOMER_SUPPORT, + "amount": 50, + "prefix": "Generated-Code-", + } + + if count is not None: + payload["count"] = count + + resp = admin_drf_client.post( + reverse("v0:discounts_api-create_batch"), + payload, + ) + + assert resp.status_code == 201 + assert ( + Discount.objects.filter(discount_code__startswith="Generated-Code-").count() + == 1 + ) + + +def test_bulk_discount_create_with_explicit_codes(admin_drf_client): + """Named codes are created verbatim, with no prefix involved.""" + resp = admin_drf_client.post( + reverse("v0:discounts_api-create_batch"), + { + "discount_type": DISCOUNT_TYPE_PERCENT_OFF, + "payment_type": PAYMENT_TYPE_CUSTOMER_SUPPORT, + "amount": 50, + "codes": ["FIRST-CODE", "SECOND-CODE"], + }, + ) + + assert resp.status_code == 201 + assert sorted(Discount.objects.values_list("discount_code", flat=True)) == [ + "FIRST-CODE", + "SECOND-CODE", + ] + + +@pytest.mark.parametrize( + "extra", + [ + pytest.param( + {"prefix": "Generated-Code-", "codes": ["A-CODE"]}, id="prefix-and-codes" + ), + pytest.param({}, id="neither-prefix-nor-codes"), + pytest.param({"codes": ["A-CODE"], "count": 2}, id="codes-with-count"), + pytest.param({"prefix": "Generated-Code-", "count": 0}, id="count-below-one"), + ], +) +def test_bulk_discount_create_rejects_ambiguous_code_sources(admin_drf_client, extra): + """ + generate_discount_code takes codes or a prefix, never both and never neither, + and count only applies to a prefix. The serializer answers 400 for the rest. + """ + resp = admin_drf_client.post( + reverse("v0:discounts_api-create_batch"), + { + "discount_type": DISCOUNT_TYPE_PERCENT_OFF, + "payment_type": PAYMENT_TYPE_CUSTOMER_SUPPORT, + "amount": 50, + **extra, + }, + ) + + assert resp.status_code == 400 + assert not Discount.objects.exists() + + # Checkout tests diff --git a/flexiblepricing/messages.py b/flexiblepricing/messages.py index 04e684783c..5651aedcab 100644 --- a/flexiblepricing/messages.py +++ b/flexiblepricing/messages.py @@ -1,8 +1,8 @@ """flexible price status change email messages""" -from mitol.mail.messages import TemplatedMessage +from mail.messages import SiteTemplatedMessage -class FlexiblePriceStatusChangeMessage(TemplatedMessage): +class FlexiblePriceStatusChangeMessage(SiteTemplatedMessage): template_name = "mail/flexible_price" name = "Flexible Price Status Change" diff --git a/mail/api.py b/mail/api.py index d555493f7f..a863f38bd5 100644 --- a/mail/api.py +++ b/mail/api.py @@ -32,6 +32,7 @@ from django.core.validators import validate_email from django.template.loader import render_to_string +from mail.constants import EMAIL_SITE_NAME from mail.exceptions import EmailSendFailureException, MultiEmailValidationError log = logging.getLogger() @@ -97,7 +98,7 @@ def get_base_context(): """Returns a dict of context variables that are needed in all emails""" return { "base_url": settings.SITE_BASE_URL, - "site_name": "MIT Learn", + "site_name": EMAIL_SITE_NAME, "mit_learn_terms_url": settings.MIT_LEARN_TERMS_URL, "mit_learn_privacy_url": settings.MIT_LEARN_PRIVACY_URL, "mit_learn_honor_code_url": settings.MIT_LEARN_HONOR_CODE_URL, diff --git a/mail/constants.py b/mail/constants.py index 69e539d320..30854f7312 100644 --- a/mail/constants.py +++ b/mail/constants.py @@ -1,5 +1,10 @@ """Constants for the mail app""" +# The brand name shown in emails (subject lines, "site_name" template context, +# etc.), independent of settings.SITE_NAME, which also drives non-email UI +# and can be overridden per-environment. +EMAIL_SITE_NAME = "MIT Learn" + EMAIL_VERIFICATION = "verification" EMAIL_PW_RESET = "password_reset" EMAIL_CHANGE_EMAIL = "change_email" diff --git a/mail/messages.py b/mail/messages.py new file mode 100644 index 0000000000..423c1c1dec --- /dev/null +++ b/mail/messages.py @@ -0,0 +1,22 @@ +"""Shared base classes for mitol-mail TemplatedMessage subclasses""" + +from mitol.mail.messages import TemplatedMessage + +from mail.constants import EMAIL_SITE_NAME + + +class SiteTemplatedMessage(TemplatedMessage): + """ + Base class for app email messages built on mitol-mail's TemplatedMessage. + + Overrides the "site_name" template context so these emails always say + "MIT Learn", regardless of settings.SITE_NAME (which also drives + non-email UI and can be overridden per-environment). + """ + + @staticmethod + def get_base_template_context() -> dict: + return { + **TemplatedMessage.get_base_template_context(), + "site_name": EMAIL_SITE_NAME, + } diff --git a/main/message.py b/main/message.py index a8e351d255..586b59feda 100644 --- a/main/message.py +++ b/main/message.py @@ -1,9 +1,9 @@ """Main message classes""" -from mitol.mail.messages import TemplatedMessage +from mail.messages import SiteTemplatedMessage -class SupportMessage(TemplatedMessage): +class SupportMessage(SiteTemplatedMessage): """Support email message""" template_name = "mail/support" diff --git a/main/settings.py b/main/settings.py index 1a1a72d780..f7fffb20d4 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.164.1" +VERSION = "1.164.2" log = logging.getLogger() diff --git a/openapi/specs/oasdiff-err-ignore.txt b/openapi/specs/oasdiff-err-ignore.txt index b4e341b8f7..173d3552b8 100644 --- a/openapi/specs/oasdiff-err-ignore.txt +++ b/openapi/specs/oasdiff-err-ignore.txt @@ -197,3 +197,22 @@ GET /api/v2/pages/?fields=*&type=cms.coursepage removed the required property `i GET /api/v2/pages/?fields=*&type=cms.programpage removed the required property `items/items/show_stay_updated` from the response with the `200` status GET /api/v2/pages/{id}/ removed the required property `oneOf[#/components/schemas/CoursePageItem]/show_stay_updated` from the response with the `200` status GET /api/v2/pages/{id}/ removed the required property `oneOf[#/components/schemas/ProgramPageItem]/show_stay_updated` from the response with the `200` status +# Spec correction: create_batch's schema previously advertised the wrong request +# serializer (V0DiscountRequest) and a single 200 response. The endpoint has +# always required a valid payment_type (generate_discount_code raises on any +# other input) and always returned 201 with an array. The spec now tells the +# truth; the API's behavior is unchanged. +POST /api/v0/discounts/create_batch/ request property `payment_type` was restricted to a list of enum values (media type: application/json) +POST /api/v0/discounts/create_batch/ request property `payment_type` was restricted to a list of enum values (media type: application/x-www-form-urlencoded) +POST /api/v0/discounts/create_batch/ the request property `payment_type` became not nullable (media type: application/json) +POST /api/v0/discounts/create_batch/ the request property `payment_type` became not nullable (media type: application/x-www-form-urlencoded) +POST /api/v0/discounts/create_batch/ the request property `payment_type` became not nullable (media type: multipart/form-data) +POST /api/v0/discounts/create_batch/ the request property `payment_type` became required (media type: application/x-www-form-urlencoded) +POST /api/v0/discounts/create_batch/ the request property `payment_type` became required (media type: application/json) +POST /api/v0/discounts/create_batch/ the request property `payment_type` became required (media type: multipart/form-data) +POST /api/v0/discounts/create_batch/ removed `#/components/schemas/PaymentTypeEnum, #/components/schemas/NullEnum` from the `payment_type` request property `oneOf` list (media type: application/x-www-form-urlencoded) +POST /api/v0/discounts/create_batch/ removed `#/components/schemas/PaymentTypeEnum, #/components/schemas/NullEnum` from the `payment_type` request property `oneOf` list (media type: application/json) +POST /api/v0/discounts/create_batch/ removed `#/components/schemas/PaymentTypeEnum, #/components/schemas/NullEnum` from the `payment_type` request property `oneOf` list (media type: multipart/form-data) +POST /api/v0/discounts/create_batch/ the `payment_type` request property `type` changed from `any` to `string` (media type: application/json) +POST /api/v0/discounts/create_batch/ removed the success response with the status `200` +POST /api/v0/discounts/create_batch/ request property `payment_type` was restricted to a list of enum values (media type: multipart/form-data) diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 534a18d535..05ced430a8 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -2276,20 +2276,22 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/V0DiscountRequest' + $ref: '#/components/schemas/BulkDiscountRequest' application/x-www-form-urlencoded: schema: - $ref: '#/components/schemas/V0DiscountRequest' + $ref: '#/components/schemas/BulkDiscountRequest' multipart/form-data: schema: - $ref: '#/components/schemas/V0DiscountRequest' + $ref: '#/components/schemas/BulkDiscountRequest' required: true responses: - '200': + '201': content: application/json: schema: - $ref: '#/components/schemas/V0Discount' + type: array + items: + $ref: '#/components/schemas/V0Discount' description: '' /api/v0/orders/history/: get: @@ -4684,6 +4686,52 @@ components: required: - assigned - errors + BulkDiscountRequest: + type: object + description: For validating bulk discount requests. + properties: + discount_type: + $ref: '#/components/schemas/DiscountTypeEnum' + redemption_type: + $ref: '#/components/schemas/RedemptionTypeEnum' + payment_type: + $ref: '#/components/schemas/PaymentTypeEnum' + amount: + type: string + format: decimal + pattern: ^-?\d{0,7}(?:\.\d{0,2})?$ + one_time: + type: boolean + default: false + once_per_user: + type: boolean + default: false + activates: + type: string + format: date-time + expires: + type: string + format: date-time + prefix: + type: string + minLength: 1 + description: Generate codes from this prefix plus a UUID. + maxLength: 63 + count: + type: integer + minimum: 1 + description: How many codes to generate from prefix. Defaults to 1. + codes: + type: array + items: + type: string + minLength: 1 + maxLength: 100 + description: The exact codes to create, instead of generating from a prefix. + required: + - amount + - discount_type + - payment_type CertificatePage: type: object description: Serializer for certificate pages, including overrides and signatory @@ -6353,25 +6401,20 @@ components: - quantity CreateBasketWithProductsRequest: type: object - description: Serializer for creating a basket with products. (For OpenAPI spec.) + description: Serializer for creating a basket with products. properties: - system_slug: - type: string - minLength: 1 product_ids: type: array items: $ref: '#/components/schemas/CreateBasketWithProductIDRequest' checkout: type: boolean + default: false discount_code: type: string - minLength: 1 + nullable: true required: - - checkout - - discount_code - product_ids - - system_slug Department: type: object description: Department model serializer diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 0f93c1343e..d589994e2c 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -2276,20 +2276,22 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/V0DiscountRequest' + $ref: '#/components/schemas/BulkDiscountRequest' application/x-www-form-urlencoded: schema: - $ref: '#/components/schemas/V0DiscountRequest' + $ref: '#/components/schemas/BulkDiscountRequest' multipart/form-data: schema: - $ref: '#/components/schemas/V0DiscountRequest' + $ref: '#/components/schemas/BulkDiscountRequest' required: true responses: - '200': + '201': content: application/json: schema: - $ref: '#/components/schemas/V0Discount' + type: array + items: + $ref: '#/components/schemas/V0Discount' description: '' /api/v0/orders/history/: get: @@ -4684,6 +4686,52 @@ components: required: - assigned - errors + BulkDiscountRequest: + type: object + description: For validating bulk discount requests. + properties: + discount_type: + $ref: '#/components/schemas/DiscountTypeEnum' + redemption_type: + $ref: '#/components/schemas/RedemptionTypeEnum' + payment_type: + $ref: '#/components/schemas/PaymentTypeEnum' + amount: + type: string + format: decimal + pattern: ^-?\d{0,7}(?:\.\d{0,2})?$ + one_time: + type: boolean + default: false + once_per_user: + type: boolean + default: false + activates: + type: string + format: date-time + expires: + type: string + format: date-time + prefix: + type: string + minLength: 1 + description: Generate codes from this prefix plus a UUID. + maxLength: 63 + count: + type: integer + minimum: 1 + description: How many codes to generate from prefix. Defaults to 1. + codes: + type: array + items: + type: string + minLength: 1 + maxLength: 100 + description: The exact codes to create, instead of generating from a prefix. + required: + - amount + - discount_type + - payment_type CertificatePage: type: object description: Serializer for certificate pages, including overrides and signatory @@ -6353,25 +6401,20 @@ components: - quantity CreateBasketWithProductsRequest: type: object - description: Serializer for creating a basket with products. (For OpenAPI spec.) + description: Serializer for creating a basket with products. properties: - system_slug: - type: string - minLength: 1 product_ids: type: array items: $ref: '#/components/schemas/CreateBasketWithProductIDRequest' checkout: type: boolean + default: false discount_code: type: string - minLength: 1 + nullable: true required: - - checkout - - discount_code - product_ids - - system_slug Department: type: object description: Department model serializer diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index 025984c2f9..2a3a4d9b0a 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -2276,20 +2276,22 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/V0DiscountRequest' + $ref: '#/components/schemas/BulkDiscountRequest' application/x-www-form-urlencoded: schema: - $ref: '#/components/schemas/V0DiscountRequest' + $ref: '#/components/schemas/BulkDiscountRequest' multipart/form-data: schema: - $ref: '#/components/schemas/V0DiscountRequest' + $ref: '#/components/schemas/BulkDiscountRequest' required: true responses: - '200': + '201': content: application/json: schema: - $ref: '#/components/schemas/V0Discount' + type: array + items: + $ref: '#/components/schemas/V0Discount' description: '' /api/v0/orders/history/: get: @@ -4684,6 +4686,52 @@ components: required: - assigned - errors + BulkDiscountRequest: + type: object + description: For validating bulk discount requests. + properties: + discount_type: + $ref: '#/components/schemas/DiscountTypeEnum' + redemption_type: + $ref: '#/components/schemas/RedemptionTypeEnum' + payment_type: + $ref: '#/components/schemas/PaymentTypeEnum' + amount: + type: string + format: decimal + pattern: ^-?\d{0,7}(?:\.\d{0,2})?$ + one_time: + type: boolean + default: false + once_per_user: + type: boolean + default: false + activates: + type: string + format: date-time + expires: + type: string + format: date-time + prefix: + type: string + minLength: 1 + description: Generate codes from this prefix plus a UUID. + maxLength: 63 + count: + type: integer + minimum: 1 + description: How many codes to generate from prefix. Defaults to 1. + codes: + type: array + items: + type: string + minLength: 1 + maxLength: 100 + description: The exact codes to create, instead of generating from a prefix. + required: + - amount + - discount_type + - payment_type CertificatePage: type: object description: Serializer for certificate pages, including overrides and signatory @@ -6353,25 +6401,20 @@ components: - quantity CreateBasketWithProductsRequest: type: object - description: Serializer for creating a basket with products. (For OpenAPI spec.) + description: Serializer for creating a basket with products. properties: - system_slug: - type: string - minLength: 1 product_ids: type: array items: $ref: '#/components/schemas/CreateBasketWithProductIDRequest' checkout: type: boolean + default: false discount_code: type: string - minLength: 1 + nullable: true required: - - checkout - - discount_code - product_ids - - system_slug Department: type: object description: Department model serializer