Group edit code rejects ReviewAssignments far too aggressively.

[ietf-svn-bot]

resolution_fixed type_defect | by rjsparks@nostrum.com

---

The code at https://trac.ietf.org/trac/ietfdb/browser/tags/7.32.0/ietf/group/views.py#L971 does several wrong things.

• It doesn't restrict what it does to the group being operated on - as it sits, being removed from one review team will case assignments in all other review teams to be rejected (or marked no-response).

• It doesn't handle the case where someone had two 'reviewer' roles for the same group, but with different email addresses (such things existing is a bug in itself, but it happened in production). It should check that the person attached to a removed role doesn't have some other reviewer role before operating on assignments.

• It may have a bug in the codepath that leads to rejection vs no-response - some of Dan Romascanu's assignments that were automatically rejected were not yet overdue.

rejection vs no-response probably needs to be rethought anyhow.

---

Issue migrated from trac:3320 at 2022-03-04 08:44:23 +0000

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

It makes more sense for requests that are not overdue to move to 'withdrawn' rather than 'rejected' since it is the group secretary (or the secretariat) performing the change to the group.

[ietf-svn-bot]

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

[ietf-svn-bot]

@rjsparks@nostrum.com set resolution to fixed

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

Fixed in a32d00a:

When removing reviewers from a review team, change the state of only that team's assignments. Check to make sure the person being removed doesn't have another reviewer Role (with a different Email) in the same team. Change the disposition of any assignments from that team for the removed reviewer to 'withdrawn' rather than 'rejected'. Fixes #3320. Commit ready for merge.

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

Fixed in 01cd13b:

Merged in a32d00a from rjsparks@nostrum.com:
When removing reviewers from a review team, change the state of only that team's assignments. Check to make sure the person being removed doesn't have another reviewer Role (with a different Email) in the same team. Change the disposition of any assignments from that team for the removed reviewer to 'withdrawn' rather than 'rejected'. Fixes #3320.

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

fwiw, the fix also handled the case when someone was removed from some other role but not the reviewer role. The tests cover this but should be tweaked to make this more obvious.