From 4c8c2318c1e6c66efd73bce2edff752b31ab1d1d Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Thu, 26 Feb 2026 12:22:36 -0500 Subject: [PATCH 01/13] Separate pending assets and incomplete zarrs --- dandiapi/api/models/version.py | 34 ++++++++++---- dandiapi/api/tests/test_version.py | 73 ++++++++++++++++++++---------- 2 files changed, 73 insertions(+), 34 deletions(-) diff --git a/dandiapi/api/models/version.py b/dandiapi/api/models/version.py index 9363b8aa3..71885e5f0 100644 --- a/dandiapi/api/models/version.py +++ b/dandiapi/api/models/version.py @@ -97,6 +97,8 @@ def publishable(self) -> bool: @property def asset_validation_errors(self) -> list[VersionAssetValidationError]: # Import here to avoid dependency cycle + from dandiapi.zarr.models import ZarrArchiveStatus + from .asset import Asset # We want to display "Pending" assets in the validation errors list, @@ -105,6 +107,7 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: # and place them first in the list. pending_assets: models.QuerySet[Asset] = ( self.assets.filter(status__in=[Asset.Status.PENDING, Asset.Status.VALIDATING]) + .exclude(zarr__status=ZarrArchiveStatus.PENDING) .annotate( field=models.Value(''), message=models.Value('asset is currently being validated, please wait.'), @@ -112,6 +115,15 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: .values('field', 'message', 'path')[:50] ) + incomplete_zarr_assets: models.QuerySet[Asset] = ( + self.assets.filter(zarr__status=ZarrArchiveStatus.PENDING) + .annotate( + field=models.Value(''), + message=models.Value('zarr asset is not yet finalized.'), + ) + .values('field', 'message', 'path') + )[:50] + # Next, get all INVALID assets. Each of these should have one or more # validation errors stored in the database. # For performance reasons, we truncate the list of INVALID assets such @@ -129,15 +141,19 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: .values('path', 'validation_errors') )[:50] - return list(pending_assets) + [ - { - 'field': error['field'], - 'message': error['message'], - 'path': asset['path'], - } - for asset in invalid_assets - for error in asset['validation_errors'] - ] + return ( + list(pending_assets) + + list(incomplete_zarr_assets) + + [ + { + 'field': error['field'], + 'message': error['message'], + 'path': asset['path'], + } + for asset in invalid_assets + for error in asset['validation_errors'] + ] + ) @staticmethod def datetime_to_version(time: datetime.datetime) -> str: diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index 4252cbc42..8b2ffe88b 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -10,6 +10,7 @@ from django.conf import settings from freezegun import freeze_time import pytest +from zarr_checksum.checksum import EMPTY_CHECKSUM from dandiapi.api.models.dandiset import Dandiset from dandiapi.api.services.metadata import version_aggregate_assets_summary @@ -20,6 +21,7 @@ PublishedVersionFactory, UserFactory, ) +from dandiapi.zarr.models import ZarrArchiveStatus if TYPE_CHECKING: from rest_framework.test import APIClient @@ -500,7 +502,9 @@ def test_version_rest_info(api_client, version): 'asset_status', [Asset.Status.PENDING, Asset.Status.VALIDATING, Asset.Status.VALID, Asset.Status.INVALID], ) -def test_version_rest_info_with_asset(api_client, draft_asset_factory, asset_status: Asset.Status): +def test_version_asset_validation_errors( + api_client, draft_asset_factory, asset_status: Asset.Status +): version = DraftVersionFactory.create(status=Version.Status.VALID) asset = draft_asset_factory(status=asset_status) version.assets.add(asset) @@ -519,31 +523,50 @@ def test_version_rest_info_with_asset(api_client, draft_asset_factory, asset_sta else [] ) - assert api_client.get( + resp = api_client.get( f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/info/' - ).data == { - 'dandiset': { - 'identifier': version.dandiset.identifier, - 'created': TIMESTAMP_RE, - 'modified': TIMESTAMP_RE, - 'contact_person': version.metadata['contributor'][0]['name'], - 'embargo_status': 'OPEN', - 'star_count': 0, - 'is_starred': False, - }, - 'version': version.version, - 'name': version.name, - 'created': TIMESTAMP_RE, - 'modified': TIMESTAMP_RE, - 'asset_count': 1, - 'active_uploads': 0, - 'metadata': version.metadata, - 'size': version.size, - 'status': Asset.Status.VALID, - 'asset_validation_errors': expected_validation_errors, - 'version_validation_errors': [], - 'contact_person': version.metadata['contributor'][0]['name'], - } + ) + assert resp.json()['asset_validation_errors'] == expected_validation_errors + + +@pytest.mark.django_db +@pytest.mark.parametrize('zarr_status', [c[0] for c in ZarrArchiveStatus.choices]) +def test_version_zarr_asset_validation_errors( + api_client, draft_asset_factory, zarr_archive_factory, zarr_status: ZarrArchiveStatus +): + version = DraftVersionFactory.create(status=Version.Status.VALID) + zarr_archive = zarr_archive_factory( + status=zarr_status, + # Zarr save fails if status is Complete with a null checksum + checksum=EMPTY_CHECKSUM if zarr_status == ZarrArchiveStatus.COMPLETE else None, + ) + asset = draft_asset_factory(status=Asset.Status.PENDING, blob=None, zarr=zarr_archive) + version.assets.add(asset) + add_version_asset_paths(version=version) + + # Create expected validation errors for pending/validating assets + expected_validation_errors = ( + [ + { + 'field': '', + 'message': 'zarr asset is not yet finalized.', + 'path': asset.path, + } + ] + if zarr_status == ZarrArchiveStatus.PENDING + else [ + { + 'field': '', + 'message': 'asset is currently being validated, please wait.', + 'path': asset.path, + } + ] + ) + + resp = api_client.get( + f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/info/' + ) + assert resp.json()['asset_validation_errors'] == expected_validation_errors @pytest.mark.django_db From f01215b34b90a73f56f7955469b238846a795330 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Wed, 25 Feb 2026 19:00:55 -0500 Subject: [PATCH 02/13] Remove AssetStatus.VALIDATING --- .../api/migrations/0031_alter_asset_status.py | 22 +++++++++++++++++++ dandiapi/api/models/asset.py | 1 - dandiapi/api/models/version.py | 2 +- dandiapi/api/tests/test_asset.py | 1 - dandiapi/api/tests/test_version.py | 5 ++--- 5 files changed, 25 insertions(+), 6 deletions(-) create mode 100644 dandiapi/api/migrations/0031_alter_asset_status.py diff --git a/dandiapi/api/migrations/0031_alter_asset_status.py b/dandiapi/api/migrations/0031_alter_asset_status.py new file mode 100644 index 000000000..ab7be1980 --- /dev/null +++ b/dandiapi/api/migrations/0031_alter_asset_status.py @@ -0,0 +1,22 @@ +# Generated by Django 5.2.7 on 2026-02-26 00:00 +from __future__ import annotations + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('api', '0030_alter_asset_path'), + ] + + operations = [ + migrations.AlterField( + model_name='asset', + name='status', + field=models.CharField( + choices=[('Pending', 'Pending'), ('Valid', 'Valid'), ('Invalid', 'Invalid')], + default='Pending', + max_length=10, + ), + ), + ] diff --git a/dandiapi/api/models/asset.py b/dandiapi/api/models/asset.py index d8c2c3646..766fff296 100644 --- a/dandiapi/api/models/asset.py +++ b/dandiapi/api/models/asset.py @@ -45,7 +45,6 @@ class EmbargoedAssetWithinOpenDandisetError(Exception): class AssetStatus(models.TextChoices): PENDING = 'Pending' - VALIDATING = 'Validating' VALID = 'Valid' INVALID = 'Invalid' diff --git a/dandiapi/api/models/version.py b/dandiapi/api/models/version.py index 71885e5f0..bf6eced40 100644 --- a/dandiapi/api/models/version.py +++ b/dandiapi/api/models/version.py @@ -106,7 +106,7 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: # Grab a random sample of 50 pending or currently validating assets # and place them first in the list. pending_assets: models.QuerySet[Asset] = ( - self.assets.filter(status__in=[Asset.Status.PENDING, Asset.Status.VALIDATING]) + self.assets.filter(status=Asset.Status.PENDING) .exclude(zarr__status=ZarrArchiveStatus.PENDING) .annotate( field=models.Value(''), diff --git a/dandiapi/api/tests/test_asset.py b/dandiapi/api/tests/test_asset.py index a23b7565a..c54ca016c 100644 --- a/dandiapi/api/tests/test_asset.py +++ b/dandiapi/api/tests/test_asset.py @@ -649,7 +649,6 @@ def test_asset_rest_info(api_client, version, asset): ('status', 'validation_error'), [ (Asset.Status.PENDING, ''), - (Asset.Status.VALIDATING, ''), (Asset.Status.VALID, ''), (Asset.Status.INVALID, 'error'), ], diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index 8b2ffe88b..b78051c31 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -292,7 +292,6 @@ def test_version_invalid(version, status): 'status', [ Asset.Status.PENDING, - Asset.Status.VALIDATING, Asset.Status.INVALID, ], ) @@ -500,7 +499,7 @@ def test_version_rest_info(api_client, version): @pytest.mark.django_db @pytest.mark.parametrize( 'asset_status', - [Asset.Status.PENDING, Asset.Status.VALIDATING, Asset.Status.VALID, Asset.Status.INVALID], + [Asset.Status.PENDING, Asset.Status.VALID, Asset.Status.INVALID], ) def test_version_asset_validation_errors( api_client, draft_asset_factory, asset_status: Asset.Status @@ -519,7 +518,7 @@ def test_version_asset_validation_errors( 'path': asset.path, } ] - if asset_status in [Asset.Status.PENDING, Asset.Status.VALIDATING] + if asset_status == Asset.Status.PENDING else [] ) From 6a027a363ac97da6d529d72c7e9f04b2d2dd961d Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Thu, 26 Feb 2026 13:52:14 -0500 Subject: [PATCH 03/13] Fix typing of Version.asset_validation_errors --- dandiapi/api/models/version.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dandiapi/api/models/version.py b/dandiapi/api/models/version.py index bf6eced40..aeac071c3 100644 --- a/dandiapi/api/models/version.py +++ b/dandiapi/api/models/version.py @@ -105,7 +105,7 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: # despite them not being stored explicitly as errors in the database. # Grab a random sample of 50 pending or currently validating assets # and place them first in the list. - pending_assets: models.QuerySet[Asset] = ( + pending_assets: models.QuerySet[VersionAssetValidationError] = ( self.assets.filter(status=Asset.Status.PENDING) .exclude(zarr__status=ZarrArchiveStatus.PENDING) .annotate( @@ -115,7 +115,7 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: .values('field', 'message', 'path')[:50] ) - incomplete_zarr_assets: models.QuerySet[Asset] = ( + incomplete_zarr_assets: models.QuerySet[VersionAssetValidationError] = ( self.assets.filter(zarr__status=ZarrArchiveStatus.PENDING) .annotate( field=models.Value(''), @@ -128,7 +128,7 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: # validation errors stored in the database. # For performance reasons, we truncate the list of INVALID assets such # that we only display errors for the 50 assets with the most errors. - invalid_assets: models.QuerySet[Asset] = ( + invalid_assets: models.QuerySet[dict] = ( self.assets.filter(status=Asset.Status.INVALID) .alias( validation_error_count=models.Func( From cd384967f0dbf3d829f92a878b59f9e40e87e930 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Thu, 26 Feb 2026 13:52:48 -0500 Subject: [PATCH 04/13] Update comments in Version.asset_validation_errors --- dandiapi/api/models/version.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/dandiapi/api/models/version.py b/dandiapi/api/models/version.py index aeac071c3..3c05fc3b0 100644 --- a/dandiapi/api/models/version.py +++ b/dandiapi/api/models/version.py @@ -102,9 +102,9 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: from .asset import Asset # We want to display "Pending" assets in the validation errors list, - # despite them not being stored explicitly as errors in the database. - # Grab a random sample of 50 pending or currently validating assets - # and place them first in the list. + # despite them not being stored explicitly as errors in the database. + # We must exclude pending zarr assets, as they are collected in the next step. + # For performance reasons, we only return the first 50 assets. pending_assets: models.QuerySet[VersionAssetValidationError] = ( self.assets.filter(status=Asset.Status.PENDING) .exclude(zarr__status=ZarrArchiveStatus.PENDING) @@ -115,6 +115,9 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: .values('field', 'message', 'path')[:50] ) + # Next, get all zarr assets which have not been finalized. These also are not stored + # explicitly as errors in the database, so we need to construct them manually. + # For performance reasons, we only return the first 50 assets. incomplete_zarr_assets: models.QuerySet[VersionAssetValidationError] = ( self.assets.filter(zarr__status=ZarrArchiveStatus.PENDING) .annotate( @@ -124,10 +127,10 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]: .values('field', 'message', 'path') )[:50] - # Next, get all INVALID assets. Each of these should have one or more - # validation errors stored in the database. + # Finally, get all INVALID assets. Each of these should have one or more + # validation errors stored in the database. # For performance reasons, we truncate the list of INVALID assets such - # that we only display errors for the 50 assets with the most errors. + # that we only display errors for the 50 assets with the most errors. invalid_assets: models.QuerySet[dict] = ( self.assets.filter(status=Asset.Status.INVALID) .alias( From 63ea016f5545da166d1382463887404fac5a8d41 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Thu, 26 Feb 2026 14:11:23 -0500 Subject: [PATCH 05/13] Fix alignment of validation errors and icons --- .../components/DLP/ValidationErrorDialog.vue | 104 +++++++++--------- 1 file changed, 49 insertions(+), 55 deletions(-) diff --git a/web/src/components/DLP/ValidationErrorDialog.vue b/web/src/components/DLP/ValidationErrorDialog.vue index e3130a54d..0671c93f9 100644 --- a/web/src/components/DLP/ValidationErrorDialog.vue +++ b/web/src/components/DLP/ValidationErrorDialog.vue @@ -33,12 +33,10 @@ v-for="(error, index) in versionValidationErrors" :key="index" > - - - - {{ getValidationErrorIcon(error.field) }} - - + + + {{ getValidationErrorIcon(error.field) }} + From d97ade1714a5071a16838c73d1037e53377d2ec8 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Thu, 26 Feb 2026 16:08:01 -0500 Subject: [PATCH 08/13] Separate asset validation errors from version info --- dandiapi/api/models/version.py | 3 +-- dandiapi/api/tests/test_version.py | 10 ++++------ dandiapi/api/views/serializers.py | 2 +- dandiapi/api/views/version.py | 10 ++++++++++ 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/dandiapi/api/models/version.py b/dandiapi/api/models/version.py index 3c05fc3b0..47f1418c5 100644 --- a/dandiapi/api/models/version.py +++ b/dandiapi/api/models/version.py @@ -94,8 +94,7 @@ def publishable(self) -> bool: # Return False if any asset is not VALID return not self.assets.exclude(status=Asset.Status.VALID).exists() - @property - def asset_validation_errors(self) -> list[VersionAssetValidationError]: + def get_asset_validation_errors(self) -> list[VersionAssetValidationError]: # Import here to avoid dependency cycle from dandiapi.zarr.models import ZarrArchiveStatus diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index b78051c31..aecf9f8b3 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -490,7 +490,6 @@ def test_version_rest_info(api_client, version): 'metadata': version.metadata, 'size': version.size, 'status': version.status, - 'asset_validation_errors': [], 'version_validation_errors': [], 'contact_person': version.metadata['contributor'][0]['name'], } @@ -523,9 +522,9 @@ def test_version_asset_validation_errors( ) resp = api_client.get( - f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/info/' + f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/asset_validation_errors/' ) - assert resp.json()['asset_validation_errors'] == expected_validation_errors + assert resp.json() == expected_validation_errors @pytest.mark.django_db @@ -563,9 +562,9 @@ def test_version_zarr_asset_validation_errors( ) resp = api_client.get( - f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/info/' + f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/asset_validation_errors/' ) - assert resp.json()['asset_validation_errors'] == expected_validation_errors + assert resp.json() == expected_validation_errors @pytest.mark.django_db @@ -641,7 +640,6 @@ def test_version_rest_update(api_client): 'metadata': saved_metadata, 'size': draft_version.size, 'status': 'Pending', - 'asset_validation_errors': [], 'version_validation_errors': [], 'contact_person': 'Vargas, Getúlio', } diff --git a/dandiapi/api/views/serializers.py b/dandiapi/api/views/serializers.py index 0132735d1..a3e8f63bf 100644 --- a/dandiapi/api/views/serializers.py +++ b/dandiapi/api/views/serializers.py @@ -373,12 +373,12 @@ class VersionDetailSerializer(VersionSerializer): class Meta(VersionSerializer.Meta): fields = [ *VersionSerializer.Meta.fields, - 'asset_validation_errors', 'version_validation_errors', 'metadata', 'contact_person', ] + # TODO: Rename back to just validation_errors # rename this field in the serializer to differentiate from asset_validation_errors version_validation_errors = serializers.JSONField(source='validation_errors') diff --git a/dandiapi/api/views/version.py b/dandiapi/api/views/version.py index bcd1f9d4d..de9c3684a 100644 --- a/dandiapi/api/views/version.py +++ b/dandiapi/api/views/version.py @@ -86,6 +86,16 @@ def info(self, request, **kwargs): ) return Response(serializer.data, status=status.HTTP_200_OK) + @swagger_auto_schema( + manual_parameters=[DANDISET_PK_PARAM, VERSION_PARAM], + responses={200: 'A list of asset validation errors.'}, + ) + @action(detail=True, methods=['GET']) + def asset_validation_errors(self, request, **kwargs): + """Get the asset validation errors on a version.""" + version: Version = self.get_object() + return Response(version.get_asset_validation_errors(), status=status.HTTP_200_OK) + @swagger_auto_schema( request_body=VersionMetadataSerializer, responses={200: VersionDetailSerializer}, From c8c68eb036ba8fe9492dfe13aa74214b987d9b66 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Thu, 26 Feb 2026 16:09:00 -0500 Subject: [PATCH 09/13] Update client to use new asset validation errors --- web/src/components/DLP/OverviewTab.vue | 3 +- .../components/DLP/ValidationErrorDialog.vue | 32 ++++++------------- web/src/rest.ts | 9 ++++-- web/src/stores/dandiset.ts | 5 +++ web/src/types/index.ts | 2 +- .../DandisetLandingView/DandisetPublish.vue | 19 +++++++---- .../DandisetValidationErrors.vue | 2 +- 7 files changed, 38 insertions(+), 34 deletions(-) diff --git a/web/src/components/DLP/OverviewTab.vue b/web/src/components/DLP/OverviewTab.vue index 3f48f1314..5a4480c08 100644 --- a/web/src/components/DLP/OverviewTab.vue +++ b/web/src/components/DLP/OverviewTab.vue @@ -158,7 +158,7 @@
store.dandiset); +const hasAssetValidationErrors = computed(() => Boolean(currentDandiset.value?.asset_validation_errors?.length)); const contributors = computed( () => props.meta.contributor?.filter( diff --git a/web/src/components/DLP/ValidationErrorDialog.vue b/web/src/components/DLP/ValidationErrorDialog.vue index 6ece968e8..d07354a53 100644 --- a/web/src/components/DLP/ValidationErrorDialog.vue +++ b/web/src/components/DLP/ValidationErrorDialog.vue @@ -118,31 +118,17 @@