[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4707: Add and satisfy OptionsDelete API test for DELETE /regions?name={{name}}

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

rawlinp commented on a change in pull request #4707:
URL: https://github.com/apache/trafficcontrol/pull/4707#discussion_r428980079



##########
File path: traffic_ops/traffic_ops_golang/api/shared_handlers.go
##########
@@ -390,30 +416,34 @@ func DeprecatedDeleteHandler(deleter Deleter, alternative *string) http.HandlerF
 		obj := reflect.New(objectType).Interface().(Deleter)
 		obj.SetInfo(inf)
 
-		deleteKeyOptionExists, userErr, sysErr, errCode := hasDeleteKeyOption(obj, inf.Params)
+		isOptionsDeleter, userErr, sysErr, errCode := checkIfOptionsDeleter(obj, inf.Params)
 		if userErr != nil || sysErr != nil {
 			HandleDeprecatedErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr, alternative)
 			return
 		}
-		if !deleteKeyOptionExists {
-			keyFields := obj.GetKeyFieldsInfo() // expecting a slice of the key fields info which is a struct with the field name and a function to convert a string into a interface{} of the right type. in most that will be [{Field:"id",Func: func(s string)(interface{},error){return strconv.Atoi(s)}}]
-			keys := make(map[string]interface{})
-			for _, kf := range keyFields {
-				paramKey := inf.Params[kf.Field]
-				if paramKey == "" {
-					HandleDeprecatedErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("missing key: "+kf.Field), nil, alternative)
-					return
+		keyFields := obj.GetKeyFieldsInfo() // expecting a slice of the key fields info which is a struct with the field name and a function to convert a string into a interface{} of the right type. in most that will be [{Field:"id",Func: func(s string)(interface{},error){return strconv.Atoi(s)}}]

Review comment:
       It looks like the DeprecatedDeleteHandler function didn't get all the same treatment as the DeleteHandler. Before, they were basically identical except for `HandleErr` vs `HandleDeprecatedErr` (before I messed).
   
   We should keep these in parity, right? Until we reduce the duplication here.




----------------------------------------------------------------
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.

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