diff --git a/dandiapi/api/tests/conftest.py b/dandiapi/api/tests/conftest.py index 393084b48..e7e43fe98 100644 --- a/dandiapi/api/tests/conftest.py +++ b/dandiapi/api/tests/conftest.py @@ -31,8 +31,8 @@ register(AssetBlobFactory) register(AssetMetadataFactory) register(DandisetFactory) -register(PublishedVersionFactory) -register(DraftVersionFactory) +register(PublishedVersionFactory, _name='published_version') +register(DraftVersionFactory, _name='draft_version') # registering DraftVersionFactory after PublishedVersionFactory means # the fixture `version` will always be a draft register(UserFactory) @@ -40,6 +40,11 @@ register(VersionMetadataFactory) +@pytest.fixture(params=[DraftVersionFactory, PublishedVersionFactory], ids=['draft', 'published']) +def version(request): + return request.param() + + @pytest.fixture def api_client() -> APIClient: return APIClient() diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index fb33def92..ac887f272 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -189,33 +189,35 @@ def test_version_rest_update_not_an_owner(api_client, user, version): @pytest.mark.django_db -def test_version_rest_publish(api_client, user, version, asset): - assign_perm('owner', user, version.dandiset) - api_client.force_authenticate(user=user) - version.assets.add(asset) +# TODO change admin_user back to a normal user once publish is allowed +def test_version_rest_publish(api_client, admin_user, draft_version, asset): + assign_perm('owner', admin_user, draft_version.dandiset) + api_client.force_authenticate(user=admin_user) + draft_version.assets.add(asset) resp = api_client.post( - f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/publish/' + f'/api/dandisets/{draft_version.dandiset.identifier}' + f'/versions/{draft_version.version}/publish/' ) assert resp.data == { 'dandiset': { - 'identifier': version.dandiset.identifier, + 'identifier': draft_version.dandiset.identifier, 'created': TIMESTAMP_RE, 'modified': TIMESTAMP_RE, }, 'version': VERSION_ID_RE, - 'name': version.name, + 'name': draft_version.name, 'created': TIMESTAMP_RE, 'modified': TIMESTAMP_RE, 'asset_count': 1, - 'size': version.size, + 'size': draft_version.size, } published_version = Version.objects.get(version=resp.data['version']) assert published_version - assert version.dandiset.versions.count() == 2 + assert draft_version.dandiset.versions.count() == 2 # The original asset should now be in both versions - assert asset == version.assets.get() + assert asset == draft_version.assets.get() assert asset == published_version.assets.get() assert asset.versions.count() == 2 @@ -229,3 +231,31 @@ def test_version_rest_publish_not_an_owner(api_client, user, version, asset): f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/publish/' ) assert resp.status_code == 403 + + +# TODO remove this test once publish is allowed +@pytest.mark.django_db +def test_version_rest_publish_not_an_admin(api_client, user, version, asset): + assign_perm('owner', user, version.dandiset) + api_client.force_authenticate(user=user) + version.assets.add(asset) + + resp = api_client.post( + f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/publish/' + ) + assert resp.status_code == 403 + assert resp.data == 'Must be an admin to publish' + + +@pytest.mark.django_db +# TODO change admin_user back to a normal user once publish is allowed +def test_version_rest_publish_not_a_draft(api_client, admin_user, published_version, asset): + assign_perm('owner', admin_user, published_version.dandiset) + api_client.force_authenticate(user=admin_user) + published_version.assets.add(asset) + + resp = api_client.post( + f'/api/dandisets/{published_version.dandiset.identifier}' + f'/versions/{published_version.version}/publish/' + ) + assert resp.status_code == 405 diff --git a/dandiapi/api/views/version.py b/dandiapi/api/views/version.py index 43292a715..ca43b6214 100644 --- a/dandiapi/api/views/version.py +++ b/dandiapi/api/views/version.py @@ -1,7 +1,6 @@ from django.utils.decorators import method_decorator from drf_yasg.utils import no_body, swagger_auto_schema from guardian.decorators import permission_required_or_403 -from guardian.utils import get_40x_or_None from rest_framework import status from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticatedOrReadOnly @@ -41,12 +40,6 @@ def update(self, request, **kwargs): """Update the metadata of a version.""" version: Version = self.get_object() - # TODO @permission_required doesn't work on methods - # https://github.com/django-guardian/django-guardian/issues/723 - response = get_40x_or_None(request, ['owner'], version.dandiset, return_403=True) - if response: - return response - serializer = VersionMetadataSerializer(data=request.data) serializer.is_valid(raise_exception=True) @@ -68,7 +61,16 @@ def update(self, request, **kwargs): @action(detail=True, methods=['POST']) @method_decorator(permission_required_or_403('owner', (Dandiset, 'pk', 'dandiset__pk'))) def publish(self, request, **kwargs): + # TODO remove this check once publish is allowed + if not request.user.is_superuser: + return Response('Must be an admin to publish', status=status.HTTP_403_FORBIDDEN) + old_version = self.get_object() + if old_version.version != 'draft': + return Response( + 'Only draft versions can be published', + status=status.HTTP_405_METHOD_NOT_ALLOWED, + ) new_version = Version.copy(old_version)