The system should not suggest Telechat reviews based on rejected Last Call reviews

[ietf-svn-bot]

owner:mark@painless-security.com resolution_fixed type_defect | by kivinen@iki.fi

---

I had one review twice in the unassigned reviews list, and I think it was there because there was last call assignment which was rejected by the reviewer, and the document was then reassigned to someone else, and I think that created two different review requests. Now when it moved to telechat both of them was shown in the unassigned reviews list. When I did assignment it suggested for the first one that it should be closed because "team will not review this version" status (as there was no need to rereview it), and for the other one it proposed "overtaken by events" reason for not reviewing the document.

---

Issue migrated from trac:2996 at 2022-03-04 07:52:08 +0000

[ietf-svn-bot]

@kivinen@iki.fi uploaded file Screenshot from 2020-06-04 15-12-43.png (44.6 KiB)

[ietf-svn-bot]

@kivinen@iki.fi uploaded file Screenshot from 2020-06-04 15-20-04.png (82.6 KiB)

[ietf-svn-bot]

@rjsparks@nostrum.com changed status from new to accepted

[ietf-svn-bot]

@rjsparks@nostrum.com changed title from Duplicate review requests when someone rejects request to The system should not suggest Telechat reviews based on rejected Last Call reviews

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

This is close to "as designed". It is correct for there to be multiple ReviewRequests when more than one person is asked to review a document. The flaw here is that you had a suggestion to assign a telechat review based on a last call review that was rejected. The suggestion code should not include that.

I'm changing the summary field of the ticket to reflect that.

[ietf-svn-bot]

@mark@painless-security.com changed status from accepted to assigned

[ietf-svn-bot]

@mark@painless-security.com changed owner from `` to mark@painless-security.com

[ietf-svn-bot]

@mark@painless-security.com commented

---

I see that the case here is that the ReviewRequest was in a state of 'requested', while the corresponding ReviewAssignment was in a state of 'rejected'. Are there any other ReviewAssignment states that should indicate that the item should be filtered out?

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

The set is

In d1306762b59cd4e43691ddbc579d88eb3e83647b: for name in ReviewAssignmentStateName.objects.all():
    ...:     print (name.slug, name)
    ...:
accepted Accepted
assigned Assigned
completed Completed
no-response No Response
overtaken Overtaken By Events
part-completed Partially Completed
rejected Rejected
unknown Unknown
withdrawn Withdrawn by Team

It would also be reasonable to filter out no-response. In all the other cases, the secretary should get the suggestion and decide what to do with it.

Just to make sure the request here is clear - the filtering is on who to suggest, not whether to suggest. The document should still show up as a suggestion to review, but it should not suggest a reviewer that has rejected a previous review assignment on the document nor one that has failed to respond to such a previous assignment.

[ietf-svn-bot]

@mark@painless-security.com commented

---

I'll admit that I'm confused by the request, then. I'm looking at the security directorate review requests page (http://localhost:8000/group/secdir/reviews/) I don't see anywhere on the unassigned review requests area (as pictured in the first screenshot) where the system would suggest a reviewer. What do you have in mind?

I'll be sure to include a no-response in my list of states to ignore.

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

If you go to /group/secdir/reviews/manage/unassigned/ (or pick any review team), in the list of possible things to assign, there will be buttons labelled "Not yet assigned". If you click them, it suggests reviewers to assign it that document to. There is an algorithm for determining who to suggest. It includes "did this person review the document before". The place to work on is in that algorithm, teaching it to not treat a rejected review assignment as "reviewed the document before".

I'll send email with a screenshot showing some key elements.

[ietf-svn-bot]

@mark@painless-security.com commented

---

Should there be a timeout for how long a rejection should last? I can imagine that a rejection might be based on, "OMG, a tree just fell over onto my car," or that it might be "I have a conflict of interest on this document." In the former case, the rejection should time out. In the latter case, the rejection should never time out.

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

We don't need code for the exceptional condition that a reviewer would want to accept the later assignment. Usually what would happen is that the rejected last call assignment would have been reassigned to and completed by someone else, and that person would do the telechat review (specifically, that's who we want the tool to suggest do the telechat review - and it already will).

If someone really did want to be back in the review path, they can work that out directly with the team secretary.

[ietf-svn-bot]

@mark@painless-security.com changed status from assigned to closed

[ietf-svn-bot]

@mark@painless-security.com changed resolution from `` to fixed