diff --git a/contentcuration/contentcuration/frontend/accounts/vuex/__tests__/module.spec.js b/contentcuration/contentcuration/frontend/accounts/vuex/__tests__/module.spec.js index 38867a0536..7c27ef720e 100644 --- a/contentcuration/contentcuration/frontend/accounts/vuex/__tests__/module.spec.js +++ b/contentcuration/contentcuration/frontend/accounts/vuex/__tests__/module.spec.js @@ -41,7 +41,6 @@ describe('account store', () => { const passwordData = { new_password1: 'testing password', new_password2: 'testing password', - token: 'testing token', uidb64: 'testing uidb64', }; return store.dispatch('account/setPassword', passwordData).then(() => { diff --git a/contentcuration/contentcuration/frontend/accounts/vuex/index.js b/contentcuration/contentcuration/frontend/accounts/vuex/index.js index f64df2778a..8f9dae3c52 100644 --- a/contentcuration/contentcuration/frontend/accounts/vuex/index.js +++ b/contentcuration/contentcuration/frontend/accounts/vuex/index.js @@ -16,12 +16,14 @@ export default { sendPasswordResetLink(context, email) { return client.post(window.Urls.auth_password_reset(), { email }); }, - setPassword(context, { uidb64, token, new_password1, new_password2 }) { + setPassword(context, { uidb64, new_password1, new_password2 }) { const data = { new_password1, new_password2, }; - return client.post(window.Urls.auth_password_reset_confirm(uidb64, token), data, { + // Django's PasswordResetConfirmView.reset_url_token + // https://docs.djangoproject.com/en/3.2/topics/auth/default/#django.contrib.auth.views.PasswordResetConfirmView + return client.post(window.Urls.auth_password_reset_confirm(uidb64, 'set-password'), data, { headers: { 'Content-type': 'application/form-url-encode', }, diff --git a/contentcuration/contentcuration/frontend/administration/components/CommunityLibraryStatusButton.vue b/contentcuration/contentcuration/frontend/administration/components/CommunityLibraryStatusButton.vue index 46006c211b..ab95f69dcb 100644 --- a/contentcuration/contentcuration/frontend/administration/components/CommunityLibraryStatusButton.vue +++ b/contentcuration/contentcuration/frontend/administration/components/CommunityLibraryStatusButton.vue @@ -105,7 +105,6 @@ .community-library-status-button { @extend %md-standard-func; - width: 9em; padding: 4px; color: v-bind('labelColor'); background-color: v-bind('color'); diff --git a/contentcuration/contentcuration/frontend/administration/pages/Channels/ChannelTable.vue b/contentcuration/contentcuration/frontend/administration/pages/Channels/ChannelTable.vue index 2dfdec7fae..c5ae96e7d5 100644 --- a/contentcuration/contentcuration/frontend/administration/pages/Channels/ChannelTable.vue +++ b/contentcuration/contentcuration/frontend/administration/pages/Channels/ChannelTable.vue @@ -304,11 +304,16 @@ fetchQueryParams: keywordSearchFetchQueryParams, } = useKeywordSearch(); + // channelStatusFilter is derived from the current channelType's options, so an + // existing status that's no longer valid for the new type already reads back as + // unset - only default it to the first option in that case. watch( channelTypeFilter, () => { - const options = channelStatusOptions.value; - channelStatusFilter.value = options.length ? options[0].value : null; + if (!channelStatusFilter.value) { + const options = channelStatusOptions.value; + channelStatusFilter.value = options.length ? options[0].value : null; + } }, { immediate: true }, ); diff --git a/contentcuration/contentcuration/frontend/administration/pages/Channels/__tests__/channelTable.spec.js b/contentcuration/contentcuration/frontend/administration/pages/Channels/__tests__/channelTable.spec.js index 07c08be509..ec50bc8925 100644 --- a/contentcuration/contentcuration/frontend/administration/pages/Channels/__tests__/channelTable.spec.js +++ b/contentcuration/contentcuration/frontend/administration/pages/Channels/__tests__/channelTable.spec.js @@ -11,8 +11,8 @@ localVue.use(router); const channelList = ['test', 'channel', 'table']; -function makeWrapper(store) { - router.replace({ name: RouteNames.CHANNELS }); +function makeWrapper(store, query = {}) { + router.replace({ name: RouteNames.CHANNELS, query }); return mount(ChannelTable, { router, @@ -71,13 +71,32 @@ describe('channelTable', () => { expect(router.currentRoute.query.keywords).toBe('keyword test'); }); - it('changing channel type filter should reset channel status filter', async () => { + it('changing channel type filter should reset channel status filter when it is no longer valid', async () => { + wrapper.vm.channelTypeFilter = ChannelTypeFilter.COMMUNITY_LIBRARY; + wrapper.vm.channelStatusFilter = 'needsReview'; + await wrapper.vm.$nextTick(); + // Kolibri library channels have no "needs review" status + wrapper.vm.channelTypeFilter = ChannelTypeFilter.KOLIBRI_LIBRARY; + await wrapper.vm.$nextTick(); + expect(wrapper.vm.channelStatusFilter).toBe('live'); + }); + it('changing channel type filter should keep the channel status filter when it is still valid', async () => { wrapper.vm.channelTypeFilter = ChannelTypeFilter.COMMUNITY_LIBRARY; wrapper.vm.channelStatusFilter = 'published'; await wrapper.vm.$nextTick(); + // "published" is a valid status for unlisted channels too wrapper.vm.channelTypeFilter = ChannelTypeFilter.UNLISTED; await wrapper.vm.$nextTick(); - expect(wrapper.vm.channelStatusFilter).toBe('live'); + expect(wrapper.vm.channelStatusFilter).toBe('published'); + }); + it('should preserve a valid channel status filter already present in the URL on mount', () => { + // Simulates navigating back to this page with filters still in the URL + // (e.g. after opening a channel and hitting the browser back button). + const backNavWrapper = makeWrapper(store, { + channelType: ChannelTypeFilter.COMMUNITY_LIBRARY, + channelStatus: 'needsReview', + }); + expect(backNavWrapper.vm.channelStatusFilter).toBe('needsReview'); }); }); describe('selection', () => { diff --git a/contentcuration/contentcuration/frontend/channelList/views/Channel/CommunityLibraryList/index.vue b/contentcuration/contentcuration/frontend/channelList/views/Channel/CommunityLibraryList/index.vue index 384133d99d..c4ee534b2a 100644 --- a/contentcuration/contentcuration/frontend/channelList/views/Channel/CommunityLibraryList/index.vue +++ b/contentcuration/contentcuration/frontend/channelList/views/Channel/CommunityLibraryList/index.vue @@ -41,6 +41,7 @@ @close="isAboutCommunityLibraryOpen = false" />
+{% load i18n %} +{% get_current_language as LANGUAGE_CODE %} +{% get_current_language_bidi as LANGUAGE_BIDI %} + + + + + + +

{% blocktrans with name=name %}Hello {{ name }},{% endblocktrans %}

+ +

{{ channel.name }} ({{ channel_url }})

+ + {% if approved %} +

{% translate "Your submission has been approved and will be added to the Community Library soon." %}

+ {% else %} +

{% translate "Your submission needs changes. Please review the notes below and resubmit after all feedback has been addressed." %}

+ {% endif %} + + {% if feedback_notes %} +

{% translate "Notes from the reviewer" %}: {{ feedback_notes }}

+ {% endif %} + +

+ {% translate "Thanks for using Kolibri Studio!" %} +
+ {% translate "The Learning Equality Team" %} +

+ + diff --git a/contentcuration/contentcuration/tests/test_user.py b/contentcuration/contentcuration/tests/test_user.py index 585932aa9c..d7362a1a43 100644 --- a/contentcuration/contentcuration/tests/test_user.py +++ b/contentcuration/contentcuration/tests/test_user.py @@ -7,7 +7,9 @@ import json import sys import tempfile +import uuid +from django.core.exceptions import PermissionDenied from django.core.management import call_command from django.test import TransactionTestCase from django.urls import reverse_lazy @@ -269,3 +271,38 @@ def test_effective_disk_space_with_canceled_subscription(self): subscription_disk_space=50 * 1024 * 1024 * 1024, ) self.assertEqual(self.user.get_effective_disk_space(), 500 * 1024 * 1024) + + def test_available_space_includes_subscription(self): + UserSubscription.objects.create( + user=self.user, + stripe_subscription_status="active", + subscription_disk_space=50 * 1024 * 1024 * 1024, + ) + self.assertEqual( + self.user.get_available_space(), + float(500 * 1024 * 1024 + 50 * 1024 * 1024 * 1024), + ) + + def test_check_space_allows_upload_within_subscription(self): + UserSubscription.objects.create( + user=self.user, + stripe_subscription_status="active", + subscription_disk_space=50 * 1024 * 1024 * 1024, + ) + try: + self.user.check_space(1024 * 1024 * 1024, uuid.uuid4().hex) + except PermissionDenied: + self.fail("Subscription space was not counted towards the upload quota") + + def test_check_space_rejects_upload_beyond_subscription(self): + UserSubscription.objects.create( + user=self.user, + stripe_subscription_status="active", + subscription_disk_space=50 * 1024 * 1024 * 1024, + ) + with self.assertRaises(PermissionDenied): + self.user.check_space(51 * 1024 * 1024 * 1024, uuid.uuid4().hex) + + def test_check_space_rejects_upload_without_subscription(self): + with self.assertRaises(PermissionDenied): + self.user.check_space(1024 * 1024 * 1024, uuid.uuid4().hex) diff --git a/contentcuration/contentcuration/tests/views/test_users.py b/contentcuration/contentcuration/tests/views/test_users.py index 4c5f635204..dfb00b88b2 100644 --- a/contentcuration/contentcuration/tests/views/test_users.py +++ b/contentcuration/contentcuration/tests/views/test_users.py @@ -1,11 +1,15 @@ import json +from django.contrib.auth.tokens import default_token_generator from django.db import IntegrityError from django.http.response import HttpResponseBadRequest from django.http.response import HttpResponseForbidden from django.http.response import HttpResponseNotAllowed from django.http.response import HttpResponseRedirectBase +from django.urls import reverse from django.urls import reverse_lazy +from django.utils.encoding import force_bytes +from django.utils.http import urlsafe_base64_encode from mock import mock from mock import patch @@ -15,6 +19,7 @@ from contentcuration.tests.base import StudioAPITestCase from contentcuration.views.users import login from contentcuration.views.users import UserActivationView +from contentcuration.views.users import UserPasswordResetConfirmView class LoginTestCase(StudioAPITestCase): @@ -179,6 +184,83 @@ def test_post__handles_integrity_error_gracefully(self, mock_register): self.assertIn("email", error_data) +class UserPasswordResetConfirmViewTestCase(StudioAPITestCase): + def setUp(self): + super(UserPasswordResetConfirmViewTestCase, self).setUp() + self.user = testdata.user(email="tester@tester.com") + self.user.set_password("old_password") + self.user.save() + self.uidb64 = urlsafe_base64_encode(force_bytes(self.user.pk)) + self.token = default_token_generator.make_token(self.user) + + def _url(self, token): + return reverse( + "auth_password_reset_confirm", + kwargs=dict(uidb64=self.uidb64, token=token), + ) + + def _get(self, token): + response = self.client.get(self._url(token)) + if response.url == self._url(UserPasswordResetConfirmView.reset_url_token): + response = self.client.get(response.url) + return response + + def _post(self, password="new_password", confirm=None): + data = dict(new_password1=password, new_password2=confirm or password) + return self.client.post( + self._url(UserPasswordResetConfirmView.reset_url_token), + data, + format="json", + ) + + def _assert_password(self, password): + self.user.refresh_from_db() + self.assertTrue(self.user.check_password(password)) + + def test_get__valid_token(self): + response = self._get(self.token) + self.assertEqual( + response.url, "/accounts/#/reset-password?uidb64={}".format(self.uidb64) + ) + + def test_get__invalid_token(self): + response = self._get("invalid-token") + self.assertEqual(response.url, "/accounts/#/reset-expired") + + def test_post__valid_session(self): + self._get(self.token) + response = self._post() + self._assert_password("new_password") + self.assertEqual(response.url, "/accounts/#/password-reset-success") + + def test_post__no_session(self): + response = self._post() + self._assert_password("old_password") + self.assertIsInstance(response, HttpResponseForbidden) + + def test_post__invalid_token_in_url(self): + self.client.post( + self._url("invalid-token"), + dict(new_password1="new_password", new_password2="new_password"), + format="json", + ) + self._assert_password("old_password") + + def test_post__invalid_form(self): + self._get(self.token) + response = self._post("new_password", "other_password") + self._assert_password("old_password") + self.assertIsInstance(response, HttpResponseForbidden) + + def test_post__reused_token(self): + self._get(self.token) + self._post() + self._get(self.token) + response = self._post("second_password") + self._assert_password("new_password") + self.assertIsInstance(response, HttpResponseForbidden) + + class UserActivationViewTestCase(StudioAPITestCase): def setUp(self): super(UserActivationViewTestCase, self).setUp() diff --git a/contentcuration/contentcuration/tests/viewsets/test_channel.py b/contentcuration/contentcuration/tests/viewsets/test_channel.py index 2c72cf2dcc..8421848da3 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_channel.py +++ b/contentcuration/contentcuration/tests/viewsets/test_channel.py @@ -1478,6 +1478,21 @@ def test_get_version_detail_returns_all_fields(self): for field in expected_fields: self.assertIn(field, data, f"Field '{field}' should be in response") + def test_get_version_detail_dedupes_duplicated_licenses(self): + """Test that duplicated license ids stored on the ChannelVersion (e.g. from a + backfill bug) are deduped in the response.""" + self.channel_version.included_licenses = [1, 2, 2, 1] + self.channel_version.non_distributable_licenses_included = [1, 1] + self.channel_version.save() + + url = reverse("channel-version-detail", kwargs={"pk": self.channel.id}) + response = self.client.get(url) + + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertEqual(data["included_licenses"], [1, 2]) + self.assertEqual(data["non_distributable_licenses_included"], [1]) + def test_get_version_detail_excludes_special_permissions_included(self): """Test that special_permissions_included is not in the response.""" special_license = AuditedSpecialPermissionsLicense.objects.create( @@ -1587,6 +1602,26 @@ def test_get_specific_channel_version(self): self.assertEqual(data["channel"], self.channel.id) self.assertEqual(data["version"], channel_version.version) + def test_get_channel_version_dedupes_duplicated_licenses(self): + """Test that duplicated license ids stored on the ChannelVersion (e.g. from a + backfill bug) are deduped in both list and detail responses.""" + channel_version = ChannelVersion.objects.filter(channel=self.channel).first() + channel_version.included_licenses = [2, 1, 2, 1] + channel_version.save() + + detail_url = reverse("channelversion-detail", kwargs={"pk": channel_version.id}) + detail_response = self.client.get(detail_url) + self.assertEqual(detail_response.status_code, 200) + self.assertEqual(detail_response.json()["included_licenses"], [1, 2]) + + list_url = reverse("channelversion-list") + f"?channel={self.channel.id}" + list_response = self.client.get(list_url) + self.assertEqual(list_response.status_code, 200) + data = list_response.json() + results = data["results"] if "results" in data else data + version_data = next(r for r in results if r["id"] == channel_version.id) + self.assertEqual(version_data["included_licenses"], [1, 2]) + def test_get_channel_versions_ordering(self): """Test ordering of channel versions.""" url = ( @@ -1682,3 +1717,76 @@ def test_non_channel_viewer_cannot_access_channel_versions(self): response = self.client.get(url) results = response.json() self.assertEqual(len(results), 0) + + def make_community_library_submission(self, channel, version, status): + submission = CommunityLibrarySubmission.objects.create( + channel=channel, + channel_version=version, + author=self.user, + ) + submission.status = status + submission.save() + return submission + + def test_anonymous_user_cannot_access_non_community_channel_versions(self): + """Test that an anonymous user gets an empty list rather than a permission error.""" + self.client.force_authenticate(user=None) + url = reverse("channelversion-list") + f"?channel={self.channel.id}" + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json()), 0) + + def test_anonymous_user_can_access_live_community_library_version(self): + """Test that an anonymous user can read the version live in the Community Library.""" + self.make_community_library_submission( + self.channel, 2, community_library_submission.STATUS_LIVE + ) + self.client.force_authenticate(user=None) + url = reverse("channelversion-list") + f"?channel={self.channel.id}" + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + results = response.json() + self.assertEqual(len(results), 1) + self.assertEqual(results[0]["version"], 2) + + def test_anonymous_user_cannot_access_unpublished_community_library_version(self): + """Test that submissions that never went live stay hidden from anonymous users.""" + for status in ( + community_library_submission.STATUS_PENDING, + community_library_submission.STATUS_APPROVED, + community_library_submission.STATUS_REJECTED, + community_library_submission.STATUS_SUPERSEDED, + ): + with self.subTest(status=status): + submission = self.make_community_library_submission( + self.channel, 2, status + ) + self.client.force_authenticate(user=None) + url = reverse("channelversion-list") + f"?channel={self.channel.id}" + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json()), 0) + submission.delete() + + def test_non_channel_viewer_can_access_live_community_library_version(self): + """Test that a signed-in non-editor can read the live Community Library version.""" + self.make_community_library_submission( + self.channel, 2, community_library_submission.STATUS_LIVE + ) + other_user = testdata.user(email="otheruser@example.com") + self.client.force_authenticate(user=other_user) + url = reverse("channelversion-list") + f"?channel={self.channel.id}" + response = self.client.get(url) + results = response.json() + self.assertEqual(len(results), 1) + self.assertEqual(results[0]["version"], 2) + + def test_live_community_library_version_does_not_expose_other_versions(self): + """Test that going live exposes only that version, not the channel's other versions.""" + self.make_community_library_submission( + self.channel, 2, community_library_submission.STATUS_LIVE + ) + self.client.force_authenticate(user=None) + url = reverse("channelversion-list") + f"?channel={self.channel_2.id}" + response = self.client.get(url) + self.assertEqual(len(response.json()), 0) diff --git a/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py b/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py index 4cd51fb2a1..149cd3ade6 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py +++ b/contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py @@ -2,6 +2,7 @@ from unittest import mock import pytz +from django.core import mail from django.urls import reverse from contentcuration.constants import ( @@ -16,6 +17,7 @@ from contentcuration.tests import testdata from contentcuration.tests.base import StudioAPITestCase from contentcuration.tests.helpers import reverse_with_query +from contentcuration.utils.urls import canonical_url from contentcuration.viewsets.sync.constants import ADDED_TO_COMMUNITY_LIBRARY @@ -731,6 +733,58 @@ def test_resolve_submission__accept_correct(self, apply_task_mock): channel_id=self.submission.channel.id, ) + self.assertEqual(len(mail.outbox), 1) + sent_email = mail.outbox[0] + self.assertEqual(sent_email.to, [self.submission.author.email]) + self.assertIn("approved", sent_email.subject.lower()) + self.assertIn("approved", sent_email.body.lower()) + self.assertIn(self.submission.channel.name, sent_email.body) + self.assertIn( + canonical_url( + reverse("channel", kwargs={"channel_id": self.submission.channel.pk}) + ), + sent_email.body, + ) + + @mock.patch( + "contentcuration.viewsets.community_library_submission.apply_channel_changes_task" + ) + @mock.patch( + "contentcuration.models.CommunityLibrarySubmission.send_resolution_email", + side_effect=Exception("SMTP is down"), + ) + def test_resolve_submission__accept_correct_when_email_fails( + self, send_email_mock, apply_task_mock + ): + """A failure to notify the author shouldn't undo or fail the resolution.""" + self.client.force_authenticate(user=self.admin_user) + response = self.client.post( + reverse( + "admin-community-library-submission-resolve", + args=[self.submission.id], + ), + self.resolve_approve_metadata, + format="json", + ) + self.assertEqual(response.status_code, 200, response.content) + + resolved_submission = CommunityLibrarySubmission.objects.get( + id=self.submission.id + ) + self.assertEqual( + resolved_submission.status, + community_library_submission_constants.STATUS_APPROVED, + ) + Change.objects.get( + channel=self.submission.channel, + change_type=ADDED_TO_COMMUNITY_LIBRARY, + ) + apply_task_mock.fetch_or_enqueue.assert_called_once_with( + self.admin_user, + channel_id=self.submission.channel.id, + ) + self.assertEqual(len(mail.outbox), 0) + @mock.patch( "contentcuration.viewsets.community_library_submission.apply_channel_changes_task" ) @@ -770,6 +824,20 @@ def test_resolve_submission__reject_correct(self, apply_task_mock): ) apply_task_mock.fetch_or_enqueue.assert_not_called() + self.assertEqual(len(mail.outbox), 1) + sent_email = mail.outbox[0] + self.assertEqual(sent_email.to, [self.submission.author.email]) + self.assertIn("needs changes", sent_email.subject.lower()) + self.assertIn("needs changes", sent_email.body.lower()) + self.assertIn(self.submission.channel.name, sent_email.body) + self.assertIn( + canonical_url( + reverse("channel", kwargs={"channel_id": self.submission.channel.pk}) + ), + sent_email.body, + ) + self.assertIn(self.feedback_notes, sent_email.body) + def test_resolve_submission__reject_missing_resolution_reason(self): self.client.force_authenticate(user=self.admin_user) metadata = self.resolve_reject_metadata.copy() diff --git a/contentcuration/contentcuration/utils/i18n.py b/contentcuration/contentcuration/utils/i18n.py index dd60689ce6..cde1b00d72 100644 --- a/contentcuration/contentcuration/utils/i18n.py +++ b/contentcuration/contentcuration/utils/i18n.py @@ -41,6 +41,20 @@ def _get_language_info(): LANGUAGE_INFO = _get_language_info() +def closest_supported_locale(lang_code): + """ + Given a content language's primary code (e.g. "es", "fr"), return the + Studio UI locale in SUPPORTED_LANGUAGES that matches it, ignoring region, + or None if Studio has no UI translation for that language. + """ + if not lang_code: + return None + for supported in SUPPORTED_LANGUAGES: + if supported.split("-")[0] == lang_code: + return supported + return None + + def language_globals(): language_code = get_language() lang_dir = "rtl" if get_language_bidi() else "ltr" diff --git a/contentcuration/contentcuration/views/users.py b/contentcuration/contentcuration/views/users.py index 2ec0a5faaa..4455e4845f 100644 --- a/contentcuration/contentcuration/views/users.py +++ b/contentcuration/contentcuration/views/users.py @@ -19,9 +19,6 @@ from django.shortcuts import redirect from django.template.loader import render_to_string from django.urls import reverse_lazy -from django.utils.decorators import method_decorator -from django.views.decorators.cache import never_cache -from django.views.decorators.debug import sensitive_post_parameters from django_registration.backends.activation.views import ActivationView from django_registration.backends.activation.views import RegistrationView from rest_framework.authentication import BasicAuthentication @@ -391,37 +388,25 @@ def post(self, request): class UserPasswordResetConfirmView(PasswordResetConfirmView): http_method_names = ["get", "post"] + success_url = "/accounts/#/password-reset-success" - @method_decorator(sensitive_post_parameters()) - @method_decorator(never_cache) - def dispatch(self, request, *args, **kwargs): - response = super(UserPasswordResetConfirmView, self).dispatch( - request, *args, **kwargs - ) + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + if self.request.method == "POST": + kwargs["data"] = json.loads(self.request.body) + return kwargs - if request.method == "POST": - return self.post(request, *args, **kwargs) + def get(self, request, *args, **kwargs): + return redirect("/accounts/#/reset-password?uidb64={}".format(kwargs["uidb64"])) - # Token is valid, redirect to password reset page - if response.status_code == 302: - return redirect( - "/accounts/#/reset-password?uidb64={}&token={}".format( - kwargs["uidb64"], kwargs["token"] - ) - ) + def form_invalid(self, form): + return HttpResponseForbidden() + def render_to_response(self, context, **response_kwargs): + if self.request.method == "POST": + return HttpResponseForbidden() return redirect("/accounts/#/reset-expired") - def get_success_url(self): - return "/accounts/#/password-reset-success" - - def post(self, request, *args, **kwargs): - form = self.form_class(self.user, json.loads(request.body)) - - if form.is_valid(): - return self.form_valid(form) - return HttpResponseForbidden() - def request_activation_link(request): if request.method != "POST": diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index 3af5c0b9dd..48da68e73e 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -957,6 +957,17 @@ def get_version_detail(self, request, pk=None) -> Response: if not version_data: return Response({}) + # Older ChannelVersion rows may have been backfilled with duplicate + # license IDs - dedupe defensively. + if version_data.get("included_licenses") is not None: + version_data["included_licenses"] = sorted( + set(version_data["included_licenses"]) + ) + if version_data.get("non_distributable_licenses_included") is not None: + version_data["non_distributable_licenses_included"] = sorted( + set(version_data["non_distributable_licenses_included"]) + ) + return Response(version_data) @action( @@ -1071,12 +1082,18 @@ class Meta: class ChannelVersionViewSet(ReadOnlyValuesViewset): queryset = ChannelVersion.objects.all() - permission_classes = [IsAuthenticated] + permission_classes = [AllowAny] pagination_class = ChannelVersionListPagination filterset_class = ChannelVersionFilter ordering_fields = ["version"] ordering = "-version" + # Older ChannelVersion rows may have been backfilled with duplicate + # license IDs - dedupe defensively. + field_map = { + "included_licenses": lambda item: sorted(set(item["included_licenses"] or [])) + } + values = ( "id", "channel", diff --git a/contentcuration/contentcuration/viewsets/community_library_submission.py b/contentcuration/contentcuration/viewsets/community_library_submission.py index 33fc6f9a94..167798d973 100644 --- a/contentcuration/contentcuration/viewsets/community_library_submission.py +++ b/contentcuration/contentcuration/viewsets/community_library_submission.py @@ -1,3 +1,5 @@ +import logging + from django.db.models import OuterRef from django.db.models import Subquery from django_filters import BaseInFilter @@ -36,6 +38,8 @@ ) from contentcuration.viewsets.user import IsAdminUser +logger = logging.getLogger(__name__) + class ChoiceInFilter(BaseInFilter, ChoiceFilter): """ @@ -358,4 +362,13 @@ def resolve(self, request, pk=None): published_version.id ) + try: + submission.send_resolution_email() + except Exception: + # The resolution itself has already been committed; a failure to + # notify the author shouldn't turn that into a 500 response. + logger.exception( + "Failed to send resolution email for submission %s", submission.pk + ) + return Response(self.serialize_object())