fixed category validator, validation of sibling items (#3690)

Co-authored-by: Stefano Ricci <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
openforis · Dec 10, 2024 · 2cb6ac9 · 2cb6ac9
1 parent d4fcccc
commit 2cb6ac9
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 35 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)
   )