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
  • Loading branch information
John Tordoff committed Jan 7, 2025
1 parent d34ceb8 commit 70f2629
Show file tree
Hide file tree
Showing 10 changed files with 181 additions and 10 deletions.
60 changes: 60 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,60 @@
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 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
)
# TODO: check message
assert not contributor.visible
9 changes: 9 additions & 0 deletions osf/models/contributor.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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():
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)
Expand Down
8 changes: 7 additions & 1 deletion osf/models/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
11 changes: 10 additions & 1 deletion osf/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,16 @@ 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):
"""
Checks if user is admin of any or of a specific institution.
"""
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
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 can not be made bibliographic contributors'):
contributor.save()
6 changes: 5 additions & 1 deletion website/profile/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -198,7 +199,10 @@ def serialize_access_requests(node):
'comment': access_request.comment,
'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
2 changes: 2 additions & 0 deletions website/static/js/contribManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 7 additions & 1 deletion website/static/js/project.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 curator your project.'
).popover({
trigger: 'hover'
});

////////////////////
// Event Handlers //
////////////////////
Expand Down
37 changes: 31 additions & 6 deletions website/templates/project/contributors.mako
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,24 @@
</th>
<th class="biblio-contrib">
Bibliographic Contributor
<i class="fa fa-question-circle visibility-info"
<i class="fa fa-question-circle visibility-info-contrib"
data-toggle="popover"
data-title="Bibliographic Contributor Information"
data-container="body"
data-placement="right"
data-html="true"
></i>
</th>
<th class="curator-contrib">
Curator
<i class="fa fa-question-circle visibility-info-curator"
data-toggle="popover"
data-title="Curator Information"
data-content="body"
data-placement="right"
data-html="true"
></i>
</th>
<th class="remove"></th>
</tr>
</thead>
Expand Down Expand Up @@ -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()}">
Expand Down

0 comments on commit 70f2629

Please sign in to comment.