diff --git a/enterprise_catalog/apps/api/tests/test_tasks.py b/enterprise_catalog/apps/api/tests/test_tasks.py index b5486a66..5e2b9db0 100644 --- a/enterprise_catalog/apps/api/tests/test_tasks.py +++ b/enterprise_catalog/apps/api/tests/test_tasks.py @@ -40,7 +40,7 @@ RestrictedCourseMetadataFactory, RestrictedRunAllowedForRestrictedCourseFactory, ) -from enterprise_catalog.apps.catalog.utils import localized_utcnow +from enterprise_catalog.apps.catalog.utils import localized_utcnow, to_timestamp # An object that represents the output of some hard work done by a task. @@ -960,18 +960,20 @@ def test_get_algolia_objects_from_course_metadata(self): test_course.catalog_queries.set(catalog_queries[0:3]) algolia_objects = tasks.get_algolia_objects_from_course_content_metadata(test_course) + advertised_course_run = test_course.json_metadata['course_runs'][0] + enroll_by_timestamp = to_timestamp(test_course.json_metadata['normalized_metadata']['enroll_by_date']) expected_transformed_advertised_course_run = { 'key': 'course-v1:edX+DemoX+2T2024', 'pacing_type': None, 'availability': 'current', 'start': '2024-02-12T11:00:00Z', - 'end': '2026-02-05T11:00:00Z', + 'end': advertised_course_run['end'], 'min_effort': None, 'max_effort': None, 'weeks_to_complete': None, - 'upgrade_deadline': 1769471999.0, - 'enroll_by': 1769471999.0, + 'upgrade_deadline': enroll_by_timestamp, + 'enroll_by': enroll_by_timestamp, 'has_enroll_by': True, 'enroll_start': None, 'has_enroll_start': False, diff --git a/enterprise_catalog/apps/api/v1/serializers.py b/enterprise_catalog/apps/api/v1/serializers.py index 20ac47c3..000d8a2e 100644 --- a/enterprise_catalog/apps/api/v1/serializers.py +++ b/enterprise_catalog/apps/api/v1/serializers.py @@ -36,6 +36,7 @@ logger = logging.getLogger(__name__) +HIGHLIGHTED_CONTENT_ORDER = ('-is_favorite', 'sort_order', 'created') def find_and_modify_catalog_query( @@ -406,7 +407,9 @@ def get_highlighted_content(self, obj): """ Returns the data for the associated content included in this HighlightSet object. """ - qs = obj.highlighted_content.order_by('created').select_related('content_metadata') + qs = obj.highlighted_content.all() + if 'highlighted_content' not in getattr(obj, '_prefetched_objects_cache', {}): + qs = qs.order_by(*HIGHLIGHTED_CONTENT_ORDER).select_related('content_metadata') return HighlightedContentSerializer(qs, many=True).data diff --git a/enterprise_catalog/apps/api/v1/tests/test_curation_views.py b/enterprise_catalog/apps/api/v1/tests/test_curation_views.py index 396da27c..3af2afcd 100644 --- a/enterprise_catalog/apps/api/v1/tests/test_curation_views.py +++ b/enterprise_catalog/apps/api/v1/tests/test_curation_views.py @@ -971,6 +971,17 @@ def test_toggle_favorite_highlight(self): self.highlighted_content_list_one[0].refresh_from_db() assert self.highlighted_content_list_one[0].is_favorite is False + # Success case - boolean false + response = self.client.post( + edit_url, { + 'content_uuid': str(self.highlighted_content_list_one[0].uuid), + 'favorite': False + }, + ) + assert response.status_code == status.HTTP_201_CREATED + self.highlighted_content_list_one[0].refresh_from_db() + assert self.highlighted_content_list_one[0].is_favorite is False + # Failure case - no content uuid response = self.client.post( edit_url, { @@ -1018,3 +1029,61 @@ def test_toggle_favorite_highlight(self): ) assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json()['Error'] == 'Highlighted content not part of the given highlight set' + + def test_toggle_favorite_highlight_sets_sort_order(self): + """ + Test favoriting highlighted content updates sort order and returns favorites first. + """ + edit_url = reverse( + 'api:v1:highlight-sets-admin-toggle-favorite-highlight', + kwargs={'uuid': str(self.highlight_set_one.uuid)} + ) + detail_url = reverse( + 'api:v1:highlight-sets-admin-detail', + kwargs={'uuid': str(self.highlight_set_one.uuid)} + ) + self.set_up_staff() + + first_favorite = self.highlighted_content_list_one[2] + second_favorite = self.highlighted_content_list_one[4] + + response = self.client.post( + edit_url, + {'content_uuid': str(first_favorite.uuid), 'favorite': 'true'}, + ) + assert response.status_code == status.HTTP_201_CREATED + first_favorite.refresh_from_db() + assert first_favorite.is_favorite is True + assert first_favorite.sort_order == 0 + + response = self.client.post( + edit_url, + {'content_uuid': str(second_favorite.uuid), 'favorite': 'true'}, + ) + assert response.status_code == status.HTTP_201_CREATED + second_favorite.refresh_from_db() + assert second_favorite.is_favorite is True + assert second_favorite.sort_order == 1 + + response = self.client.get(detail_url) + assert response.status_code == status.HTTP_200_OK + highlighted_content = response.json()['highlighted_content'] + assert highlighted_content[0]['uuid'] == str(first_favorite.uuid) + assert highlighted_content[1]['uuid'] == str(second_favorite.uuid) + + response = self.client.post( + edit_url, + {'content_uuid': str(first_favorite.uuid), 'favorite': 'false'}, + ) + assert response.status_code == status.HTTP_201_CREATED + first_favorite.refresh_from_db() + second_favorite.refresh_from_db() + assert first_favorite.is_favorite is False + assert first_favorite.sort_order == 0 + assert second_favorite.is_favorite is True + assert second_favorite.sort_order == 0 + + response = self.client.get(detail_url) + assert response.status_code == status.HTTP_200_OK + highlighted_content = response.json()['highlighted_content'] + assert highlighted_content[0]['uuid'] == str(second_favorite.uuid) diff --git a/enterprise_catalog/apps/api/v1/views/curation/highlights.py b/enterprise_catalog/apps/api/v1/views/curation/highlights.py index f1911e52..c8b17b53 100644 --- a/enterprise_catalog/apps/api/v1/views/curation/highlights.py +++ b/enterprise_catalog/apps/api/v1/views/curation/highlights.py @@ -1,6 +1,9 @@ import logging from uuid import UUID +from django.db import transaction +from django.db.models import F, Max, Prefetch +from django.utils import timezone from django.utils.decorators import method_decorator from django.views.decorators.cache import cache_page from django.views.decorators.vary import vary_on_cookie @@ -50,6 +53,7 @@ REQUEST_CACHE_NAMESPACE = 'CURATION_REQUEST_CACHE' CONTENT_PER_HIGHLIGHTSET_LIMIT = 24 HIGHLIGHTSETS_PER_ENTERPRISE_LIMIT = 16 +HIGHLIGHTED_CONTENT_ORDER = ('-is_favorite', 'sort_order', 'created') logger = logging.getLogger(__name__) @@ -273,9 +277,12 @@ def base_queryset(self): kwargs.update({'enterprise_curation__enterprise_uuid': self.requested_enterprise_uuid}) if self.requested_highlight_set_uuid: kwargs.update({'uuid': self.requested_highlight_set_uuid}) + highlighted_content_queryset = HighlightedContent.objects.select_related('content_metadata').order_by( + *HIGHLIGHTED_CONTENT_ORDER + ) return HighlightSet.objects.filter(**kwargs).prefetch_related( 'enterprise_curation', - 'highlighted_content', + Prefetch('highlighted_content', queryset=highlighted_content_queryset), ).order_by('-created') @@ -569,29 +576,67 @@ def toggle_favorite_highlight(self, request, uuid, *args, **kwargs): `Error` key. 201: If highlighted content favorite state was successfully updated. """ - highlight_set = HighlightSet.objects.get(uuid=uuid) content_uuid = request.data.get('content_uuid') favorite_param = request.data.get('favorite') - if not favorite_param: + + if not content_uuid: + return Response({'Error': 'Missing content_uuid parameter'}, status=status.HTTP_400_BAD_REQUEST) + if favorite_param is None: return Response({'Error': 'Missing favorite parameter'}, status=status.HTTP_400_BAD_REQUEST) + try: favorite_toggle = str_to_bool(favorite_param) - if not content_uuid: - return Response({'Error': 'Missing content_uuid parameter'}, status=status.HTTP_400_BAD_REQUEST) - highlighted_content = HighlightedContent.objects.get(uuid=content_uuid) - if highlighted_content.catalog_highlight_set.uuid != highlight_set.uuid: - return Response({'Error': 'Highlighted content not part of the given highlight set'}, - status=status.HTTP_400_BAD_REQUEST) - highlighted_content.is_favorite = favorite_toggle - highlighted_content.save() - return Response({}, status=status.HTTP_201_CREATED) except TypeError: return Response({'Error': f'favorite parameter "{favorite_param}" is not a valid true/false value'}, status=status.HTTP_400_BAD_REQUEST) - except HighlightedContent.DoesNotExist: + + try: + content_uuid = UUID(str(content_uuid)) + highlight_set_uuid = UUID(str(uuid)) + except (AttributeError, TypeError, ValueError): return Response({'Error': 'content_uuid does not refer to any existing highlighted content'}, status=status.HTTP_400_BAD_REQUEST) + with transaction.atomic(): + try: + highlighted_content = ( + HighlightedContent.objects + .select_for_update() + .only('uuid', 'catalog_highlight_set_id', 'is_favorite', 'sort_order') + .get(uuid=content_uuid) + ) + except HighlightedContent.DoesNotExist: + return Response({'Error': 'content_uuid does not refer to any existing highlighted content'}, + status=status.HTTP_400_BAD_REQUEST) + + if highlighted_content.catalog_highlight_set_id != highlight_set_uuid: + return Response({'Error': 'Highlighted content not part of the given highlight set'}, + status=status.HTTP_400_BAD_REQUEST) + + highlight_set_content = HighlightedContent.objects.filter(catalog_highlight_set_id=highlight_set_uuid) + if highlighted_content.is_favorite != favorite_toggle: + # Highlight sets are bounded to 24 items, so locking the set keeps sort_order updates consistent + # without adding meaningful overhead. + list(highlight_set_content.select_for_update().values_list('uuid', flat=True)) + update_fields = ['is_favorite', 'sort_order', 'modified'] + if favorite_toggle: + max_sort_order = highlight_set_content.filter(is_favorite=True).aggregate( + Max('sort_order') + )['sort_order__max'] + highlighted_content.sort_order = 0 if max_sort_order is None else max_sort_order + 1 + else: + removed_sort_order = highlighted_content.sort_order + highlighted_content.sort_order = 0 + highlight_set_content.filter( + is_favorite=True, + sort_order__gt=removed_sort_order, + ).update(sort_order=F('sort_order') - 1, modified=timezone.now()) + + highlighted_content.is_favorite = favorite_toggle + highlighted_content.save(update_fields=update_fields) + + return Response({}, status=status.HTTP_201_CREATED) + @action(detail=True, methods=['post'], url_path='add-content') def add_content(self, request, uuid, *args, **kwargs): """ diff --git a/enterprise_catalog/apps/catalog/models.py b/enterprise_catalog/apps/catalog/models.py index b3f4a51a..3182444a 100644 --- a/enterprise_catalog/apps/catalog/models.py +++ b/enterprise_catalog/apps/catalog/models.py @@ -804,7 +804,7 @@ def json_metadata(self): # runs were actually found for this specific course and the requester's # specific Catalog. if restricted_course_metadata_for_catalog_query: - # pylint: disable=protected-access, unsubscriptable-object + # pylint: disable=protected-access return restricted_course_metadata_for_catalog_query[0]._json_metadata return self._json_metadata @@ -857,7 +857,6 @@ class Meta: restricted_run_allowed_for_restricted_course = models.ManyToManyField( ContentMetadata, through='RestrictedRunAllowedForRestrictedCourse', - through_fields=('course', 'run'), ) history = HistoricalRecords() diff --git a/enterprise_catalog/apps/catalog/tests/factories.py b/enterprise_catalog/apps/catalog/tests/factories.py index 93ef6394..0f2c0465 100644 --- a/enterprise_catalog/apps/catalog/tests/factories.py +++ b/enterprise_catalog/apps/catalog/tests/factories.py @@ -29,6 +29,8 @@ FAKE_CONTENT_AUTHOR_NAME = 'Partner Name' FAKE_CONTENT_AUTHOR_UUID = uuid4() FAKE_CONTENT_TITLE_PREFIX = 'Fake Content Title' +FUTURE_ENROLL_BY_DATE = (datetime.datetime.utcnow() + datetime.timedelta(days=365)).strftime('%Y-%m-%dT23:59:59Z') +FUTURE_COURSE_END_DATE = (datetime.datetime.utcnow() + datetime.timedelta(days=375)).strftime('%Y-%m-%dT11:00:00Z') fake = Faker() @@ -89,7 +91,7 @@ def _json_metadata(self): 'uuid': str(self.content_uuid), 'title': self.title, 'normalized_metadata': { - 'enroll_by_date': '2026-01-26T23:59:59Z', + 'enroll_by_date': FUTURE_ENROLL_BY_DATE, }, } if self.content_type == COURSE: @@ -122,7 +124,7 @@ def _json_metadata(self): 'type': 'verified', 'price': '50.00', 'currency': 'USD', - 'upgrade_deadline': '2026-01-26T23:59:59Z', + 'upgrade_deadline': FUTURE_ENROLL_BY_DATE, 'upgrade_deadline_override': None, 'credit_provider': None, 'credit_hours': None, @@ -131,7 +133,7 @@ def _json_metadata(self): } ], 'start': '2024-02-12T11:00:00Z', - 'end': '2026-02-05T11:00:00Z', + 'end': FUTURE_COURSE_END_DATE, 'fixed_price_price_usd': None, 'first_enrollable_paid_seat_price': 50, }]