Skip to content

Commit aca3b41

Browse files
committed
Add special handling for ReviewerSettings to merge person feature. Fixes ietf-tools#3183. Commit ready for merge
- Legacy-Id: 18860
1 parent cc65d97 commit aca3b41

2 files changed

Lines changed: 72 additions & 3 deletions

File tree

ietf/person/tests.py

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@
1717

1818
from ietf.community.models import CommunityList
1919
from ietf.group.factories import RoleFactory
20+
from ietf.group.models import Group
2021
from ietf.nomcom.models import NomCom
2122
from ietf.nomcom.test_data import nomcom_test_data
2223
from ietf.nomcom.factories import NomComFactory, NomineeFactory, NominationFactory, FeedbackFactory, PositionFactory
2324
from ietf.person.factories import EmailFactory, PersonFactory, UserFactory
2425
from ietf.person.models import Person, Alias
2526
from ietf.person.utils import (merge_persons, determine_merge_order, send_merge_notification,
26-
handle_users, get_extra_primary, dedupe_aliases, move_related_objects, merge_nominees, merge_users)
27+
handle_users, get_extra_primary, dedupe_aliases, move_related_objects, merge_nominees,
28+
handle_reviewer_settings, merge_users)
29+
from ietf.review.models import ReviewerSettings
2730
from ietf.utils.test_utils import TestCase, login_testing_unauthorized
2831
from ietf.utils.mail import outbox, empty_outbox
2932

@@ -187,6 +190,40 @@ def test_send_merge_notification(self):
187190
self.assertEqual(len(outbox),len_before+1)
188191
self.assertTrue('IETF Datatracker records merged' in outbox[-1]['Subject'])
189192

193+
def test_handle_reviewer_settings(self):
194+
groups = Group.objects.all()
195+
# no ReviewerSettings
196+
source = PersonFactory()
197+
target = PersonFactory()
198+
result = handle_reviewer_settings(source, target)
199+
self.assertEqual(result, '')
200+
201+
# source ReviewerSettings only
202+
source = PersonFactory()
203+
target = PersonFactory()
204+
ReviewerSettings.objects.create(team=groups[0],person=source,min_interval=14)
205+
result = handle_reviewer_settings(source, target)
206+
self.assertEqual(result, '')
207+
208+
# source and target ReviewerSettings, non-conflicting
209+
source = PersonFactory()
210+
target = PersonFactory()
211+
rs1 = ReviewerSettings.objects.create(team=groups[0],person=source,min_interval=14)
212+
ReviewerSettings.objects.create(team=groups[1],person=target,min_interval=14)
213+
result = handle_reviewer_settings(source, target)
214+
self.assertEqual(result, '')
215+
216+
# source and target ReviewerSettings, conflicting
217+
source = PersonFactory()
218+
target = PersonFactory()
219+
rs1 = ReviewerSettings.objects.create(team=groups[0],person=source,min_interval=14)
220+
ReviewerSettings.objects.create(team=groups[0],person=target,min_interval=7)
221+
self.assertEqual(source.reviewersettings_set.count(), 1)
222+
result = handle_reviewer_settings(source, target)
223+
self.assertEqual(result, 'REVIEWER SETTINGS ACTION: dropping duplicate ReviewSettings for team: {}'.format(rs1.team))
224+
self.assertEqual(source.reviewersettings_set.count(), 0)
225+
self.assertEqual(target.reviewersettings_set.count(), 1)
226+
190227
def test_handle_users(self):
191228
source1 = get_person_no_user()
192229
target1 = get_person_no_user()
@@ -199,7 +236,7 @@ def test_handle_users(self):
199236

200237
# no Users
201238
result = handle_users(source1, target1)
202-
self.assertTrue('DATATRACKER LOGIN ACTION: none' in result)
239+
self.assertTrue("DATATRACKER LOGIN ACTION: none" in result)
203240

204241
# target user
205242
result = handle_users(source2, target2)
@@ -268,6 +305,22 @@ def test_merge_persons(self):
268305
self.assertFalse(Person.objects.filter(id=source_id))
269306
self.assertFalse(source_user.is_active)
270307

308+
def test_merge_persons_reviewer_settings(self):
309+
secretariat_role = RoleFactory(group__acronym='secretariat', name_id='secr')
310+
user = secretariat_role.person.user
311+
request = HttpRequest()
312+
request.user = user
313+
source = PersonFactory()
314+
target = PersonFactory()
315+
groups = Group.objects.all()
316+
ReviewerSettings.objects.create(team=groups[0],person=source,min_interval=14)
317+
ReviewerSettings.objects.create(team=groups[0],person=target,min_interval=7)
318+
merge_persons(request, source, target, file=StringIO())
319+
self.assertFalse(Person.objects.filter(pk=source.pk))
320+
self.assertEqual(target.reviewersettings_set.count(), 1)
321+
rs = target.reviewersettings_set.first()
322+
self.assertEqual(rs.min_interval, 7)
323+
271324
def test_merge_users(self):
272325
person = PersonFactory()
273326
source = person.user

ietf/person/utils.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ def merge_persons(request, source, target, file=sys.stdout, verbose=False):
3030
email.save()
3131
changes.append('EMAIL ACTION: {} no longer marked as primary'.format(email.address))
3232

33-
changes.append(handle_users(source,target))
33+
changes.append(handle_users(source, target))
34+
reviewer_changes = handle_reviewer_settings(source, target)
35+
if reviewer_changes:
36+
changes.extend(reviewer_changes)
3437
merge_nominees(source, target)
3538
move_related_objects(source, target, file=file, verbose=verbose)
3639
dedupe_aliases(target)
@@ -67,6 +70,19 @@ def get_extra_primary(source,target):
6770
else:
6871
return []
6972

73+
def handle_reviewer_settings(source, target):
74+
'''
75+
Person.ReviewerSettings are restricted to one object per team. If
76+
both source and target have ReviewerSettings for the same team
77+
remove the source ReviewerSetting and report action.
78+
'''
79+
changes = []
80+
for rs in source.reviewersettings_set.all():
81+
if target.reviewersettings_set.filter(team=rs.team):
82+
changes.append('REVIEWER SETTINGS ACTION: dropping duplicate ReviewSettings for team: {}'.format(rs.team))
83+
rs.delete()
84+
return changes
85+
7086
def handle_users(source,target,check_only=False):
7187
'''
7288
Deactivates extra Users. Retains target user. If check_only == True, just return a string

0 commit comments

Comments
 (0)