validate curator's non-biblio status and display details on contrbuto…

…r.mako

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)
+        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/models/contributor.py

@@ -1,6 +1,7 @@
-from django.db import models
+from django.db import models, IntegrityError
 from osf.utils.fields import NonNaiveDateTimeField
-from osf.utils import permissions
+from osf.utils import permissions, workflows
+from django.utils.functional import cached_property
 
 
 class AbstractBaseContributor(models.Model):
@@ -35,12 +36,33 @@ class Contributor(AbstractBaseContributor):
     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
         # NOTE: Adds an _order column
         order_with_respect_to = 'node'
 
+    def save(self, *args, **kwargs):
+        if not self.user.is_institutional_admin():
+            return super().save(*args, **kwargs)
+        elif self.visible:
+            raise IntegrityError('Curators cannot be made bibliographic contributors')
+        else:
+            return super().save(*args, **kwargs)
+
 
 class PreprintContributor(AbstractBaseContributor):
     preprint = models.ForeignKey('Preprint', on_delete=models.CASCADE)

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/user.py

@@ -644,7 +644,26 @@ 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):
+    def is_institutional_admin(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(
+                node=node,
+                user=self,
+            )
+            if contrib:
+                return contrib.get().is_curator
+            else:
+                return False
+
+        if not institution:
+            return self.groups.filter(
+                name__startswith='institution_',
+                name__endswith='_institutional_admins'
+            ).exists()
+
         group_name = institution.format_group('institutional_admins')
         return self.groups.filter(name=group_name).exists()
 

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
@@ -26,6 +27,8 @@
 
 from osf.utils import notifications as notify
 
+from api.base.exceptions import Conflict
+
 class BaseMachine(Machine):
 
     action = None
@@ -199,12 +202,17 @@ 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')
+                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]

osf_tests/test_institutional_admin_contributors.py

@@ -0,0 +1,42 @@
+import pytest
+from osf.models import Contributor
+from osf_tests.factories import (
+    AuthUserFactory,
+    ProjectFactory,
+    InstitutionFactory
+)
+from django.db.utils import IntegrityError
+
+@pytest.mark.django_db
+class TestContributorModel:
+
+    @pytest.fixture()
+    def user(self):
+        return AuthUserFactory()
+
+    @pytest.fixture()
+    def project(self):
+        return ProjectFactory()
+
+    @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
+
+    def test_contributor_with_visible_and_institutional_admin_raises_error(self, institutional_admin, project, institution):
+        contributor = Contributor(
+            user=institutional_admin,
+            node=project,
+            visible=False,
+        )
+        contributor.save()
+
+        contributor.visible = True
+
+        with pytest.raises(IntegrityError, match='Curators cannot be made bibliographic contributors'):
+            contributor.save()

website/profile/utils.py

@@ -15,7 +15,7 @@ def get_profile_image_url(user, size=settings.PROFILE_IMAGE_MEDIUM):
                              use_ssl=True,
                              size=size)
 
-def serialize_user(user, node=None, admin=False, full=False, is_profile=False, include_node_counts=False):
+def serialize_user(user, node=None, admin=False, full=False, is_profile=False, include_node_counts=False, is_expected_contrib=True):
     """
     Return a dictionary representation of a registered user.
 
@@ -33,10 +33,11 @@ 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),
         'profile_image_url': user.profile_image_url(size=settings.PROFILE_IMAGE_MEDIUM),
         'active': user.is_active,
     }
-    if node is not None:
+    if node is not None and is_expected_contrib:
         if admin:
             flags = {
                 'visible': False,
@@ -194,12 +195,15 @@ def serialize_access_requests(node):
     """Serialize access requests for a node"""
     return [
         {
-            'user': serialize_user(access_request.creator),
+            'user': serialize_user(access_request.creator, is_expected_contrib=False),
             'comment': access_request.comment,
             'requested_permissions': access_request.requested_permissions,
             'id': access_request._id
         } for access_request in node.requests.filter(
-            request_type=workflows.RequestTypes.ACCESS.value,
+            request_type__in=[
+                workflows.NodeRequestTypes.ACCESS.value,
+                workflows.NodeRequestTypes.INSTITUTIONAL_REQUEST.value
+            ],
             machine_state=workflows.DefaultStates.PENDING.value
         ).select_related('creator')
     ]

website/project/views/contributor.py

@@ -3,6 +3,7 @@
 from flask import request
 from django.core.exceptions import ValidationError
 from django.utils import timezone
+from django.db import IntegrityError
 
 from framework import forms, status
 from framework.auth import cas
@@ -287,6 +288,13 @@ def project_manage_contributors(auth, node, **kwargs):
         node.manage_contributors(contributors, auth=auth, save=True)
     except (ValueError, NodeStateError) as error:
         raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data={'message_long': error.args[0]})
+    except IntegrityError as error:
+        status.push_status_message(
+            'You can not make an institutional admin a bibliographic contributor.',
+            kind='error',
+            trust=False
+        )
+        raise HTTPError(http_status.HTTP_409_CONFLICT, data={'message_long': error.args[0]})
 
     # If user has removed herself from project, alert; redirect to
     # node summary if node is public, else to user's dashboard page

website/static/js/accessRequestManager.js

@@ -22,8 +22,8 @@ var AccessRequestModel = function(accessRequest, pageOwner, isRegistration, isPa
 
     self.permissionText = ko.observable(self.options.permissionMap[self.permission()]);
 
-    self.visible = ko.observable(true);
-
+    self.is_curator = ko.observable(accessRequest.user.is_curator || false);
+    self.visible = ko.observable(!accessRequest.user.is_curator);
     self.pageOwner = pageOwner;
 
     self.expanded = ko.observable(false);

website/static/js/contribManager.js

@@ -59,6 +59,8 @@ var ContributorModel = function(contributor, currentUserCanEdit, pageOwner, isRe
 
     self.permission = ko.observable(contributor.permission);
 
+    self.is_curator = ko.observable(contributor.is_curator || false);
+
     self.permissionText = ko.observable(self.options.permissionMap[self.permission()]);
 
     self.visible = ko.observable(contributor.visible);

website/static/js/project.js

@@ -217,12 +217,18 @@ $(document).ready(function() {
            'in the Contributors list and in project citations. Non-bibliographic contributors ' +
             'can read and modify the project as normal.';
 
-    $('.visibility-info').attr(
+    $('.visibility-info-contrib').attr(
         'data-content', bibliographicContribInfoHtml
     ).popover({
         trigger: 'hover'
     });
 
+    $('.visibility-info-curator').attr(
+        'data-content', 'An administrator designated by your affiliated institution to curate your project.'
+    ).popover({
+        trigger: 'hover'
+    });
+
     ////////////////////
     // Event Handlers //
     ////////////////////