diff --git a/ietf/nomcom/admin.py b/ietf/nomcom/admin.py index 21a4b59d9..d7456d1ba 100644 --- a/ietf/nomcom/admin.py +++ b/ietf/nomcom/admin.py @@ -13,8 +13,9 @@ class NominationAdmin(admin.ModelAdmin): class NomineeAdmin(admin.ModelAdmin): - list_display = ('email',) - + list_display = ('email', 'person', 'duplicated', 'nomcom') + search_fields = ('email__address', 'person__name', ) + list_filter = ('nomcom', ) class NomineePositionAdmin(admin.ModelAdmin): pass diff --git a/ietf/nomcom/forms.py b/ietf/nomcom/forms.py index 273b97da4..dfdc431b3 100644 --- a/ietf/nomcom/forms.py +++ b/ietf/nomcom/forms.py @@ -11,7 +11,7 @@ from django.utils.html import mark_safe from ietf.dbtemplate.forms import DBTemplateForm from ietf.group.models import Group, Role from ietf.ietfauth.utils import role_required -from ietf.name.models import RoleName, FeedbackTypeName +from ietf.name.models import RoleName, FeedbackTypeName, NomineePositionStateName from ietf.nomcom.models import ( NomCom, Nomination, Nominee, NomineePosition, Position, Feedback, ReminderDates ) from ietf.nomcom.utils import (NOMINATION_RECEIPT_TEMPLATE, FEEDBACK_RECEIPT_TEMPLATE, @@ -19,7 +19,8 @@ from ietf.nomcom.utils import (NOMINATION_RECEIPT_TEMPLATE, FEEDBACK_RECEIPT_TEM make_nomineeposition, make_nomineeposition_for_newperson, create_feedback_email) from ietf.person.models import Email -from ietf.person.fields import SearchableEmailField, SearchablePersonField, SearchablePersonsField +from ietf.person.fields import (SearchableEmailField, SearchableEmailsField, + SearchablePersonField, SearchablePersonsField ) from ietf.utils.fields import MultiEmailField from ietf.utils.mail import send_mail from ietf.mailtrigger.utils import gather_address_lists @@ -221,14 +222,95 @@ class EditNomcomForm(forms.ModelForm): raise forms.ValidationError('Invalid public key. Error was: %s' % error) -class MergeForm(forms.Form): +class MergeNomineeForm(forms.Form): + + primary_email = SearchableEmailField( + help_text="Select the email of the Nominee record you want to use as the primary record.") + secondary_emails = SearchableEmailsField( + help_text="Select all the duplicates that should be consolidated with the primary " + "Nominee record. Nominations already received with any of these email address " + "will be moved to show under the primary address." ) + + def __init__(self, *args, **kwargs): + self.nomcom = kwargs.pop('nomcom', None) + super(MergeNomineeForm, self).__init__(*args, **kwargs) + + def clean_primary_email(self): + email = self.cleaned_data['primary_email'] + nominees = Nominee.objects.get_by_nomcom(self.nomcom).not_duplicated().filter(email__address=email) + if not nominees: + msg = "No nominee with this email exists" + self._errors["primary_email"] = self.error_class([msg]) + + return email + + def clean_secondary_emails(self): + emails = self.cleaned_data['secondary_emails'] + for email in emails: + nominees = Nominee.objects.get_by_nomcom(self.nomcom).not_duplicated().filter(email__address=email) + if not nominees: + msg = "No nominee with email %s exists" % email + self._errors["primary_email"] = self.error_class([msg]) + break + + return emails + + def clean(self): + primary_email = self.cleaned_data.get("primary_email") + secondary_emails = self.cleaned_data.get("secondary_emails") + if primary_email and secondary_emails: + if primary_email in secondary_emails: + msg = "Primary and secondary email address must be differents" + self._errors["primary_email"] = self.error_class([msg]) + return self.cleaned_data + + def save(self): + primary_email = self.cleaned_data.get("primary_email") + secondary_emails = self.cleaned_data.get("secondary_emails") + + primary_nominee = Nominee.objects.get_by_nomcom(self.nomcom).get(email__address=primary_email) + while primary_nominee.duplicated: + primary_nominee = primary_nominee.duplicated + secondary_nominees = Nominee.objects.get_by_nomcom(self.nomcom).filter(email__address__in=secondary_emails) + for nominee in secondary_nominees: + # move nominations + nominee.nomination_set.all().update(nominee=primary_nominee) + # move feedback + for fb in nominee.feedback_set.all(): + fb.nominees.remove(nominee) + fb.nominees.add(primary_nominee) + # move nomineepositions + for nominee_position in nominee.nomineeposition_set.all(): + primary_nominee_positions = NomineePosition.objects.filter(position=nominee_position.position, + nominee=primary_nominee) + primary_nominee_position = primary_nominee_positions and primary_nominee_positions[0] or None + + if primary_nominee_position: + # if already a nomineeposition object for a position and nominee, + # update the nomineepostion of primary nominee with the state + if nominee_position.state.slug == 'accepted' or primary_nominee_position.state.slug == 'accepted': + primary_nominee_position.state = NomineePositionStateName.objects.get(slug='accepted') + primary_nominee_position.save() + if nominee_position.state.slug == 'declined' and primary_nominee_position.state.slug == 'pending': + primary_nominee_position.state = NomineePositionStateName.objects.get(slug='declined') + primary_nominee_position.save() + else: + # It is not allowed two or more nomineeposition objects with same position and nominee + # move nominee_position object to primary nominee + nominee_position.nominee = primary_nominee + nominee_position.save() + + nominee.duplicated = primary_nominee + nominee.save() + +class MergePersonForm(forms.Form): primary_person = SearchablePersonField(help_text="Select the person you want the datatracker to keep") duplicate_persons = SearchablePersonsField(help_text="Select all the duplicates that should be merged into the primary person record") def __init__(self, *args, **kwargs): self.nomcom = kwargs.pop('nomcom', None) - super(MergeForm, self).__init__(*args, **kwargs) + super(MergePersonForm, self).__init__(*args, **kwargs) def clean(self): primary_person = self.cleaned_data.get("primary_person") @@ -767,6 +849,20 @@ class EditNomineeForm(forms.ModelForm): self.fields['nominee_email'].initial = self.instance.email self.fields['nominee_email'].help_text = "If the address you are looking for does not appear in this list, ask the nominee (or the secretariat) to add the address to their datatracker account and ensure it is marked as active." + def clean(self): + nominee_email = self.cleaned_data.get("nominee_email") + others = Nominee.objects.filter(email=nominee_email, nomcom=self.instance.nomcom) + if others.exists(): + msg = ( "Changing email address for %s (#%s): There already exists a nominee " + "with email address <%s>: %s -- please use the " + "Merge Nominee " + "form instead." % ( + self.instance.name(), + self.instance.pk, nominee_email, + (", ".join( "%s (#%s)" %( n.name(), n.pk) for n in others))) ) + raise forms.ValidationError(mark_safe(msg)) + return self.cleaned_data + def save(self, commit=True): nominee = super(EditNomineeForm, self).save(commit=False) nominee_email = self.cleaned_data.get("nominee_email") diff --git a/ietf/nomcom/tests.py b/ietf/nomcom/tests.py index ebdd60fa5..6c9dab942 100644 --- a/ietf/nomcom/tests.py +++ b/ietf/nomcom/tests.py @@ -3,6 +3,7 @@ import datetime import os import shutil +import urlparse from pyquery import PyQuery import StringIO @@ -76,7 +77,8 @@ class NomcomViewsTest(TestCase): # private urls self.private_index_url = reverse('nomcom_private_index', kwargs={'year': self.year}) - self.private_merge_url = reverse('nomcom_private_merge', kwargs={'year': self.year}) + self.private_merge_person_url = reverse('ietf.nomcom.views.private_merge_person', kwargs={'year': self.year}) + self.private_merge_nominee_url = reverse('ietf.nomcom.views.private_merge_nominee', kwargs={'year': self.year}) self.edit_members_url = reverse('nomcom_edit_members', kwargs={'year': self.year}) self.edit_nomcom_url = reverse('nomcom_edit_nomcom', kwargs={'year': self.year}) self.private_nominate_url = reverse('nomcom_private_nominate', kwargs={'year': self.year}) @@ -175,6 +177,211 @@ class NomcomViewsTest(TestCase): self.client.logout() + def test_private_merge_view(self): + """Verify private nominee merge view""" + + nominees = [u'nominee0@example.com', + u'nominee1@example.com', + u'nominee2@example.com', + u'nominee3@example.com'] + + # do nominations + login_testing_unauthorized(self, COMMUNITY_USER, self.public_nominate_url) + self.nominate_view(public=True, + nominee_email=nominees[0], + position='IAOC') + self.nominate_view(public=True, + nominee_email=nominees[0], + position='IAOC') + self.nominate_view(public=True, + nominee_email=nominees[0], + position='IAB') + self.nominate_view(public=True, + nominee_email=nominees[0], + position='TSV') + self.nominate_view(public=True, + nominee_email=nominees[1], + position='IAOC') + self.nominate_view(public=True, + nominee_email=nominees[1], + position='IAOC') + self.nominate_view(public=True, + nominee_email=nominees[2], + position='IAB') + self.nominate_view(public=True, + nominee_email=nominees[2], + position='IAB') + self.nominate_view(public=True, + nominee_email=nominees[3], + position='TSV') + self.nominate_view(public=True, + nominee_email=nominees[3], + position='TSV') + # Check nominee positions + self.assertEqual(NomineePosition.objects.count(), 6) + self.assertEqual(Feedback.objects.nominations().count(), 10) + + # Accept and declined nominations + nominee_position = NomineePosition.objects.get(position__name='IAOC', + nominee__email__address=nominees[0]) + nominee_position.state = NomineePositionStateName.objects.get(slug='accepted') + nominee_position.save() + + nominee_position = NomineePosition.objects.get(position__name='IAOC', + nominee__email__address=nominees[1]) + nominee_position.state = NomineePositionStateName.objects.get(slug='declined') + nominee_position.save() + + nominee_position = NomineePosition.objects.get(position__name='IAB', + nominee__email__address=nominees[2]) + nominee_position.state = NomineePositionStateName.objects.get(slug='declined') + nominee_position.save() + + nominee_position = NomineePosition.objects.get(position__name='TSV', + nominee__email__address=nominees[3]) + nominee_position.state = NomineePositionStateName.objects.get(slug='accepted') + nominee_position.save() + + self.client.logout() + + # fill questionnaires (internally the function does new nominations) + self.access_chair_url(self.add_questionnaire_url) + + self.add_questionnaire(public=False, + nominee_email=nominees[0], + position='IAOC') + self.add_questionnaire(public=False, + nominee_email=nominees[1], + position='IAOC') + self.add_questionnaire(public=False, + nominee_email=nominees[2], + position='IAB') + self.add_questionnaire(public=False, + nominee_email=nominees[3], + position='TSV') + self.assertEqual(Feedback.objects.questionnaires().count(), 4) + + self.client.logout() + + ## Add feedbacks (internally the function does new nominations) + self.access_member_url(self.private_feedback_url) + self.feedback_view(public=False, + nominee_email=nominees[0], + position='IAOC') + self.feedback_view(public=False, + nominee_email=nominees[1], + position='IAOC') + self.feedback_view(public=False, + nominee_email=nominees[2], + position='IAB') + self.feedback_view(public=False, + nominee_email=nominees[3], + position='TSV') + + self.assertEqual(Feedback.objects.comments().count(), 4) + self.assertEqual(Feedback.objects.nominations().count(), 18) + self.assertEqual(Feedback.objects.nominations().filter(nominees__email__address=nominees[0]).count(), 6) + self.assertEqual(Feedback.objects.nominations().filter(nominees__email__address=nominees[1]).count(), 4) + self.assertEqual(Feedback.objects.nominations().filter(nominees__email__address=nominees[2]).count(), 4) + self.assertEqual(Feedback.objects.nominations().filter(nominees__email__address=nominees[3]).count(), 4) + for nominee in nominees: + self.assertEqual(Feedback.objects.comments().filter(nominees__email__address=nominee).count(), + 1) + self.assertEqual(Feedback.objects.questionnaires().filter(nominees__email__address=nominee).count(), + 1) + + self.client.logout() + + ## merge nominations + self.access_chair_url(self.private_merge_nominee_url) + + test_data = {"secondary_emails": "%s, %s" % (nominees[0], nominees[1]), + "primary_email": nominees[0]} + response = self.client.post(self.private_merge_nominee_url, test_data) + self.assertEqual(response.status_code, 200) + q = PyQuery(response.content) + self.assertTrue(q("form .has-error")) + + test_data = {"primary_email": nominees[0], + "secondary_emails": ""} + response = self.client.post(self.private_merge_nominee_url, test_data) + self.assertEqual(response.status_code, 200) + q = PyQuery(response.content) + self.assertTrue(q("form .has-error")) + + test_data = {"primary_email": "", + "secondary_emails": nominees[0]} + response = self.client.post(self.private_merge_nominee_url, test_data) + self.assertEqual(response.status_code, 200) + q = PyQuery(response.content) + self.assertTrue(q("form .has-error")) + + test_data = {"primary_email": "unknown@example.com", + "secondary_emails": nominees[0]} + response = self.client.post(self.private_merge_nominee_url, test_data) + self.assertEqual(response.status_code, 200) + q = PyQuery(response.content) + self.assertTrue(q("form .has-error")) + + test_data = {"primary_email": nominees[0], + "secondary_emails": "unknown@example.com"} + response = self.client.post(self.private_merge_nominee_url, test_data) + self.assertEqual(response.status_code, 200) + q = PyQuery(response.content) + self.assertTrue(q("form .has-error")) + + test_data = {"secondary_emails": """%s, + %s, + %s""" % (nominees[1], nominees[2], nominees[3]), + "primary_email": nominees[0]} + + response = self.client.post(self.private_merge_nominee_url, test_data) + self.assertEqual(response.status_code, 302) + redirect_url = response["Location"] + redirect_path = urlparse.urlparse(redirect_url).path + self.assertEqual(redirect_path, reverse('ietf.nomcom.views.private_index', kwargs={"year": NOMCOM_YEAR})) + + response = self.client.get(redirect_url) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "alert-success") + + self.assertEqual(Nominee.objects.filter(email__address=nominees[1], + duplicated__isnull=False).count(), 1) + self.assertEqual(Nominee.objects.filter(email__address=nominees[2], + duplicated__isnull=False).count(), 1) + self.assertEqual(Nominee.objects.filter(email__address=nominees[3], + duplicated__isnull=False).count(), 1) + + nominee = Nominee.objects.get(email__address=nominees[0]) + + self.assertEqual(Nomination.objects.filter(nominee=nominee).count(), 18) + self.assertEqual(Feedback.objects.nominations().filter(nominees__in=[nominee]).count(), + 18) + self.assertEqual(Feedback.objects.comments().filter(nominees__in=[nominee]).count(), + 4) + self.assertEqual(Feedback.objects.questionnaires().filter(nominees__in=[nominee]).count(), + 4) + + for nominee_email in nominees[1:]: + self.assertEqual(Feedback.objects.nominations().filter(nominees__email__address=nominee_email).count(), + 0) + self.assertEqual(Feedback.objects.comments().filter(nominees__email__address=nominee_email).count(), + 0) + self.assertEqual(Feedback.objects.questionnaires().filter(nominees__email__address=nominee_email).count(), + 0) + + self.assertEqual(NomineePosition.objects.filter(nominee=nominee).count(), 3) + + # Check nominations state + self.assertEqual(NomineePosition.objects.get(position__name='TSV', + nominee=nominee).state.slug, u'accepted') + self.assertEqual(NomineePosition.objects.get(position__name='IAOC', + nominee=nominee).state.slug, u'accepted') + self.assertEqual(NomineePosition.objects.get(position__name='IAB', + nominee=nominee).state.slug, u'declined') + + self.client.logout() + def change_members(self, members): members_emails = u','.join(['%s%s' % (member, EMAIL_DOMAIN) for member in members]) test_data = {'members': members_emails, @@ -982,7 +1189,7 @@ class InactiveNomcomTests(TestCase): self._test_send_reminders_closed('questionnaire') def test_merge_closed(self): - url = reverse('nomcom_private_merge', kwargs={'year':self.nc.year()}) + url = reverse('ietf.nomcom.views.private_merge_person', kwargs={'year':self.nc.year()}) login_testing_unauthorized(self, self.chair.user.username, url) response = self.client.get(url) q = PyQuery(response.content) @@ -1485,7 +1692,7 @@ Junk body for testing def test_request_merge(self): nominee1, nominee2 = self.nc.nominee_set.all()[:2] - url = reverse('nomcom_private_merge',kwargs={'year':self.nc.year()}) + url = reverse('ietf.nomcom.views.private_merge_person',kwargs={'year':self.nc.year()}) login_testing_unauthorized(self,self.chair.user.username,url) empty_outbox() response = self.client.get(url) diff --git a/ietf/nomcom/urls.py b/ietf/nomcom/urls.py index 69473b229..f62140050 100644 --- a/ietf/nomcom/urls.py +++ b/ietf/nomcom/urls.py @@ -1,6 +1,7 @@ from django.conf.urls import patterns, url from ietf.nomcom.forms import EditMembersForm, EditMembersFormPreview +from ietf.nomcom import views urlpatterns = patterns('ietf.nomcom.views', url(r'^$', 'index'), @@ -18,7 +19,8 @@ urlpatterns = patterns('ietf.nomcom.views', url(r'^(?P\d{4})/private/view-feedback/pending/$', 'view_feedback_pending', name='nomcom_view_feedback_pending'), url(r'^(?P\d{4})/private/view-feedback/nominee/(?P\d+)$', 'view_feedback_nominee', name='nomcom_view_feedback_nominee'), url(r'^(?P\d{4})/private/edit/nominee/(?P\d+)$', 'edit_nominee', name='nomcom_edit_nominee'), - url(r'^(?P\d{4})/private/merge/$', 'private_merge', name='nomcom_private_merge'), + url(r'^(?P\d{4})/private/merge-nominee/?$', views.private_merge_nominee), + url(r'^(?P\d{4})/private/merge-person/?$', views.private_merge_person), # url(r'^(?P\d{4})/private/send-reminder-mail/$', RedirectView.as_view(url=reverse_lazy('nomcom_send_reminder_mail',kwargs={'year':year,'type':'accept'}))), url(r'^(?P\d{4})/private/send-reminder-mail/(?P\w+)/$', 'send_reminder_mail', name='nomcom_send_reminder_mail'), url(r'^(?P\d{4})/private/edit-members/$', EditMembersFormPreview(EditMembersForm), name='nomcom_edit_members'), diff --git a/ietf/nomcom/views.py b/ietf/nomcom/views.py index 6326f6f42..a43444bed 100644 --- a/ietf/nomcom/views.py +++ b/ietf/nomcom/views.py @@ -23,7 +23,7 @@ from ietf.message.models import Message from ietf.nomcom.decorators import nomcom_private_key_required from ietf.nomcom.forms import (NominateForm, NominateNewPersonForm, FeedbackForm, QuestionnaireForm, - MergeForm, NomComTemplateForm, PositionForm, + MergeNomineeForm, MergePersonForm, NomComTemplateForm, PositionForm, PrivateKeyForm, EditNomcomForm, EditNomineeForm, PendingFeedbackForm, ReminderDatesForm, FullFeedbackFormSet, FeedbackEmailForm, NominationResponseCommentForm) @@ -264,30 +264,54 @@ def send_reminder_mail(request, year, type): @role_required("Nomcom Chair", "Nomcom Advisor") -def private_merge(request, year): +def private_merge_person(request, year): nomcom = get_nomcom_by_year(year) if nomcom.group.state_id != 'active': messages.warning(request, "This Nomcom is not active.") form = None else: if request.method == 'POST': - form = MergeForm(request.POST, nomcom=nomcom ) + form = MergePersonForm(request.POST, nomcom=nomcom ) if form.is_valid(): form.save() messages.success(request, 'A merge request has been sent to the secretariat.') return redirect('nomcom_private_index',year=year) else: - form = MergeForm(nomcom=nomcom) + form = MergePersonForm(nomcom=nomcom) - return render_to_response('nomcom/private_merge.html', + return render_to_response('nomcom/private_merge_person.html', {'nomcom': nomcom, 'year': year, 'form': form, - 'selected': 'merge', + 'selected': 'merge_person', 'is_chair_task' : True, }, RequestContext(request)) +@role_required("Nomcom Chair", "Nomcom Advisor") +def private_merge_nominee(request, year): + nomcom = get_nomcom_by_year(year) + if nomcom.group.state_id != 'active': + messages.warning(request, "This Nomcom is not active.") + form = None + else: + if request.method == 'POST': + form = MergeNomineeForm(request.POST, nomcom=nomcom ) + if form.is_valid(): + form.save() + messages.success(request, 'The Nominee records have been joined.') + return redirect('nomcom_private_index',year=year) + else: + form = MergeNomineeForm(nomcom=nomcom) + + return render_to_response('nomcom/private_merge_nominee.html', + {'nomcom': nomcom, + 'year': year, + 'form': form, + 'selected': 'merge_nominee', + 'is_chair_task' : True, + }, RequestContext(request)) + def requirements(request, year): nomcom = get_nomcom_by_year(year) positions = nomcom.position_set.all() diff --git a/ietf/templates/nomcom/nomcom_private_base.html b/ietf/templates/nomcom/nomcom_private_base.html index 78fe8256a..72419bd4d 100644 --- a/ietf/templates/nomcom/nomcom_private_base.html +++ b/ietf/templates/nomcom/nomcom_private_base.html @@ -33,7 +33,8 @@
  • Enter questionnaire response
  • Send accept reminder
  • Send questionnaire reminder
    • -
  • Request Nominee Merge
    • +
  • Request Person Record Merge
    • +
  • Merge Nominee Records
    • {% endif %}
  • Edit Settings
    • diff --git a/ietf/templates/nomcom/private_merge_nominee.html b/ietf/templates/nomcom/private_merge_nominee.html new file mode 100644 index 000000000..b8d8f2d92 --- /dev/null +++ b/ietf/templates/nomcom/private_merge_nominee.html @@ -0,0 +1,50 @@ +{% extends "nomcom/nomcom_private_base.html" %} +{# Copyright The IETF Trust 2015, All Rights Reserved #} +{% load origin %} +{% load staticfiles %} + +{% load bootstrap3 %} + +{% block pagehead %} + + +{% endblock %} + +{% block subtitle %} - Merge Nominee Records {% endblock %} + +{% block nomcom_content %} + {% origin %} +

    Merge Nominee Records

    + +

    + The nomination system encourages the community to nominate people by selecting + their email address from the set of addresses the tracker already knows. In order + to allow a person who does not yet have a datatracker account to be nominated, the + system also provides a way for the community to nominate people with a new, + previously unknown email address. +

    +

    + Occasionally, this results in multiple Nominee records which are associated with + the same person, but using different email addresses. In this case, the form to + Request Merge of Person Records + cannot be use, since it is the nominee records, not the person records which needs + to be merged. When this happens, you can use this form to merge the Nominee records. +

    + + {% if form %} +
    + {% csrf_token %} + {% bootstrap_form form %} + + {% buttons %} + + {% endbuttons %} +
    + {% endif %} + +{% endblock %} + +{% block js %} + + +{% endblock %} diff --git a/ietf/templates/nomcom/private_merge.html b/ietf/templates/nomcom/private_merge_person.html similarity index 81% rename from ietf/templates/nomcom/private_merge.html rename to ietf/templates/nomcom/private_merge_person.html index fce825ec2..c1d95eca4 100644 --- a/ietf/templates/nomcom/private_merge.html +++ b/ietf/templates/nomcom/private_merge_person.html @@ -10,11 +10,11 @@ {% endblock %} -{% block subtitle %} - Request Nominee Merge {% endblock %} +{% block subtitle %} - Request Merge of Person Records {% endblock %} {% block nomcom_content %} {% origin %} -

    Request Nominee Merge

    +

    Request Merge of Person Records

    The nomination system encourages the community to nominate people by selecting @@ -33,6 +33,13 @@ for verifying that the addresses both belong to the same person, and a tool that can correct the relevant data.

    +

    + On the other hand, if you have multiple Nominee records which refer to the same Person + record, rather than to different Person records for the same individual, you should + use the + Merge Nominee Records + form, not this form. +

    {% if form %}