diff --git a/internal/services/pullreq/handlers_branch.go b/internal/services/pullreq/handlers_branch.go index 3dceee943..89b97f404 100644 --- a/internal/services/pullreq/handlers_branch.go +++ b/internal/services/pullreq/handlers_branch.go @@ -24,6 +24,21 @@ import ( func (s *Service) triggerPREventOnBranchUpdate(ctx context.Context, event *events.Event[*gitevents.BranchUpdatedPayload], ) error { + // we should always update PR mergeable status check when target branch is updated. + // - main + // |- develop + // |- feature1 + // |- feature2 + // when feature2 merge changes into develop branch then feature1 branch is not consistent anymore + // and need to run mergeable check even nothing was changed on feature1, same applies to main if someone + // push new commit to main then develop should merge status should be unchecked. + if branch, err := getBranchFromRef(event.Payload.Ref); err == nil { + err = s.pullreqStore.UpdateMergeCheckStatus(ctx, event.Payload.RepoID, branch, enum.MergeCheckStatusUnchecked) + if err != nil { + return err + } + } + // TODO: This function is currently executed directly on branch update event. // TODO: But it should be executed after the PR's head ref has been updated. // TODO: This is to make sure the commit exists on the target repository for forked repositories. @@ -148,17 +163,11 @@ func (s *Service) forEveryOpenPR(ctx context.Context, repoID int64, ref string, fn func(pr *types.PullReq) error, ) { - const refPrefix = "refs/heads/" const largeLimit = 1000000 - if !strings.HasPrefix(ref, refPrefix) { - log.Ctx(ctx).Error().Msg("failed to get branch name from branch ref") - return - } - - branch := ref[len(refPrefix):] + branch, err := getBranchFromRef(ref) if len(branch) == 0 { - log.Ctx(ctx).Error().Msg("got an empty branch name from branch ref") + log.Ctx(ctx).Err(err).Send() return } @@ -182,3 +191,16 @@ func (s *Service) forEveryOpenPR(ctx context.Context, } } } + +func getBranchFromRef(ref string) (string, error) { + const refPrefix = "refs/heads/" + if !strings.HasPrefix(ref, refPrefix) { + return "", fmt.Errorf("failed to get branch name from branch ref %s", ref) + } + + branch := ref[len(refPrefix):] + if len(branch) == 0 { + return "", fmt.Errorf("got an empty branch name from branch ref %s", ref) + } + return branch, nil +} diff --git a/internal/store/database.go b/internal/store/database.go index 43027ea0c..fafd43224 100644 --- a/internal/store/database.go +++ b/internal/store/database.go @@ -285,6 +285,9 @@ type ( // It will set new values to the ActivitySeq, Version and Updated fields. UpdateActivitySeq(ctx context.Context, pr *types.PullReq) (*types.PullReq, error) + // Update all PR where target branch points to new SHA + UpdateMergeCheckStatus(ctx context.Context, targetRepo int64, targetBranch string, status enum.MergeCheckStatus) error + // Delete the pull request. Delete(ctx context.Context, id int64) error diff --git a/internal/store/database/pullreq.go b/internal/store/database/pullreq.go index 8ad4bbd44..866575daa 100644 --- a/internal/store/database/pullreq.go +++ b/internal/store/database/pullreq.go @@ -335,6 +335,30 @@ func (s *PullReqStore) UpdateActivitySeq(ctx context.Context, pr *types.PullReq) }) } +// UpdateMergeCheckStatus updates the pull request's mergeability status for all pr which target branch points to targetBranch. +func (s *PullReqStore) UpdateMergeCheckStatus(ctx context.Context, targetRepo int64, targetBranch string, status enum.MergeCheckStatus) error { + const query = ` + UPDATE pullreqs + SET + pullreq_updated = $1 + ,pullreq_merge_check_status = $2 + WHERE pullreq_target_repo_id = $3 AND + pullreq_target_branch = $4 AND + pullreq_state not in ($5, $6)` + + db := dbtx.GetAccessor(ctx, s.db) + + updatedAt := time.Now() + + _, err := db.ExecContext(ctx, query, updatedAt, status, targetRepo, targetBranch, + enum.PullReqStateClosed, enum.PullReqStateClosed) + if err != nil { + return database.ProcessSQLErrorf(err, "Failed to update mergeable status check %s in pull requests", status) + } + + return nil +} + // Delete the pull request. func (s *PullReqStore) Delete(ctx context.Context, id int64) error { const pullReqDelete = `DELETE FROM pullreqs WHERE pullreq_id = $1`