Skip to content

Commit a28594e

Browse files
authored
feat: add option to email users about duplicate accounts. Fixes ietf-tools#8174. (ietf-tools#9850)
* feat: add option to email users about duplicate accounts. Fixes ietf-tools#8174. * fix: split person merge into two views * fix: use form for validation * fix: update text of merge request email * fix: update copyright date * fix: use custom field classes in MergeRequestForm
1 parent 4ff4805 commit a28594e

9 files changed

Lines changed: 254 additions & 50 deletions

File tree

ietf/person/forms.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,26 @@
1-
# Copyright The IETF Trust 2018-2020, All Rights Reserved
1+
# Copyright The IETF Trust 2018-2025, All Rights Reserved
22
# -*- coding: utf-8 -*-
33

44

55
from django import forms
6+
67
from ietf.person.models import Person
8+
from ietf.utils.fields import MultiEmailField, NameAddrEmailField
79

810

911
class MergeForm(forms.Form):
1012
source = forms.IntegerField(label='Source Person ID')
1113
target = forms.IntegerField(label='Target Person ID')
1214

15+
def __init__(self, *args, **kwargs):
16+
self.readonly = False
17+
if 'readonly' in kwargs:
18+
self.readonly = kwargs.pop('readonly')
19+
super().__init__(*args, **kwargs)
20+
if self.readonly:
21+
self.fields['source'].widget.attrs['readonly'] = True
22+
self.fields['target'].widget.attrs['readonly'] = True
23+
1324
def clean_source(self):
1425
return self.get_person(self.cleaned_data['source'])
1526

@@ -21,3 +32,11 @@ def get_person(self, pk):
2132
return Person.objects.get(pk=pk)
2233
except Person.DoesNotExist:
2334
raise forms.ValidationError("ID does not exist")
35+
36+
37+
class MergeRequestForm(forms.Form):
38+
to = MultiEmailField()
39+
frm = NameAddrEmailField()
40+
reply_to = MultiEmailField()
41+
subject = forms.CharField()
42+
body = forms.CharField(widget=forms.Textarea)

ietf/person/tests.py

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright The IETF Trust 2014-2022, All Rights Reserved
1+
# Copyright The IETF Trust 2014-2025, All Rights Reserved
22
# -*- coding: utf-8 -*-
33

44

@@ -10,7 +10,6 @@
1010
from PIL import Image
1111
from pyquery import PyQuery
1212

13-
1413
from django.core.exceptions import ValidationError
1514
from django.http import HttpRequest
1615
from django.test import override_settings
@@ -23,6 +22,7 @@
2322
from ietf.community.models import CommunityList
2423
from ietf.group.factories import RoleFactory
2524
from ietf.group.models import Group
25+
from ietf.message.models import Message
2626
from ietf.nomcom.models import NomCom
2727
from ietf.nomcom.test_data import nomcom_test_data
2828
from ietf.nomcom.factories import NomComFactory, NomineeFactory, NominationFactory, FeedbackFactory, PositionFactory
@@ -208,21 +208,21 @@ def test_merge(self):
208208
def test_merge_with_params(self):
209209
p1 = get_person_no_user()
210210
p2 = PersonFactory()
211-
url = urlreverse("ietf.person.views.merge") + "?source={}&target={}".format(p1.pk, p2.pk)
211+
url = urlreverse("ietf.person.views.merge_submit") + "?source={}&target={}".format(p1.pk, p2.pk)
212212
login_testing_unauthorized(self, "secretary", url)
213213
r = self.client.get(url)
214214
self.assertContains(r, 'retaining login', status_code=200)
215215

216216
def test_merge_with_params_bad_id(self):
217-
url = urlreverse("ietf.person.views.merge") + "?source=1000&target=2000"
217+
url = urlreverse("ietf.person.views.merge_submit") + "?source=1000&target=2000"
218218
login_testing_unauthorized(self, "secretary", url)
219219
r = self.client.get(url)
220220
self.assertContains(r, 'ID does not exist', status_code=200)
221221

222222
def test_merge_post(self):
223223
p1 = get_person_no_user()
224224
p2 = PersonFactory()
225-
url = urlreverse("ietf.person.views.merge")
225+
url = urlreverse("ietf.person.views.merge_submit")
226226
expected_url = urlreverse("ietf.secr.rolodex.views.view", kwargs={'id': p2.pk})
227227
login_testing_unauthorized(self, "secretary", url)
228228
data = {'source': p1.pk, 'target': p2.pk}
@@ -451,6 +451,30 @@ def test_dots(self):
451451
ncchair = RoleFactory(group__acronym='nomcom2020',group__type_id='nomcom',name_id='chair').person
452452
self.assertEqual(get_dots(ncchair),['nomcom'])
453453

454+
def test_send_merge_request(self):
455+
empty_outbox()
456+
message_count_before = Message.objects.count()
457+
source = PersonFactory()
458+
target = PersonFactory()
459+
url = urlreverse('ietf.person.views.send_merge_request')
460+
url = url + f'?source={source.pk}&target={target.pk}'
461+
login_testing_unauthorized(self, 'secretary', url)
462+
r = self.client.get(url)
463+
initial = r.context['form'].initial
464+
subject = 'Action requested: Merging possible duplicate IETF Datatracker accounts'
465+
self.assertEqual(initial['to'], ', '.join([source.user.username, target.user.username]))
466+
self.assertEqual(initial['subject'], subject)
467+
self.assertEqual(initial['reply_to'], 'support@ietf.org')
468+
self.assertEqual(r.status_code, 200)
469+
r = self.client.post(url, data=initial)
470+
self.assertEqual(r.status_code, 302)
471+
self.assertEqual(len(outbox), 1)
472+
self.assertIn(source.user.username, outbox[0]['To'])
473+
message_count_after = Message.objects.count()
474+
message = Message.objects.last()
475+
self.assertEqual(message_count_after, message_count_before + 1)
476+
self.assertIn(source.user.username, message.to)
477+
454478

455479
class TaskTests(TestCase):
456480
@mock.patch("ietf.person.tasks.log.log")

ietf/person/urls.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1+
# Copyright The IETF Trust 2009-2025, All Rights Reserved
2+
# -*- coding: utf-8 -*-
13
from ietf.person import views, ajax
24
from ietf.utils.urls import url
35

46
urlpatterns = [
57
url(r'^merge/?$', views.merge),
8+
url(r'^merge/submit/?$', views.merge_submit),
9+
url(r'^merge/send_request/?$', views.send_merge_request),
610
url(r'^search/(?P<model_name>(person|email))/$', views.ajax_select2_search),
711
url(r'^(?P<personid>[0-9]+)/email.json$', ajax.person_email_json),
812
url(r'^(?P<email_or_name>[^/]+)$', views.profile),

ietf/person/views.py

Lines changed: 75 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,26 @@
1-
# Copyright The IETF Trust 2012-2020, All Rights Reserved
1+
# Copyright The IETF Trust 2012-2025, All Rights Reserved
22
# -*- coding: utf-8 -*-
33

44

55
from io import StringIO, BytesIO
66
from PIL import Image
77

8+
from django.conf import settings
89
from django.contrib import messages
910
from django.db.models import Q
1011
from django.http import HttpResponse, Http404
1112
from django.shortcuts import render, redirect
13+
from django.template.loader import render_to_string
1214
from django.utils import timezone
1315

1416
import debug # pyflakes:ignore
1517

1618
from ietf.ietfauth.utils import role_required
1719
from ietf.person.models import Email, Person
1820
from ietf.person.fields import select2_id_name_json
19-
from ietf.person.forms import MergeForm
21+
from ietf.person.forms import MergeForm, MergeRequestForm
2022
from ietf.person.utils import handle_users, merge_persons, lookup_persons
23+
from ietf.utils.mail import send_mail_text
2124

2225

2326
def ajax_select2_search(request, model_name):
@@ -98,16 +101,19 @@ def photo(request, email_or_name):
98101
@role_required("Secretariat")
99102
def merge(request):
100103
form = MergeForm()
101-
method = 'get'
104+
return render(request, 'person/merge.html', {'form': form})
105+
106+
107+
@role_required("Secretariat")
108+
def merge_submit(request):
102109
change_details = ''
103110
warn_messages = []
104111
source = None
105112
target = None
106113

107114
if request.method == "GET":
108-
form = MergeForm()
109115
if request.GET:
110-
form = MergeForm(request.GET)
116+
form = MergeForm(request.GET, readonly=True)
111117
if form.is_valid():
112118
source = form.cleaned_data.get('source')
113119
target = form.cleaned_data.get('target')
@@ -116,12 +122,9 @@ def merge(request):
116122
if source.user.last_login and target.user.last_login and source.user.last_login > target.user.last_login:
117123
warn_messages.append('WARNING: The most recently used login is being deleted!')
118124
change_details = handle_users(source, target, check_only=True)
119-
method = 'post'
120-
else:
121-
method = 'get'
122125

123126
if request.method == "POST":
124-
form = MergeForm(request.POST)
127+
form = MergeForm(request.POST, readonly=True)
125128
if form.is_valid():
126129
source = form.cleaned_data.get('source')
127130
source_id = source.id
@@ -136,11 +139,72 @@ def merge(request):
136139
messages.error(request, output)
137140
return redirect('ietf.secr.rolodex.views.view', id=target.pk)
138141

139-
return render(request, 'person/merge.html', {
142+
return render(request, 'person/merge_submit.html', {
140143
'form': form,
141-
'method': method,
142144
'change_details': change_details,
143145
'source': source,
144146
'target': target,
145147
'warn_messages': warn_messages,
146148
})
149+
150+
151+
@role_required("Secretariat")
152+
def send_merge_request(request):
153+
if request.method == 'GET':
154+
merge_form = MergeForm(request.GET)
155+
if merge_form.is_valid():
156+
source = merge_form.cleaned_data['source']
157+
target = merge_form.cleaned_data['target']
158+
to = []
159+
if source.email():
160+
to.append(source.email().address)
161+
if target.email():
162+
to.append(target.email().address)
163+
if source.user:
164+
source_account = source.user.username
165+
else:
166+
source_account = source.email()
167+
if target.user:
168+
target_account = target.user.username
169+
else:
170+
target_account = target.email()
171+
sender_name = request.user.person.name
172+
subject = 'Action requested: Merging possible duplicate IETF Datatracker accounts'
173+
context = {
174+
'source_account': source_account,
175+
'target_account': target_account,
176+
'sender_name': sender_name,
177+
}
178+
body = render_to_string('person/merge_request_email.txt', context)
179+
initial = {
180+
'to': ', '.join(to),
181+
'frm': settings.DEFAULT_FROM_EMAIL,
182+
'reply_to': 'support@ietf.org',
183+
'subject': subject,
184+
'body': body,
185+
'by': request.user.person.pk,
186+
}
187+
form = MergeRequestForm(initial=initial)
188+
else:
189+
messages.error(request, "Error requesting merge email: " + merge_form.errors.as_text())
190+
return redirect("ietf.person.views.merge")
191+
192+
if request.method == 'POST':
193+
form = MergeRequestForm(request.POST)
194+
if form.is_valid():
195+
extra = {"Reply-To": form.cleaned_data.get("reply_to")}
196+
send_mail_text(
197+
request,
198+
form.cleaned_data.get("to"),
199+
form.cleaned_data.get("frm"),
200+
form.cleaned_data.get("subject"),
201+
form.cleaned_data.get("body"),
202+
extra=extra,
203+
)
204+
205+
messages.success(request, "The merge confirmation email was sent.")
206+
return redirect("ietf.person.views.merge")
207+
208+
return render(request, "person/send_merge_request.html", {
209+
"form": form,
210+
})

ietf/templates/person/merge.html

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
{# Copyright The IETF Trust 2018-2025, All Rights Reserved #}
12
{% extends "base.html" %}
2-
{# Copyright The IETF Trust 2015, All Rights Reserved #}
33
{% load static %}
44
{% load django_bootstrap5 %}
55
{% block title %}Merge Persons{% endblock %}
@@ -8,45 +8,17 @@ <h1>Merge Person Records</h1>
88
<p class="alert alert-info my-3">
99
This tool will merge two Person records into one. If both records have logins and you want to retain the one on the left, use the Swap button to swap source and target records.
1010
</p>
11-
<form method="{{ method }}">
12-
{% if method == 'post' %}
13-
{% csrf_token %}
14-
{% endif %}
11+
<form method="GET" action="{% url 'ietf.person.views.merge_submit' %}">
1512
<div class="row mt-3">
1613
<div class="col-md-6">
1714
{% bootstrap_field form.source %}
18-
{% if source %}
19-
{% with person=source %}
20-
{% include "person/person_info.html" %}
21-
{% endwith %}
22-
{% endif %}
2315
</div>
2416
<div class="col-md-6">
2517
{% bootstrap_field form.target %}
26-
{% if target %}
27-
{% with person=target %}
28-
{% include "person/person_info.html" %}
29-
{% endwith %}
30-
{% endif %}
3118
</div>
3219
</div>
33-
{% if change_details %}<div class="alert alert-info my-3" role="alert">{{ change_details }}</div>{% endif %}
34-
{% if warn_messages %}
35-
{% for message in warn_messages %}<div class="alert alert-warning my-3" role="alert">{{ message }}</div>{% endfor %}
36-
{% endif %}
37-
{% if method == 'post' %}
38-
<a class="btn btn-primary"
39-
href="{% url 'ietf.person.views.merge' %}?source={{ target.pk }}&amp;target={{ source.pk }}"
40-
role="button">
41-
Swap
42-
</a>
43-
{% endif %}
44-
<button type="submit" class="btn btn-warning">
45-
{% if method == 'post' %}
46-
Merge
47-
{% else %}
48-
Submit
49-
{% endif %}
20+
<button type="submit" class="btn btn-warning" title="Get Person information">
21+
Get Info
5022
</button>
5123
</form>
5224
{% endblock %}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
Hello,
2+
3+
We have identified multiple IETF Datatracker accounts that may represent a single person:
4+
5+
https://datatracker.ietf.org/person/{{ source_account }}
6+
7+
and
8+
9+
https://datatracker.ietf.org/person/{{ target_account }}
10+
11+
If this is so then it is important that we merge the accounts.
12+
13+
This email is being sent to the primary emails associated with each Datatracker account.
14+
15+
Please respond to this message individually from the email account(s) you control so we can take the appropriate action.
16+
17+
If these should be merged, please identify which account you would like to keep the login credentials from.
18+
19+
If you are associated with but no longer have access to one of the email accounts, then please let us know and we will follow up to determine how to proceed.
20+
21+
22+
{{ sender_name }}
23+
IETF Support

0 commit comments

Comments
 (0)