{% set badge_class="bg-success" if review.is_complete else "bg-warning" %}
Requested by {{ review.submitter.name }} {% if review.is_complete %} @@ -19,6 +31,7 @@
diff --git a/django/library/migrations/0024_add_release_status.py b/django/library/migrations/0024_add_release_status.py
index 9bd8e5417..4430181d6 100644
--- a/django/library/migrations/0024_add_release_status.py
+++ b/django/library/migrations/0024_add_release_status.py
@@ -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.",
diff --git a/django/library/migrations/0025_add_peerreview_closed.py b/django/library/migrations/0025_add_peerreview_closed.py
new file mode 100644
index 000000000..733519c1b
--- /dev/null
+++ b/django/library/migrations/0025_add_peerreview_closed.py
@@ -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,
+ ),
+ ),
+ ]
diff --git a/django/library/models.py b/django/library/models.py
index 6911c94b0..75a49dde7 100644
--- a/django/library/models.py
+++ b/django/library/models.py
@@ -13,7 +13,7 @@
from django.core.files.images import ImageFile
from django.core.files.storage import FileSystemStorage
from django.db import models, transaction
-from django.db.models import Prefetch, Q
+from django.db.models import Prefetch, Q, Count, Max
from django.utils.functional import cached_property
from django.urls import reverse
from django.utils import timezone
@@ -383,6 +383,31 @@ def with_contributors(self, release_contributor_qs=None, user=None, **kwargs):
)
)
+ def with_reviews(self, reviews):
+ return (
+ self.filter(releases__review__in=reviews)
+ .prefetch_related(
+ Prefetch(
+ "releases",
+ queryset=CodebaseRelease.objects.filter(review__in=reviews)
+ .select_related("review")
+ .annotate(
+ n_accepted_invites=Count(
+ "review__invitation_set",
+ filter=Q(review__invitation_set__accepted=True),
+ )
+ ),
+ )
+ )
+ .annotate(
+ min_n_accepted_invites=Count(
+ "releases__review__invitation_set",
+ filter=Q(releases__review__invitation_set__accepted=True),
+ )
+ )
+ .annotate(max_last_modified=Max("releases__review__last_modified"))
+ )
+
@staticmethod
def cache_contributors(codebases):
"""Add all_contributors property to all codebases in queryset.
@@ -1247,7 +1272,8 @@ def get_notify_reviewers_of_changes_url(self):
)
def get_review(self):
- return getattr(self, "review", None)
+ review = getattr(self, "review", None)
+ return review if review and not review.closed else None
def get_review_status_display(self):
review = self.get_review()
@@ -1738,6 +1764,8 @@ class PeerReviewEvent(models.TextChoices):
RELEASE_CERTIFIED = "release_certified", _(
"Editor has taken reviewer feedback into account and certified this release as peer reviewed"
)
+ REVIEW_CLOSED = "review_closed", _("Peer review was closed")
+ REVIEW_REOPENED = "review_reopened", _("Peer review was reopened")
class PeerReviewQuerySet(models.QuerySet):
@@ -1760,6 +1788,36 @@ def find_candidate_reviewers(self, query=None):
return get_search_queryset(query, queryset)
return queryset
+ def review_open(self, **kwargs):
+ return self.filter(closed=False, **kwargs)
+
+ def requires_editor_input(self):
+ return self.annotate(
+ n_accepted_invites=Count(
+ "invitation_set", filter=Q(invitation_set__accepted=True)
+ )
+ ).filter(
+ Q(n_accepted_invites=0) | Q(status=ReviewStatus.AWAITING_EDITOR_FEEDBACK)
+ )
+
+ def author_changes_requested(self):
+ return self.filter(status=ReviewStatus.AWAITING_AUTHOR_CHANGES)
+
+ def reviewer_feedback_requested(self):
+ return self.filter(status=ReviewStatus.AWAITING_REVIEWER_FEEDBACK)
+
+ def with_filters(self, query_params):
+ queryset = self
+ if not query_params.get("include_closed"):
+ queryset = queryset.review_open()
+ if query_params.get("requires_editor_input"):
+ queryset = queryset.requires_editor_input()
+ if query_params.get("author_changes_requested"):
+ queryset = queryset.author_changes_requested()
+ if query_params.get("reviewer_feedback_requested"):
+ queryset = queryset.reviewer_feedback_requested()
+ return queryset
+
def completed(self, **kwargs):
return self.filter(status=ReviewStatus.COMPLETE, **kwargs)
@@ -1779,6 +1837,7 @@ class PeerReview(models.Model):
help_text=_("The current status of this review."),
max_length=32,
)
+ closed = models.BooleanField(default=False)
codebase_release = models.OneToOneField(
CodebaseRelease, related_name="review", on_delete=models.PROTECT
)
@@ -1793,6 +1852,10 @@ class PeerReview(models.Model):
FieldPanel("status"),
]
+ @property
+ def is_open(self):
+ return not self.closed
+
@property
def is_complete(self):
return self.status == ReviewStatus.COMPLETE
@@ -1811,6 +1874,9 @@ def get_simplified_status_display(self):
def get_edit_url(self):
return reverse("library:profile-edit", kwargs={"user_pk": self.user.pk})
+ def get_change_closed_url(self):
+ return reverse("library:peer-review-change-closed", kwargs={"slug": self.slug})
+
def get_invite(self, member_profile):
return self.invitation_set.filter(candidate_reviewer=member_profile).first()
@@ -1840,15 +1906,7 @@ def set_complete_status(self, editor: MemberProfile):
self.codebase_release.save()
self.codebase_release.codebase.peer_reviewed = True
self.codebase_release.codebase.save()
-
- # FIXME: consider moving this into explicit send_model_certified_email()
- send_markdown_email(
- subject="Peer review completed",
- template_name="library/review/email/model_certified.jinja",
- context={"review": self},
- to=[self.submitter.email],
- cc=[settings.REVIEW_EDITOR_EMAIL],
- )
+ self.send_model_certified_email()
def log(self, message: str, action: PeerReviewEvent, author: MemberProfile):
return self.event_set.create(message=message, action=action.name, author=author)
@@ -1865,12 +1923,37 @@ def editor_change_review_status(self, editor: MemberProfile, status: ReviewStatu
def author_resubmitted_changes(self, changes_made=None):
author = self.submitter
self.log(
- message="Release has been resubmitted for review: {}".format(changes_made),
+ message=f"Release has been resubmitted for review: {changes_made}",
action=PeerReviewEvent.AUTHOR_RESUBMITTED,
author=author,
)
self.send_author_updated_content_email()
+ def close(self, submitter_or_editor: MemberProfile):
+ if self.closed:
+ return
+
+ self.closed = True
+ self.log(
+ message=f"Peer review closed by {'submitter' if submitter_or_editor == self.submitter else 'editor'}",
+ action=PeerReviewEvent.REVIEW_CLOSED,
+ author=submitter_or_editor,
+ )
+ self.save()
+ # do we want to send an email when a review is closed?
+
+ def reopen(self, submitter_or_editor: MemberProfile):
+ if not self.closed:
+ return
+
+ self.closed = False
+ self.log(
+ message=f"Peer review re-opened by {'submitter' if submitter_or_editor == self.submitter else 'editor'}",
+ action=PeerReviewEvent.REVIEW_REOPENED,
+ author=submitter_or_editor,
+ )
+ self.save()
+
def send_author_updated_content_email(self):
qs = self.invitation_set.filter(accepted=True)
# if there are no currently accepted invitations, status should be set to awaiting editor feedback
@@ -1891,6 +1974,15 @@ def send_author_requested_peer_review_email(self):
to=[settings.REVIEW_EDITOR_EMAIL],
)
+ def send_model_certified_email(self):
+ send_markdown_email(
+ subject="Peer review completed",
+ template_name="library/review/email/model_certified.jinja",
+ context={"review": self},
+ to=[self.submitter.email],
+ cc=[settings.REVIEW_EDITOR_EMAIL],
+ )
+
@cached_property
def review_status_json(self):
return json.dumps(
@@ -1910,8 +2002,9 @@ def contact_author_name(self):
@classmethod
def get_codebase_latest_active_review(cls, codebase):
+ """returns the latest review for a codebase that is not complete and not closed"""
qs = cls.objects.filter(codebase_release__codebase=codebase).exclude(
- status=ReviewStatus.COMPLETE
+ Q(status=ReviewStatus.COMPLETE) | Q(closed=True)
)
return qs.latest("date_created") if qs.exists() else None
@@ -2195,7 +2288,7 @@ def reviewer_completed(self):
)
@transaction.atomic
- def editor_called_for_revisions(self):
+ def editor_called_for_revisions(self, editor: MemberProfile):
"""Add an editor called for revisions event to the log
Preconditions:
@@ -2203,7 +2296,6 @@ def editor_called_for_revisions(self):
editor updated feedback
"""
review = self.invitation.review
- editor = self.invitation.editor
review.log(
action=PeerReviewEvent.REVISIONS_REQUESTED,
author=editor,
diff --git a/django/library/tests/test_views.py b/django/library/tests/test_views.py
index b87f95fb3..100f92601 100644
--- a/django/library/tests/test_views.py
+++ b/django/library/tests/test_views.py
@@ -278,6 +278,26 @@ def test_request_peer_review_existing_review(self):
PeerReview.objects.filter(codebase_release=second_release).count(), 0
)
+ def test_request_peer_review_existing_closed_review(self):
+ self.client.login(
+ username=self.submitter.username, password=self.user_factory.password
+ )
+ first_release = ReleaseSetup.setUpPublishableDraftRelease(self.codebase)
+ pr = PeerReview.objects.create(
+ codebase_release=first_release, submitter=self.submitter.member_profile
+ )
+ pr.close(self.submitter.member_profile)
+ second_release = ReleaseSetup.setUpPublishableDraftRelease(self.codebase)
+
+ response = self.client.post(
+ second_release.get_request_peer_review_url(),
+ HTTP_ACCEPT="application/json",
+ )
+
+ self.assertTrue(
+ PeerReview.objects.filter(codebase_release=second_release).exists()
+ )
+
class CodebaseReleaseUnpublishedFilesTestCase(
ApiAccountMixin, ResponseStatusCodesMixin, TestCase
diff --git a/django/library/urls.py b/django/library/urls.py
index f020c9ab7..1cb0a60c5 100644
--- a/django/library/urls.py
+++ b/django/library/urls.py
@@ -61,6 +61,11 @@
views.PeerReviewEditorView.as_view(),
name="peer-review-detail",
),
+ path(
+ "reviews/Feedback
-