Fix #2590 - Allow secretary to adjust date when completing a review.

This also fixes other issues identified in #2590, around the
modification of historical document events. The behaviour is now:
- When the assigned reviewer posts a review, a single event is
  created, set to current date/time.
- When the secretary records a review in the datatracker, they may
  set a different completion date, which is autofilled if an email
  is selected. One event is generated for the original completion
  date, and one for the secretary's action.
- Each revision generates a new event, rather than updating previous
  existing events.
 - Legacy-Id: 16670
This commit is contained in:
Sasha Romijn 2019-08-26 12:58:31 +00:00
parent de9cde9e43
commit 3942f9acc7
2 changed files with 85 additions and 12 deletions

View file

@ -688,12 +688,17 @@ class ReviewTests(TestCase):
"review_content": "This is a review\nwith two lines",
"review_url": "",
"review_file": "",
# Custom completion should be ignored - review posted by assignee is always set to now
"completion_date": "2012-12-24",
"completion_time": "12:13:14",
})
self.assertEqual(r.status_code, 302)
assignment = reload_db_objects(assignment)
self.assertEqual(assignment.state_id, "completed")
self.assertNotEqual(assignment.completed_on, None)
# Completed time should be close to now, but will not be exactly, so check within 10s margin
completed_time_diff = datetime.datetime.now() - assignment.completed_on
self.assertLess(completed_time_diff, datetime.timedelta(seconds=10))
with io.open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f:
self.assertEqual(f.read(), "This is a review\nwith two lines")
@ -704,6 +709,47 @@ class ReviewTests(TestCase):
self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in assignment.review.external_url)
def test_complete_review_enter_content_by_secretary(self):
assignment, url = self.setup_complete_review_test()
login_testing_unauthorized(self, 'reviewsecretary', url)
empty_outbox()
r = self.client.post(url, data={
"result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="ready").pk,
"state": ReviewAssignmentStateName.objects.get(slug="completed").pk,
"reviewed_rev": assignment.review_request.doc.rev,
"review_submission": "enter",
"review_content": "This is a review\nwith two lines",
"review_url": "",
"review_file": "",
"completion_date": "2012-12-24",
"completion_time": "12:13:14",
})
self.assertEqual(r.status_code, 302)
# The secretary is allowed to set a custom completion date (#2590)
assignment = reload_db_objects(assignment)
self.assertEqual(assignment.state_id, "completed")
self.assertEqual(assignment.completed_on, datetime.datetime(2012, 12, 24, 12, 13, 14))
# There should be two events:
# - the event logging when the change when it was entered, i.e. very close to now.
# - the completion of the review, set to the provided date/time
events = ReviewAssignmentDocEvent.objects.filter(doc=assignment.review_request.doc).order_by('-time')
event0_time_diff = datetime.datetime.now() - events[0].time
self.assertLess(event0_time_diff, datetime.timedelta(seconds=10))
self.assertEqual(events[1].time, datetime.datetime(2012, 12, 24, 12, 13, 14))
with io.open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f:
self.assertEqual(f.read(), "This is a review\nwith two lines")
self.assertEqual(len(outbox), 1)
self.assertTrue(assignment.review_request.team.list_email in outbox[0]["To"])
self.assertTrue("This is a review" in outbox[0].get_payload(decode=True).decode("utf-8"))
self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in assignment.review.external_url)
def test_complete_notify_ad_because_team_settings(self):
assignment, url = self.setup_complete_review_test()
assignment.review_request.team.reviewteamsettings.notify_ad_when.add(ReviewResultName.objects.get(slug='issues'))
@ -879,8 +925,11 @@ class ReviewTests(TestCase):
assignment = reload_db_objects(assignment)
self.assertEqual(assignment.state_id, "completed")
event = ReviewAssignmentDocEvent.objects.get(type="closed_review_assignment", review_assignment=assignment)
self.assertEqual(event.time, datetime.datetime(2012, 12, 24, 12, 13, 14))
# The revision event time should be the date the revision was submitted, i.e. not backdated
event1 = assignment.review_request.doc.latest_event(ReviewAssignmentDocEvent)
event_time_diff = datetime.datetime.now() - event1.time
self.assertLess(event_time_diff, datetime.timedelta(seconds=10))
self.assertTrue('revised' in event1.desc.lower())
with io.open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f:
self.assertEqual(f.read(), "This is a review\nwith two lines")
@ -904,8 +953,11 @@ class ReviewTests(TestCase):
assignment = reload_db_objects(assignment)
self.assertEqual(assignment.review.rev, "01")
event = ReviewAssignmentDocEvent.objects.get(type="closed_review_assignment", review_assignment=assignment)
self.assertEqual(event.time, datetime.datetime(2013, 12, 24, 11, 11, 11))
event2 = assignment.review_request.doc.latest_event(ReviewAssignmentDocEvent)
event_time_diff = datetime.datetime.now() - event2.time
self.assertLess(event_time_diff, datetime.timedelta(seconds=10))
# Ensure that a new event was created for the new revision (#2590)
self.assertNotEqual(event1.id, event2.id)
self.assertEqual(len(outbox), 0)

View file

@ -520,7 +520,7 @@ class CompleteReviewForm(forms.Form):
if revising_review:
del self.fields["cc"]
else:
elif is_reviewer:
del self.fields["completion_date"]
del self.fields["completion_time"]
@ -647,6 +647,7 @@ def complete_review(request, name, assignment_id):
need_to_email_review = review_submission != "link" and assignment.review_request.team.list_email and not revising_review
submitted_on_different_date = completion_datetime.date() != datetime.date.today()
desc = "Request for {} review by {} {}: {}. Reviewer: {}.".format(
assignment.review_request.type.name,
assignment.review_request.team.acronym.upper(),
@ -656,18 +657,38 @@ def complete_review(request, name, assignment_id):
)
if need_to_email_review:
desc += " " + "Sent review to list."
close_event = ReviewAssignmentDocEvent.objects.filter(type="closed_review_assignment", review_assignment=assignment).first()
if not close_event:
close_event = ReviewAssignmentDocEvent(type="closed_review_assignment", review_assignment=assignment)
if revising_review:
desc += " Review has been revised by {}.".format(request.user.person)
elif submitted_on_different_date:
desc += " Submission of review completed at an earlier date."
close_event = ReviewAssignmentDocEvent(type="closed_review_assignment", review_assignment=assignment)
close_event.doc = assignment.review_request.doc
close_event.rev = assignment.review_request.doc.rev
close_event.by = request.user.person
close_event.desc = desc
close_event.state = assignment.state
close_event.time = completion_datetime
close_event.time = datetime.datetime.now()
close_event.save()
# If the completion date is different, record when the initial review was made too.
if not revising_review and submitted_on_different_date:
desc = "Request for {} review by {} {}: {}. Reviewer: {}.".format(
assignment.review_request.type.name,
assignment.review_request.team.acronym.upper(),
assignment.state.name,
assignment.result.name,
assignment.reviewer.person,
)
initial_close_event = ReviewAssignmentDocEvent(type="closed_review_assignment",
review_assignment=assignment)
initial_close_event.doc = assignment.review_request.doc
initial_close_event.rev = assignment.review_request.doc.rev
initial_close_event.by = request.user.person
initial_close_event.desc = desc
initial_close_event.state = assignment.state
initial_close_event.time = completion_datetime
initial_close_event.save()
if assignment.state_id == "part-completed" and not revising_review:
existing_assignments = ReviewAssignment.objects.filter(review_request__doc=assignment.review_request.doc, review_request__team=assignment.review_request.team, state__in=("assigned", "accepted", "completed"))