From 9e7c0f7fff6f749bc04b8acb8635f848eaa5823d Mon Sep 17 00:00:00 2001 From: Jakub Komarek Date: Wed, 9 Jul 2025 23:13:11 +0200 Subject: [PATCH 1/3] CommunityLibrarySubmission viewset --- ...ysubmission_submission_date_created_idx.py | 19 + contentcuration/contentcuration/models.py | 5 + .../test_community_library_submission.py | 408 ++++++++++++++++++ contentcuration/contentcuration/urls.py | 8 + .../viewsets/community_library_submission.py | 145 +++++++ 5 files changed, 585 insertions(+) create mode 100644 contentcuration/contentcuration/migrations/0155_communitylibrarysubmission_submission_date_created_idx.py create mode 100644 contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py create mode 100644 contentcuration/contentcuration/viewsets/community_library_submission.py diff --git a/contentcuration/contentcuration/migrations/0155_communitylibrarysubmission_submission_date_created_idx.py b/contentcuration/contentcuration/migrations/0155_communitylibrarysubmission_submission_date_created_idx.py new file mode 100644 index 0000000000..65e7a8bb40 --- /dev/null +++ b/contentcuration/contentcuration/migrations/0155_communitylibrarysubmission_submission_date_created_idx.py @@ -0,0 +1,19 @@ +# Generated by Django 3.2.24 on 2025-07-08 20:09 +from django.db import migrations +from django.db import models + + +class Migration(migrations.Migration): + + dependencies = [ + ("contentcuration", "0154_community_library_submission"), + ] + + operations = [ + migrations.AddIndex( + model_name="communitylibrarysubmission", + index=models.Index( + fields=["-date_created"], name="submission_date_created_idx" + ), + ), + ] diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 375cd3a9a6..6d0e0e93a8 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -2616,6 +2616,10 @@ class Meta: name="unique_channel_with_channel_version", ), ] + indexes = [ + # Useful for cursor pagination + models.Index(fields=["-date_created"], name="submission_date_created_idx"), + ] ASSESSMENT_ID_INDEX_NAME = "assessment_id_idx" @@ -2655,6 +2659,7 @@ def has_changes(self): class Meta: indexes = [ + # Useful for cursor pagination models.Index(fields=["assessment_id"], name=ASSESSMENT_ID_INDEX_NAME), ] diff --git a/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py b/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py new file mode 100644 index 0000000000..8cb9e5e7c2 --- /dev/null +++ b/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py @@ -0,0 +1,408 @@ +from urllib.parse import urlencode + +from django.urls import reverse + +from contentcuration.models import CommunityLibrarySubmission +from contentcuration.tests import testdata +from contentcuration.tests.base import StudioAPITestCase + + +def reverse_with_query( + viewname, urlconf=None, args=None, kwargs=None, current_app=None, query=None +): + """ + This helper wraps the Django `reverse` function to support the `query` argument. + This argument is supported natively since Django 5.2, so when Django is updated + above this version, this helper can be removed. + """ + url = reverse( + viewname, urlconf=urlconf, args=args, kwargs=kwargs, current_app=current_app + ) + if query: + return f"{url}?{urlencode(query)}" + return url + + +class CRUDTestCase(StudioAPITestCase): + @property + def new_submission_metadata(self): + return { + "channel": self.channel_without_submission.id, + "countries": [self.country1.code], + } + + @property + def updated_submission_metadata(self): + return { + "countries": [ + "C2", + ], + "channel": self.channel_with_submission1.id, + } + + def setUp(self): + super().setUp() + self.author_user = testdata.user(email="author@user.com") + self.editor_user = testdata.user(email="editor@user.com") + self.forbidden_user = testdata.user(email="forbidden@user.com") + self.admin_user = testdata.user(email="admin@user.com") + self.admin_user.is_admin = True + self.admin_user.save() + + self.country1 = testdata.country(name="Country 1", code="C1") + self.country2 = testdata.country(name="Country 2", code="C2") + + self.channel_with_submission1 = testdata.channel() + self.channel_with_submission1.public = True + self.channel_with_submission1.version = 1 + self.channel_with_submission1.editors.add(self.author_user) + self.channel_with_submission1.editors.add(self.editor_user) + self.channel_with_submission1.save() + + self.channel_with_submission2 = testdata.channel() + self.channel_with_submission2.public = True + self.channel_with_submission2.version = 1 + self.channel_with_submission2.editors.add(self.author_user) + self.channel_with_submission2.editors.add(self.editor_user) + self.channel_with_submission2.save() + + self.channel_without_submission = testdata.channel() + self.channel_without_submission.public = True + self.channel_without_submission.version = 1 + self.channel_without_submission.editors.add(self.author_user) + self.channel_without_submission.editors.add(self.editor_user) + self.channel_without_submission.save() + + self.unpublished_channel = testdata.channel() + self.unpublished_channel.public = False + self.unpublished_channel.version = 0 + self.unpublished_channel.editors.add(self.author_user) + self.unpublished_channel.editors.add(self.editor_user) + self.unpublished_channel.save() + + self.existing_submission1 = testdata.community_library_submission() + self.existing_submission1.channel = self.channel_with_submission1 + self.existing_submission1.author = self.author_user + self.existing_submission1.save() + self.existing_submission1.countries.add(self.country1) + self.existing_submission1.save() + + self.existing_submission2 = testdata.community_library_submission() + self.existing_submission2.channel = self.channel_with_submission2 + self.existing_submission2.author = self.author_user + self.existing_submission2.save() + self.existing_submission2.countries.add(self.country1) + self.existing_submission2.save() + + def test_create_submission__is_editor(self): + self.client.force_authenticate(user=self.editor_user) + submission = self.new_submission_metadata + response = self.client.post( + reverse("community-library-submission-list"), + submission, + format="json", + ) + self.assertEqual(response.status_code, 201, response.content) + + def test_create_submission__is_forbidden(self): + self.client.force_authenticate(user=self.forbidden_user) + submission = self.new_submission_metadata + response = self.client.post( + reverse("community-library-submission-list"), + submission, + format="json", + ) + self.assertEqual(response.status_code, 400, response.content) + + def test_create_submission__unpublished_channel(self): + self.client.force_authenticate(user=self.editor_user) + submission = self.new_submission_metadata + submission["channel"] = self.unpublished_channel.id + + response = self.client.post( + reverse("community-library-submission-list"), + submission, + format="json", + ) + self.assertEqual(response.status_code, 400, response.content) + + def test_create_submission__explicit_channel_version(self): + self.client.force_authenticate(user=self.editor_user) + submission_metadata = self.new_submission_metadata + submission_metadata["channel_version"] = 2 + response = self.client.post( + reverse("community-library-submission-list"), + submission_metadata, + format="json", + ) + self.assertEqual(response.status_code, 201, response.content) + + created_submission_id = response.data["id"] + created_submission = CommunityLibrarySubmission.objects.get( + id=created_submission_id + ) + + # The explicitly set channel version should be ignored by the serializer + self.assertEqual(created_submission.channel_version, 1) + + def test_list_submissions__is_editor(self): + self.client.force_authenticate(user=self.editor_user) + response = self.client.get( + reverse( + "community-library-submission-list", + ) + ) + self.assertEqual(response.status_code, 200, response.content) + + results = response.data + self.assertEqual(len(results), 2) + + def test_list_submissions__is_admin(self): + self.client.force_authenticate(user=self.admin_user) + response = self.client.get( + reverse( + "community-library-submission-list", + ) + ) + self.assertEqual(response.status_code, 200, response.content) + + results = response.data + self.assertEqual(len(results), 2) + + def test_list_submissions__is_forbidden(self): + self.client.force_authenticate(user=self.forbidden_user) + response = self.client.get( + reverse( + "community-library-submission-list", + ) + ) + self.assertEqual(response.status_code, 200, response.content) + + results = response.data + self.assertEqual(len(results), 0) + + def test_list_submissions__filter_by_channel(self): + self.client.force_authenticate(user=self.editor_user) + response = self.client.get( + reverse_with_query( + "community-library-submission-list", + query={"channel": self.channel_with_submission1.id}, + ) + ) + self.assertEqual(response.status_code, 200, response.content) + + results = response.data + self.assertEqual(len(results), 1) + self.assertEqual(results[0]["channel_id"], self.channel_with_submission1.id) + + def test_list_submissions__pagination(self): + self.client.force_authenticate(user=self.author_user) + response = self.client.get( + reverse_with_query( + "community-library-submission-list", + query={"max_results": 1}, + ) + ) + self.assertEqual(response.status_code, 200, response.content) + + results = response.data["results"] + more = response.data["more"] + + self.assertEqual(len(results), 1) + self.assertIsNotNone(more) + + cursor = more["cursor"] + + response = self.client.get( + reverse_with_query( + "community-library-submission-list", + query={"max_results": 1, "cursor": cursor}, + ) + ) + self.assertEqual(response.status_code, 200, response.content) + + results = response.data["results"] + more = response.data["more"] + + self.assertEqual(len(results), 1) + self.assertIsNone(more) + + def test_get_single_submission__is_editor(self): + self.client.force_authenticate(user=self.editor_user) + response = self.client.get( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + format="json", + ) + self.assertEqual(response.status_code, 200, response.content) + + result = response.data + self.assertEqual(result["id"], self.existing_submission1.id) + + def test_get_single_submission__is_admin(self): + self.client.force_authenticate(user=self.admin_user) + response = self.client.get( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + format="json", + ) + self.assertEqual(response.status_code, 200, response.content) + + result = response.data + self.assertEqual(result["id"], self.existing_submission1.id) + + def test_get_single_submission__is_forbidden(self): + self.client.force_authenticate(user=self.forbidden_user) + response = self.client.get( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + format="json", + ) + self.assertEqual(response.status_code, 404, response.content) + + def test_get_single_submission__author_name(self): + self.client.force_authenticate(user=self.author_user) + response = self.client.get( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + format="json", + ) + self.assertEqual(response.status_code, 200, response.content) + + result = response.data + self.assertEqual(result["author_first_name"], self.author_user.first_name) + self.assertEqual(result["author_last_name"], self.author_user.last_name) + + def test_update_submission__is_author(self): + self.client.force_authenticate(user=self.author_user) + response = self.client.put( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + self.updated_submission_metadata, + format="json", + ) + self.assertEqual(response.status_code, 200, response.content) + + updated_submission = CommunityLibrarySubmission.objects.get( + id=self.existing_submission1.id + ) + self.assertEqual(updated_submission.countries.count(), 1) + self.assertEqual(updated_submission.countries.first().code, "C2") + + def test_update_submission__is_editor(self): + self.client.force_authenticate(user=self.editor_user) + response = self.client.put( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + self.updated_submission_metadata, + format="json", + ) + self.assertEqual(response.status_code, 404, response.content) + + def test_update_submission__is_admin(self): + self.client.force_authenticate(user=self.admin_user) + response = self.client.put( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + self.updated_submission_metadata, + format="json", + ) + self.assertEqual(response.status_code, 200, response.content) + + updated_submission = CommunityLibrarySubmission.objects.get( + id=self.existing_submission1.id + ) + self.assertEqual(updated_submission.countries.count(), 1) + self.assertEqual(updated_submission.countries.first().code, "C2") + + def test_update_submission__change_channel(self): + self.client.force_authenticate(user=self.admin_user) + submission_metadata = self.updated_submission_metadata + submission_metadata["channel"] = self.channel_without_submission.id + response = self.client.put( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + submission_metadata, + format="json", + ) + self.assertEqual(response.status_code, 400, response.content) + + def test_delete_submission__is_author(self): + self.client.force_authenticate(user=self.author_user) + response = self.client.delete( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + format="json", + ) + self.assertEqual(response.status_code, 204, response.content) + self.assertFalse( + CommunityLibrarySubmission.objects.filter( + id=self.existing_submission1.id + ).exists() + ) + + def test_delete_submission__is_editor(self): + self.client.force_authenticate(user=self.editor_user) + response = self.client.delete( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + format="json", + ) + self.assertEqual(response.status_code, 404, response.content) + self.assertTrue( + CommunityLibrarySubmission.objects.filter( + id=self.existing_submission1.id + ).exists() + ) + + def test_delete_submission__is_admin(self): + self.client.force_authenticate(user=self.admin_user) + response = self.client.delete( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + format="json", + ) + self.assertEqual(response.status_code, 204, response.content) + self.assertFalse( + CommunityLibrarySubmission.objects.filter( + id=self.existing_submission1.id + ).exists() + ) + + def test_delete_submission__is_forbidden(self): + self.client.force_authenticate(user=self.forbidden_user) + response = self.client.delete( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + format="json", + ) + self.assertEqual(response.status_code, 404, response.content) + self.assertTrue( + CommunityLibrarySubmission.objects.filter( + id=self.existing_submission1.id + ).exists() + ) diff --git a/contentcuration/contentcuration/urls.py b/contentcuration/contentcuration/urls.py index 94fe8511e3..b948effc8b 100644 --- a/contentcuration/contentcuration/urls.py +++ b/contentcuration/contentcuration/urls.py @@ -38,6 +38,9 @@ from contentcuration.viewsets.channel import ChannelViewSet from contentcuration.viewsets.channelset import ChannelSetViewSet from contentcuration.viewsets.clipboard import ClipboardViewSet +from contentcuration.viewsets.community_library_submission import ( + CommunityLibrarySubmissionViewSet, +) from contentcuration.viewsets.contentnode import ContentNodeViewSet from contentcuration.viewsets.feedback import FlagFeedbackEventViewSet from contentcuration.viewsets.feedback import RecommendationsEventViewSet @@ -80,6 +83,11 @@ def get_redirect_url(self, *args, **kwargs): RecommendationsInteractionEventViewSet, basename="recommendations-interaction", ) +router.register( + r"communitylibrary_submission", + CommunityLibrarySubmissionViewSet, + basename="community-library-submission", +) urlpatterns = [ re_path(r"^api/", include(router.urls)), diff --git a/contentcuration/contentcuration/viewsets/community_library_submission.py b/contentcuration/contentcuration/viewsets/community_library_submission.py new file mode 100644 index 0000000000..907460752c --- /dev/null +++ b/contentcuration/contentcuration/viewsets/community_library_submission.py @@ -0,0 +1,145 @@ +from django_filters.rest_framework import DjangoFilterBackend +from rest_framework.permissions import IsAuthenticated +from rest_framework.relations import PrimaryKeyRelatedField +from rest_framework.serializers import ValidationError + +from contentcuration.models import Channel +from contentcuration.models import CommunityLibrarySubmission +from contentcuration.models import Country +from contentcuration.utils.pagination import ValuesViewsetCursorPagination +from contentcuration.viewsets.base import BulkListSerializer +from contentcuration.viewsets.base import BulkModelSerializer +from contentcuration.viewsets.base import ReadOnlyValuesViewset +from contentcuration.viewsets.base import RESTCreateModelMixin +from contentcuration.viewsets.base import RESTDestroyModelMixin +from contentcuration.viewsets.base import RESTUpdateModelMixin +from contentcuration.viewsets.common import UserFilteredPrimaryKeyRelatedField + + +class CommunityLibrarySubmissionSerializer(BulkModelSerializer): + countries = PrimaryKeyRelatedField( + many=True, + queryset=Country.objects.all(), + required=False, + ) + channel = UserFilteredPrimaryKeyRelatedField( + queryset=Channel.objects.all(), + edit=False, + ) + + class Meta: + model = CommunityLibrarySubmission + fields = [ + "id", + "description", + "channel", + "countries", + "categories", + ] + list_serializer_class = BulkListSerializer + + def create(self, validated_data): + channel = validated_data["channel"] + user = self.context["request"].user + + if channel.version == 0: + # The channel is not published + raise ValidationError( + "Cannot create a community library submission for an " + "unpublished channel." + ) + + if not channel.editors.filter(id=user.id).exists(): + raise ValidationError( + "Only editors can create a community library " + "submission for this channel." + ) + + validated_data["channel_version"] = channel.version + validated_data["author"] = self.context["request"].user + + countries = validated_data.pop("countries", []) + instance = super().create(validated_data) + + for country in countries: + instance.countries.add(country) + + instance.save() + return instance + + def update(self, instance, validated_data): + if instance.channel.id != validated_data["channel"].id: + raise ValidationError( + "Cannot change the channel corresponding to " + "a community library submission." + ) + + countries = validated_data.pop("countries", []) + instance.countries.set(countries) + + return super().update(instance, validated_data) + + +class CommunityLibrarySubmissionPagination(ValuesViewsetCursorPagination): + ordering = "-date_created" + page_size_query_param = "max_results" + max_page_size = 100 + + +class CommunityLibrarySubmissionViewSet( + RESTCreateModelMixin, + RESTUpdateModelMixin, + RESTDestroyModelMixin, + ReadOnlyValuesViewset, +): + values = ( + "id", + "description", + "channel_id", + "channel_version", + "author_id", + "author__first_name", + "author__last_name", + "categories", + "date_created", + "status", + ) + field_map = { + "author_first_name": "author__first_name", + "author_last_name": "author__last_name", + } + filter_backends = [DjangoFilterBackend] + filterset_fields = ["channel"] + permission_classes = [IsAuthenticated] + pagination_class = CommunityLibrarySubmissionPagination + serializer_class = CommunityLibrarySubmissionSerializer + + def get_queryset(self): + base_queryset = CommunityLibrarySubmission.objects.all() + user = self.request.user + queryset = CommunityLibrarySubmission.filter_view_queryset( + base_queryset, user + ).order_by("-date_created") + + return queryset + + def get_edit_queryset(self): + base_queryset = CommunityLibrarySubmission.objects.all() + user = self.request.user + queryset = CommunityLibrarySubmission.filter_edit_queryset(base_queryset, user) + + return queryset + + def consolidate(self, items, queryset): + countries = {} + for (submission_id, country_code,) in Country.objects.filter( + community_library_submissions__in=queryset + ).values_list("community_library_submissions", "code"): + if submission_id not in countries: + countries[submission_id] = [] + countries[submission_id].append(country_code) + + for item in items: + item["countries"] = countries.get(item["id"], []) + + return items From 6ab6775c7ad8b8c152d0f1191d170570816d06d3 Mon Sep 17 00:00:00 2001 From: Jakub Komarek Date: Sat, 12 Jul 2025 09:37:30 +0200 Subject: [PATCH 2/3] Minor changes from PR feedback --- contentcuration/contentcuration/models.py | 1 - .../test_community_library_submission.py | 4 +--- .../viewsets/community_library_submission.py | 20 ++----------------- 3 files changed, 3 insertions(+), 22 deletions(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 6d0e0e93a8..d174261fa8 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -2659,7 +2659,6 @@ def has_changes(self): class Meta: indexes = [ - # Useful for cursor pagination models.Index(fields=["assessment_id"], name=ASSESSMENT_ID_INDEX_NAME), ] diff --git a/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py b/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py index 8cb9e5e7c2..85651dbc3e 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py +++ b/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py @@ -211,12 +211,10 @@ def test_list_submissions__pagination(self): self.assertEqual(len(results), 1) self.assertIsNotNone(more) - cursor = more["cursor"] - response = self.client.get( reverse_with_query( "community-library-submission-list", - query={"max_results": 1, "cursor": cursor}, + query=more, ) ) self.assertEqual(response.status_code, 200, response.content) diff --git a/contentcuration/contentcuration/viewsets/community_library_submission.py b/contentcuration/contentcuration/viewsets/community_library_submission.py index 907460752c..b7168b1e68 100644 --- a/contentcuration/contentcuration/viewsets/community_library_submission.py +++ b/contentcuration/contentcuration/viewsets/community_library_submission.py @@ -61,8 +61,7 @@ def create(self, validated_data): countries = validated_data.pop("countries", []) instance = super().create(validated_data) - for country in countries: - instance.countries.add(country) + instance.countries.set(countries) instance.save() return instance @@ -113,22 +112,7 @@ class CommunityLibrarySubmissionViewSet( permission_classes = [IsAuthenticated] pagination_class = CommunityLibrarySubmissionPagination serializer_class = CommunityLibrarySubmissionSerializer - - def get_queryset(self): - base_queryset = CommunityLibrarySubmission.objects.all() - user = self.request.user - queryset = CommunityLibrarySubmission.filter_view_queryset( - base_queryset, user - ).order_by("-date_created") - - return queryset - - def get_edit_queryset(self): - base_queryset = CommunityLibrarySubmission.objects.all() - user = self.request.user - queryset = CommunityLibrarySubmission.filter_edit_queryset(base_queryset, user) - - return queryset + queryset = CommunityLibrarySubmission.objects.all().order_by("-date_created") def consolidate(self, items, queryset): countries = {} From ba77607f4ef2942c655d3c40797d21c8dc5b521f Mon Sep 17 00:00:00 2001 From: Jakub Komarek Date: Sat, 12 Jul 2025 10:25:38 +0200 Subject: [PATCH 3/3] Fix partial updates when channel is missing --- .../test_community_library_submission.py | 18 ++++++++++++++++++ .../viewsets/community_library_submission.py | 5 ++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py b/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py index 85651dbc3e..d4de9e7d6b 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py +++ b/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py @@ -341,6 +341,24 @@ def test_update_submission__change_channel(self): ) self.assertEqual(response.status_code, 400, response.content) + def test_partial_update_submission__missing_channel(self): + self.client.force_authenticate(user=self.admin_user) + submission_metadata = self.updated_submission_metadata + del submission_metadata["channel"] + response = self.client.patch( + reverse( + "community-library-submission-detail", + args=[self.existing_submission1.id], + ), + submission_metadata, + format="json", + ) + self.assertEqual(response.status_code, 200, response.content) + updated_submission = CommunityLibrarySubmission.objects.get( + id=self.existing_submission1.id + ) + self.assertEqual(updated_submission.channel, self.channel_with_submission1) + def test_delete_submission__is_author(self): self.client.force_authenticate(user=self.author_user) response = self.client.delete( diff --git a/contentcuration/contentcuration/viewsets/community_library_submission.py b/contentcuration/contentcuration/viewsets/community_library_submission.py index b7168b1e68..2aef31bfba 100644 --- a/contentcuration/contentcuration/viewsets/community_library_submission.py +++ b/contentcuration/contentcuration/viewsets/community_library_submission.py @@ -67,7 +67,10 @@ def create(self, validated_data): return instance def update(self, instance, validated_data): - if instance.channel.id != validated_data["channel"].id: + if ( + "channel" in validated_data + and instance.channel.id != validated_data["channel"].id + ): raise ValidationError( "Cannot change the channel corresponding to " "a community library submission."