You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by "keon94 (via GitHub)" <gi...@apache.org> on 2023/05/18 06:59:16 UTC

[GitHub] [incubator-devlake] keon94 opened a new pull request, #5227: [feat-4864] Support for deleting blueprints and projects

keon94 opened a new pull request, #5227:
URL: https://github.com/apache/incubator-devlake/pull/5227

   ### ⚠️ Pre Checklist
   
   > Please complete _ALL_ items in this checklist, and remove before submitting
   
   - [ ] I have read through the [Contributing Documentation](https://devlake.apache.org/community/).
   - [ ] I have added relevant tests.
   - [ ] I have added relevant documentation.
   - [ ] I will add labels to the PR, such as `pr-type/bug-fix`, `pr-type/feature-development`, etc.
   
   <!--
   Thanks for submitting a pull request!
   
   We appreciate you spending the time to work on these changes.
   Please fill out as many sections below as possible.
   -->
   
   ### Summary
   What does this PR do?
   
   ### Does this close any open issues?
   Closes xx
   
   ### Screenshots
   Include any relevant screenshots here.
   
   ### Other Information
   Any other information that is important to this PR.
   


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


[GitHub] [incubator-devlake] hezyin merged pull request #5227: [feat-4864] Support for deleting blueprints

Posted by "hezyin (via GitHub)" <gi...@apache.org>.
hezyin merged PR #5227:
URL: https://github.com/apache/incubator-devlake/pull/5227


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


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

Posted by "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