From d37847b67425421b781e1c870659eb907a96f447 Mon Sep 17 00:00:00 2001
From: mk2155 <mk2155@cam.ac.uk>
Date: Fri, 7 Feb 2025 13:49:00 +0000
Subject: [PATCH] fix: correct read only create behaviour
Read only create failed to work correctly due to the way DRF serializers
behave. Updates to work consistently, and adds test to cover this case.
---
data_manager_api/tests/test_views.py | 37 +++++++++++++++++++++++++---
data_manager_api/views.py | 8 +++++-
2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/data_manager_api/tests/test_views.py b/data_manager_api/tests/test_views.py
index cc6a8e9..273d8a0 100644
--- a/data_manager_api/tests/test_views.py
+++ b/data_manager_api/tests/test_views.py
@@ -204,13 +204,10 @@ def test_account_delete_valid_at_earlier(api_client):
assert Account.objects.filter(crsid=instance.crsid, deleted_at=None).exists()
-def test_read_only_mode(api_client, settings):
+def test_read_only_mode_delete(api_client, settings):
settings.DATA_MANAGER_READ_ONLY = True
account = AccountFactory(account_details=True, account_identifier=True)
- account_details = account.account_details
- account_identifier = account.account_identifier.first()
-
valid_at = account.valid_at + timedelta(hours=1)
response = api_client.delete(
@@ -223,6 +220,16 @@ def test_read_only_mode(api_client, settings):
response = api_client.delete(build_account_detail_url("notpresent", valid_at))
assert response.status_code == status.HTTP_204_NO_CONTENT
+ assert Account.objects.filter(crsid=account.crsid, deleted_at=None).exists()
+
+
+def test_read_only_mode_update(api_client, settings):
+ settings.DATA_MANAGER_READ_ONLY = True
+
+ account = AccountFactory(account_details=True, account_identifier=True)
+ account_details = account.account_details
+ account_identifier = account.account_identifier.first()
+ valid_at = account.valid_at + timedelta(hours=1)
initial_data = data_account_factory(account, account_details, account_identifier)
new_data = {
@@ -244,6 +251,28 @@ def test_read_only_mode(api_client, settings):
assert Account.objects.get(crsid=account.crsid).account_details.name == initial_data["name"]
+def test_read_only_mode_create(api_client, settings):
+ settings.DATA_MANAGER_READ_ONLY = True
+
+ account = AccountFactory.build()
+
+ data = data_account_factory(account)
+
+ crsid = account.crsid
+ valid_at = account.valid_at
+
+ # Sanity-check: no record in DB
+ assert not Account.objects.filter(crsid=crsid).exists()
+ assert not AccountIdentifier.objects.filter(account_id=account).exists()
+
+ # We try to update while the object does not exist yet
+ response = api_client.put(build_account_detail_url(crsid, valid_at), data)
+
+ # We instead created the object
+ assert response.status_code == status.HTTP_201_CREATED
+ assert not Account.objects.filter(crsid=crsid).exists()
+
+
@pytest.mark.parametrize("method", ("put", "delete"))
def test_account_valid_at_missing(api_client, method):
account_details = AccountDetailsFactory()
diff --git a/data_manager_api/views.py b/data_manager_api/views.py
index d305244..b4b176f 100644
--- a/data_manager_api/views.py
+++ b/data_manager_api/views.py
@@ -140,8 +140,14 @@ class AccountViewSet(viewsets.GenericViewSet):
# forcibly invalidate the prefetch cache on the instance.
instance._prefetched_objects_cache = {}
+ # If instance is not set on the serializer, attempting to call `serializer.data` will
+ # result in an exception, as the instance object passed to the to_representation call will
+ # be a dictionary.
+ # The instance will be set either if one exists in the database, or after a call to
+ # serializer.save.
+ response_data = serializer.data if serializer.instance else serializer.initial_data
return Response(
- serializer.data, status=status.HTTP_200_OK if instance else status.HTTP_201_CREATED
+ response_data, status=status.HTTP_200_OK if instance else status.HTTP_201_CREATED
)
def get_exception_handler(self):
--
GitLab