Skip to content

Commit

Permalink
Use convention of user as last arg
Browse files Browse the repository at this point in the history
* as per PR review
  • Loading branch information
madwort committed Mar 20, 2024
1 parent 26c9d38 commit 1595366
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 19 deletions.
16 changes: 8 additions & 8 deletions airlock/business_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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():
Expand Down
4 changes: 2 additions & 2 deletions local_db/data_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 9 additions & 9 deletions tests/unit/test_business_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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 (
Expand All @@ -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 (
Expand All @@ -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 (
Expand Down

0 comments on commit 1595366

Please sign in to comment.