Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions modules/actions/workflows.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,20 +377,28 @@ func matchIssuesEvent(issuePayload *api.IssuePayload, evt *jobparser.Event) bool
// Actions with the same name:
// opened, edited, closed, reopened, assigned, unassigned, milestoned, demilestoned
// Actions need to be converted:
// label_updated -> labeled
// label_updated -> labeled (when adding) or unlabeled (when removing)
// label_cleared -> unlabeled
// Unsupported activity types:
// deleted, transferred, pinned, unpinned, locked, unlocked

action := issuePayload.Action
switch action {
actions := []string{}
switch issuePayload.Action {
case api.HookIssueLabelUpdated:
action = "labeled"
if len(issuePayload.Changes.AddedLabels) > 0 {
actions = append(actions, "labeled")
}
if len(issuePayload.Changes.RemovedLabels) > 0 {
actions = append(actions, "unlabeled")
}
case api.HookIssueLabelCleared:
action = "unlabeled"
actions = append(actions, "unlabeled")
default:
actions = append(actions, string(issuePayload.Action))
}

for _, val := range vals {
if glob.MustCompile(val, '/').Match(string(action)) {
if slices.ContainsFunc(actions, glob.MustCompile(val, '/').Match) {
matchTimes++
break
}
Expand Down
181 changes: 181 additions & 0 deletions modules/actions/workflows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,184 @@ func TestDetectMatched(t *testing.T) {
})
}
}

func TestMatchIssuesEvent(t *testing.T) {
testCases := []struct {
desc string
payload *api.IssuePayload
yamlOn string
expected bool
eventType string
}{
{
desc: "Label deletion should trigger unlabeled event",
payload: &api.IssuePayload{
Action: api.HookIssueLabelUpdated,
Issue: &api.Issue{
Labels: []*api.Label{},
},
Changes: &api.ChangesPayload{
RemovedLabels: []*api.Label{
{ID: 123, Name: "deleted-label"},
},
},
},
yamlOn: "on:\n issues:\n types: [unlabeled]",
expected: true,
eventType: "unlabeled",
},
{
desc: "Label deletion with existing labels should trigger unlabeled event",
payload: &api.IssuePayload{
Action: api.HookIssueLabelUpdated,
Issue: &api.Issue{
Labels: []*api.Label{
{ID: 456, Name: "existing-label"},
},
},
Changes: &api.ChangesPayload{
AddedLabels: nil,
RemovedLabels: []*api.Label{
{ID: 123, Name: "deleted-label"},
},
},
},
yamlOn: "on:\n issues:\n types: [unlabeled]",
expected: true,
eventType: "unlabeled",
},
{
desc: "Label addition should trigger labeled event",
payload: &api.IssuePayload{
Action: api.HookIssueLabelUpdated,
Issue: &api.Issue{
Labels: []*api.Label{
{ID: 123, Name: "new-label"},
},
},
Changes: &api.ChangesPayload{
AddedLabels: []*api.Label{
{ID: 123, Name: "new-label"},
},
RemovedLabels: []*api.Label{}, // Empty array, no labels removed
},
},
yamlOn: "on:\n issues:\n types: [labeled]",
expected: true,
eventType: "labeled",
},
{
desc: "Label clear should trigger unlabeled event",
payload: &api.IssuePayload{
Action: api.HookIssueLabelCleared,
Issue: &api.Issue{
Labels: []*api.Label{},
},
},
yamlOn: "on:\n issues:\n types: [unlabeled]",
expected: true,
eventType: "unlabeled",
},
{
desc: "Both adding and removing labels should trigger labeled event",
payload: &api.IssuePayload{
Action: api.HookIssueLabelUpdated,
Issue: &api.Issue{
Labels: []*api.Label{
{ID: 789, Name: "new-label"},
},
},
Changes: &api.ChangesPayload{
AddedLabels: []*api.Label{
{ID: 789, Name: "new-label"},
},
RemovedLabels: []*api.Label{
{ID: 123, Name: "deleted-label"},
},
},
},
yamlOn: "on:\n issues:\n types: [labeled]",
expected: true,
eventType: "labeled",
},
{
desc: "Both adding and removing labels should trigger unlabeled event",
payload: &api.IssuePayload{
Action: api.HookIssueLabelUpdated,
Issue: &api.Issue{
Labels: []*api.Label{
{ID: 789, Name: "new-label"},
},
},
Changes: &api.ChangesPayload{
AddedLabels: []*api.Label{
{ID: 789, Name: "new-label"},
},
RemovedLabels: []*api.Label{
{ID: 123, Name: "deleted-label"},
},
},
},
yamlOn: "on:\n issues:\n types: [unlabeled]",
expected: true,
eventType: "unlabeled",
},
{
desc: "Both adding and removing labels should trigger both events",
payload: &api.IssuePayload{
Action: api.HookIssueLabelUpdated,
Issue: &api.Issue{
Labels: []*api.Label{
{ID: 789, Name: "new-label"},
},
},
Changes: &api.ChangesPayload{
AddedLabels: []*api.Label{
{ID: 789, Name: "new-label"},
},
RemovedLabels: []*api.Label{
{ID: 123, Name: "deleted-label"},
},
},
},
yamlOn: "on:\n issues:\n types: [labeled, unlabeled]",
expected: true,
eventType: "multiple",
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
evts, err := GetEventsFromContent([]byte(tc.yamlOn))
assert.NoError(t, err)
assert.Len(t, evts, 1)

// Test if the event matches as expected
assert.Equal(t, tc.expected, matchIssuesEvent(tc.payload, evts[0]))

// For extra validation, check that action mapping works correctly
if tc.eventType == "multiple" {
// Skip direct action mapping validation for multiple events case
// as one action can map to multiple event types
return
}

// Determine expected action for single event case
var expectedAction string
switch tc.payload.Action {
case api.HookIssueLabelUpdated:
if tc.eventType == "labeled" {
expectedAction = "labeled"
} else if tc.eventType == "unlabeled" {
expectedAction = "unlabeled"
}
case api.HookIssueLabelCleared:
expectedAction = "unlabeled"
default:
expectedAction = string(tc.payload.Action)
}

assert.Equal(t, expectedAction, tc.eventType, "Event type should match expected")
})
}
}
4 changes: 4 additions & 0 deletions modules/structs/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,10 @@ type ChangesPayload struct {
Body *ChangesFromPayload `json:"body,omitempty"`
// Changes made to the reference
Ref *ChangesFromPayload `json:"ref,omitempty"`
// Changes made to the labels added
AddedLabels []*Label `json:"added_labels"`
// Changes made to the labels removed
RemovedLabels []*Label `json:"removed_labels"`
}

// PullRequestPayload represents a payload information of pull request event.
Expand Down
69 changes: 50 additions & 19 deletions services/actions/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")

Expand All @@ -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)
Expand All @@ -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)
}

Expand Down