-
Notifications
You must be signed in to change notification settings - Fork 336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ENG-6668] Add Contributor Page Improvements for Institutional Access #10825
Changes from 6 commits
4e507b1
52d1a43
d1a7412
5c33833
756a53d
8ef8cd6
0da606a
2bbcf07
6c8eb0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
@@ -0,0 +1,18 @@ | ||
# Generated by Django 4.2.13 on 2025-01-09 21:22 | ||
|
||
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), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,7 @@ | |
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 | ||
|
@@ -57,6 +58,7 @@ | |
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 | ||
|
||
|
@@ -644,9 +646,43 @@ 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): | ||
group_name = institution.format_group('institutional_admins') | ||
return self.groups.filter(name=group_name).exists() | ||
def is_institutional_admin_at(self, institution): | ||
""" | ||
Checks if user is admin of a specific institution. | ||
""" | ||
return self.has_perms( | ||
institution.groups['institutional_admins'], | ||
institution | ||
) | ||
|
||
def is_institutional_admin(self): | ||
""" | ||
Checks if user is admin of any institution. | ||
""" | ||
return self.groups.filter( | ||
name__startswith='institution_', | ||
name__endswith='_institutional_admins' | ||
).exists() | ||
|
||
def is_institutional_curator(self, node): | ||
""" | ||
Checks if user is user has curator permissions for a node. | ||
""" | ||
return Contributor.objects.filter( | ||
node=node, | ||
user=self, | ||
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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to be sure, is there any place we can cache this info, such as a request object, rather than relying on this query, the same way we did with caching on the contributor object if it's a curator? Same reason as before, to avoid making this query for every contributor that gets serialized. |
||
|
||
def group_role(self, group): | ||
""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overwriting
save
doesn't always cut it, but this looks like that only place we.update
visibility, arguably we want to retain the ability to change the visibility which leans away from placing in DB constraints. However I think this is all visibility updates in the existing code.