ATV-205 | Fix document status change causing error with json payload

.pre-commit-config.yaml

@@ -30,3 +30,7 @@ repos:
       - id: commitlint
         stages: [commit-msg, manual]
         additional_dependencies: ["@commitlint/config-conventional"]
+  - repo: https://github.com/koalaman/shellcheck-precommit
+    rev: v0.10.0
+    hooks:
+      - id: shellcheck

Dockerfile

@@ -1,13 +1,13 @@
 # ==============================
-FROM registry.access.redhat.com/ubi8/python-311 as appbase
+FROM registry.access.redhat.com/ubi8/python-311 AS appbase
 # ==============================
 
 EXPOSE 8000/tcp
 
 USER root
 
 RUN yum --disableplugin subscription-manager -y --allowerasing update \
-    && yum --disableplugin subscription-manager -y install pcre-devel \
+    && yum --disableplugin subscription-manager -y install pcre-devel nmap-ncat \
     && yum --disableplugin subscription-manager -y clean all
 
 COPY scripts /scripts
@@ -38,7 +38,7 @@ COPY --chown=1000:0 docker-entrypoint.sh /entrypoint/docker-entrypoint.sh
 ENTRYPOINT ["/entrypoint/docker-entrypoint.sh"]
 
 # ==============================
-FROM appbase as development
+FROM appbase AS development
 # ==============================
 
 COPY --chown=1000:0 requirements-dev.txt ./requirements-dev.txt
@@ -49,15 +49,15 @@ ENV DEV_SERVER=1
 COPY --chown=1000:0 . /app/
 
 # ==============================
-FROM appbase as staticbuilder
+FROM appbase AS staticbuilder
 # ==============================
 
 ENV STATIC_ROOT /var/static
 COPY --chown=1000:0 . /app/
 RUN SECRET_KEY="only-used-for-collectstatic" python manage.py collectstatic --noinput
 
 # ==============================
-FROM appbase as production
+FROM appbase AS production
 # ==============================
 
 # fatal: detected dubious ownership in repository at '/app'

docker-compose.yml

@@ -1,4 +1,3 @@
-version: "3.7"
 services:
     postgres:
         image: postgres:14

docker-entrypoint.sh

@@ -3,8 +3,13 @@
 set -e
 
 # Wait for the database
-if [ -z "$SKIP_DATABASE_CHECK" -o "$SKIP_DATABASE_CHECK" = "0" ]; then
-    wait-for-it.sh "${DATABASE_HOST}:${DATABASE_PORT-5432}" --timeout=30
+if [ -z "$SKIP_DATABASE_CHECK" ] || [ "$SKIP_DATABASE_CHECK" = "0" ]; then
+    until nc --verbose --wait 30 -z "${DATABASE_HOST}" "${DATABASE_PORT-5432}"
+    do
+      echo "Waiting for postgres database connection..."
+      sleep 1
+    done
+    echo "Database is up!"
 fi
 
 # Apply database migrations
@@ -17,14 +22,14 @@ fi
 # Create admin user. Generate password if there isn't one in the environment variables
 if [[ "$CREATE_ADMIN_USER" = "1" ]]; then
     if [[ "$ADMIN_USER_PASSWORD" ]]; then
-      ./manage.py add_admin_user -u admin -p $ADMIN_USER_PASSWORD -e admin@hel.ninja
+      ./manage.py add_admin_user -u admin -p "$ADMIN_USER_PASSWORD" -e admin@hel.ninja
     else
       ./manage.py add_admin_user -u admin -e admin@hel.ninja
     fi
 fi
 
 # Start server
-if [[ ! -z "$@" ]]; then
+if [[ -n "$*" ]]; then
     "$@"
 elif [[ "$DEV_SERVER" = "1" ]]; then
     python -Wd ./manage.py runserver 0.0.0.0:8000

documents/api/viewsets.py

@@ -443,6 +443,10 @@ class DocumentViewSet(AuditLoggingModelViewSet):
     filterset_class = DocumentFilterSet
     queryset = Document.objects.none()
 
+    def __init__(self, **kwargs):
+        super().__init__(**kwargs)
+        self.request_data_extra_fields = {}
+
     def get_queryset(self):
         user = self.request.user
         service = get_service_from_request(self.request)
@@ -580,13 +584,20 @@ def partial_update(self, request, pk, *args, **kwargs):
             status_history_serializer.is_valid(raise_exception=True)
             status_history_serializer.save()
 
-            # Make sure the request data query dict is mutable, assign status_timestamp field to be updated.
-            request.data._mutable = True
-            request.data["status_timestamp"] = timezone.now()
-            request.data._mutable = False
+            # Update status_timestamp field also if the status has changed
+            self.request_data_extra_fields["status_timestamp"] = timezone.now()
 
         return super().partial_update(request, pk, *args, **kwargs)
 
+    def get_serializer(self, *args, **kwargs):
+        if self.request_data_extra_fields:
+            # Request data itself is immutable so we modify the copy of the
+            # data before it's given to the serializer.
+            data = kwargs["data"].copy()
+            data.update(self.request_data_extra_fields)
+            kwargs["data"] = data
+        return super().get_serializer(*args, **kwargs)
+
     def update(self, request, *args, **kwargs):
         # Only allow for PATCH updates as described by the documentation
         # PUT requests will fail

documents/tests/snapshots/snap_test_api_retrieve_document.py

@@ -19,7 +19,7 @@
     "id": "485af718-d9d1-46b9-ad7b-33ea054126e3",
     "locked_after": None,
     "metadata": {},
-    "service": "service 212",
+    "service": "service 215",
     "status": {
         "timestamp": "2020-06-01T03:00:00+03:00",
         "value": "testing",
@@ -47,7 +47,7 @@
     "id": "485af718-d9d1-46b9-ad7b-33ea054126e3",
     "locked_after": None,
     "metadata": {},
-    "service": "service 215",
+    "service": "service 218",
     "status": {
         "timestamp": "2020-06-01T03:00:00+03:00",
         "value": "testing",
@@ -75,7 +75,7 @@
     "id": "485af718-d9d1-46b9-ad7b-33ea054126e3",
     "locked_after": None,
     "metadata": {},
-    "service": "service 214",
+    "service": "service 217",
     "status": {
         "timestamp": "2020-06-01T03:00:00+03:00",
         "value": "testing",

documents/tests/test_api_patch_document.py

@@ -6,6 +6,7 @@
 from dateutil.relativedelta import relativedelta
 from dateutil.utils import today
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.utils import timezone
 from freezegun import freeze_time
 from guardian.shortcuts import assign_perm
 from rest_framework import status
@@ -663,3 +664,46 @@ def test_audit_log_is_created_when_patching(user, attachments, ip_address):
         ).count()
         == 1
     )
+
+
+@pytest.mark.parametrize("format", ["multipart", "json"])
+def test_patch_status_update_in_multiple_formats(service_api_client, format):
+    """Updating status (and status_timestamp) should work with multipart and json
+    formatting.
+    """
+    document = DocumentFactory(
+        service=service_api_client.service,
+        status="testing",
+    )
+
+    response = service_api_client.patch(
+        reverse("documents-detail", args=[document.id]),
+        {"status": "changed status"},
+        format=format,
+    )
+
+    assert response.status_code == status.HTTP_200_OK
+    document.refresh_from_db()
+    assert document.status == "changed status"
+
+
+@freeze_time("2021-06-30T12:00:00")
+def test_patch_status_timestamp_is_updated(service_api_client):
+    """When updating the status of a document, the status_timestamp should also get
+    updated automatically.
+    """
+    document = DocumentFactory(
+        service=service_api_client.service,
+        status="testing",
+    )
+
+    with freeze_time("2024-12-20T12:00:00"):
+        dt = timezone.now()
+        response = service_api_client.patch(
+            reverse("documents-detail", args=[document.id]),
+            {"status": "changed status"},
+        )
+
+    assert response.status_code == status.HTTP_200_OK
+    document.refresh_from_db()
+    assert document.status_timestamp == dt