implement reject_file

opensafely-core · Mar 15, 2024 · 59dfb42 · 59dfb42
1 parent b98ae03
commit 59dfb42
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 6 deletions.
diff --git a/airlock/business_logic.py b/airlock/business_logic.py
@@ -511,9 +511,7 @@ def get_file_approvals(self, release_request: ReleaseRequest):
         return bll._dal.get_file_approvals(release_request.id)
 
 
-    def approve_file(self, release_request: ReleaseRequest, user: User, path: Path):
-        """"Approve a file"""
-
+    def _verify_permission_to_review_file(self, release_request: ReleaseRequest, user: User, path: Path):
         if release_request.status != RequestStatus.SUBMITTED:
             raise self.ApprovalPermissionDenied(
                 f"cannot approve file from request in state {release_request.status.name}"
@@ -534,13 +532,19 @@ def approve_file(self, release_request: ReleaseRequest, user: User, path: Path):
                 f"file is not part of the request"
             )
 
+    def approve_file(self, release_request: ReleaseRequest, user: User, path: Path):
+        """"Approve a file"""
+
+        self._verify_permission_to_review_file(release_request, user, path)
+
         bll._dal.approve_file(release_request.id, user, path)
 
-    def reject_file(self):
+    def reject_file(self, release_request: ReleaseRequest, user: User, path: Path):
         """Reject a file"""
 
-        pass
+        self._verify_permission_to_review_file(release_request, user, path)
 
+        bll._dal.reject_file(release_request.id, user, path)
 
 def _get_configured_bll():
     DataAccessLayer = import_string(settings.AIRLOCK_DATA_ACCESS_LAYER)

diff --git a/local_db/data_access.py b/local_db/data_access.py
@@ -121,6 +121,7 @@ def add_file_to_request(self, request_id, relpath: Path, group_name: str):
 
 
     def get_file_approvals(self, request_id):
+        # TODO: return dict
         return FileReview.objects.filter(file__filegroup__request_id=request_id).all()
 
 
@@ -142,4 +143,18 @@ def approve_file(self, request_id, user, relpath):
 
 
     def reject_file(self, request_id, user, relpath):
-        pass
+        with transaction.atomic():
+            try:
+                request_file = RequestFileMetadata.objects.get(
+                    filegroup__request_id=request_id, relpath=relpath
+                )
+            except RequestFileMetadata.DoesNotExist:
+                raise BusinessLogicLayer.FileNotFound(
+                    f"file {relpath} not part of a request {request_id}"
+                )
+            review, _ = FileReview.objects.get_or_create(
+                file=request_file, reviewer=user
+            )
+            review.status = FileApprovalStatus.REJECTED
+            review.save()
+
diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py
@@ -496,7 +496,52 @@ def test_approve_file(bll):
     )
 
     assert len(bll.get_file_approvals(release_request)) == 0
+
     bll.approve_file(release_request, checker, path)
+
     assert len(bll.get_file_approvals(release_request)) == 1
     assert bll.get_file_approvals(release_request)[0].status == FileApprovalStatus.APPROVED
 
+
+def test_reject_file(bll):
+    release_request, path, author = setup_empty_release_request()
+    checker = User(2, "checker", [], True)
+
+    bll.add_file_to_request(release_request, path, author)
+    bll.set_status(
+        release_request=release_request,
+        to_status=RequestStatus.SUBMITTED,
+        user=author
+    )
+
+    assert len(bll.get_file_approvals(release_request)) == 0
+
+    bll.reject_file(release_request, checker, path)
+
+    assert len(bll.get_file_approvals(release_request)) == 1
+    assert bll.get_file_approvals(release_request)[0].status == FileApprovalStatus.REJECTED
+
+
+def test_approve_then_reject_file(bll):
+    release_request, path, author = setup_empty_release_request()
+    checker = User(2, "checker", [], True)
+
+    bll.add_file_to_request(release_request, path, author)
+    bll.set_status(
+        release_request=release_request,
+        to_status=RequestStatus.SUBMITTED,
+        user=author
+    )
+
+    assert len(bll.get_file_approvals(release_request)) == 0
+
+    bll.approve_file(release_request, checker, path)
+
+    assert len(bll.get_file_approvals(release_request)) == 1
+    assert bll.get_file_approvals(release_request)[0].status == FileApprovalStatus.APPROVED
+
+    bll.reject_file(release_request, checker, path)
+
+    assert len(bll.get_file_approvals(release_request)) == 1
+    assert bll.get_file_approvals(release_request)[0].status == FileApprovalStatus.REJECTED
+