[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #5227: [feat-4864] Support for deleting blueprints

["keon94 (via GitHub)" <gi...@apache.org>]

keon94 commented on code in PR #5227:
URL: https://github.com/apache/incubator-devlake/pull/5227#discussion_r1201255715


##########
backend/helpers/pluginhelper/api/scope_generic_helper.go:
##########
@@ -272,83 +273,72 @@ func (c *GenericScopeApiHelper[Conn, Scope, Tr]) GetScope(input *plugin.ApiResou
 
 func (c *GenericScopeApiHelper[Conn, Scope, Tr]) DeleteScope(input *plugin.ApiResourceInput) ([]*models.Blueprint, errors.Error) {
 	params := c.extractFromDeleteReqParam(input)
-	if params == nil || params.connectionId == 0 {
-		return nil, errors.BadInput.New("invalid path params: \"connectionId\" not set")
+	err := c.DeleteScopes(&plugin.BulkDeleteScopeRequest{
+		ConnectionId:   params.connectionId,
+		ScopeIds:       []string{params.scopeId},
+		Plugin:         params.plugin,
+		DeleteDataOnly: params.deleteDataOnly,
+	})
+	if err != nil {
+		return nil, err
 	}
-	if len(params.scopeId) == 0 || params.scopeId == "0" {
-		return nil, errors.BadInput.New("invalid path params: \"scopeId\" not set/invalid")
+	var impactedBlueprints []*models.Blueprint
+	if !params.deleteDataOnly {
+		blueprintsMap, err := c.bpManager.GetBlueprintsByScopes(params.connectionId, params.scopeId)
+		if err != nil {
+			return nil, errors.Default.Wrap(err, fmt.Sprintf("error retrieving scope with scope ID %s", params.scopeId))
+		}
+		blueprints := blueprintsMap[params.scopeId]
+		// update the blueprints (remove scope reference from them)
+		impactedBlueprints, err = c.updateBlueprints(blueprints, params.scopeId)
+		if err != nil {
+			return nil, err
+		}
 	}
-	err := c.dbHelper.VerifyConnection(params.connectionId)
-	if err != nil {
-		return nil, errors.Default.Wrap(err, fmt.Sprintf("error verifying connection for connection ID %d", params.connectionId))
+	return impactedBlueprints, nil
+}
+
+func (c *GenericScopeApiHelper[Conn, Scope, Tr]) DeleteScopes(req *plugin.BulkDeleteScopeRequest) errors.Error {
+	if req.ConnectionId == 0 {
+		return errors.BadInput.New("connectionId was empty on the input")
 	}
-	db := c.db
-	blueprintsMap, err := c.bpManager.GetBlueprintsByScopes(params.connectionId, params.scopeId)
-	if err != nil {
-		return nil, errors.Default.Wrap(err, fmt.Sprintf("error retrieving scope with scope ID %s", params.scopeId))
+	if len(req.ScopeIds) == 0 {
+		return errors.BadInput.New("scopeId(s) was empty on the input")
 	}
-	blueprints := blueprintsMap[params.scopeId]
-	// find all tables for this plugin
-	tables, err := getAffectedTables(params.plugin)
+	err := c.dbHelper.VerifyConnection(req.ConnectionId)
 	if err != nil {
-		return nil, errors.Default.Wrap(err, fmt.Sprintf("error getting database tables managed by plugin %s", params.plugin))
+		return errors.Default.Wrap(err, fmt.Sprintf("error verifying connection for connection ID %d", req.ConnectionId))
 	}
 	// delete all the plugin records referencing this scope
 	if c.reflectionParams.RawScopeParamName != "" {
-		scopeParamValue := params.scopeId
-		if c.opts.GetScopeParamValue != nil {
-			scopeParamValue, err = c.opts.GetScopeParamValue(c.db, params.scopeId)
+		scopeIds := req.ScopeIds
+		if c.opts.GetScopeParamValues != nil {
+			scopeIds, err = c.opts.GetScopeParamValues(c.db, scopeIds...)
 			if err != nil {
-				return nil, errors.Default.Wrap(err, fmt.Sprintf("error extracting scope parameter name for scope %s", params.scopeId))
+				return errors.Default.Wrap(err, fmt.Sprintf("error extracting scope parameter name for scopes: %v", scopeIds))
 			}
 		}
-		for _, table := range tables {
-			err = db.Exec(createDeleteQuery(table, c.reflectionParams.RawScopeParamName, scopeParamValue))
-			if err != nil {
-				return nil, errors.Default.Wrap(err, fmt.Sprintf("error deleting data bound to scope %s for plugin %s", params.scopeId, params.plugin))
-			}
+		// find all tables for this plugin
+		tables, err := getAffectedTables(req.Plugin)
+		if err != nil {
+			return errors.Default.Wrap(err, fmt.Sprintf("error getting database tables managed by plugin %s", req.Plugin))
 		}
-	}
-	var impactedBlueprints []*models.Blueprint
-	if !params.deleteDataOnly {
-		// Delete the scope itself
-		err = c.dbHelper.DeleteScope(params.connectionId, params.scopeId)
+		err = c.transactionalDelete(tables, scopeIds)
 		if err != nil {
-			return nil, errors.Default.Wrap(err, fmt.Sprintf("error deleting scope %s", params.scopeId))
+			return errors.Default.Wrap(err, fmt.Sprintf("error deleting data bound to scopes for plugin %v", req.ScopeIds))
 		}
-		// update the blueprints (remove scope reference from them)
-		for _, blueprint := range blueprints {

Review Comment:
   moved this bp logic into the updateBlueprints function to simplify code



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