Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf09bea7

Browse files
Sumit189lunny
andauthored
[Fix] Trigger 'unlabeled' event when label is Deleted from PR (#34316)
This pull request updates the handling of issue label events inworkflows 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` todifferentiate between "labeled" and "unlabeled" events based on whetherlabels 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 anissue event.### Testing improvements:* Added `TestMatchIssuesEvent` in `modules/actions/workflows_test.go` tocover scenarios such as label addition, label deletion, and labelclearing, ensuring the correct event type is triggered.---------Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
1 parent0b706b0 commitf09bea7

File tree

4 files changed

+249
-25
lines changed

4 files changed

+249
-25
lines changed

‎modules/actions/workflows.go‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -377,20 +377,28 @@ 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-
switchaction {
385+
actions:=[]string{}
386+
switchissuePayload.Action {
387387
caseapi.HookIssueLabelUpdated:
388-
action="labeled"
388+
iflen(issuePayload.Changes.AddedLabels)>0 {
389+
actions=append(actions,"labeled")
390+
}
391+
iflen(issuePayload.Changes.RemovedLabels)>0 {
392+
actions=append(actions,"unlabeled")
393+
}
389394
caseapi.HookIssueLabelCleared:
390-
action="unlabeled"
395+
actions=append(actions,"unlabeled")
396+
default:
397+
actions=append(actions,string(issuePayload.Action))
391398
}
399+
392400
for_,val:=rangevals {
393-
ifglob.MustCompile(val,'/').Match(string(action)) {
401+
ifslices.ContainsFunc(actions,glob.MustCompile(val,'/').Match) {
394402
matchTimes++
395403
break
396404
}

‎modules/actions/workflows_test.go‎

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

‎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-
funcnotifyIssueChange(ctx context.Context,doer*user_model.User,issue*issues_model.Issue,event webhook_module.HookEventType,action api.HookIssueAction) {
207+
funcnotifyIssueChange(ctx context.Context,doer*user_model.User,issue*issues_model.Issue,event webhook_module.HookEventType,action api.HookIssueAction,addedLabels,removedLabels []*issues_model.Label) {
208208
varerrerror
209209
iferr=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+
varaddedAPILabels []*api.Label
220+
ifaddedLabels!=nil {
221+
addedAPILabels=make([]*api.Label,0,len(addedLabels))
222+
for_,label:=rangeaddedLabels {
223+
addedAPILabels=append(addedAPILabels,convert.ToLabel(label,issue.Repo,doer))
224+
}
225+
}
226+
227+
// Get removed labels from context if present
228+
varremovedAPILabels []*api.Label
229+
ifremovedLabels!=nil {
230+
removedAPILabels=make([]*api.Label,0,len(removedLabels))
231+
for_,label:=rangeremovedLabels {
232+
removedAPILabels=append(removedAPILabels,convert.ToLabel(label,issue.Repo,doer))
233+
}
234+
}
235+
219236
ifissue.IsPull {
220237
iferr=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)
[8]ページ先頭
©2009-2025 Movatter.jp