diff --git a/airlock/business_logic.py b/airlock/business_logic.py index 69ad14b4..3ba989a0 100644 --- a/airlock/business_logic.py +++ b/airlock/business_logic.py @@ -10,7 +10,7 @@ from enum import Enum from functools import cached_property from pathlib import Path -from typing import Any, Protocol, Self, cast +from typing import Any, Protocol, Self, Sequence, cast from django.conf import settings from django.urls import reverse @@ -113,7 +113,7 @@ class RequestFileStatus: vote: RequestFileVote | None -class CommentVisibility(Enum): +class Visibility(Enum): """The visibility of comments.""" # only visible to output-checkers @@ -124,16 +124,16 @@ class CommentVisibility(Enum): @classmethod def choices(cls): return { - CommentVisibility.PRIVATE: "Only visible to output-checkers", - CommentVisibility.PUBLIC: "Visible to all", + Visibility.PRIVATE: "Only visible to output-checkers", + Visibility.PUBLIC: "Visible to all users", } - def description(self): + # These will be for tooltips once those are working inside of pills + def description(self): # pragma: no cover return self.choices()[self] - @cached_property - def independent_description(self): - return "Only visible to you until both reviews submitted" + def blinded_description(self): # pragma: no cover + return "Only visible to you until two reviews have been completed" class ReviewTurnPhase(Enum): @@ -256,6 +256,8 @@ def from_request( **kwargs: str, ): # Note: kwargs go straight to extra + # set review_turn from request + kwargs["review_turn"] = str(request.review_turn) event = cls( type=type, user=user.username, @@ -287,6 +289,83 @@ def __str__(self): def description(self): return AUDIT_MSG_FORMATS[self.type] + @property + def review_turn(self) -> int: + return int(self.extra.get("review_turn", 0)) + + @property + def author(self) -> str: + return self.user + + @property + def visibility(self) -> Visibility: + v = self.extra.get("visibility") + if v: + return Visibility[v.upper()] + else: + return Visibility.PUBLIC + + +class VisibleItem(Protocol): + @property + def author(self) -> str: + raise NotImplementedError() + + @property + def review_turn(self) -> int: + raise NotImplementedError() + + @property + def visibility(self) -> Visibility: + raise NotImplementedError() + + +def filter_visible_items( + items: Sequence[VisibleItem], + current_turn: int, + current_phase: ReviewTurnPhase, + user_can_review: bool, + user: User, +): + """Filter a list of items to only include items this user is allowed to view. + + This depends on the current turn, phase, and whether the user is the author + of said item. + """ + for item in items: + # you can always see things you've authored. Doing this first + # simplifies later logic, and avoids potential bugs with users adding + # items but then they can not see the item they just added + if item.author == user.username: + yield item + continue + + match item.visibility: + case Visibility.PUBLIC: + # can always see public items from previous turns and completed turns + if ( + item.review_turn < current_turn + or current_phase == ReviewTurnPhase.COMPLETE + ): + yield item + # can see public items for other users if CONSOLIDATING and can review + elif current_phase == ReviewTurnPhase.CONSOLIDATING and user_can_review: + yield item + case Visibility.PRIVATE: + # have to be able to review this request to see *any* private items + if user_can_review: + # can always see private items from previous turns + if ( + item.review_turn < current_turn + or current_phase == ReviewTurnPhase.COMPLETE + ): + yield item + # can only see private items from current turn if we are not INDEPENDENT + elif current_phase != ReviewTurnPhase.INDEPENDENT: + yield item + case _: # pragma: nocover + assert False + class AirlockContainer(Protocol): """Structural typing class for a instance of a Workspace or ReleaseRequest @@ -772,7 +851,7 @@ class Comment: comment: str author: str created_at: datetime - visibility: CommentVisibility + visibility: Visibility review_turn: int @classmethod @@ -916,52 +995,30 @@ def get_request_file_status( def get_visible_comments_for_group( self, group: str, user: User - ) -> list[tuple[Comment, dict[str, str]]]: + ) -> list[tuple[Comment, str]]: filegroup = self.filegroups[group] current_phase = self.get_turn_phase() - can_see_review_comments = self.user_can_review(user) comments = [] + visible_comments = filter_visible_items( + filegroup.comments, + self.review_turn, + current_phase, + self.user_can_review(user), + user, + ) - for comment in filegroup.comments: - if current_phase == ReviewTurnPhase.INDEPENDENT: - # comments are temporarily hidden - metadata = { - "description": comment.visibility.independent_description, - "class": "comment_blinded", - } + for comment in visible_comments: + # does this comment need to be blinded? + if ( + comment.review_turn == self.review_turn + and current_phase == ReviewTurnPhase.INDEPENDENT + ): + html_class = "comment_blinded" else: - metadata = { - "description": comment.visibility.description(), - "class": f"comment_{comment.visibility.name.lower()}", - } - - # you can always see comments you've authored. Doing this first - # simplifies later logic, and avoids potential bugs with users - # adding comments but then they can see the comment they just added - if comment.author == user.username: - comments.append((comment, metadata)) - continue + html_class = f"comment_{comment.visibility.name.lower()}" - match comment.visibility: - case CommentVisibility.PUBLIC: - # can always see public comments from previous rounds - if comment.review_turn < self.review_turn: - comments.append((comment, metadata)) - elif current_phase == ReviewTurnPhase.CONSOLIDATING: - if can_see_review_comments: - comments.append((comment, metadata)) - case CommentVisibility.PRIVATE: - if can_see_review_comments: - # can see private comments from previous round - if comment.review_turn < self.review_turn: - comments.append((comment, metadata)) - # we have completed independent review stage, so output - # checker's can see private comments - elif current_phase != ReviewTurnPhase.INDEPENDENT: - comments.append((comment, metadata)) - case _: # pragma: nocover - assert False + comments.append((comment, html_class)) return comments @@ -1001,20 +1058,20 @@ def get_turn_phase(self) -> ReviewTurnPhase: def get_writable_comment_visibilities_for_user( self, user: User - ) -> list[CommentVisibility]: + ) -> list[Visibility]: """What comment visibilities should this user be able to write for this request?""" is_author = user.username == self.author # author can only ever create public comments if is_author: - return [CommentVisibility.PUBLIC] + return [Visibility.PUBLIC] # non-author non-output-checker, also only public if not user.output_checker: - return [CommentVisibility.PUBLIC] + return [Visibility.PUBLIC] # all other cases - the output-checker can choose to write public or private comments - return [CommentVisibility.PRIVATE, CommentVisibility.PUBLIC] + return [Visibility.PRIVATE, Visibility.PUBLIC] def abspath(self, relpath): """Returns abspath to the file on disk. @@ -1269,7 +1326,7 @@ def group_comment_create( request_id: str, group: str, comment: str, - visibility: CommentVisibility, + visibility: Visibility, review_turn: int, username: str, audit: AuditEvent, @@ -2200,7 +2257,7 @@ def group_comment_create( release_request: ReleaseRequest, group: str, comment: str, - visibility: CommentVisibility, + visibility: Visibility, user: User, ): if not user.output_checker and release_request.workspace not in user.workspaces: @@ -2215,6 +2272,7 @@ def group_comment_create( group=group, comment=comment, review_turn=str(release_request.review_turn), + visibility=visibility.name, ) self._dal.group_comment_create( @@ -2258,24 +2316,33 @@ def group_comment_delete( release_request.id, group, comment_id, user.username, audit ) - def get_audit_log( + def get_request_audit_log( self, - user: str | None = None, - workspace: str | None = None, - request: str | None = None, + user: User, + request: ReleaseRequest, group: str | None = None, exclude_readonly: bool = False, size: int | None = None, ) -> list[AuditEvent]: - return self._dal.get_audit_log( - user=user, - workspace=workspace, - request=request, + """Fetches the audit log for this request, filtering for what the user can see.""" + + audits = self._dal.get_audit_log( + request=request.id, group=group, exclude=READONLY_EVENTS if exclude_readonly else set(), size=size, ) + return list( + filter_visible_items( + audits, + request.review_turn, + request.get_turn_phase(), + request.user_can_review(user), + user, + ) + ) + def audit_workspace_file_access( self, workspace: Workspace, path: UrlPath, user: User ): diff --git a/airlock/forms.py b/airlock/forms.py index d56bd62b..99b03091 100644 --- a/airlock/forms.py +++ b/airlock/forms.py @@ -1,7 +1,7 @@ from django import forms from django.forms.formsets import BaseFormSet, formset_factory -from airlock.business_logic import CommentVisibility, FileGroup, RequestFileType +from airlock.business_logic import FileGroup, RequestFileType, Visibility class ListField(forms.Field): @@ -142,9 +142,7 @@ def __init__(self, visibilities, *args, **kwargs): # filter only the supplied visibilities, as it can vary depending on # user and request state choices = [ - (k.name, v) - for k, v in CommentVisibility.choices().items() - if k in visibilities + (k.name, v) for k, v in Visibility.choices().items() if k in visibilities ] self.fields["visibility"].choices = choices # type: ignore # choose first in list as default selected value diff --git a/airlock/management/commands/audit.py b/airlock/management/commands/audit.py index a951ac6a..1ba2ed98 100644 --- a/airlock/management/commands/audit.py +++ b/airlock/management/commands/audit.py @@ -20,7 +20,7 @@ def add_arguments(self, parser): @transaction.atomic() def handle(self, *args, **options): - audit_log = bll.get_audit_log( + audit_log = bll._dal.get_audit_log( user=options["user"], workspace=options["workspace"], request=options["request"], diff --git a/airlock/templates/file_browser/group.html b/airlock/templates/file_browser/group.html index 65257b91..dc2796ef 100644 --- a/airlock/templates/file_browser/group.html +++ b/airlock/templates/file_browser/group.html @@ -27,17 +27,32 @@ {% if show_c3 %}
{% #list_group %} - {% for comment, metadata in group_comments %} + {% for comment, comment_class in group_comments %} {% fragment as comment_status %} {% if request.user.output_checker %} - {% pill variant="info" text=metadata.description %} + {% if release_request.get_turn_phase.name == "INDEPENDENT" %} + {% #pill variant="info" text="Blinded" class="group" %} + Blinded + {% comment%} + TODO: get tooltips working for pills + {% tooltip content=comment.visibility.blinded_description %} + {% endcomment%} + {% /pill%} + {% endif %} + {% #pill variant="info" class="group" %} + {{ comment.visibility.name.title }} + {% comment%} + TODO: get tooltips working for pills + {%tooltip content=comment.visibility.description %} + {% endcomment%} + {% /pill%} {% endif %} {% pill variant="info" text=comment.created_at|date:"Y-m-d H:i" %} {% endfragment %} - {% #list_group_rich_item custom_status=comment_status class=metadata.class title=comment.author %} + {% #list_group_rich_item custom_status=comment_status class=comment_class title=comment.author %} {{ comment.comment }} {% if request.user.username == comment.author %}
diff --git a/airlock/templates/file_browser/workspace.html b/airlock/templates/file_browser/workspace.html index b8d37ab8..c8c120e0 100644 --- a/airlock/templates/file_browser/workspace.html +++ b/airlock/templates/file_browser/workspace.html @@ -5,5 +5,3 @@ {% /description_item %} {% /description_list %} {% /card %} - -{% include "activity.html" %} diff --git a/airlock/views/request.py b/airlock/views/request.py index cc12d0f0..3ed18582 100644 --- a/airlock/views/request.py +++ b/airlock/views/request.py @@ -14,10 +14,10 @@ from airlock.business_logic import ( ROOT_PATH, - CommentVisibility, RequestFileType, RequestFileVote, RequestStatus, + Visibility, bll, ) from airlock.file_browser_api import get_request_tree @@ -161,7 +161,11 @@ def request_view(request, request_id: str, path: str = ""): if relpath == ROOT_PATH: # viewing the root - activity = bll.get_audit_log(request=release_request.id, exclude_readonly=True) + activity = bll.get_request_audit_log( + user=request.user, + request=release_request, + exclude_readonly=True, + ) # if we are viewing a group page, load the specific group data and forms elif len(relpath.parts) == 1: @@ -194,8 +198,11 @@ def request_view(request, request_id: str, path: str = ""): kwargs={"request_id": request_id, "group": group}, ) - group_activity = bll.get_audit_log( - request=release_request.id, group=group, exclude_readonly=True + group_activity = bll.get_request_audit_log( + user=request.user, + request=release_request, + group=group, + exclude_readonly=True, ) if not is_author: @@ -642,7 +649,7 @@ def group_comment_create(request, request_id, group): release_request, group=group, comment=form.cleaned_data["comment"], - visibility=CommentVisibility[form.cleaned_data["visibility"]], + visibility=Visibility[form.cleaned_data["visibility"]], user=request.user, ) except bll.RequestPermissionDenied as exc: # pragma: nocover diff --git a/airlock/views/workspace.py b/airlock/views/workspace.py index e5b389a5..e2760471 100644 --- a/airlock/views/workspace.py +++ b/airlock/views/workspace.py @@ -103,15 +103,8 @@ def workspace_view(request, workspace_name: str, path: str = ""): in VALID_WORKSPACEFILE_STATES_TO_ADD ) - activity = [] project = workspace.project() - # we are viewing the root, so load workspace audit log - if path == "": - activity = bll.get_audit_log( - workspace=workspace.name, exclude_readonly=True, size=20 - ) - if path_item.is_directory() or path not in workspace.manifest["outputs"]: code_url = None else: @@ -150,7 +143,6 @@ def workspace_view(request, workspace_name: str, path: str = ""): kwargs={"workspace_name": workspace_name}, ), # for workspace summary page - "activity": activity, "project": project, # for code button "code_url": code_url, diff --git a/local_db/data_access.py b/local_db/data_access.py index b32b67a4..e7fa0e37 100644 --- a/local_db/data_access.py +++ b/local_db/data_access.py @@ -5,12 +5,12 @@ AuditEvent, AuditEventType, BusinessLogicLayer, - CommentVisibility, DataAccessLayerProtocol, RequestFileType, RequestFileVote, RequestStatus, RequestStatusOwner, + Visibility, ) from airlock.types import UrlPath from local_db.models import ( @@ -418,7 +418,7 @@ def group_comment_create( request_id: str, group: str, comment: str, - visibility: CommentVisibility, + visibility: Visibility, review_turn: int, username: str, audit: AuditEvent, diff --git a/local_db/migrations/0018_filegroupcomment_visibility.py b/local_db/migrations/0018_filegroupcomment_visibility.py index 5cd87406..56323ad2 100644 --- a/local_db/migrations/0018_filegroupcomment_visibility.py +++ b/local_db/migrations/0018_filegroupcomment_visibility.py @@ -16,8 +16,8 @@ class Migration(migrations.Migration): model_name="filegroupcomment", name="visibility", field=local_db.models.EnumField( - default=airlock.business_logic.CommentVisibility.PUBLIC, - enum=airlock.business_logic.CommentVisibility, + default=airlock.business_logic.Visibility.PUBLIC, + enum=airlock.business_logic.Visibility, ), preserve_default=False, ), diff --git a/local_db/models.py b/local_db/models.py index b8861f4d..e5aa17a0 100644 --- a/local_db/models.py +++ b/local_db/models.py @@ -11,10 +11,10 @@ from airlock.business_logic import ( AuditEventType, - CommentVisibility, RequestFileType, RequestFileVote, RequestStatus, + Visibility, ) @@ -158,7 +158,7 @@ class FileGroupComment(models.Model): comment = models.TextField() author = models.TextField() # just username, as we have no User model - visibility = EnumField(enum=CommentVisibility) + visibility = EnumField(enum=Visibility) review_turn = models.IntegerField() created_at = models.DateTimeField(default=timezone.now) diff --git a/tests/integration/views/test_request.py b/tests/integration/views/test_request.py index ccc12664..29483446 100644 --- a/tests/integration/views/test_request.py +++ b/tests/integration/views/test_request.py @@ -5,11 +5,11 @@ from airlock.business_logic import ( AuditEventType, - CommentVisibility, RequestFileType, RequestFileVote, RequestStatus, RequestStatusOwner, + Visibility, bll, ) from airlock.types import UrlPath @@ -76,7 +76,6 @@ def test_request_view_root_summary(airlock_client): assert ">\n 1\n <" in response.rendered_content assert "Recent activity" in response.rendered_content assert "audit_user" in response.rendered_content - assert "Created request" in response.rendered_content def test_request_view_root_group(airlock_client, settings): @@ -101,7 +100,7 @@ def test_request_view_root_group(airlock_client, settings): release_request, group="group1", comment="private comment", - visibility=CommentVisibility.PRIVATE, + visibility=Visibility.PRIVATE, user=airlock_client.user, ) @@ -109,7 +108,6 @@ def test_request_view_root_group(airlock_client, settings): assert response.status_code == 200 assert "Recent activity" in response.rendered_content assert "audit_user" in response.rendered_content - assert "Added file" in response.rendered_content assert "private comment" in response.rendered_content @@ -498,9 +496,9 @@ def test_request_contents_file(airlock_client): response = airlock_client.get("/requests/content/id/default/file.txt") assert response.status_code == 200 assert response.content == b'
\ntest\n
\n' - audit_log = bll.get_audit_log( - user=airlock_client.user.username, - request=release_request.id, + audit_log = bll.get_request_audit_log( + user=airlock_client.user, + request=release_request, ) assert audit_log[0].type == AuditEventType.REQUEST_FILE_VIEW assert audit_log[0].path == UrlPath("default/file.txt") @@ -549,9 +547,9 @@ def test_request_download_file(airlock_client): assert response.as_attachment assert list(response.streaming_content) == [b"test"] - audit_log = bll.get_audit_log( - user=airlock_client.user.username, - request=release_request.id, + audit_log = bll.get_request_audit_log( + user=airlock_client.user, + request=release_request, ) assert audit_log[0].type == AuditEventType.REQUEST_FILE_DOWNLOAD assert audit_log[0].path == UrlPath("default/file.txt") @@ -632,9 +630,9 @@ def test_request_download_file_permissions( assert response.as_attachment assert list(response.streaming_content) == [b"test"] - audit_log = bll.get_audit_log( - user=airlock_client.user.username, - request=release_request.id, + audit_log = bll.get_request_audit_log( + user=airlock_client.user, + request=release_request, ) assert audit_log[0].type == AuditEventType.REQUEST_FILE_DOWNLOAD else: @@ -1682,10 +1680,10 @@ def test_group_edit_bad_group(airlock_client): @pytest.mark.parametrize( "output_checker,visibility,allowed", [ - (False, CommentVisibility.PUBLIC, True), - (False, CommentVisibility.PRIVATE, False), - (True, CommentVisibility.PUBLIC, True), - (True, CommentVisibility.PRIVATE, True), + (False, Visibility.PUBLIC, True), + (False, Visibility.PRIVATE, False), + (True, Visibility.PUBLIC, True), + (True, Visibility.PRIVATE, True), ], ) def test_group_comment_create_success( diff --git a/tests/integration/views/test_workspace.py b/tests/integration/views/test_workspace.py index e4ea0810..935db081 100644 --- a/tests/integration/views/test_workspace.py +++ b/tests/integration/views/test_workspace.py @@ -4,11 +4,9 @@ from django.urls import reverse from airlock.business_logic import ( - AuditEventType, Project, RequestFileType, RequestStatus, - bll, ) from airlock.types import UrlPath from tests import factories @@ -36,18 +34,11 @@ def test_workspace_view_summary(airlock_client): ) workspace = factories.create_workspace("workspace") factories.write_workspace_file(workspace, "file.txt") - # create audit event to appear on activity - factories.create_release_request( - workspace, user=factories.create_user("audit_user") - ) response = airlock_client.get("/workspaces/view/workspace/") assert "file.txt" in response.rendered_content assert "release-request-button" not in response.rendered_content assert "TESTPROJECT" in response.rendered_content - assert "Recent activity" in response.rendered_content - assert "audit_user" in response.rendered_content - assert "Created request" in response.rendered_content def test_workspace_view_archived_inactive(airlock_client): @@ -171,7 +162,7 @@ def test_workspace_view_with_file(airlock_client): (RequestStatus.SUBMITTED), ], ) -def test_workspace_view_with_updated_file(airlock_client, request_status): +def test_workspace_view_with_updated_file(bll, airlock_client, request_status): author = factories.create_user("author", workspaces=["test-workspace"]) airlock_client.login_with_user(author) @@ -449,12 +440,6 @@ def test_workspace_contents_file(airlock_client): response = airlock_client.get("/workspaces/content/workspace/file.txt") assert response.status_code == 200 assert response.content == b'
\ntest\n
\n' - audit = bll.get_audit_log( - user=airlock_client.user.username, - workspace="workspace", - ) - assert audit[0].type == AuditEventType.WORKSPACE_FILE_VIEW - assert audit[0].path == UrlPath("file.txt") def test_workspace_contents_dir(airlock_client): diff --git a/tests/local_db/test_data_access.py b/tests/local_db/test_data_access.py index 6a3de484..c0487f89 100644 --- a/tests/local_db/test_data_access.py +++ b/tests/local_db/test_data_access.py @@ -4,8 +4,8 @@ AuditEvent, AuditEventType, BusinessLogicLayer, - CommentVisibility, RequestStatus, + Visibility, ) from airlock.types import UrlPath from local_db import data_access, models @@ -166,7 +166,7 @@ def test_group_comment_delete_bad_params(): release_request.id, "group", "author comment", - CommentVisibility.PUBLIC, + Visibility.PUBLIC, release_request.review_turn, author, audit, diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py index 20b1bd11..f7a998b2 100644 --- a/tests/unit/test_business_logic.py +++ b/tests/unit/test_business_logic.py @@ -1,6 +1,7 @@ import hashlib import inspect import json +from dataclasses import dataclass from hashlib import file_digest from io import BytesIO from pathlib import Path @@ -16,16 +17,20 @@ AuditEventType, BusinessLogicLayer, CodeRepo, - CommentVisibility, + Comment, DataAccessLayerProtocol, NotificationEventType, + ReleaseRequest, RequestFileDecision, RequestFileType, RequestFileVote, RequestStatus, + ReviewTurnPhase, + Visibility, Workspace, ) from airlock.types import UrlPath, WorkspaceFileStatus +from airlock.users import User from tests import factories @@ -639,7 +644,7 @@ def test_provider_request_release_files(mock_old_api, mock_notifications, bll, f ] assert [event["event_type"] for event in request_json] == expected_notifications - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) expected_audit_logs = [ # create request AuditEventType.REQUEST_CREATE, @@ -941,12 +946,13 @@ def test_provider_get_or_create_current_request_for_user(bll): assert release_request.workspace == "workspace" assert release_request.author == user.username - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log == [ - AuditEvent.from_request( - release_request, - AuditEventType.REQUEST_CREATE, - user=user, + AuditEvent( + type=AuditEventType.REQUEST_CREATE, + user=user.username, + workspace=workspace.name, + request=release_request.id, ) ] @@ -1003,12 +1009,13 @@ def test_provider_get_current_request_for_former_user(bll): assert release_request.workspace == "workspace" assert release_request.author == user.username - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log == [ - AuditEvent.from_request( - release_request, - AuditEventType.REQUEST_CREATE, - user=user, + AuditEvent( + type=AuditEventType.REQUEST_CREATE, + user=user.username, + workspace="workspace", + request=release_request.id, ) ] @@ -1215,7 +1222,7 @@ def test_set_status(current, future, valid_author, valid_checker, withdrawn_afte if valid_author: bll.set_status(release_request1, future, user=author) assert release_request1.status == future - audit_log = bll.get_audit_log(request=release_request1.id) + audit_log = bll._dal.get_audit_log(request=release_request1.id) assert audit_log[0].type == audit_type assert audit_log[0].user == author.username assert audit_log[0].request == release_request1.id @@ -1227,7 +1234,7 @@ def test_set_status(current, future, valid_author, valid_checker, withdrawn_afte if valid_checker: bll.set_status(release_request2, future, user=checker) assert release_request2.status == future - audit_log = bll.get_audit_log(request=release_request2.id) + audit_log = bll._dal.get_audit_log(request=release_request2.id) assert audit_log[0].type == audit_type assert audit_log[0].user == checker.username assert audit_log[0].request == release_request2.id @@ -1602,7 +1609,7 @@ def test_add_file_to_request_states(status, success, bll): bll.add_file_to_request(release_request, path, author) assert release_request.abspath("default" / path).exists() - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[0] == AuditEvent.from_request( release_request, AuditEventType.REQUEST_FILE_ADD, @@ -1803,7 +1810,7 @@ def test_update_file_to_request_states( assert release_request.abspath("group" / path).exists() - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[0] == AuditEvent.from_request( release_request, AuditEventType.REQUEST_FILE_UPDATE, @@ -1869,7 +1876,7 @@ def test_withdraw_file_from_request_pending(bll): bll.withdraw_file_from_request(release_request, "group" / path1, user=author) - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[0] == AuditEvent.from_request( release_request, AuditEventType.REQUEST_FILE_WITHDRAW, @@ -1882,7 +1889,7 @@ def test_withdraw_file_from_request_pending(bll): bll.withdraw_file_from_request(release_request, "group" / path2, user=author) - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[0] == AuditEvent.from_request( release_request, AuditEventType.REQUEST_FILE_WITHDRAW, @@ -1917,7 +1924,7 @@ def test_withdraw_file_from_request_returned(bll): RequestFileType.WITHDRAWN, ] - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[0] == AuditEvent.from_request( release_request, AuditEventType.REQUEST_FILE_WITHDRAW, @@ -2148,12 +2155,71 @@ def setup_empty_release_request(): return release_request, path, author -def test_get_visible_comments_for_group(bll): +@dataclass +class VisibleItemsHelper: + """Helper class to make assertions about visiblity of comments and audit logs. + + It will fetch comments and audits that are visible a specific request and + user, and store them to make assertions about. + """ + + comments: list[Comment] + audits: list[AuditEvent] + + @classmethod + def for_group( + cls, request: ReleaseRequest, user: User, group, bll: BusinessLogicLayer + ): + return cls( + comments=request.get_visible_comments_for_group(group, user), + audits=bll.get_request_audit_log(user, request, group), + ) + + def is_comment_visible(self, text: str, author: User) -> bool: + for c, _ in self.comments: + if c.comment == text and c.author == author.username: + return True + + return False + + def is_audit_visible(self, type_: AuditEventType, author: User) -> bool: + for audit in self.audits: + if audit.type == type_ and audit.user == author.username: + return True + + return False + + def comment(self, text: str, author: User) -> bool: + """Is this comment in the list of visible items we have?""" + if not self.is_comment_visible(text=text, author=author): + return False + + return self.is_audit_visible( + type_=AuditEventType.REQUEST_COMMENT, author=author + ) + + def vote(self, vote: RequestFileVote, author: User) -> bool: + """Is this vote in the list of visible items we have?""" + match vote: + case RequestFileVote.APPROVED: + event_type = AuditEventType.REQUEST_FILE_APPROVE + case RequestFileVote.REJECTED: + event_type = AuditEventType.REQUEST_FILE_REJECT + case _: # pragma: nocover + assert False + + return self.is_audit_visible(type_=event_type, author=author) + + +def test_request_comment_and_audit_visibility(bll): # This test is long and complex. # - # It walks through a couple of rounds of back and forth review, validating - # that the comments that are visibile to various users at different points - # in the process are correct. + # It tests both get_visible_comments_for_group() and + # get_request_audit_log(), and uses the custom VisibleItems helper above. + # + # It walks through a couple of rounds and turns of back and forth review, + # validating that the comments that are visibile to various users at + # different points in the process are correct. # author = factories.create_user("author1", workspaces=["workspace"]) checkers = factories.get_default_output_checkers() @@ -2162,226 +2228,365 @@ def test_get_visible_comments_for_group(bll): "workspace", status=RequestStatus.SUBMITTED, author=author, - files=[factories.request_file(group="group", approved=True)], + files=[factories.request_file(group="group", path="file.txt", approved=True)], ) bll.group_comment_create( release_request, "group", "turn 1 checker 0 private", - CommentVisibility.PRIVATE, + Visibility.PRIVATE, checkers[0], ) bll.group_comment_create( release_request, "group", "turn 1 checker 1 private", - CommentVisibility.PRIVATE, + Visibility.PRIVATE, checkers[1], ) - # helper function to fetch the current comments for a user - def get_comments(user): - nonlocal release_request - release_request = factories.refresh_release_request(release_request) - return [ - c[0].comment - for c in release_request.get_visible_comments_for_group("group", user) - ] + release_request = factories.refresh_release_request(release_request) - # in RequestPhase.INDEPENDENT, can only see own comments - assert get_comments(checkers[0]) == ["turn 1 checker 0 private"] - assert get_comments(checkers[1]) == ["turn 1 checker 1 private"] - assert get_comments(author) == [] assert release_request.review_turn == 1 + assert release_request.get_turn_phase() == ReviewTurnPhase.INDEPENDENT + + def get_visible_items(user): + return VisibleItemsHelper.for_group(release_request, user, "group", bll) + + # in ReviewTurnPhase.INDEPENDENT, checkers can only see own comments, author can see nothing + visible = get_visible_items(checkers[0]) + assert visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert not visible.comment("turn 1 checker 1 private", checkers[1]) + assert not visible.vote(RequestFileVote.APPROVED, checkers[1]) + + visible = get_visible_items(checkers[1]) + assert not visible.comment("turn 1 checker 0 private", checkers[0]) + assert not visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + + visible = get_visible_items(author) + assert not visible.comment("turn 1 checker 0 private", checkers[0]) + assert not visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert not visible.comment("turn 1 checker 1 private", checkers[1]) + assert not visible.vote(RequestFileVote.APPROVED, checkers[1]) factories.submit_independent_review(release_request) - bll.group_comment_create( release_request, "group", "turn 1 checker 0 public", - CommentVisibility.PUBLIC, + Visibility.PUBLIC, checkers[0], ) + release_request = factories.refresh_release_request(release_request) - # in RequestPhase.CONSOLIDATING, checkers should see all private comments - # and pending public comments, but author should not see any yet - assert get_comments(checkers[0]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - ] - assert get_comments(checkers[1]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - ] - assert get_comments(author) == [] assert release_request.review_turn == 1 + assert release_request.get_turn_phase() == ReviewTurnPhase.CONSOLIDATING - bll.return_request(release_request, checkers[0]) - - # in RequestPhase.COMPLETE, checkers should see all private comments + # in ReviewTurnPhase.CONSOLIDATING, checkers should see all private comments # and pending public comments, but author should not see any yet - assert get_comments(checkers[0]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - ] - assert get_comments(checkers[1]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - ] - assert get_comments(author) == ["turn 1 checker 0 public"] - assert release_request.review_turn == 2 + for checker in checkers: + visible = get_visible_items(checker) + assert visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + + # author still cannot see anything + visible = get_visible_items(author) + assert not visible.comment("turn 1 checker 0 private", checkers[0]) + assert not visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert not visible.comment("turn 1 checker 1 private", checkers[1]) + assert not visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert not visible.comment("turn 1 checker 0 public", checkers[0]) + bll.return_request(release_request, checkers[0]) + release_request = factories.refresh_release_request(release_request) bll.group_comment_create( release_request, "group", "turn 2 author public", - CommentVisibility.PUBLIC, + Visibility.PUBLIC, author, ) + release_request = factories.refresh_release_request(release_request) - # in RequestPhase.AUTHOR, checkers should see all public/private comments - # they've made, but not any the author has made, as it hasn't yet been - # returned. - assert get_comments(checkers[0]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - ] - assert get_comments(checkers[1]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - ] - assert get_comments(author) == ["turn 1 checker 0 public", "turn 2 author public"] + assert release_request.review_turn == 2 + assert release_request.get_turn_phase() == ReviewTurnPhase.AUTHOR + + # in ReviewTurnPhase.AUTHOR, checkers should see turn 1 comments, but not authors turn 2 comments. + # Author should turn 1 and 2 public comments + for checker in checkers: + visible = get_visible_items(checker) + assert visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + assert not visible.comment("turn 2 author public", author) + + # author can see al turn 1 public comments and votes, their turn 2 public comments, but no private comments. + visible = get_visible_items(author) + assert not visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert not visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + assert visible.comment("turn 2 author public", author) bll.submit_request(release_request, author) release_request = factories.refresh_release_request(release_request) + assert release_request.review_turn == 3 bll.group_comment_create( release_request, "group", "turn 3 checker 0 private", - CommentVisibility.PRIVATE, + Visibility.PRIVATE, checkers[0], ) bll.group_comment_create( release_request, "group", "turn 3 checker 1 private", - CommentVisibility.PRIVATE, + Visibility.PRIVATE, checkers[1], ) + # checker0 rejects the file now. Not realistic, but we want to check that + # the audit log for later round votes is hidden + bll.reject_file( + release_request, + release_request.get_request_file_from_output_path("file.txt"), + checkers[0], + ) + release_request = factories.refresh_release_request(release_request) - # in RequestPhase.INDEPENDENT for a 2nd round + # in ReviewTurnPhase.INDEPENDENT for a 2nd round # Checkers should see previous round's private comments, but not this rounds # Author should see previous round's public comments, but not any this round - assert get_comments(checkers[0]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - "turn 2 author public", - "turn 3 checker 0 private", - ] - assert get_comments(checkers[1]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - "turn 2 author public", - "turn 3 checker 1 private", - ] - assert get_comments(author) == ["turn 1 checker 0 public", "turn 2 author public"] - assert release_request.review_turn == 3 + visible = get_visible_items(checkers[0]) + assert visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + assert visible.comment("turn 2 author public", author) + assert visible.comment("turn 3 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.REJECTED, checkers[0]) + assert not visible.comment("turn 3 checker 1 private", checkers[1]) + + visible = get_visible_items(checkers[1]) + assert visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + assert visible.comment("turn 2 author public", author) + assert not visible.comment("turn 3 checker 0 private", checkers[0]) + assert not visible.vote(RequestFileVote.REJECTED, checkers[0]) + assert visible.comment("turn 3 checker 1 private", checkers[1]) + + visible = get_visible_items(author) + assert not visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert not visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + assert visible.comment("turn 2 author public", author) + assert not visible.comment("turn 3 checker 0 private", checkers[0]) + assert not visible.vote(RequestFileVote.REJECTED, checkers[0]) + assert not visible.comment("turn 3 checker 1 private", checkers[1]) factories.submit_independent_review(release_request) + release_request = factories.refresh_release_request(release_request) + bll.group_comment_create( + release_request, + "group", + "turn 3 checker 0 public", + Visibility.PUBLIC, + checkers[0], + ) + release_request = factories.refresh_release_request(release_request) - # in RequestPhase.CONSOLIDATING for a 2nd round + # in ReviewTurnPhase.CONSOLIDATING for a 2nd round # Checkers should see previous and current round's private comments, # Author should see previous round's public comments, but not any private comments - assert get_comments(checkers[0]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - "turn 2 author public", - "turn 3 checker 0 private", - "turn 3 checker 1 private", - ] - assert get_comments(checkers[1]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - "turn 2 author public", - "turn 3 checker 0 private", - "turn 3 checker 1 private", - ] - assert get_comments(author) == ["turn 1 checker 0 public", "turn 2 author public"] + for checker in checkers: + visible = get_visible_items(checker) + assert visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + assert visible.comment("turn 2 author public", author) + assert visible.comment("turn 3 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.REJECTED, checkers[0]) + assert visible.comment("turn 3 checker 1 private", checkers[1]) + assert visible.comment("turn 3 checker 0 public", checkers[0]) + + # author sees no private comments, and no turn 3 things. + visible = get_visible_items(author) + assert not visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert not visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + assert visible.comment("turn 2 author public", author) + assert not visible.comment("turn 3 checker 0 private", checkers[0]) + assert not visible.vote(RequestFileVote.REJECTED, checkers[0]) + assert not visible.comment("turn 3 checker 1 private", checkers[1]) + assert not visible.comment("turn 3 checker 0 public", checkers[0]) + + # reject the request + bll.set_status(release_request, RequestStatus.REJECTED, checkers[0]) + release_request = factories.refresh_release_request(release_request) + # no increment, as there was no return to author + assert release_request.review_turn == 3 + assert release_request.get_turn_phase() == ReviewTurnPhase.COMPLETE + + # COMPLETE has special handling - test it works + # checkers can see all things + for checker in checkers: + visible = get_visible_items(checker) + assert visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + assert visible.comment("turn 2 author public", author) + assert visible.comment("turn 3 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.REJECTED, checkers[0]) + assert visible.comment("turn 3 checker 1 private", checkers[1]) + assert visible.comment("turn 3 checker 0 public", checkers[0]) + + # Author should see all public comments, regardless of round, but not any private comments + visible = get_visible_items(author) + assert not visible.comment("turn 1 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.APPROVED, checkers[0]) + assert not visible.comment("turn 1 checker 1 private", checkers[1]) + assert visible.vote(RequestFileVote.APPROVED, checkers[1]) + assert visible.comment("turn 1 checker 0 public", checkers[0]) + assert visible.comment("turn 2 author public", author) + assert not visible.comment("turn 3 checker 0 private", checkers[0]) + assert visible.vote(RequestFileVote.REJECTED, checkers[0]) + assert not visible.comment("turn 3 checker 1 private", checkers[1]) + assert visible.comment("turn 3 checker 0 public", checkers[0]) + + +def test_get_visible_comments_for_group_class(bll): + author = factories.create_user("author", workspaces=["workspace"]) + checkers = factories.get_default_output_checkers() - # assume returned + release_request = factories.create_request_at_status( + "workspace", + status=RequestStatus.SUBMITTED, + author=author, + files=[factories.request_file(group="group", path="file.txt", approved=True)], + ) bll.group_comment_create( release_request, "group", - "turn 3 checker 0 public", - CommentVisibility.PUBLIC, + "turn 1 checker 0 private", + Visibility.PRIVATE, checkers[0], ) - - assert get_comments(checkers[0]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - "turn 2 author public", - "turn 3 checker 0 private", - "turn 3 checker 1 private", - "turn 3 checker 0 public", - ] - assert get_comments(checkers[1]) == [ - "turn 1 checker 0 private", + bll.group_comment_create( + release_request, + "group", "turn 1 checker 1 private", + Visibility.PRIVATE, + checkers[1], + ) + bll.group_comment_create( + release_request, + "group", "turn 1 checker 0 public", - "turn 2 author public", - "turn 3 checker 0 private", - "turn 3 checker 1 private", - "turn 3 checker 0 public", - ] - assert get_comments(author) == ["turn 1 checker 0 public", "turn 2 author public"] + Visibility.PUBLIC, + checkers[0], + ) + + release_request = factories.refresh_release_request(release_request) + assert release_request.review_turn == 1 + assert release_request.get_turn_phase() == ReviewTurnPhase.INDEPENDENT + + def get_comment_metadata(user): + return [ + m for _, m in release_request.get_visible_comments_for_group("group", user) + ] + + assert get_comment_metadata(checkers[0]) == ["comment_blinded", "comment_blinded"] + assert get_comment_metadata(checkers[1]) == ["comment_blinded"] + assert get_comment_metadata(author) == [] + + factories.submit_independent_review(release_request) + release_request = factories.refresh_release_request(release_request) + assert release_request.review_turn == 1 + assert release_request.get_turn_phase() == ReviewTurnPhase.CONSOLIDATING + + for checker in checkers: + assert get_comment_metadata(checker) == [ + "comment_private", + "comment_private", + "comment_public", + ] + + assert get_comment_metadata(author) == [] bll.return_request(release_request, checkers[0]) + release_request = factories.refresh_release_request(release_request) + assert release_request.review_turn == 2 + assert release_request.get_turn_phase() == ReviewTurnPhase.AUTHOR - # in RequestPhase.COMPLETE for a 2nd round - # Checkers should see all historic comments, - # Author should see previous round's public comments, but not any private comments - assert get_comments(checkers[0]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - "turn 2 author public", - "turn 3 checker 0 private", - "turn 3 checker 1 private", - "turn 3 checker 0 public", - ] - assert get_comments(checkers[1]) == [ - "turn 1 checker 0 private", - "turn 1 checker 1 private", - "turn 1 checker 0 public", - "turn 2 author public", + for checker in checkers: + assert get_comment_metadata(checker) == [ + "comment_private", + "comment_private", + "comment_public", + ] + + assert get_comment_metadata(author) == ["comment_public"] + + bll.submit_request(release_request, author) + release_request = factories.refresh_release_request(release_request) + assert release_request.review_turn == 3 + assert release_request.get_turn_phase() == ReviewTurnPhase.INDEPENDENT + + bll.group_comment_create( + release_request, + "group", "turn 3 checker 0 private", + Visibility.PRIVATE, + checkers[0], + ) + bll.group_comment_create( + release_request, + "group", "turn 3 checker 1 private", - "turn 3 checker 0 public", + Visibility.PRIVATE, + checkers[1], + ) + + release_request = factories.refresh_release_request(release_request) + + # comments from previous round are visible + assert get_comment_metadata(checkers[0]) == [ + "comment_private", + "comment_private", + "comment_public", + "comment_blinded", ] - assert get_comments(author) == [ - "turn 1 checker 0 public", - "turn 2 author public", - "turn 3 checker 0 public", + assert get_comment_metadata(checkers[1]) == [ + "comment_private", + "comment_private", + "comment_public", + "comment_blinded", ] - assert release_request.review_turn == 4 - def test_release_request_filegroups_with_no_files(bll): release_request, _, _ = setup_empty_release_request() @@ -2602,7 +2807,7 @@ def test_approve_file(bll): == RequestFileDecision.INCOMPLETE ) - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[0] == AuditEvent.from_request( release_request, AuditEventType.REQUEST_FILE_APPROVE, @@ -2682,7 +2887,7 @@ def test_request_changes_to_file(bll): == RequestFileDecision.INCOMPLETE ) - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[0] == AuditEvent.from_request( release_request, AuditEventType.REQUEST_FILE_REQUEST_CHANGES, @@ -3116,7 +3321,7 @@ def test_group_edit_author(bll): assert release_request.filegroups["group"].context == "foo" assert release_request.filegroups["group"].controls == "bar" - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[0].request == release_request.id assert audit_log[0].type == AuditEventType.REQUEST_EDIT assert audit_log[0].user == author.username @@ -3227,10 +3432,10 @@ def test_group_comment_create_success( # check all visibilities bll.group_comment_create( - release_request, "group", "private", CommentVisibility.PRIVATE, checker + release_request, "group", "private", Visibility.PRIVATE, checker ) bll.group_comment_create( - release_request, "group", "public", CommentVisibility.PUBLIC, author + release_request, "group", "public", Visibility.PUBLIC, author ) release_request = factories.refresh_release_request(release_request) @@ -3244,14 +3449,14 @@ def test_group_comment_create_success( comments = release_request.filegroups["group"].comments assert comments[0].comment == "private" - assert comments[0].visibility == CommentVisibility.PRIVATE + assert comments[0].visibility == Visibility.PRIVATE assert comments[0].author == "checker" assert comments[1].comment == "public" - assert comments[1].visibility == CommentVisibility.PUBLIC + assert comments[1].visibility == Visibility.PUBLIC assert comments[1].author == "author" - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[1].request == release_request.id assert audit_log[1].type == AuditEventType.REQUEST_COMMENT @@ -3283,18 +3488,18 @@ def test_group_comment_create_permissions(bll): with pytest.raises(bll.RequestPermissionDenied): bll.group_comment_create( - release_request, "group", "question?", CommentVisibility.PUBLIC, other + release_request, "group", "question?", Visibility.PUBLIC, other ) bll.group_comment_create( - release_request, "group", "collaborator", CommentVisibility.PUBLIC, collaborator + release_request, "group", "collaborator", Visibility.PUBLIC, collaborator ) release_request = factories.refresh_release_request(release_request) assert len(release_request.filegroups["group"].comments) == 1 bll.group_comment_create( - release_request, "group", "checker", CommentVisibility.PUBLIC, checker + release_request, "group", "checker", Visibility.PUBLIC, checker ) release_request = factories.refresh_release_request(release_request) @@ -3314,10 +3519,10 @@ def test_group_comment_delete_success(bll): assert release_request.filegroups["group"].comments == [] bll.group_comment_create( - release_request, "group", "typo comment", CommentVisibility.PUBLIC, other + release_request, "group", "typo comment", Visibility.PUBLIC, other ) bll.group_comment_create( - release_request, "group", "not-a-typo comment", CommentVisibility.PUBLIC, other + release_request, "group", "not-a-typo comment", Visibility.PUBLIC, other ) release_request = factories.refresh_release_request(release_request) @@ -3338,7 +3543,7 @@ def test_group_comment_delete_success(bll): assert current_comment.comment == "not-a-typo comment" assert current_comment.author == "other" - audit_log = bll.get_audit_log(request=release_request.id) + audit_log = bll._dal.get_audit_log(request=release_request.id) assert audit_log[2].request == release_request.id assert audit_log[2].type == AuditEventType.REQUEST_COMMENT assert audit_log[2].user == other.username @@ -3373,10 +3578,10 @@ def test_group_comment_delete_permissions(bll): ) bll.group_comment_create( - release_request, "group", "author comment", CommentVisibility.PUBLIC, author + release_request, "group", "author comment", Visibility.PUBLIC, author ) bll.group_comment_create( - release_request, "group", "checker comment", CommentVisibility.PUBLIC, checker + release_request, "group", "checker comment", Visibility.PUBLIC, checker ) release_request = factories.refresh_release_request(release_request) @@ -3416,7 +3621,7 @@ def test_group_comment_create_invalid_params(bll): bll.group_comment_delete(release_request, "group", 1, author) bll.group_comment_create( - release_request, "group", "author comment", CommentVisibility.PUBLIC, author + release_request, "group", "author comment", Visibility.PUBLIC, author ) release_request = factories.refresh_release_request(release_request)