From 1595366e0a934f9962cd0bede4f829814997127c Mon Sep 17 00:00:00 2001 From: Tom Ward Date: Wed, 20 Mar 2024 15:33:48 +0000 Subject: [PATCH] Use convention of user as last arg * as per PR review --- airlock/business_logic.py | 16 ++++++++-------- local_db/data_access.py | 4 ++-- tests/unit/test_business_logic.py | 18 +++++++++--------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/airlock/business_logic.py b/airlock/business_logic.py index 01fa01677..d2305b40b 100644 --- a/airlock/business_logic.py +++ b/airlock/business_logic.py @@ -634,7 +634,7 @@ def get_file_approvals(self, release_request: ReleaseRequest): ] def _verify_permission_to_review_file( - self, release_request: ReleaseRequest, user: User, path: Path + self, release_request: ReleaseRequest, relpath: UrlPath, user: User ): if release_request.status != RequestStatus.SUBMITTED: raise self.ApprovalPermissionDenied( @@ -651,24 +651,24 @@ def _verify_permission_to_review_file( "only an output checker can approve a file" ) - if path not in release_request.output_files_set(): + if relpath not in release_request.output_files_set(): raise self.ApprovalPermissionDenied( "file is not an output file on this request" ) - def approve_file(self, release_request: ReleaseRequest, user: User, path: Path): + def approve_file(self, release_request: ReleaseRequest, relpath: UrlPath, user: User): """ "Approve a file""" - self._verify_permission_to_review_file(release_request, user, path) + self._verify_permission_to_review_file(release_request, relpath, user) - bll._dal.approve_file(release_request.id, user, path) + bll._dal.approve_file(release_request.id, relpath, user) - def reject_file(self, release_request: ReleaseRequest, user: User, path: Path): + def reject_file(self, release_request: ReleaseRequest, relpath: UrlPath, user: User): """Reject a file""" - self._verify_permission_to_review_file(release_request, user, path) + self._verify_permission_to_review_file(release_request, relpath, user) - bll._dal.reject_file(release_request.id, user, path) + bll._dal.reject_file(release_request.id, relpath, user) def _get_configured_bll(): diff --git a/local_db/data_access.py b/local_db/data_access.py index 71676628d..805d6581d 100644 --- a/local_db/data_access.py +++ b/local_db/data_access.py @@ -167,7 +167,7 @@ def get_file_approvals(self, request_id): ).all() ] - def approve_file(self, request_id, user, relpath): + def approve_file(self, request_id, relpath, user): with transaction.atomic(): # nb. the business logic layer approve_file() should confirm that this path # is part of the request before calling this method @@ -181,7 +181,7 @@ def approve_file(self, request_id, user, relpath): review.status = FileApprovalStatus.APPROVED review.save() - def reject_file(self, request_id, user, relpath): + def reject_file(self, request_id, relpath, user): with transaction.atomic(): request_file = RequestFileMetadata.objects.get( filegroup__request_id=request_id, relpath=relpath diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py index c8731046e..fcaa5b800 100644 --- a/tests/unit/test_business_logic.py +++ b/tests/unit/test_business_logic.py @@ -570,7 +570,7 @@ def test_approve_file_not_submitted(bll): bll.add_file_to_request(release_request, path, author) with pytest.raises(bll.ApprovalPermissionDenied): - bll.approve_file(release_request, checker, path) + bll.approve_file(release_request, path, checker) def test_approve_file_not_your_own(bll): @@ -582,7 +582,7 @@ def test_approve_file_not_your_own(bll): ) with pytest.raises(bll.ApprovalPermissionDenied): - bll.approve_file(release_request, author, path) + bll.approve_file(release_request, path, author) def test_approve_file_not_checker(bll): @@ -595,7 +595,7 @@ def test_approve_file_not_checker(bll): ) with pytest.raises(bll.ApprovalPermissionDenied): - bll.approve_file(release_request, author2, path) + bll.approve_file(release_request, path, author2) def test_approve_file_not_part_of_request(bll): @@ -609,7 +609,7 @@ def test_approve_file_not_part_of_request(bll): bad_path = Path("path/file2.txt") with pytest.raises(bll.ApprovalPermissionDenied): - bll.approve_file(release_request, checker, bad_path) + bll.approve_file(release_request, bad_path, checker) def test_approve_supporting_file(bll): @@ -623,7 +623,7 @@ def test_approve_supporting_file(bll): release_request=release_request, to_status=RequestStatus.SUBMITTED, user=author ) with pytest.raises(bll.ApprovalPermissionDenied): - bll.approve_file(release_request, checker, path) + bll.approve_file(release_request, path, checker) def test_approve_file(bll): @@ -637,7 +637,7 @@ def test_approve_file(bll): assert len(bll.get_file_approvals(release_request)) == 0 - bll.approve_file(release_request, checker, path) + bll.approve_file(release_request, path, checker) assert len(bll.get_file_approvals(release_request)) == 1 assert ( @@ -657,7 +657,7 @@ def test_reject_file(bll): assert len(bll.get_file_approvals(release_request)) == 0 - bll.reject_file(release_request, checker, path) + bll.reject_file(release_request, path, checker) assert len(bll.get_file_approvals(release_request)) == 1 assert ( @@ -676,14 +676,14 @@ def test_approve_then_reject_file(bll): assert len(bll.get_file_approvals(release_request)) == 0 - bll.approve_file(release_request, checker, path) + bll.approve_file(release_request, path, checker) 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) + bll.reject_file(release_request, path, checker) assert len(bll.get_file_approvals(release_request)) == 1 assert (