Skip to content

Commit

Permalink
feat: allow peer reviews to be closed
Browse files Browse the repository at this point in the history
analogous to closing on github

serves two purposes:
1. de-cluttering the dashboard, particularly in the case of legacy
   reviews on public releases that stall when changes are requested and
   a new release has to be made (resolves comses/planning#32)
2. allowing an author/requester to close inactive reviews since we now
   only allow one active review per codebase open at one time

also includes some small refactors/bugfixes (resolves comses/planning#172)
  • Loading branch information
sgfost committed Sep 29, 2023
1 parent 57ae229 commit 3244d0e
Show file tree
Hide file tree
Showing 17 changed files with 308 additions and 75 deletions.
2 changes: 2 additions & 0 deletions django/core/jinja2/core/member_profiles/retrieve.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@
<span class="badge bg-gray">Review complete</span>
{% elif not invitation.accepted %}
<span class="badge bg-primary">Declined review request</span>
{% elif invitation.review.closed %}
<span class="badge bg-danger">Review closed</span>
{% else %}
{% set badge_class="bg-warning" if invitation.review.is_awaiting_reviewer_feedback else "bg-info" %}
<span class="badge {{badge_class}}">{{ invitation.review.get_status_display() }}</span>
Expand Down
1 change: 1 addition & 0 deletions django/library/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ class Meta:


class PeerReviewFilterForm(forms.Form):
include_closed = forms.BooleanField(required=False)
requires_editor_input = forms.BooleanField(required=False)
author_changes_requested = forms.BooleanField(required=False)
reviewer_feedback_requested = forms.BooleanField(required=False)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% extends "sidebar_layout.jinja" %}
{% from "common.jinja" import breadcrumb, embed_discourse_comments, share_card, search_tag_href, member_profile_href %}
{% from "library/review/includes/macros.jinja" import include_review_reminders %}
{% from "library/review/includes/macros.jinja" import include_review_reminders, confirm_change_closed_modal %}

{% set open_code_badge_url = request.build_absolute_uri(static("images/icons/open-code-badge.png")) %}
{% set codebase = release.codebase %}
Expand Down Expand Up @@ -442,7 +442,14 @@
<button type="submit" class="btn btn-danger my-1 w-100">Notify reviewers of changes</button>
</form>
{% elif review is not none %}
<h4 class="mt-3 text-muted">Review Status:</h4>
<button type="button" class="btn btn-warning my-1 w-100" disabled>{{ review.get_status_display() }}</button>
{% if not review.is_complete %}
<form method="post" action="{{ review.get_change_closed_url() }}">
{{ csrf_input }}
{{ confirm_change_closed_modal(review, show_reopen=False) }}
</form>
{% endif %}
{% endif %}
{% endwith %}
{# end has_change_perm #}
Expand Down
8 changes: 7 additions & 1 deletion django/library/jinja2/library/review/dashboard.jinja
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{% extends 'sidebar_layout.jinja' %}
{% from "library/review/includes/macros.jinja" import display_closed_status %}

{% block introduction %}
<h1>Peer Review Editor Dashboard</h1>
Expand All @@ -13,7 +14,12 @@
</h4>
{% for release in codebase.releases.all() %}
{% set review = release.review %}
<h5>Review: <a href="{{ review.get_absolute_url() }}">{{ release.version_number }}</a></h5>
<h5>
Review: <a href="{{ review.get_absolute_url() }}">
{{ display_closed_status(review) }}
{{ release.version_number }}
</a>
</h5>
<p class='card-text'>
{% set badge_class="bg-success" if review.is_complete else "bg-warning" %}
<div class='row'>
Expand Down
15 changes: 14 additions & 1 deletion django/library/jinja2/library/review/editor_update.jinja
Original file line number Diff line number Diff line change
@@ -1,12 +1,24 @@
{% extends 'base.jinja' %}
{% from 'common.jinja' import format_date %}
{% from "library/review/includes/macros.jinja" import display_closed_status, confirm_change_closed_modal %}

{% block introduction %}
<h1>CoMSES Net Peer Review Editor</h1>
{% endblock %}

{% block content %}
<h2>Peer review for <a href="{{ review.codebase_release.share_url }}">{{ review.title }}</a></h2>
<div class="d-flex justify-content-between">
<h2>
Peer review for <a href="{{ review.codebase_release.share_url }}">{{ review.title }}</a>
{{ display_closed_status(review) }}
</h2>
{% if not review.is_complete %}
<form method="post" action="{{ review.get_change_closed_url() }}">
{{ csrf_input }}
{{ confirm_change_closed_modal(review, show_reopen=True) }}
</form>
{% endif %}
</div>
<p class='lead'>
Requested by <a href="{{ review.submitter.get_absolute_url() }}">{{ review.submitter.name }}</a>
{% if review.is_complete %}
Expand All @@ -19,6 +31,7 @@
<div id="review-editor" data-review-id="{{ review.slug }}"
data-status-levels="{{ review.review_status_json }}"
data-status="{{ review.status }}"
data-closed="{{ review.closed }}"
</div>
{% endblock %}

Expand Down
44 changes: 44 additions & 0 deletions django/library/jinja2/library/review/includes/macros.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,50 @@
fas {% if q %}fa-check text-success{% else %}fa-times text-danger{% endif %}
{% endmacro %}

{% macro display_closed_status(review) %}
{% if review.is_complete %}
<i class="far fa-check-circle text-success" title="complete"></i>
{% elif review.closed %}
<i class="far fa-times-circle text-danger" title="closed"></i>
{% else %}
<i class="far fa-dot-circle text-secondary" title="open"></i>
{% endif %}
{% endmacro %}

{% macro confirm_change_closed_modal(review, show_reopen=False) %}
{% set action = "reopen" if review.closed else "close" %}
{% if not review.closed %}
<button type="button" data-bs-toggle='modal' data-bs-target='#confirm-review-change-closed' class='btn btn-danger my-1 w-100'>
Close review
</button>
{% elif show_reopen %}
<button type="button" data-bs-toggle='modal' data-bs-target='#confirm-review-change-closed' class='btn btn-success my-1 w-100'>
Re-open review
</button>
{% endif %}
<div class='modal fade' id='confirm-review-change-closed' tabindex='-1' role='dialog'
aria-labelledby='confirm-review-change-closed-label' aria-hidden='true'>
<div class='modal-dialog modal-dialog-centered modal-dialog-scrollable' role='document'>
<div class='modal-content'>
<div class='modal-header'>
<h4 class='modal-title' id='confirm-review-change-closed-label'>
Review for {{ review.codebase_release.codebase.title }} v{{ review.codebase_release.version_number }}
</h4>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close">
</button>
</div>
<div class='modal-body'>
Are you sure you wish to {{ action }} this review?
</div>
<div class='modal-footer'>
<button type="button" class="btn btn-outline-gray" data-bs-dismiss="modal">Cancel</button>
<button type="submit" name="action" value="{{ action }}" class="btn btn-primary">Confirm</button>
</div>
</div>
</div>
</div>
{% endmacro %}

{% macro display_reviewer_feedback(feedback, include_private=false, include_editor=false) %}
<h1>Reviewer Comments</h1>
<p>
Expand Down
2 changes: 1 addition & 1 deletion django/library/migrations/0024_add_release_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ class Migration(migrations.Migration):
choices=[
("draft", "Draft"),
("under_review", "Under review"),
("review_complete", "Review complete"),
("published", "Published"),
("unpublished", "Unpublished"),
("review_complete", "Review complete"),
],
default="draft",
help_text="The current status of this codebase release.",
Expand Down
45 changes: 45 additions & 0 deletions django/library/migrations/0025_add_peerreview_closed.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Generated by Django 3.2.20 on 2023-09-29 17:18

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("library", "0024_add_release_status"),
]

operations = [
migrations.AddField(
model_name="peerreview",
name="closed",
field=models.BooleanField(default=False),
),
migrations.AlterField(
model_name="peerrevieweventlog",
name="action",
field=models.CharField(
choices=[
("invitation_sent", "Reviewer has been invited"),
("invitation_resent", "Reviewer invitation has been resent"),
("invitation_accepted", "Reviewer has accepted invitation"),
("invitation_declined", "Reviewer has declined invitation"),
("reviewer_feedback_submitted", "Reviewer has given feedback"),
("author_resubmitted", "Author has resubmitted release for review"),
("review_status_updated", "Editor manually changed review status"),
(
"revisions_requested",
"Editor has requested revisions to this release",
),
(
"release_certified",
"Editor has taken reviewer feedback into account and certified this release as peer reviewed",
),
("review_closed", "Peer review was closed"),
("review_reopened", "Peer review was reopened"),
],
help_text="status action requested.",
max_length=32,
),
),
]
Loading

0 comments on commit 3244d0e

Please sign in to comment.