From 0272d919e52186dba80441ef953f3fa21156c508 Mon Sep 17 00:00:00 2001 From: Mike VanDenburgh Date: Mon, 19 Sep 2022 19:36:58 -0400 Subject: [PATCH] Improve endpoint response, update test Add specific error messages instead of returing "Dandiset metadata or asset metadata is not valid" everytime a publish can't occur --- dandiapi/api/tests/test_version.py | 24 +++++++++++++++++------- dandiapi/api/views/version.py | 30 ++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index a86bd73c9..f2fe2ee1a 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -678,14 +678,24 @@ def test_version_rest_publish_not_a_draft(api_client, user, published_version, a @pytest.mark.django_db @pytest.mark.parametrize( - 'status', + 'status,expected_data,expected_status_code', [ - Version.Status.PENDING, - Version.Status.VALIDATING, - Version.Status.INVALID, + (Version.Status.PENDING, 'Dandiset metadata or asset metadata is not valid', 400), + (Version.Status.VALIDATING, 'Dandiset is currently being validated', 409), + (Version.Status.INVALID, 'Dandiset metadata or asset metadata is not valid', 400), + (Version.Status.PUBLISHED, 'No changes since last publish', 400), + (Version.Status.PUBLISHING, 'Dandiset is currently being published', 423), ], ) -def test_version_rest_publish_invalid_metadata(api_client, user, draft_version, asset, status): +def test_version_rest_publish_invalid( + api_client: APIClient, + user: User, + draft_version: Version, + asset: Asset, + status: str, + expected_data: str, + expected_status_code: int, +): assign_perm('owner', user, draft_version.dandiset) api_client.force_authenticate(user=user) draft_version.assets.add(asset) @@ -697,8 +707,8 @@ def test_version_rest_publish_invalid_metadata(api_client, user, draft_version, f'/api/dandisets/{draft_version.dandiset.identifier}' f'/versions/{draft_version.version}/publish/' ) - assert resp.status_code == 400 - assert resp.data == 'Dandiset metadata or asset metadata is not valid' + assert resp.status_code == expected_status_code + assert resp.data == expected_data @pytest.mark.django_db diff --git a/dandiapi/api/views/version.py b/dandiapi/api/views/version.py index 1b8a3480a..c9defa7df 100644 --- a/dandiapi/api/views/version.py +++ b/dandiapi/api/views/version.py @@ -124,14 +124,28 @@ def publish(self, request, **kwargs): ): raise ValidationError('Cannot publish dandisets which contain zarrs') if not version.valid: - resp_text = ( - 'No changes since last publish' - if version.status == Version.Status.PUBLISHED - else 'Dandiset is currently being published' - if version.status == Version.Status.PUBLISHING - else 'Dandiset metadata or asset metadata is not valid' - ) - return Response(resp_text, status=status.HTTP_400_BAD_REQUEST) + match version.status: + case Version.Status.PUBLISHED: + resp_text, resp_status = ( + 'No changes since last publish', + status.HTTP_400_BAD_REQUEST, + ) + case Version.Status.PUBLISHING: + resp_text, resp_status = ( + 'Dandiset is currently being published', + status.HTTP_423_LOCKED, + ) + case Version.Status.VALIDATING: + resp_text, resp_status = ( + 'Dandiset is currently being validated', + status.HTTP_409_CONFLICT, + ) + case _: + resp_text, resp_status = ( + 'Dandiset metadata or asset metadata is not valid', + status.HTTP_400_BAD_REQUEST, + ) + return Response(resp_text, status=resp_status) version.status = Version.Status.PUBLISHING version.save(update_fields=['status'])