Give users the ability to split incorrectly merged candidates

ynr/apps/candidates/views/people.py

@@ -13,9 +13,9 @@
     HttpResponsePermanentRedirect,
     HttpResponseRedirect,
 )
-from django.shortcuts import get_object_or_404
+from django.shortcuts import get_object_or_404, redirect
 from django.template.loader import render_to_string
-from django.urls import reverse
+from django.urls import reverse, reverse_lazy
 from django.utils.decorators import method_decorator
 from django.views.decorators.cache import cache_control
 from django.views.generic import FormView, TemplateView, UpdateView, View
@@ -24,7 +24,12 @@
 from elections.mixins import ElectionMixin
 from elections.models import Election
 from elections.uk.forms import SelectBallotForm
-from people.forms.forms import NewPersonForm, UpdatePersonForm
+from people.forms.forms import (
+    NewPersonForm,
+    PersonSplitForm,
+    PersonSplitFormSet,
+    UpdatePersonForm,
+)
 from people.forms.formsets import (
     PersonIdentifierFormsetFactory,
     PersonMembershipFormsetFactory,
@@ -490,6 +495,167 @@ def form_valid(self, all_forms):
         )
 
 
+class PersonSplitView(FormView):
+    template_name = "people/split_person.html"
+    form_class = PersonSplitForm
+
+    def get_review_url(self):
+        person_id = self.kwargs.get("person_id")
+        return reverse_lazy(
+            "review_split_person", kwargs={"person_id": person_id}
+        )
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context["person"] = self.get_person()
+        context["person_id"] = self.kwargs["person_id"]
+        context["formset"] = self.get_form()
+        return context
+
+    def get_initial_data(self, form_class=None):
+        person = self.get_person()
+        #: TO DO: Figure out how to return attribute names and values only for those that are not empty
+        return [
+            {"attribute_name": "name", "attribute_value": person.name},
+            # {"attribute_name": "image", "attribute_value": person.image.image if person.image else None},
+            {"attribute_name": "gender", "attribute_value": person.gender},
+            # {
+            # "attribute_name": "birth_date",
+            # "attribute_value": person.birth_date if person.birth_date else None,
+            # },
+            # {
+            #     "attribute_name": "death_date",
+            #     "attribute_value": person.death_date if person.death_date else None,
+            # },
+            # {"attribute_name": "summary", "attribute_value": person.summary if person.summary else None },
+            # {
+            # "attribute_name": "biography",
+            # "attribute_value": person.biography if person.biography else None,
+            # },
+            # {
+            # "attribute_name": "other_names",
+            # "attribute_value": person.other_names.all if person.other_names.all else None,
+            # },
+            # {
+            #     "attribute_name": "memberships",
+            #     "attribute_value": person.memberships.all if person.memberships.all else None,
+            # },
+        ]
+
+    def get_form(self, form_class=None):
+        return PersonSplitFormSet(
+            initial=self.get_initial_data(),
+        )
+
+    def get_person(self):
+        person_id = self.kwargs.get("person_id")
+        return get_object_or_404(Person, pk=person_id)
+
+    def form_valid(self, formset):
+        choices = {
+            "keep": [],
+            "move": [],
+            "both": [],
+        }
+        for form in formset:
+            attribute_name = form.cleaned_data["attribute_name"]
+            attribute_value = form.cleaned_data["attribute_value"]
+            choice = form.cleaned_data["choice"]
+            person_id = self.kwargs.get("person_id")
+            if choice == "keep":
+                choices["keep"].append({attribute_name: attribute_value})
+            elif choice == "move":
+                choices["move"].append({attribute_name: attribute_value})
+            elif choice == "both":
+                choices["both"].append({attribute_name: attribute_value})
+        if choices:
+            self.request.session["choices"] = choices
+            self.request.session["person_id"] = person_id
+            return redirect(self.get_review_url())
+        return self.form_invalid(formset)
+
+    def post(self, request, *args, **kwargs):
+        formset = PersonSplitFormSet(request.POST)
+        if formset.is_valid():
+            return self.form_valid(formset)
+        return self.form_invalid(formset)
+
+
+class ReviewPersonSplitView(TemplateView):
+    template_name = "people/review_split_person.html"
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context["choices"] = self.request.session.get("choices", {})
+        context["person"] = get_object_or_404(
+            Person, pk=self.request.session.get("person_id")
+        )
+        return context
+
+
+class ConfirmPersonSplitView(TemplateView):
+    template_name = "people/confirm_split_person.html"
+
+    def get_success_url(self, person_id, new_person_id=None):
+        person_id = self.kwargs.get("person_id")
+        if new_person_id:
+            return reverse(
+                "confirm_split_person",
+                kwargs={"person_id": person_id, "new_person_id": new_person_id},
+            )
+        return reverse("confirm_split_person", kwargs={"person_id": person_id})
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        person_id = kwargs.get("person_id")
+        context["person"] = get_object_or_404(Person, pk=person_id)
+        return context
+
+    def post(self, request, *args, **kwargs):
+        person_id = self.request.session.get("person_id")
+        person = get_object_or_404(Person, pk=person_id)
+        choices = request.session.get("choices", {})
+        new_person_data = self.split(person, choices)
+        new_person_id = new_person_data.id if new_person_data else None
+        return redirect(
+            self.get_success_url(
+                person_id=person_id, new_person_id=new_person_id
+            )
+        )
+
+    def split(self, person, choices):
+        person_id = person.id
+        new_person = None
+        if not choices:
+            # TODO: ADD A MESSAGE TO SAY THAT NO CHOICES WERE MADE?
+            # TODO: Do we even need to do this?
+            return redirect("person-view", person_id=person_id)
+        if choices["both"] or choices["move"]:
+            new_person = Person.objects.create()
+        for choice in choices["keep"]:
+            for key, value in choice.items():
+                setattr(person, key, value)
+            person.save()
+        for choice in choices["move"]:
+            # create a new person with the chosen attribute values
+            new_person = Person.objects.create()
+            for key, value in choice.items():
+                setattr(new_person, key, value)
+            # remove this attribute from the original person
+            # TODO: however if this is a required person field, such as name, how can we handle it?
+            # perhaps we need to add a text input to the form in the previous step
+            # to allow the user to enter a new value
+            for key, value in choice.items():
+                setattr(person, key, None)
+        for choice in choices["both"]:
+            # create a new person with the chosen attribute values
+            for key, value in choice.items():
+                setattr(new_person, key, value)
+        person.save()
+        new_person.save()
+        return new_person
+
+
 class NewPersonSelectElectionView(LoginRequiredMixin, FormView):
     """
     For when we know new person's name, but not the election they are standing

ynr/apps/elections/uk/templates/candidates/person-view.html

@@ -19,7 +19,7 @@
 
     <!-- Just Open Graph metadata -->
     <meta property="og:type" content="article" />
-    <meta property="og:image" content="{{ person.get_display_image_url }}" />
+    {% comment %} <meta property="og:image" content="{{ person.get_display_image_url }}" /> {% endcomment %}
     <meta property="og:image:height" content="80" />
     <meta property="og:image:width" content="80" />
     <meta property="og:site_name" content="{{ site.name }}" />
@@ -30,7 +30,7 @@
     {% if settings.TWITTER_USERNAME %}
       <meta name="twitter:site" content="@{{ settings.TWITTER_USERNAME }}" />
     {% endif %}
-    <meta property="twitter:image" content="{{ person.get_display_image_url }}" />
+    {% comment %} <meta property="twitter:image" content="{{ person.get_display_image_url }}" /> {% endcomment %}
     <meta property="twitter:image:width" content="160" />
     <meta property="twitter:image:height" content="160" />
 {% endblock %}
@@ -48,7 +48,7 @@
 <div class="person__hero">
   <div class="person__photo">
 
-    <img class="person-avatar" src="{{ person.get_display_image_url }}"/>
+    {% comment %} <img class="person-avatar" src="{{ person.get_display_image_url }}"/> {% endcomment %}
     {% if not person.person_image and user.is_authenticated %}
       <a class="upload-photo" href="{% url 'photo-upload' person.id %}">
         Upload photo
@@ -256,6 +256,27 @@ <h2>Is this a duplicate person?</h2>
     </p>
     <input type="submit" class="button alert" value="Suggest duplicate">
   </form>
+
+  <div class="person__actions__action person__actions__merge">
+    <h2>Split this person</h2>
+    {% if user_can_edit and person_edits_allowed %}
+      <p>Has this person been merged incorrectly?</p>
+      {% if user.is_authenticated %}
+        <a href="{% url 'person-split' person.id %}" class="button">Split person</a>
+        {% if person.queued_image %} 
+          {% if user_can_review_photos %}
+            <p>{{ person.name }} has a photo that needs to be reviewed</p>
+            <a href="{{ person.get_absolute_queued_image_url}}" class="button">Review image</a>
+          {% endif %} 
+        {% endif %} 
+      {% else %}
+        <a href="{% url 'account_login' %}?next={{ redirect_after_login }}" class="button">Log in to split</a>
+      {% endif %}
+    {% else %}
+      <h2>Edits disabled</h2>
+      {% include 'candidates/_edits_disallowed_message.html' %}
+    {% endif %}
+  </div>
 
   <div class="person__actions__action person__actions__data">
     <h2>Use this data!</h2>

ynr/apps/people/forms/forms.py

@@ -4,6 +4,7 @@
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.core.validators import URLValidator, validate_email
+from django.forms import formset_factory
 from django.utils import timezone
 from django.utils.functional import cached_property
 from facebook_data.tasks import extract_fb_page_id
@@ -14,10 +15,7 @@
     PreviousPartyAffiliationsField,
 )
 from parties.models import Party
-from people.forms.fields import (
-    CurrentUnlockedBallotsField,
-    StrippedCharField,
-)
+from people.forms.fields import CurrentUnlockedBallotsField, StrippedCharField
 from people.helpers import (
     clean_mastodon_username,
     clean_twitter_username,
@@ -407,6 +405,20 @@ class UpdatePersonForm(BasePersonForm):
     pass
 
 
+class PersonSplitForm(forms.Form):
+    CHOICES = [
+        ("keep", "Keep for original person"),
+        ("move", "Move to a new person"),
+        ("both", "Do both"),
+    ]
+    attribute_name = forms.CharField(widget=forms.HiddenInput())
+    attribute_value = forms.CharField(widget=forms.HiddenInput())
+    choice = forms.ChoiceField(choices=CHOICES, widget=forms.RadioSelect)
+
+
+PersonSplitFormSet = formset_factory(PersonSplitForm, extra=0)
+
+
 class OtherNameForm(forms.ModelForm):
     class Meta:
         model = OtherName

ynr/apps/people/splitting.py

@@ -0,0 +1,65 @@
+from collections import defaultdict
+
+from candidates.models.versions import (
+    is_a_merge,
+    version_timestamp_key,
+)
+
+
+class PersonSplitter:
+    """This class is responsible for splitting incorrectly merged candidates.
+    Inverse of ynr/apps/people/merging.py
+    """
+
+    def __init__(self, person):
+        self.person = person
+
+    def merged_version(self, person):
+        """Search through the person versions for the person merge history.
+        and return the last merge version.
+        """
+        versions_data = self.person.versions
+        version_id_to_parent_ids = {}
+        if not self.person.versions:
+            return version_id_to_parent_ids
+        ordered_versions = sorted(versions_data, key=version_timestamp_key)
+        person_id_to_ordered_versions = defaultdict(list)
+        # Divide all the version with the same ID into separate ordered
+        # lists, and record the parent of each version that we get from
+        # doing that:
+        merged_versions = []
+        for version in ordered_versions:
+            version_id = version["version_id"]
+            person_id = version["data"]["id"]
+            versions_for_person_id = person_id_to_ordered_versions[person_id]
+            if versions_for_person_id:
+                last_version_id = versions_for_person_id[-1]["version_id"]
+                version_id_to_parent_ids[version_id] = [last_version_id]
+            else:
+                version_id_to_parent_ids[version_id] = []
+            versions_for_person_id.append(version)
+        # Now go through looking for versions that represent merges. Note
+        # that it's *possible* for someone to create a new version that
+        # doesn't represent a merge but which has a information_source
+        # message that makes it look like one. We try to raise an
+        # exception if this might have happened, by checking that (a) the
+        # person ID in the message also has history in this versions array
+        # and (b) the number of unique person IDs in the versions is one
+        # more than the number of versions that look like merges. We raise
+        # an exception in either of these situations.
+
+        number_of_person_ids = len(person_id_to_ordered_versions.keys())
+        number_of_merges = 0
+        for version in ordered_versions:
+            version_id = version["version_id"]
+            merged_from = is_a_merge(version)
+            if merged_from:
+                number_of_merges += 1
+                # TO DO: Do we want the most recent merged version or should we present the user with options?
+                merged_versions.append(version)
+        if number_of_person_ids != number_of_merges + 1:
+            raise ValueError(
+                "The number of unique person IDs in the versions is not one more than the number of versions that look like merges."
+            )
+
+        return merged_versions[0]["data"]

ynr/apps/people/templates/people/confirm_split_person.html

@@ -0,0 +1,12 @@
+{% extends 'base.html' %}
+{% load thumbnail %}
+{% load static %}
+{% load pipeline %}
+
+{% block content %}
+    <h1>Confirm Split</h1>
+	<h2>Original Person: <a href="{% url 'person-view' new_person_id %}">{{ person_id }}</a></h2>
+
+    <h2>New Person: <a href="{% url 'person-view' new_person_id %}">{{ new_person_id}}</a></h2>
+
+{% endblock %}