[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3297: fix(config-ui): transformation not updating correctly when switching boards

[GitBox <gi...@apache.org>]

e2corporation commented on code in PR #3297:
URL: https://github.com/apache/incubator-devlake/pull/3297#discussion_r991750540


##########
config-ui/src/pages/configure/settings/jira.jsx:
##########
@@ -177,58 +177,35 @@ export default function JiraSettings(props) {
   }, [typeMappingAll, onSettingsChange, configuredBoard?.id])
 
   useEffect(() => {
-    if (typeMappingBug && typeMappingIncident && typeMappingRequirement) {
-      const RequirementMappings =
-        typeMappingRequirement !== ''
-          ? typeMappingRequirement.map((r) =>
-              createTypeMapObject(r.value, MAPPING_TYPES.Requirement)
-            )
-          : []
-      const IncidentMappings =
-        typeMappingIncident !== ''
-          ? typeMappingIncident.map((i) =>
-              createTypeMapObject(i.value, MAPPING_TYPES.Incident)
-            )
-          : []
-      const BugMappings =
-        typeMappingBug !== ''
-          ? typeMappingBug.map((b) =>
-              createTypeMapObject(b.value, MAPPING_TYPES.Bug)
-            )
-          : []
-      const CombinedMappings = [
-        ...RequirementMappings,
-        ...IncidentMappings,
-        ...BugMappings
-      ].filter((m) => m !== null)
-      const MappingTypeObjects = CombinedMappings.reduce((pV, cV) => {
-        return { ...cV, ...pV }
-      }, {})
-      setTypeMappingAll(MappingTypeObjects)
-      console.log(
-        '>> INCIDENT TYPE MAPPING OBJECTS....',
-        RequirementMappings,
-        IncidentMappings,
-        BugMappings
-      )
-      console.log('>> ALL MAPPINGS COMBINED...', CombinedMappings)
-      console.log(
-        '>> FINAL MAPPING OBJECTS FOR API REQUEST...',
-        MappingTypeObjects
-      )
-    }
-  }, [typeMappingBug, typeMappingIncident, typeMappingRequirement])
+    setTypeMappingAll((ma) => ({
+      ...ma,
+      ...(typeMappingBug || [])
+        .map((r) => createTypeMapObject(r.value, MAPPING_TYPES.Bug))
+        .reduce((c, p) => ({ ...c, ...p }), {})
+    }))
+  }, [typeMappingBug])
 
   useEffect(() => {
-    console.log('>> CONN SETTINGS OBJECT ', connection)
-    if (connection && connection.id) {
-      // Parse Type Mappings (V2)
-      // setStatusMappings([])
-    }
-  }, [connection])
+    setTypeMappingAll((ma) => ({
+      ...ma,
+      ...(typeMappingIncident || [])
+        .map((r) => createTypeMapObject(r.value, MAPPING_TYPES.Incident))
+        .reduce((c, p) => ({ ...c, ...p }), {})
+    }))
+  }, [typeMappingIncident])
+
+  useEffect(() => {
+    setTypeMappingAll((ma) => ({
+      ...ma,
+      ...(typeMappingRequirement || [])
+        .map((r) => createTypeMapObject(r.value, MAPPING_TYPES.Requirement))
+        .reduce((c, p) => ({ ...c, ...p }), {})
+    }))
+  }, [typeMappingRequirement])

Review Comment:
   Effects run asynchronously, there's a reason why the existing logic was using 1 Effect in the first place, `typeMappingAll` may encounter collisions as a result. The 3 separate requirement groups should all be processed at the same time, not separately.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org