From 9e7de4599027e26b75d0cdf6042a9580556ce588 Mon Sep 17 00:00:00 2001 From: Tom Ward Date: Fri, 15 Mar 2024 16:09:49 +0000 Subject: [PATCH] fix ruff --- airlock/business_logic.py | 30 ++++++++++---- local_db/data_access.py | 22 ++++++---- .../migrations/0002_requestmetadata_status.py | 2 +- local_db/migrations/0004_filereview.py | 3 +- local_db/models.py | 11 +++-- tests/unit/test_business_logic.py | 41 +++++++++++-------- 6 files changed, 70 insertions(+), 39 deletions(-) diff --git a/airlock/business_logic.py b/airlock/business_logic.py index ddeeb55e..a715feb6 100644 --- a/airlock/business_logic.py +++ b/airlock/business_logic.py @@ -432,14 +432,22 @@ def check_status( # check permissions # author transitions - if to_status in [RequestStatus.PENDING, RequestStatus.SUBMITTED, RequestStatus.WITHDRAWN]: + if to_status in [ + RequestStatus.PENDING, + RequestStatus.SUBMITTED, + RequestStatus.WITHDRAWN, + ]: if user.username != release_request.author: raise self.RequestPermissionDenied( f"only {user.username} can set status to {to_status.name}" ) # output checker transitions - if to_status in [RequestStatus.APPROVED, RequestStatus.REJECTED, RequestStatus.RELEASED]: + if to_status in [ + RequestStatus.APPROVED, + RequestStatus.REJECTED, + RequestStatus.RELEASED, + ]: if not user.output_checker: raise self.RequestPermissionDenied( f"only an output checker can set status to {to_status.name}" @@ -484,7 +492,10 @@ def add_file_to_request( f"only author {release_request.author} can add files to this request" ) - if release_request.status not in [RequestStatus.PENDING, RequestStatus.SUBMITTED]: + if release_request.status not in [ + RequestStatus.PENDING, + RequestStatus.SUBMITTED, + ]: raise self.RequestPermissionDenied( f"cannot add file to request in state {release_request.status.name}" ) @@ -535,7 +546,9 @@ def get_file_approvals(self, release_request: ReleaseRequest): for r in bll._dal.get_file_approvals(release_request.id) ] - def _verify_permission_to_review_file(self, release_request: ReleaseRequest, user: User, path: Path): + 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}" @@ -543,7 +556,7 @@ def _verify_permission_to_review_file(self, release_request: ReleaseRequest, use if user.username == release_request.author: raise self.ApprovalPermissionDenied( - f"cannot approve files in your own request" + "cannot approve files in your own request" ) if not user.output_checker: @@ -552,12 +565,10 @@ def _verify_permission_to_review_file(self, release_request: ReleaseRequest, use ) if path not in release_request.file_set(): - raise self.ApprovalPermissionDenied( - f"file is not part of the request" - ) + raise self.ApprovalPermissionDenied("file is not part of the request") def approve_file(self, release_request: ReleaseRequest, user: User, path: Path): - """"Approve a file""" + """ "Approve a file""" self._verify_permission_to_review_file(release_request, user, path) @@ -570,6 +581,7 @@ def reject_file(self, release_request: ReleaseRequest, user: User, path: Path): bll._dal.reject_file(release_request.id, user, path) + def _get_configured_bll(): DataAccessLayer = import_string(settings.AIRLOCK_DATA_ACCESS_LAYER) return BusinessLogicLayer(DataAccessLayer()) diff --git a/local_db/data_access.py b/local_db/data_access.py index 173ba726..c6bb7850 100644 --- a/local_db/data_access.py +++ b/local_db/data_access.py @@ -7,7 +7,13 @@ DataAccessLayerProtocol, RequestStatus, ) -from local_db.models import FileGroupMetadata, RequestFileMetadata, RequestMetadata, FileReview, FileApprovalStatus +from local_db.models import ( + FileApprovalStatus, + FileGroupMetadata, + FileReview, + RequestFileMetadata, + RequestMetadata, +) class LocalDBDataAccessLayer(DataAccessLayerProtocol): @@ -64,7 +70,7 @@ def _filereview(self, file_review: FileReview): reviewer=file_review.reviewer, status=file_review.status, created_at=file_review.created_at, - updated_at=file_review.updated_at, + updated_at=file_review.updated_at, ) def _get_or_create_filegroupmetadata(self, request_id: str, group_name: str): @@ -98,7 +104,9 @@ def get_requests_authored_by_user(self, username: str): def get_outstanding_requests_for_review(self): return [ self._request(metadata) - for metadata in RequestMetadata.objects.filter(status=RequestStatus.SUBMITTED) + for metadata in RequestMetadata.objects.filter( + status=RequestStatus.SUBMITTED + ) ] def set_status(self, request_id: str, status: RequestStatus): @@ -134,14 +142,14 @@ def add_file_to_request(self, request_id, relpath: Path, group_name: str): metadata = self._find_metadata(request_id) return self._get_filegroups(metadata) - def get_file_approvals(self, request_id): return [ self._filereview(r) - for r in FileReview.objects.filter(file__filegroup__request_id=request_id).all() + for r in FileReview.objects.filter( + file__filegroup__request_id=request_id + ).all() ] - def approve_file(self, request_id, user, relpath): with transaction.atomic(): try: @@ -158,7 +166,6 @@ def approve_file(self, request_id, user, relpath): review.status = FileApprovalStatus.APPROVED review.save() - def reject_file(self, request_id, user, relpath): with transaction.atomic(): try: @@ -174,4 +181,3 @@ def reject_file(self, request_id, user, relpath): ) review.status = FileApprovalStatus.REJECTED review.save() - diff --git a/local_db/migrations/0002_requestmetadata_status.py b/local_db/migrations/0002_requestmetadata_status.py index c5e00577..d63a3be7 100644 --- a/local_db/migrations/0002_requestmetadata_status.py +++ b/local_db/migrations/0002_requestmetadata_status.py @@ -17,7 +17,7 @@ class Migration(migrations.Migration): name="status", field=local_db.models.StatusField( status_enum=airlock.business_logic.RequestStatus, - default=airlock.business_logic.RequestStatus["PENDING"] + default=airlock.business_logic.RequestStatus["PENDING"], ), ), ] diff --git a/local_db/migrations/0004_filereview.py b/local_db/migrations/0004_filereview.py index 5fbf9a69..c1fd8392 100644 --- a/local_db/migrations/0004_filereview.py +++ b/local_db/migrations/0004_filereview.py @@ -2,9 +2,10 @@ import django.db.models.deletion import django.utils.timezone -import local_db.models from django.db import migrations, models +import local_db.models + class Migration(migrations.Migration): dependencies = [ diff --git a/local_db/models.py b/local_db/models.py index 447ed9b4..cb360eab 100644 --- a/local_db/models.py +++ b/local_db/models.py @@ -2,7 +2,7 @@ from django.utils import timezone from ulid import ulid -from airlock.business_logic import RequestStatus, FileApprovalStatus +from airlock.business_logic import FileApprovalStatus, RequestStatus def local_request_id(): @@ -31,7 +31,9 @@ def get_prep_value(self, value): try: return value.name except Exception as exc: - raise exc.__class__(f"value {value} should be instance of {self.status_enum}") from exc + raise exc.__class__( + f"value {value} should be instance of {self.status_enum}" + ) from exc class RequestMetadata(models.Model): @@ -70,6 +72,7 @@ class RequestFileMetadata(models.Model): class Meta: unique_together = ("relpath", "filegroup") + class FileReview(models.Model): """An output checker's review of a file""" @@ -77,7 +80,9 @@ class FileReview(models.Model): RequestFileMetadata, related_name="reviews", on_delete=models.CASCADE ) reviewer = models.TextField() - status = StatusField(status_enum=FileApprovalStatus, default=FileApprovalStatus.REJECTED) + status = StatusField( + status_enum=FileApprovalStatus, default=FileApprovalStatus.REJECTED + ) created_at = models.DateTimeField(default=timezone.now) updated_at = models.DateTimeField(default=timezone.now) diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py index eb683735..efe86857 100644 --- a/tests/unit/test_business_logic.py +++ b/tests/unit/test_business_logic.py @@ -6,7 +6,14 @@ from django.conf import settings import old_api -from airlock.business_logic import BusinessLogicLayer, RequestStatus, UrlPath, Workspace, FileApprovalStatus, FileReview +from airlock.business_logic import ( + BusinessLogicLayer, + FileApprovalStatus, + FileReview, + RequestStatus, + UrlPath, + Workspace, +) from airlock.users import User from tests import factories @@ -480,9 +487,7 @@ def test_approve_file_not_part_of_request(bll): bll.add_file_to_request(release_request, path, author) bll.set_status( - release_request=release_request, - to_status=RequestStatus.SUBMITTED, - user=author + release_request=release_request, to_status=RequestStatus.SUBMITTED, user=author ) bad_path = Path("path/file2.txt") @@ -496,9 +501,7 @@ def test_approve_file(bll): bll.add_file_to_request(release_request, path, author) bll.set_status( - release_request=release_request, - to_status=RequestStatus.SUBMITTED, - user=author + release_request=release_request, to_status=RequestStatus.SUBMITTED, user=author ) assert len(bll.get_file_approvals(release_request)) == 0 @@ -506,7 +509,9 @@ def test_approve_file(bll): 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 + assert ( + bll.get_file_approvals(release_request)[0].status == FileApprovalStatus.APPROVED + ) assert type(bll.get_file_approvals(release_request)[0]) == FileReview @@ -516,9 +521,7 @@ def test_reject_file(bll): bll.add_file_to_request(release_request, path, author) bll.set_status( - release_request=release_request, - to_status=RequestStatus.SUBMITTED, - user=author + release_request=release_request, to_status=RequestStatus.SUBMITTED, user=author ) assert len(bll.get_file_approvals(release_request)) == 0 @@ -526,7 +529,9 @@ def test_reject_file(bll): 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 + assert ( + bll.get_file_approvals(release_request)[0].status == FileApprovalStatus.REJECTED + ) def test_approve_then_reject_file(bll): @@ -535,9 +540,7 @@ def test_approve_then_reject_file(bll): bll.add_file_to_request(release_request, path, author) bll.set_status( - release_request=release_request, - to_status=RequestStatus.SUBMITTED, - user=author + release_request=release_request, to_status=RequestStatus.SUBMITTED, user=author ) assert len(bll.get_file_approvals(release_request)) == 0 @@ -545,9 +548,13 @@ def test_approve_then_reject_file(bll): 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 + 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 + assert ( + bll.get_file_approvals(release_request)[0].status == FileApprovalStatus.REJECTED + )