From 226cec88bed38aedd36beb88eb5f686b9b18b525 Mon Sep 17 00:00:00 2001 From: Stefano Ricci <1219739+SteRiccio@users.noreply.github.com> Date: Wed, 25 Oct 2023 08:34:47 +0200 Subject: [PATCH] Fixed error importing data in records with hierarchical key attributes; warn if multiple records matching same keys are found; (#3111) * fixed data import error when record has hierarchical key attributes * data import: check multiple records matching keys * job errors: do not sort rows --------- Co-authored-by: Stefano Ricci --- core/i18n/resources/en.js | 4 ++- core/record/_record/recordNodesUpdater.js | 4 +-- core/record/nodeValues.js | 27 ++++++++++++------- .../_validator/validatorErrorKeys.js | 1 + .../service/DataImportJob/recordProvider.js | 18 ++++++++----- .../App/JobMonitor/JobErrors/JobErrors.js | 5 ---- 6 files changed, 36 insertions(+), 23 deletions(-) diff --git a/core/i18n/resources/en.js b/core/i18n/resources/en.js index 5ff1e9ef81..6eff1d8375 100644 --- a/core/i18n/resources/en.js +++ b/core/i18n/resources/en.js @@ -1477,6 +1477,7 @@ Levels will be renamed into level_1, level_2... level_N and an extra 'area' prop invalidTaxonCode: 'Invalid code in column {{headers}}: {{value}}', missingRequiredHeaders: 'Missing required columns: {{missingRequiredHeaders}}', errorUpdatingValues: 'Error updating values', + multipleRecordsMatchingKeys: 'Multiple records found matching keys "{{keyValues}}"', recordAlreadyExisting: 'Record with keys "{{keyValues}}" already existing', recordInAnalysisStepCannotBeUpdated: 'Record with keys "{{keyValues}}" is in Analysis step and cannot be updated', recordKeyMissing: 'Missing value for key attribute "{{keyName}}"', @@ -1525,6 +1526,8 @@ Levels will be renamed into level_1, level_2... level_N and an extra 'area' prop record: { keyDuplicate: 'Duplicate record key', entityKeyDuplicate: 'Duplicate key', + entityKeyValueNotSpecified: 'Entity key value for entity "{{entityName}}" not specified', + missingAncestorForEntity: 'Cannot find ancestor "{{ancestorName}}" for entity "{{entityName}}"', uniqueAttributeDuplicate: 'Duplicate value', valueInvalid: 'Invalid value', valueRequired: 'Required value', @@ -1736,7 +1739,6 @@ Try to refresh the page.`, }, entity: { keyDuplicate: 'Duplicate entity key', - keyValueNotSpecified: 'Key value for attribute {{keyDefName}} not specified', }, nodes: { count: { diff --git a/core/record/_record/recordNodesUpdater.js b/core/record/_record/recordNodesUpdater.js index 106c9c9162..507c82c001 100644 --- a/core/record/_record/recordNodesUpdater.js +++ b/core/record/_record/recordNodesUpdater.js @@ -127,7 +127,7 @@ const _addEntityAndKeyValues = const keyValue = keyValuesByDefUuid[NodeDef.getUuid(keyDef)] if (Objects.isEmpty(keyValue)) { - throw new SystemError('record.entity.keyValueNotSpecified', { + throw new SystemError('validationErrors.record.entityKeyValueNotSpecified', { keyDefName: NodeDef.getName(keyDef), }) } @@ -193,7 +193,7 @@ const _getOrCreateEntityByKeys = })(record) if (!entityParent) { - throw new SystemError('record.cannotFindAncestorForEntity', { + throw new SystemError('validationErrors.record.missingAncestorForEntity', { entityName: NodeDef.getName(entityDef), ancestorName: NodeDef.getName(entityParentDef), }) diff --git a/core/record/nodeValues.js b/core/record/nodeValues.js index 7d1b6c6f4f..537f040f66 100644 --- a/core/record/nodeValues.js +++ b/core/record/nodeValues.js @@ -1,6 +1,6 @@ import * as R from 'ramda' -import { DateFormats, Dates, Objects } from '@openforis/arena-core' +import { CategoryItems, DateFormats, Dates, Objects } from '@openforis/arena-core' import * as StringUtils from '@core/stringUtils' import * as Survey from '@core/survey/survey' @@ -13,16 +13,11 @@ const singlePropValueEqualComparator = ({ value, valueSearch }) => const getValueCode = (value) => (StringUtils.isString(value) ? value : value[Node.valuePropsCode.code]) const getValueItemUuid = (value) => value[Node.valuePropsCode.itemUuid] -const extractCategoryItemUuidFromValue = ({ survey, nodeDef, record, parentNode, value, strict }) => { +const extractCategoryItemUuidFromValue = ({ survey, nodeDef, record, parentNode, value }) => { const itemUuid = getValueItemUuid(value) if (itemUuid) { return itemUuid } - if (strict) { - // strict comparison: if itemUuid is not defined, do not convert value "code" into itemUuid - return null - } - // find itemUuid by code const code = getValueCode(value) if (!Objects.isEmpty(code)) { @@ -37,6 +32,15 @@ const extractCategoryItemUuidFromValue = ({ survey, nodeDef, record, parentNode, return null } +const extractCategoryItemCodeFromValue = ({ survey, value }) => { + const itemUuid = getValueItemUuid(value) + if (itemUuid) { + const item = Survey.getCategoryItemByUuid(itemUuid)(survey) + return CategoryItems.getCode(item) + } + return getValueCode(value) +} + const dateTimeComparator = ({ formatsSource, formatTo }) => ({ value, valueSearch }) => { @@ -55,14 +59,19 @@ const dateTimeComparator = const valueComparatorByNodeDefType = { [NodeDef.nodeDefType.boolean]: singlePropValueEqualComparator, [NodeDef.nodeDefType.code]: ({ survey, nodeDef, record, parentNode, value, valueSearch, strict }) => { - const itemUuid = extractCategoryItemUuidFromValue({ survey, nodeDef, record, parentNode, value, strict }) + if (!strict || !record) { + // compare just codes (record not available, tricky to find the "correct" category item without knowing its parent item) + const code = extractCategoryItemCodeFromValue({ survey, value }) + const codeSearch = extractCategoryItemCodeFromValue({ survey, value: valueSearch }) + return code && codeSearch && code === codeSearch + } + const itemUuid = extractCategoryItemUuidFromValue({ survey, nodeDef, record, parentNode, value }) const itemUuidSearch = extractCategoryItemUuidFromValue({ survey, nodeDef, record, parentNode, value: valueSearch, - strict, }) return itemUuidSearch && itemUuid && itemUuidSearch === itemUuid }, diff --git a/core/validation/_validator/validatorErrorKeys.js b/core/validation/_validator/validatorErrorKeys.js index c77141350c..bfd761c7ea 100644 --- a/core/validation/_validator/validatorErrorKeys.js +++ b/core/validation/_validator/validatorErrorKeys.js @@ -50,6 +50,7 @@ export const ValidatorErrorKeys = { recordInAnalysisStepCannotBeUpdated: 'validationErrors.dataImport.recordInAnalysisStepCannotBeUpdated', emptyFile: 'validationErrors.dataImport.emptyFile', errorUpdatingValues: 'validationErrors.dataImport.errorUpdatingValues', + multipleRecordsMatchingKeys: 'validationErrors.dataImport.multipleRecordsMatchingKeys', recordAlreadyExisting: 'validationErrors.dataImport.recordAlreadyExisting', recordKeyMissing: 'validationErrors.dataImport.recordKeyMissing', recordNotFound: 'validationErrors.dataImport.recordNotFound', diff --git a/server/modules/dataImport/service/DataImportJob/recordProvider.js b/server/modules/dataImport/service/DataImportJob/recordProvider.js index f647bcef65..2cd9680468 100644 --- a/server/modules/dataImport/service/DataImportJob/recordProvider.js +++ b/server/modules/dataImport/service/DataImportJob/recordProvider.js @@ -35,7 +35,7 @@ const fetchOrCreateRecord = async ({ valuesByDefUuid, currentRecord, context, tx checkRootKeysSpecified({ rootKeyDefs, rootKeyValuesFormatted }) - const recordSummary = recordsSummary.find((record) => + const recordSummariesMatchingKeys = recordsSummary.filter((record) => rootKeyDefs.every((rootKeyDef) => { const keyValueInRecord = record[A.camelize(NodeDef.getName(rootKeyDef))] const keyValueInRow = valuesByDefUuid[NodeDef.getUuid(rootKeyDef)] @@ -49,7 +49,7 @@ const fetchOrCreateRecord = async ({ valuesByDefUuid, currentRecord, context, tx }) ) - const keyNameValuesPairs = rootKeyDefs + const keyNameValuePairs = rootKeyDefs .map((keyDef, index) => { const name = NodeDef.getName(keyDef) const value = rootKeyValuesFormatted[index] @@ -59,8 +59,8 @@ const fetchOrCreateRecord = async ({ valuesByDefUuid, currentRecord, context, tx if (insertNewRecords) { // check if record with the same key values already exists - if (recordSummary) { - throw new SystemError(Validation.messageKeys.dataImport.recordAlreadyExisting, { keyValues: keyNameValuesPairs }) + if (recordSummariesMatchingKeys.length === 1) { + throw new SystemError(Validation.messageKeys.dataImport.recordAlreadyExisting, { keyValues: keyNameValuePairs }) } const recordToInsert = Record.newRecord(user, cycle) let record = null @@ -84,13 +84,19 @@ const fetchOrCreateRecord = async ({ valuesByDefUuid, currentRecord, context, tx // insertNewRecords === false : updating existing record + if (recordSummariesMatchingKeys.length > 1) { + throw new SystemError(Validation.messageKeys.dataImport.multipleRecordsMatchingKeys, { + keyValues: keyNameValuePairs, + }) + } + const recordSummary = recordSummariesMatchingKeys[0] if (!recordSummary) { - throw new SystemError(Validation.messageKeys.dataImport.recordNotFound, { keyValues: keyNameValuesPairs }) + throw new SystemError(Validation.messageKeys.dataImport.recordNotFound, { keyValues: keyNameValuePairs }) } if (!updateRecordsInAnalysis && Record.isInAnalysisStep(recordSummary)) { throw new SystemError(Validation.messageKeys.dataImport.recordInAnalysisStepCannotBeUpdated, { - keyValues: keyNameValuesPairs, + keyValues: keyNameValuePairs, }) } diff --git a/webapp/views/App/JobMonitor/JobErrors/JobErrors.js b/webapp/views/App/JobMonitor/JobErrors/JobErrors.js index 67379b7a6a..de6f0ebaf6 100644 --- a/webapp/views/App/JobMonitor/JobErrors/JobErrors.js +++ b/webapp/views/App/JobMonitor/JobErrors/JobErrors.js @@ -57,11 +57,6 @@ const JobErrors = ({ errorKeyHeaderName, exportFileName: exportFileNameProp, job ]} density="compact" exportFileName={exportFileName} - initialState={{ - sorting: { - sortModel: [{ field: 'errorKey', sort: 'asc' }], - }, - }} rows={Object.entries(errors).map(([errorKey, error]) => ({ errorKey, error,