Skip to content

Commit

Permalink
validate curator's non-biblio status and display details on contrbut…
Browse files Browse the repository at this point in the history
…or.mako and make boolean is_curator field
  • Loading branch information
John Tordoff committed Jan 9, 2025
1 parent d9540f3 commit 2ea118e
Show file tree
Hide file tree
Showing 17 changed files with 269 additions and 29 deletions.
2 changes: 1 addition & 1 deletion api/requests/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_or_curator(institution):
return True
else:
raise exceptions.PermissionDenied({'institution': 'You do not have permission to perform this action for this institution.'})
Expand Down
2 changes: 1 addition & 1 deletion api/users/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_or_curator(institution) and institution.institutional_request_access_enabled
else:
raise exceptions.ValidationError('Not valid message type.')
2 changes: 1 addition & 1 deletion api/users/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
67 changes: 67 additions & 0 deletions api_tests/nodes/views/test_node_contributor_insti_admin.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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='institution',
name='institutional_request_access_enabled',
Expand Down
11 changes: 10 additions & 1 deletion osf/models/contributor.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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):
Expand All @@ -41,6 +42,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_or_curator():
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)
Expand Down
7 changes: 6 additions & 1 deletion osf/models/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
16 changes: 14 additions & 2 deletions osf/models/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down
18 changes: 17 additions & 1 deletion osf/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,23 @@ 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_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:
return Contributor.objects.filter(
node=node,
user=self,
is_curator=True,
).exists()

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()

Expand Down
24 changes: 17 additions & 7 deletions osf/utils/machines.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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]
Expand Down
42 changes: 42 additions & 0 deletions osf_tests/test_institutional_admin_contributors.py
Original file line number Diff line number Diff line change
@@ -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()
12 changes: 8 additions & 4 deletions website/profile/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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_or_curator(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,
Expand Down Expand Up @@ -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')
]
8 changes: 8 additions & 0 deletions website/project/views/contributor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions website/static/js/accessRequestManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 2ea118e

Please sign in to comment.