fix: api responses enhancements

api/appStore/deployment/CommonDeploymentRestHandler.go

@@ -87,12 +87,12 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI
 	if len(appId) > 0 {
 		appIdentifier, err := handler.helmAppService.DecodeAppId(appId)
 		if err != nil {
-			err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid app id"}
+			// err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid app id"}
 			return appOfferingMode, installedAppDto, err
 		}
 		installedAppDto, err = handler.installedAppService.GetInstalledAppByClusterNamespaceAndName(appIdentifier)
 		if err != nil {
-			err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"}
+			// err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"}
 			return appOfferingMode, installedAppDto, err
 		}
 		// this is the case when hyperion apps does not linked yet
@@ -103,7 +103,7 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI
 			installedAppDto.AppOfferingMode = appOfferingMode
 			appIdentifier, err := handler.helmAppService.DecodeAppId(appId)
 			if err != nil {
-				err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid app id"}
+				// err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid app id, expected format clusterId|namespace|releaseName"}
 				return appOfferingMode, installedAppDto, err
 			}
 			installedAppDto.ClusterId = appIdentifier.ClusterId
@@ -113,16 +113,17 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI
 	} else if len(installedAppId) > 0 {
 		installedAppId, err := strconv.Atoi(installedAppId)
 		if err != nil {
-			err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid installed app id"}
+			handler.Logger.Errorw("Invalid installedAppId expected int value", "installedAppId", installedAppId, "err", err)
 			return appOfferingMode, installedAppDto, err
 		}
 		installedAppDto, err = handler.installedAppService.GetInstalledAppByInstalledAppId(installedAppId)
 		if err != nil {
-			err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"}
+			// err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"}
 			return appOfferingMode, installedAppDto, err
 		}
 	} else {
 		err := &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "app id missing in request"}
+		handler.Logger.Errorw("appId is missing and is a required field", "appId", appId, "err", err)
 		return appOfferingMode, installedAppDto, err
 	}
 	if installedAppDto != nil && installedAppDto.InstalledAppId > 0 {
@@ -188,6 +189,7 @@ func (handler *CommonDeploymentRestHandlerImpl) GetDeploymentHistoryValues(w htt
 	v := r.URL.Query()
 	installedAppId := v.Get("installedAppId")
 	appId := v.Get("appId")
+
 	appOfferingMode, installedAppDto, err := handler.getAppOfferingMode(installedAppId, appId)
 	if err != nil {
 		common.WriteJsonResp(w, err, "bad request", http.StatusBadRequest)

api/cluster/EnvironmentRestHandler.go

@@ -447,11 +447,15 @@ func (impl EnvironmentRestHandlerImpl) GetCombinedEnvironmentListForDropDownByCl
 			id, err := strconv.Atoi(clusterId)
 			if err != nil {
 				impl.logger.Errorw("request err, GetCombinedEnvironmentListForDropDownByClusterIds", "err", err, "clusterIdString", clusterIdString)
-				common.WriteJsonResp(w, err, "please send valid cluster Ids", http.StatusBadRequest)
+				common.HandleParameterError(w, r, "ids", clusterIdString)
 				return
 			}
 			clusterIds = append(clusterIds, id)
 		}
+	} else {
+		impl.logger.Errorw("request err empty query param, GetCombinedEnvironmentListForDropDownByClusterIds", "err", err, "clusterIdString", clusterIdString)
+		common.HandleParameterError(w, r, "ids", clusterIdString)
+		return
 	}
 	token := r.Header.Get("token")
 	clusters, err := impl.environmentClusterMappingsService.GetCombinedEnvironmentListForDropDownByClusterIds(token, clusterIds, impl.rbacEnforcementUtil.CheckAuthorizationForGlobalEnvironment)

api/devtronResource/DevtronResourceHistoryRestHandler.go

@@ -54,9 +54,16 @@ func (handler *HistoryRestHandlerImpl) GetDeploymentHistory(w http.ResponseWrite
 	queryParams := apiBean.GetHistoryQueryParams{}
 	err := decoder.Decode(&queryParams, v)
 	if err != nil {
+		handler.logger.Errorw("error in decoding query parameters", "queryParams", v, "err", err)
 		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
 		return
 	}
+	// Validating that filterCriteria is provided
+	if len(queryParams.FilterCriteria) == 0 {
+		handler.logger.Errorw("missing required filterCriteria parameter")
+		common.HandleParameterError(w, r, "filterCriteria", "")
+		return
+	}
 	decodedReqBean, err := handler.apiReqDecoderService.GetFilterCriteriaParamsForDeploymentHistory(queryParams.FilterCriteria)
 	if err != nil {
 		handler.logger.Errorw("error in getting filter criteria params", "err", err, "filterCriteria", queryParams.FilterCriteria)

api/helm-app/service/helper.go

@@ -29,7 +29,7 @@ import (
 func DecodeExternalAppAppId(appId string) (*bean.AppIdentifier, error) {
 	component := strings.Split(appId, "|")
 	if len(component) != 3 {
-		return nil, fmt.Errorf("malformed app id %s", appId)
+		return nil, fmt.Errorf("malformed appId,  appId: %s, expected format clusterId|namespace|releaseName", appId)
 	}
 	clusterId, err := strconv.Atoi(component[0])
 	if err != nil {

api/restHandler/app/pipeline/AutoCompleteRestHandler.go

@@ -136,7 +136,11 @@ func (handler DevtronAppAutoCompleteRestHandlerImpl) GetAppListForAutocomplete(w
 			return
 		}
 	} else {
-		teamIdInt, err = strconv.Atoi(teamId)
+		teamIdInt, err = common.ExtractIntPathParamWithContext(w, r, "teamId", teamId+" team")
+		if err != nil {
+			// Error already written by ExtractIntPathParamWithContext
+			return
+		}
 		if err != nil {
 			common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
 			return

api/restHandler/app/pipeline/configure/BuildPipelineRestHandler.go

@@ -1209,15 +1209,21 @@ func (handler *PipelineConfigRestHandlerImpl) FetchMaterialInfo(w http.ResponseW
 
 func (handler *PipelineConfigRestHandlerImpl) GetCIPipelineById(w http.ResponseWriter, r *http.Request) {
 	token := r.Header.Get("token")
-	vars := mux.Vars(r)
-	appId, err := strconv.Atoi(vars["appId"])
+	// vars := mux.Vars(r)
+	//appId, err := strconv.Atoi(vars["appId"])
+	appId, err := common.ExtractIntPathParamWithContext(w, r, "appId", "app")
 	if err != nil {
-		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		// response already written by ExtractIntPathParamWithContext
+		//common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		handler.Logger.Error("request err getting , GetCIPipelineById", "appId", appId, "err", err)
 		return
 	}
-	pipelineId, err := strconv.Atoi(vars["pipelineId"])
+	//pipelineId, err := strconv.Atoi(vars["pipelineId"])
+	pipelineId, err := common.ExtractIntPathParamWithContext(w, r, "pipelineId", "pipeline")
 	if err != nil {
-		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		// response already written by ExtractIntPathParamWithContext
+		// common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		handler.Logger.Error("request err getting , GetCIPipelineById", "pipelineId", pipelineId, "err", err)
 		return
 	}
 
@@ -1711,20 +1717,29 @@ func (handler *PipelineConfigRestHandlerImpl) FetchWorkflowDetails(w http.Respon
 		common.HandleUnauthorized(w, r)
 		return
 	}
-	vars := mux.Vars(r)
-	appId, err := strconv.Atoi(vars["appId"])
+	// vars := mux.Vars(r)
+	// appId, err = strconv.Atoi(vars["appId"])
+	appId, err := common.ExtractIntPathParamWithContext(w, r, "appId", "app")
 	if err != nil {
-		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		// response already written by ExtractIntPathParamWithContext
+		// common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		handler.Logger.Error("request err getting , FetchWorkflowDetails", "appId", appId, "err", err)
 		return
 	}
-	pipelineId, err := strconv.Atoi(vars["pipelineId"])
+	// pipelineId, err := strconv.Atoi(vars["pipelineId"])
+	pipelineId, err := common.ExtractIntPathParamWithContext(w, r, "pipelineId", "pipeline")
 	if err != nil {
-		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		// response already written by ExtractIntPathParamWithContext
+		// common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		handler.Logger.Error("request err getting , FetchWorkflowDetails", "pipelineId", pipelineId, "err", err)
 		return
 	}
-	buildId, err := strconv.Atoi(vars["workflowId"])
+	// buildId, err := strconv.Atoi(vars["workflowId"])
+	buildId, err := common.ExtractIntPathParamWithContext(w, r, "workflowId", "workflow")
 	if err != nil || buildId == 0 {
-		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		// response already written by ExtractIntPathParamWithContext
+		// common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		handler.Logger.Error("request err getting , FetchWorkflowDetails", "buildId", buildId, "err", err)
 		return
 	}
 	handler.Logger.Infow("request payload, FetchWorkflowDetails", "appId", appId, "pipelineId", pipelineId, "buildId", buildId, "buildId", buildId)

pkg/devtronResource/APIReqDecoderService.go

@@ -8,6 +8,7 @@ import (
 	"go.uber.org/zap"
 	"net/http"
 	"strconv"
+	"strings"
 )
 
 // APIReqDecoderService is common service used for getting decoded devtronResource and history related api params.
@@ -52,11 +53,26 @@ func (impl *APIReqDecoderServiceImpl) GetFilterCriteriaParamsForDeploymentHistor
 		return nil, &util2.ApiError{
 			HttpStatusCode:  http.StatusBadRequest,
 			Code:            "400",
-			InternalMessage: "invalid format filter criteria!",
-			UserMessage:     "invalid format filter criteria!",
+			InternalMessage: "missing required identifiers: either (appId and envId) or pipelineId must be provided to fetch history",
+			UserMessage:     "Please provide either both App ID and Env ID, or a Pipeline ID to view history.",
 		}
 	}
 	if resp.PipelineId == 0 {
+		missingResources := []string{}
+		if resp.AppId == 0 {
+			missingResources = append(missingResources, "appId")
+		}
+		if resp.EnvId == 0 {
+			missingResources = append(missingResources, "EnvId")
+		}
+		if len(missingResources) > 0 {
+			return nil, &util2.ApiError{
+				HttpStatusCode:  http.StatusBadRequest,
+				Code:            "400",
+				InternalMessage: "missing required resources: " + strings.Join(missingResources, ", "),
+				UserMessage:     "missing required resources: " + strings.Join(missingResources, ", "),
+			}
+		}
 		pipelineObj, err := impl.pipelineRepository.FindActiveByAppIdAndEnvId(resp.AppId, resp.EnvId)
 		if err != nil {
 			impl.logger.Errorw("error in getting pipeline", "appId", resp.AppId, "envId", resp.EnvId, "err", err)