add contributor page improvements for Institutional Access

api_tests/nodes/views/test_node_contributor_insti_admin.py

@@ -0,0 +1,61 @@
+import pytest
+from osf.models import Contributor, NodeLog
+from osf_tests.factories import (
+    AuthUserFactory,
+    ProjectFactory,
+    InstitutionFactory,
+)
+from api.base.settings.defaults import API_BASE
+from rest_framework import status
+from tests.utils import assert_latest_log
+
+
+@pytest.mark.django_db
+class TestChangeInstitutionalAdminContributor:
+
+    @pytest.fixture()
+    def user(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, user, institutional_admin):
+        project = ProjectFactory(creator=user)
+        project.add_contributor(institutional_admin, visible=False)
+        return project
+
+    @pytest.fixture()
+    def url_contrib(self, project, user):
+        return f'/{API_BASE}nodes/{project._id}/contributors/{user._id}/'
+
+    def test_cannot_set_institutional_admin_contributor_bibliographic(self, app, user, 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=user.auth,
+            expect_errors=True
+        )
+        assert res.status_code == 409
+        project.reload()
+        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 osf.utils.fields import NonNaiveDateTimeField
 from osf.utils import permissions
+from django.db import IntegrityError
 
 
 class AbstractBaseContributor(models.Model):
@@ -30,7 +31,6 @@ def permission(self):
 
 class Contributor(AbstractBaseContributor):
     node = models.ForeignKey('AbstractNode', on_delete=models.CASCADE)
-
     @property
     def _id(self):
         return f'{self.node._id}-{self.user._id}'
@@ -41,6 +41,14 @@ class Meta:
         # 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 can not 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

@@ -1079,7 +1079,13 @@ 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
+            )
+            contributor.visible = True
+            contributor.save()
         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

@@ -668,6 +668,10 @@ def get_full_name(self):
     def get_short_name(self):
         return self.username
 
+    @property
+    def is_institutional_admin(self):
+        return self.groups.filter(name__startswith='institution_', name__endswith='_institutional_admins').exists()
+
     def __unicode__(self):
         return self.get_short_name()
 

osf_tests/test_institutional_admin_contributors.py

@@ -0,0 +1,75 @@
+import pytest
+from django.core.exceptions import ValidationError
+from osf.models import Contributor, Institution
+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, user):
+        return ProjectFactory(creator=user)
+
+    @pytest.fixture()
+    def institution(self):
+        return InstitutionFactory()
+
+    def test_contributor_with_visible_and_institutional_admin_raises_error(self, user, project, institution):
+        contributor = Contributor(
+            user=user,
+            node=project,
+            visible=True,
+            institutional_admin=institution
+        )
+        with pytest.raises(IntegrityError, match='new row for relation "osf_contributor" violates check constraint "no_visible_with_institutional_admin"'):
+            contributor.save()  # Use clean() for validation logic
+
+    def test_contributor_with_visible_but_no_institutional_admin(self, user, project):
+        # Ensure no duplicate contributor exists
+        Contributor.objects.filter(user=user, node=project).delete()
+
+        contributor = Contributor(
+            user=user,
+            node=project,
+            visible=True,
+            institutional_admin=None
+        )
+        # This should not raise an error
+        contributor.save()
+
+    def test_contributor_with_institutional_admin_but_not_visible(self, user, project, institution):
+        # Ensure no duplicate contributor exists
+        Contributor.objects.filter(user=user, node=project).delete()
+
+        contributor = Contributor(
+            user=user,
+            node=project,
+            visible=False,
+            institutional_admin=institution
+        )
+        # This should not raise an error
+        contributor.save()
+
+    def test_database_constraint_no_visible_with_institutional_admin(self, user, project, institution):
+        # Ensure no duplicate contributor exists
+        Contributor.objects.filter(user=user, node=project).delete()
+
+        Contributor.objects.create(
+            user=user,
+            node=project,
+            visible=False,
+            institutional_admin=institution
+        )  # Should succeed
+
+        with pytest.raises(Exception):  # Check database constraint
+            Contributor.objects.create(
+                user=user,
+                node=project,
+                visible=True,
+                institutional_admin=institution
+            )

website/profile/utils.py

@@ -33,6 +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,
         'profile_image_url': user.profile_image_url(size=settings.PROFILE_IMAGE_MEDIUM),
         'active': user.is_active,
     }

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/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/templates/project/contributors.mako

@@ -196,6 +196,16 @@
                     data-html="true"
                 ></i>
             </th>
+            <th class="curator-contrib">
+                Curator
+                <i class="fa fa-question-circle visibility-info"
+                    data-toggle="popover"
+                    data-title="Curator Information"
+                    data-container="body"
+                    data-placement="right"
+                    data-html="true"
+                ></i>
+            </th>
             <th class="remove"></th>
         </tr>
     </thead>
@@ -307,12 +317,27 @@
             </div>
         </td>
         <td>
-            <div class="header" data-bind="visible: contributor.expanded() && $root.collapsed()"></div>
-            <div class="td-content" data-bind="visible: !$root.collapsed() || contributor.expanded()">
+            <div data-bind="ifnot: contributor.is_curator()">
+                <div class="header" data-bind="visible: contributor.expanded() && $root.collapsed()"></div>
+                <div class="td-content" data-bind="visible: !$root.collapsed() || contributor.expanded()">
+                    <input
+                        type="checkbox" aria-label="Bibliographic User Checkbox" class="biblio visible-filter"
+                        data-bind="checked: visible, enable: $data.canEdit() && !contributor.isParentAdmin && !deleteStaged()"
+                    />
+                </div>
+            </div>
+            <div data-bind="if: contributor.is_curator()">
                 <input
-                    type="checkbox" aria-label="Bibliographic User Checkbox" class="biblio visible-filter"
-                    data-bind="checked: visible, enable: $data.canEdit() && !contributor.isParentAdmin && !deleteStaged()"
-                />
+                    type='checkbox'
+                    aria-label='Curator Disabled Bibliographic User Checkbox'
+                    style='background-color: lightgray;'
+                    disabled
+                >
+            </div>
+        </td>
+        <td>
+            <div data-bind="if: contributor.is_curator()">
+                <input type="checkbox" aria-label="Curator Confirmation Checkbox" disabled  checked>
             </div>
         </td>
         <td data-bind="css: {'add-remove': !$root.collapsed()}">