Skip to content

Commit

Permalink
Use flat listing for asset validation errors
Browse files Browse the repository at this point in the history
  • Loading branch information
jjnesbitt committed May 24, 2023
1 parent d1d4d00 commit 66a073b
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 21 deletions.
13 changes: 8 additions & 5 deletions dandiapi/api/models/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def valid(self) -> bool:
return not self.assets.exclude(status=Asset.Status.VALID).exists()

@property
def asset_validation_errors(self) -> dict[str, list[dict[str, str]]]:
def asset_validation_errors(self) -> list[dict[str, str]]:
# Import here to avoid dependency cycle
from .asset import Asset

Expand All @@ -95,19 +95,22 @@ def asset_validation_errors(self) -> dict[str, list[dict[str, str]]]:
'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
asset_error_map = {}
errors = []
for asset in invalid_assets:
# Must be pending or validating
if asset['status'] != Asset.Status.INVALID:
asset_error_map[asset['path']] = [asset_validating_error]
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']:
asset_error_map[asset['path']] = asset['validation_errors']
errors.extend([inject_path(asset, err) for err in asset['validation_errors']])

return asset_error_map
return errors

@staticmethod
def datetime_to_version(time: datetime.datetime) -> str:
Expand Down
22 changes: 12 additions & 10 deletions dandiapi/api/tests/test_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ def test_version_rest_info(api_client, version):
'metadata': version.metadata,
'size': version.size,
'status': 'Pending',
'asset_validation_errors': {},
'asset_validation_errors': [],
'version_validation_errors': [],
'contact_person': version.metadata['contributor'][0]['name'],
}
Expand All @@ -434,14 +434,16 @@ def test_version_rest_info_with_asset(
add_version_asset_paths(version=version)

# Create expected validation errors for pending/validating assets
asset_validating_error = {
'field': '',
'message': 'asset is currently being validated, please wait.',
}
expected_validation_error = (
{asset.path: [asset_validating_error]}
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 {}
else []
)

assert api_client.get(
Expand All @@ -462,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'],
}
Expand Down Expand Up @@ -537,7 +539,7 @@ def test_version_rest_update(api_client, user, draft_version):
'metadata': saved_metadata,
'size': draft_version.size,
'status': 'Pending',
'asset_validation_errors': {},
'asset_validation_errors': [],
'version_validation_errors': [],
'contact_person': 'Vargas, Getúlio',
}
Expand Down
15 changes: 13 additions & 2 deletions web/src/components/DLP/ValidationErrorDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
class="overflow-y-auto"
>
<v-expansion-panels multiple>
<template v-for="(errors, path) in assetValidationErrors">
<template v-for="(errors, path) in groupedAssetValidationErrors">
<v-list-item
:key="path"
>
Expand Down Expand Up @@ -144,7 +144,7 @@ import { VALIDATION_ICONS } from '@/utils/constants';
const props = defineProps({
assetValidationErrors: {
type: Object as PropType<Record<string, ValidationError[]>>,
type: Array as PropType<ValidationError[]>,
required: true,
},
versionValidationErrors: {
Expand All @@ -170,6 +170,17 @@ watch(() => props.selectedTab, (val) => {
const showMetadataTab = computed(() => !!props.versionValidationErrors.length);
const showAssetsTab = computed(() => !!Object.keys(props.assetValidationErrors));
const groupedAssetValidationErrors = computed(() => {
const path_asset_map: Record<string, ValidationError[]> = {};
props.assetValidationErrors.forEach((err) => {
if (!(err.path in path_asset_map)) {
path_asset_map[err.path] = [];
}
path_asset_map[err.path].push(err);
});
return path_asset_map;
});
function getValidationErrorIcon(errorField: string): string {
const icons = Object.keys(VALIDATION_ICONS).filter((field) => errorField.includes(field));
Expand Down
3 changes: 2 additions & 1 deletion web/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface Dandiset {
export interface ValidationError {
field: string,
message: string,
path: string,
}

export interface Version {
Expand All @@ -39,7 +40,7 @@ export interface Version {
modified: string,
dandiset: Dandiset,
metadata?: DandisetMetadata,
asset_validation_errors: Record<string, ValidationError[]>,
asset_validation_errors: ValidationError[],
version_validation_errors: ValidationError[],
contact_person?: string,
}
Expand Down
4 changes: 1 addition & 3 deletions web/src/views/DandisetLandingView/DandisetPublish.vue
Original file line number Diff line number Diff line change
Expand Up @@ -506,9 +506,7 @@ const numAssetValidationErrors = computed(() => {
return 0;
}

return Object.values(
currentDandiset.value.asset_validation_errors,
).reduce((sum, errs) => sum + errs.length, 0);
return currentDandiset.value.asset_validation_errors.length;
});
const publishButtonDisabled = computed(() => !!(
currentDandiset.value?.version_validation_errors.length
Expand Down

0 comments on commit 66a073b

Please sign in to comment.