[COST-4182] Return status code 412 when enabled tags limit is exceeded (

#4675)

* Return status code 412 when enabled tags limit is exceeded
  Include limit and number of enabled tags in the response body to allow for
  localization of the message.

* Raise exception if validation fails
  This makes the code read better since there is less indentation.

* Only check limit in the enable view
  It would be cleaner to use a custom APIException class but the response created
  by the custom exception formatter doesn’t make sense for the exepected response
  from this API.

* Add tests for the enabled tags limit
* Update API documentation

docs/specs/openapi.json

@@ -5668,6 +5668,16 @@
                     "401": {
                         "description": "Unauthorized"
                     },
+                    "412": {
+                        "description": "Request would exceed enabled tags limit",
+                        "content": {
+                            "application/json": {
+                                "schema": {
+                                    "$ref": "#/components/schemas/SettingsTagsEnabledLimitError"
+                                }
+                            }
+                        }
+                    },
                     "500": {
                         "description": "Unexpected Error",
                         "content": {
@@ -9980,6 +9990,22 @@
                     }
                 }
             },
+            "SettingsTagsEnabledLimitError": {
+                "properties": {
+                    "error": {
+                        "type": "string",
+                        "default": "The maximum number of enabled tags is 200."
+                    },
+                    "enabled": {
+                        "type": "integer",
+                        "example": 187
+                    },
+                    "limit": {
+                        "type": "integer",
+                        "example": 200
+                    }
+                }
+            },
             "GetAwsCategorySettings": {
                 "allOf": [{
                     "$ref": "#/components/schemas/ListPagination"

koku/api/settings/tags/view.py

@@ -1,5 +1,7 @@
 import logging
+import typing as t
 
+from django.db.models.query import QuerySet
 from django.utils.decorators import method_decorator
 from django.views.decorators.cache import never_cache
 from django_filters import ModelMultipleChoiceFilter
@@ -63,36 +65,43 @@ class SettingsTagUpdateView(APIView):
     def put(self, request: Request, **kwargs) -> Response:
         uuid_list = request.data.get("ids", [])
         serializer = SettingsTagIDSerializer(data={"id_list": uuid_list})
-        if serializer.is_valid():
-            if Config.ENABLED_TAG_LIMIT > 0:
-                if self.enabled_tags_count >= Config.ENABLED_TAG_LIMIT:
-                    return Response(
-                        {
-                            "error": (
-                                f"Maximum number of enabled tags exceeded. There are {self.enabled_tags_count} "
-                                f"tags enabled and the limit is {Config.ENABLED_TAG_LIMIT}."
-                            )
-                        },
-                        status=status.HTTP_400_BAD_REQUEST,
-                    )
+        serializer.is_valid(raise_exception=True)
 
-            data = EnabledTagKeys.objects.filter(uuid__in=uuid_list)
-            data.update(enabled=self.enabled)
-            EnabledTagKeys.objects.bulk_update(data, ["enabled"])
+        objects = EnabledTagKeys.objects.filter(uuid__in=uuid_list)
+        if response := self._check_limit(objects):
+            return response
 
-            return Response(status=status.HTTP_204_NO_CONTENT)
+        objects.update(enabled=self.enabled)
+        EnabledTagKeys.objects.bulk_update(objects, ["enabled"])
 
-        return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+        return Response(status=status.HTTP_204_NO_CONTENT)
 
 
 class SettingsEnableTagView(SettingsTagUpdateView):
     enabled = True
 
+    def _check_limit(self, qs: QuerySet) -> t.Optional[Response]:
+        if Config.ENABLED_TAG_LIMIT > 0:
+            # Only count UUIDs requested to be enabled that are currently disabled.
+            records_to_update_count = qs.filter(enabled=False).count()
+            future_enabled_tags_count = self.enabled_tags_count + records_to_update_count
+            if future_enabled_tags_count > Config.ENABLED_TAG_LIMIT:
+                return Response(
+                    {
+                        "error": f"The maximum number of enabled tags is {Config.ENABLED_TAG_LIMIT}.",
+                        "enabled": self.enabled_tags_count,
+                        "limit": Config.ENABLED_TAG_LIMIT,
+                    },
+                    status=status.HTTP_412_PRECONDITION_FAILED,
+                )
+
     @property
     def enabled_tags_count(self):
         return EnabledTagKeys.objects.filter(enabled=True).count()
 
 
 class SettingsDisableTagView(SettingsTagUpdateView):
     enabled = False
-    enabled_tags_count = -1
+
+    def _check_limit(self, qs):
+        pass

koku/api/settings/test/tags/test_api.py

@@ -211,7 +211,7 @@ def test_enable_tags_empty_id_list(self):
             client = rest_framework.test.APIClient()
             enable_response = client.put(tags_enable_url, {"ids": []}, format="json", **self.headers)
 
-        error_details = enable_response.data.get("id_list", {})[0].lower()
+        error_details = enable_response.data["errors"][0]["detail"].lower()
 
         self.assertEqual(enable_response.status_code, status.HTTP_400_BAD_REQUEST, enable_response.data)
         self.assertIn("this list may not be empty", error_details)
@@ -225,7 +225,7 @@ def test_enable_tags_bad_uuid(self):
             client = rest_framework.test.APIClient()
             enable_response = client.put(tags_enable_url, {"ids": ["bad-uuid"]}, format="json", **self.headers)
 
-        error_details = enable_response.data.get("id_list", {}).get(0, [""])[0].lower()
+        error_details = enable_response.data["errors"][0]["detail"].lower()
 
         self.assertEqual(enable_response.status_code, status.HTTP_400_BAD_REQUEST, enable_response.data)
         self.assertIn("invalid uuid supplied", error_details)
@@ -267,9 +267,92 @@ def test_enable_tags_over_limit(self, mock_enabled_limit):
             client = rest_framework.test.APIClient()
             enable_response = client.put(tags_enable_url, {"ids": uuids}, format="json", **self.headers)
 
-        error = enable_response.data.get("error", "").lower()
-        self.assertEqual(enable_response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertIn("maximum number of enabled tags exceeded", error)
+        try:
+            error = enable_response.data.get("error", "").lower()
+        except AttributeError:
+            error = {"data": {}}
+
+        expected_keys = {"error", "enabled", "limit"}
+
+        self.assertEqual(enable_response.status_code, status.HTTP_412_PRECONDITION_FAILED)
+        self.assertIn("maximum number of enabled tags", error)
+        self.assertEqual(expected_keys, enable_response.data.keys())
+
+    def test_enable_tags_would_exceed_limit(self):
+        """With fewer tags enabled than the limit, try to enable more than the limit"""
+
+        tags_enable_url = reverse("tags-enable")
+        uuids = [obj.uuid for obj in self.disabled_objs]
+
+        # Set the limit slightly more than the current number of enabled tags
+        with patch("api.settings.tags.view.Config", ENABLED_TAG_LIMIT=len(self.enabled_objs) + 4):
+            with schema_context(self.schema_name):
+                client = rest_framework.test.APIClient()
+                enable_response = client.put(tags_enable_url, {"ids": uuids}, format="json", **self.headers)
+
+        try:
+            error = enable_response.data.get("error", "").lower()
+        except AttributeError:
+            error = {"data": {}}
+
+        expected_keys = {"error", "enabled", "limit"}
+
+        self.assertEqual(enable_response.status_code, status.HTTP_412_PRECONDITION_FAILED)
+        self.assertIn("maximum number of enabled tags", error)
+        self.assertEqual(expected_keys, enable_response.data.keys())
+
+    def test_enable_tags_mixed_tags_would_not_exceed_limit(self):
+        """With fewer tags enabled than the limit, try to enable some tags that
+        are already enabled and some tags that are currently disabled where the
+        total will not exceed the limit.
+
+        This tests that only tags that would be enabled, not currently enabled tags,
+        count towards the future total enabled tags.
+        """
+
+        tags_enable_url = reverse("tags-enable")
+        tags_url = reverse("settings-tags")
+        count_to_limit = 4
+        new_limit = len(self.enabled_objs) + count_to_limit
+        uuids = [obj.uuid for obj in self.disabled_objs[:count_to_limit]]
+
+        # Set the limit slightly more than the current number of enabled tags
+        with patch("api.settings.tags.view.Config", ENABLED_TAG_LIMIT=new_limit):
+            with schema_context(self.schema_name):
+                client = rest_framework.test.APIClient()
+                enable_response = client.put(tags_enable_url, {"ids": uuids}, format="json", **self.headers)
+                get_response = client.get(tags_url, {"enabled": True}, **self.headers)
+
+        self.assertEqual(enable_response.status_code, status.HTTP_204_NO_CONTENT)
+        self.assertEqual(get_response.data["meta"]["count"], new_limit)
+
+    def test_enable_tags_mixed_tags_would_exceed_limit(self):
+        """With fewer tags enabled than the limit, try to enable some tags that
+        are already enabled and some tags that are currently disabled where the
+        total will exceed the limit.
+        """
+
+        tags_enable_url = reverse("tags-enable")
+        count_to_limit = 4
+        new_limit = len(self.enabled_objs) + count_to_limit
+        uuids = [obj.uuid for obj in self.disabled_objs[: count_to_limit + 1]]
+
+        # Set the limit slightly more than the current number of enabled tags
+        with patch("api.settings.tags.view.Config", ENABLED_TAG_LIMIT=new_limit):
+            with schema_context(self.schema_name):
+                client = rest_framework.test.APIClient()
+                enable_response = client.put(tags_enable_url, {"ids": uuids}, format="json", **self.headers)
+
+        try:
+            error = enable_response.data.get("error", "").lower()
+        except AttributeError:
+            error = {"data": {}}
+
+        expected_keys = {"error", "enabled", "limit"}
+
+        self.assertEqual(enable_response.status_code, status.HTTP_412_PRECONDITION_FAILED)
+        self.assertIn("maximum number of enabled tags", error)
+        self.assertEqual(expected_keys, enable_response.data.keys())
 
     @patch("api.settings.tags.view.Config", ENABLED_TAG_LIMIT=-1)
     def test_enable_tags_limit_disabled(self, mock_enabled_limit):
@@ -284,5 +367,5 @@ def test_enable_tags_limit_disabled(self, mock_enabled_limit):
             enable_response = client.put(tags_enable_url, {"ids": uuids}, format="json", **self.headers)
             get_response = client.get(tags_url, {"filter[enabled]": True, "limit": 100}, **self.headers)
 
-        self.assertEqual(enable_response.status_code, status.HTTP_204_NO_CONTENT, enable_response.data)
+        self.assertEqual(enable_response.status_code, status.HTTP_204_NO_CONTENT)
         self.assertEqual(get_response.data["meta"]["count"], self.total_record_length)