Refactor reviewer queue policy handling of "skip" setting. Fixes ietf-tools#3038. Commit ready for merge.

 - Legacy-Id: 18833

Author: jennifer-richards

ietf/review/migrations/0027_unique_constraint_for_reviewersettings.py

@@ -0,0 +1,89 @@
+# Generated by Django 2.2.17 on 2021-01-24 07:24
+
+from django.db import migrations, models
+
+
+
+def forward(apps, schema_editor):
+    """Forward migration
+    
+    Attempts to reconcile and remove duplicate ReviewerSettings
+    """
+    ReviewerSettings = apps.get_model('review', 'ReviewerSettings')
+    HistoricalReviewerSettings = apps.get_model('review', 'HistoricalReviewerSettings')
+
+    def reconcile_duplicate_settings(duplicate_settings, histories):
+        """Helper to decide how to handle duplicate settings"""
+        team = duplicate_settings[0].team
+        person = duplicate_settings[0].person
+        assert(all((s.person == person and s.team == team for s in duplicate_settings)))
+    
+        print('\n>> Found duplicate settings for {}'.format(duplicate_settings[0]))
+        # In the DB as of Dec 2020, the only duplicate settings sets were pairs where the
+        # earlier PK had a change history and the latter PK did not. Based on this, assuming
+        # a change history indicates that the settings are important. If only one has history,
+        # use that one. If multiple have a history, throw an error.
+        settings_with_history = [s for s in duplicate_settings if histories[s.pk].count() > 0]
+        if len(settings_with_history) == 0:
+            duplicate_settings.sort(key=lambda s: s.pk)
+            keep = duplicate_settings[-1]
+            reason = 'chosen by pk'
+        elif len(settings_with_history) == 1:
+            keep = settings_with_history[0]
+            reason = 'chosen because has change history'
+        else:
+            # Don't try to guess what to do if multiple settings have change histories
+            raise RuntimeError(
+                'Multiple ReviewerSettings with change history for {}. Please resolve manually.'.format(
+                    settings_with_history[0]
+                )
+            )
+            
+        print('>>    Keeping pk={} ({})'.format(keep.pk, reason))
+        for settings in duplicate_settings:
+            if settings.pk != keep.pk:
+                print('>>    Deleting pk={}'.format(settings.pk))
+                settings.delete()
+    
+    # forward migration starts here
+    if ReviewerSettings.objects.count() == 0:
+        return  # nothing to do
+
+    records = dict()  # list of records, keyed by (person_id, team_id)
+    for rs in ReviewerSettings.objects.all().order_by('pk'):
+        key = (rs.person_id, rs.team_id)
+        if key in records:
+            records[key].append(rs)
+        else:
+            records[key] = [rs]
+
+    for duplicate_settings in records.values():
+        if len(duplicate_settings) > 1:
+            histories = dict()
+            for ds in duplicate_settings:
+                histories[ds.pk] = HistoricalReviewerSettings.objects.filter(
+                    id=ds.pk
+                )
+            reconcile_duplicate_settings(duplicate_settings, histories)
+
+def reverse(apps, schema_editor):
+    """Reverse migration
+    
+    Does nothing, but no harm in reverse migration.
+    """
+    pass
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('review', '0026_repair_more_assignments'),
+    ]
+
+    operations = [
+        migrations.RunPython(forward, reverse),
+        migrations.AddConstraint(
+            model_name='reviewersettings',
+            constraint=models.UniqueConstraint(fields=('team', 'person'), name='unique_reviewer_settings_per_team_person'),
+        ),
+    ]

ietf/review/models.py

@@ -45,6 +45,8 @@ def __str__(self):
 
     class Meta:
         verbose_name_plural = "reviewer settings"
+        constraints = [models.UniqueConstraint(fields=['team', 'person'],
+                                               name='unique_reviewer_settings_per_team_person')]
 
 class ReviewSecretarySettings(models.Model):
     """Keeps track of admin data associated with a secretary in a team."""