Skip to content

Commit

Permalink
add more tests and logical changes
Browse files Browse the repository at this point in the history
  • Loading branch information
Johnetordoff committed Jan 31, 2025
1 parent 2b22253 commit ebc6caa
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 14 deletions.
36 changes: 23 additions & 13 deletions api/requests/serializers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from django.db import transaction
from django.db import transaction, IntegrityError
from rest_framework import exceptions
from rest_framework import serializers as ser

Expand Down Expand Up @@ -209,18 +209,28 @@ def _create_node_request(self, node, validated_data) -> NodeRequest:
comment = validated_data.get('comment', '')
requested_permissions = validated_data.get('requested_permissions')
with transaction.atomic():
node_request, created = NodeRequest.objects.update_or_create(
target=node,
creator=creator,
defaults={
'comment': comment,
'machine_state': DefaultStates.INITIAL.value,
'request_type': request_type,
'requested_permissions': requested_permissions,
},
)
if not created and request_type != NodeRequestTypes.INSTITUTIONAL_REQUEST.value:
raise Conflict(f"Users may not have more than one {request_type} request per node.")
try:
node_request, created = NodeRequest.objects.update_or_create(
target=node,
creator=creator,
request_type=request_type,
defaults={
'comment': comment,
'machine_state': DefaultStates.INITIAL.value,
'requested_permissions': requested_permissions,
},
)
if not created and request_type != NodeRequestTypes.INSTITUTIONAL_REQUEST.value:
raise Conflict(f"Users may not have more than one {request_type} request per node.")
except (NodeRequest.MultipleObjectsReturned, IntegrityError):
node_request = NodeRequest.objects.filter(
target=node,
creator=creator,
).order_by('-created').first()
node_request.comment = comment
node_request.requested_permissions = requested_permissions
node_request.machine_state = DefaultStates.INITIAL.value
node_request.request_type = request_type

node_request.save()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ def test_requester_can_resubmit(self, app, project, institutional_admin, url, cr
node_request.refresh_from_db()
assert node_request.machine_state == 'pending'

def test_requester_can_after_access_resubmit(self, app, project, institutional_admin, url, create_payload_non_institutional_access, create_payload):
def test_requester_can_make_insti_request_after_access_resubmit(self, app, project, institutional_admin, url, create_payload_non_institutional_access, create_payload):
"""
Test that a requester can submit another access request, then institutional access for the same node.
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import pytest
from osf_tests.factories import (
NodeRequestFactory,
AuthUserFactory,
NodeFactory,
InstitutionFactory
)
from api.base.settings.defaults import API_BASE
from osf.utils.workflows import NodeRequestTypes

@pytest.mark.django_db
class TestUniqueIndexOnNodeRequest:

@pytest.fixture()
def url(self, project):
return f'/{API_BASE}nodes/{project._id}/requests/'

@pytest.fixture()
def user(self):
return AuthUserFactory()

@pytest.fixture()
def project(self, user_with_affiliation, institution):
node = NodeFactory()
node.add_affiliated_institution(institution, user_with_affiliation)
return node

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

@pytest.fixture()
def user_with_affiliation(self, institution):
user = AuthUserFactory()
user.add_or_update_affiliated_institution(institution)
institution.get_group('institutional_admins').user_set.add(user)
return user

@pytest.fixture()
def create_payload(self, project, institution):
return {
'data': {
'attributes': {
'comment': 'Testing unique index',
'request_type': NodeRequestTypes.INSTITUTIONAL_REQUEST.value,
},
'relationships': {
'institution': {
'data': {
'id': institution._id,
'type': 'institutions'
}
},
},
'type': 'node-requests'
}
}

def test_multiple_node_requests_error(self, app, project, user, user_with_affiliation, url, create_payload):
"""
Ensure that multiple NodeRequest objects for the same user and target raise an error.
"""
NodeRequestFactory(
target=project,
creator=user_with_affiliation,
request_type=NodeRequestTypes.INSTITUTIONAL_REQUEST.value,
machine_state='accepted',
)
NodeRequestFactory(
target=project,
creator=user_with_affiliation,
request_type=NodeRequestTypes.INSTITUTIONAL_REQUEST.value,
machine_state='accepted',
)

res = app.post_json_api(
url,
create_payload,
auth=user_with_affiliation.auth,
)
assert res.status_code == 201

0 comments on commit ebc6caa

Please sign in to comment.