Skip to content

Commit

Permalink
fixed comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bodintsov committed Jan 8, 2025
1 parent aad9717 commit c8e345c
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 31 deletions.
2 changes: 1 addition & 1 deletion api/institutions/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class InstitutionSerializer(JSONAPISerializer):
ser.CharField(read_only=True),
permission='view_institutional_metrics',
)
can_request_access = ser.CharField(read_only=True)
institutional_request_access_enabled = ser.CharField(read_only=True)
links = LinksField({
'self': 'get_api_url',
'html': 'get_absolute_html_url',
Expand Down
3 changes: 3 additions & 0 deletions api/requests/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ def has_permission(self, request, view):
except Institution.DoesNotExist:
raise exceptions.ValidationError({'institution': 'Institution is does not exist.'})

if not institution.institutional_request_access_enabled:
raise exceptions.PermissionDenied({'institution': 'Institutional request is not enabled.'})

if get_user_auth(request).user.is_institutional_admin(institution):
return True
else:
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)
return user.is_institutional_admin(institution) and institution.institutional_request_access_enabled
else:
raise exceptions.ValidationError('Not valid message type.')
54 changes: 52 additions & 2 deletions api_tests/requests/views/test_node_request_institutional_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ def url(self, project):

@pytest.fixture()
def institution(self):
return InstitutionFactory(can_request_access=True)
return InstitutionFactory(institutional_request_access_enabled=True)

@pytest.fixture()
def institution2(self):
def institution_without_access(self):
return InstitutionFactory()

@pytest.fixture()
Expand All @@ -30,6 +30,12 @@ def user_with_affiliation(self, institution):
user.add_or_update_affiliated_institution(institution)
return user

@pytest.fixture()
def user_with_affiliation_on_institution_without_access(self, institution2):
user = AuthUserFactory()
user.add_or_update_affiliated_institution(institution2)
return user

@pytest.fixture()
def user_without_affiliation(self):
return AuthUserFactory()
Expand All @@ -40,6 +46,12 @@ def institutional_admin(self, institution):
institution.get_group('institutional_admins').user_set.add(admin_user)
return admin_user

@pytest.fixture()
def institutional_admin_on_institution_without_access(self, institution2):
admin_user = AuthUserFactory()
institution2.get_group('institutional_admins').user_set.add(admin_user)
return admin_user

@pytest.fixture()
def create_payload(self, institution, user_with_affiliation):
return {
Expand All @@ -66,6 +78,32 @@ def create_payload(self, institution, user_with_affiliation):
}
}

@pytest.fixture()
def create_payload_on_institution_without_access(self, institution_without_access, user_with_affiliation_on_institution_without_access):
return {
'data': {
'attributes': {
'comment': 'Wanna Philly Philly?',
'request_type': NodeRequestTypes.INSTITUTIONAL_REQUEST.value,
},
'relationships': {
'institution': {
'data': {
'id': institution_without_access._id,
'type': 'institutions'
}
},
'message_recipient': {
'data': {
'id': user_with_affiliation_on_institution_without_access._id,
'type': 'users'
}
}
},
'type': 'node-requests'
}
}

@pytest.fixture()
def node_with_disabled_access_requests(self, institution):
node = NodeFactory()
Expand Down Expand Up @@ -112,6 +150,18 @@ def test_institutional_admin_can_add_requested_permission(self, app, project, in
assert node_request.request_type == NodeRequestTypes.INSTITUTIONAL_REQUEST.value
assert node_request.requested_permissions == 'admin'

def test_institutional_admin_can_not_add_requested_permission(self, app, project, institutional_admin_on_institution_without_access, url, create_payload_on_institution_without_access):
"""
Test that an institutional admin can not make an institutional access request on institution with disabled access .
"""
create_payload_on_institution_without_access['data']['attributes']['requested_permissions'] = 'admin'

res = app.post_json_api(
url, create_payload_on_institution_without_access, auth=institutional_admin_on_institution_without_access.auth, expect_errors=True
)

assert res.status_code == 403

def test_institutional_admin_needs_institution(self, app, project, institutional_admin, url, create_payload):
"""
Test that the payload needs the institution relationship and gives the correct error message.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class TestUserMessageInstitutionalAccess:

@pytest.fixture()
def institution(self):
return InstitutionFactory(can_request_access=True)
return InstitutionFactory(institutional_request_access_enabled=True)

@pytest.fixture()
def institution_without_access(self):
Expand Down Expand Up @@ -115,7 +115,7 @@ def test_institutional_admin_can_not_create_message(self, mock_send_mail, app, i
institution_without_access, url_with_affiliation_on_institution_without_access,
payload):
"""
Ensure an institutional admin cannot create a `UserMessage` with a `message` and `institution` witch has 'can_request_access' as False
Ensure an institutional admin cannot create a `UserMessage` with a `message` and `institution` witch has 'institutional_request_access_enabled' as False
"""
mock_send_mail.return_value = mock.MagicMock()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 4.2.13 on 2024-12-12 19:37
# Generated by Django 4.2.15 on 2025-01-08 12:24

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='institution',
name='institutional_request_access_enabled',
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name='noderequest',
name='requested_permissions',
Expand Down
18 changes: 0 additions & 18 deletions osf/migrations/0026_institution_can_request_access.py

This file was deleted.

2 changes: 1 addition & 1 deletion osf/models/institution.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class Institution(DirtyFieldsMixin, Loggable, ObjectIDMixin, BaseModel, Guardian
related_name='institutions'
)

can_request_access = models.BooleanField(default=False)
institutional_request_access_enabled = models.BooleanField(default=False)
is_deleted = models.BooleanField(default=False, db_index=True)
deleted = NonNaiveDateTimeField(null=True, blank=True)
deactivated = NonNaiveDateTimeField(null=True, blank=True)
Expand Down
2 changes: 1 addition & 1 deletion osf/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ def osf_groups(self):

def is_institutional_admin(self, institution):
group_name = institution.format_group('institutional_admins')
return self.groups.filter(name=group_name).exists() and institution.can_request_access
return self.groups.filter(name=group_name).exists()

def group_role(self, group):
"""
Expand Down
5 changes: 1 addition & 4 deletions osf_tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,14 +257,11 @@ class InstitutionFactory(DjangoModelFactory):
email_domains = FakeList('domain_name', n=1)
orcid_record_verified_source = ''
delegation_protocol = ''
can_request_access = False
institutional_request_access_enabled = False

class Meta:
model = models.Institution

@classmethod
def _create(cls, *args, **kwargs):
return super()._create(*args, **kwargs)

class NodeLicenseRecordFactory(DjangoModelFactory):
year = factory.Faker('year')
Expand Down

0 comments on commit c8e345c

Please sign in to comment.