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


##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
   useEffect(() => {
     setSelectValue(
       transformation?.deploymentPattern ||
-        transformation?.deploymentPattern === ''
+        transformation?.deploymentPattern === '' ||
+        transformation?.productionPattern ||
+        transformation?.productionPattern === ''
         ? 1
         : 0
     )
-  }, [transformation?.deploymentPattern])
+  }, [transformation?.deploymentPattern, transformation?.productionPattern])
 
   const handleChangeSelectValue = (sv) => {
     if (entityIdKey && sv === 0) {
-      onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+      onSettingsChange(
+        { deploymentPattern: undefined, productionPattern: undefined },

Review Comment:
   Using `undefined` for String inputs is not desired practice. When you set 
this to `undefined` it will propagate to `$activeTransformation` which is the 
main `$transformation` object at this layer, and React does not expect or want 
an undefined value for its input components.
   
   It appears you are wanting to use `undefined` as a means to determine the 
"Off" state for radio selectors, when we should be using `empty` string instead 
(as originally designed).
   
   If empty string means something else to the backend, we may need to consider 
using another string value to "disable" a deploy tag, perhaps "[disabled]" or 
"/disabled/"



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to