diff --git a/course_discovery/apps/api/filters.py b/course_discovery/apps/api/filters.py index 3cda3bfdfd..fa293061e8 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 (for credit seats only).") @property def qs(self): @@ -148,11 +152,11 @@ def qs(self): if not isinstance(self.queryset, QuerySet): return self.queryset - return super().qs + return super().qs.distinct() 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..b5c4ba2108 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): @@ -819,6 +820,14 @@ class Meta: 'credit_hours', 'sku', 'bulk_sku' ) + def to_representation(self, instance): + data = super().to_representation(instance) + if instance.type and instance.type.slug != Seat.CREDIT: + data.pop('credit_provider', None) + data.pop('credit_hours', None) + data.pop('upgrade_deadline', None) + return data + class CourseEntitlementSerializer(BaseModelSerializer): """Serializer for the ``CourseEntitlement`` model.""" @@ -966,7 +975,6 @@ def prefetch_queryset(cls, queryset=None): '_official_version', 'course__partner', 'restricted_run', - Prefetch('seats', queryset=SeatSerializer.prefetch_queryset()), ) class Meta: @@ -1074,6 +1082,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 +1102,48 @@ 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. + - Ensure non-credit seats do not carry credit metadata. + """ + 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}" + ) + else: + # Non-credit seat should not include credit metadata + if s.get('credit_provider') or s.get('credit_hours'): + raise serializers.ValidationError( + "Non-credit seats cannot include credit metadata fields." + ) + 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 +1193,38 @@ 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_value = seat.get('type') + if isinstance(seat_type_value, str): + seat_type = SeatType.objects.filter(slug=seat_type_value).first() + elif hasattr(seat_type_value, 'slug'): + seat_type = seat_type_value + else: + seat_type = None + + if not seat_type: + continue # skip invalid seat data safely + + defaults = { + 'price': seat.get('price', 0), + 'upgrade_deadline_override': seat.get('upgrade_deadline_override'), + 'credit_provider': seat.get('credit_provider'), + 'credit_hours': seat.get('credit_hours'), + } + + obj_seat, created = instance.seats.get_or_create(type=seat_type, defaults=defaults) + + if not created: + for field, value in defaults.items(): + # Only overwrite if explicitly passed in seat data + if field in seat: + setattr(obj_seat, field, value) + obj_seat.full_clean() # ensure model validation runs + 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..d4db2db83c 100644 --- a/course_discovery/apps/api/tests/test_serializers.py +++ b/course_discovery/apps/api/tests/test_serializers.py @@ -5,6 +5,7 @@ from urllib.parse import urlencode import ddt +import pytest import responses from django.test import TestCase from django.utils.text import slugify @@ -54,6 +55,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 +765,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 +840,84 @@ 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_non_credit_seat_with_credit_fields_rejected(self): + """Serializer should reject non-credit seats carrying credit metadata.""" + verified_type = factories.SeatTypeFactory.verified() + cr = factories.CourseRunFactory() + request = make_request() + payload = { + "seats": [ + {"type": verified_type.slug, "price": 100, "credit_provider": "BadProv", "credit_hours": 2} + ] + } + serializer = CourseRunSerializer(instance=cr, data=payload, partial=True, context={'request': request}) + assert not serializer.is_valid(), "Serializer should reject credit metadata for non-credit seat" + assert "Non-credit seats cannot include credit metadata" in str(serializer.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 +2012,35 @@ 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 fields for credit seats.""" + 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 + assert "upgrade_deadline" in serialized + + @pytest.mark.django_db + def test_seat_serializer_hides_credit_fields_for_non_credit_seat(self): + """Non-credit seats should NOT expose credit fields.""" + verified_type = factories.SeatTypeFactory.verified() + seat = factories.SeatFactory( + type=verified_type, + price=100, + credit_provider="fake_provider", + credit_hours=2, + ) + serialized = SeatSerializer(seat).data + assert "credit_provider" not in serialized or serialized["credit_provider"] is None + assert "credit_hours" not in serialized or serialized["credit_hours"] is None + class MinimalPersonSerializerTests(TestCase): def setUp(self): diff --git a/course_discovery/apps/course_metadata/migrations/0357_alter_historicalseat_credit_hours_and_more.py b/course_discovery/apps/course_metadata/migrations/0357_alter_historicalseat_credit_hours_and_more.py new file mode 100644 index 0000000000..1c09fa8355 --- /dev/null +++ b/course_discovery/apps/course_metadata/migrations/0357_alter_historicalseat_credit_hours_and_more.py @@ -0,0 +1,33 @@ +# Generated by Django 4.2.23 on 2025-10-11 15:20 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_metadata', '0356_add_course_editor_update_bulk_operation'), + ] + + operations = [ + migrations.AlterField( + model_name='historicalseat', + name='credit_hours', + field=models.IntegerField(blank=True, help_text='Number of credit hours awarded for this seat.', null=True), + ), + migrations.AlterField( + model_name='historicalseat', + name='credit_provider', + field=models.CharField(blank=True, help_text='The provider granting academic credit for this seat, if applicable.', max_length=255, null=True), + ), + migrations.AlterField( + model_name='seat', + name='credit_hours', + field=models.IntegerField(blank=True, help_text='Number of credit hours awarded for this seat.', null=True), + ), + migrations.AlterField( + model_name='seat', + name='credit_provider', + field=models.CharField(blank=True, help_text='The provider granting academic credit for this seat, if applicable.', max_length=255, null=True), + ), + ] diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index fff40f6026..2c60f01dca 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -2810,13 +2810,18 @@ 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} if seat_type.slug in prices: 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 + # Waffle switch to control dummy SKU generation logic for 2U purpose. if IS_COURSE_RUN_FOR_DUMMY_SKU_GENERATION.is_enabled(): generate_sku(None, self) # Generates a SKU for the provide by Seat @@ -2824,11 +2829,13 @@ def update_or_create_seat_helper(self, seat_type, prices, upgrade_deadline_overr 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. @@ -2843,7 +2850,9 @@ 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 @@ -3234,8 +3243,10 @@ class Seat(ManageHistoryMixin, DraftModelMixin, TimeStampedModel): currency = models.ForeignKey(Currency, models.CASCADE, default='USD') _upgrade_deadline = models.DateTimeField(null=True, blank=True, db_column='upgrade_deadline') upgrade_deadline_override = models.DateTimeField(null=True, blank=True) - credit_provider = models.CharField(max_length=255, null=True, blank=True) - credit_hours = models.IntegerField(null=True, blank=True) + credit_provider = models.CharField(max_length=255, null=True, blank=True, + help_text="The provider granting academic credit for this seat, if applicable.") + credit_hours = models.IntegerField(null=True, blank=True, + help_text="Number of credit hours awarded for this seat.") sku = models.CharField(max_length=128, null=True, blank=True) bulk_sku = models.CharField(max_length=128, null=True, blank=True) @@ -3280,6 +3291,30 @@ def upgrade_deadline(self): def upgrade_deadline(self, value): self._upgrade_deadline = value + def clean(self): + """ + Enforce unique credit seats and ensure credit fields are only allowed for credit seat types. + """ + 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." + ) + + if self.type and self.type.slug != Seat.CREDIT: + if self.credit_provider or self.credit_hours or self.upgrade_deadline: + raise ValidationError( + 'Credit fields (credit_provider, credit_hours, upgrade_deadline) ' + 'are only valid for credit seat type.' + ) + 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 7494df545c..81fff102fa 100644 --- a/course_discovery/apps/course_metadata/tests/test_models.py +++ b/course_discovery/apps/course_metadata/tests/test_models.py @@ -1743,6 +1743,90 @@ 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() + + @pytest.mark.django_db + def test_credit_seat_without_provider_allowed(self): + """Credit seats without credit_provider should save successfully.""" + credit_type = factories.SeatTypeFactory.credit() + course_run = factories.CourseRunFactory() + seat = factories.SeatFactory.create( + course_run=course_run, + type=credit_type, + credit_provider=None, + credit_hours=None, + ) + seat.full_clean() + seat.save() + assert seat.credit_provider is None + assert seat.credit_hours is None + + @pytest.mark.django_db + def test_update_or_create_seat_helper_creates_credit_seat_with_metadata(self): + """Verify CourseRun.update_or_create_seat_helper stores credit_provider and credit_hours.""" + credit_type = factories.SeatTypeFactory.credit() + course_run = factories.CourseRunFactory() + seat = course_run.update_or_create_seat_helper( + seat_type=credit_type, + prices={'credit': 300}, + upgrade_deadline_override=None, + credit_provider='asu', + credit_hours=4, + ) + assert seat.credit_provider == 'asu' + assert seat.credit_hours == 4 + assert seat.type.slug == Seat.CREDIT + + 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 # pylint: disable=protected-access + # Override takes precedence + seat.upgrade_deadline_override = seat._upgrade_deadline.replace(year=seat._upgrade_deadline.year + 1) # pylint: disable=protected-access + assert seat.upgrade_deadline == seat.upgrade_deadline_override + @patch('course_discovery.apps.course_metadata.models.IS_COURSE_RUN_FOR_DUMMY_SKU_GENERATION') @patch('course_discovery.apps.course_metadata.models.generate_sku') def test_generate_sku_called_when_waffle_enabled(self, mock_generate_sku, mock_switch):