Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/cli-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ jobs:
- name: Run dandi-api tests in dandi-cli
if: matrix.dandi-version == 'master'
run: >
uvx --with "dandi[test] @ git+https://github.com/dandi/dandi-cli"
uvx --with "dandi[test] @ git+https://github.com/dandi/dandi-cli@use-new-asset-validation-errors"
pytest --pyargs -v -s --dandi-api dandi
env:
DANDI_TESTS_PERSIST_DOCKER_COMPOSE: "1"
Expand Down
22 changes: 22 additions & 0 deletions dandiapi/api/migrations/0031_alter_asset_status.py
Original file line number Diff line number Diff line change
@@ -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,
),
),
]
1 change: 0 additions & 1 deletion dandiapi/api/models/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ class EmbargoedAssetWithinOpenDandisetError(Exception):

class AssetStatus(models.TextChoices):
PENDING = 'Pending'
VALIDATING = 'Validating'
VALID = 'Valid'
INVALID = 'Invalid'

Expand Down
62 changes: 40 additions & 22 deletions dandiapi/api/models/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,29 +94,43 @@ 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

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.
pending_assets: models.QuerySet[Asset] = (
self.assets.filter(status__in=[Asset.Status.PENDING, Asset.Status.VALIDATING])
# 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)
.annotate(
field=models.Value(''),
message=models.Value('asset is currently being validated, please wait.'),
)
.values('field', 'message', 'path')[:50]
.values('field', 'message', 'path')
)

# 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(
field=models.Value(''),
message=models.Value('zarr asset is not yet finalized.'),
)
.values('field', 'message', 'path')
)

# 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.
invalid_assets: models.QuerySet[Asset] = (
# 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(
validation_error_count=models.Func(
Expand All @@ -127,17 +141,21 @@ def asset_validation_errors(self) -> list[VersionAssetValidationError]:
)
.order_by('-validation_error_count')
.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:
Expand Down
1 change: 0 additions & 1 deletion dandiapi/api/tests/test_asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
],
Expand Down
86 changes: 53 additions & 33 deletions dandiapi/api/tests/test_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,6 +21,7 @@
PublishedVersionFactory,
UserFactory,
)
from dandiapi.zarr.models import ZarrArchiveStatus

if TYPE_CHECKING:
from rest_framework.test import APIClient
Expand Down Expand Up @@ -290,7 +292,6 @@ def test_version_invalid(version, status):
'status',
[
Asset.Status.PENDING,
Asset.Status.VALIDATING,
Asset.Status.INVALID,
],
)
Expand Down Expand Up @@ -489,18 +490,19 @@ def test_version_rest_info(api_client, version):
'metadata': version.metadata,
'size': version.size,
'status': version.status,
'asset_validation_errors': [],
'version_validation_errors': [],
'validation_errors': [],
'contact_person': version.metadata['contributor'][0]['name'],
}


@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_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)
Expand All @@ -515,35 +517,54 @@ def test_version_rest_info_with_asset(api_client, draft_asset_factory, asset_sta
'path': asset.path,
}
]
if asset_status in [Asset.Status.PENDING, Asset.Status.VALIDATING]
if asset_status == Asset.Status.PENDING
else []
)

assert 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'],
}
resp = api_client.get(
f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/asset_validation_errors/'
)
assert resp.json() == 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}/asset_validation_errors/'
)
assert resp.json() == expected_validation_errors


@pytest.mark.django_db
Expand Down Expand Up @@ -619,8 +640,7 @@ def test_version_rest_update(api_client):
'metadata': saved_metadata,
'size': draft_version.size,
'status': 'Pending',
'asset_validation_errors': [],
'version_validation_errors': [],
'validation_errors': [],
'contact_person': 'Vargas, Getúlio',
}

Expand Down
6 changes: 1 addition & 5 deletions dandiapi/api/views/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,15 +373,11 @@ class VersionDetailSerializer(VersionSerializer):
class Meta(VersionSerializer.Meta):
fields = [
*VersionSerializer.Meta.fields,
'asset_validation_errors',
'version_validation_errors',
'validation_errors',
'metadata',
'contact_person',
]

# rename this field in the serializer to differentiate from asset_validation_errors
version_validation_errors = serializers.JSONField(source='validation_errors')

def get_contact_person(self, obj):
return extract_contact_person(obj)

Expand Down
10 changes: 10 additions & 0 deletions dandiapi/api/views/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
3 changes: 2 additions & 1 deletion web/src/components/DLP/OverviewTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@
<div
v-else-if="!assetSummary || !Object.keys(assetSummary).length"
class="font-italic font-weight-bold"
v-text="`This Dandiset does not contain any valid assets.${currentDandiset?.asset_validation_errors.length ? ' Please check the asset validation errors on the right panel.' : ''}`"
v-text="`This Dandiset does not contain any valid assets.${hasAssetValidationErrors ? ' Please check the asset validation errors on the right panel.' : ''}`"
/>
<div
v-for="([type, items], i) in Object.entries(assetSummary)"
Expand Down Expand Up @@ -288,6 +288,7 @@ const display = useDisplay();

const store = useDandisetStore();
const currentDandiset = computed(() => store.dandiset);
const hasAssetValidationErrors = computed(() => Boolean(currentDandiset.value?.asset_validation_errors?.length));

const contributors = computed(
() => props.meta.contributor?.filter(
Expand Down
Loading
Loading