Skip to content

Commit e9b7c57

Browse files
rpcrossrpcross
committed
Refactor ietf/bin/merge-person-records to facilitate testing. Add tests. Fixes ietf-tools#2162. Commit ready for merge.
 - Legacy-Id: 13567
1 parent 77f4bf2 commit e9b7c57

5 files changed

Lines changed: 267 additions & 225 deletions

File tree

‎ietf/bin/merge-person-records‎

Lines changed: 5 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -27,83 +27,12 @@ django.setup()
2727
# -------------------------------------------------------------------------------------
2828

2929
import argparse
30-
import datetime
31-
import pprint
32-
import syslog
3330
from django.contrib import admin
34-
from django.contrib.auth.models import User
3531
from ietf.person.models import Person
32+
from ietf.person.utils import (merge_persons, send_merge_notification, handle_users,
33+
determine_merge_order)
3634
from ietf.utils.log import log
37-
from ietf.utils.mail import send_mail
3835

39-
def dedupe_aliaises(person):
40-
'''
41-
Check person for duplicate aliases and purge
42-
'''
43-
seen = []
44-
for alias in person.alias_set.all():
45-
if alias.name in seen:
46-
alias.delete()
47-
else:
48-
seen.append(alias.name)
49-
50-
def determine_merge_order(source,target):
51-
'''
52-
Determine merge order. Select Person that has related User. If both have Users
53-
select one with most recent login
54-
'''
55-
if source.user and not target.user:
56-
source,target = target,source # swap merge order
57-
if source.user and target.user:
58-
source,target = sorted([source,target],key=lambda a: a.user.last_login if a.user.last_login else datetime.datetime.min)
59-
return source,target
60-
61-
def get_extra_primary(source,target):
62-
'''
63-
Inspect email addresses and return list of those that should no longer be primary
64-
'''
65-
if source.email_set.filter(primary=True) and target.email_set.filter(primary=True):
66-
return source.email_set.filter(primary=True)
67-
else:
68-
return []
69-
70-
def handle_users(source,target,check_only=False):
71-
'''
72-
Deletes extra Users. Retains target user. If check_only == True, just return a string
73-
describing action, otherwise perform user changes and return string.
74-
'''
75-
if not (source.user or target.user):
76-
return "DATATRACKER LOGIN ACTION: none (no login defined)"
77-
if not source.user and target.user:
78-
return "DATATRACKER LOGIN ACTION: retaining login {}".format(target.user)
79-
if source.user and not target.user:
80-
message = "DATATRACKER LOGIN ACTION: retaining login {}".format(source.user)
81-
if not check_only:
82-
target.user = source.user
83-
target.save()
84-
return message
85-
if source.user and target.user:
86-
message = "DATATRACKER LOGIN ACTION: retaining login: {}, removing login: {}".format(target.user,source.user)
87-
if not check_only:
88-
syslog.syslog('merge-person-records: deleting user {}'.format(source.user.username))
89-
user = source.user
90-
source.user = None
91-
source.save()
92-
#user.delete()
93-
return message
94-
95-
def send_notification(person,changes):
96-
'''
97-
Send an email to the merge target (Person) notifying them of the changes
98-
'''
99-
send_mail(request = None,
100-
to = person.email_address(),
101-
frm = "IETF Secretariat <ietf-secretariat@ietf.org>",
102-
subject = "IETF Datatracker records merged",
103-
template = "utils/merge_person_records.txt",
104-
context = dict(person=person,changes='\n'.join(changes)),
105-
extra = {}
106-
)
10736

10837
def main():
10938
parser = argparse.ArgumentParser()
@@ -112,8 +41,6 @@ def main():
11241
parser.add_argument('-f','--force', help='force merge order',action='store_true')
11342
parser.add_argument('-v','--verbose', help='verbose output',action='store_true')
11443
args = parser.parse_args()
115-
changes = []
116-
syslog.openlog(os.path.basename(__file__), syslog.LOG_PID, syslog.LOG_USER)
11744

11845
source = Person.objects.get(pk=args.source_id)
11946
target = Person.objects.get(pk=args.target_id)
@@ -129,62 +56,11 @@ def main():
12956
if response.lower() != 'y':
13057
sys.exit()
13158

132-
# write log
133-
syslog.syslog("Merging person records {} => {}".format(source.pk,target.pk))
134-
135-
# handle primary emails
136-
for email in get_extra_primary(source,target):
137-
email.primary = False
138-
email.save()
139-
changes.append('EMAIL ACTION: {} no longer marked as primary'.format(email.address))
140-
141-
# handle users
142-
changes.append(handle_users(source,target))
143-
144-
# find all related objects and migrate
145-
related_objects = [ f for f in source._meta.get_fields()
146-
if (f.one_to_many or f.one_to_one)
147-
and f.auto_created and not f.concrete ]
148-
for related_object in related_objects:
149-
accessor = related_object.get_accessor_name()
150-
field_name = related_object.field.name
151-
queryset = getattr(source, accessor).all()
152-
if args.verbose:
153-
print "Merging {}:{}".format(accessor,queryset.count())
154-
kwargs = { field_name:target }
155-
queryset.update(**kwargs)
156-
157-
# check aliases
158-
dedupe_aliaises(target)
159-
160-
# copy other attributes
161-
for field in ('ascii','ascii_short','address','affiliation'):
162-
if getattr(source,field) and not getattr(target,field):
163-
setattr(target,field,getattr(source,field))
164-
target.save()
165-
166-
# check for any remaining relationships and exit if more found
167-
objs = [source]
168-
opts = Person._meta
169-
user = User.objects.filter(is_superuser=True).first()
170-
admin_site = admin.site
171-
using = 'default'
172-
deletable_objects = admin.utils.get_deleted_objects(
173-
objs, opts, user, admin_site, using)
174-
175-
deletable_objects_summary = deletable_objects[1]
176-
if len(deletable_objects_summary) > 1: # should only inlcude one object (Person)
177-
print "Not Deleting Person: {}({})".format(source.ascii,source.pk)
178-
print "Related objects remain:"
179-
pprint.pprint(deletable_objects[1])
180-
sys.exit(1)
181-
182-
if args.verbose:
183-
print "Deleting Person: {}({})".format(source.ascii,source.pk)
184-
source.delete()
59+
# perform merge
60+
success, changes = merge_persons(source, target, verbose=args.verbose)
18561

18662
# send email notification
187-
send_notification(target,changes)
63+
send_merge_notification(target,changes)
18864

18965
if __name__ == "__main__":
19066
main()

‎ietf/nomcom/tests.py‎

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import shutil
55
import urlparse
66
from pyquery import PyQuery
7-
import StringIO
87

98
from django.db import IntegrityError
109
from django.db.models import Max
@@ -22,8 +21,6 @@
2221
from ietf.group.models import Group
2322
from ietf.message.models import Message
2423

25-
from ietf.person.utils import merge_persons
26-
2724
from ietf.nomcom.test_data import nomcom_test_data, generate_cert, check_comments, \
2825
COMMUNITY_USER, CHAIR_USER, \
2926
MEMBER_USER, SECRETARIAT_USER, EMAIL_DOMAIN, NOMCOM_YEAR
@@ -37,7 +34,7 @@
3734
from ietf.nomcom.factories import NomComFactory, FeedbackFactory, TopicFactory, \
3835
nomcom_kwargs_for_year, provide_private_key_to_test_client, \
3936
key
40-
from ietf.person.factories import PersonFactory, EmailFactory, UserFactory
37+
from ietf.person.factories import PersonFactory, EmailFactory
4138
from ietf.dbtemplate.factories import DBTemplateFactory
4239
from ietf.dbtemplate.models import DBTemplate
4340

@@ -1777,34 +1774,6 @@ def test_not_yet(self):
17771774
# No questionnaire responses
17781775
self.do_common_work(reverse('ietf.nomcom.views.private_questionnaire',kwargs={'year':self.nc.year()}),False)
17791776

1780-
class MergePersonTests(TestCase):
1781-
def setUp(self):
1782-
build_test_public_keys_dir(self)
1783-
self.nc = NomComFactory(**nomcom_kwargs_for_year())
1784-
self.author = PersonFactory.create().email_set.first().address
1785-
self.nominee1, self.nominee2 = self.nc.nominee_set.all()[:2]
1786-
self.person1, self.person2 = self.nominee1.person, self.nominee2.person
1787-
self.position = self.nc.position_set.first()
1788-
for nominee in [self.nominee1, self.nominee2]:
1789-
f = FeedbackFactory.create(author=self.author,nomcom=self.nc,type_id='nomina')
1790-
f.positions.add(self.position)
1791-
f.nominees.add(nominee)
1792-
UserFactory(is_superuser=True)
1793-
1794-
def tearDown(self):
1795-
clean_test_public_keys_dir(self)
1796-
1797-
def test_merge_person(self):
1798-
person1, person2 = [nominee.person for nominee in self.nc.nominee_set.all()[:2]]
1799-
stream = StringIO.StringIO()
1800-
1801-
self.assertEqual(self.nc.nominee_set.count(),4)
1802-
self.assertEqual(self.nominee1.feedback_set.count(),1)
1803-
self.assertEqual(self.nominee2.feedback_set.count(),1)
1804-
merge_persons(person1,person2,stream)
1805-
self.assertEqual(self.nc.nominee_set.count(),3)
1806-
self.assertEqual(self.nc.nominee_set.get(pk=self.nominee2.pk).feedback_set.count(),2)
1807-
self.assertFalse(self.nc.nominee_set.filter(pk=self.nominee1.pk).exists())
18081777

18091778
class AcceptingTests(TestCase):
18101779
def setUp(self):

‎ietf/person/factories.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ class Meta:
4242
ascii = factory.LazyAttribute(lambda p: unicode(unidecode(p.name).strip()))
4343

4444
class Params:
45-
with_bio = factory.Trait(
46-
biography = u"\n\n".join(fake.paragraphs()),
47-
)
45+
with_bio = factory.Trait(biography = u"\n\n".join(fake.paragraphs()))
4846

4947
@factory.post_generation
5048
def default_aliases(obj, create, extracted, **kwargs): # pylint: disable=no-self-argument

‎ietf/person/tests.py‎

Lines changed: 130 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
# -*- coding: utf-8 -*-
22
from __future__ import unicode_literals
33

4+
import datetime
45
import json
56
from pyquery import PyQuery
6-
7+
from StringIO import StringIO
78
from django.urls import reverse as urlreverse
89

910
import debug # pyflakes:ignore
1011

11-
from ietf.person.factories import EmailFactory,PersonFactory
12-
from ietf.person.models import Person
12+
#from ietf.nomcom.models import Nominee, NomCom
13+
#from ietf.nomcom.test_data import nomcom_test_data
14+
from ietf.person.factories import EmailFactory, PersonFactory
15+
from ietf.person.models import Person, Alias
16+
from ietf.person.utils import (merge_persons, determine_merge_order, send_merge_notification,
17+
handle_users, get_extra_primary, dedupe_aliases, move_related_objects)
1318
from ietf.utils.test_data import make_test_data
1419
from ietf.utils.test_utils import TestCase
1520
from ietf.utils.mail import outbox, empty_outbox
@@ -78,3 +83,125 @@ def test_duplicate_person_name(self):
7883
Person.objects.create(name="Duplicate Test")
7984
Person.objects.create(name="Duplicate Test")
8085
self.assertTrue("possible duplicate" in outbox[0]["Subject"].lower())
86+
87+
class PersonUtilsTests(TestCase):
88+
def get_person_no_user(self):
89+
person = PersonFactory()
90+
person.user = None
91+
person.save()
92+
return person
93+
94+
def test_determine_merge_order(self):
95+
p1 = self.get_person_no_user()
96+
p2 = PersonFactory()
97+
p3 = self.get_person_no_user()
98+
p4 = PersonFactory()
99+
100+
# target has User
101+
results = determine_merge_order(p1, p2)
102+
self.assertEqual(results,(p1,p2))
103+
104+
# source has User
105+
results = determine_merge_order(p2, p1)
106+
self.assertEqual(results,(p1,p2))
107+
108+
# neither have User
109+
results = determine_merge_order(p1, p3)
110+
self.assertEqual(results,(p1,p3))
111+
112+
# both have User
113+
today = datetime.datetime.today()
114+
p2.user.last_login = today
115+
p2.user.save()
116+
p4.user.last_login = today - datetime.timedelta(days=30)
117+
p4.user.save()
118+
results = determine_merge_order(p2, p4)
119+
self.assertEqual(results,(p4,p2))
120+
121+
def test_send_merge_notification(self):
122+
person = PersonFactory()
123+
len_before = len(outbox)
124+
send_merge_notification(person,['Record Merged'])
125+
self.assertEqual(len(outbox),len_before+1)
126+
self.assertTrue('IETF Datatracker records merged' in outbox[-1]['Subject'])
127+
128+
def test_handle_users(self):
129+
source1 = self.get_person_no_user()
130+
target1 = self.get_person_no_user()
131+
source2 = self.get_person_no_user()
132+
target2 = PersonFactory()
133+
source3 = PersonFactory()
134+
target3 = self.get_person_no_user()
135+
source4 = PersonFactory()
136+
target4 = PersonFactory()
137+
138+
# no Users
139+
result = handle_users(source1, target1)
140+
self.assertTrue('DATATRACKER LOGIN ACTION: none' in result)
141+
142+
# target user
143+
result = handle_users(source2, target2)
144+
self.assertTrue("DATATRACKER LOGIN ACTION: retaining login {}".format(target2.user) in result)
145+
146+
# source user
147+
user = source3.user
148+
result = handle_users(source3, target3)
149+
self.assertTrue("DATATRACKER LOGIN ACTION: retaining login {}".format(user) in result)
150+
self.assertTrue(target3.user == user)
151+
152+
# both have user
153+
source_user = source4.user
154+
target_user = target4.user
155+
result = handle_users(source4, target4)
156+
self.assertTrue("DATATRACKER LOGIN ACTION: retaining login: {}, removing login: {}".format(target_user,source_user) in result)
157+
self.assertTrue(target4.user == target_user)
158+
self.assertTrue(source4.user == None)
159+
160+
def test_get_extra_primary(self):
161+
source = PersonFactory()
162+
target = PersonFactory()
163+
extra = get_extra_primary(source, target)
164+
self.assertTrue(extra == list(source.email_set.filter(primary=True)))
165+
166+
def test_dedupe_aliases(self):
167+
person = PersonFactory()
168+
Alias.objects.create(person=person, name='Joe')
169+
Alias.objects.create(person=person, name='Joe')
170+
self.assertEqual(person.alias_set.filter(name='Joe').count(),2)
171+
dedupe_aliases(person)
172+
self.assertEqual(person.alias_set.filter(name='Joe').count(),1)
173+
174+
"""
175+
def test_merge_nominees(self):
176+
nomcom_test_data()
177+
nomcom = NomCom.objects.first()
178+
source = PersonFactory()
179+
source.nominee_set.create(nomcom=nomcom,email=source.email())
180+
#source = Nominee.objects.first().email.person
181+
target = PersonFactory()
182+
print source
183+
print source.nominee_set.all()
184+
merge_nominees(source, target)
185+
self.assertTrue(target.nominee_set.all())
186+
"""
187+
188+
def test_move_related_objects(self):
189+
source = PersonFactory()
190+
target = PersonFactory()
191+
source_email = source.email_set.first()
192+
source_alias = source.alias_set.first()
193+
move_related_objects(source, target, file=StringIO())
194+
self.assertTrue(source_email in target.email_set.all())
195+
self.assertTrue(source_alias in target.alias_set.all())
196+
197+
def test_merge_persons(self):
198+
source = PersonFactory()
199+
target = PersonFactory()
200+
source_id = source.pk
201+
source_email = source.email_set.first()
202+
source_alias = source.alias_set.first()
203+
merge_persons(source, target, file=StringIO())
204+
self.assertTrue(source_email in target.email_set.all())
205+
self.assertTrue(source_alias in target.alias_set.all())
206+
self.assertFalse(Person.objects.filter(id=source_id))
207+

0 commit comments

Comments
 (0)