From 52bfddb44d88a3c49bf20cfe925c96534110644f Mon Sep 17 00:00:00 2001 From: Ostap Zherebetskyi Date: Wed, 1 Oct 2025 14:24:54 +0300 Subject: [PATCH 1/2] Refactor provider admin/moderator management --- admin/preprint_providers/urls.py | 3 +- admin/preprint_providers/views.py | 56 +++++------------ admin/templates/institutions/detail.html | 19 +++--- .../templates/preprint_providers/detail.html | 2 +- .../preprint_providers/edit_moderators.html | 63 +++++++++++++++++++ 5 files changed, 88 insertions(+), 55 deletions(-) create mode 100644 admin/templates/preprint_providers/edit_moderators.html diff --git a/admin/preprint_providers/urls.py b/admin/preprint_providers/urls.py index 120c6639ab7..a829cd3304a 100644 --- a/admin/preprint_providers/urls.py +++ b/admin/preprint_providers/urls.py @@ -18,6 +18,7 @@ re_path(r'^(?P[a-z0-9]+)/delete/$', views.DeletePreprintProvider.as_view(), name='delete'), re_path(r'^(?P[a-z0-9]+)/export/$', views.ExportPreprintProvider.as_view(), name='export'), re_path(r'^(?P[a-z0-9]+)/share_source/$', views.ShareSourcePreprintProvider.as_view(), name='share_source'), - re_path(r'^(?P[a-z0-9]+)/register/$', views.PreprintProviderRegisterModeratorOrAdmin.as_view(), name='register_moderator_admin'), re_path(r'^(?P[a-z0-9]+)/edit/$', views.PreprintProviderChangeForm.as_view(), name='edit'), + re_path(r'^(?P[a-z0-9]+)/add_admin_or_moderator/$', views.PreprintAddAdminOrModerator.as_view(), name='add_admin_or_moderator'), + re_path(r'^(?P[a-z0-9]+)/remove_admins_and_moderators/$', views.PreprintRemoveAdminsAndModerators.as_view(), name='remove_admins_and_moderators'), ] diff --git a/admin/preprint_providers/views.py b/admin/preprint_providers/views.py index d841981fe84..10035702ea4 100644 --- a/admin/preprint_providers/views.py +++ b/admin/preprint_providers/views.py @@ -1,25 +1,23 @@ import json -from django.http import Http404 from django.core import serializers from django.core.exceptions import ValidationError -from django.urls import reverse_lazy, reverse +from django.urls import reverse_lazy from django.http import HttpResponse, JsonResponse from django.views.generic import ListView, DetailView, View, CreateView, DeleteView, TemplateView, UpdateView -from django.views.generic.edit import FormView from django.contrib import messages from django.contrib.auth.mixins import PermissionRequiredMixin from django.forms.models import model_to_dict from django.shortcuts import redirect, render -from django.utils.functional import cached_property from admin.base import settings from admin.base.forms import ImportFileForm -from admin.preprint_providers.forms import PreprintProviderForm, PreprintProviderCustomTaxonomyForm, PreprintProviderRegisterModeratorOrAdminForm -from osf.models import PreprintProvider, Subject, OSFUser, RegistrationProvider, CollectionProvider +from admin.preprint_providers.forms import PreprintProviderForm, PreprintProviderCustomTaxonomyForm +from osf.models import PreprintProvider, Subject, RegistrationProvider, CollectionProvider from osf.models.provider import rules_to_subjects, WhitelistedSHAREPreprintProvider from website import settings as website_settings +from admin.providers.views import AddAdminOrModerator, RemoveAdminsAndModerators FIELDS_TO_NOT_IMPORT_EXPORT = ['access_token', 'share_source', 'subjects_acceptable', 'primary_collection'] @@ -454,43 +452,17 @@ def get(self, request): return render(request, self.template_name, {'share_api_url': share_api_url, 'api_v2_url': api_v2_url}) -class PreprintProviderRegisterModeratorOrAdmin(PermissionRequiredMixin, FormView): +class PreprintAddAdminOrModerator(AddAdminOrModerator): permission_required = 'osf.change_preprintprovider' + template_name = 'preprint_providers/edit_moderators.html' + provider_class = PreprintProvider + url_namespace = 'preprint_providers' raise_exception = True - template_name = 'preprint_providers/register_moderator_admin.html' - form_class = PreprintProviderRegisterModeratorOrAdminForm - - @cached_property - def target_provider(self): - return PreprintProvider.objects.get(id=self.kwargs['preprint_provider_id']) - - def get_form_kwargs(self): - kwargs = super().get_form_kwargs() - kwargs['provider_groups'] = self.target_provider.group_objects - return kwargs - - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context['provider_name'] = self.target_provider.name - return context - - def form_valid(self, form): - user_id = form.cleaned_data.get('user_id') - osf_user = OSFUser.load(user_id) - - if not osf_user: - raise Http404(f'OSF user with id "{user_id}" not found. Please double check.') - - if osf_user.has_groups(self.target_provider.group_names): - messages.error(self.request, f'User with guid: {user_id} is already a moderator or admin') - return super().form_invalid(form) - group = form.cleaned_data.get('group_perms') - self.target_provider.add_to_group(osf_user, group) - osf_user.save() - messages.success(self.request, f'Permissions update successful for OSF User {osf_user.username}!') - return super().form_valid(form) - - def get_success_url(self): - return reverse('preprint_providers:register_moderator_admin', kwargs={'preprint_provider_id': self.kwargs['preprint_provider_id']}) +class PreprintRemoveAdminsAndModerators(RemoveAdminsAndModerators): + permission_required = 'osf.change_preprintprovider' + template_name = 'preprint_providers/edit_moderators.html' + provider_class = PreprintProvider + url_namespace = 'preprint_providers' + raise_exception = True diff --git a/admin/templates/institutions/detail.html b/admin/templates/institutions/detail.html index 47315d8e8d7..43f56c3b563 100644 --- a/admin/templates/institutions/detail.html +++ b/admin/templates/institutions/detail.html @@ -24,12 +24,15 @@ Delete institution {% endif %} {% if perms.osf.change_institution %} - {% if institution.deactivated is None %} - Deactivate institution - {% else %} - Reactivate institution - {% endif %} + {% if institution.deactivated is None %} + Deactivate institution + {% else %} + Reactivate institution + {% endif %} {% endif %} + {% if perms.osf.change_institution %} + Manage Admins + {% endif %} @@ -64,12 +67,6 @@

Banner:

-
- Create Moderator/Admin -
-
- Contributors list -
{% endif %} diff --git a/admin/templates/preprint_providers/detail.html b/admin/templates/preprint_providers/detail.html index 109dc50bd0b..b945829699b 100644 --- a/admin/templates/preprint_providers/detail.html +++ b/admin/templates/preprint_providers/detail.html @@ -36,7 +36,7 @@ Setup Share Source {% endif %} {% if perms.osf.change_preprintprovider %} - Register Moderator/Admin + Manage Admins and Moderators Modify Preprint Provider {% if show_taxonomies %} Modify Custom Taxonomy diff --git a/admin/templates/preprint_providers/edit_moderators.html b/admin/templates/preprint_providers/edit_moderators.html new file mode 100644 index 00000000000..dc2901907eb --- /dev/null +++ b/admin/templates/preprint_providers/edit_moderators.html @@ -0,0 +1,63 @@ +{% extends "base.html" %} +{% load static %} +{% load render_bundle from webpack_loader %} +{% block title %} + Preprint Provider +{% endblock title %} +{% block content %} +
+
+ {% if messages %} +
    + {% for message in messages %} + {{ message }} + {% endfor %} +
+ {% endif %} +
+
+
+

{{ provider.name }}

+
+
+
+
+
+ {% csrf_token %} + + + + +
+
+
+
+
+
+
+ {% csrf_token %} + + + + + {% for moderator in moderators %} + + + + + + {% endfor %} + {% for admin in admins %} + + + + + + {% endfor %} +
NameType
{{ moderator.fullname }} ({{moderator.username}})Moderator
{{ admin.fullname }} ({{admin.username}})Admin
+ +
+
+
+
+{% endblock content %} From d145e87f3ccc14aa07c814d9ee08ef39a513bcf0 Mon Sep 17 00:00:00 2001 From: Ostap Zherebetskyi Date: Wed, 1 Oct 2025 15:04:42 +0300 Subject: [PATCH 2/2] add preprint provider admin/moderator tests --- admin_tests/preprint_providers/test_views.py | 96 +++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/admin_tests/preprint_providers/test_views.py b/admin_tests/preprint_providers/test_views.py index 83bba5eca0f..29e65641635 100644 --- a/admin_tests/preprint_providers/test_views.py +++ b/admin_tests/preprint_providers/test_views.py @@ -29,7 +29,7 @@ from admin.preprint_providers import views from admin.preprint_providers.forms import PreprintProviderForm from admin.base.forms import ImportFileForm - +from django.contrib.messages.storage.fallback import FallbackStorage import website pytestmark = pytest.mark.django_db @@ -424,3 +424,97 @@ def provider_factory(self): def view(self, req): plain_view = views.ProcessCustomTaxonomy() return setup_view(plain_view, req) + + +from osf.migrations import update_provider_auth_groups + +@pytest.mark.urls('admin.base.urls') +class TestEditModerators: + + @pytest.fixture() + def req(self, user): + req = RequestFactory().get('/fake_path') + req.user = user + return req + + @pytest.fixture() + def provider(self): + provider = PreprintProviderFactory() + update_provider_auth_groups() + return provider + + @pytest.fixture() + def moderator(self, provider): + user = AuthUserFactory() + provider.add_to_group(user, 'moderator') + return user + + @pytest.fixture() + def user(self): + return AuthUserFactory() + + @pytest.fixture() + def remove_moderator_view(self, req, provider): + view = views.PreprintRemoveAdminsAndModerators() + view = setup_view(view, req) + view.kwargs = {'provider_id': provider.id} + return view + + @pytest.fixture() + def add_moderator_view(self, req, provider): + view = views.PreprintAddAdminOrModerator() + view = setup_view(view, req) + view.kwargs = {'provider_id': provider.id} + return view + + def test_get(self, add_moderator_view, remove_moderator_view, req): + res = add_moderator_view.get(req) + assert res.status_code == 200 + + res = remove_moderator_view.get(req) + assert res.status_code == 200 + + def test_post_remove(self, remove_moderator_view, req, moderator, provider): + moderator_id = f'Moderator-{moderator.id}' + req.POST = { + 'csrfmiddlewaretoken': 'fake csfr', + moderator_id: ['on'] + } + + # django.contrib.messages has a bug which effects unittests + # more info here -> https://code.djangoproject.com/ticket/17971 + setattr(req, 'session', 'session') + messages = FallbackStorage(req) + setattr(req, '_messages', messages) + + res = remove_moderator_view.post(req) + assert res.status_code == 302 + assert not provider.get_group('moderator').user_set.all() + + def test_post_add(self, add_moderator_view, req, user, provider): + req.POST = { + 'csrfmiddlewaretoken': 'fake csfr', + 'add-moderators-form': [user._id], + 'moderator': ['Add Moderator'] + } + + # django.contrib.messages has a bug which effects unittests + # more info here -> https://code.djangoproject.com/ticket/17971 + setattr(req, 'session', 'session') + messages = FallbackStorage(req) + setattr(req, '_messages', messages) + + res = add_moderator_view.post(req) + assert res.status_code == 302 + assert user in provider.get_group('moderator').user_set.all() + + # try to add the same user, but another group + req.POST = { + 'csrfmiddlewaretoken': 'fake csfr', + 'add-moderators-form': [user._id], + 'admin': ['Add Admin'] + } + res = add_moderator_view.post(req) + assert res.status_code == 302 + assert user in provider.get_group('moderator').user_set.all() + assert user not in provider.get_group('admin').user_set.all()