You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2018/07/09 19:41:56 UTC

[GitHub] dangogh commented on a change in pull request #2519: TO Go: ignore "id" in JSON for PUT

dangogh commented on a change in pull request #2519: TO Go:  ignore "id" in JSON for PUT
URL: https://github.com/apache/trafficcontrol/pull/2519#discussion_r201123444
 
 

 ##########
 File path: traffic_ops/traffic_ops_golang/api/shared_handlers.go
 ##########
 @@ -255,22 +255,29 @@ func UpdateHandler(typeFactory CRUDFactory) http.HandlerFunc {
 			handleErrs(http.StatusBadRequest, errors.New("unable to parse required keys from request body"))
 			return // TODO verify?
 		}
-		for _, keyFieldInfo := range keyFields {
-			paramKey := params[keyFieldInfo.Field]
+		for _, kf := range keyFields {
+			paramKey := params[kf.Field]
 			if paramKey == "" {
-				log.Errorf("missing key: %s", keyFieldInfo.Field)
-				handleErrs(http.StatusBadRequest, errors.New("missing key: "+keyFieldInfo.Field))
+				log.Errorf("missing key: %s", kf.Field)
+				handleErrs(http.StatusBadRequest, errors.New("missing key: "+kf.Field))
 				return
 			}
 
-			paramValue, err := keyFieldInfo.Func(paramKey)
+			paramValue, err := kf.Func(paramKey)
 			if err != nil {
-				log.Errorf("failed to parse key %s: %s", keyFieldInfo.Field, err)
-				handleErrs(http.StatusBadRequest, errors.New("failed to parse key: "+keyFieldInfo.Field))
+				log.Errorf("failed to parse key %s: %s", kf.Field, err)
+				handleErrs(http.StatusBadRequest, errors.New("failed to parse key: "+kf.Field))
 				return
 			}
 
-			if paramValue != keys[keyFieldInfo.Field] {
+			if kf.Field == "id" && paramValue != "" {
 
 Review comment:
   I considered that,  but was unsure of the ramifications if there are multiple keys.  But,  I think you're right -- it shouldn't matter what the key is -- it should be required to come from params rather than the JSON.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services