Merge pull request #533 from opensafely-core/user_can_review

Add user_can_review function
opensafely-core · Jul 15, 2024 · 3f0f815 · 3f0f815
2 parents 46be417 + 05c295e
commit 3f0f815
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 6 deletions.
diff --git a/airlock/business_logic.py b/airlock/business_logic.py
@@ -864,15 +864,15 @@ def get_request_file_status(
         rfile = self.get_request_file_from_urlpath(relpath)
         phase = self.get_turn_phase()
         decision = RequestFileDecision.INCOMPLETE
-        is_output_checker = user.output_checker and self.author != user.username
+        can_review = self.user_can_review(user)
 
         match phase:
             case ReviewTurnPhase.INDEPENDENT:
                 # already set - no one knows the current status
                 pass
             case ReviewTurnPhase.CONSOLIDATING:
-                # only output-checkers know the current status
-                if is_output_checker:
+                # only users who can review this request know the current status
+                if can_review:
                     decision = rfile.get_decision()
             case ReviewTurnPhase.COMPLETE | ReviewTurnPhase.AUTHOR:
                 # everyone knows the current status
@@ -891,9 +891,7 @@ def get_visible_comments_for_group(
     ) -> list[tuple[Comment, dict[str, str]]]:
         filegroup = self.filegroups[group]
         current_phase = self.get_turn_phase()
-        can_see_review_comments = (
-            user.output_checker and not user.username == self.author
-        )
+        can_see_review_comments = self.user_can_review(user)
 
         comments = []
 
@@ -1062,6 +1060,9 @@ def completed_reviews_count(self):
     def status_owner(self) -> RequestStatusOwner:
         return BusinessLogicLayer.STATUS_OWNERS[self.status]
 
+    def user_can_review(self, user: User) -> bool:
+        return user.output_checker and not user.username == self.author
+
     def can_be_released(self) -> bool:
         return (
             self.status in [RequestStatus.REVIEWED, RequestStatus.APPROVED]

diff --git a/tests/functional/test_login.py b/tests/functional/test_login.py
@@ -8,6 +8,7 @@
 @mock.patch("airlock.login_api.session.post", autospec=True)
 def test_login(requests_post, settings, page, live_server):
     settings.AIRLOCK_API_TOKEN = "test_api_token"
+    settings.DEV_USERS = {}
 
     api_response = requests_post.return_value
     api_response.status_code = 200