Skip to content

Commit

Permalink
Disable complete button until ready, show messages
Browse files Browse the repository at this point in the history
A review can only be completed once all files have been reviewed;
hide the Complete review button until then.

Also show a reminder message if a user has reviewed all files but
has not pressed the button to complete their review yet.
  • Loading branch information
rebkwok committed Jun 26, 2024
1 parent 0d2eb0f commit cb82c8a
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 4 deletions.
7 changes: 6 additions & 1 deletion airlock/templates/file_browser/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,16 @@
Complete review
{% tooltip class="airlock-tooltip" content="You have already completed your review" %}
{% /button %}
{% else %}
{% elif user_has_reviewed_all_files %}
<form action="{{ request_review_url }}" method="POST">
{% csrf_token %}
{% #button type="submit" class="btn-sm" tooltip="Complete Review" variant="secondary" id="complete-review-button" %}Complete review{% /button %}
</form>
{% else %}
{% #button disabled=True class="relative group btn-sm" variant="secondary" id="complete-review-button" %}
Complete review
{% tooltip class="airlock-tooltip" content="You must review all files before you can complete your review" %}
{% /button %}
{% endif %}
{% endif %}
{% /airlock_header %}
Expand Down
16 changes: 14 additions & 2 deletions airlock/views/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,18 @@ def request_view(request, request_id: str, path: str = ""):
request=release_request.id, group=group, exclude_readonly=True, size=20
)

user_has_completed_review = (
request.user.username in release_request.completed_reviews
)
user_has_reviewed_all_files = release_request.all_files_reviewed_by_reviewer(
request.user
)

if user_has_reviewed_all_files and not user_has_completed_review:
messages.success(
request, "All files reviewed. Your review can now be completed."
)

request_submit_url = reverse(
"request_submit",
kwargs={"request_id": request_id},
Expand Down Expand Up @@ -236,8 +248,8 @@ def request_view(request, request_id: str, path: str = ""):
"request_return_url": request_return_url,
"request_withdraw_url": request_withdraw_url,
"release_files_url": release_files_url,
"user_has_completed_review": request.user.username
in release_request.completed_reviews,
"user_has_completed_review": user_has_completed_review,
"user_has_reviewed_all_files": user_has_reviewed_all_files,
"activity": activity,
"group_edit_form": group_edit_form,
"group_edit_url": group_edit_url,
Expand Down
13 changes: 13 additions & 0 deletions tests/functional/test_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,11 @@ def test_e2e_release_files(
find_and_click(page.locator("#outstanding-requests").get_by_role("link"))
expect(page.locator("body")).to_contain_text(request_id)

complete_review_button = page.locator("#complete-review-button")
# output checker hasn't reviewed files yet, complete review button visible but disabled
expect(complete_review_button).to_be_visible()
expect(complete_review_button).to_be_disabled()

# Reuse the locators from the workspace view to click on filegroup and then file
# Click to open the filegroup tree
find_and_click(filegroup_link)
Expand All @@ -360,13 +365,21 @@ def test_e2e_release_files(
find_and_click(page.locator("#file-reject-button"))
expect(page.locator("#file-reset-button")).to_have_attribute("aria-pressed", "true")

# output checker has now reviewed all output files
expect(complete_review_button).to_be_visible()
expect(complete_review_button).to_be_enabled()

# Change our minds & remove the review
expect(page.locator("#file-reset-button")).to_have_attribute("aria-pressed", "true")
find_and_click(page.locator("#file-reset-button"))
expect(page.locator("#file-reject-button")).to_have_attribute(
"aria-pressed", "false"
)

# complete review button disabled again
expect(complete_review_button).to_be_visible()
expect(complete_review_button).to_be_disabled()

# Think some more & finally approve the file
expect(page.locator("#file-approve-button")).to_have_attribute(
"aria-pressed", "false"
Expand Down
14 changes: 13 additions & 1 deletion tests/integration/views/test_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -678,14 +678,26 @@ def test_request_review_output_checker(airlock_client):
status=RequestStatus.SUBMITTED,
files=[factories.request_file(approved=True, checkers=[airlock_client.user])],
)

response = airlock_client.get(release_request.get_url())
# Files have been reviewed but review has not been completed yet
assert "All files reviewed" in list(response.context["messages"])[0].message

response = airlock_client.post(
f"/requests/review/{release_request.id}", follow=True
)

assert response.status_code == 200
persisted_request = factories.refresh_release_request(release_request)
assert persisted_request.status == RequestStatus.PARTIALLY_REVIEWED
assert "Your review has been completed" in response.rendered_content
assert (
"Your review has been completed"
in list(response.context["messages"])[0].message
)

response = airlock_client.get(release_request.get_url())
# Reminder message no longer shown now that review is complete
assert list(response.context["messages"]) == []


def test_request_review_non_output_checker(airlock_client):
Expand Down

0 comments on commit cb82c8a

Please sign in to comment.