Correctly validate moderation status

SchemaNode serialize method is executed before validation
so invalid value where not validated by "OneOf".

Remove the serialize method and convert the values
to the enum in the views.

This change uncovered a couple other issues:

- Moderation status should be required on update status endpoint
- Correct expose a message when annotation_updated is outdated

Author: marcospri

h/schemas/api/group.py

@@ -28,7 +28,7 @@
 class FilterGroupAnnotationsSchema(colander.Schema):
     """Schema for validating filter-group-annotations API data."""
 
-    moderation_status = ModerationStatusNode(missing=None)
+    moderation_status = ModerationStatusNode(missing=None, default=None)
 
 
 class GroupAPISchema(JSONSchema):

h/schemas/api/moderation.py

@@ -3,21 +3,16 @@
 import colander
 from pyramid import httpexceptions
 
-from h.models.annotation import ModerationStatus
-
 
 class ModerationStatusNode(colander.SchemaNode):
     schema_type = colander.String
     validator = colander.OneOf(["APPROVED", "PENDING", "DENIED", "SPAM"])
 
-    def deserialize(self, cstruct):
-        return ModerationStatus(cstruct) if cstruct else None
-
 
 class ChangeAnnotationModerationStatusSchema(colander.Schema):
     """Schema for validating change-annotation-moderation-status API data."""
 
-    moderation_status = ModerationStatusNode()
+    moderation_status = ModerationStatusNode(missing=colander.required)
     annotation_updated = colander.SchemaNode(
         colander.DateTime(),
         description="Sentinel value to avoid conflicting updates. It should match the current value of annotation.updated",
@@ -33,5 +28,5 @@ def validator(self, _node, cstruct):
         annotation_updated = self._annotation.updated.replace(tzinfo=UTC).timestamp()
         if annotation_updated != cstruct["annotation_updated"].timestamp():
             raise httpexceptions.HTTPConflict(
-                explanation="The annotation has been updated since the moderation status was set."
+                detail="The annotation has been updated since the moderation status was set."
             )

h/views/api/group_annotations.py

@@ -1,4 +1,5 @@
 from h.models import Annotation
+from h.models.annotation import ModerationStatus
 from h.schemas.api.group import FilterGroupAnnotationsSchema
 from h.schemas.pagination import Pagination
 from h.schemas.util import validate_query_params
@@ -23,7 +24,11 @@ def list_annotations(context: GroupContext, request):
     group = context.group
     annotation_json_service = request.find_service(name="annotation_json")
 
-    moderation_status_filter = params["moderation_status"]
+    moderation_status_filter = (
+        ModerationStatus(params["moderation_status"])
+        if params.get("moderation_status")
+        else None
+    )
     query = AnnotationReadService.annotation_search_query(
         groupid=group.pubid, moderation_status=moderation_status_filter
     )

h/views/api/moderation.py

@@ -52,7 +52,7 @@ def change_annotation_moderation_status(context, request):
     params = validate_json(
         ChangeAnnotationModerationStatusSchema(context.annotation), request
     )
-    status = params["moderation_status"]
+    status = ModerationStatus(params["moderation_status"])
     request.find_service(name="annotation_moderation").set_status(
         context.annotation, status, request.user
     )

tests/functional/api/moderation_test.py

@@ -78,7 +78,7 @@ def test_it_returns_http_404_if_annotation_is_in_world_group(
         # The current user does not have moderation rights on the world group
         assert res.status_code == 404
 
-    def test_it_returns_http_404_if_no_authn(self, app, group_annotation):
+    def test_it_returns_http_404_if_no_auth(self, app, group_annotation):
         res = app.delete(
             f"/api/annotations/{group_annotation.id}/hide",
             expect_errors=True,

tests/unit/h/schemas/api/group_test.py

@@ -1,7 +1,6 @@
 import pytest
 from webob.multidict import MultiDict
 
-from h.models.annotation import ModerationStatus
 from h.models.group import (
     AUTHORITY_PROVIDED_ID_MAX_LENGTH,
     GROUP_DESCRIPTION_MAX_LENGTH,
@@ -203,7 +202,7 @@ def test_it(self, schema):
 
         validated_data = validate_query_params(schema, data)
 
-        assert validated_data == {"moderation_status": ModerationStatus.PENDING}
+        assert validated_data == {"moderation_status": "PENDING"}
 
     def test_it_when_empty(self, schema):
         data = MultiDict({})

tests/unit/h/views/api/group_annotations_test.py

@@ -4,6 +4,7 @@
 from sqlalchemy import func, select
 
 from h.models import Annotation
+from h.schemas.base import ValidationError
 from h.traversal import (
     GroupContext,
 )
@@ -40,6 +41,12 @@ def test_it(
             ],
         }
 
+    def test_it_with_wrong_filter(self, context, pyramid_request):
+        pyramid_request.params["moderation_status"] = "wrong"
+
+        with pytest.raises(ValidationError):
+            list_annotations(context, pyramid_request)
+
     @pytest.fixture
     def context(self, factories):
         return create_autospec(