revert last commit and fix data model by adding more booleans and ser…

…ializing more directly from the objects to the template

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_tests/nodes/views/test_node_contributor_insti_admin.py

@@ -33,7 +33,7 @@ def institutional_admin(self, institution):
     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)
+        project.add_contributor(institutional_admin, visible=False, make_curator=True)
         return project
 
     @pytest.fixture()

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

@@ -43,9 +43,8 @@ class Meta:
         order_with_respect_to = 'node'
 
     def save(self, *args, **kwargs):
-        if self.user.is_institutional_admin():
-            if self.visible:
-                raise IntegrityError('Curators cannot be made bibliographic contributors')
+        if self.is_curator and self.visible:
+            raise IntegrityError('Curators cannot be made bibliographic contributors')
 
         return super().save(*args, **kwargs)
 

osf/models/mixins.py

@@ -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
@@ -1838,6 +1839,8 @@ def get_visible(self, user):
     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 user.is_curator(self):
+            raise ValueError('Curators cannot be made bibliographic contributors')
         kwargs = self.contributor_kwargs
         kwargs['user'] = user
         kwargs['visible'] = True

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

@@ -45,7 +45,6 @@
 from .spam import SpamMixin
 from .session import UserSessionMap
 from .tag import Tag
-from .request import NodeRequest
 from .validators import validate_email, validate_social, validate_history_item
 from osf.utils.datetime_aware_jsonfield import DateTimeAwareJSONField
 from osf.utils.fields import NonNaiveDateTimeField, LowercaseEmailField, ensure_str
@@ -58,7 +57,6 @@
 from website.util.metrics import OsfSourceTags, unregistered_created_source_tag
 from importlib import import_module
 from osf.utils.requests import get_headers_from_request
-from osf.utils.workflows import NodeRequestTypes
 
 SessionStore = import_module(settings.SESSION_ENGINE).SessionStore
 
@@ -674,16 +672,6 @@ def is_institutional_curator(self, node):
             is_curator=True,
         ).exists()
 
-    def has_institutional_request(self, node):
-        """
-        Checks if user a has requested a node using the institutional access request feature.
-        """
-        return NodeRequest.objects.filter(
-            request_type=NodeRequestTypes.INSTITUTIONAL_REQUEST.value,
-            target=node,
-            creator=self,
-        ).exists()
-
     def group_role(self, group):
         """
         For the given OSFGroup, return the user's role - either member or manager

osf_tests/test_institutional_admin_contributors.py

@@ -1,9 +1,11 @@
 import pytest
+
 from osf.models import Contributor
 from osf_tests.factories import (
     AuthUserFactory,
     ProjectFactory,
-    InstitutionFactory
+    InstitutionFactory,
+    NodeRequestFactory
 )
 from django.db.utils import IntegrityError
 
@@ -14,6 +16,26 @@ class TestContributorModel:
     def user(self):
         return AuthUserFactory()
 
+    @pytest.fixture()
+    def user_with_institutional_request(self, project):
+        user = AuthUserFactory()
+        NodeRequestFactory(
+            target=project,
+            creator=user,
+            is_institutional_request=True,
+        )
+        return user
+
+    @pytest.fixture()
+    def user_with_non_institutional_request(self, project):
+        user = AuthUserFactory()
+        NodeRequestFactory(
+            target=project,
+            creator=user,
+            is_institutional_request=False,
+        )
+        return user
+
     @pytest.fixture()
     def project(self):
         return ProjectFactory()
@@ -37,6 +59,25 @@ def curator(self, institutional_admin, project):
             is_curator=True
         )
 
+    def test_contributor_with_visible_and_pending_request_raises_error(self, user_with_institutional_request, project, institution):
+        user_with_institutional_request.save()
+        user_with_institutional_request.visible = True
+        user_with_institutional_request.refresh_from_db()
+        assert user_with_institutional_request.visible
+
+        try:
+            project.add_contributor(user_with_institutional_request, make_curator=True)
+        except IntegrityError as e:
+            assert e.args == ('Curators cannot be made bibliographic contributors',)
+
+    def test_contributor_with_visible_and_valid_request(self, user_with_non_institutional_request, project, institution):
+        user_with_non_institutional_request.save()
+        user_with_non_institutional_request.visible = True
+        user_with_non_institutional_request.save()
+
+        user_with_non_institutional_request.refresh_from_db()
+        assert user_with_non_institutional_request.visible
+
     def test_contributor_with_visible_and_institutional_admin_raises_error(self, curator, project, institution):
         curator.save()
         curator.visible = True

website/profile/utils.py

@@ -33,8 +33,6 @@ 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:],
-        'has_institutional_request': user.has_institutional_request(node),
-        'is_curator': user.is_institutional_curator(node),
         'profile_image_url': user.profile_image_url(size=settings.PROFILE_IMAGE_MEDIUM),
         'active': user.is_active,
     }
@@ -53,7 +51,8 @@ def serialize_user(user, node=None, admin=False, full=False, is_profile=False, i
             is_contributor_obj = isinstance(contrib, Contributor)
             flags = {
                 'visible': contrib.visible if is_contributor_obj else node.contributor_set.filter(user=user, visible=True).exists(),
-                'permission': contrib.permission if is_contributor_obj else None
+                'permission': contrib.permission if is_contributor_obj else None,
+                'is_curator': contrib.is_curator,
             }
         ret.update(flags)
     if user.is_registered:
@@ -197,6 +196,7 @@ def serialize_access_requests(node):
     return [
         {
             'user': serialize_user(access_request.creator),
+            'is_institutional_request': access_request.is_institutional_request,
             'comment': access_request.comment,
             'requested_permissions': access_request.requested_permissions,
             'id': access_request._id

website/static/js/accessRequestManager.js

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

website/templates/project/contributors.mako

@@ -405,23 +405,23 @@
         <td>
             <div class="header" data-bind="visible: accessRequest.expanded()  && $root.collapsed()"></div>
             <div class="td-content" data-bind="visible: !$root.collapsed() || accessRequest.expanded()">
-            <div data-bind="ifnot: accessRequest.user.has_institutional_request">
+            <div data-bind="ifnot: accessRequest.is_institutional_request">
                 <input
                     type="checkbox" class="biblio"
                     data-bind="checked: visible"
                 />
             </div>
-            <div data-bind="if: accessRequest.user.has_institutional_request">
+            <div data-bind="if: accessRequest.is_institutional_request">
                 <input type="checkbox" aria-label="Curator Confirmation Checkbox" disabled>
             </div>
         </td>
         <td>
             <div class="header" data-bind="visible: accessRequest.expanded()  && $root.collapsed()"></div>
             <div class="td-content" data-bind="visible: !$root.collapsed() || accessRequest.expanded()">
-                <div data-bind="ifnot: accessRequest.user.has_institutional_request">
+                <div data-bind="ifnot: accessRequest.is_institutional_request">
                     <input type="checkbox" aria-label="Curator Confirmation Checkbox" disabled>
                 </div>
-                <div data-bind="if: accessRequest.user.has_institutional_request">
+                <div data-bind="if: accessRequest.is_institutional_request">
                     <input type="checkbox" aria-label="Curator Confirmation Checkbox" disabled  checked>
                 </div>
             </div>