From 2cb6ac958ea01c57ab1252918eed657de41cbdba Mon Sep 17 00:00:00 2001 From: Stefano Ricci <1219739+SteRiccio@users.noreply.github.com> Date: Tue, 10 Dec 2024 12:38:25 +0100 Subject: [PATCH] fixed category validator, validation of sibling items (#3690) Co-authored-by: Stefano Ricci Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- core/objectUtils.js | 26 +++++++++++-------- server/modules/category/categoryValidator.js | 26 +++++++++++-------- .../category/manager/categoryManager.js | 11 ++++---- .../category/repository/categoryRepository.js | 24 +++++++++++------ 4 files changed, 52 insertions(+), 35 deletions(-) diff --git a/core/objectUtils.js b/core/objectUtils.js index da5740dc80..ebd27ae8e8 100644 --- a/core/objectUtils.js +++ b/core/objectUtils.js @@ -130,17 +130,21 @@ export const toIndexedObj = (array, propNameOrExtractor) => export const toUuidIndexedObj = R.partialRight(toIndexedObj, [keys.uuid]) -export const groupByProp = (propNameOrExtractor) => (items) => - items.reduce((acc, item) => { - const prop = _getProp(propNameOrExtractor)(item) - let itemsByProp = acc[prop] - if (!itemsByProp) { - itemsByProp = [] - acc[prop] = itemsByProp - } - itemsByProp.push(item) - return acc - }, {}) +export const groupByProps = + (...propNamesOrExtractors) => + (items) => + items.reduce((acc, item) => { + const props = propNamesOrExtractors.map((propNameOrExtractor) => _getProp(propNameOrExtractor)(item)) + let itemsPartial = R.path(props)(acc) + if (!itemsPartial) { + itemsPartial = [] + } + itemsPartial.push(item) + setInPath(props, itemsPartial)(acc) + return acc + }, {}) + +export const groupByProp = groupByProps export const clone = (obj) => (R.isNil(obj) ? obj : JSON.parse(JSON.stringify(obj))) diff --git a/server/modules/category/categoryValidator.js b/server/modules/category/categoryValidator.js index 2d78a453d2..046558ae23 100644 --- a/server/modules/category/categoryValidator.js +++ b/server/modules/category/categoryValidator.js @@ -56,8 +56,9 @@ const validateLevels = async ({ category, itemsCache, bigCategory }) => { // ====== ITEMS -const validateItemCodeUniqueness = (siblingsAndSelfByCode) => (_propName, item) => { - const isUnique = siblingsAndSelfByCode[CategoryItem.getCode(item)]?.length === 1 +const validateItemCodeUniqueness = (itemsByParentAndCode) => (_propName, item) => { + const siblingItems = R.path([CategoryItem.getParentUuid(item), CategoryItem.getCode(item)])(itemsByParentAndCode) + const isUnique = siblingItems?.length === 1 return isUnique ? null : { key: Validation.messageKeys.categoryEdit.codeDuplicate } } @@ -68,11 +69,11 @@ const validateNotEmptyChildrenItems = ? { key: Validation.messageKeys.categoryEdit.childrenEmpty, severity: ValidationResult.severity.warning } : null -const itemValidators = ({ isLeaf, siblingsAndSelfByCode, childrenCount = 0 }) => ({ +const itemValidators = ({ isLeaf, itemsByParentAndCode, childrenCount = 0 }) => ({ [`${CategoryItem.keys.props}.${CategoryItem.keysProps.code}`]: [ Validator.validateRequired(Validation.messageKeys.categoryEdit.codeRequired), Validator.validateNotKeyword(Validation.messageKeys.categoryEdit.codeCannotBeKeyword), - validateItemCodeUniqueness(siblingsAndSelfByCode), + validateItemCodeUniqueness(itemsByParentAndCode), ], [keys.children]: [validateNotEmptyChildrenItems({ isLeaf, childrenCount })], }) @@ -182,15 +183,15 @@ const validateItemsAndDescendants = async ({ const pushItems = (items) => { // Group sibling items by code to optimize item code uniqueness check // do it only one time for every sibling - const siblingsAndSelfByCode = ObjectUtils.groupByProp(CategoryItem.getCode)(items) + const itemsByParentAndCode = ObjectUtils.groupByProps(CategoryItem.getParentUuid, CategoryItem.getCode)(items) items.forEach((item) => { - item.siblingsAndSelfByCode = siblingsAndSelfByCode + item.itemsByParentAndCode = itemsByParentAndCode stack.push(item) }) } const popItem = (item) => { - delete item['siblingsAndSelfByCode'] + delete item['itemsByParentAndCode'] stack.pop() processed++ onProgress?.({ total, processed }) @@ -200,7 +201,7 @@ const validateItemsAndDescendants = async ({ while (!R.isEmpty(stack) && !stopIfFn?.()) { const item = stack[stack.length - 1] // Do not pop item: it can be visited again - const { siblingsAndSelfByCode } = item + const { itemsByParentAndCode } = item const itemUuid = CategoryItem.getUuid(item) const isLeaf = Category.isItemLeaf(item)(category) const itemChildren = itemsCache.getItemChildren(itemUuid) @@ -214,7 +215,7 @@ const validateItemsAndDescendants = async ({ /* eslint-disable no-await-in-loop */ validation = await Validator.validate( item, - itemValidators({ isLeaf, siblingsAndSelfByCode, childrenCount }), + itemValidators({ isLeaf, itemsByParentAndCode, childrenCount }), false ) validation = _validateItemExtraProps({ extraDefs, validation, srsIndex })(item) @@ -294,7 +295,10 @@ export const validateCategory = async ({ export const validateItems = async ({ category, itemsToValidate, itemsCountByItemUuid }) => { const prevValidation = Category.getValidation(category) - const siblingsAndSelfByCode = ObjectUtils.groupByProp(CategoryItem.getCode)(itemsToValidate) + const itemsByParentAndCode = ObjectUtils.groupByProps( + CategoryItem.getParentUuid, + CategoryItem.getCode + )(itemsToValidate) const prevItemsValidation = Validation.getFieldValidation(keys.items)(prevValidation) let itemsValidationUpdated = prevItemsValidation @@ -305,7 +309,7 @@ export const validateItems = async ({ category, itemsToValidate, itemsCountByIte const itemValidation = await Validator.validate( item, - itemValidators({ isLeaf, siblingsAndSelfByCode, childrenCount }) + itemValidators({ isLeaf, itemsByParentAndCode, childrenCount }) ) itemsValidationUpdated = Validation.assocFieldValidation(itemUuid, itemValidation)(itemsValidationUpdated) } diff --git a/server/modules/category/manager/categoryManager.js b/server/modules/category/manager/categoryManager.js index db8d10ee89..db1a9329c8 100644 --- a/server/modules/category/manager/categoryManager.js +++ b/server/modules/category/manager/categoryManager.js @@ -133,11 +133,12 @@ const _afterItemUpdate = async ({ surveyId, categoryUuid, itemUuid, prevItem }, } const itemsToValidate = [] for await (const code of codes) { - const itemsCount = await CategoryRepository.countItemsByLevelAndCode({ surveyId, levelUuid, code, draft }, client) - if (itemsCount <= Category.maxCategoryItemsInIndex) { - itemsToValidate.push( - ...(await CategoryRepository.fetchItemsByLevelAndCode({ surveyId, levelUuid, code, draft }, client)) - ) + const items = await CategoryRepository.fetchItemsByLevelParentAndCode( + { surveyId, levelUuid, parentUuid, code, draft }, + client + ) + if (items.length > 0 && items.length <= Category.maxCategoryItemsInIndex) { + itemsToValidate.push(...items) } } const addItemToValidate = (item) => { diff --git a/server/modules/category/repository/categoryRepository.js b/server/modules/category/repository/categoryRepository.js index 6bc7ebde69..909c44c134 100644 --- a/server/modules/category/repository/categoryRepository.js +++ b/server/modules/category/repository/categoryRepository.js @@ -250,33 +250,41 @@ export const fetchItemsByCategoryUuid = async ( return backup || draft ? items : R.filter((item) => item.published)(items) } -const getWhereConditionItemsWithLevelAndCode = ({ draft, tableAlias = 'i' }) => { +const getWhereConditionItemsWithLevelParentAndCode = ({ draft, parentUuid = null, tableAlias = 'i' }) => { const codeColumn = DbUtils.getPropColCombined(CategoryItem.keysProps.code, draft, `${tableAlias}.`, true) - return `${tableAlias}.level_uuid = $/levelUuid/ AND COALESCE(${codeColumn}, '') = $/code/` + return `${tableAlias}.level_uuid = $/levelUuid/ + AND parent_uuid ${parentUuid ? '= $/parentUuid/' : ' IS NULL'} + AND COALESCE(${codeColumn}, '') = $/code/` } -export const countItemsByLevelAndCode = async ({ surveyId, levelUuid, code, draft = false }, client = db) => { +export const countItemsByLevelParentAndCode = async ( + { surveyId, levelUuid, parentUuid, code, draft = false }, + client = db +) => { const schema = Schemata.getSchemaSurvey(surveyId) const row = await client.one( `SELECT COUNT(*) FROM ${schema}.category_item i - WHERE ${getWhereConditionItemsWithLevelAndCode({ draft })} + WHERE ${getWhereConditionItemsWithLevelParentAndCode({ parentUuid, draft })} `, - { levelUuid, code: Strings.defaultIfEmpty('')(code) } + { levelUuid, parentUuid, code: Strings.defaultIfEmpty('')(code) } ) return Number(row.count) } -export const fetchItemsByLevelAndCode = async ({ surveyId, levelUuid, code, draft = false }, client = db) => { +export const fetchItemsByLevelParentAndCode = async ( + { surveyId, levelUuid, parentUuid, code, draft = false }, + client = db +) => { const schema = Schemata.getSchemaSurvey(surveyId) const items = await client.map( ` SELECT i.* FROM ${schema}.category_item i - WHERE ${getWhereConditionItemsWithLevelAndCode({ draft })} + WHERE ${getWhereConditionItemsWithLevelParentAndCode({ parentUuid, draft })} ORDER BY i.id `, - { levelUuid, code: Strings.defaultIfEmpty('')(code) }, + { levelUuid, parentUuid, code: Strings.defaultIfEmpty('')(code) }, (def) => DB.transformCallback(def, draft, true) )