Skip to content

Commit 67d3335

Browse files
lunnylunny
committed
Merge branch 'label_delete_event_fix' of github.com:Sumit189/gitea into Sumit189-label_delete_event_fix
2 parents 87362b4 + 6aa8ba3 commit 67d3335

File tree

4 files changed

+258
-26
lines changed

4 files changed

+258
-26
lines changed

‎modules/actions/workflows.go‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -377,21 +377,35 @@ func matchIssuesEvent(issuePayload *api.IssuePayload, evt *jobparser.Event) bool
377377
// Actions with the same name:
378378
// opened, edited, closed, reopened, assigned, unassigned, milestoned, demilestoned
379379
// Actions need to be converted:
380-
// label_updated -> labeled
380+
// label_updated -> labeled (when adding) or unlabeled (when removing)
381381
// label_cleared -> unlabeled
382382
// Unsupported activity types:
383383
// deleted, transferred, pinned, unpinned, locked, unlocked
384384

385-
action := issuePayload.Action
386-
switch action {
385+
actions := []string{}
386+
switch issuePayload.Action {
387387
case api.HookIssueLabelUpdated:
388-
action = "labeled"
388+
if len(issuePayload.Changes.AddedLabels) > 0 {
389+
actions = append(actions, "labeled")
390+
}
391+
if len(issuePayload.Changes.RemovedLabels) > 0 {
392+
actions = append(actions, "unlabeled")
393+
}
389394
case api.HookIssueLabelCleared:
390-
action = "unlabeled"
395+
actions = append(actions, "unlabeled")
396+
default:
397+
actions = append(actions, string(issuePayload.Action))
391398
}
399+
392400
for _, val := range vals {
393-
if glob.MustCompile(val, '/').Match(string(action)) {
394-
matchTimes++
401+
for _, action := range actions {
402+
if glob.MustCompile(val, '/').Match(action) {
403+
matchTimes++
404+
break
405+
}
406+
}
407+
// Once a match is found for this value, we can move to the next one
408+
if matchTimes > 0 {
395409
break
396410
}
397411
}

‎modules/actions/workflows_test.go‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,3 +154,186 @@ func TestDetectMatched(t *testing.T) {
154154
})
155155
}
156156
}
157+
158+
func TestMatchIssuesEvent(t *testing.T) {
159+
testCases := []struct {
160+
desc string
161+
payload *api.IssuePayload
162+
yamlOn string
163+
expected bool
164+
eventType string
165+
}{
166+
{
167+
desc: "Label deletion should trigger unlabeled event",
168+
payload: &api.IssuePayload{
169+
Action: api.HookIssueLabelUpdated,
170+
Issue: &api.Issue{
171+
Labels: []*api.Label{},
172+
},
173+
Changes: &api.ChangesPayload{
174+
RemovedLabels: []*api.Label{
175+
{ID: 123, Name: "deleted-label"},
176+
},
177+
},
178+
},
179+
yamlOn: "on:\n issues:\n types: [unlabeled]",
180+
expected: true,
181+
eventType: "unlabeled",
182+
},
183+
{
184+
desc: "Label deletion with existing labels should trigger unlabeled event",
185+
payload: &api.IssuePayload{
186+
Action: api.HookIssueLabelUpdated,
187+
Issue: &api.Issue{
188+
Labels: []*api.Label{
189+
{ID: 456, Name: "existing-label"},
190+
},
191+
},
192+
Changes: &api.ChangesPayload{
193+
AddedLabels: []*api.Label{
194+
{ID: 456, Name: "existing-label"},
195+
},
196+
RemovedLabels: []*api.Label{
197+
{ID: 123, Name: "deleted-label"},
198+
},
199+
},
200+
},
201+
yamlOn: "on:\n issues:\n types: [unlabeled]",
202+
expected: true,
203+
eventType: "unlabeled",
204+
},
205+
{
206+
desc: "Label addition should trigger labeled event",
207+
payload: &api.IssuePayload{
208+
Action: api.HookIssueLabelUpdated,
209+
Issue: &api.Issue{
210+
Labels: []*api.Label{
211+
{ID: 123, Name: "new-label"},
212+
},
213+
},
214+
Changes: &api.ChangesPayload{
215+
AddedLabels: []*api.Label{
216+
{ID: 123, Name: "new-label"},
217+
},
218+
RemovedLabels: []*api.Label{}, // Empty array, no labels removed
219+
},
220+
},
221+
yamlOn: "on:\n issues:\n types: [labeled]",
222+
expected: true,
223+
eventType: "labeled",
224+
},
225+
{
226+
desc: "Label clear should trigger unlabeled event",
227+
payload: &api.IssuePayload{
228+
Action: api.HookIssueLabelCleared,
229+
Issue: &api.Issue{
230+
Labels: []*api.Label{},
231+
},
232+
},
233+
yamlOn: "on:\n issues:\n types: [unlabeled]",
234+
expected: true,
235+
eventType: "unlabeled",
236+
},
237+
{
238+
desc: "Both adding and removing labels should trigger labeled event",
239+
payload: &api.IssuePayload{
240+
Action: api.HookIssueLabelUpdated,
241+
Issue: &api.Issue{
242+
Labels: []*api.Label{
243+
{ID: 789, Name: "new-label"},
244+
},
245+
},
246+
Changes: &api.ChangesPayload{
247+
AddedLabels: []*api.Label{
248+
{ID: 789, Name: "new-label"},
249+
},
250+
RemovedLabels: []*api.Label{
251+
{ID: 123, Name: "deleted-label"},
252+
},
253+
},
254+
},
255+
yamlOn: "on:\n issues:\n types: [labeled]",
256+
expected: true,
257+
eventType: "labeled",
258+
},
259+
{
260+
desc: "Both adding and removing labels should trigger unlabeled event",
261+
payload: &api.IssuePayload{
262+
Action: api.HookIssueLabelUpdated,
263+
Issue: &api.Issue{
264+
Labels: []*api.Label{
265+
{ID: 789, Name: "new-label"},
266+
},
267+
},
268+
Changes: &api.ChangesPayload{
269+
AddedLabels: []*api.Label{
270+
{ID: 789, Name: "new-label"},
271+
},
272+
RemovedLabels: []*api.Label{
273+
{ID: 123, Name: "deleted-label"},
274+
},
275+
},
276+
},
277+
yamlOn: "on:\n issues:\n types: [unlabeled]",
278+
expected: true,
279+
eventType: "unlabeled",
280+
},
281+
{
282+
desc: "Both adding and removing labels should trigger both events",
283+
payload: &api.IssuePayload{
284+
Action: api.HookIssueLabelUpdated,
285+
Issue: &api.Issue{
286+
Labels: []*api.Label{
287+
{ID: 789, Name: "new-label"},
288+
},
289+
},
290+
Changes: &api.ChangesPayload{
291+
AddedLabels: []*api.Label{
292+
{ID: 789, Name: "new-label"},
293+
},
294+
RemovedLabels: []*api.Label{
295+
{ID: 123, Name: "deleted-label"},
296+
},
297+
},
298+
},
299+
yamlOn: "on:\n issues:\n types: [labeled, unlabeled]",
300+
expected: true,
301+
eventType: "multiple",
302+
},
303+
}
304+
305+
for _, tc := range testCases {
306+
t.Run(tc.desc, func(t *testing.T) {
307+
evts, err := GetEventsFromContent([]byte(tc.yamlOn))
308+
assert.NoError(t, err)
309+
assert.Len(t, evts, 1)
310+
311+
// Test if the event matches as expected
312+
assert.Equal(t, tc.expected, matchIssuesEvent(tc.payload, evts[0]))
313+
314+
// For extra validation, check that action mapping works correctly
315+
if tc.eventType == "multiple" {
316+
// Skip direct action mapping validation for multiple events case
317+
// as one action can map to multiple event types
318+
return
319+
}
320+
321+
// Determine expected action for single event case
322+
var expectedAction string
323+
switch tc.payload.Action {
324+
case api.HookIssueLabelUpdated:
325+
if tc.eventType == "labeled" {
326+
expectedAction = "labeled"
327+
} else if tc.eventType == "unlabeled" {
328+
expectedAction = "unlabeled"
329+
}
330+
case api.HookIssueLabelCleared:
331+
expectedAction = "unlabeled"
332+
default:
333+
expectedAction = string(tc.payload.Action)
334+
}
335+
336+
assert.Equal(t, expectedAction, tc.eventType, "Event type should match expected")
337+
})
338+
}
339+
}

‎modules/structs/hook.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,10 @@ type ChangesPayload struct {
418418
Body *ChangesFromPayload `json:"body,omitempty"`
419419
// Changes made to the reference
420420
Ref *ChangesFromPayload `json:"ref,omitempty"`
421+
// Changes made to the labels added
422+
AddedLabels []*Label `json:"added_labels"`
423+
// Changes made to the labels removed
424+
RemovedLabels []*Label `json:"removed_labels"`
421425
}
422426

423427
// PullRequestPayload represents a payload information of pull request event.

‎services/actions/notifier.go‎

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ func (n *actionsNotifier) IssueChangeAssignee(ctx context.Context, doer *user_mo
169169
hookEvent = webhook_module.HookEventPullRequestAssign
170170
}
171171

172-
notifyIssueChange(ctx, doer, issue, hookEvent, action)
172+
notifyIssueChange(ctx, doer, issue, hookEvent, action, nil, nil)
173173
}
174174

175175
// IssueChangeMilestone notifies assignee to notifiers
@@ -188,11 +188,11 @@ func (n *actionsNotifier) IssueChangeMilestone(ctx context.Context, doer *user_m
188188
hookEvent = webhook_module.HookEventPullRequestMilestone
189189
}
190190

191-
notifyIssueChange(ctx, doer, issue, hookEvent, action)
191+
notifyIssueChange(ctx, doer, issue, hookEvent, action, nil, nil)
192192
}
193193

194194
func (n *actionsNotifier) IssueChangeLabels(ctx context.Context, doer *user_model.User, issue *issues_model.Issue,
195-
_, _ []*issues_model.Label,
195+
addedLabels, removedLabels []*issues_model.Label,
196196
) {
197197
ctx = withMethod(ctx, "IssueChangeLabels")
198198

@@ -201,10 +201,10 @@ func (n *actionsNotifier) IssueChangeLabels(ctx context.Context, doer *user_mode
201201
hookEvent = webhook_module.HookEventPullRequestLabel
202202
}
203203

204-
notifyIssueChange(ctx, doer, issue, hookEvent, api.HookIssueLabelUpdated)
204+
notifyIssueChange(ctx, doer, issue, hookEvent, api.HookIssueLabelUpdated, addedLabels, removedLabels)
205205
}
206206

207-
func notifyIssueChange(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, event webhook_module.HookEventType, action api.HookIssueAction) {
207+
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) {
208208
var err error
209209
if err = issue.LoadRepo(ctx); err != nil {
210210
log.Error("LoadRepo: %v", err)
@@ -216,34 +216,65 @@ func notifyIssueChange(ctx context.Context, doer *user_model.User, issue *issues
216216
return
217217
}
218218

219+
var addedAPILabels []*api.Label
220+
if addedLabels != nil {
221+
addedAPILabels = make([]*api.Label, 0, len(addedLabels))
222+
for _, label := range addedLabels {
223+
addedAPILabels = append(addedAPILabels, convert.ToLabel(label, issue.Repo, doer))
224+
}
225+
}
226+
227+
// Get removed labels from context if present
228+
var removedAPILabels []*api.Label
229+
if removedLabels != nil {
230+
removedAPILabels = make([]*api.Label, 0, len(removedLabels))
231+
for _, label := range removedLabels {
232+
removedAPILabels = append(removedAPILabels, convert.ToLabel(label, issue.Repo, doer))
233+
}
234+
}
235+
219236
if issue.IsPull {
220237
if err = issue.LoadPullRequest(ctx); err != nil {
221238
log.Error("loadPullRequest: %v", err)
222239
return
223240
}
241+
242+
payload := &api.PullRequestPayload{
243+
Action: action,
244+
Index: issue.Index,
245+
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil),
246+
Repository: convert.ToRepo(ctx, issue.Repo, access_model.Permission{AccessMode: perm_model.AccessModeNone}),
247+
Sender: convert.ToUser(ctx, doer, nil),
248+
Changes: &api.ChangesPayload{
249+
AddedLabels: addedAPILabels,
250+
RemovedLabels: removedAPILabels,
251+
},
252+
}
253+
224254
newNotifyInputFromIssue(issue, event).
225255
WithDoer(doer).
226-
WithPayload(&api.PullRequestPayload{
227-
Action: action,
228-
Index: issue.Index,
229-
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil),
230-
Repository: convert.ToRepo(ctx, issue.Repo, access_model.Permission{AccessMode: perm_model.AccessModeNone}),
231-
Sender: convert.ToUser(ctx, doer, nil),
232-
}).
256+
WithPayload(payload).
233257
WithPullRequest(issue.PullRequest).
234258
Notify(ctx)
235259
return
236260
}
261+
237262
permission, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster)
263+
payload := &api.IssuePayload{
264+
Action: action,
265+
Index: issue.Index,
266+
Issue: convert.ToAPIIssue(ctx, doer, issue),
267+
Repository: convert.ToRepo(ctx, issue.Repo, permission),
268+
Sender: convert.ToUser(ctx, doer, nil),
269+
Changes: &api.ChangesPayload{
270+
AddedLabels: addedAPILabels,
271+
RemovedLabels: removedAPILabels,
272+
},
273+
}
274+
238275
newNotifyInputFromIssue(issue, event).
239276
WithDoer(doer).
240-
WithPayload(&api.IssuePayload{
241-
Action: action,
242-
Index: issue.Index,
243-
Issue: convert.ToAPIIssue(ctx, doer, issue),
244-
Repository: convert.ToRepo(ctx, issue.Repo, permission),
245-
Sender: convert.ToUser(ctx, doer, nil),
246-
}).
277+
WithPayload(payload).
247278
Notify(ctx)
248279
}
249280

0 commit comments

Comments
 (0)