diff --git a/organizations/__init__.py b/organizations/__init__.py index cf29514f..4597297e 100644 --- a/organizations/__init__.py +++ b/organizations/__init__.py @@ -1,4 +1,4 @@ """ edx-organizations app initialization module """ -__version__ = '6.7.1' # pragma: no cover +__version__ = '6.8.0' # pragma: no cover diff --git a/organizations/v0/tests/test_views.py b/organizations/v0/tests/test_views.py index 9fcaf961..a00f222b 100644 --- a/organizations/v0/tests/test_views.py +++ b/organizations/v0/tests/test_views.py @@ -76,7 +76,7 @@ def test_create_organization(self): self.assertEqual(len(orgs), 2) def test_update_organization(self): - """ Verify Organization can be updated via PUT endpoint. """ + """ Verify Organizations can be updated via PUT endpoint. """ org = OrganizationFactory() data = { 'name': 'changed-name', @@ -91,12 +91,51 @@ def test_update_organization(self): orgs = Organization.objects.all() self.assertEqual(len(orgs), 2) + def test_update_reactivates_inactive_organization(self): + """ Verify inactive Organization can be updated via PUT endpoint, reactivating them. """ + org = OrganizationFactory(active=False) + data = { + 'name': 'changed-name', + 'short_name': org.short_name, + } + url = reverse('v0:organization-detail', kwargs={'short_name': org.short_name}) + response = self.client.put(url, json.dumps(data), content_type='application/json') + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['active'], True) + self.assertEqual(response.data['name'], data['name']) + self.assertEqual(response.data['short_name'], org.short_name) + self.assertEqual(response.data['description'], org.description) + + def test_activeness_may_not_be_specified(self): + """ Verify that 'active' cannot be specified through the HTTP API. """ + org = OrganizationFactory(active=False) + data = { + 'name': 'changed-name', + 'short_name': org.short_name, + 'active': False, + } + url = reverse('v0:organization-detail', kwargs={'short_name': org.short_name}) + response = self.client.put(url, json.dumps(data), content_type='application/json') + self.assertEqual(response.status_code, 400) + def test_patch_endpoint(self): """ Verify PATCH endpoint returns 405 because we use PUT for create and update""" url = reverse('v0:organization-detail', kwargs={'short_name': 'dummy'}) response = self.client.patch(url, json={}) self.assertEqual(response.status_code, 405) + def test_post_endpoint(self): + """ Verify POST endpoint returns 405 because we use PUT for create and update""" + url = reverse('v0:organization-detail', kwargs={'short_name': 'dummy'}) + response = self.client.post(url, json={}) + self.assertEqual(response.status_code, 405) + + def test_delete_endpoint(self): + """ Verify DELETE endpoint returns 405 because we don't support it""" + url = reverse('v0:organization-detail', kwargs={'short_name': 'dummy'}) + response = self.client.delete(url) + self.assertEqual(response.status_code, 405) + def test_create_as_only_staff_user(self): self.user.is_staff = True self.user.is_superuser = False diff --git a/organizations/v0/views.py b/organizations/v0/views.py index 4e98f8d1..0cef6689 100644 --- a/organizations/v0/views.py +++ b/organizations/v0/views.py @@ -7,6 +7,7 @@ from rest_framework import status from rest_framework import viewsets from rest_framework.authentication import SessionAuthentication +from rest_framework.exceptions import ValidationError from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -18,17 +19,46 @@ class OrganizationsViewSet(mixins.UpdateModelMixin, viewsets.ReadOnlyModelViewSet): """ Organization view to: - - fetch list organization data or single organization using organization short name. - - create or update an organization via the PUT endpoint. + - list organization data (GET .../) + - retrieve single organization (GET .../) + - create or update an organization via the PUT endpoint (PUT .../) """ - queryset = Organization.objects.filter(active=True) + queryset = Organization.objects.all() serializer_class = OrganizationSerializer lookup_field = 'short_name' authentication_classes = (JwtAuthentication, SessionAuthentication) permission_classes = (IsAuthenticated, UserIsStaff) + def get_queryset(self): + """ + Get the queryset to use in the request. + + For listing and retieving organizations, we only want to include + active ones. + + For creating and updating organizations, we want to include all of + them, which allows API users to "create" (i.e., reactivate) + organizations that exist internally but are inactive. + """ + if self.request.method == "GET": + return self.queryset.filter(active=True) + return self.queryset + def update(self, request, *args, **kwargs): - """ We perform both Update and Create action via the PUT method. """ + """ + We perform both Update and Create action via the PUT method. + + The 'active' field may not be specified via the HTTP API, since it + is always assumed to be True. So: + (1) new organizations created through the API are always Active, and + (2) existing organizations updated through the API always end up Active, + regardless of whether or not they were previously active. + """ + if 'active' in self.request.data: + raise ValidationError( + "Value of 'active' may not be specified via Organizations HTTP API." + ) + self.request.data['active'] = True try: return super().update(request, *args, **kwargs) except Http404: @@ -38,5 +68,7 @@ def update(self, request, *args, **kwargs): return Response(serializer.data) def partial_update(self, request, *args, **kwargs): - """ We disable PATCH because all updates and creates should use the PUT action above. """ + """ + We disable PATCH because all updates and creates should use the PUT action above. + """ return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED)