From d7870e527f8102529f74d4cb96759720b4748fba Mon Sep 17 00:00:00 2001
From: Peter Inglesby <peter.inglesby@gmail.com>
Date: Thu, 29 Feb 2024 07:18:59 +0000
Subject: [PATCH] Partial proof of concept for splitting up business logic
 layer from data access layer

---
 airlock/api.py  | 42 +++++++++++++++++++++++-------------------
 local_db/api.py | 21 +++++++++++----------
 2 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/airlock/api.py b/airlock/api.py
index 3167e0d3e..abe9820b3 100644
--- a/airlock/api.py
+++ b/airlock/api.py
@@ -284,12 +284,12 @@ def check_status(
         there's not transaction protection.
         """
         # validate state logic
-        valid_transitions = self.VALID_STATE_TRANSITIONS.get(release_request.status, [])
+        # valid_transitions = self.VALID_STATE_TRANSITIONS.get(release_request.status, [])
 
-        if to_status not in valid_transitions:
-            raise self.InvalidStateTransition(
-                f"from {release_request.status.name} to {to_status.name}"
-            )
+        # if to_status not in valid_transitions:
+        #     raise self.InvalidStateTransition(
+        #         f"from {release_request.status.name} to {to_status.name}"
+        #     )
 
         # check permissions
         # author transitions
@@ -314,22 +314,26 @@ def check_status(
     def set_status(
         self, release_request: ReleaseRequest, to_status: Status, user: User
     ):
-        """Set the status of the request.
-
-        This will validate the transition, and then mutate the request object.
-
-        As calling set_status will mutate the passed ReleaseRequest, in cases
-        where we may want to call to external services (e.g. job-server) to
-        mutate external state, and these calls might fail, we provide
-        a look-before-you-leap API. That is, when changing status and related
-        state, an implementer should call `check_status(...)` first to validate
-        the desired state transition and permissions are valid, then mutate
-        their own state, and then call `set_status(...)` if successful to
-        mutate the passed ReleaseRequest object.
-        """
+        """Set the status of the request."""
 
-        # validate first
+        # We're using this only to validate that the user has permission to move
+        # release_request into to_status.
         self.check_status(release_request, to_status, user)
+
+        valid_from_statuses = [
+            status
+            for status in self.VALID_STATE_TRANSITIONS
+            if to_status in self.VALID_STATE_TRANSITIONS[status]
+        ]
+
+        num_updates = self.update_status(
+            release_request, to_status, valid_from_statuses
+        )
+        if num_updates == 0:
+            raise self.InvalidStateTransition(
+                f"from {release_request.status.name} to {to_status.name}"
+            )
+
         release_request.status = to_status
 
     def add_file_to_request(
diff --git a/local_db/api.py b/local_db/api.py
index 7a0cab9d8..17a13d37f 100644
--- a/local_db/api.py
+++ b/local_db/api.py
@@ -1,6 +1,6 @@
 from dataclasses import dataclass
 from pathlib import Path
-from typing import Optional
+from typing import List, Optional
 
 from django.db import transaction
 
@@ -119,15 +119,16 @@ def get_outstanding_requests_for_review(self, user: User):
 
         return requests
 
-    def set_status(self, request: ReleaseRequest, status: Status, user: User):
-        with transaction.atomic():
-            # validate transition/permissions ahead of time
-            self.check_status(request, status, user)
-            # persist state change
-            metadata = self._find_metadata(request.id)
-            metadata.status = status
-            metadata.save()
-            super().set_status(request, status, user)
+    def update_status(
+        self,
+        request: ReleaseRequest,
+        to_status: Status,
+        valid_from_statuses: List[Status],
+    ):
+        return RequestMetadata.objects.filter(
+            id=request.id,
+            status__in=valid_from_statuses,
+        ).update(status=to_status)
 
     def add_file_to_request(
         self,