Merge 8b6f3ba1e93a2b2deb64d99df0ba6382ec4a03da into b07e03956af8f29464067b19cb5cacee358b592f

modules/actions/workflows.go

@@ -362,21 +362,35 @@ 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.Issue.Labels) > 0 {
+					actions = append(actions, "labeled")
+				}
+				if len(issuePayload.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)) {
-					matchTimes++
+				for _, action := range actions {
+					if glob.MustCompile(val, '/').Match(action) {
+						matchTimes++
+						break
+					}
+				}
+				// Once a match is found for this value, we can move to the next one
+				if matchTimes > 0 {
 					break
 				}
 			}

modules/actions/workflows_test.go

@@ -136,3 +136,159 @@ 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{},
+				},
+				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"},
+					},
+				},
+				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"},
+					},
+				},
+				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"},
+					},
+				},
+				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"},
+					},
+				},
+				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"},
+					},
+				},
+				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")
+		})
+	}
+}

modules/structs/hook.go

@@ -310,13 +310,14 @@ const (
 
 // IssuePayload represents the payload information that is sent along with an issue event.
 type IssuePayload struct {
-	Action     HookIssueAction `json:"action"`
-	Index      int64           `json:"number"`
-	Changes    *ChangesPayload `json:"changes,omitempty"`
-	Issue      *Issue          `json:"issue"`
-	Repository *Repository     `json:"repository"`
-	Sender     *User           `json:"sender"`
-	CommitID   string          `json:"commit_id"`
+	Action        HookIssueAction `json:"action"`
+	Index         int64           `json:"number"`
+	Changes       *ChangesPayload `json:"changes,omitempty"`
+	RemovedLabels []*Label        `json:"removed_labels"`
+	Issue         *Issue          `json:"issue"`
+	Repository    *Repository     `json:"repository"`
+	Sender        *User           `json:"sender"`
+	CommitID      string          `json:"commit_id"`
 }
 
 // JSONPayload encodes the IssuePayload to JSON, with an indentation of two spaces.
@@ -341,6 +342,7 @@ type PullRequestPayload struct {
 	Action            HookIssueAction `json:"action"`
 	Index             int64           `json:"number"`
 	Changes           *ChangesPayload `json:"changes,omitempty"`
+	RemovedLabels     []*Label        `json:"removed_labels"`
 	PullRequest       *PullRequest    `json:"pull_request"`
 	RequestedReviewer *User           `json:"requested_reviewer"`
 	Repository        *Repository     `json:"repository"`

services/actions/notifier.go

@@ -26,6 +26,12 @@ type actionsNotifier struct {
 	notify_service.NullNotifier
 }
 
+type contextKey string
+
+const (
+	removedLabelsKey contextKey = "GiteaRemovedLabels"
+)
+
 var _ notify_service.Notifier = &actionsNotifier{}
 
 // NewNotifier create a new actionsNotifier notifier
@@ -189,7 +195,7 @@ func (n *actionsNotifier) IssueChangeMilestone(ctx context.Context, doer *user_m
 }
 
 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")
 
@@ -198,6 +204,22 @@ func (n *actionsNotifier) IssueChangeLabels(ctx context.Context, doer *user_mode
 		hookEvent = webhook_module.HookEventPullRequestLabel
 	}
 
+	// Track removedLabels for the webhook payload
+	var removedAPILabels []*api.Label
+	if len(removedLabels) > 0 {
+		if err := issue.LoadRepo(ctx); err != nil {
+			log.Error("LoadRepo: %v", err)
+			return
+		}
+
+		removedAPILabels = make([]*api.Label, 0, len(removedLabels))
+		for _, label := range removedLabels {
+			removedAPILabels = append(removedAPILabels, convert.ToLabel(label, issue.Repo, doer))
+		}
+
+		ctx = context.WithValue(ctx, removedLabelsKey, removedAPILabels)
+	}
+
 	notifyIssueChange(ctx, doer, issue, hookEvent, api.HookIssueLabelUpdated)
 }
 
@@ -213,34 +235,50 @@ func notifyIssueChange(ctx context.Context, doer *user_model.User, issue *issues
 		return
 	}
 
+	// Get removed labels from context if present
+	var removedAPILabels []*api.Label
+	if removedLabelsValue := ctx.Value(removedLabelsKey); removedLabelsValue != nil {
+		if labels, ok := removedLabelsValue.([]*api.Label); ok {
+			removedAPILabels = labels
+		}
+	}
+
 	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),
+			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),
+		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)
 }