diff --git a/admin/institutions/urls.py b/admin/institutions/urls.py index 6aa5cf7e0df..2e260cf4f1b 100644 --- a/admin/institutions/urls.py +++ b/admin/institutions/urls.py @@ -20,5 +20,7 @@ name='list_and_add_admin'), re_path(r'^(?P[0-9]+)/remove_admins/$', views.InstitutionRemoveAdmin.as_view(), name='remove_admins'), + re_path(r'^(?P[0-9]+)/affiliations/$', views.InstitutionListAndAddAffiliation.as_view(), name='affiliations'), + re_path(r'^(?P[0-9]+)/remove_affiliations/$', views.InstitutionRemoveAffiliation.as_view(), name='remove_affiliations'), ] diff --git a/admin/institutions/views.py b/admin/institutions/views.py index 46e6a0a7745..536d916d937 100644 --- a/admin/institutions/views.py +++ b/admin/institutions/views.py @@ -373,3 +373,63 @@ def form_valid(self, form): def get_success_url(self): return reverse('institutions:register_metrics_admin', kwargs={'institution_id': self.kwargs['institution_id']}) + + +class InstitutionAffiliationBaseView(PermissionRequiredMixin, ListView): + permission_required = 'osf.change_institution' + template_name = 'institutions/edit_affiliations.html' + raise_exception = True + + def get_queryset(self): + return Institution.objects.get(id=self.kwargs['institution_id']) + + def get_context_data(self, **kwargs): + institution = Institution.objects.get(id=self.kwargs['institution_id']) + context = super().get_context_data(**kwargs) + context['institution'] = institution + context['affiliations'] = institution.get_institution_users() + return context + + +class InstitutionListAndAddAffiliation(InstitutionAffiliationBaseView): + + def get_permission_required(self): + if self.request.method == 'GET': + return ('osf.view_institution',) + return (self.permission_required,) + + def post(self, request, *args, **kwargs): + institution = Institution.objects.get(id=self.kwargs['institution_id']) + data = dict(request.POST) + del data['csrfmiddlewaretoken'] # just to remove the key from the form dict + + target_user = OSFUser.load(data['add-affiliation-form'][0]) + if target_user is None: + messages.error(request, f'User for guid: {data["add-affiliation-form"][0]} could not be found') + return redirect('institutions:affiliations', institution_id=institution.id) + + target_user.add_or_update_affiliated_institution(institution) + + messages.success(request, f'The following user was successfully added: {target_user.fullname} ({target_user.username})') + + return redirect('institutions:affiliations', institution_id=institution.id) + + +class InstitutionRemoveAffiliation(InstitutionAffiliationBaseView): + + def post(self, request, *args, **kwargs): + institution = Institution.objects.get(id=self.kwargs['institution_id']) + data = dict(request.POST) + del data['csrfmiddlewaretoken'] # just to remove the key from the form dict + + to_be_removed = list(data.keys()) + removed_affiliations = [user.replace('User-', '') for user in to_be_removed if 'User-' in user] + affiliated_users = OSFUser.objects.filter(id__in=removed_affiliations) + for user in affiliated_users: + user.remove_affiliated_institution(institution._id) + + if affiliated_users: + users_names = ' ,'.join(affiliated_users.values_list('fullname', flat=True)) + messages.success(request, f'The following users were successfully removed: {users_names}') + + return redirect('institutions:affiliations', institution_id=institution.id) diff --git a/admin/templates/institutions/detail.html b/admin/templates/institutions/detail.html index 2ce1ad20a03..1df4b6061fe 100644 --- a/admin/templates/institutions/detail.html +++ b/admin/templates/institutions/detail.html @@ -29,6 +29,7 @@ {% else %} Reactivate institution {% endif %} + Affiliations {% endif %} {% if perms.osf.change_institution %} Manage Admins diff --git a/admin/templates/institutions/edit_affiliations.html b/admin/templates/institutions/edit_affiliations.html new file mode 100644 index 00000000000..e4e25cf1ac6 --- /dev/null +++ b/admin/templates/institutions/edit_affiliations.html @@ -0,0 +1,55 @@ +{% extends "base.html" %} +{% load static %} +{% load render_bundle from webpack_loader %} +{% block title %} + Institution Affiliations +{% endblock title %} +{% block content %} +
+
+ {% if messages %} +
    + {% for message in messages %} + {{ message }} + {% endfor %} +
+ {% endif %} +
+
+
+

{{ institution.name }}

+
+
+
+
+
+ {% csrf_token %} + + + +
+
+
+
+
+
+
+ {% csrf_token %} + + + + + {% for user in affiliations %} + + + + + + {% endfor %} +
NameUsername
{{ user.fullname }}{{ user.username }}
+ +
+
+
+
+{% endblock content %} \ No newline at end of file diff --git a/admin/templates/users/affiliated_institutions.html b/admin/templates/users/affiliated_institutions.html new file mode 100644 index 00000000000..2f80ff79350 --- /dev/null +++ b/admin/templates/users/affiliated_institutions.html @@ -0,0 +1,55 @@ +{% extends "base.html" %} +{% load static %} +{% load render_bundle from webpack_loader %} +{% block title %} + Affiliated Institutions +{% endblock title %} +{% block content %} +
+
+ {% if messages %} +
    + {% for message in messages %} + {{ message }} + {% endfor %} +
+ {% endif %} +
+
+
+

{{ institution.name }}

+
+
+
+
+
+ {% csrf_token %} + + + +
+
+
+
+
+
+
+ {% csrf_token %} + + + + + {% for institution in institutions %} + + + + + + {% endfor %} +
NameGuid
{{ institution.name }}{{ institution.guid }}
+ +
+
+
+
+{% endblock content %} \ No newline at end of file diff --git a/admin/templates/users/user.html b/admin/templates/users/user.html index 96cda4689aa..5cc9244d484 100644 --- a/admin/templates/users/user.html +++ b/admin/templates/users/user.html @@ -38,6 +38,9 @@ {% include "users/mark_spam.html" with user=user %} {% include "users/reindex_user_elastic.html" with user=user %} {% include "users/reindex_user_share.html" with user=user %} + {% if perms.osf.change_institution %} + Affiliations + {% endif %} diff --git a/admin/users/urls.py b/admin/users/urls.py index 3c87ab1e332..c692c52c4bc 100644 --- a/admin/users/urls.py +++ b/admin/users/urls.py @@ -30,4 +30,6 @@ name='reindex-share-user'), re_path(r'^(?P[a-z0-9]+)/merge_accounts/$', views.UserMergeAccounts.as_view(), name='merge-accounts'), re_path(r'^(?P[a-z0-9]+)/draft_registrations/$', views.UserDraftRegistrationsList.as_view(), name='draft-registrations'), + re_path(r'^(?P[a-z0-9]+)/affiliations/$', views.UserListAndAddAffiliations.as_view(), name='affiliations'), + re_path(r'^(?P[a-z0-9]+)/remove_affiliations/$', views.UserRemoveAffiliations.as_view(), name='remove_affiliations'), ] diff --git a/admin/users/views.py b/admin/users/views.py index 1584c78158e..2b0b1047721 100644 --- a/admin/users/views.py +++ b/admin/users/views.py @@ -23,6 +23,7 @@ from osf.models.notification_type import NotificationType from framework.auth import get_user from framework.auth.core import generate_verification_key +from osf.models.institution import Institution from website import search from website.settings import EXTERNAL_IDENTITY_PROFILE @@ -623,3 +624,60 @@ def get_context_data(self, **kwargs): 'draft_registrations': query_set } ) + + +class UserAffiliationBaseView(UserMixin, ListView): + permission_required = 'osf.change_institution' + template_name = 'users/affiliated_institutions.html' + raise_exception = True + + def get_queryset(self): + # Django template does not like attributes with underscores for some reason, so we annotate. + return self.get_object().get_affiliated_institutions().annotate( + guid=F('_id') + ) + + def get_context_data(self, **kwargs): + institutions = self.get_queryset() + context = super().get_context_data(**kwargs) + context['institutions'] = institutions + context['user'] = self.get_object() + return context + + +class UserRemoveAffiliations(UserAffiliationBaseView): + + def post(self, request, *args, **kwargs): + user = self.get_object() + data = dict(request.POST) + + to_be_removed = list(data.keys()) + removed_affiliations = [institution.replace('institution-', '') for institution in to_be_removed if 'institution-' in institution] + institutions_qs = Institution.objects.filter(id__in=removed_affiliations) + for institution in institutions_qs: + user.remove_affiliated_institution(institution._id) + + if institutions_qs: + institutions_names = ' ,'.join(institutions_qs.values_list('name', flat=True)) + messages.success(request, f'The following users were successfully removed: {institutions_names}') + + return redirect('users:affiliations', guid=user.guid) + + +class UserListAndAddAffiliations(UserAffiliationBaseView): + + def post(self, request, *args, **kwargs): + user = self.get_object() + data = dict(request.POST) + del data['csrfmiddlewaretoken'] # just to remove the key from the form dict + + institution = Institution.load(data['add-affiliation-form'][0]) + if institution is None: + messages.error(request, f'Institution for guid: {data["add-affiliation-form"][0]} could not be found') + return redirect('users:affiliations', guid=user.guid) + + user.add_or_update_affiliated_institution(institution) + + messages.success(request, f'The following institution was successfully added: {institution.name}') + + return redirect('users:affiliations', guid=user.guid) diff --git a/api_tests/preprints/views/test_preprint_versions.py b/api_tests/preprints/views/test_preprint_versions.py index 9d0af151461..e33834132c2 100644 --- a/api_tests/preprints/views/test_preprint_versions.py +++ b/api_tests/preprints/views/test_preprint_versions.py @@ -832,7 +832,7 @@ def test_list_versions_pre_mod(self): # Pre moderation V4 (Pending) with capture_notifications(): pre_mod_preprint_v4 = PreprintFactory.create_version( - create_from=pre_mod_preprint_v2, + create_from=pre_mod_preprint_v3, creator=self.creator, final_machine_state='initial', is_published=False, diff --git a/osf/management/commands/fix_versioned_guids.py b/osf/management/commands/fix_versioned_guids.py new file mode 100644 index 00000000000..76f1a32feba --- /dev/null +++ b/osf/management/commands/fix_versioned_guids.py @@ -0,0 +1,73 @@ +import logging + +from django.contrib.contenttypes.models import ContentType +from django.core.management.base import BaseCommand +from django.db import transaction +from django.db.models import Prefetch + +from osf.models import GuidVersionsThrough, Guid, Preprint +from osf.utils.workflows import ReviewStates + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = 'Fix' + + def add_arguments(self, parser): + parser.add_argument( + '--dry-run', + action='store_true', + dest='dry_run', + help='Run the command without saving changes', + ) + + @transaction.atomic + def handle(self, *args, **options): + dry_run = options.get('dry_run', False) + fix_versioned_guids(dry_run=dry_run) + if dry_run: + transaction.set_rollback(True) + + +def fix_versioned_guids(dry_run: bool): + content_type = ContentType.objects.get_for_model(Preprint) + versions_queryset = GuidVersionsThrough.objects.order_by('-version') + processed_count = 0 + updated_count = 0 + skipped_count = 0 + errors_count = 0 + for guid in ( + Guid.objects.filter(content_type=content_type) + .prefetch_related(Prefetch('versions', queryset=versions_queryset)) + .iterator(chunk_size=500) + ): + processed_count += 1 + if not guid.versions: + skipped_count += 1 + continue + for version in guid.versions.all(): + last_version_object_id = version.object_id + if guid.object_id == last_version_object_id: + skipped_count += 1 + break + if version.referent.machine_state not in (ReviewStates.INITIAL.value, ReviewStates.REJECTED.value, ReviewStates.WITHDRAWN.value): + continue + try: + guid.object_id = last_version_object_id + guid.referent = version.referent + if not dry_run: + guid.save() + updated_count += 1 + except Exception as e: + logger.error(f"Error occurred during patching {guid._id=}", exc_info=e) + errors_count += 1 + + if dry_run: + logger.error( + f"Processed: {processed_count}, Would update: {updated_count}, Skipped: {skipped_count}, Errors: {errors_count}" + ) + else: + logger.error( + f"Processed: {processed_count}, Updated: {updated_count}, Skipped: {skipped_count}, Errors: {errors_count}" + ) diff --git a/osf/models/preprint.py b/osf/models/preprint.py index 7a61b31db06..52983527a8d 100644 --- a/osf/models/preprint.py +++ b/osf/models/preprint.py @@ -58,7 +58,6 @@ ) from django.contrib.postgres.fields import ArrayField from api.share.utils import update_share -from api.providers.workflows import Workflows logger = logging.getLogger(__name__) @@ -1692,27 +1691,13 @@ def run_submit(self, user): user: The user triggering this transition. """ ret = super().run_submit(user=user) - provider = self.provider - reviews_workflow = provider.reviews_workflow - # Only post moderation is relevant for Preprint, and hybrid moderation is included for integrity purpose. - need_guid_update = any( - [ - reviews_workflow == Workflows.POST_MODERATION.value, - reviews_workflow == Workflows.HYBRID_MODERATION.value and - any([ - provider.get_group('moderator') in user.groups.all(), - provider.get_group('admin') in user.groups.all() - ]) - ] - ) - # Only update the base guid obj to refer to the new version 1) if the provider is post-moderation, or 2) if the - # provider is hybrid-moderation and if the user who submits the preprint is a moderator or admin. - if need_guid_update: - base_guid_obj = self.versioned_guids.first().guid - base_guid_obj.referent = self - base_guid_obj.object_id = self.pk - base_guid_obj.content_type = ContentType.objects.get_for_model(self) - base_guid_obj.save() + + base_guid_obj = self.versioned_guids.first().guid + base_guid_obj.referent = self + base_guid_obj.object_id = self.pk + base_guid_obj.content_type = ContentType.objects.get_for_model(self) + base_guid_obj.save() + return ret def run_accept(self, user, comment, **kwargs): @@ -1724,14 +1709,6 @@ def run_accept(self, user, comment, **kwargs): comment: Text describing why. """ ret = super().run_accept(user=user, comment=comment, **kwargs) - reviews_workflow = self.provider.reviews_workflow - if reviews_workflow == Workflows.PRE_MODERATION.value or reviews_workflow == Workflows.HYBRID_MODERATION.value: - base_guid_obj = self.versioned_guids.first().guid - base_guid_obj.referent = self - base_guid_obj.object_id = self.pk - base_guid_obj.content_type = ContentType.objects.get_for_model(self) - base_guid_obj.save() - versioned_guid = self.versioned_guids.first() if versioned_guid.is_rejected: versioned_guid.is_rejected = False @@ -1747,9 +1724,38 @@ def run_reject(self, user, comment): comment: Text describing why. """ ret = super().run_reject(user=user, comment=comment) - versioned_guid = self.versioned_guids.first() - versioned_guid.is_rejected = True - versioned_guid.save() + current_version_guid = self.versioned_guids.first() + current_version_guid.is_rejected = True + current_version_guid.save() + + self.rollback_main_guid() + + return ret + + def rollback_main_guid(self): + """Reset main guid to resolve to last versioned guid which is not withdrawn/rejected if there is one. + """ + guid = None + for version in self.versioned_guids.all()[1:]: # skip first guid as it refers to current version + guid = version.guid + if guid.referent.machine_state not in (ReviewStates.REJECTED, ReviewStates.WITHDRAWN): + break + if guid: + guid.referent = self + guid.object_id = self.pk + guid.content_type = ContentType.objects.get_for_model(self) + guid.save() + + def run_withdraw(self, user, comment): + """Override `ReviewableMixin`/`MachineableMixin`. + Run the 'withdraw' state transition and create a corresponding Action. + + Params: + user: The user triggering this transition. + comment: Text describing why. + """ + ret = super().run_withdraw(user=user, comment=comment) + self.rollback_main_guid() return ret @receiver(post_save, sender=Preprint) diff --git a/tests/test_preprints.py b/tests/test_preprints.py index 205d356c65b..3083b43461b 100644 --- a/tests/test_preprints.py +++ b/tests/test_preprints.py @@ -2542,8 +2542,8 @@ def test_unpublished_version_pre_mod_submit_and_accept(self, preprint_pre_mod, c assert new_version.is_published is False assert new_version.machine_state == ReviewStates.PENDING.value guid_obj = new_version.get_guid() - assert guid_obj.object_id == preprint_pre_mod.pk - assert guid_obj.referent == preprint_pre_mod + assert guid_obj.object_id == new_version.pk + assert guid_obj.referent == new_version assert guid_obj.content_type == ContentType.objects.get_for_model(Preprint) with capture_notifications(): @@ -2578,8 +2578,8 @@ def test_new_version_pre_mod_submit_and_reject(self, preprint_pre_mod, creator, assert new_version.is_published is False assert new_version.machine_state == ReviewStates.PENDING.value guid_obj = new_version.get_guid() - assert guid_obj.object_id == preprint_pre_mod.pk - assert guid_obj.referent == preprint_pre_mod + assert guid_obj.object_id == new_version.pk + assert guid_obj.referent == new_version assert guid_obj.content_type == ContentType.objects.get_for_model(Preprint) with capture_notifications(): @@ -2588,8 +2588,8 @@ def test_new_version_pre_mod_submit_and_reject(self, preprint_pre_mod, creator, assert new_version.machine_state == ReviewStates.REJECTED.value assert new_version.versioned_guids.first().is_rejected is True guid_obj = new_version.get_guid() - assert guid_obj.object_id == preprint_pre_mod.pk - assert guid_obj.referent == preprint_pre_mod + assert guid_obj.object_id == new_version.pk + assert guid_obj.referent == new_version def test_unpublished_preprint_post_mod_submit_and_accept(self, unpublished_preprint_post_mod, creator, moderator): assert unpublished_preprint_post_mod.is_published is False