From 85f4861f9f1deca485554d3bb2c9f0e70e40bafc Mon Sep 17 00:00:00 2001
From: Ryan Cross <rcross@amsl.com>
Date: Sun, 18 Mar 2018 18:01:56 +0000
Subject: [PATCH] Add view for merge person records. Commit ready for merge.
- Legacy-Id: 14862
---
ietf/person/forms.py | 23 +++++++
ietf/person/tests.py | 61 ++++++++++++++-----
ietf/person/urls.py | 1 +
ietf/person/views.py | 61 ++++++++++++++++++-
ietf/static/ietf/css/ietf.css | 7 +++
.../person/mail/possible_duplicates.txt | 3 +
ietf/templates/person/merge.html | 48 +++++++++++++++
ietf/templates/person/person_info.html | 22 +++++++
8 files changed, 209 insertions(+), 17 deletions(-)
create mode 100644 ietf/person/forms.py
create mode 100644 ietf/templates/person/merge.html
create mode 100644 ietf/templates/person/person_info.html
diff --git a/ietf/person/forms.py b/ietf/person/forms.py
new file mode 100644
index 000000000..aa7015b5c
--- /dev/null
+++ b/ietf/person/forms.py
@@ -0,0 +1,23 @@
+# Copyright The IETF Trust 2017, All Rights Reserved
+
+from __future__ import unicode_literals
+
+from django import forms
+from ietf.person.models import Person
+
+
+class MergeForm(forms.Form):
+ source = forms.IntegerField(label='Source Person ID')
+ target = forms.IntegerField(label='Target Person ID')
+
+ def clean_source(self):
+ return self.get_person(self.cleaned_data['source'])
+
+ def clean_target(self):
+ return self.get_person(self.cleaned_data['target'])
+
+ def get_person(self, pk):
+ try:
+ return Person.objects.get(pk=pk)
+ except Person.DoesNotExist:
+ raise forms.ValidationError("ID does not exist")
diff --git a/ietf/person/tests.py b/ietf/person/tests.py
index 8880e3423..caf32beb4 100644
--- a/ietf/person/tests.py
+++ b/ietf/person/tests.py
@@ -19,12 +19,18 @@ from ietf.person.models import Person, Alias
from ietf.person.utils import (merge_persons, determine_merge_order, send_merge_notification,
handle_users, get_extra_primary, dedupe_aliases, move_related_objects, merge_nominees, merge_users)
from ietf.utils.test_data import make_test_data
-from ietf.utils.test_utils import TestCase
+from ietf.utils.test_utils import TestCase, login_testing_unauthorized
from ietf.utils.mail import outbox, empty_outbox
-class PersonTests(TestCase):
+def get_person_no_user():
+ person = PersonFactory()
+ person.user = None
+ person.save()
+ return person
+
+class PersonTests(TestCase):
def test_ajax_search_emails(self):
draft = make_test_data()
person = draft.ad
@@ -87,18 +93,43 @@ class PersonTests(TestCase):
Person.objects.create(name="Duplicate Test")
self.assertTrue("possible duplicate" in outbox[0]["Subject"].lower())
+ def test_merge(self):
+ url = urlreverse("ietf.person.views.merge")
+ login_testing_unauthorized(self, "secretary", url)
+ r = self.client.get(url)
+ self.assertEqual(r.status_code, 200)
+
+ def test_merge_with_params(self):
+ p1 = get_person_no_user()
+ p2 = PersonFactory()
+ url = urlreverse("ietf.person.views.merge") + "?source={}&target={}".format(p1.pk, p2.pk)
+ login_testing_unauthorized(self, "secretary", url)
+ r = self.client.get(url)
+ self.assertContains(r, 'retaining login', status_code=200)
+
+ def test_merge_with_params_bad_id(self):
+ url = urlreverse("ietf.person.views.merge") + "?source=1000&target=2000"
+ login_testing_unauthorized(self, "secretary", url)
+ r = self.client.get(url)
+ self.assertContains(r, 'ID does not exist', status_code=200)
+
+ def test_merge_post(self):
+ p1 = get_person_no_user()
+ p2 = PersonFactory()
+ url = urlreverse("ietf.person.views.merge")
+ expected_url = urlreverse("ietf.secr.rolodex.views.view", kwargs={'id': p2.pk})
+ login_testing_unauthorized(self, "secretary", url)
+ data = {'source': p1.pk, 'target': p2.pk}
+ r = self.client.post(url, data, follow=True)
+ self.assertRedirects(r, expected_url)
+ self.assertContains(r, 'Merged', status_code=200)
+ self.assertFalse(Person.objects.filter(pk=p1.pk))
class PersonUtilsTests(TestCase):
- def get_person_no_user(self):
- person = PersonFactory()
- person.user = None
- person.save()
- return person
-
def test_determine_merge_order(self):
- p1 = self.get_person_no_user()
+ p1 = get_person_no_user()
p2 = PersonFactory()
- p3 = self.get_person_no_user()
+ p3 = get_person_no_user()
p4 = PersonFactory()
# target has User
@@ -130,12 +161,12 @@ class PersonUtilsTests(TestCase):
self.assertTrue('IETF Datatracker records merged' in outbox[-1]['Subject'])
def test_handle_users(self):
- source1 = self.get_person_no_user()
- target1 = self.get_person_no_user()
- source2 = self.get_person_no_user()
+ source1 = get_person_no_user()
+ target1 = get_person_no_user()
+ source2 = get_person_no_user()
target2 = PersonFactory()
source3 = PersonFactory()
- target3 = self.get_person_no_user()
+ target3 = get_person_no_user()
source4 = PersonFactory()
target4 = PersonFactory()
@@ -224,4 +255,4 @@ class PersonUtilsTests(TestCase):
merge_users(source, target)
self.assertIn(communitylist, target.communitylist_set.all())
self.assertIn(feedback, target.feedback_set.all())
- self.assertIn(nomination, target.nomination_set.all())
+ self.assertIn(nomination, target.nomination_set.all())
diff --git a/ietf/person/urls.py b/ietf/person/urls.py
index 3a801bfbe..e9b68d9ed 100644
--- a/ietf/person/urls.py
+++ b/ietf/person/urls.py
@@ -2,6 +2,7 @@ from ietf.person import views, ajax
from ietf.utils.urls import url
urlpatterns = [
+ url(r'^merge/$', views.merge),
url(r'^search/(?P<model_name>(person|email))/$', views.ajax_select2_search),
url(r'^(?P<personid>[a-z0-9]+).json$', ajax.person_json),
url(r'^(?P<email_or_name>[^/]+)$', views.profile),
diff --git a/ietf/person/views.py b/ietf/person/views.py
index 70d36cde2..1821799ae 100644
--- a/ietf/person/views.py
+++ b/ietf/person/views.py
@@ -1,13 +1,19 @@
import datetime
+from StringIO import StringIO
+from django.contrib import messages
from django.db.models import Q
from django.http import HttpResponse, Http404
-from django.shortcuts import render, get_object_or_404
+from django.shortcuts import render, get_object_or_404, redirect
import debug # pyflakes:ignore
+from ietf.ietfauth.utils import role_required
from ietf.person.models import Email, Person, Alias
from ietf.person.fields import select2_id_name_json
+from ietf.person.forms import MergeForm
+from ietf.person.utils import handle_users, merge_persons
+
def ajax_select2_search(request, model_name):
if model_name == "email":
@@ -37,7 +43,7 @@ def ajax_select2_search(request, model_name):
all_emails = request.GET.get("a", "0") == "1"
if model == Email:
- objs = objs.exclude(person=None).order_by('person__name')
+ objs = objs.exclude(person=None).order_by('person__name')
if not all_emails:
objs = objs.filter(active=True)
if only_users:
@@ -66,3 +72,54 @@ def profile(request, email_or_name):
if not persons:
raise Http404
return render(request, 'person/profile.html', {'persons': persons, 'today':datetime.date.today()})
+
+
+@role_required("Secretariat")
+def merge(request):
+ form = MergeForm()
+ method = 'get'
+ change_details = ''
+ warn_messages = []
+ source = None
+ target = None
+
+ if request.method == "GET":
+ form = MergeForm()
+ if request.GET:
+ form = MergeForm(request.GET)
+ if form.is_valid():
+ source = form.cleaned_data.get('source')
+ target = form.cleaned_data.get('target')
+ if source.user and target.user:
+ warn_messages.append('WARNING: Both Person records have logins. Be sure to specify the record to keep in the Target field.')
+ if source.user.last_login > target.user.last_login:
+ warn_messages.append('WARNING: The most recently used login is being deleted!')
+ change_details = handle_users(source, target, check_only=True)
+ method = 'post'
+ else:
+ method = 'get'
+
+ if request.method == "POST":
+ form = MergeForm(request.POST)
+ if form.is_valid():
+ source = form.cleaned_data.get('source')
+ source_id = source.id
+ target = form.cleaned_data.get('target')
+ # Do merge with force
+ output = StringIO()
+ success, changes = merge_persons(source, target, file=output)
+ if success:
+ messages.success(request, u'Merged {} ({}) to {} ({}). {})'.format(
+ source.name, source_id, target.name, target.id, changes))
+ else:
+ messages.error(request, output)
+ return redirect('ietf.secr.rolodex.views.view', id=target.pk)
+
+ return render(request, 'person/merge.html', {
+ 'form': form,
+ 'method': method,
+ 'change_details': change_details,
+ 'source': source,
+ 'target': target,
+ 'warn_messages': warn_messages,
+ })
diff --git a/ietf/static/ietf/css/ietf.css b/ietf/static/ietf/css/ietf.css
index 2259f605c..e86400958 100644
--- a/ietf/static/ietf/css/ietf.css
+++ b/ietf/static/ietf/css/ietf.css
@@ -911,3 +911,10 @@ blockquote {
line-height: 1.0;
cursor: pointer;
}
+
+/* === Person ===================================================== */
+
+.person-info {
+ margin-bottom: 1.5em;
+}
+
diff --git a/ietf/templates/person/mail/possible_duplicates.txt b/ietf/templates/person/mail/possible_duplicates.txt
index 1c1772cf7..614eec268 100644
--- a/ietf/templates/person/mail/possible_duplicates.txt
+++ b/ietf/templates/person/mail/possible_duplicates.txt
@@ -16,3 +16,6 @@ Please check to see if they represent the same actual person, and if so, merge t
username: {% if person.user %}{{person.user.username}}{% else %}None{% endif %}
{% endfor %} {% endautoescape %}
+
+Merge Link:
+{% url "ietf.person.views.merge" %}?source={{ persons.0.pk}}&target={{ persons.1.pk }}
\ No newline at end of file
diff --git a/ietf/templates/person/merge.html b/ietf/templates/person/merge.html
new file mode 100644
index 000000000..98fbd8b2b
--- /dev/null
+++ b/ietf/templates/person/merge.html
@@ -0,0 +1,48 @@
+{% extends "base.html" %}
+{# Copyright The IETF Trust 2015, All Rights Reserved #}
+{% load staticfiles %}
+{% load bootstrap3 %}
+
+{% block title %}Merge Persons{% endblock %}
+
+{% block content %}
+
+ <h1>Merge Person Records</h1>
+ <p>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.</p>
+ <form action="" method="{{ method }}">{% if method == 'post' %}{% csrf_token %}{% endif %}
+ <div class="row">
+ <div class="form-group">
+ <div class="col-md-6">
+ {% bootstrap_field form.source %}
+ {% if source %}
+ {% with person=source %}
+ {% include "person/person_info.html" %}
+ {% endwith %}
+ {% endif %}
+ </div>
+
+ <div class="col-md-6">
+ {% bootstrap_field form.target %}
+ {% if target %}
+ {% with person=target %}
+ {% include "person/person_info.html" %}
+ {% endwith %}
+ {% endif %}
+ </div>
+ </div>
+ </div>
+ {% if change_details %}
+ <div class="alert alert-info" role="alert">{{ change_details }}</div>
+ {% endif %}
+ {% if warn_messages %}
+ {% for message in warn_messages %}
+ <div class="alert alert-warning" role="alert">{{ message }}</div>
+ {% endfor %}
+ {% endif %}
+ {% if method == 'post' %}
+ <a class="btn btn-default" href="{% url 'ietf.person.views.merge' %}?source={{ target.pk }}&target={{ source.pk }}" role="button">Swap</a>
+ {% endif %}
+ <button type="submit" class="btn btn-default">{% if method == 'post' %}Merge{% else %}Submit{% endif %}</button>
+ </form>
+
+{% endblock %}
diff --git a/ietf/templates/person/person_info.html b/ietf/templates/person/person_info.html
new file mode 100644
index 000000000..c1a800bc4
--- /dev/null
+++ b/ietf/templates/person/person_info.html
@@ -0,0 +1,22 @@
+ <div class="person-info">
+ <div class="row">
+ <div class="col-md-2">Name:</div><div class="col-md-10">{{ person.name }}</div>
+ </div>
+ <div class="row">
+ <div class="col-md-2">Address:</div><div class="col-md-10">{{ person.address }}</div>
+ </div>
+ <div class="row">
+ <div class="col-md-2">Affiliation:</div><div class="col-md-10">{{ person.affiliation}}</div>
+ </div>
+ <div class="row">
+ <div class="col-md-2">Login:</div><div class="col-md-10">{% if person.user %}{{ person.user }} (last used: {% if person.user.last_login %}{{ person.user.last_login|date:"Y-m-d" }}{% else %}never{% endif %}){% endif %}</div>
+ </div>
+ {% for email in person.email_set.all %}
+ <div class="row">
+ <div class="col-md-2">{% if forloop.first %}Email{{ person.email_set.count|pluralize }}:{% endif %}</div><div class="col-md-10">{{ email.address }}</div>
+ </div>
+ {% endfor %}
+ <div class="row">
+ <div class="col-md-2">Role{{ person.role_set.count|pluralize }}:</div><div class="col-md-10">{% for role in person.role_set.all %}{{ role.name }} {{ role.group.acronym }}{% if not forloop.last %}, {% endif %}{% endfor %}</div>
+ </div>
+ </div>
\ No newline at end of file