From 8171f74c00379029f382536bd365cf885d87c654 Mon Sep 17 00:00:00 2001 From: John Tordoff <> Date: Thu, 9 Jan 2025 13:32:40 -0500 Subject: [PATCH] make is_curator a boolean field to optimize --- api/requests/permissions.py | 2 +- api/users/permissions.py | 2 +- api/users/serializers.py | 2 +- ...> 0025_contributor_is_curator_and_more.py} | 7 ++++++- osf/models/contributor.py | 19 +++---------------- osf/models/mixins.py | 7 ++++++- osf/models/user.py | 11 ++++------- osf/utils/machines.py | 7 +++++-- website/profile/utils.py | 2 +- 9 files changed, 28 insertions(+), 31 deletions(-) rename osf/migrations/{0025_noderequest_requested_permissions_and_more.py => 0025_contributor_is_curator_and_more.py} (93%) diff --git a/api/requests/permissions.py b/api/requests/permissions.py index cb08a6a94e5..a405daaba84 100644 --- a/api/requests/permissions.py +++ b/api/requests/permissions.py @@ -78,7 +78,7 @@ def has_permission(self, request, view): except Institution.DoesNotExist: raise exceptions.ValidationError({'institution': 'Institution is does not exist.'}) - if get_user_auth(request).user.is_institutional_admin(institution): + if get_user_auth(request).user.is_institutional_admin_or_curator(institution): return True else: raise exceptions.PermissionDenied({'institution': 'You do not have permission to perform this action for this institution.'}) diff --git a/api/users/permissions.py b/api/users/permissions.py index 7ca5e3ef862..0be9aa3f580 100644 --- a/api/users/permissions.py +++ b/api/users/permissions.py @@ -79,6 +79,6 @@ def has_permission(self, request, view) -> bool: message_type = request.data.get('message_type') if message_type == MessageTypes.INSTITUTIONAL_REQUEST: - return user.is_institutional_admin(institution) + return user.is_institutional_admin_or_curator(institution) else: raise exceptions.ValidationError('Not valid message type.') diff --git a/api/users/serializers.py b/api/users/serializers.py index c1b8f49fc89..b733909cdda 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -760,7 +760,7 @@ def create(self, validated_data: Dict[str, Any]) -> UserMessage: 'institution', ) - if not sender.is_institutional_admin(institution): + if not sender.is_institutional_admin_or_curator(institution): raise Conflict({'sender': 'Only institutional administrators can create messages.'}) if not recipient.is_affiliated_with_institution(institution): diff --git a/osf/migrations/0025_noderequest_requested_permissions_and_more.py b/osf/migrations/0025_contributor_is_curator_and_more.py similarity index 93% rename from osf/migrations/0025_noderequest_requested_permissions_and_more.py rename to osf/migrations/0025_contributor_is_curator_and_more.py index 32183df5067..a34f0add24c 100644 --- a/osf/migrations/0025_noderequest_requested_permissions_and_more.py +++ b/osf/migrations/0025_contributor_is_curator_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.13 on 2024-12-12 19:37 +# Generated by Django 4.2.13 on 2025-01-09 18:05 from django.conf import settings from django.db import migrations, models @@ -14,6 +14,11 @@ class Migration(migrations.Migration): ] operations = [ + migrations.AddField( + model_name='contributor', + name='is_curator', + field=models.BooleanField(default=False), + ), migrations.AddField( model_name='noderequest', name='requested_permissions', diff --git a/osf/models/contributor.py b/osf/models/contributor.py index 448c5f102fb..70384527885 100644 --- a/osf/models/contributor.py +++ b/osf/models/contributor.py @@ -1,7 +1,6 @@ from django.db import models, IntegrityError from osf.utils.fields import NonNaiveDateTimeField -from osf.utils import permissions, workflows -from django.utils.functional import cached_property +from osf.utils import permissions class AbstractBaseContributor(models.Model): @@ -31,24 +30,12 @@ def permission(self): class Contributor(AbstractBaseContributor): node = models.ForeignKey('AbstractNode', on_delete=models.CASCADE) + is_curator = models.BooleanField(default=False) @property def _id(self): return f'{self.node._id}-{self.user._id}' - @cached_property - def is_curator(self): - """ - Determine if the user is a curator on this node. - This avoids querying the database repeatedly by caching the result. - """ - from osf.models import NodeRequest - return NodeRequest.objects.filter( - target=self.node, - creator=self.user, - request_type=workflows.NodeRequestTypes.INSTITUTIONAL_REQUEST - ).exists() - class Meta: unique_together = ('user', 'node') # Make contributors orderable @@ -56,7 +43,7 @@ class Meta: order_with_respect_to = 'node' def save(self, *args, **kwargs): - if not self.user.is_institutional_admin(): + if not self.user.is_institutional_admin_or_curator(): return super().save(*args, **kwargs) elif self.visible: raise IntegrityError('Curators cannot be made bibliographic contributors') diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 17c35db04a6..ed64ebe6457 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1327,7 +1327,7 @@ def _get_admin_contributors_query(self, users, require_active=True): return qs def add_contributor(self, contributor, permissions=None, visible=True, - send_email=None, auth=None, log=True, save=False): + send_email=None, auth=None, log=True, save=False, make_curator=False): """Add a contributor to the project. :param User contributor: The contributor to be added @@ -1393,6 +1393,11 @@ def add_contributor(self, contributor, permissions=None, visible=True, if getattr(self, 'get_identifier_value', None) and self.get_identifier_value('doi'): request, user_id = get_request_and_user_id() self.update_or_enqueue_on_resource_updated(user_id, first_save=False, saved_fields=['contributors']) + + if make_curator: + contributor_obj.is_curator = True + contributor_obj.save() + return contrib_to_add def add_contributors(self, contributors, auth=None, log=True, save=False): diff --git a/osf/models/user.py b/osf/models/user.py index ac88209b92d..41ed77bb5aa 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -644,19 +644,16 @@ def osf_groups(self): OSFGroup = apps.get_model('osf.OSFGroup') return get_objects_for_user(self, 'member_group', OSFGroup, with_superuser=False) - def is_institutional_admin(self, institution=None, node=None): + def is_institutional_admin_or_curator(self, institution=None, node=None): """ Checks if user is admin of any or of a specific institution, or and curator on a specific node. """ if node: - contrib = Contributor.objects.filter( + return Contributor.objects.filter( node=node, user=self, - ) - if contrib: - return contrib.get().is_curator - else: - return False + is_curator=True, + ).exists() if not institution: return self.groups.filter( diff --git a/osf/utils/machines.py b/osf/utils/machines.py index deb27419ce3..2cb1ead8ff6 100644 --- a/osf/utils/machines.py +++ b/osf/utils/machines.py @@ -8,7 +8,6 @@ from osf.exceptions import InvalidTransitionError from osf.models.preprintlog import PreprintLog from osf.models.action import ReviewAction, NodeRequestAction, PreprintRequestAction - from osf.utils import permissions from osf.utils.workflows import ( DefaultStates, @@ -20,6 +19,7 @@ APPROVAL_TRANSITIONS, CollectionSubmissionStates, COLLECTION_SUBMISSION_TRANSITIONS, + NodeRequestTypes ) from website.mails import mails from website.reviews import signals as reviews_signals @@ -202,13 +202,16 @@ def save_changes(self, ev): if ev.event.name == DefaultTriggers.ACCEPT.value: if not self.machineable.target.is_contributor(self.machineable.creator): contributor_permissions = ev.kwargs.get('permissions', permissions.READ) + make_curator = True if self.machineable.request_type == NodeRequestTypes.INSTITUTIONAL_REQUEST.value else False try: self.machineable.target.add_contributor( self.machineable.creator, auth=Auth(ev.kwargs['user']), permissions=contributor_permissions, visible=ev.kwargs.get('visible', True), - send_email=f'{self.machineable.request_type}_request') + send_email=f'{self.machineable.request_type}_request', + make_curator=make_curator, + ) except IntegrityError as e: if 'Curators cannot be made bibliographic contributors' in str(e): raise Conflict(str(e)) from e diff --git a/website/profile/utils.py b/website/profile/utils.py index afd8e267db8..a8d4cb1637c 100644 --- a/website/profile/utils.py +++ b/website/profile/utils.py @@ -33,7 +33,7 @@ def serialize_user(user, node=None, admin=False, full=False, is_profile=False, i 'surname': user.family_name, 'fullname': fullname, 'shortname': fullname if len(fullname) < 50 else fullname[:23] + '...' + fullname[-23:], - 'is_curator': user.is_institutional_admin(node=node), + 'is_curator': user.is_institutional_admin_or_curator(node=node), 'profile_image_url': user.profile_image_url(size=settings.PROFILE_IMAGE_MEDIUM), 'active': user.is_active, }