From cec0e0c9d8f8c843c28724979ec2a1c5c357d5ed Mon Sep 17 00:00:00 2001 From: Tero Kivinen Date: Wed, 20 Mar 2024 20:04:37 -0400 Subject: [PATCH] Make review settings history usable. (#7205) --- ietf/doc/templatetags/ietf_filters.py | 18 +++++++++++ ietf/review/policies.py | 32 ++++++++++++------- .../group/change_reviewer_settings.html | 8 ++++- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/ietf/doc/templatetags/ietf_filters.py b/ietf/doc/templatetags/ietf_filters.py index cfed7aa1db..a791aad383 100644 --- a/ietf/doc/templatetags/ietf_filters.py +++ b/ietf/doc/templatetags/ietf_filters.py @@ -881,3 +881,21 @@ def badgeify(blob): ) return text + +@register.filter +def simple_history_delta_changes(history): + """Returns diff between given history and previous entry.""" + prev = history.prev_record + if prev: + delta = history.diff_against(prev) + return delta.changes + return [] + +@register.filter +def simple_history_delta_change_cnt(history): + """Returns number of changes between given history and previous entry.""" + prev = history.prev_record + if prev: + delta = history.diff_against(prev) + return len(delta.changes) + return 0 diff --git a/ietf/review/policies.py b/ietf/review/policies.py index 6738db95ff..91398a1b24 100644 --- a/ietf/review/policies.py +++ b/ietf/review/policies.py @@ -131,12 +131,15 @@ def _update_skip_next(self, rotation_pks, assignee_person): assignee_index = rotation_pks.index(assignee_person.pk) skipped = rotation_pks[0:assignee_index] skipped_settings = self.team.reviewersettings_set.filter(person__in=skipped) # list of PKs is valid here + changed = [] for ss in skipped_settings: - ss.skip_next = max(0, ss.skip_next - 1) # ensure we don't go negative - bulk_update_with_history(skipped_settings, + if ss.skip_next > 0: + ss.skip_next = max(0, ss.skip_next - 1) # ensure we don't go negative + ss._change_reason = "Skip count decremented" + changed.append(ss) + bulk_update_with_history(changed, ReviewerSettings, - ['skip_next'], - default_change_reason='skipped') + ['skip_next']) def _assignment_in_order(self, rotation_pks, assignee_person): """Is this an in-order assignment?""" @@ -262,12 +265,15 @@ def _filter_unavailable_reviewers(self, reviewers, review_req=None): def _clear_request_next_assignment(self, person): s = self._reviewer_settings_for(person) - s.request_assignment_next = False - s.save() + if s.request_assignment_next: + s.request_assignment_next = False + s._change_reason = "Clearing request next assignment" + s.save() def _add_skip(self, person): s = self._reviewer_settings_for(person) s.skip_next += 1 + s._change_reason = "Incrementing skip count" s.save() def _reviewer_settings_for(self, person): @@ -484,6 +490,7 @@ def set_wants_to_be_next(self, reviewer_person): # Instead, the "assign me next" flag is set. settings = self._reviewer_settings_for(reviewer_person) settings.request_assignment_next = True + settings._change_reason = "Setting request next assignment" settings.save() def _update_skip_next(self, rotation_pks, assignee_person): @@ -523,20 +530,22 @@ def _update_skip_next(self, rotation_pks, assignee_person): min_skip_next = min([rs.skip_next for rs in rotation_settings.values()]) next_reviewer_index = None + changed = [] for index, pk in enumerate(unfolded_rotation_pks): rs = rotation_settings.get(pk) if (rs is None) or (rs.skip_next == min_skip_next): next_reviewer_index = index break else: - rs.skip_next = max(0, rs.skip_next - 1) # ensure never negative + if rs.skip_next > 0: + rs.skip_next = max(0, rs.skip_next - 1) # ensure never negative + rs._change_reason = "Skip count decremented" + changed.append(rs) log.assertion('next_reviewer_index is not None') # some entry in the list must have the minimum value - - bulk_update_with_history(rotation_settings.values(), + bulk_update_with_history(changed, ReviewerSettings, - ['skip_next'], - default_change_reason='skipped') + ['skip_next']) next_reviewer_pk = unfolded_rotation_pks[next_reviewer_index] NextReviewerInTeam.objects.update_or_create( @@ -578,6 +587,7 @@ def set_wants_to_be_next(self, reviewer_person): # who rejected a review and no further action is needed. settings = self._reviewer_settings_for(reviewer_person) settings.request_assignment_next = True + settings._change_reason = "Setting request next assignment" settings.save() diff --git a/ietf/templates/group/change_reviewer_settings.html b/ietf/templates/group/change_reviewer_settings.html index d18d523bab..9ecec5633c 100644 --- a/ietf/templates/group/change_reviewer_settings.html +++ b/ietf/templates/group/change_reviewer_settings.html @@ -105,11 +105,17 @@

History of settings

{% if reviewersettings.history.all %} {% for h in reviewersettings.history.all %} + {% if h|simple_history_delta_change_cnt > 0 or h.history_change_reason != "skipped" and h.history_change_reason %} {{ h.history_date|date }} {% person_link h.history_user.person %} - {{ h.history_change_reason }} + {% if h.history_change_reason != "skipped" and h.history_change_reason %} {{ h.history_change_reason }}
{% endif %} + {% for change in h|simple_history_delta_changes %} + {{ change.field }} changed from "{{change.old}}" to "{{change.new}}"
+ {% endfor %} + + {% endif %} {% endfor %} {% endif %}