Skip to content

Commit

Permalink
fix ruff
Browse files Browse the repository at this point in the history
  • Loading branch information
madwort committed Mar 15, 2024
1 parent 2ec66d8 commit 9e7de45
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 39 deletions.
30 changes: 21 additions & 9 deletions airlock/business_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -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}"
)
Expand Down Expand Up @@ -535,15 +546,17 @@ 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}"
)

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:
Expand All @@ -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)

Expand All @@ -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())
Expand Down
22 changes: 14 additions & 8 deletions local_db/data_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -174,4 +181,3 @@ def reject_file(self, request_id, user, relpath):
)
review.status = FileApprovalStatus.REJECTED
review.save()

2 changes: 1 addition & 1 deletion local_db/migrations/0002_requestmetadata_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
),
),
]
3 changes: 2 additions & 1 deletion local_db/migrations/0004_filereview.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
11 changes: 8 additions & 3 deletions local_db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -70,14 +72,17 @@ class RequestFileMetadata(models.Model):
class Meta:
unique_together = ("relpath", "filegroup")


class FileReview(models.Model):
"""An output checker's review of a file"""

file = models.ForeignKey(
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)

Expand Down
41 changes: 24 additions & 17 deletions tests/unit/test_business_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Expand All @@ -496,17 +501,17 @@ 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

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


Expand All @@ -516,17 +521,17 @@ 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

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):
Expand All @@ -535,19 +540,21 @@ 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

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
)

0 comments on commit 9e7de45

Please sign in to comment.