Add view for merge person records. Commit ready for merge.

 - Legacy-Id: 14862

Author: rpcross

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")

ietf/person/tests.py

@@ -19,12 +19,18 @@
 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 @@ def test_duplicate_person_name(self):
         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)
 
-class PersonUtilsTests(TestCase):
-    def get_person_no_user(self):
-        person = PersonFactory()
-        person.user = None
-        person.save()
-        return person
+    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 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 @@ def test_send_merge_notification(self):
         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 @@ def test_merge_users(self):
         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())

ietf/person/urls.py

@@ -2,6 +2,7 @@
 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),

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,
+    })

ietf/static/ietf/css/ietf.css

@@ -911,3 +911,10 @@ blockquote {
   line-height: 1.0;
   cursor: pointer;
 }
+
+/* === Person ===================================================== */
+
+.person-info {
+  margin-bottom: 1.5em;
+}
+

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 }}

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 %}

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>