feat: support update/partial_update in SubsidyAccessPolicyViewSet.

ENT-7149

enterprise_access/apps/api/filters/base.py

@@ -29,6 +29,6 @@ def get_filterset_class(self, view, queryset=None):
         """
         Returns None if this is a retrieve() operation.
         """
-        if view.action == 'retrieve':
+        if view.action in ('retrieve', 'update', 'partial_update'):
             return None
         return super().get_filterset_class(view, queryset)

enterprise_access/apps/api/serializers/__init__.py

@@ -13,7 +13,8 @@
     SubsidyAccessPolicyRedeemableResponseSerializer,
     SubsidyAccessPolicyRedeemRequestSerializer,
     SubsidyAccessPolicyRedemptionRequestSerializer,
-    SubsidyAccessPolicyResponseSerializer
+    SubsidyAccessPolicyResponseSerializer,
+    SubsidyAccessPolicyUpdateRequestSerializer
 )
 from .subsidy_requests import (
     CouponCodeRequestSerializer,

enterprise_access/apps/api/serializers/subsidy_access_policy.py

@@ -17,6 +17,22 @@
 logger = logging.getLogger(__name__)
 
 
+def validate_no_extra_custom_fields_for_policy_class(policy_class, fields_and_values):
+    """
+    Validates that no fields are included in the serializer
+    data pertaining to one type of SubsidyAccessPolicy that are
+    only valid for *another* type of SubsidyAccessPolicy.
+    e.g. ``per_learner_spend_limit`` is not a valid field for creating
+    or updating a ``PerLearnerEnrollmentSubsidyAccessPolicy`` record.
+    """
+    unused_custom_fields = set(policy_class.ALL_CUSTOM_FIELDS) - set(policy_class.REQUIRED_CUSTOM_FIELDS)
+    if unused_custom_fields.intersection(fields_and_values.keys()):
+        return (
+            f"Extraneous fields for {policy_class.__name__} policy type: "
+            f"{list(unused_custom_fields)}."
+        )
+
+
 class SubsidyAccessPolicyResponseSerializer(serializers.ModelSerializer):
     """
     A read-only Serializer for responding to requests for ``SubsidyAccessPolicy`` records.
@@ -138,12 +154,12 @@ def validate(self, attrs):
             custom_policy_field_errors.append(
                 f"Missing fields for {policy_type} policy type: {policy_class.REQUIRED_CUSTOM_FIELDS}."
             )
-        # Here's the "no more" part:
-        unused_custom_fields = set(policy_class.ALL_CUSTOM_FIELDS) - set(policy_class.REQUIRED_CUSTOM_FIELDS)
-        if unused_custom_fields.intersection(attrs.keys()):
-            custom_policy_field_errors.append(
-                f"Extraneous fields for {policy_type} policy type: {list(unused_custom_fields)}."
-            )
+
+        # Here's the "no more" part
+        extra_field_error_msg = validate_no_extra_custom_fields_for_policy_class(policy_class, attrs)
+        if extra_field_error_msg:
+            custom_policy_field_errors.append(extra_field_error_msg)
+
         if custom_policy_field_errors:
             raise serializers.ValidationError(custom_policy_field_errors)
 
@@ -239,6 +255,101 @@ class SubsidyAccessPolicyDeleteRequestSerializer(serializers.Serializer):
     )
 
 
+class SubsidyAccessPolicyUpdateRequestSerializer(serializers.ModelSerializer):
+    """
+    Request Serializer for PUT or PATCH requests to update a subsidy access policy.
+
+    For views: SubsidyAccessPolicyViewSet.update and SubsidyAccessPolicyViewSet.partial_update.
+    """
+    class Meta:
+        model = SubsidyAccessPolicy
+        fields = (
+            'description',
+            'active',
+            'catalog_uuid',
+            'subsidy_uuid',
+            'access_method',
+            'spend_limit',
+            'per_learner_spend_limit',
+            'per_learner_enrollment_limit',
+        )
+        extra_kwargs = {
+            'description': {
+                'required': False,
+                'allow_blank': False,
+                'min_length': None,
+                'max_length': 200,
+                'trim_whitespace': True,
+            },
+            'active': {
+                'allow_null': False,
+                'required': False,
+            },
+            'catalog_uuid': {
+                'allow_null': False,
+                'required': False,
+            },
+            'subsidy_uuid': {
+                'allow_null': False,
+                'required': False,
+            },
+            'access_method': {
+                'allow_null': False,
+                'required': False,
+            },
+            'spend_limit': {
+                'allow_null': True,
+                'required': False,
+            },
+            'per_learner_enrollment_limit': {
+                'allow_null': True,
+                'required': False,
+            },
+            'per_learner_spend_limit': {
+                'allow_null': True,
+                'required': False,
+            },
+        }
+
+    def validate(self, attrs):
+        """
+        Raises a ValidationError if any field not explicitly declared
+        as a field in this serializer defintion is provided as input.
+        """
+        unknown = sorted(set(self.initial_data) - set(self.fields))
+        if unknown:
+            raise serializers.ValidationError("Field(s) are not updatable: {}".format(", ".join(unknown)))
+        return attrs
+
+    def update(self, instance, validated_data):
+        """
+        Overwrites the update() method to check that no fields
+        that are valid in a type of SubsidyAccessPolicy that is *different*
+        from the type of ``instance`` are present in ``validated_data``.
+
+        We have to do this validation here so that we have access
+        to a ``SubsidyAccessPolicy`` instance.  It's not required
+        for the caller of the policy update view to provide a policy type,
+        so we can't infer the desired type from the request payload.
+        """
+        extra_field_error_msg = validate_no_extra_custom_fields_for_policy_class(
+            instance.__class__,
+            validated_data,
+        )
+        if extra_field_error_msg:
+            raise serializers.ValidationError(extra_field_error_msg)
+
+        return super().update(instance, validated_data)
+
+    def to_representation(self, instance):
+        """
+        Once a SubsidyAccessPolicy has been updated, we want to serialize
+        more fields from the instance than are required in this, the input serializer.
+        """
+        read_serializer = SubsidyAccessPolicyResponseSerializer(instance)
+        return read_serializer.data
+
+
 class SubsidyAccessPolicyRedeemableResponseSerializer(serializers.ModelSerializer):
     """
     Response serializer to represent redeemable policies.

enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py

@@ -178,7 +178,7 @@ def test_policy_crud_views_unauthorized_forbidden(self, role_context_dict, expec
     @ddt.unpack
     def test_policy_crud_write_views_unauthorized_forbidden(self, role_context_dict, expected_response_code):
         """
-        Tests that we get expected 40x responses for all of the policy readonly views.
+        Tests that we get expected 40x responses for all of the policy write views.
         """
         # Set the JWT-based auth that we'll use for every request
         if role_context_dict:
@@ -190,6 +190,13 @@ def test_policy_crud_write_views_unauthorized_forbidden(self, role_context_dict,
         response = self.client.delete(reverse('api:v1:subsidy-access-policies-detail', kwargs=request_kwargs))
         self.assertEqual(response.status_code, expected_response_code)
 
+        # Test the update and partial_update views.
+        response = self.client.put(reverse('api:v1:subsidy-access-policies-detail', kwargs=request_kwargs))
+        self.assertEqual(response.status_code, expected_response_code)
+
+        response = self.client.patch(reverse('api:v1:subsidy-access-policies-detail', kwargs=request_kwargs))
+        self.assertEqual(response.status_code, expected_response_code)
+
 
 @ddt.ddt
 class TestAuthenticatedPolicyCRUDViews(CRUDViewTestMixin, APITestWithMocks):
@@ -352,6 +359,151 @@ def test_destroy_view(self, request_payload, expected_change_reason):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(expected_response, response.json())
 
+    @ddt.data(True, False)
+    def test_update_views(self, is_patch):
+        """
+        Test that the update and partial_update views can modify certain
+        fields of a policy record.
+        """
+        # Set the JWT-based auth to an operator.
+        self.set_jwt_cookie([
+            {'system_wide_role': SYSTEM_ENTERPRISE_OPERATOR_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}
+        ])
+
+        policy_for_edit = PerLearnerSpendCapLearnerCreditAccessPolicyFactory(
+            enterprise_customer_uuid=self.enterprise_uuid,
+            spend_limit=5,
+            active=False,
+        )
+
+        request_payload = {
+            'description': 'the new description',
+            'active': True,
+            'catalog_uuid': str(uuid4()),
+            'subsidy_uuid': str(uuid4()),
+            'access_method': AccessMethods.ASSIGNED,
+            'spend_limit': None,
+            'per_learner_spend_limit': 10000,
+        }
+
+        action = self.client.patch if is_patch else self.client.put
+        url = reverse(
+            'api:v1:subsidy-access-policies-detail',
+            kwargs={'uuid': str(policy_for_edit.uuid)}
+        )
+        response = action(url, data=request_payload)
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        expected_response = {
+            'access_method': AccessMethods.ASSIGNED,
+            'active': True,
+            'catalog_uuid': request_payload['catalog_uuid'],
+            'description': request_payload['description'],
+            'enterprise_customer_uuid': str(self.enterprise_uuid),
+            'per_learner_enrollment_limit': None,
+            'per_learner_spend_limit': request_payload['per_learner_spend_limit'],
+            'policy_type': 'PerLearnerSpendCreditAccessPolicy',
+            'spend_limit': request_payload['spend_limit'],
+            'subsidy_uuid': request_payload['subsidy_uuid'],
+            'uuid': str(policy_for_edit.uuid),
+        }
+        self.assertEqual(expected_response, response.json())
+
+    @ddt.data(
+        {
+            'enterprise_customer_uuid': str(uuid4()),
+            'uuid': str(uuid4()),
+            'policy_type': 'PerLearnerEnrollmentCapCreditAccessPolicy',
+            'created': '1970-01-01 12:00:00Z',
+            'modified': '1970-01-01 12:00:00Z',
+            'nonsense_key': 'ship arriving too late to save a drowning witch',
+        },
+    )
+    def test_update_views_fields_disallowed_for_update(self, request_payload):
+        """
+        Test that the update and partial_update views can NOT modify fields
+        of a policy record that are not included in the update request serializer fields defintion.
+        """
+        # Set the JWT-based auth to an operator.
+        self.set_jwt_cookie([
+            {'system_wide_role': SYSTEM_ENTERPRISE_OPERATOR_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}
+        ])
+
+        policy_for_edit = PerLearnerSpendCapLearnerCreditAccessPolicyFactory(
+            enterprise_customer_uuid=self.enterprise_uuid,
+            spend_limit=5,
+            active=False,
+        )
+        url = reverse(
+            'api:v1:subsidy-access-policies-detail',
+            kwargs={'uuid': str(policy_for_edit.uuid)}
+        )
+
+        expected_unknown_keys = ", ".join(sorted(request_payload.keys()))
+
+        # Test the PUT view
+        response = self.client.put(url, data=request_payload)
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            response.json(),
+            {'non_field_errors': [f'Field(s) are not updatable: {expected_unknown_keys}']},
+        )
+
+        # Test the PATCH view
+        response = self.client.patch(url, data=request_payload)
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+        self.assertEqual(
+            response.json(),
+            {'non_field_errors': [f'Field(s) are not updatable: {expected_unknown_keys}']},
+        )
+
+    @ddt.data(
+        {
+            'policy_class': PerLearnerSpendCapLearnerCreditAccessPolicyFactory,
+            'request_payload': {
+                'per_learner_enrollment_limit': 10,
+            },
+        },
+        {
+            'policy_class': PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory,
+            'request_payload': {
+                'per_learner_spend_limit': 1000,
+            },
+        },
+    )
+    @ddt.unpack
+    def test_update_view_validates_fields_vs_policy_type(self, policy_class, request_payload):
+        """
+        Test that the update view can NOT modify fields
+        of a policy record that are relevant only to a different
+        type of policy.
+        """
+        # Set the JWT-based auth to an operator.
+        self.set_jwt_cookie([
+            {'system_wide_role': SYSTEM_ENTERPRISE_OPERATOR_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}
+        ])
+
+        policy_for_edit = policy_class(
+            enterprise_customer_uuid=self.enterprise_uuid,
+            spend_limit=5,
+            active=False,
+        )
+        url = reverse(
+            'api:v1:subsidy-access-policies-detail',
+            kwargs={'uuid': str(policy_for_edit.uuid)}
+        )
+
+        response = self.client.put(url, data=request_payload)
+
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        expected_error_message = (
+            f"Extraneous fields for {policy_for_edit.__class__.__name__} policy type: "
+            f"{list(request_payload)}."
+        )
+        self.assertEqual(response.json(), [expected_error_message])
+
 
 @ddt.ddt
 class TestAdminPolicyCreateView(CRUDViewTestMixin, APITestWithMocks):