From bf36fe807e530777708c8a962faf3b779bd0fd9a Mon Sep 17 00:00:00 2001 From: rjv31 Date: Mon, 22 Sep 2025 11:44:34 +0000 Subject: [PATCH] feat(api): enhance seat endpoint to support metadata (credit provider, hours, upgrade deadline) --- course_discovery/apps/api/filters.py | 6 +- course_discovery/apps/api/serializers.py | 72 ++++++++++-- .../apps/api/tests/test_serializers.py | 111 +++++++++++++++++- .../apps/course_metadata/models.py | 24 +++- .../apps/course_metadata/tests/test_models.py | 52 ++++++++ 5 files changed, 252 insertions(+), 13 deletions(-) diff --git a/course_discovery/apps/api/filters.py b/course_discovery/apps/api/filters.py index 3cda3bfdfd..474a73bb63 100644 --- a/course_discovery/apps/api/filters.py +++ b/course_discovery/apps/api/filters.py @@ -140,6 +140,10 @@ class CourseRunFilter(FilterSetMixin, filters.FilterSet): marketable = filters.BooleanFilter(method='filter_marketable') keys = CharListFilter(field_name='key', lookup_expr='in') license = filters.CharFilter(field_name='license', lookup_expr='iexact') + credit_provider = filters.CharFilter( + field_name='seats__credit_provider', + lookup_expr='iexact', + help_text="Filter course runs by credit provider") @property def qs(self): @@ -152,7 +156,7 @@ def qs(self): class Meta: model = CourseRun - fields = ('keys', 'hidden', 'license',) + fields = ('keys', 'hidden', 'license', 'credit_provider',) class ProgramFilter(FilterSetMixin, filters.FilterSet): diff --git a/course_discovery/apps/api/serializers.py b/course_discovery/apps/api/serializers.py index 3f60a54992..e92464036e 100644 --- a/course_discovery/apps/api/serializers.py +++ b/course_discovery/apps/api/serializers.py @@ -15,6 +15,7 @@ from django.contrib.auth import get_user_model from django.db.models import Count from django.db.models.query import Prefetch +from django.utils import timezone from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ from django_countries.serializer_fields import CountryField @@ -800,12 +801,12 @@ class SeatSerializer(BaseModelSerializer): min_value=0, ) currency = serializers.SlugRelatedField(read_only=True, slug_field='code') - upgrade_deadline = serializers.DateTimeField() - upgrade_deadline_override = serializers.DateTimeField() - credit_provider = serializers.CharField() - credit_hours = serializers.IntegerField() - sku = serializers.CharField() - bulk_sku = serializers.CharField() + upgrade_deadline = serializers.DateTimeField(required=False, allow_null=True) + upgrade_deadline_override = serializers.DateTimeField(required=False, allow_null=True) + credit_provider = serializers.CharField(required=False, allow_null=True, allow_blank=True) + credit_hours = serializers.IntegerField(required=False, allow_null=True) + sku = serializers.CharField(required=False, allow_null=True, allow_blank=True) + bulk_sku = serializers.CharField(required=False, allow_null=True, allow_blank=True) @classmethod def prefetch_queryset(cls): @@ -967,6 +968,7 @@ def prefetch_queryset(cls, queryset=None): 'course__partner', 'restricted_run', Prefetch('seats', queryset=SeatSerializer.prefetch_queryset()), + 'seats' ) class Meta: @@ -1074,6 +1076,7 @@ class CourseRunSerializer(MinimalCourseRunSerializer): ) estimated_hours = serializers.SerializerMethodField() enterprise_subscription_inclusion = serializers.BooleanField(required=False) + seats = SeatSerializer(many=True, required=False) @classmethod def prefetch_queryset(cls, queryset=None): @@ -1093,8 +1096,41 @@ def prefetch_queryset(cls, queryset=None): 'video__image', 'language__translations', Prefetch('staff', queryset=MinimalPersonSerializer.prefetch_queryset()), + Prefetch('seats', queryset=SeatSerializer.prefetch_queryset()), ) + def validate_seats(self, seats): + """ + Validate credit seat metadata: + - Prevent duplicate credit providers for credit seats. + - Ensure credit_hours is positive if a provider is set. + """ + providers = [] + for s in seats: + # normalize type: could be SeatType object or slug string + seat_type = getattr(s.get('type'), 'slug', s.get('type')) + if seat_type == Seat.CREDIT: + provider = s.get('credit_provider') + hours = s.get('credit_hours') + if provider: + # check for duplicates + if provider in providers: + raise serializers.ValidationError( + f"Duplicate credit provider(s): {provider}" + ) + providers.append(provider) + # check credit_hours + if hours is None or hours <= 0: + raise serializers.ValidationError( + f"Credit hours must be a positive integer for provider {provider}" + ) + upgrade_deadline_override = s.get('upgrade_deadline_override') + if upgrade_deadline_override and upgrade_deadline_override < timezone.now(): + raise serializers.ValidationError( + "Upgrade deadline override cannot be in the past." + ) + return seats + class Meta(MinimalCourseRunSerializer.Meta): fields = MinimalCourseRunSerializer.Meta.fields + ( 'course', 'full_description', 'announcement', 'video', 'seats', 'content_language', 'license', 'outcome', @@ -1144,7 +1180,29 @@ def update(self, instance, validated_data): # Handle writing nested video data separately if 'get_video' in validated_data: self.update_video(instance, validated_data.pop('get_video')) - return super().update(instance, validated_data) + seats_data = validated_data.pop('seats', None) + instance = super().update(instance, validated_data) + if seats_data is not None: + for seat in seats_data: + seat_type = getattr(seat.get('type'), 'slug', seat.get('type')) + credit_provider = seat.get('credit_provider') + credit_hours = seat.get('credit_hours') + upgrade_deadline_override = seat.get('upgrade_deadline_override') + price = seat.get('price', 0) + obj_seat, created = instance.seats.get_or_create(type=seat_type, defaults={ + 'price': price, + 'credit_provider': credit_provider, + 'credit_hours': credit_hours, + 'upgrade_deadline_override': upgrade_deadline_override, + }) + + if not created: + obj_seat.price = price + obj_seat.credit_provider = credit_provider + obj_seat.credit_hours = credit_hours + obj_seat.upgrade_deadline_override = upgrade_deadline_override + obj_seat.save() + return instance def validate(self, attrs): course = attrs.get('course', None) diff --git a/course_discovery/apps/api/tests/test_serializers.py b/course_discovery/apps/api/tests/test_serializers.py index ad21deeaf1..c87689b20a 100644 --- a/course_discovery/apps/api/tests/test_serializers.py +++ b/course_discovery/apps/api/tests/test_serializers.py @@ -5,7 +5,9 @@ from urllib.parse import urlencode import ddt +import pytest import responses +from django.core.exceptions import ValidationError from django.test import TestCase from django.utils.text import slugify from django.utils.timezone import now @@ -44,7 +46,9 @@ from course_discovery.apps.core.tests.mixins import ElasticsearchTestMixin, LMSAPIClientMixin from course_discovery.apps.core.utils import serialize_datetime from course_discovery.apps.course_metadata.choices import CourseRunStatus, ProgramStatus -from course_discovery.apps.course_metadata.models import AbstractLocationRestrictionModel, CourseReview, CourseType +from course_discovery.apps.course_metadata.models import ( + AbstractLocationRestrictionModel, CourseReview, CourseType, Seat +) from course_discovery.apps.course_metadata.search_indexes.documents import ( CourseDocument, CourseRunDocument, LearnerPathwayDocument, PersonDocument, ProgramDocument ) @@ -54,6 +58,7 @@ PersonSearchDocumentSerializer, PersonSearchModelSerializer, ProgramSearchDocumentSerializer, ProgramSearchModelSerializer ) +from course_discovery.apps.course_metadata.tests import factories from course_discovery.apps.course_metadata.tests.factories import ( AdditionalMetadataFactory, AdditionalPromoAreaFactory, BulkOperationTaskFactory, CertificateInfoFactory, CollaboratorFactory, CorporateEndorsementFactory, CourseEditorFactory, CourseEntitlementFactory, CourseFactory, @@ -763,7 +768,23 @@ def get_expected_data(cls, course_run, request): 'weeks_to_complete': course_run.weeks_to_complete, 'instructors': [], 'staff': [], - 'seats': [], + 'seats': [ + { + 'type': seat.type.slug, + 'price': str(seat.price), + 'currency': seat.currency.code, + 'upgrade_deadline': json_date_format(seat.upgrade_deadline), + 'upgrade_deadline_override': ( + json_date_format(seat.upgrade_deadline_override) + if seat.upgrade_deadline_override else None + ), + 'credit_provider': seat.credit_provider, + 'credit_hours': seat.credit_hours, + 'sku': seat.sku, + 'bulk_sku': seat.bulk_sku, + } + for seat in course_run.seats.all() + ], 'modified': json_date_format(course_run.modified), 'level_type': course_run.level_type.name_t, 'availability': course_run.availability, @@ -822,6 +843,69 @@ def test_draft_and_official(self): assert serializer.data['marketing_url'] is not None assert serializer.data['marketing_url'] == course_run.marketing_url + @pytest.mark.django_db + def test_course_run_serializer_includes_credit_seats_info(self): + """CourseRunSerializer should include seats with credit info.""" + credit_type = factories.SeatTypeFactory.credit() + cr = factories.CourseRunFactory() + factories.SeatFactory( + course_run=cr, + type=credit_type, + price=150, + credit_provider="providerA", + credit_hours=5, + ) + request = make_request() + data = CourseRunSerializer(cr, context={'request': request}).data + seats = data.get("seats", []) + assert seats, "Expected seats to be present" + seat0 = seats[0] + assert seat0["credit_provider"] == "providerA" + assert seat0["credit_hours"] == 5 + + @pytest.mark.django_db + def test_duplicate_credit_provider_validation_in_serializer(self): + """Serializer must reject payloads that try to assign same credit_provider twice for same run.""" + credit_type = factories.SeatTypeFactory.credit() + cr = factories.CourseRunFactory() + request = make_request() + payload = { + "seats": [ + {"type": credit_type.slug, "price": 100, "credit_provider": "X", "credit_hours": 2}, + {"type": credit_type.slug, "price": 120, "credit_provider": "X", "credit_hours": 4}, + ] + } + serializer = CourseRunSerializer(instance=cr, data=payload, partial=True, context={'request': request}) + assert not serializer.is_valid(), "Serializer should reject duplicate credit_provider" + errors = serializer.errors.get("seats") or serializer.errors + assert "Duplicate credit provider" in str(errors) + + @pytest.mark.django_db + def test_update_credit_fields_via_course_run_serializer(self): + """Updating an existing seat via CourseRunSerializer should change credit_provider/credit_hours.""" + credit_type = factories.SeatTypeFactory.credit() + cr = factories.CourseRunFactory() + factories.SeatFactory( + course_run=cr, + type=credit_type, + price=100, + credit_provider=None, + credit_hours=None, + ) + payload = { + "seats": [ + {"type": credit_type.slug, "price": 110, "credit_provider": "NewProv", "credit_hours": 7} + ] + } + request = make_request() + serializer = CourseRunSerializer(instance=cr, data=payload, partial=True, context={'request': request}) + assert serializer.is_valid(), serializer.errors + updated = serializer.save() + updated_seat = updated.seats.filter(type=credit_type).first() + assert updated_seat.credit_provider == "NewProv" + assert updated_seat.credit_hours == 7 + assert str(updated_seat.price) == "110.00" + class CourseRunWithProgramsSerializerTests(TestCase): def setUp(self): @@ -1916,6 +2000,29 @@ def test_price_validation_errors(self, price): assert serializer.errors['price'] + @pytest.mark.django_db + def test_seat_serializer_includes_credit_fields(self): + """Ensure that SeatSerializer outputs credit_provider and credit_hours when present.""" + credit_type = factories.SeatTypeFactory.credit() + seat = factories.SeatFactory( + type=credit_type, + price=200, + credit_provider="asu", + credit_hours=3, + ) + serialized = SeatSerializer(seat).data + assert serialized["credit_provider"] == "asu" + assert serialized["credit_hours"] == 3 + + @pytest.mark.django_db + def test_seat_serializer_defaults_credit_fields_to_none(self): + """Non-credit seats should serialize credit fields as None.""" + verified_type = factories.SeatTypeFactory.verified() + seat = factories.SeatFactory(type=verified_type, price=100) + serialized = SeatSerializer(seat).data + assert serialized["credit_provider"] is None + assert serialized["credit_hours"] is None + class MinimalPersonSerializerTests(TestCase): def setUp(self): diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index fcee5b7d2d..e6abefcef5 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -2810,7 +2810,7 @@ def get_seat_default_upgrade_deadline(self, seat_type): return None return subtract_deadline_delta(self.end, settings.PUBLISHER_UPGRADE_DEADLINE_DAYS) - def update_or_create_seat_helper(self, seat_type, prices, upgrade_deadline_override): + def update_or_create_seat_helper(self, seat_type, prices, upgrade_deadline_override, credit_provider=None, credit_hours=None): default_deadline = self.get_seat_default_upgrade_deadline(seat_type) defaults = {'upgrade_deadline': default_deadline} @@ -2818,16 +2818,20 @@ def update_or_create_seat_helper(self, seat_type, prices, upgrade_deadline_overr defaults['price'] = prices[seat_type.slug] if seat_type.slug == Seat.VERIFIED: defaults['upgrade_deadline_override'] = upgrade_deadline_override + if seat_type.slug == Seat.CREDIT: + defaults['credit_provider'] = credit_provider + defaults['credit_hours'] = credit_hours seat, __ = Seat.everything.update_or_create( course_run=self, type=seat_type, draft=True, + credit_provider=credit_provider if seat_type.slug == Seat.CREDIT else None, defaults=defaults, ) return seat - def update_or_create_seats(self, run_type=None, prices=None, upgrade_deadline_override=None): + def update_or_create_seats(self, run_type=None, prices=None, upgrade_deadline_override=None, credit_provider=None, credit_hours=None): """ Updates or creates draft seats for a course run. @@ -2842,7 +2846,7 @@ def update_or_create_seats(self, run_type=None, prices=None, upgrade_deadline_ov seats = [] for seat_type in seat_types: - seats.append(self.update_or_create_seat_helper(seat_type, prices, upgrade_deadline_override)) + seats.append(self.update_or_create_seat_helper(seat_type, prices, upgrade_deadline_override, credit_provider=credit_provider, credit_hours=credit_hours)) # Deleting seats here since they would be orphaned otherwise. # One example of how this situation can happen is if a course team is switching between @@ -3279,6 +3283,20 @@ def upgrade_deadline(self): def upgrade_deadline(self, value): self._upgrade_deadline = value + def clean(self): + if self.type.slug == Seat.CREDIT and self.credit_provider: + duplicates = Seat.objects.filter( + course_run=self.course_run, + type__slug=Seat.CREDIT, + credit_provider=self.credit_provider, + ) + if self.pk: + duplicates = duplicates.exclude(pk=self.pk) + if duplicates.exists(): + raise ValidationError( + f"A credit seat with provider '{self.credit_provider}' already exists for this course run." + ) + class CourseEntitlement(ManageHistoryMixin, DraftModelMixin, TimeStampedModel): """ Model storing product metadata for a Course. """ diff --git a/course_discovery/apps/course_metadata/tests/test_models.py b/course_discovery/apps/course_metadata/tests/test_models.py index 67b182d173..d47f1c65e7 100644 --- a/course_discovery/apps/course_metadata/tests/test_models.py +++ b/course_discovery/apps/course_metadata/tests/test_models.py @@ -1743,6 +1743,58 @@ def test_verified_upgrade_deadline_reset_on_upgrade_deadline_override_change(sel assert verified_seat.upgrade_deadline_override is not None assert verified_seat.upgrade_deadline == new_deadline_override + def test_seat_types_includes_credit_when_credit_seat_present(self): + """CourseRun.seat_types should include 'credit' if a credit seat exists.""" + credit_type = factories.SeatTypeFactory.credit() + course_run = factories.CourseRunFactory() + factories.SeatFactory( + course_run=course_run, + type=credit_type, + credit_provider="mit", + credit_hours=3, + ) + + seat_types = course_run.seat_types + seat_type_slugs = [s.slug for s in seat_types] + assert Seat.CREDIT in seat_type_slugs + + def test_credit_seat_stores_provider_and_hours(self): + """A credit seat should correctly save credit_provider and credit_hours.""" + credit_type = factories.SeatTypeFactory.credit() + seat = factories.SeatFactory( + type=credit_type, + price=250, + credit_provider="mit", + credit_hours=4, + ) + assert seat.type.slug == Seat.CREDIT + assert seat.credit_provider == "mit" + assert seat.credit_hours == 4 + + def test_non_credit_seat_allows_null_credit_fields(self): + """Non-credit seats should not require credit fields.""" + verified_type = factories.SeatTypeFactory.verified() + seat = factories.SeatFactory(type=verified_type, price=100) + assert seat.credit_provider is None + assert seat.credit_hours is None + + def test_duplicate_credit_provider_on_same_run_raises_validation_error(self): + credit_type = factories.SeatTypeFactory.credit() + course_run = factories.CourseRunFactory() + factories.SeatFactory(course_run=course_run, type=credit_type, credit_provider="harvard") + seat = factories.SeatFactory.build(course_run=course_run, type=credit_type, credit_provider="harvard") + with pytest.raises(ValidationError): + seat.full_clean() + + def test_upgrade_deadline_property_prefers_override(self): + """upgrade_deadline should return override if present, else fallback to _upgrade_deadline.""" + seat = factories.SeatFactory(upgrade_deadline_override=None) + # Initially falls back to _upgrade_deadline + assert seat.upgrade_deadline == seat._upgrade_deadline + # Override takes precedence + seat.upgrade_deadline_override = seat._upgrade_deadline.replace(year=seat._upgrade_deadline.year + 1) + assert seat.upgrade_deadline == seat.upgrade_deadline_override + class CourseRunTestsThatNeedSetUp(OAuth2Mixin, TestCase): """