Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix actions notify bug #31866

Merged
merged 5 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion services/actions/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ func (n *actionsNotifier) ForkRepository(ctx context.Context, doer *user_model.U
// Add to hook queue for created repo after session commit.
if u.IsOrganization() {
newNotifyInput(repo, doer, webhook_module.HookEventRepository).
WithRef(oldRepo.DefaultBranch).
WithRef(git.RefNameFromBranch(oldRepo.DefaultBranch).String()).
WithPayload(&api.RepositoryPayload{
Action: api.HookRepoCreated,
Repository: convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeOwner}),
Expand Down
25 changes: 15 additions & 10 deletions services/actions/notifier_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ type notifyInput struct {
Event webhook_module.HookEventType

// optional
Ref string
Ref git.RefName
Payload api.Payloader
PullRequest *issues_model.PullRequest
}
Expand All @@ -89,7 +89,7 @@ func (input *notifyInput) WithDoer(doer *user_model.User) *notifyInput {
}

func (input *notifyInput) WithRef(ref string) *notifyInput {
input.Ref = ref
input.Ref = git.RefName(ref)
return input
}

Expand All @@ -101,7 +101,7 @@ func (input *notifyInput) WithPayload(payload api.Payloader) *notifyInput {
func (input *notifyInput) WithPullRequest(pr *issues_model.PullRequest) *notifyInput {
input.PullRequest = pr
if input.Ref == "" {
input.Ref = pr.GetGitRefName()
input.Ref = git.RefName(pr.GetGitRefName())
}
return input
}
Expand Down Expand Up @@ -144,20 +144,25 @@ func notify(ctx context.Context, input *notifyInput) error {
defer gitRepo.Close()

ref := input.Ref
if ref != input.Repo.DefaultBranch && actions_module.IsDefaultBranchWorkflow(input.Event) {
if ref.BranchName() != input.Repo.DefaultBranch && actions_module.IsDefaultBranchWorkflow(input.Event) {
if ref != "" {
log.Warn("Event %q should only trigger workflows on the default branch, but its ref is %q. Will fall back to the default branch",
input.Event, ref)
}
ref = input.Repo.DefaultBranch
ref = git.RefNameFromBranch(input.Repo.DefaultBranch)
}
if ref == "" {
log.Warn("Ref of event %q is empty, will fall back to the default branch", input.Event)
ref = input.Repo.DefaultBranch
ref = git.RefNameFromBranch(input.Repo.DefaultBranch)
}

commitID, err := gitRepo.GetRefCommitID(ref.String())
if err != nil {
return fmt.Errorf("gitRepo.GetRefCommitID: %w", err)
}

// Get the commit object for the ref
commit, err := gitRepo.GetCommit(ref)
commit, err := gitRepo.GetCommit(commitID)
if err != nil {
return fmt.Errorf("gitRepo.GetCommit: %w", err)
}
Expand All @@ -168,7 +173,7 @@ func notify(ctx context.Context, input *notifyInput) error {

var detectedWorkflows []*actions_module.DetectedWorkflow
actionsConfig := input.Repo.MustGetUnit(ctx, unit_model.TypeActions).ActionsConfig()
shouldDetectSchedules := input.Event == webhook_module.HookEventPush && git.RefName(input.Ref).BranchName() == input.Repo.DefaultBranch
shouldDetectSchedules := input.Event == webhook_module.HookEventPush && input.Ref.BranchName() == input.Repo.DefaultBranch
workflows, schedules, err := actions_module.DetectWorkflows(gitRepo, commit,
input.Event,
input.Payload,
Expand Down Expand Up @@ -220,12 +225,12 @@ func notify(ctx context.Context, input *notifyInput) error {
}

if shouldDetectSchedules {
if err := handleSchedules(ctx, schedules, commit, input, ref); err != nil {
if err := handleSchedules(ctx, schedules, commit, input, ref.String()); err != nil {
return err
}
}

return handleWorkflows(ctx, detectedWorkflows, commit, input, ref)
return handleWorkflows(ctx, detectedWorkflows, commit, input, ref.String())
}

func skipWorkflows(input *notifyInput, commit *git.Commit) bool {
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/actions_trigger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ func TestCreateDeleteRefEvent(t *testing.T) {
Title: "add workflow",
RepoID: repo.ID,
Event: "delete",
Ref: "main",
Ref: "refs/heads/main",
WorkflowID: "createdelete.yml",
CommitSHA: branch.CommitID,
})
Expand All @@ -442,7 +442,7 @@ func TestCreateDeleteRefEvent(t *testing.T) {
Title: "add workflow",
RepoID: repo.ID,
Event: "delete",
Ref: "main",
Ref: "refs/heads/main",
WorkflowID: "createdelete.yml",
CommitSHA: branch.CommitID,
})
Expand Down