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(person|email))/$', views.ajax_select2_search), url(r'^(?P[a-z0-9]+).json$', ajax.person_json), url(r'^(?P[^/]+)$', 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 %} + +

Merge Person Records

+

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.

+
{% if method == 'post' %}{% csrf_token %}{% endif %} +
+
+
+ {% bootstrap_field form.source %} + {% if source %} + {% with person=source %} + {% include "person/person_info.html" %} + {% endwith %} + {% endif %} +
+ +
+ {% bootstrap_field form.target %} + {% if target %} + {% with person=target %} + {% include "person/person_info.html" %} + {% endwith %} + {% endif %} +
+
+
+ {% if change_details %} + + {% endif %} + {% if warn_messages %} + {% for message in warn_messages %} + + {% endfor %} + {% endif %} + {% if method == 'post' %} + Swap + {% endif %} + +
+ +{% 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 @@ +
+
+
Name:
{{ person.name }}
+
+
+
Address:
{{ person.address }}
+
+
+
Affiliation:
{{ person.affiliation}}
+
+
+
Login:
{% if person.user %}{{ person.user }} (last used: {% if person.user.last_login %}{{ person.user.last_login|date:"Y-m-d" }}{% else %}never{% endif %}){% endif %}
+
+ {% for email in person.email_set.all %} +
+
{% if forloop.first %}Email{{ person.email_set.count|pluralize }}:{% endif %}
{{ email.address }}
+
+ {% endfor %} +
+
Role{{ person.role_set.count|pluralize }}:
{% for role in person.role_set.all %}{{ role.name }} {{ role.group.acronym }}{% if not forloop.last %}, {% endif %}{% endfor %}
+
+
\ No newline at end of file