fix: labels are missing in the pull request payload removing a label

When ReplaceIssueLabels calls issue.LoadLabels it was a noop because
issue.isLabelsLoaded is still set to true because of the call  to
issue.LoadLabels that was done at the beginning of the function.
This commit is contained in:
Earl Warren 2024-11-06 15:36:02 +01:00
parent 0fb48872ac
commit c801838690
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
2 changed files with 133 additions and 56 deletions

View file

@ -496,6 +496,7 @@ func ReplaceIssueLabels(ctx context.Context, issue *Issue, labels []*Label, doer
} }
} }
issue.isLabelsLoaded = false
issue.Labels = nil issue.Labels = nil
if err = issue.LoadLabels(ctx); err != nil { if err = issue.LoadLabels(ctx); err != nil {
return err return err

View file

@ -84,7 +84,23 @@ jobs:
baseGitRepo.Close() baseGitRepo.Close()
}() }()
// prepare the pull request // prepare the repository labels
labelStr := "/api/v1/repos/user2/repo-pull-request/labels"
labelsCount := 2
labels := make([]*api.Label, labelsCount)
for i := 0; i < labelsCount; i++ {
color := "abcdef"
req := NewRequestWithJSON(t, "POST", labelStr, &api.CreateLabelOption{
Name: fmt.Sprintf("label%d", i),
Color: color,
}).AddTokenAuth(token)
resp := MakeRequest(t, req, http.StatusCreated)
labels[i] = new(api.Label)
DecodeJSON(t, resp, &labels[i])
assert.Equal(t, color, labels[i].Color)
}
// create the pull request
testEditFileToNewBranch(t, session, "user2", "repo-pull-request", "main", "wip-something", "README.md", "Hello, world 1") testEditFileToNewBranch(t, session, "user2", "repo-pull-request", "main", "wip-something", "README.md", "Hello, world 1")
testPullCreate(t, session, "user2", "repo-pull-request", true, "main", "wip-something", "Commit status PR") testPullCreate(t, session, "user2", "repo-pull-request", true, "main", "wip-something", "Commit status PR")
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: baseRepo.ID}) pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: baseRepo.ID})
@ -94,24 +110,15 @@ jobs:
issueURL := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%s", "user2", "repo-pull-request", fmt.Sprintf("%d", pr.Issue.Index)) issueURL := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%s", "user2", "repo-pull-request", fmt.Sprintf("%d", pr.Issue.Index))
// prepare the labels // prepare the labels
labelStr := "/api/v1/repos/user2/repo-pull-request/labels"
req := NewRequestWithJSON(t, "POST", labelStr, &api.CreateLabelOption{
Name: "mylabel",
Color: "abcdef",
Description: "description mylabel",
}).AddTokenAuth(token)
resp := MakeRequest(t, req, http.StatusCreated)
label := new(api.Label)
DecodeJSON(t, resp, &label)
labelURL := fmt.Sprintf("%s/labels", issueURL) labelURL := fmt.Sprintf("%s/labels", issueURL)
// prepare the milestone // prepare the milestone
milestoneStr := "/api/v1/repos/user2/repo-pull-request/milestones" milestoneStr := "/api/v1/repos/user2/repo-pull-request/milestones"
req = NewRequestWithJSON(t, "POST", milestoneStr, &api.CreateMilestoneOption{ req := NewRequestWithJSON(t, "POST", milestoneStr, &api.CreateMilestoneOption{
Title: "mymilestone", Title: "mymilestone",
State: "open", State: "open",
}).AddTokenAuth(token) }).AddTokenAuth(token)
resp = MakeRequest(t, req, http.StatusCreated) resp := MakeRequest(t, req, http.StatusCreated)
milestone := new(api.Milestone) milestone := new(api.Milestone)
DecodeJSON(t, resp, &milestone) DecodeJSON(t, resp, &milestone)
@ -128,47 +135,97 @@ jobs:
return false return false
} }
count := 0 assertActionRun := func(t *testing.T, sha, onType string, action api.HookIssueAction, actionRun *actions_model.ActionRun) {
assert.Equal(t, fmt.Sprintf("%s.yml", onType), actionRun.WorkflowID)
assert.Equal(t, sha, actionRun.CommitSHA)
assert.Equal(t, actions_module.GithubEventPullRequest, actionRun.TriggerEvent)
event, err := actionRun.GetPullRequestEventPayload()
require.NoError(t, err)
assert.Equal(t, action, event.Action)
}
type assertType func(t *testing.T, sha, onType string, action api.HookIssueAction, actionRuns []*actions_model.ActionRun)
assertActionRuns := func(t *testing.T, sha, onType string, action api.HookIssueAction, actionRuns []*actions_model.ActionRun) {
require.Len(t, actionRuns, 1)
assertActionRun(t, sha, onType, action, actionRuns[0])
}
for _, testCase := range []struct { for _, testCase := range []struct {
onType string onType string
jobID string jobID string
doSomething func() doSomething func()
actionRunCount int
action api.HookIssueAction action api.HookIssueAction
hasLabel bool assert assertType
}{ }{
{ {
onType: "opened", onType: "opened",
doSomething: func() {}, doSomething: func() {},
actionRunCount: 1,
action: api.HookIssueOpened, action: api.HookIssueOpened,
assert: assertActionRuns,
}, },
{ {
onType: "synchronize", onType: "synchronize",
doSomething: func() { doSomething: func() {
testEditFile(t, session, "user2", "repo-pull-request", "wip-something", "README.md", "Hello, world 2") testEditFile(t, session, "user2", "repo-pull-request", "wip-something", "README.md", "Hello, world 2")
}, },
actionRunCount: 1,
action: api.HookIssueSynchronized, action: api.HookIssueSynchronized,
assert: assertActionRuns,
}, },
{ {
onType: "labeled", onType: "labeled",
doSomething: func() { doSomething: func() {
req := NewRequestWithJSON(t, "POST", labelURL, &api.IssueLabelsOption{ req := NewRequestWithJSON(t, "POST", labelURL, &api.IssueLabelsOption{
Labels: []any{label.ID}, Labels: []any{labels[0].ID, labels[1].ID},
}).AddTokenAuth(token) }).AddTokenAuth(token)
MakeRequest(t, req, http.StatusOK) MakeRequest(t, req, http.StatusOK)
}, },
actionRunCount: 2,
action: api.HookIssueLabelUpdated, action: api.HookIssueLabelUpdated,
hasLabel: true, assert: func(t *testing.T, sha, onType string, action api.HookIssueAction, actionRuns []*actions_model.ActionRun) {
assertActionRun(t, sha, onType, api.HookIssueLabelUpdated, actionRuns[0])
assertActionRun(t, sha, onType, api.HookIssueLabelUpdated, actionRuns[1])
},
}, },
{ {
onType: "unlabeled", onType: "unlabeled",
doSomething: func() { doSomething: func() {
req := NewRequestWithJSON(t, "PUT", labelURL, &api.IssueLabelsOption{ req := NewRequestWithJSON(t, "PUT", labelURL, &api.IssueLabelsOption{
Labels: []any{}, Labels: []any{labels[0].ID},
}).AddTokenAuth(token) }).AddTokenAuth(token)
MakeRequest(t, req, http.StatusOK) MakeRequest(t, req, http.StatusOK)
}, },
actionRunCount: 3,
action: api.HookIssueLabelCleared, action: api.HookIssueLabelCleared,
hasLabel: true, assert: func(t *testing.T, sha, onType string, action api.HookIssueAction, actionRuns []*actions_model.ActionRun) {
foundPayloadWithLabels := false
knownLabels := []string{"label0", "label1"}
for _, actionRun := range actionRuns {
assert.Equal(t, sha, actionRun.CommitSHA)
assert.Equal(t, actions_module.GithubEventPullRequest, actionRun.TriggerEvent)
event, err := actionRun.GetPullRequestEventPayload()
require.NoError(t, err)
switch event.Action {
case api.HookIssueLabelUpdated:
assert.Equal(t, "labeled.yml", actionRun.WorkflowID)
assert.Equal(t, "label0", event.Label.Name)
require.Len(t, event.PullRequest.Labels, 1)
assert.Contains(t, "label0", event.PullRequest.Labels[0].Name)
case api.HookIssueLabelCleared:
assert.Equal(t, "unlabeled.yml", actionRun.WorkflowID)
assert.Contains(t, knownLabels, event.Label.Name)
if len(event.PullRequest.Labels) > 0 {
foundPayloadWithLabels = true
assert.Contains(t, knownLabels, event.PullRequest.Labels[0].Name)
}
default:
require.Fail(t, fmt.Sprintf("unexpected action '%s'", event.Action))
}
}
assert.True(t, foundPayloadWithLabels, "expected at least one clear label payload with non empty labels")
},
}, },
{ {
onType: "assigned", onType: "assigned",
@ -178,7 +235,9 @@ jobs:
}).AddTokenAuth(token) }).AddTokenAuth(token)
MakeRequest(t, req, http.StatusCreated) MakeRequest(t, req, http.StatusCreated)
}, },
actionRunCount: 1,
action: api.HookIssueAssigned, action: api.HookIssueAssigned,
assert: assertActionRuns,
}, },
{ {
onType: "unassigned", onType: "unassigned",
@ -188,7 +247,9 @@ jobs:
}).AddTokenAuth(token) }).AddTokenAuth(token)
MakeRequest(t, req, http.StatusCreated) MakeRequest(t, req, http.StatusCreated)
}, },
actionRunCount: 1,
action: api.HookIssueUnassigned, action: api.HookIssueUnassigned,
assert: assertActionRuns,
}, },
{ {
onType: "milestoned", onType: "milestoned",
@ -198,7 +259,9 @@ jobs:
}).AddTokenAuth(token) }).AddTokenAuth(token)
MakeRequest(t, req, http.StatusCreated) MakeRequest(t, req, http.StatusCreated)
}, },
actionRunCount: 1,
action: api.HookIssueMilestoned, action: api.HookIssueMilestoned,
assert: assertActionRuns,
}, },
{ {
onType: "demilestoned", onType: "demilestoned",
@ -209,7 +272,9 @@ jobs:
}).AddTokenAuth(token) }).AddTokenAuth(token)
MakeRequest(t, req, http.StatusCreated) MakeRequest(t, req, http.StatusCreated)
}, },
actionRunCount: 1,
action: api.HookIssueDemilestoned, action: api.HookIssueDemilestoned,
assert: assertActionRuns,
}, },
{ {
onType: "closed", onType: "closed",
@ -219,7 +284,9 @@ jobs:
err = issue_service.ChangeStatus(db.DefaultContext, pr.Issue, user2, sha, true) err = issue_service.ChangeStatus(db.DefaultContext, pr.Issue, user2, sha, true)
require.NoError(t, err) require.NoError(t, err)
}, },
actionRunCount: 1,
action: api.HookIssueClosed, action: api.HookIssueClosed,
assert: assertActionRuns,
}, },
{ {
onType: "reopened", onType: "reopened",
@ -229,43 +296,52 @@ jobs:
err = issue_service.ChangeStatus(db.DefaultContext, pr.Issue, user2, sha, false) err = issue_service.ChangeStatus(db.DefaultContext, pr.Issue, user2, sha, false)
require.NoError(t, err) require.NoError(t, err)
}, },
actionRunCount: 1,
action: api.HookIssueReOpened, action: api.HookIssueReOpened,
assert: assertActionRuns,
}, },
} { } {
t.Run(testCase.onType, func(t *testing.T) { t.Run(testCase.onType, func(t *testing.T) {
defer func() {
// cleanup leftovers, start from scratch
_, err = db.DeleteByBean(db.DefaultContext, actions_model.ActionRun{RepoID: baseRepo.ID})
require.NoError(t, err)
_, err = db.DeleteByBean(db.DefaultContext, actions_model.ActionRunJob{RepoID: baseRepo.ID})
require.NoError(t, err)
}()
// trigger the onType event // trigger the onType event
testCase.doSomething() testCase.doSomething()
count++ count := testCase.actionRunCount
context := fmt.Sprintf("%[1]s / %[1]s (pull_request)", testCase.onType) context := fmt.Sprintf("%[1]s / %[1]s (pull_request)", testCase.onType)
// wait for a new ActionRun to be created var actionRuns []*actions_model.ActionRun
assert.Eventually(t, func() bool {
return count == unittest.GetCount(t, &actions_model.ActionRun{RepoID: baseRepo.ID}) // wait for ActionRun(s) to be created
require.Eventually(t, func() bool {
actionRuns = make([]*actions_model.ActionRun, 0)
require.NoError(t, db.GetEngine(db.DefaultContext).Where("repo_id=?", baseRepo.ID).Find(&actionRuns))
return assert.Len(t, actionRuns, count)
}, 30*time.Second, 1*time.Second) }, 30*time.Second, 1*time.Second)
// verify the expected ActionRun was created // verify the expected ActionRuns were created
sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName())
require.NoError(t, err) require.NoError(t, err)
actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, WorkflowID: fmt.Sprintf("%s.yml", testCase.onType)})
assert.Equal(t, sha, actionRun.CommitSHA)
assert.Equal(t, actions_module.GithubEventPullRequest, actionRun.TriggerEvent)
event, err := actionRun.GetPullRequestEventPayload()
if testCase.hasLabel {
assert.NotNil(t, event.Label)
}
require.NoError(t, err)
assert.Equal(t, testCase.action, event.Action)
// verify the expected ActionRunJob was created and is StatusWaiting
job := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{JobID: testCase.onType, CommitSHA: sha})
assert.Equal(t, actions_model.StatusWaiting, job.Status)
// verify the commit status changes to CommitStatusSuccess when the job changes to StatusSuccess // verify the commit status changes to CommitStatusSuccess when the job changes to StatusSuccess
assert.True(t, checkCommitStatus(sha, context, api.CommitStatusPending)) assert.True(t, checkCommitStatus(sha, context, api.CommitStatusPending))
for _, actionRun := range actionRuns {
// verify the expected ActionRunJob was created and is StatusWaiting
job := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: actionRun.ID, CommitSHA: sha})
assert.Equal(t, actions_model.StatusWaiting, job.Status)
// change the state of the job to success
job.Status = actions_model.StatusSuccess job.Status = actions_model.StatusSuccess
actions_service.CreateCommitStatus(db.DefaultContext, job) actions_service.CreateCommitStatus(db.DefaultContext, job)
}
// verify the commit status changed to CommitStatusSuccess because the job(s) changed to StatusSuccess
assert.True(t, checkCommitStatus(sha, context, api.CommitStatusSuccess)) assert.True(t, checkCommitStatus(sha, context, api.CommitStatusSuccess))
testCase.assert(t, sha, testCase.onType, testCase.action, actionRuns)
}) })
} }
}) })