Add collection param validation

WordPress · Sep 7, 2023 · 0047964 · 0047964
1 parent a7738b4
commit 0047964
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 15 deletions.
diff --git a/api/api/controllers/search_controller.py b/api/api/controllers/search_controller.py
@@ -521,7 +521,10 @@ def build_collection_query(
             search_query["filter"].append({"terms": {parameter: arguments}})
 
     # Exclude mature content and disabled sources
-    if not search_params.validated_data["include_sensitive_results"]:
+    include_sensitive_results = search_params.validated_data.get(
+        "include_sensitive_results", False
+    )
+    if not include_sensitive_results:
         search_query["must_not"].append({"term": {"mature": True}})
     if excluded_providers := get_excluded_providers():
         search_query["must_not"].append({"terms": {"provider": excluded_providers}})

diff --git a/api/api/serializers/media_serializers.py b/api/api/serializers/media_serializers.py
@@ -396,13 +396,15 @@ class MediaCollectionRequestSerializer(MediaListRequestSerializer):
         help_text="Get the collection for selected creator. "
         "Must be used with `source`.",
         required=False,
+        max_length=200,
     )
     source = serializers.CharField(
         label="source",
         help_text="Get the collection for selected source. "
         "Can be used with `creator` to get the creator "
         "collection from this source.",
         required=False,
+        max_length=200,
     )
 
     def validate_tag(self, value):
@@ -421,6 +423,20 @@ def validate_source(self, value):
             )
         return source
 
+    def validate(self, data):
+        data = super().validate(data)
+        if "creator" in self.initial_data and "source" not in self.initial_data:
+            raise serializers.ValidationError(
+                "Cannot set `source` without `creator`. "
+                "Use both of these or neither of them."
+            )
+        if not data.get("source") and not data.get("creator") and not data.get("tag"):
+            raise serializers.ValidationError(
+                "At least one of `source`, `creator` or `tag` must be "
+                "set for a collection view."
+            )
+        return data
+
 
 class MediaThumbnailRequestSerializer(serializers.Serializer):
     """This serializer parses and validates thumbnail query string parameters."""

diff --git a/api/test/unit/controllers/test_search_controller.py b/api/test/unit/controllers/test_search_controller.py
@@ -777,36 +777,49 @@ def test_post_process_results_recurses_as_needed(
     ("data", "expected_query"),
     [
         pytest.param(
-            {"unstable__include_sensitive_results": False, "tag": "art"},
+            {"unstable__include_sensitive_results": True, "tag": "art"},
             Q(
                 "bool",
                 filter=[{"terms": {"tags.name.keyword": ["art"]}}],
-                must_not=[{"term": {"mature": True}}],
                 should=DEFAULT_RANK_QUERY,
             ),
-            id="filter_by_tag_without_sensitive",
+            id="filter_by_tag_with_sensitive_bool",
         ),
         pytest.param(
-            {"unstable__include_sensitive_results": True, "tag": "art"},
+            {"tag": "art"},
             Q(
                 "bool",
                 filter=[{"terms": {"tags.name.keyword": ["art"]}}],
+                must_not=[{"term": {"mature": True}}],
+                should=DEFAULT_RANK_QUERY,
+            ),
+            id="filter_by_tag",
+        ),
+        pytest.param(
+            {"tag": "art", "license": "cc0,by", "category": "photograph"},
+            Q(
+                "bool",
+                filter=[
+                    {"terms": {"tags.name.keyword": ["art"]}},
+                    {"terms": {"license.keyword": ["cc0", "by"]}},
+                    {"terms": {"category": ["photograph"]}},
+                ],
                 should=DEFAULT_RANK_QUERY,
+                must_not=[{"term": {"mature": True}}],
             ),
-            id="filter_by_tag_with_sensitive",
+            id="filter_by_tag_with_other_filters",
         ),
         pytest.param(
-            {"unstable__include_sensitive_results": False, "source": "flickr"},
+            {"source": "flickr"},
             Q(
                 "bool",
                 filter=[{"terms": {"source.keyword": ["flickr"]}}],
                 must_not=[{"term": {"mature": True}}],
             ),
-            id="filter_by_source_without_sensitive",
+            id="filter_by_source",
         ),
         pytest.param(
             {
-                "unstable__include_sensitive_results": False,
                 "source": "flickr",
                 "creator": "nasa",
             },
@@ -818,21 +831,17 @@ def test_post_process_results_recurses_as_needed(
                 ],
                 must_not=[{"term": {"mature": True}}],
             ),
-            id="filter_by_creator_without_sensitive",
+            id="filter_by_creator",
         ),
     ],
 )
 @mock.patch("api.controllers.search_controller.Search", wraps=Search)
 def test_build_collection_query(mock_search_class, data, expected_query):
-    # Setup
     mock_search = mock_search_class.return_value
-
-    search_params = image_serializers.ImageCollectionRequestSerializer(data)
+    search_params = image_serializers.ImageCollectionRequestSerializer(data=data)
     search_params.is_valid()
 
-    # Action
     build_collection_query(mock_search, search_params)
     actual_query = mock_search.query.call_args[0][0]
 
-    # Validate
     assert actual_query == expected_query