From 4cb6e7d9b4b5d442ba58830b62f868c9b01a330d Mon Sep 17 00:00:00 2001
From: "J.-S. Caux" <J.S.Caux@uva.nl>
Date: Sat, 17 Nov 2018 19:00:01 +0100
Subject: [PATCH] Improve merging Contributors
---
profiles/forms.py | 18 ++++++++--------
profiles/models.py | 9 ++++----
.../templates/profiles/profile_merge.html | 2 +-
scipost/forms.py | 20 +++++++++++++++++-
.../0018_contributor_duplicate_of.py | 21 +++++++++++++++++++
scipost/models.py | 13 +++++++++++-
scipost/views.py | 13 +++++++++---
...contributor_duplicate_accounts_merged.html | 18 ++++++++++++++++
...contributor_duplicate_accounts_merged.json | 8 +++++++
9 files changed, 103 insertions(+), 19 deletions(-)
create mode 100644 scipost/migrations/0018_contributor_duplicate_of.py
create mode 100644 templates/email/contributors/inform_contributor_duplicate_accounts_merged.html
create mode 100644 templates/email/contributors/inform_contributor_duplicate_accounts_merged.json
diff --git a/profiles/forms.py b/profiles/forms.py
index 3b182359a..ff35d4d39 100644
--- a/profiles/forms.py
+++ b/profiles/forms.py
@@ -98,10 +98,11 @@ class ProfileMergeForm(forms.Form):
data = super().clean()
if self.cleaned_data['to_merge'] == self.cleaned_data['to_merge_into']:
self.add_error(None, 'A Profile cannot be merged into itself.')
- if self.cleaned_data['to_merge'].has_contributor and \
- self.cleaned_data['to_merge_into'].has_contributor:
- self.add_error(None, 'Each of these two Profiles has a Contributor. '
- 'Cannot merge. If these are distinct people or if two separate '
+ if self.cleaned_data['to_merge'].has_active_contributor and \
+ self.cleaned_data['to_merge_into'].has_active_contributor:
+ self.add_error(None, 'Each of these two Profiles has an active Contributor. '
+ 'Merge the Contributors first.\n'
+ 'If these are distinct people or if two separate '
'accounts are needed, a ProfileNonDuplicate instance should be created; '
'contact techsupport.')
return data
@@ -114,13 +115,15 @@ class ProfileMergeForm(forms.Form):
profile = self.cleaned_data['to_merge_into']
profile_old = self.cleaned_data['to_merge']
- # Merge scientific information from old Profile to the new Profile.
+ # Merge information from old to new Profile.
profile.expertises = list(
set(profile_old.expertises or []) | set(profile.expertises or []))
if profile.orcid_id is None:
profile.orcid_id = profile_old.orcid_id
if profile.webpage is None:
profile.webpage = profile_old.webpage
+ if profile_old.has_active_contributor and not profile.has_active_contributor:
+ profile.contributor = profile_old.contributor
profile.save() # Save all the field updates.
profile.topics.add(*profile_old.topics.all())
@@ -128,13 +131,10 @@ class ProfileMergeForm(forms.Form):
if hasattr(profile_old, 'unregisteredauthor') and profile_old.unregisteredauthor:
profile.unregisteredauthor.merge(profile_old.unregisteredauthor)
- # Merge email and Contributor information
+ # Merge email
profile_old.emails.exclude(
email__in=profile.emails.values_list('email', flat=True)).update(
primary=False, profile=profile)
- if hasattr(profile_old, 'contributor') and profile_old.contributor:
- profile.contributor = profile_old.contributor
- profile.contributor.save()
# Move all invitations to the "new" profile.
profile_old.refereeinvitation_set.all().update(profile=profile)
diff --git a/profiles/models.py b/profiles/models.py
index f840a6ca0..3d1d9d69b 100644
--- a/profiles/models.py
+++ b/profiles/models.py
@@ -77,13 +77,14 @@ class Profile(models.Model):
return getattr(self.emails.filter(primary=True).first(), 'email', '')
@property
- def has_contributor(self):
- has_contributor = False
+ def has_active_contributor(self):
+ has_active_contributor = False
try:
- has_contributor = (self.contributor is not None)
+ has_active_contributor = (self.contributor is not None and
+ self.contributor.is_active)
except Contributor.DoesNotExist:
pass
- return has_contributor
+ return has_active_contributor
def get_absolute_url(self):
return reverse('profiles:profile_detail', kwargs={'pk': self.id})
diff --git a/profiles/templates/profiles/profile_merge.html b/profiles/templates/profiles/profile_merge.html
index b58a65b3c..20dd5aea6 100644
--- a/profiles/templates/profiles/profile_merge.html
+++ b/profiles/templates/profiles/profile_merge.html
@@ -16,7 +16,7 @@
<div class="row">
<div class="col-12">
<h1 class="highlight">Merge Profiles {{ profile_to_merge.id }} and {{ profile_to_merge_into.id }}</h1>
- {% if profile_to_merge.contributor and profile_to_merge.contributor.user.is_active and profile_to_merge_into.contributor and not profile_to_merge_into.contributor.user.is_active %}
+ {% if profile_to_merge.has_active_contributor and not profile_to_merge_into.has_active_contributor %}
<h3 class="text-danger">Warning: the Profile to merge is associated to an active Contributor, while the one to merge into is not</h3>
<p>Consider <a href="{% url 'profiles:merge' %}?to_merge={{ profile_to_merge_into.id }}&to_merge_into={{ profile_to_merge.id }}" method="get">merging the other way around</a></p>
{% endif %}
diff --git a/scipost/forms.py b/scipost/forms.py
index 6c9e89a65..0528e0312 100644
--- a/scipost/forms.py
+++ b/scipost/forms.py
@@ -41,6 +41,7 @@ from commentaries.models import Commentary
from comments.models import Comment
from funders.models import Grant
from journals.models import PublicationAuthorsTable, Publication
+from mails.utils import DirectMailUtil
from submissions.models import Submission, EditorialAssignment, RefereeInvitation, Report, \
EditorialCommunication, EICRecommendation
from theses.models import ThesisLink
@@ -439,20 +440,31 @@ class ContributorMergeForm(forms.Form):
contrib_from = self.cleaned_data['to_merge']
contrib_into = self.cleaned_data['to_merge_into']
+ both_contribs_active = contrib_from.is_active and contrib_info.is_active
+
contrib_from_qs = Contributor.objects.filter(pk=contrib_from.id)
contrib_into_qs = Contributor.objects.filter(pk=contrib_into.id)
# Step 1: update all fields within Contributor
if contrib_from.profile and not contrib_into.profile:
- contrib_into_qs.update(profile=contrib_from.profile)
+ profile = contrib_from.profile
+ contrib_from_qs.update(profile=None)
+ contrib_into_qs.update(profile=profile)
User.objects.filter(pk=contrib_from.user.id).update(is_active=False)
User.objects.filter(pk=contrib_into.user.id).update(is_active=True)
+ if contrib_from.invitation_key and not contrib_into.invitation_key:
+ contrib_into_qs.update(invitation_key=contrib_into.invitation_key)
+ if contrib_from.activation_key and not contrib_into.activation_key:
+ contrib_into_qs.update(activation_key=contrib_into.activation_key)
contrib_from_qs.update(status=DOUBLE_ACCOUNT)
if contrib_from.orcid_id and not contrib_into.orcid_id:
contrib_into_qs.update(orcid_id=contrib_from.orcid_id)
if contrib_from.personalwebpage and not contrib_into.personalwebpage:
contrib_into_qs.update(personalwebpage=contrib_from.personalwebpage)
+ # Specify duplicate_of for deactivated Contributor
+ contrib_from_qs.update(duplicate_of=contrib_into)
+
# Step 2: update all ForeignKey relations
Affiliation.objects.filter(contributor=contrib_from).update(contributor=contrib_into)
Fellowship.objects.filter(contributor=contrib_from).update(contributor=contrib_into)
@@ -597,6 +609,12 @@ class ContributorMergeForm(forms.Form):
for nom in motions:
nom.in_disagreement.remove(contrib_from)
nom.in_disagreement.add(contrib_into)
+ # If both accounts were active, inform the Contributor of the merge
+ if both_contribs_active or True:
+ mail_sender = DirectMailUtil(
+ mail_code='contributors/inform_contributor_duplicate_accounts_merged',
+ contrib_from=contrib_from)
+ mail_sender.send()
return Contributor.objects.get(id=contrib_into.id)
diff --git a/scipost/migrations/0018_contributor_duplicate_of.py b/scipost/migrations/0018_contributor_duplicate_of.py
new file mode 100644
index 000000000..cbaa585ff
--- /dev/null
+++ b/scipost/migrations/0018_contributor_duplicate_of.py
@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.4 on 2018-11-17 17:01
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('scipost', '0017_auto_20181115_2150'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='contributor',
+ name='duplicate_of',
+ field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='duplicates', to='scipost.Contributor'),
+ ),
+ ]
diff --git a/scipost/models.py b/scipost/models.py
index d55050e0b..960c721d2 100644
--- a/scipost/models.py
+++ b/scipost/models.py
@@ -16,7 +16,7 @@ from django.utils import timezone
from .behaviors import TimeStampedModel, orcid_validator
from .constants import (
- SCIPOST_DISCIPLINES, SCIPOST_SUBJECT_AREAS, subject_areas_dict, DISABLED,
+ SCIPOST_DISCIPLINES, SCIPOST_SUBJECT_AREAS, subject_areas_dict, NORMAL_CONTRIBUTOR, DISABLED,
TITLE_CHOICES, INVITATION_STYLE, INVITATION_TYPE, INVITATION_CONTRIBUTOR, INVITATION_FORMAL,
AUTHORSHIP_CLAIM_PENDING, AUTHORSHIP_CLAIM_STATUS, CONTRIBUTOR_STATUSES, NEWLY_REGISTERED)
from .fields import ChoiceArrayField
@@ -63,6 +63,9 @@ class Contributor(models.Model):
related_name="contrib_vetted_by", blank=True, null=True)
accepts_SciPost_emails = models.BooleanField(
default=True, verbose_name="I accept to receive SciPost emails")
+ # If this Contributor is merged into another, then this field is set to point to the new one:
+ duplicate_of = models.ForeignKey('scipost.Contributor', on_delete=models.SET_NULL,
+ null=True, blank=True, related_name='duplicates')
objects = ContributorQuerySet.as_manager()
@@ -82,6 +85,14 @@ class Contributor(models.Model):
"""Return public information page url."""
return reverse('scipost:contributor_info', args=(self.id,))
+ @property
+ def is_active(self):
+ """
+ Checks if the Contributor is registered, vetted,
+ and has not been deactivated for any reason.
+ """
+ return self.user.is_active and self.status == NORMAL_CONTRIBUTOR
+
@property
def is_currently_available(self):
"""Check if Contributor is currently not marked as unavailable."""
diff --git a/scipost/views.py b/scipost/views.py
index f6f07c3e8..53ad7d488 100644
--- a/scipost/views.py
+++ b/scipost/views.py
@@ -964,6 +964,10 @@ def contributor_info(request, contributor_id):
class ContributorDuplicateListView(PermissionsMixin, PaginationMixin, ListView):
"""
List Contributors with potential (not yet handled) duplicates.
+ Two sources of duplicates are separately considered:
+ - duplicate full names (last name + first name)
+ - duplicate email addresses.
+
"""
permission_required = 'scipost.can_vet_registration_requests'
model = Contributor
@@ -996,10 +1000,13 @@ class ContributorDuplicateListView(PermissionsMixin, PaginationMixin, ListView):
def contributor_merge(request):
"""
Handles the merging of data from one Contributor instance to another,
- to solve multiple registration issues.
+ to solve one person - multiple registrations issues.
+
+ Both instances are preserved, but the merge_from instance's
+ status is set to DOUBLE_ACCOUNT and its User is set to inactive.
- Both instances are preserved, but the merge_from instance is set to inactive,
- and its status is set to DOUBLE_ACCOUNT.
+ If both Contributor instances were active, then the account owner
+ is emailed with information about the merge.
"""
merge_form = ContributorMergeForm(request.POST or None, initial=request.GET)
context = {'merge_form': merge_form}
diff --git a/templates/email/contributors/inform_contributor_duplicate_accounts_merged.html b/templates/email/contributors/inform_contributor_duplicate_accounts_merged.html
new file mode 100644
index 000000000..fe79c1490
--- /dev/null
+++ b/templates/email/contributors/inform_contributor_duplicate_accounts_merged.html
@@ -0,0 +1,18 @@
+<p>
+ Dear {{ contrib_from.duplicate_of.get_title_display }} {{ contrib_from.duplicate_of.user.last_name }},
+</p>
+<p>
+ We noticed that you had two separate registrations at SciPost, and have consolidated your two accounts into a single active one, namely your account with username <strong>{{ contrib_from.duplicate_of.user.username }}</strong>.
+</p>
+<p>
+ Your alternate account with username <strong>{{ contrib_from.user.username }}</strong> has been deactivated, but all the data associated to it has been transferred to your active account.
+</p>
+<p>
+ Please get in touch with us at <a href="mailto:techsupport@scipost.org">SciPost techsupport</a> if you have any questions.
+</p>
+<p>
+ Many thanks,
+ <br><br>
+ The SciPost Team
+</p>
+{% include 'email/_footer.html' %}
diff --git a/templates/email/contributors/inform_contributor_duplicate_accounts_merged.json b/templates/email/contributors/inform_contributor_duplicate_accounts_merged.json
new file mode 100644
index 000000000..f7987957d
--- /dev/null
+++ b/templates/email/contributors/inform_contributor_duplicate_accounts_merged.json
@@ -0,0 +1,8 @@
+{
+ "subject": "SciPost: duplicate accounts merged",
+ "to_address": "user.email",
+ "bcc_to": "admin@scipost.org",
+ "from_address_name": "SciPost Admin",
+ "from_address": "admin@scipost.org",
+ "context_object": "contrib_from"
+}
--
GitLab