diff --git a/profiles/templates/profiles/_profile_card.html b/profiles/templates/profiles/_profile_card.html index 49698c21080a857be7bbad6498aa1984a4d05a34..7a3a54b1214ebbb7f71ebd5d1ebff78d457fd13e 100644 --- a/profiles/templates/profiles/_profile_card.html +++ b/profiles/templates/profiles/_profile_card.html @@ -5,7 +5,7 @@ <div class="card"> <div class="card-header"> - Details + Details for profile {{ profile.id }} </div> <div class="card-body"> <table class="table"> @@ -18,11 +18,11 @@ <td> <table class="table table-sm"> <thead> - <tr> - <th colspan="2">Email</th> - <th>Still valid</th> - <th></th> - </tr> + <tr> + <th colspan="2">Email</th> + <th>Still valid</th> + <th></th> + </tr> </thead> {% for profile_mail in profile.emails.all %} <tr> @@ -56,7 +56,7 @@ <tr><td>Webpage</td><td><a href="{{ profile.webpage }}" target="_blank">{{ profile.webpage }}</a></td></tr> <tr><td>Accepts SciPost emails</td><td>{{ profile.accepts_SciPost_emails }}</td></tr> <tr><td>Accepts refereeing requests</td><td>{{ profile.accepts_refereeing_requests }}</td></tr> - <tr><td>Contributor</td><td>{% if profile.contributor %}Yes (<a href="{% url 'scipost:contributor_info' contributor_id=profile.contributor.id %}" target="_blank">info link</a>){% else %}No{% endif %}</td></tr> + <tr><td>Contributor</td><td>{% if profile.contributor %}Yes, id: {{ profile.contributor.pk }}, status: {{ profile.contributor.get_status_display }}, user active: {{ profile.contributor.user.is_active }} (<a href="{% url 'scipost:contributor_info' contributor_id=profile.contributor.id %}" target="_blank">info link</a>){% else %}No{% endif %}</td></tr> </table> </div> </div> diff --git a/profiles/templates/profiles/profile_form.html b/profiles/templates/profiles/profile_form.html index 1f8b76b6cbc9edf97e51333ff71ee8bd377103b5..b5e021cfcd8da36d25df6b56fd5325c926068176 100644 --- a/profiles/templates/profiles/profile_form.html +++ b/profiles/templates/profiles/profile_form.html @@ -4,7 +4,7 @@ {% block breadcrumb_items %} {{ block.super }} - <span class="breadcrumb-item">{% if form.instance.id %}Update {{ form.instance }}{% else %}Add new Profile{% endif %}</span> + <span class="breadcrumb-item">{% if form.instance.id %}Update {{ form.instance }}{% else %}Add new Profile {% if from_type %}(from {{ from_type }}){% endif %}{% endif %}</span> {% endblock %} {% block pagetitle %}: Profiles{% endblock pagetitle %} @@ -16,7 +16,7 @@ <h4>Matching profiles found for this {{ from_type }}</h4> <ul> {% for matching_profile in matching_profiles %} - <li>{{ matching_profile }} <a href="{% url 'profiles:profile_match' profile_id=matching_profile.id from_type=from_type pk=pk %}">match this {{ from_type }} to this Profile</a> + <li>{{ matching_profile }} (id {{ matching_profile.id }}, {{ matching_profile.email }}) <a href="{% url 'profiles:profile_match' profile_id=matching_profile.id from_type=from_type pk=pk %}"><i class="fa fa-arrow-right"></i> Match this {{ from_type }} to this Profile</a> </li> {% endfor %} </ul> diff --git a/profiles/templates/profiles/profile_merge.html b/profiles/templates/profiles/profile_merge.html index 12cc944d6516421265fcfa9e221f70e01dd338aa..b58a65b3c35b5ea65099a2c68d20e22ba2a5a38f 100644 --- a/profiles/templates/profiles/profile_merge.html +++ b/profiles/templates/profiles/profile_merge.html @@ -16,6 +16,10 @@ <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 %} + <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 %} </div> </div> <div class="row"> diff --git a/profiles/views.py b/profiles/views.py index 191f199ddf47ec2f48fc53e1f1410706e847412c..3ed0d8c792a0e256f6ca0d6fc9f897013b9c31e4 100644 --- a/profiles/views.py +++ b/profiles/views.py @@ -70,7 +70,7 @@ class ProfileCreateView(PermissionsMixin, CreateView): matching_profiles = matching_profiles.filter( Q(last_name=reginv.last_name) | Q(emails__email__in=reginv.email)) - context['matching_profiles'] = matching_profiles + context['matching_profiles'] = matching_profiles.distinct() return context def get_initial(self): @@ -134,26 +134,22 @@ def profile_match(request, profile_id, from_type, pk): Contributor, UnregisteredAuthor, RefereeInvitation, RegistrationInvitation. """ profile = get_object_or_404(Profile, pk=profile_id) + nr_rows = 0 if from_type == 'contributor': - contributor = get_object_or_404(Contributor, pk=pk) - contributor.profile = profile - contributor.save() - messages.success(request, 'Profile matched with Contributor') + nr_rows = Contributor.objects.filter(pk=pk).update(profile=profile) elif from_type == 'unregisteredauthor': - unreg_auth = get_object_or_404(UnregisteredAuthor, pk=pk) - unreg_auth.profile = profile - unreg_auth.save() - messages.success(request, 'Profile matched with UnregisteredAuthor') + nr_rows = UnregisteredAuthor.objects.filter(pk=pk).update(profile=profile) elif from_type == 'refereeinvitation': - ref_inv = get_object_or_404(RefereeInvitation, pk=pk) - ref_inv.profile = profile - ref_inv.save() - messages.success(request, 'Profile matched with RefereeInvitation') + nr_rows = RefereeInvitation.objects.filter(pk=pk).update(profile=profile) elif from_type == 'registrationinvitation': - reg_inv = get_object_or_404(RegistrationInvitation, pk=pk) - reg_inv.profile = profile - reg_inv.save() - messages.success(request, 'Profile matched with RegistrationInvitation') + nr_rows = RegistrationInvitation.objects.filter(pk=pk).update(profile=profile) + if nr_rows == 1: + messages.success(request, 'Profile matched with %s' % from_type) + else: + messages.error( + request, + 'Error: Profile matching with %s: updated %s rows instead of 1!' + 'Please contact techsupport' % (from_type, nr_rows)) return redirect(reverse('profiles:profiles')) @@ -274,15 +270,29 @@ def profile_merge(request): merge_form = ProfileMergeForm(request.POST or None, initial=request.GET) context = {'merge_form': merge_form} - if merge_form.is_valid(): - profile = merge_form.save() - messages.success(request, 'Profiles merged') - return redirect(profile.get_absolute_url()) + if request.method == 'POST': + if merge_form.is_valid(): + profile = merge_form.save() + messages.success(request, 'Profiles merged') + return redirect(profile.get_absolute_url()) + else: + try: + context.update({ + 'profile_to_merge': get_object_or_404( + Profile, pk=merge_form.cleaned_data['to_merge'].id), + 'profile_to_merge_into': get_object_or_404( + Profile, pk=merge_form.cleaned_data['to_merge_into'].id) + }) + except ValueError: + raise Http404 + elif request.method == 'GET': try: context.update({ - 'profile_to_merge': get_object_or_404(Profile, pk=int(request.GET['to_merge'])), - 'profile_to_merge_into': get_object_or_404(Profile, pk=int(request.GET['to_merge_into'])) + 'profile_to_merge': get_object_or_404(Profile, + pk=int(request.GET['to_merge'])), + 'profile_to_merge_into': get_object_or_404(Profile, + pk=int(request.GET['to_merge_into'])) }) except ValueError: raise Http404 diff --git a/scipost/admin.py b/scipost/admin.py index 097b6c7f38f195d2998fb863965c5f2e6d085719..8678bee7b3c6c41b23f5ae9bef251ac3320871c7 100644 --- a/scipost/admin.py +++ b/scipost/admin.py @@ -41,6 +41,7 @@ class UserAdmin(UserAdmin): ContactToUserInline, ProductionUserInline ] + list_display = ['username', 'email', 'first_name', 'last_name', 'is_active', 'is_staff'] search_fields = ['last_name', 'email'] diff --git a/scipost/forms.py b/scipost/forms.py index ef72924e574b1550f45cbb399b7908005193f8ec..6c9e89a6551d5992516918f19deca6cc33595069 100644 --- a/scipost/forms.py +++ b/scipost/forms.py @@ -26,11 +26,11 @@ from haystack.forms import ModelSearchForm as HayStackSearchForm from .behaviors import orcid_validator from .constants import ( SCIPOST_DISCIPLINES, TITLE_CHOICES, SCIPOST_FROM_ADDRESSES, NO_SCIENTIST, DOUBLE_ACCOUNT, - BARRED, DISABLED) + BARRED) from .decorators import has_contributor from .fields import ReCaptchaField from .models import Contributor, DraftInvitation, UnavailabilityPeriod, \ - Remark, DraftInvitation, AuthorshipClaim, PrecookedEmail + Remark, AuthorshipClaim, PrecookedEmail from affiliations.models import Affiliation, Institution from common.forms import MonthYearWidget, ModelChoiceFieldwithid @@ -447,7 +447,7 @@ class ContributorMergeForm(forms.Form): contrib_into_qs.update(profile=contrib_from.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) - contrib_from_qs.update(status=DISABLED) + 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: diff --git a/scipost/managers.py b/scipost/managers.py index 54d96ac02a455f9258a6cfd8607c17a0f623e216..9037134c7b44dc6249cc28f4e8559799d9a2cee9 100644 --- a/scipost/managers.py +++ b/scipost/managers.py @@ -7,7 +7,7 @@ from django.db.models import Count, Q from django.db.models.functions import Concat, Lower from django.utils import timezone -from .constants import NORMAL_CONTRIBUTOR, NEWLY_REGISTERED, AUTHORSHIP_CLAIM_PENDING +from .constants import NORMAL_CONTRIBUTOR, NEWLY_REGISTERED, DOUBLE_ACCOUNT, AUTHORSHIP_CLAIM_PENDING class FellowManager(models.Manager): @@ -41,7 +41,8 @@ class ContributorQuerySet(models.QuerySet): def awaiting_vetting(self): """Filter Contributors that have not been vetted through.""" - return self.filter(user__is_active=True, status=NEWLY_REGISTERED) + return self.filter(user__is_active=True, status=NEWLY_REGISTERED + ).exclude(status=DOUBLE_ACCOUNT) def fellows(self): """TODO: NEEDS UPDATE TO NEW FELLOWSHIP RELATIONS.""" @@ -53,7 +54,8 @@ class ContributorQuerySet(models.QuerySet): last names). Admins and superusers are explicitly excluded. """ - contribs = self.active().exclude(user__is_superuser=True).exclude(user__is_staff=True + contribs = self.exclude(status=DOUBLE_ACCOUNT + ).exclude(user__is_superuser=True).exclude(user__is_staff=True ).annotate(full_name=Concat('user__last_name', 'user__first_name')) duplicates = contribs.values('full_name').annotate( nr_count=Count('full_name')).filter(nr_count__gt=1) @@ -64,7 +66,8 @@ class ContributorQuerySet(models.QuerySet): """ Return Contributors having duplicate emails. """ - duplicates = self.active().exclude(user__is_superuser=True).exclude(user__is_staff=True + duplicates = self.exclude(status=DOUBLE_ACCOUNT + ).exclude(user__is_superuser=True).exclude(user__is_staff=True ).values(lower_email=Lower('user__email')).annotate( Count('id')).order_by('user__last_name').filter(id__count__gt=1) return self.annotate(lower_email=Lower('user__email') diff --git a/scipost/templates/scipost/_public_info_as_table.html b/scipost/templates/scipost/_public_info_as_table.html index 1f0f93e93ff44d17a377c2632e39254646ebc0d8..2a5dee4578da777288e5279b34a2bac0717eced8 100644 --- a/scipost/templates/scipost/_public_info_as_table.html +++ b/scipost/templates/scipost/_public_info_as_table.html @@ -15,4 +15,9 @@ </td> </tr> <tr><td>Personal web page: </td><td>{{ contributor.personalwebpage|default:'-' }}</td></tr> + {% if perms.scipost.can_vet_registration_requests %} + <tr><td>Status</td><td>{{ contributor.get_status_display }}</td></tr> + <tr><td>User active?</td><td>{{ contributor.user.is_active }}</td></tr> + <tr><td>Id</td><td>{{ contributor.id }}{% if contributor.profile %} <a href="{% url 'profiles:profile_detail' pk=contributor.profile.id %}">View Profile <i class="fa fa-arrow-right"></i></a>{% endif %}</td></tr> + {% endif %} </table> diff --git a/scipost/templates/scipost/contributor_info.html b/scipost/templates/scipost/contributor_info.html index d65751ed7db2636235ff7754b3725448eed99fb8..0969da038368d52d0d76039dba9bbc15143e6018 100644 --- a/scipost/templates/scipost/contributor_info.html +++ b/scipost/templates/scipost/contributor_info.html @@ -6,9 +6,16 @@ <h1 class="highlight mb-4">Contributor info: {{ contributor.get_title_display }} {{ contributor.user.first_name }} {{ contributor.user.last_name }}</h1> -{% include "scipost/_public_info_as_table.html" with contributor=contributor %} -<br> +<div class="card"> + <div class="card-header"> + Details + </div> + <div class="card-body"> + {% include "scipost/_public_info_as_table.html" with contributor=contributor %} + </div> +</div> + {% if contributor_publications %} <div class="row"> diff --git a/scipost/templates/scipost/contributor_merge.html b/scipost/templates/scipost/contributor_merge.html index 73b0700f146369d0acd2b7c2de1270b1095d1aff..e743d2cf8bc258305424e6a8f1ccc6d573fc46d0 100644 --- a/scipost/templates/scipost/contributor_merge.html +++ b/scipost/templates/scipost/contributor_merge.html @@ -16,18 +16,22 @@ <div class="row"> <div class="col-12"> <h1 class="highlight">Merge Contributors {{ contributor_to_merge.id }} and {{ contributor_to_merge_into.id }}</h1> + {% if contributor_to_merge.user.is_active and not contributor_to_merge_into.user.is_active %} + <h3 class="text-danger">Warning: the contributor to merge is active, while the one to merge into is not</h3> + <p>Consider <a href="{% url 'scipost:contributor_merge' %}?to_merge={{ contributor_to_merge_into.id }}&to_merge_into={{ contributor_to_merge.id }}" method="get">merging the other way around</a></p> + {% endif %} </div> </div> <div class="row"> <div class="col-12"> <h3 class="highlight">Contributor {{ contributor_to_merge.id }}</h3> - {% include "scipost/_public_info_as_table.html" with contributor=contributor %} + {% include "scipost/_public_info_as_table.html" with contributor=contributor_to_merge %} </div> </div> <div class="row"> <div class="col-12"> <h3 class="highlight">Contributor {{ contributor_to_merge_into.id }}</h3> - {% include "scipost/_public_info_as_table.html" with contributor=contributor %} + {% include "scipost/_public_info_as_table.html" with contributor=contributor_to_merge_into %} </div> </div> diff --git a/scipost/views.py b/scipost/views.py index e1f9085b0be3902f22668ba6fe9a07cb7617ba47..f6f07c3e8edc2ec3e290f8e8fb7d2248f0615959 100644 --- a/scipost/views.py +++ b/scipost/views.py @@ -963,14 +963,14 @@ def contributor_info(request, contributor_id): class ContributorDuplicateListView(PermissionsMixin, PaginationMixin, ListView): """ - List Contributors with potential duplicates. + List Contributors with potential (not yet handled) duplicates. """ permission_required = 'scipost.can_vet_registration_requests' model = Contributor template_name = 'scipost/contributor_duplicate_list.html' def get_queryset(self): - queryset = Contributor.objects.active() + queryset = Contributor.objects.all() if self.request.GET.get('kind') == 'names': queryset = queryset.with_duplicate_names() elif self.request.GET.get('kind') == 'emails': @@ -998,15 +998,28 @@ def contributor_merge(request): Handles the merging of data from one Contributor instance to another, to solve multiple registration issues. - Both instances are preserved, but the merge_from instance 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. """ merge_form = ContributorMergeForm(request.POST or None, initial=request.GET) context = {'merge_form': merge_form} - if merge_form.is_valid(): - contributor = merge_form.save() - messages.success(request, 'Contributors merged') - return redirect(reverse('scipost:contributor_duplicates')) + if request.method == 'POST': + if merge_form.is_valid(): + contributor = merge_form.save() + messages.success(request, 'Contributors merged') + return redirect(reverse('scipost:contributor_duplicates')) + else: + try: + context.update({ + 'contributor_to_merge': get_object_or_404( + Contributor, pk=merge_form.cleaned_data['to_merge'].id), + 'contributor_to_merge_into': get_object_or_404( + Contributor, pk=merge_form.cleaned_data['to_merge_into'].id) + }) + except ValueError: + raise Http404 + elif request.method == 'GET': try: context.update({