[Fix] Trigger 'unlabeled' event when label is Deleted from PR (#34316)
Some checks failed
release-nightly / nightly-binary (push) Has been cancelled
release-nightly / nightly-docker-rootful (push) Has been cancelled
release-nightly / nightly-docker-rootless (push) Has been cancelled

This pull request updates the handling of issue label events in
workflows to distinguish between label additions and deletions,
introduces corresponding test cases, and extends the `IssuePayload`
structure to support this functionality.

### Enhancements to issue label event handling:
* Updated `matchIssuesEvent` in `modules/actions/workflows.go` to
differentiate between "labeled" and "unlabeled" events based on whether
labels were added or removed.
* Added a new field, `RemovedLabels`, to the `IssuePayload` struct in
`modules/structs/hook.go` to track labels that were removed during an
issue event.

### Testing improvements:
* Added `TestMatchIssuesEvent` in `modules/actions/workflows_test.go` to
cover scenarios such as label addition, label deletion, and label
clearing, ensuring the correct event type is triggered.

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
Sumit
2025-09-24 22:15:38 +05:30
committed by GitHub
parent 0b706b0825
commit f09bea7af1
4 changed files with 249 additions and 25 deletions

View File

@@ -169,7 +169,7 @@ func (n *actionsNotifier) IssueChangeAssignee(ctx context.Context, doer *user_mo
hookEvent = webhook_module.HookEventPullRequestAssign
}
notifyIssueChange(ctx, doer, issue, hookEvent, action)
notifyIssueChange(ctx, doer, issue, hookEvent, action, nil, nil)
}
// IssueChangeMilestone notifies assignee to notifiers
@@ -188,11 +188,11 @@ func (n *actionsNotifier) IssueChangeMilestone(ctx context.Context, doer *user_m
hookEvent = webhook_module.HookEventPullRequestMilestone
}
notifyIssueChange(ctx, doer, issue, hookEvent, action)
notifyIssueChange(ctx, doer, issue, hookEvent, action, nil, nil)
}
func (n *actionsNotifier) IssueChangeLabels(ctx context.Context, doer *user_model.User, issue *issues_model.Issue,
_, _ []*issues_model.Label,
addedLabels, removedLabels []*issues_model.Label,
) {
ctx = withMethod(ctx, "IssueChangeLabels")
@@ -201,10 +201,10 @@ func (n *actionsNotifier) IssueChangeLabels(ctx context.Context, doer *user_mode
hookEvent = webhook_module.HookEventPullRequestLabel
}
notifyIssueChange(ctx, doer, issue, hookEvent, api.HookIssueLabelUpdated)
notifyIssueChange(ctx, doer, issue, hookEvent, api.HookIssueLabelUpdated, addedLabels, removedLabels)
}
func notifyIssueChange(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, event webhook_module.HookEventType, action api.HookIssueAction) {
func notifyIssueChange(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, event webhook_module.HookEventType, action api.HookIssueAction, addedLabels, removedLabels []*issues_model.Label) {
var err error
if err = issue.LoadRepo(ctx); err != nil {
log.Error("LoadRepo: %v", err)
@@ -216,34 +216,65 @@ func notifyIssueChange(ctx context.Context, doer *user_model.User, issue *issues
return
}
var addedAPILabels []*api.Label
if addedLabels != nil {
addedAPILabels = make([]*api.Label, 0, len(addedLabels))
for _, label := range addedLabels {
addedAPILabels = append(addedAPILabels, convert.ToLabel(label, issue.Repo, doer))
}
}
// Get removed labels from context if present
var removedAPILabels []*api.Label
if removedLabels != nil {
removedAPILabels = make([]*api.Label, 0, len(removedLabels))
for _, label := range removedLabels {
removedAPILabels = append(removedAPILabels, convert.ToLabel(label, issue.Repo, doer))
}
}
if issue.IsPull {
if err = issue.LoadPullRequest(ctx); err != nil {
log.Error("loadPullRequest: %v", err)
return
}
payload := &api.PullRequestPayload{
Action: action,
Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil),
Repository: convert.ToRepo(ctx, issue.Repo, access_model.Permission{AccessMode: perm_model.AccessModeNone}),
Sender: convert.ToUser(ctx, doer, nil),
Changes: &api.ChangesPayload{
AddedLabels: addedAPILabels,
RemovedLabels: removedAPILabels,
},
}
newNotifyInputFromIssue(issue, event).
WithDoer(doer).
WithPayload(&api.PullRequestPayload{
Action: action,
Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil),
Repository: convert.ToRepo(ctx, issue.Repo, access_model.Permission{AccessMode: perm_model.AccessModeNone}),
Sender: convert.ToUser(ctx, doer, nil),
}).
WithPayload(payload).
WithPullRequest(issue.PullRequest).
Notify(ctx)
return
}
permission, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster)
payload := &api.IssuePayload{
Action: action,
Index: issue.Index,
Issue: convert.ToAPIIssue(ctx, doer, issue),
Repository: convert.ToRepo(ctx, issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil),
Changes: &api.ChangesPayload{
AddedLabels: addedAPILabels,
RemovedLabels: removedAPILabels,
},
}
newNotifyInputFromIssue(issue, event).
WithDoer(doer).
WithPayload(&api.IssuePayload{
Action: action,
Index: issue.Index,
Issue: convert.ToAPIIssue(ctx, doer, issue),
Repository: convert.ToRepo(ctx, issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil),
}).
WithPayload(payload).
Notify(ctx)
}