refactor request_view

airlock/business_logic.py

@@ -401,6 +401,16 @@ def output_files_set(self):
             for request_file in filegroup.output_files
         }
 
+    def get_file_review_for_reviewer(self, urlpath: UrlPath, reviewer: str):
+        return next(
+            (
+                r
+                for r in self.get_request_file(urlpath).reviews
+                if r.reviewer == reviewer
+            ),
+            None,
+        )
+
     def is_supporting_file(self, urlpath: UrlPath):
         try:
             return self.get_request_file(urlpath).filetype == RequestFileType.SUPPORTING

airlock/views/request.py

@@ -86,9 +86,6 @@ def request_view(request, request_id: str, path: str = ""):
         file_approve_url = None
         file_reject_url = None
     else:
-        # get_path_item_from_tree_or_404() guarantees this file exists
-        group, request_file = release_request.get_request_group_and_file(path)
-
         file_approve_url = reverse(
             "file_approve",
             kwargs={"request_id": request_id, "path": path},
@@ -98,17 +95,13 @@ def request_view(request, request_id: str, path: str = ""):
             kwargs={"request_id": request_id, "path": path},
         )
 
-        existing_reviews = [
-            r
-            for r in release_request.filegroups[group]
-            .files[request_file.relpath]
-            .reviews
-            if r.reviewer == request.user.username
-        ]
-        if existing_reviews != []:
-            if existing_reviews[0].status == FileReviewStatus.APPROVED:
+        existing_review = release_request.get_file_review_for_reviewer(
+            path, request.user.username
+        )
+        if existing_review:
+            if existing_review.status == FileReviewStatus.APPROVED:
                 file_approve_url = None
-            else:
+            elif existing_review.status == FileReviewStatus.REJECTED:
                 file_reject_url = None
 
     context = {

tests/unit/test_business_logic.py

@@ -759,6 +759,47 @@ def test_approve_then_reject_file(bll):
     assert len(current_reviews) == 1
 
 
+def test_get_file_review_for_reviewer(bll):
+    release_request, path, author = setup_empty_release_request()
+    checker = factories.create_user("checker", [], True)
+    checker2 = factories.create_user("checker2", [], True)
+
+    bll.add_file_to_request(release_request, path, author, "default")
+    bll.set_status(
+        release_request=release_request, to_status=RequestStatus.SUBMITTED, user=author
+    )
+
+    assert len(_get_current_file_reviews(bll, release_request, path, checker)) == 0
+    fullpath = "default" / path
+    assert release_request.get_file_review_for_reviewer(fullpath, "checker") is None
+
+    bll.approve_file(release_request, path, checker)
+    bll.reject_file(release_request, path, checker2)
+    release_request = factories.refresh_release_request(release_request, author)
+
+    assert (
+        release_request.get_file_review_for_reviewer(fullpath, "checker").status
+        is FileReviewStatus.APPROVED
+    )
+    assert (
+        release_request.get_file_review_for_reviewer(fullpath, "checker2").status
+        is FileReviewStatus.REJECTED
+    )
+
+    bll.reject_file(release_request, path, checker)
+    bll.approve_file(release_request, path, checker2)
+    release_request = factories.refresh_release_request(release_request, author)
+
+    assert (
+        release_request.get_file_review_for_reviewer(fullpath, "checker").status
+        is FileReviewStatus.REJECTED
+    )
+    assert (
+        release_request.get_file_review_for_reviewer(fullpath, "checker2").status
+        is FileReviewStatus.APPROVED
+    )
+
+
 # add DAL method names to this if they do not require auditing
 DAL_AUDIT_EXCLUDED = {
     "get_release_request",