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/10 19:14:41 UTC

[GitHub] [incubator-devlake] keon94 opened a new pull request, #5153: Issues/4762-part1: Minimal support for deleting scopes

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

   ### ⚠️ Pre Checklist
   
   ### 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] keon94 commented on a diff in pull request #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#discussion_r1191541378


##########
backend/helpers/pluginhelper/api/api_rawdata.go:
##########
@@ -37,16 +38,23 @@ type RawData struct {
 	CreatedAt time.Time
 }
 
+type TaskOptions interface {
+	GetParams() any
+}
+
 // RawDataSubTaskArgs FIXME ...
 type RawDataSubTaskArgs struct {
 	Ctx plugin.SubTaskContext
 
 	//	Table store raw data
 	Table string `comment:"Raw data table name"`
 
-	//	This struct will be JSONEncoded and stored into database along with raw data itself, to identity minimal
-	//	set of data to be process, for example, we process JiraIssues by Board
-	Params interface{} `comment:"To identify a set of records with same UrlTemplate, i.e. {ConnectionId, BoardId} for jira entities"`
+	// Deprecated: Use Options instead

Review Comment:
   That will require widespread change across all the plugins which you wanted to avoid here. That can be deferred to another refactor 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] keon94 commented on a diff in pull request #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#discussion_r1191246744


##########
backend/helpers/pluginhelper/api/scope_helper.go:
##########
@@ -207,24 +243,53 @@ func (c *ScopeApiHelper[Conn, Scope, Tr]) GetScopeList(input *plugin.ApiResource
 	if err != nil {
 		return nil, err
 	}
+	if params.loadBlueprints {
+		if len(scopeIdFieldName) == 0 {
+			return nil, errors.Default.New("scope Id field name is not known") //temporary, limited solution until I properly refactor all of this in another PR
+		}
+		scopesById := c.mapByScopeId(apiScopes, scopeIdFieldName[0])
+		var scopeIds []string
+		for id := range scopesById {
+			scopeIds = append(scopeIds, id)
+		}
+		blueprintMap, err := c.bpManager.GetBlueprintsByScopes(scopeIds...)

Review Comment:
   Should it filter by bother connectionId and scopeId? Right now it only does based on scopeId



-- 
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] klesh commented on a diff in pull request #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#discussion_r1191287674


##########
backend/helpers/pluginhelper/api/api_rawdata.go:
##########
@@ -37,16 +38,23 @@ type RawData struct {
 	CreatedAt time.Time
 }
 
+type TaskOptions interface {
+	GetParams() any
+}
+
 // RawDataSubTaskArgs FIXME ...
 type RawDataSubTaskArgs struct {
 	Ctx plugin.SubTaskContext
 
 	//	Table store raw data
 	Table string `comment:"Raw data table name"`
 
-	//	This struct will be JSONEncoded and stored into database along with raw data itself, to identity minimal
-	//	set of data to be process, for example, we process JiraIssues by Board
-	Params interface{} `comment:"To identify a set of records with same UrlTemplate, i.e. {ConnectionId, BoardId} for jira entities"`
+	// Deprecated: Use Options instead

Review Comment:
   I see, can we reuse the variable instead of creating a new one?



-- 
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 #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#discussion_r1191231792


##########
backend/helpers/pluginhelper/api/api_rawdata.go:
##########
@@ -37,16 +38,23 @@ type RawData struct {
 	CreatedAt time.Time
 }
 
+type TaskOptions interface {
+	GetParams() any
+}
+
 // RawDataSubTaskArgs FIXME ...
 type RawDataSubTaskArgs struct {
 	Ctx plugin.SubTaskContext
 
 	//	Table store raw data
 	Table string `comment:"Raw data table name"`
 
-	//	This struct will be JSONEncoded and stored into database along with raw data itself, to identity minimal
-	//	set of data to be process, for example, we process JiraIssues by Board
-	Params interface{} `comment:"To identify a set of records with same UrlTemplate, i.e. {ConnectionId, BoardId} for jira entities"`
+	// Deprecated: Use Options instead

Review Comment:
   The idea here is to remove this boilerplate of setting the params in every single subtask of a plugin. This value is a constant for a given plugin (as far as I'm aware - correct me if I'm wrong), so it makes a lot more sense to have it be set in a single place and be enforced.  Doing this using an interface made the most sense to me.



-- 
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 #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#discussion_r1191251040


##########
backend/helpers/pluginhelper/api/scope_helper.go:
##########
@@ -245,9 +310,95 @@ func (c *ScopeApiHelper[Conn, Scope, Tr]) GetScope(input *plugin.ApiResourceInpu
 			return nil, errors.NotFound.New("transformationRule not found")
 		}
 	}
-	scopeRes := &ScopeRes[Scope]{scope, reflect.ValueOf(rule).FieldByName("Name").String()}
+	scopeRes := &ScopeRes[Scope]{
+		Scope:                  scope,
+		TransformationRuleName: reflect.ValueOf(rule).FieldByName("Name").String(),
+	}
 	return &plugin.ApiResourceOutput{Body: scopeRes, Status: http.StatusOK}, nil
 }
+func (c *ScopeApiHelper[Conn, Scope, Tr]) DeleteScope(input *plugin.ApiResourceInput, scopeIdFieldName string, rawScopeParamName string,
+	getScopeParamValue func(db dal.Dal, scopeId string) (string, errors.Error)) (*plugin.ApiResourceOutput, errors.Error) {
+	params := c.extractFromDeleteReqParam(input)
+	if params == nil || params.connectionId == 0 {
+		return nil, errors.BadInput.New("invalid path params: \"connectionId\" not set")
+	}
+	if len(params.scopeId) == 0 || params.scopeId == "0" {
+		return nil, errors.BadInput.New("invalid path params: \"scopeId\" not set/invalid")
+	}
+	err := c.VerifyConnection(params.connectionId)
+	if err != nil {
+		return nil, errors.Default.Wrap(err, fmt.Sprintf("error verifying connection for connection ID %d", params.connectionId))
+	}
+	db := c.db
+	blueprintsMap, err := c.bpManager.GetBlueprintsByScopes(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]
+	// find all tables for this plugin
+	tables, err := getPluginTables(params.plugin)
+	if err != nil {
+		return nil, errors.Default.Wrap(err, fmt.Sprintf("error getting database tables managed by plugin %s", params.plugin))
+	}
+	// delete all the plugin records referencing this scope
+	if rawScopeParamName != "" {

Review Comment:
   I don't think webhook even uses this API. It probably needs a separate solution



-- 
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] klesh commented on a diff in pull request #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#discussion_r1191295841


##########
backend/helpers/pluginhelper/api/scope_helper.go:
##########
@@ -207,24 +243,53 @@ func (c *ScopeApiHelper[Conn, Scope, Tr]) GetScopeList(input *plugin.ApiResource
 	if err != nil {
 		return nil, err
 	}
+	if params.loadBlueprints {
+		if len(scopeIdFieldName) == 0 {
+			return nil, errors.Default.New("scope Id field name is not known") //temporary, limited solution until I properly refactor all of this in another PR
+		}
+		scopesById := c.mapByScopeId(apiScopes, scopeIdFieldName[0])
+		var scopeIds []string
+		for id := range scopesById {
+			scopeIds = append(scopeIds, id)
+		}
+		blueprintMap, err := c.bpManager.GetBlueprintsByScopes(scopeIds...)

Review Comment:
   Pardon?  Did you mean both?
   `ScopeId` itself is not sufficient to identify a scope, it should be used with `connectionId`, shouldn't it?
   ![image](https://github.com/apache/incubator-devlake/assets/61080/e18102c7-755f-446e-8bf1-c83af8874350)
   



-- 
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 pull request #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#issuecomment-1545351686

   > Nice work, However, my pagerduty account has expired, unable to run it locally. Please attache a set of screenshots to demonstrate the records in related tables are deleted correctly, including `raw layer` `tool layer` , `domain layer` and the `_devlake_collector_latest_state` tables.
   
   @klesh 
   Before Delete:
   ![image](https://github.com/apache/incubator-devlake/assets/25063936/237eb948-d141-41e4-978b-19f41f9d5d2a)
   ![image](https://github.com/apache/incubator-devlake/assets/25063936/75f88a04-a127-4ee1-b4c4-9c03c8ce0401)
   ![image](https://github.com/apache/incubator-devlake/assets/25063936/550b16b7-fb7b-44bd-8354-7126b6496b83)
   ![image](https://github.com/apache/incubator-devlake/assets/25063936/86572df7-6bf5-473a-ac32-8f76db422c46)
   
   
   After Delete:
   ![image](https://github.com/apache/incubator-devlake/assets/25063936/b79f9d95-5076-4a72-8475-03015edd96f8)
   ![image](https://github.com/apache/incubator-devlake/assets/25063936/60f6a266-5a1a-472d-88bb-dc019ea97754)
   ![image](https://github.com/apache/incubator-devlake/assets/25063936/88754348-fbfd-4d72-8085-75cba36f3a33)
   ![image](https://github.com/apache/incubator-devlake/assets/25063936/832a7d00-0b5d-478a-a403-c1083b4a4989)
   
   
   


-- 
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] klesh commented on a diff in pull request #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#discussion_r1191879708


##########
backend/helpers/pluginhelper/api/api_rawdata.go:
##########
@@ -37,16 +38,23 @@ type RawData struct {
 	CreatedAt time.Time
 }
 
+type TaskOptions interface {
+	GetParams() any
+}
+
 // RawDataSubTaskArgs FIXME ...
 type RawDataSubTaskArgs struct {
 	Ctx plugin.SubTaskContext
 
 	//	Table store raw data
 	Table string `comment:"Raw data table name"`
 
-	//	This struct will be JSONEncoded and stored into database along with raw data itself, to identity minimal
-	//	set of data to be process, for example, we process JiraIssues by Board
-	Params interface{} `comment:"To identify a set of records with same UrlTemplate, i.e. {ConnectionId, BoardId} for jira entities"`
+	// Deprecated: Use Options instead

Review Comment:
   I see, you are right. the `Params` is moved to the `TaskOptions`, It is the right way to go.



-- 
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] klesh commented on a diff in pull request #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#discussion_r1191298284


##########
backend/helpers/pluginhelper/api/scope_helper.go:
##########
@@ -245,9 +310,95 @@ func (c *ScopeApiHelper[Conn, Scope, Tr]) GetScope(input *plugin.ApiResourceInpu
 			return nil, errors.NotFound.New("transformationRule not found")
 		}
 	}
-	scopeRes := &ScopeRes[Scope]{scope, reflect.ValueOf(rule).FieldByName("Name").String()}
+	scopeRes := &ScopeRes[Scope]{
+		Scope:                  scope,
+		TransformationRuleName: reflect.ValueOf(rule).FieldByName("Name").String(),
+	}
 	return &plugin.ApiResourceOutput{Body: scopeRes, Status: http.StatusOK}, nil
 }
+func (c *ScopeApiHelper[Conn, Scope, Tr]) DeleteScope(input *plugin.ApiResourceInput, scopeIdFieldName string, rawScopeParamName string,
+	getScopeParamValue func(db dal.Dal, scopeId string) (string, errors.Error)) (*plugin.ApiResourceOutput, errors.Error) {
+	params := c.extractFromDeleteReqParam(input)
+	if params == nil || params.connectionId == 0 {
+		return nil, errors.BadInput.New("invalid path params: \"connectionId\" not set")
+	}
+	if len(params.scopeId) == 0 || params.scopeId == "0" {
+		return nil, errors.BadInput.New("invalid path params: \"scopeId\" not set/invalid")
+	}
+	err := c.VerifyConnection(params.connectionId)
+	if err != nil {
+		return nil, errors.Default.Wrap(err, fmt.Sprintf("error verifying connection for connection ID %d", params.connectionId))
+	}
+	db := c.db
+	blueprintsMap, err := c.bpManager.GetBlueprintsByScopes(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]
+	// find all tables for this plugin
+	tables, err := getPluginTables(params.plugin)
+	if err != nil {
+		return nil, errors.Default.Wrap(err, fmt.Sprintf("error getting database tables managed by plugin %s", params.plugin))
+	}
+	// delete all the plugin records referencing this scope
+	if rawScopeParamName != "" {

Review Comment:
   You are right, remember to deal with it.



-- 
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] klesh commented on a diff in pull request #5153: [feat-4762] part1: Minimal support for deleting scopes

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #5153:
URL: https://github.com/apache/incubator-devlake/pull/5153#discussion_r1190801860


##########
backend/helpers/pluginhelper/api/api_rawdata.go:
##########
@@ -37,16 +38,23 @@ type RawData struct {
 	CreatedAt time.Time
 }
 
+type TaskOptions interface {
+	GetParams() any
+}
+
 // RawDataSubTaskArgs FIXME ...
 type RawDataSubTaskArgs struct {
 	Ctx plugin.SubTaskContext
 
 	//	Table store raw data
 	Table string `comment:"Raw data table name"`
 
-	//	This struct will be JSONEncoded and stored into database along with raw data itself, to identity minimal
-	//	set of data to be process, for example, we process JiraIssues by Board
-	Params interface{} `comment:"To identify a set of records with same UrlTemplate, i.e. {ConnectionId, BoardId} for jira entities"`
+	// Deprecated: Use Options instead

Review Comment:
   Please explain the reason behind it, I don't understand.



##########
backend/helpers/pluginhelper/api/scope_helper.go:
##########
@@ -207,24 +243,53 @@ func (c *ScopeApiHelper[Conn, Scope, Tr]) GetScopeList(input *plugin.ApiResource
 	if err != nil {
 		return nil, err
 	}
+	if params.loadBlueprints {
+		if len(scopeIdFieldName) == 0 {
+			return nil, errors.Default.New("scope Id field name is not known") //temporary, limited solution until I properly refactor all of this in another PR
+		}
+		scopesById := c.mapByScopeId(apiScopes, scopeIdFieldName[0])
+		var scopeIds []string
+		for id := range scopesById {
+			scopeIds = append(scopeIds, id)
+		}
+		blueprintMap, err := c.bpManager.GetBlueprintsByScopes(scopeIds...)

Review Comment:
   Is this correct? I believe the `connectionId` is missing here.



##########
backend/helpers/pluginhelper/api/scope_helper.go:
##########
@@ -245,9 +310,95 @@ func (c *ScopeApiHelper[Conn, Scope, Tr]) GetScope(input *plugin.ApiResourceInpu
 			return nil, errors.NotFound.New("transformationRule not found")
 		}
 	}
-	scopeRes := &ScopeRes[Scope]{scope, reflect.ValueOf(rule).FieldByName("Name").String()}
+	scopeRes := &ScopeRes[Scope]{
+		Scope:                  scope,
+		TransformationRuleName: reflect.ValueOf(rule).FieldByName("Name").String(),
+	}
 	return &plugin.ApiResourceOutput{Body: scopeRes, Status: http.StatusOK}, nil
 }
+func (c *ScopeApiHelper[Conn, Scope, Tr]) DeleteScope(input *plugin.ApiResourceInput, scopeIdFieldName string, rawScopeParamName string,
+	getScopeParamValue func(db dal.Dal, scopeId string) (string, errors.Error)) (*plugin.ApiResourceOutput, errors.Error) {
+	params := c.extractFromDeleteReqParam(input)
+	if params == nil || params.connectionId == 0 {
+		return nil, errors.BadInput.New("invalid path params: \"connectionId\" not set")
+	}
+	if len(params.scopeId) == 0 || params.scopeId == "0" {
+		return nil, errors.BadInput.New("invalid path params: \"scopeId\" not set/invalid")
+	}
+	err := c.VerifyConnection(params.connectionId)
+	if err != nil {
+		return nil, errors.Default.Wrap(err, fmt.Sprintf("error verifying connection for connection ID %d", params.connectionId))
+	}
+	db := c.db
+	blueprintsMap, err := c.bpManager.GetBlueprintsByScopes(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]
+	// find all tables for this plugin
+	tables, err := getPluginTables(params.plugin)
+	if err != nil {
+		return nil, errors.Default.Wrap(err, fmt.Sprintf("error getting database tables managed by plugin %s", params.plugin))
+	}
+	// delete all the plugin records referencing this scope
+	if rawScopeParamName != "" {
+		scopeParamValue := params.scopeId
+		if getScopeParamValue != nil {
+			scopeParamValue, err = getScopeParamValue(c.db, params.scopeId) // this function is optional - use it if API data params stores a value different to the scope id (e.g. github plugin)
+			if err != nil {
+				return nil, errors.Default.Wrap(err, fmt.Sprintf("error extracting scope parameter name for scope %s", params.scopeId))
+			}
+		}
+		for _, table := range tables {
+			err = db.Exec(createDeleteQuery(table, 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))
+			}
+		}

Review Comment:
   I think we have to delete related records from the `_devlake_collector_latest_state` table as well



##########
backend/helpers/pluginhelper/api/scope_helper.go:
##########
@@ -245,9 +310,95 @@ func (c *ScopeApiHelper[Conn, Scope, Tr]) GetScope(input *plugin.ApiResourceInpu
 			return nil, errors.NotFound.New("transformationRule not found")
 		}
 	}
-	scopeRes := &ScopeRes[Scope]{scope, reflect.ValueOf(rule).FieldByName("Name").String()}
+	scopeRes := &ScopeRes[Scope]{
+		Scope:                  scope,
+		TransformationRuleName: reflect.ValueOf(rule).FieldByName("Name").String(),
+	}
 	return &plugin.ApiResourceOutput{Body: scopeRes, Status: http.StatusOK}, nil
 }
+func (c *ScopeApiHelper[Conn, Scope, Tr]) DeleteScope(input *plugin.ApiResourceInput, scopeIdFieldName string, rawScopeParamName string,
+	getScopeParamValue func(db dal.Dal, scopeId string) (string, errors.Error)) (*plugin.ApiResourceOutput, errors.Error) {
+	params := c.extractFromDeleteReqParam(input)
+	if params == nil || params.connectionId == 0 {
+		return nil, errors.BadInput.New("invalid path params: \"connectionId\" not set")
+	}
+	if len(params.scopeId) == 0 || params.scopeId == "0" {
+		return nil, errors.BadInput.New("invalid path params: \"scopeId\" not set/invalid")
+	}
+	err := c.VerifyConnection(params.connectionId)
+	if err != nil {
+		return nil, errors.Default.Wrap(err, fmt.Sprintf("error verifying connection for connection ID %d", params.connectionId))
+	}
+	db := c.db
+	blueprintsMap, err := c.bpManager.GetBlueprintsByScopes(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]
+	// find all tables for this plugin
+	tables, err := getPluginTables(params.plugin)
+	if err != nil {
+		return nil, errors.Default.Wrap(err, fmt.Sprintf("error getting database tables managed by plugin %s", params.plugin))
+	}
+	// delete all the plugin records referencing this scope
+	if rawScopeParamName != "" {

Review Comment:
   There is a catch, the `webhook` plugin doesn't have `scopes` only `connections`.
   



##########
backend/helpers/pluginhelper/api/scope_helper.go:
##########
@@ -245,9 +310,95 @@ func (c *ScopeApiHelper[Conn, Scope, Tr]) GetScope(input *plugin.ApiResourceInpu
 			return nil, errors.NotFound.New("transformationRule not found")
 		}
 	}
-	scopeRes := &ScopeRes[Scope]{scope, reflect.ValueOf(rule).FieldByName("Name").String()}
+	scopeRes := &ScopeRes[Scope]{
+		Scope:                  scope,
+		TransformationRuleName: reflect.ValueOf(rule).FieldByName("Name").String(),
+	}
 	return &plugin.ApiResourceOutput{Body: scopeRes, Status: http.StatusOK}, nil
 }
+func (c *ScopeApiHelper[Conn, Scope, Tr]) DeleteScope(input *plugin.ApiResourceInput, scopeIdFieldName string, rawScopeParamName string,
+	getScopeParamValue func(db dal.Dal, scopeId string) (string, errors.Error)) (*plugin.ApiResourceOutput, errors.Error) {
+	params := c.extractFromDeleteReqParam(input)
+	if params == nil || params.connectionId == 0 {
+		return nil, errors.BadInput.New("invalid path params: \"connectionId\" not set")
+	}
+	if len(params.scopeId) == 0 || params.scopeId == "0" {
+		return nil, errors.BadInput.New("invalid path params: \"scopeId\" not set/invalid")
+	}
+	err := c.VerifyConnection(params.connectionId)
+	if err != nil {
+		return nil, errors.Default.Wrap(err, fmt.Sprintf("error verifying connection for connection ID %d", params.connectionId))
+	}
+	db := c.db
+	blueprintsMap, err := c.bpManager.GetBlueprintsByScopes(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]
+	// find all tables for this plugin
+	tables, err := getPluginTables(params.plugin)

Review Comment:
   the function name is confusing, I suggest to rename it to `getAffectedTables`



-- 
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] klesh merged pull request #5153: [feat-4762] part1: Minimal support for deleting scopes

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


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