diff --git a/dandiapi/api/models/version.py b/dandiapi/api/models/version.py index c3ce3e17e..a0d7feed7 100644 --- a/dandiapi/api/models/version.py +++ b/dandiapi/api/models/version.py @@ -81,7 +81,7 @@ def valid(self) -> bool: return not self.assets.exclude(status=Asset.Status.VALID).exists() @property - def asset_validation_errors(self) -> list[str]: + def asset_validation_errors(self) -> list[dict[str, str]]: # Import here to avoid dependency cycle from .asset import Asset @@ -90,18 +90,25 @@ def asset_validation_errors(self) -> list[str]: status=Asset.Status.VALID ).values('status', 'path', 'validation_errors') - # Aggregate errors + asset_validating_error = { + 'field': '', + 'message': 'asset is currently being validated, please wait.', + } + + def inject_path(asset: dict, err: dict): + return {**err, 'path': asset['path']} + + # Aggregate errors, ensuring the path of the asset is included errors = [] for asset in invalid_assets: - if asset['status'] == Asset.Status.INVALID: - errors.extend(asset['validation_errors']) - else: - errors.append( - { - 'field': asset['path'], - 'message': 'asset is currently being validated, please wait.', - } - ) + # Must be pending or validating + if asset['status'] != Asset.Status.INVALID: + errors.append(inject_path(asset, asset_validating_error)) + continue + + # Must be invalid, only add entry in map if it has any errors + if asset['validation_errors']: + errors.extend([inject_path(asset, err) for err in asset['validation_errors']]) return errors diff --git a/dandiapi/api/tests/test_version.py b/dandiapi/api/tests/test_version.py index eb33ce59c..ae9994066 100644 --- a/dandiapi/api/tests/test_version.py +++ b/dandiapi/api/tests/test_version.py @@ -422,35 +422,29 @@ def test_version_rest_info(api_client, version): @pytest.mark.django_db @pytest.mark.parametrize( - 'asset_status,expected_validation_error', - [ - ( - Asset.Status.PENDING, - [{'field': '', 'message': 'asset is currently being validated, please wait.'}], - ), - ( - Asset.Status.VALIDATING, - [{'field': '', 'message': 'asset is currently being validated, please wait.'}], - ), - (Asset.Status.VALID, []), - (Asset.Status.INVALID, []), - ], + 'asset_status', + [Asset.Status.PENDING, Asset.Status.VALIDATING, Asset.Status.VALID, Asset.Status.INVALID], ) def test_version_rest_info_with_asset( - api_client, - draft_version_factory, - draft_asset_factory, - asset_status: Asset.Status, - expected_validation_error: str, + api_client, draft_version_factory, draft_asset_factory, asset_status: Asset.Status ): version = draft_version_factory(status=Version.Status.VALID) asset = draft_asset_factory(status=asset_status) version.assets.add(asset) add_version_asset_paths(version=version) - # These validation error types should have the asset path prepended to them: - if asset_status == Asset.Status.PENDING or asset_status == Asset.Status.VALIDATING: - expected_validation_error[0]['field'] = asset.path + # Create expected validation errors for pending/validating assets + expected_validation_errors = ( + [ + { + 'field': '', + 'message': 'asset is currently being validated, please wait.', + 'path': asset.path, + } + ] + if asset_status in [Asset.Status.PENDING, Asset.Status.VALIDATING] + else [] + ) assert api_client.get( f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/info/' @@ -470,7 +464,7 @@ def test_version_rest_info_with_asset( 'metadata': version.metadata, 'size': version.size, 'status': Asset.Status.VALID, - 'asset_validation_errors': expected_validation_error, + 'asset_validation_errors': expected_validation_errors, 'version_validation_errors': [], 'contact_person': version.metadata['contributor'][0]['name'], } diff --git a/web/src/components/DLP/ValidationErrorDialog.vue b/web/src/components/DLP/ValidationErrorDialog.vue new file mode 100644 index 000000000..258e98dec --- /dev/null +++ b/web/src/components/DLP/ValidationErrorDialog.vue @@ -0,0 +1,199 @@ + + + + + Metadata + + + Assets + + + + + + + Fix issues + + + + + + + {{ getValidationErrorIcon(error.field) }} + + + + + + {{ error.field }}: + + {{ error.message }} + + + + + + + + + + + + + + + + + mdi-alert-plus + + + {{ getValidationErrorIcon(errors[0].field) }} + + + + + + + + {{ path }} + + {{ errors[0].field }} - + + {{ errors[0].message }} + + + + + + + + + {{ path }} + Click to expand + + + + + + + {{ getValidationErrorIcon(error.field) }} + + + + + {{ error.field }}: + + {{ error.message }} + + + + + + + + + + + + + + + diff --git a/web/src/types/index.ts b/web/src/types/index.ts index 550ea4d08..6be6486c4 100644 --- a/web/src/types/index.ts +++ b/web/src/types/index.ts @@ -30,6 +30,7 @@ export interface DandisetSearchResult extends Dandiset { export interface ValidationError { field: string, message: string, + path: string, } export interface Version { diff --git a/web/src/views/DandisetLandingView/DandisetPublish.vue b/web/src/views/DandisetLandingView/DandisetPublish.vue index 0caf78b6b..0198a1e36 100644 --- a/web/src/views/DandisetLandingView/DandisetPublish.vue +++ b/web/src/views/DandisetLandingView/DandisetPublish.vue @@ -200,167 +200,78 @@ - + + + + + + - - - - - - - - - mdi-playlist-remove - - - - - - This Dandiset has {{ currentDandiset.version_validation_errors.length }} - metadata validation error(s). - - - - - - Fix issues with metadata - - - - + + - - - - - {{ getValidationErrorIcon(error.field) }} - - - - - - {{ error.field }}: - - {{ error.message }} - - - - - - + + + + - Fix issues - - - - - - + + + + + + - - - - - - - - - mdi-database-remove - - - - - - This Dandiset has {{ currentDandiset.asset_validation_errors.length }} - asset validation error(s). - - - - - - Fix issues with assets - - - - + + - - - - - {{ getValidationErrorIcon(error.field) }} - - - - - - {{ error.field }}: - - {{ error.message }} - - - - - - - - + mdi-database-remove + + + + + + This Dandiset has {{ numAssetValidationErrors }} + asset validation error(s). + + + + + This Version @@ -448,8 +359,10 @@ import { useDandisetStore } from '@/stores/dandiset'; import router from '@/router'; import type { User, Version } from '@/types'; -import { draftVersion, VALIDATION_ICONS } from '@/utils/constants'; -import { open as openMeditor } from '@/components/Meditor/state'; +import { draftVersion } from '@/utils/constants'; +import { open as meditorOpen } from '@/components/Meditor/state'; + +import ValidationErrorDialog from '@/components/DLP/ValidationErrorDialog.vue'; const PUBLISH_CHECKLIST = [ 'A descriptive title (e.g., Data related to foraging behavior in bees rather than Smith et al 2022)', @@ -460,14 +373,6 @@ const PUBLISH_CHECKLIST = [ 'References to code in GitHub, related publications, etc.', ]; -function getValidationErrorIcon(errorField: string): string { - const icons = Object.keys(VALIDATION_ICONS).filter((field) => errorField.includes(field)); - if (icons.length > 0) { - return (VALIDATION_ICONS as any)[icons[0]]; - } - return VALIDATION_ICONS.DEFAULT; -} - // Sort versions from most recently modified to least recently modified. // The DRAFT version is always the first element when present. function sortVersions(v1: Version, v2: Version): number { @@ -586,9 +491,30 @@ onUnmounted(() => { window.clearInterval(timer); }); +// Error dialog +const errorDialogOpen = ref(false); +type ErrorCategory = 'metadata' | 'assets'; +const selectedTab = ref('metadata'); +function openErrorDialog(tab: ErrorCategory) { + errorDialogOpen.value = true; + selectedTab.value = tab; +} + +function openMeditor() { + errorDialogOpen.value = false; + meditorOpen.value = true; +} + +const numAssetValidationErrors = computed(() => { + if (currentDandiset.value === null) { + return 0; + } + + return currentDandiset.value.asset_validation_errors.length; +}); const publishButtonDisabled = computed(() => !!( currentDandiset.value?.version_validation_errors.length - || currentDandiset.value?.asset_validation_errors.length + || numAssetValidationErrors.value || currentDandiset.value?.dandiset.embargo_status !== 'OPEN' || publishDisabledMessage.value ));