Merge branch 'feature/institutional_access' of https://github.com/Cen…

…terForOpenScience/osf.io into feature/institutional_access

* 'feature/institutional_access' of https://github.com/CenterForOpenScience/osf.io:
  [ENG-6668] Add Contributor Page Improvements for Institutional Access (CenterForOpenScience#10825)

api/requests/permissions.py

@@ -81,7 +81,7 @@ def has_permission(self, request, view):
         if not institution.institutional_request_access_enabled:
             raise exceptions.PermissionDenied({'institution': 'Institutional request access is not enabled.'})
 
-        if get_user_auth(request).user.is_institutional_admin(institution):
+        if get_user_auth(request).user.is_institutional_admin_at(institution):
             return True
         else:
             raise exceptions.PermissionDenied({'institution': 'You do not have permission to perform this action for this institution.'})

api/requests/serializers.py

@@ -177,6 +177,8 @@ def create(self, validated_data) -> NodeRequest:
     def make_node_institutional_access_request(self, node, validated_data) -> NodeRequest:
         sender = self.context['request'].user
         node_request = self._create_node_request(node, validated_data)
+        node_request.is_institutional_request = True
+        node_request.save()
         institution = Institution.objects.get(_id=validated_data['institution'])
         recipient = OSFUser.load(validated_data.get('message_recipient'))
 

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) and institution.institutional_request_access_enabled
+            return user.is_institutional_admin_at(institution) and institution.institutional_request_access_enabled
         else:
             raise exceptions.ValidationError('Not valid message type.')

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_at(institution):
             raise Conflict({'sender': 'Only institutional administrators can create messages.'})
 
         if not recipient.is_affiliated_with_institution(institution):

api_tests/nodes/views/test_node_contributor_insti_admin.py

@@ -0,0 +1,67 @@
+import pytest
+from osf.models import Contributor
+from osf_tests.factories import (
+    AuthUserFactory,
+    ProjectFactory,
+    InstitutionFactory,
+)
+from api.base.settings.defaults import API_BASE
+
+
+@pytest.mark.django_db
+class TestChangeInstitutionalAdminContributor:
+
+    @pytest.fixture()
+    def project_admin(self):
+        return AuthUserFactory()
+
+    @pytest.fixture()
+    def project_admin2(self):
+        return AuthUserFactory()
+
+    @pytest.fixture()
+    def institution(self):
+        return InstitutionFactory()
+
+    @pytest.fixture()
+    def institutional_admin(self, institution):
+        admin_user = AuthUserFactory()
+        institution.get_group('institutional_admins').user_set.add(admin_user)
+        return admin_user
+
+    @pytest.fixture()
+    def project(self, project_admin, project_admin2, institutional_admin):
+        project = ProjectFactory(creator=project_admin)
+        project.add_contributor(project_admin2, permissions='admin', visible=False)
+        project.add_contributor(institutional_admin, visible=False, make_curator=True)
+        return project
+
+    @pytest.fixture()
+    def url_contrib(self, project, institutional_admin):
+        return f'/{API_BASE}nodes/{project._id}/contributors/{institutional_admin._id}/'
+
+    def test_cannot_set_institutional_admin_contributor_bibliographic(self, app, project_admin, project,
+                                                                      institutional_admin, url_contrib):
+        res = app.put_json_api(
+            url_contrib,
+            {
+                'data': {
+                    'id': f'{project._id}-{institutional_admin._id}',
+                    'type': 'contributors',
+                    'attributes': {
+                        'bibliographic': True,
+                    }
+                }
+            },
+            auth=project_admin.auth,
+            expect_errors=True
+        )
+
+        assert res.status_code == 409
+        assert res.json['errors'][0]['detail'] == 'Curators cannot be made bibliographic contributors'
+
+        contributor = Contributor.objects.get(
+            node=project,
+            user=institutional_admin
+        )
+        assert not contributor.visible

osf/migrations/0025_noderequest_requested_permissions_and_more.py → osf/migrations/0025_contributor_is_curator_and_more.py

@@ -1,4 +1,4 @@
-# Generated by Django 4.2.15 on 2025-01-08 12:24
+# Generated by Django 4.2.13 on 2025-01-09 19:19
 
 from django.conf import settings
 from django.db import migrations, models

osf/migrations/0026_contributor_is_curator_and_more.py

@@ -0,0 +1,28 @@
+# Generated by Django 4.2.13 on 2025-01-10 19:27
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('osf', '0025_contributor_is_curator_and_more'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='contributor',
+            name='is_curator',
+            field=models.BooleanField(default=False),
+        ),
+        migrations.AddField(
+            model_name='noderequest',
+            name='is_institutional_request',
+            field=models.BooleanField(default=False),
+        ),
+        migrations.AddField(
+            model_name='preprintrequest',
+            name='is_institutional_request',
+            field=models.BooleanField(default=False),
+        ),
+    ]

osf/models/contributor.py

@@ -1,4 +1,4 @@
-from django.db import models
+from django.db import models, IntegrityError
 from osf.utils.fields import NonNaiveDateTimeField
 from osf.utils import permissions
 
@@ -30,6 +30,7 @@ def permission(self):
 
 class Contributor(AbstractBaseContributor):
     node = models.ForeignKey('AbstractNode', on_delete=models.CASCADE)
+    is_curator = models.BooleanField(default=False)
 
     @property
     def _id(self):
@@ -41,6 +42,12 @@ class Meta:
         # NOTE: Adds an _order column
         order_with_respect_to = 'node'
 
+    def save(self, *args, **kwargs):
+        if self.is_curator and self.visible:
+            raise IntegrityError('Curators cannot be made bibliographic contributors')
+
+        return super().save(*args, **kwargs)
+
 
 class PreprintContributor(AbstractBaseContributor):
     preprint = models.ForeignKey('Preprint', on_delete=models.CASCADE)

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
@@ -1338,6 +1338,7 @@ def add_contributor(self, contributor, permissions=None, visible=True,
         :param Auth auth: All the auth information including user, API key
         :param bool log: Add log to self
         :param bool save: Save after adding contributor
+        :param bool make_curator incicates whether the user should be an institituional curator
         :returns: Whether contributor was added
         """
         send_email = send_email or self.contributor_email_template
@@ -1393,6 +1394,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):
@@ -1839,7 +1845,11 @@ def set_visible(self, user, visible, log=True, auth=None, save=False):
         if visible and not self.contributor_class.objects.filter(**kwargs).exists():
             set_visible_kwargs = kwargs
             set_visible_kwargs['visible'] = False
-            self.contributor_class.objects.filter(**set_visible_kwargs).update(visible=True)
+            contribs = self.contributor_class.objects.filter(**set_visible_kwargs)
+            if self.guardian_object_type == 'node' and contribs.filter(is_curator=True).exists():
+                raise ValueError('Curators cannot be made bibliographic contributors')
+            contribs.update(visible=True)
+
         elif not visible and self.contributor_class.objects.filter(**kwargs).exists():
             num_visible_kwargs = self.contributor_kwargs
             num_visible_kwargs['visible'] = True

osf/models/node.py

@@ -15,7 +15,7 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.core.paginator import Paginator
 from django.urls import reverse
-from django.db import models, connection
+from django.db import models, connection, IntegrityError
 from django.db.models.signals import post_save
 from django.dispatch import receiver
 from django.utils import timezone
@@ -77,6 +77,7 @@
 from website.util.metrics import OsfSourceTags, CampaignSourceTags
 from website.util import api_url_for, api_v2_url, web_url_for
 from .base import BaseModel, GuidMixin, GuidMixinQuerySet
+from api.base.exceptions import Conflict
 from api.caching.tasks import update_storage_usage
 from api.caching import settings as cache_settings
 from api.caching.utils import storage_usage_cache
@@ -1079,7 +1080,18 @@ def set_visible(self, user, visible, log=True, auth=None, save=False):
         if not self.is_contributor(user):
             raise ValueError(f'User {user} not in contributors')
         if visible and not Contributor.objects.filter(node=self, user=user, visible=True).exists():
-            Contributor.objects.filter(node=self, user=user, visible=False).update(visible=True)
+            contributor = Contributor.objects.get(
+                node=self,
+                user=user,
+                visible=False
+            )
+            try:
+                contributor.visible = True
+                contributor.save()
+            except IntegrityError as e:
+                if 'Curators cannot be made bibliographic contributors' in str(e):
+                    raise Conflict(str(e)) from e
+                raise e
         elif not visible and Contributor.objects.filter(node=self, user=user, visible=True).exists():
             if Contributor.objects.filter(node=self, visible=True).count() == 1:
                 raise ValueError('Must have at least one visible contributor')

osf/models/request.py

@@ -12,6 +12,7 @@ class Meta:
     request_type = models.CharField(max_length=31, choices=RequestTypes.choices())
     creator = models.ForeignKey('OSFUser', related_name='submitted_%(class)s', on_delete=models.CASCADE)
     comment = models.TextField(null=True, blank=True)
+    is_institutional_request = models.BooleanField(default=False)
 
     @property
     def target(self):

osf/models/user.py

@@ -644,9 +644,33 @@ 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):
-        group_name = institution.format_group('institutional_admins')
-        return self.groups.filter(name=group_name).exists()
+    def is_institutional_admin_at(self, institution):
+        """
+        Checks if user is admin of a specific institution.
+        """
+        return self.has_perms(
+            institution.groups['institutional_admins'],
+            institution
+        )
+
+    def is_institutional_admin(self):
+        """
+        Checks if user is admin of any institution.
+        """
+        return self.groups.filter(
+            name__startswith='institution_',
+            name__endswith='_institutional_admins'
+        ).exists()
+
+    def is_institutional_curator(self, node):
+        """
+        Checks if user is user has curator permissions for a node.
+        """
+        return Contributor.objects.filter(
+            node=node,
+            user=self,
+            is_curator=True,
+        ).exists()
 
     def group_role(self, group):
         """

osf/utils/machines.py

@@ -1,4 +1,5 @@
 from django.utils import timezone
+from django.db import IntegrityError
 from transitions import Machine, MachineError
 
 from api.providers.workflows import Workflows
@@ -7,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,
@@ -19,13 +19,16 @@
     APPROVAL_TRANSITIONS,
     CollectionSubmissionStates,
     COLLECTION_SUBMISSION_TRANSITIONS,
+    NodeRequestTypes
 )
 from website.mails import mails
 from website.reviews import signals as reviews_signals
 from website.settings import DOMAIN, OSF_SUPPORT_EMAIL, OSF_CONTACT_EMAIL
 
 from osf.utils import notifications as notify
 
+from api.base.exceptions import Conflict
+
 class BaseMachine(Machine):
 
     action = None
@@ -199,12 +202,19 @@ 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)
-                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')
+                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',
+                        make_curator=self.machineable.request_type == NodeRequestTypes.INSTITUTIONAL_REQUEST.value,
+                    )
+                except IntegrityError as e:
+                    if 'Curators cannot be made bibliographic contributors' in str(e):
+                        raise Conflict(str(e)) from e
+                    raise e
 
     def resubmission_allowed(self, ev):
         # TODO: [PRODUCT-395]