[BD-14] Allow inactive orgs to be updated through HTTP API (#165)

Since LMS/Studio organizations have been completely backfilled,
there are now hundreds of Inactive organizations in the
edxapp database. Inactive organizations are not surfaced to
users, and hence are effectively non-existent.

If an organization is created or edited in an external
data source (such as Course Discovery), that source
should be able to push the update to the Organizations
API exposed by edx-organizations. Currently, however,
the Organizations API will return 400 if one tries to
create/update an organization that exists but is
inactive. This could potentially lead to Course
Discovery <-> LMS/Studio data synchronization issues
down the road.

This commit fixes that by making three changes:
 1. Any organization may be updated via the PUT method,
    regardless of whether or not it is active.
 2. Upon being updated, the organization is marked as
    active, whether it was previously active, inactive,
    or non-existent.
 3. The 'active' field may not be set or updated via
    the HTTP API, because that would be in conflict
    with change #2.

Also, clean up HTTP API docstrings,
and test that DELETE and POST method
are unsupported.

Bump version to 6.8.0.

TNL-7913

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

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

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 .../<short_name>)
+        - create or update an organization via the PUT endpoint (PUT .../<short_name>)
     """
-    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)