diff --git a/rdmo/accounts/admin.py b/rdmo/accounts/admin.py index 4e4bece4f2..ac5a14342a 100644 --- a/rdmo/accounts/admin.py +++ b/rdmo/accounts/admin.py @@ -1,4 +1,6 @@ from django.contrib import admin +from django.contrib.sites.models import Site +from django.db.models import Count, Value from .models import AdditionalField, AdditionalFieldValue, ConsentFieldValue, Role @@ -20,7 +22,34 @@ def has_add_permission(self, request, obj=None): class RoleAdmin(admin.ModelAdmin): search_fields = ('user__username', 'user__email') - list_filter = ('member', 'manager') + list_filter = ('member', 'manager', 'editor', 'reviewer') + + list_display = ('user', 'members', 'managers', 'editors', 'reviewers') + + def get_queryset(self, request): + return Role.objects.prefetch_related( + 'member', 'manager', 'editor', 'reviewer').annotate( + Count('member'), Count('manager'), Count('editor'), Count('reviewer'), + sites_count=Value(Site.objects.count()) + ) + + @staticmethod + def render_all_sites_or_join(obj, field_name: str) -> str: + if getattr(obj, f'{field_name}__count', 0) == obj.sites_count: + return 'all Sites' + return ', '.join([site.domain for site in getattr(obj, field_name).all()]) + + def members(self, obj): + return self.render_all_sites_or_join(obj, 'member') + + def managers(self, obj): + return self.render_all_sites_or_join(obj, 'manager') + + def editors(self, obj): + return self.render_all_sites_or_join(obj, 'editor') + + def reviewers(self, obj): + return self.render_all_sites_or_join(obj, 'reviewer') admin.site.register(AdditionalField, AdditionalFieldAdmin) diff --git a/rdmo/accounts/apps.py b/rdmo/accounts/apps.py index 2f28eb0b9a..b7c205ad2a 100644 --- a/rdmo/accounts/apps.py +++ b/rdmo/accounts/apps.py @@ -5,6 +5,3 @@ class AccountsConfig(AppConfig): name = 'rdmo.accounts' verbose_name = _('Accounts') - - def ready(self): - from . import rules diff --git a/rdmo/accounts/migrations/0020_add_role_editor_and_reviewer.py b/rdmo/accounts/migrations/0020_add_role_editor_and_reviewer.py new file mode 100644 index 0000000000..566f260410 --- /dev/null +++ b/rdmo/accounts/migrations/0020_add_role_editor_and_reviewer.py @@ -0,0 +1,24 @@ +# Generated by Django 3.2.16 on 2023-02-17 14:25 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('sites', '0002_alter_domain_unique'), + ('accounts', '0019_delete_proxyuser'), + ] + + operations = [ + migrations.AddField( + model_name='role', + name='editor', + field=models.ManyToManyField(blank=True, help_text='The sites for which this user is an editor.', related_name='editors', to='sites.Site', verbose_name='Editor'), + ), + migrations.AddField( + model_name='role', + name='reviewer', + field=models.ManyToManyField(blank=True, help_text='The sites for which this user is a reviewer.', related_name='reviewers', to='sites.Site', verbose_name='Reviewer'), + ), + ] diff --git a/rdmo/accounts/models.py b/rdmo/accounts/models.py index 78ad3a61be..9d9c097a76 100644 --- a/rdmo/accounts/models.py +++ b/rdmo/accounts/models.py @@ -137,6 +137,16 @@ class Role(models.Model): verbose_name=_('Manager'), help_text=_('The sites for which this user is manager.') ) + editor = models.ManyToManyField( + Site, related_name='editors', blank=True, + verbose_name=_('Editor'), + help_text=_('The sites for which this user is an editor.') + ) + reviewer = models.ManyToManyField( + Site, related_name='reviewers', blank=True, + verbose_name=_('Reviewer'), + help_text=_('The sites for which this user is a reviewer.') + ) class Meta: ordering = ('user', ) diff --git a/rdmo/accounts/rules.py b/rdmo/accounts/rules.py deleted file mode 100644 index cd0def179b..0000000000 --- a/rdmo/accounts/rules.py +++ /dev/null @@ -1,16 +0,0 @@ -import rules -from django.contrib.sites.models import Site - -from .utils import is_site_manager as is_site_manager_util - - -@rules.predicate -def is_site_manager(manager, user): - if is_site_manager_util(manager): - current_site = Site.objects.get_current() - return current_site in user.role.member.all() - else: - return False - - -rules.add_perm('auth.view_user_object', is_site_manager) diff --git a/rdmo/accounts/serializers/v1.py b/rdmo/accounts/serializers/v1.py index c78bb001c8..ed9e627b6a 100644 --- a/rdmo/accounts/serializers/v1.py +++ b/rdmo/accounts/serializers/v1.py @@ -34,13 +34,17 @@ class RoleSerializer(serializers.ModelSerializer): member = SiteSerializer(many=True) manager = SiteSerializer(many=True) + editor = SiteSerializer(many=True) + reviewer = SiteSerializer(many=True) class Meta: model = Role fields = ( 'id', 'member', - 'manager' + 'manager', + 'editor', + 'reviewer' ) diff --git a/rdmo/accounts/tests/test_admin.py b/rdmo/accounts/tests/test_admin.py new file mode 100644 index 0000000000..c5d51dc50b --- /dev/null +++ b/rdmo/accounts/tests/test_admin.py @@ -0,0 +1,13 @@ +import pytest + +from django.urls import reverse + + +roles = ('member', 'manager', 'editor', 'reviewer') + + +@pytest.mark.parametrize('role', roles) +def test_admin_accounts_role(admin_client, role): + url = reverse('admin:accounts_role_changelist') + f'?q={role}' + response = admin_client.get(url) + assert response.status_code == 200 diff --git a/rdmo/accounts/tests/test_views.py b/rdmo/accounts/tests/test_views.py index ef06426c68..30ceef5283 100644 --- a/rdmo/accounts/tests/test_views.py +++ b/rdmo/accounts/tests/test_views.py @@ -16,10 +16,24 @@ ('editor', 'editor'), ('reviewer', 'reviewer'), ('user', 'user'), + ('site', 'site'), ('api', 'api'), ('anonymous', None), ) +other_site_users = ( + 'foo-user', + 'foo-manager', + 'foo-editor', + 'foo-reviewer', + 'bar-user', + 'bar-manager', + 'bar-editor', + 'bar-reviewer', +) + +users += tuple(zip(other_site_users, other_site_users)) # add (other site users and passwords) + boolean_toggle = (True, False) diff --git a/rdmo/accounts/tests/test_viewsets.py b/rdmo/accounts/tests/test_viewsets.py index dc648067ab..10fd0369f2 100644 --- a/rdmo/accounts/tests/test_viewsets.py +++ b/rdmo/accounts/tests/test_viewsets.py @@ -3,27 +3,49 @@ from django.urls import reverse users = ( - ('site', 'site'), - ('api', 'api'), + ('site', 'site'), # site manager for all sites + ('editor', 'editor'), # editor for all sites + ('reviewer', 'reviewer'), # reviewer for all sites + ('api', 'api'), # has all roles for all sites, same as superuser + ('admin', 'admin'), # superuser ('user', 'user'), - ('anonymous', None), + ('anonymous', None) +) + +more_example_users = ( + 'example-user', + 'example-manager', + 'example-editor', + 'example-reviewer' +) + +members_from_other_sites = ( + 'other', + 'foo-user', + 'foo-manager', + 'foo-editor', + 'foo-reviewer', + 'bar-user', + 'bar-manager', + 'bar-editor', + 'bar-reviewer', ) status_map = { 'list': { - 'site': 200, 'api': 200, 'user': 200, 'anonymous': 401 + 'editor': 403, 'reviewer': 403, 'site': 403, 'api': 403, 'user': 403, 'anonymous': 401, 'admin' : 200 }, 'detail': { - 'site': 200, 'api': 200, 'user': 404, 'anonymous': 401 + 'editor': 404, 'reviewer': 404, 'site': 404, 'api': 404, 'user': 404, 'anonymous': 401, 'admin' : 200 }, 'create': { - 'site': 405, 'api': 405, 'user': 405, 'anonymous': 401 + 'editor': 403, 'reviewer': 403, 'site': 403, 'api': 403, 'user': 403, 'anonymous': 401, 'admin' : 405 }, 'update': { - 'site': 405, 'api': 405, 'user': 405, 'anonymous': 401 + 'editor': 405, 'reviewer': 405, 'site': 405, 'api': 405, 'user': 405, 'anonymous': 401, 'admin' : 405 }, 'delete': { - 'site': 405, 'api': 405, 'user': 405, 'anonymous': 401 + 'editor': 405, 'reviewer': 405, 'site': 405, 'api': 405, 'user': 405, 'anonymous': 401, 'admin' : 405 } } @@ -41,11 +63,11 @@ def test_list(db, client, username, password): response = client.get(url) assert response.status_code == status_map['list'][username], response.json() if response.status_code == 200: - if username == 'api': - assert len(response.json()) == 11 + if username == 'api' or username == 'admin': + assert len(response.json()) == get_user_model().objects.count() elif username == 'site': - # the site admin must not see the user 'other' - assert len(response.json()) == 10 + # the site manager for example.com must see only the members of example.com + assert len(response.json()) == get_user_model().objects.count() - len(members_from_other_sites) else: assert len(response.json()) == 0 @@ -54,12 +76,11 @@ def test_list(db, client, username, password): def test_detail(db, client, username, password): client.login(username=username, password=password) instances = get_user_model().objects.all() - for instance in instances: url = reverse(urlnames['detail'], args=[instance.pk]) response = client.get(url) - if username == 'site' and instance.username == 'other': - # the site admin must not see the user 'other' + if username == 'site' and not instance.role.member.filter(domain__contains='example.com').exists(): + # the site manager for example.com must see only the members of example.com assert response.status_code == 404, response.json() else: assert response.status_code == status_map['detail'][username], response.json() diff --git a/rdmo/accounts/viewsets.py b/rdmo/accounts/viewsets.py index 8cc62c19cd..baa733cb85 100644 --- a/rdmo/accounts/viewsets.py +++ b/rdmo/accounts/viewsets.py @@ -1,10 +1,11 @@ from django.contrib.auth import get_user_model -from django.contrib.sites.models import Site from django_filters.rest_framework import DjangoFilterBackend from rest_framework.viewsets import ReadOnlyModelViewSet from rdmo.core.permissions import HasModelPermission, HasObjectPermission +from rdmo.management.rules import is_editor, is_reviewer +from .models import Role from .serializers.v1 import UserSerializer from .utils import is_site_manager @@ -16,8 +17,7 @@ def get_users_for_user(self, user): if user.has_perm('auth.view_user'): return get_user_model().objects.all() elif is_site_manager(user): - current_site = Site.objects.get_current() - return get_user_model().objects.filter(role__member=current_site) + return get_user_model().objects.filter(role__member__id__in=user.role.manager.all()).distinct() return get_user_model().objects.none() @@ -36,4 +36,7 @@ class UserViewSet(UserViewSetMixin, ReadOnlyModelViewSet): def get_queryset(self): return self.get_users_for_user(self.request.user) \ - .prefetch_related('groups', 'role__member', 'role__manager', 'memberships') + .prefetch_related('groups', + 'role__member', 'role__manager', + 'role__editor', 'role__reviewer', + 'memberships') diff --git a/rdmo/conditions/admin.py b/rdmo/conditions/admin.py index 12e953f6d9..17db94527c 100644 --- a/rdmo/conditions/admin.py +++ b/rdmo/conditions/admin.py @@ -24,6 +24,7 @@ class ConditionAdmin(admin.ModelAdmin): list_display = ('uri', 'source', 'relation', 'target_text', 'target_option') readonly_fields = ('uri', ) list_filter = ('relation', ) + filter_horizontal = ('editors', ) admin.site.register(Condition, ConditionAdmin) diff --git a/rdmo/conditions/imports.py b/rdmo/conditions/imports.py index 3598459e72..e33edeaf49 100644 --- a/rdmo/conditions/imports.py +++ b/rdmo/conditions/imports.py @@ -1,5 +1,7 @@ import logging +from django.contrib.sites.models import Site + from rdmo.core.imports import (set_common_fields, set_foreign_field, validate_instance) @@ -34,5 +36,6 @@ def import_condition(element, save=False): logger.info('Condition created with uri %s.', element.get('uri')) condition.save() + condition.editors.add(Site.objects.get_current()) return condition diff --git a/rdmo/conditions/migrations/0023_condition_editors.py b/rdmo/conditions/migrations/0023_condition_editors.py new file mode 100644 index 0000000000..981a734920 --- /dev/null +++ b/rdmo/conditions/migrations/0023_condition_editors.py @@ -0,0 +1,19 @@ +# Generated by Django 3.2.16 on 2023-02-24 14:14 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('sites', '0002_alter_domain_unique'), + ('conditions', '0022_condition_locked'), + ] + + operations = [ + migrations.AddField( + model_name='condition', + name='editors', + field=models.ManyToManyField(blank=True, help_text='The sites that can edit this condition (in a multi site setup).', related_name='condition_editors', to='sites.Site', verbose_name='Editors'), + ), + ] diff --git a/rdmo/conditions/models.py b/rdmo/conditions/models.py index b89524cb86..4251305dfb 100644 --- a/rdmo/conditions/models.py +++ b/rdmo/conditions/models.py @@ -1,4 +1,5 @@ from django.conf import settings +from django.contrib.sites.models import Site from django.db import models from django.utils.translation import gettext_lazy as _ @@ -54,6 +55,11 @@ class Condition(models.Model): verbose_name=_('Locked'), help_text=_('Designates whether this condition can be changed.') ) + editors = models.ManyToManyField( + Site, related_name='%(class)s_editors', blank=True, + verbose_name=_('Editors'), + help_text=_('The sites that can edit this condition (in a multi site setup).') + ) source = models.ForeignKey( Attribute, db_constraint=False, blank=True, null=True, on_delete=models.SET_NULL, related_name='conditions', verbose_name=_('Source'), @@ -89,7 +95,7 @@ def save(self, *args, **kwargs): def copy(self, uri_prefix, key): condition = copy_model(self, uri_prefix=uri_prefix, key=key, source=self.source, target_option=self.target_option) - + return condition @property diff --git a/rdmo/conditions/serializers/v1.py b/rdmo/conditions/serializers/v1.py index 264cdf37f4..d262a3b30f 100644 --- a/rdmo/conditions/serializers/v1.py +++ b/rdmo/conditions/serializers/v1.py @@ -1,7 +1,8 @@ from rest_framework import serializers from rdmo.core.serializers import (ElementExportSerializerMixin, - ElementModelSerializerMixin) + ElementModelSerializerMixin, + ReadOnlyObjectPermissionsSerializerMixin) from rdmo.domain.models import Attribute from rdmo.options.models import OptionSet from rdmo.questions.models import Page, Question, QuestionSet @@ -11,7 +12,8 @@ from ..validators import ConditionLockedValidator, ConditionUniqueURIValidator -class ConditionSerializer(ElementModelSerializerMixin, serializers.ModelSerializer): +class ConditionSerializer(ElementModelSerializerMixin, ReadOnlyObjectPermissionsSerializerMixin, + serializers.ModelSerializer): model = serializers.SerializerMethodField() key = serializers.SlugField(required=True) @@ -23,6 +25,8 @@ class ConditionSerializer(ElementModelSerializerMixin, serializers.ModelSerializ questions = serializers.PrimaryKeyRelatedField(queryset=Question.objects.all(), required=False, many=True) tasks = serializers.PrimaryKeyRelatedField(queryset=Task.objects.all(), required=False, many=True) + read_only = serializers.SerializerMethodField() + class Meta: model = Condition fields = ( @@ -33,6 +37,8 @@ class Meta: 'key', 'comment', 'locked', + 'read_only', + 'editors', 'source', 'relation', 'target_text', diff --git a/rdmo/conditions/static/conditions/js/conditions.js b/rdmo/conditions/static/conditions/js/conditions.js index a134a215e1..f0bea5f171 100644 --- a/rdmo/conditions/static/conditions/js/conditions.js +++ b/rdmo/conditions/static/conditions/js/conditions.js @@ -14,6 +14,7 @@ angular.module('conditions', ['core']) attributes: $resource(baseurl + 'api/v1/domain/attributes/:id/'), options: $resource(baseurl + 'api/v1/options/options/:id/'), settings: $resource(baseurl + 'api/v1/core/settings/'), + sites: $resource(baseurl + 'api/v1/core/sites/') }; /* configure factories */ @@ -36,6 +37,7 @@ angular.module('conditions', ['core']) service.init = function(options) { service.relations = resources.relations.query(); service.settings = resources.settings.get(); + service.sites = resources.sites.query(); service.uri_prefixes = [] service.uri_prefix = '' service.filter = sessionStorage.getItem('conditions_filter') || ''; diff --git a/rdmo/conditions/templates/conditions/conditions.html b/rdmo/conditions/templates/conditions/conditions.html index 327ed5d093..e8a7428098 100644 --- a/rdmo/conditions/templates/conditions/conditions.html +++ b/rdmo/conditions/templates/conditions/conditions.html @@ -95,6 +95,9 @@

{% trans 'Conditions' %}

+ + {% trans 'Conditions' %} ng-click="service.openDeleteModal('conditions', condition)">
- {% trans 'Condition' %} + + {% trans 'Condition' %} + {$ condition.uri $}
@@ -136,6 +141,27 @@

{% trans 'Conditions' %}

{$ condition.description $} + {% if settings.MULTISITE %} +
+
+
+ {% trans 'Editors' %} +
+
+
    +
  • + {$ editor.name $} +
  • +
  • + {% trans 'All sites' %} +
  • +
+
+
+
+ {% endif %} + diff --git a/rdmo/conditions/templates/conditions/conditions_modal_delete_conditions.html b/rdmo/conditions/templates/conditions/conditions_modal_delete_conditions.html index 62c9f79c1c..91ce2063c9 100644 --- a/rdmo/conditions/templates/conditions/conditions_modal_delete_conditions.html +++ b/rdmo/conditions/templates/conditions/conditions_modal_delete_conditions.html @@ -36,10 +36,14 @@