From c9a5381bb5676bc6057abc9b6245178fd94a0c9c Mon Sep 17 00:00:00 2001
From: George Katsikas <giorgakis.katsikas@gmail.com>
Date: Mon, 21 Oct 2024 12:50:23 +0200
Subject: [PATCH] various fixes in assignment update email
fix appraisals count not showing
hide appraisal qualification count
prefer accepting offers of least overloaded fellow
skip deprecated test
---
scipost_django/scipost/models.py | 5 +++++
.../commands/send_assignment_stage_update.py | 1 +
.../submissions/models/assignment.py | 11 -----------
.../submissions/tests/test_models.py | 4 ++++
scipost_django/submissions/views/__init__.py | 18 ++++++++++++++++++
.../update_authors_assignment_stage.html | 4 ++--
6 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/scipost_django/scipost/models.py b/scipost_django/scipost/models.py
index e42cb6241..8c2f2b01a 100644
--- a/scipost_django/scipost/models.py
+++ b/scipost_django/scipost/models.py
@@ -43,8 +43,10 @@ from conflicts.models import ConflictOfInterest
today = timezone.now().date()
if TYPE_CHECKING:
+ from django.db.models.manager import RelatedManager
from django.contrib.auth.models import User
from profiles.models import Profile
+ from colleges.models.fellowship import Fellowship
def get_sentinel_user():
@@ -119,6 +121,9 @@ class Contributor(models.Model):
objects = ContributorQuerySet.as_manager()
+ if TYPE_CHECKING:
+ fellowships: "RelatedManager[Fellowship]"
+
class Meta:
ordering = ["user__last_name", "user__first_name"]
diff --git a/scipost_django/submissions/management/commands/send_assignment_stage_update.py b/scipost_django/submissions/management/commands/send_assignment_stage_update.py
index 109190f8e..5c53b2eee 100644
--- a/scipost_django/submissions/management/commands/send_assignment_stage_update.py
+++ b/scipost_django/submissions/management/commands/send_assignment_stage_update.py
@@ -48,6 +48,7 @@ class Command(BaseCommand):
f"authors/update_authors_assignment_stage",
submission=submission,
weeks_passed=weeks_passed,
+ should_include_appraisals=weeks_passed == 1,
weeks_until_assignment_deadline=weeks_until_assignment_deadline,
default_assignment_period_weeks=default_assignment_period_weeks,
)
diff --git a/scipost_django/submissions/models/assignment.py b/scipost_django/submissions/models/assignment.py
index 28b0a1b49..80ecf13e8 100644
--- a/scipost_django/submissions/models/assignment.py
+++ b/scipost_django/submissions/models/assignment.py
@@ -349,17 +349,6 @@ class ConditionalAssignmentOffer(models.Model):
if self.status != self.STATUS_OFFERED:
raise ValueError("The offer has already been processed.")
- # Guard that the current offer is the earliest instance of all identical offers
- # for this submission. Only the offering person may be different.
- identical_offers = ConditionalAssignmentOffer.objects.filter(
- submission=self.submission,
- condition_type=self.condition_type,
- condition_details=self.condition_details,
- status=self.STATUS_OFFERED,
- )
- if identical_offers.order_by("offered_on").first() != self:
- raise ValueError("The offer is not the earliest instance of its kind.")
-
self.condition.accept(offer=self)
self.status = self.STATUS_ACCEPTED
diff --git a/scipost_django/submissions/tests/test_models.py b/scipost_django/submissions/tests/test_models.py
index fcd293e88..8e6502e96 100644
--- a/scipost_django/submissions/tests/test_models.py
+++ b/scipost_django/submissions/tests/test_models.py
@@ -1,6 +1,7 @@
__copyright__ = "Copyright © Stichting SciPost (SciPost Foundation)"
__license__ = "AGPL v3"
+from unittest import skip
from common.faker import fake
from django.test import TestCase
@@ -57,6 +58,8 @@ class TestConditionalAssignmentOfferAcceptance(TestCase):
self.offer.accept(self.offer.submission.submitted_by)
self.offer.finalize()
+ self.offer.refresh_from_db()
+
# Creates an editorial assignment for the self.offering fellow
self.assertIsNotNone(
self.offer.submission.editorial_assignments.filter(to=self.offer.offered_by)
@@ -115,6 +118,7 @@ class TestConditionalAssignmentOfferAcceptanceFailure(TestCase):
with self.assertRaises(ValueError):
offer.accept(offer.submission.submitted_by)
+ @skip("This test should be moved to a view instead")
def test_cannot_accept_later_identical_offer(self):
offer = ConditionalAssignmentOfferFactory(
offered_until=fake.aware.date_time_this_year(
diff --git a/scipost_django/submissions/views/__init__.py b/scipost_django/submissions/views/__init__.py
index 55eaf4927..5e1d61aa3 100644
--- a/scipost_django/submissions/views/__init__.py
+++ b/scipost_django/submissions/views/__init__.py
@@ -667,6 +667,9 @@ def extend_assignment_deadline(
def accept_conditional_assignment_offer(request, identifier_w_vn_nr, offer_id):
"""
Accept a conditional assignment offer and redirect to the Submission's detail page.
+
+ The offer_id is used to determine the TYPE of offer to accept.
+ The accepted offer may actually be different in order to balance the load of the referees.
"""
submission = get_object_or_404(
Submission, preprint__identifier_w_vn_nr=identifier_w_vn_nr
@@ -681,6 +684,20 @@ def accept_conditional_assignment_offer(request, identifier_w_vn_nr, offer_id):
if request.user.contributor not in submission.authors.all():
raise PermissionDenied("Only verified authors can accept conditional offers.")
+ # Find all identical offers and sort them by the fellow's ongoing assignments
+ # Select the offer with the least ongoing assignments, or the oldest if equal
+ identical_offers = ConditionalAssignmentOffer.objects.filter(
+ submission=submission,
+ condition_type=offer.condition_type,
+ condition_details=offer.condition_details,
+ status=ConditionalAssignmentOffer.STATUS_OFFERED,
+ ).order_by("offered_on")
+ sorted_offers = sorted(
+ identical_offers,
+ key=lambda x: x.offered_by.editorial_assignments.ongoing().count(),
+ )
+ offer = sorted_offers[0]
+
try:
offer.accept(by=request.user.contributor)
offer.finalize()
@@ -1145,6 +1162,7 @@ def editorial_workflow(request):
"""
return render(request, "submissions/editorial_workflow.html")
+
@login_required
@permission_required("scipost.can_assign_submissions", raise_exception=True)
@transaction.atomic
diff --git a/scipost_django/templates/email/authors/update_authors_assignment_stage.html b/scipost_django/templates/email/authors/update_authors_assignment_stage.html
index 05348663a..8f88ab962 100644
--- a/scipost_django/templates/email/authors/update_authors_assignment_stage.html
+++ b/scipost_django/templates/email/authors/update_authors_assignment_stage.html
@@ -10,9 +10,9 @@
Your Submission is in the "seeking assignment" stage, where unfortunately no Editor-in-charge has yet volunteered to take charge of your Submission. At SciPost, Editorial Fellows choose which Submissions they wish to handle according to their interests, expertise and availability. While in this stage, your Submission will be visible to all Fellows, who may choose to take charge at any time. If this occurs, you will be immediately informed and the refereeing process will begin. Otherwise, we will keep searching for an Editor-in-charge and keep you informed of any new developments.
</p>
-{% if weeks_passed == 1 and submission.qualification_set.exists %}
+{% if should_include_appraisals and submission.qualification_set.exists %}
<p>
- Over the past week, your submission has been examined by {{ submission.qualification_set.all|length }} Fellow{{ submission.qualification_set.all|pluralize }}, of which {{ submission.qualification_set.qualified|length }} {{ submission.qualification_set.qualified|pluralize:"is,are" }} sufficiently qualified to take charge.
+ Over the past week, your submission has been examined by {{ submission.qualification_set.all|length }} Fellow{{ submission.qualification_set.all|pluralize }}, but there was no volunteer among them willing to take charge.
</p>
{% endif %}
--
GitLab