Skip to content

WIP: change PR flow to AGit #35111

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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: 2 additions & 0 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
CommentTypeUnpin // 37 unpin Issue/PullRequest

CommentTypeChangeTimeEstimate // 38 Change time estimate
CommentTypeChangePRFlowType // 39 Change pull request's flow type

Check failure on line 118 in models/issues/comment.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

File is not properly formatted (gofmt)

Check failure on line 118 in models/issues/comment.go

View workflow job for this annotation

GitHub Actions / lint-backend

File is not properly formatted (gofmt)

Check failure on line 118 in models/issues/comment.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

File is not properly formatted (gofmt)
)

var commentStrings = []string{
Expand Down Expand Up @@ -157,6 +158,7 @@
"pin",
"unpin",
"change_time_estimate",
"change_flow_type",
}

func (t CommentType) String() string {
Expand Down
36 changes: 36 additions & 0 deletions models/issues/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,30 @@
PullRequestFlowAGit
)

var PullRequestFlowMap map[PullRequestFlow]string = map[PullRequestFlow]string{

Check failure on line 116 in models/issues/pull.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

var-declaration: should omit type map[PullRequestFlow]string from declaration of var PullRequestFlowMap; it will be inferred from the right-hand side (revive)

Check failure on line 116 in models/issues/pull.go

View workflow job for this annotation

GitHub Actions / lint-backend

var-declaration: should omit type map[PullRequestFlow]string from declaration of var PullRequestFlowMap; it will be inferred from the right-hand side (revive)

Check failure on line 116 in models/issues/pull.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

var-declaration: should omit type map[PullRequestFlow]string from declaration of var PullRequestFlowMap; it will be inferred from the right-hand side (revive)
PullRequestFlowGithub: "github",
PullRequestFlowAGit: "agit",
}
var PullRequestFlowTypeUnknown error = errors.New("Unknown pull request type")

Check failure on line 120 in models/issues/pull.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

var-declaration: should omit type error from declaration of var PullRequestFlowTypeUnknown; it will be inferred from the right-hand side (revive)

Check failure on line 120 in models/issues/pull.go

View workflow job for this annotation

GitHub Actions / lint-backend

error-naming: error var PullRequestFlowTypeUnknown should have name of the form ErrFoo (revive)

Check failure on line 120 in models/issues/pull.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

var-declaration: should omit type error from declaration of var PullRequestFlowTypeUnknown; it will be inferred from the right-hand side (revive)

func PullRequestFlowFromString(strtype string) (PullRequestFlow, error) {
for k, v := range PullRequestFlowMap {
if v == strtype {
return k, nil
}
}

return 0, PullRequestFlowTypeUnknown
}

func PullRequestFlowTypeToString(flow PullRequestFlow) string {
v, ok := PullRequestFlowMap[flow]
if ok {
return v
}
return ""
}

// PullRequest represents relation between pull request and repositories.
type PullRequest struct {
ID int64 `xorm:"pk autoincr"`
Expand Down Expand Up @@ -462,6 +486,18 @@
return pr.HeadRepoID != pr.BaseRepoID
}

func (pr *PullRequest) ConvertToAgitPullRequest(ctx context.Context, doer *user_model.User) (err error) {
if pr.IsAgitFlow() {
return nil
}

pr.Flow = PullRequestFlowAGit
pr.HeadRepoID = pr.BaseRepoID
pr.HeadBranch = doer.LowerName + "/" + pr.HeadBranch

return pr.UpdateColsIfNotMerged(ctx, "flow", "head_repo_id", "head_branch")
}

// NewPullRequest creates new pull request with labels for repository.
func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids []string, pr *PullRequest) (err error) {
ctx, committer, err := db.TxContext(ctx)
Expand Down
4 changes: 4 additions & 0 deletions modules/structs/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ type PullRequest struct {
Closed *time.Time `json:"closed_at"`

PinOrder int `json:"pin_order"`

// swagger:enum["agit","github"]
Flow string `json:"flow"`
}

// PRBranchInfo information about a branch
Expand Down Expand Up @@ -107,6 +110,7 @@ type EditPullRequestOption struct {
Deadline *time.Time `json:"due_date"`
RemoveDeadline *bool `json:"unset_due_date"`
AllowMaintainerEdit *bool `json:"allow_maintainer_edit"`
FlowType string `json:"flow_type"`
}

// ChangedFile store information about files affected by the pull request
Expand Down
40 changes: 40 additions & 0 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,46 @@ func EditPullRequest(ctx *context.APIContext) {
notify_service.PullRequestChangeTargetBranch(ctx, ctx.Doer, pr, form.Base)
}

// change pull request type from branch or from AGit
if !pr.HasMerged && len(form.FlowType) != 0 {
flow, err := issues_model.PullRequestFlowFromString(form.FlowType)
if err != nil {
ctx.APIErrorInternal(err)
return
}
err = nil
if !issue.IsPoster(ctx.Doer.ID) && !ctx.Repo.CanWrite(unit.TypeCode) {
// not implemented
ctx.Status(http.StatusForbidden)
return
}
if flow != pr.Flow {
switch flow {
case issues_model.PullRequestFlowGithub:
// not implemented
ctx.Status(http.StatusForbidden)
return
case issues_model.PullRequestFlowAGit:
err = pull_service.ChangePullRequestFlowToAgit(ctx, pr, ctx.Doer)
}
}
if err != nil {
if issues_model.IsErrPullRequestAlreadyExists(err) {
ctx.APIError(http.StatusConflict, err)
return
} else if issues_model.IsErrIssueIsClosed(err) {
ctx.APIError(http.StatusUnprocessableEntity, err)
return
} else if pull_service.IsErrPullRequestHasMerged(err) {
ctx.APIError(http.StatusConflict, err)
return
}
ctx.APIErrorInternal(err)
return
}
notify_service.PullRequestChangeFlowType(ctx, ctx.Doer, pr)
}

// update allow edits
if form.AllowMaintainerEdit != nil {
if err := pull_service.SetAllowEdits(ctx, ctx.Doer, pr, *form.AllowMaintainerEdit); err != nil {
Expand Down
26 changes: 26 additions & 0 deletions services/actions/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,32 @@
Notify(ctx)
}

func (n *actionsNotifier) PullRequestChangeFlowType(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest) {
ctx = withMethod(ctx, "PullRequestChangeFlowType")

if err := pr.LoadIssue(ctx); err != nil {
log.Error("LoadAttributes: %v", err)
return
}

if err := pr.Issue.LoadRepo(ctx); err != nil {
log.Error("pr.Issue.LoadRepo: %v", err)
return
}

permission, _ := access_model.GetUserRepoPermission(ctx, pr.Issue.Repo, pr.Issue.Poster)
newNotifyInput(pr.Issue.Repo, doer, webhook_module.HookEventPullRequest).
WithPayload(&api.PullRequestPayload{
Action: api.HookIssueEdited,

Check failure on line 707 in services/actions/notifier.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

File is not properly formatted (gofmt)

Check failure on line 707 in services/actions/notifier.go

View workflow job for this annotation

GitHub Actions / lint-backend

File is not properly formatted (gofmt)

Check failure on line 707 in services/actions/notifier.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

File is not properly formatted (gofmt)
Index: pr.Issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, pr, nil),
Repository: convert.ToRepo(ctx, pr.Issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil),
}).
WithPullRequest(pr).
Notify(ctx)
}

func (n *actionsNotifier) PullRequestChangeTargetBranch(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest, oldBranch string) {
ctx = withMethod(ctx, "PullRequestChangeTargetBranch")

Expand Down
1 change: 1 addition & 0 deletions services/convert/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u
Created: pr.Issue.CreatedUnix.AsTimePtr(),
Updated: pr.Issue.UpdatedUnix.AsTimePtr(),
PinOrder: util.Iif(apiIssue.PinOrder == -1, 0, apiIssue.PinOrder),
Flow: issues_model.PullRequestFlowTypeToString(pr.Flow),

// output "[]" rather than null to align to github outputs
RequestedReviewers: []*api.User{},
Expand Down
1 change: 1 addition & 0 deletions services/forms/user_form_hidden_comments.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var hiddenCommentTypeGroups = hiddenCommentTypeGroupsType{
"branch": {
/*11*/ issues_model.CommentTypeDeleteBranch,
/*25*/ issues_model.CommentTypeChangeTargetBranch,
/*39*/ issues_model.CommentTypeChangePRFlowType,
},
"time_tracking": {
/*12*/ issues_model.CommentTypeStartTracking,
Expand Down
1 change: 1 addition & 0 deletions services/notify/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type Notifier interface {
PullRequestReview(ctx context.Context, pr *issues_model.PullRequest, review *issues_model.Review, comment *issues_model.Comment, mentions []*user_model.User)
PullRequestCodeComment(ctx context.Context, pr *issues_model.PullRequest, comment *issues_model.Comment, mentions []*user_model.User)
PullRequestChangeTargetBranch(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest, oldBranch string)
PullRequestChangeFlowType(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest)
PullRequestPushCommits(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest, comment *issues_model.Comment)
PullReviewDismiss(ctx context.Context, doer *user_model.User, review *issues_model.Review, comment *issues_model.Comment)

Expand Down
7 changes: 7 additions & 0 deletions services/notify/notify.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,13 @@ func PullRequestChangeTargetBranch(ctx context.Context, doer *user_model.User, p
}
}

// PullRequestChangeFlowType notifies when a pull request's source branch was changed
func PullRequestChangeFlowType(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest) {
for _, notifier := range notifiers {
notifier.PullRequestChangeFlowType(ctx, doer, pr)
}
}

// PullRequestPushCommits notifies when push commits to pull request's head branch
func PullRequestPushCommits(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest, comment *issues_model.Comment) {
for _, notifier := range notifiers {
Expand Down
4 changes: 4 additions & 0 deletions services/notify/null.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ func (*NullNotifier) PullRequestSynchronized(ctx context.Context, doer *user_mod
func (*NullNotifier) PullRequestChangeTargetBranch(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest, oldBranch string) {
}

// PullRequestChangeFlowType places a place holder function
func (*NullNotifier) PullRequestChangeFlowType(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest) {
}

// PullRequestPushCommits notifies when push commits to pull request's head branch
func (*NullNotifier) PullRequestPushCommits(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest, comment *issues_model.Comment) {
}
Expand Down
56 changes: 56 additions & 0 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,58 @@
err.ID, err.IssueID, err.HeadRepoID, err.BaseRepoID, err.HeadBranch, err.BaseBranch)
}

// ChangePullRequestFlowToAgit changes the source branch of this pull request, as the given user
func ChangePullRequestFlowToAgit(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User) (err error) {
releaser, err := globallock.Lock(ctx, getPullWorkingLockKey(pr.ID))
if err != nil {
log.Error("lock.Lock(): %v", err)
return fmt.Errorf("lock.Lock: %w", err)
}
defer releaser()

// Current target branch is already the same
if pr.Flow == issues_model.PullRequestFlowAGit {
return nil
}

if pr.Issue.IsClosed {
return issues_model.ErrIssueIsClosed{
ID: pr.Issue.ID,
RepoID: pr.Issue.RepoID,
Index: pr.Issue.Index,
IsPull: true,
}
}

if pr.HasMerged {
return ErrPullRequestHasMerged{
ID: pr.ID,
IssueID: pr.Index,
HeadRepoID: pr.HeadRepoID,
BaseRepoID: pr.BaseRepoID,
HeadBranch: pr.HeadBranch,
BaseBranch: pr.BaseBranch,
}
}

if err = pr.ConvertToAgitPullRequest(ctx, doer); err != nil {
return fmt.Errorf("Failed to convert PR to AGit: %w", err)
}

// Create comment
options := &issues_model.CreateCommentOptions{
Type: issues_model.CommentTypeChangePRFlowType,

Check failure on line 290 in services/pull/pull.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

File is not properly formatted (gofmt)

Check failure on line 290 in services/pull/pull.go

View workflow job for this annotation

GitHub Actions / lint-backend

File is not properly formatted (gofmt)

Check failure on line 290 in services/pull/pull.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

File is not properly formatted (gofmt)
Doer: doer,
Repo: pr.Issue.Repo,
Issue: pr.Issue,
}
if _, err = issues_model.CreateComment(ctx, options); err != nil {
return fmt.Errorf("CreateChangeSourceBranchComment: %w", err)
}

return nil
}

// ChangeTargetBranch changes the target branch of this pull request, as the given user.
func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, targetBranch string) (err error) {
releaser, err := globallock.Lock(ctx, getPullWorkingLockKey(pr.ID))
Expand Down Expand Up @@ -1003,6 +1055,10 @@
return statuses, lastStatus, err
}

func IsBranchEqual(ctx context.Context, gitRepo *git.Repository, branch1, branch2 string) (bool, error) {
return false, nil
}

// IsHeadEqualWithBranch returns if the commits of branchName are available in pull request head
func IsHeadEqualWithBranch(ctx context.Context, pr *issues_model.PullRequest, branchName string) (bool, error) {
var err error
Expand Down
20 changes: 20 additions & 0 deletions services/webhook/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,26 @@
}
}

func (m *webhookNotifier) PullRequestChangeFlowType(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest) {
if err := pr.LoadIssue(ctx); err != nil {
log.Error("LoadIssue: %v", err)
return
}

issue := pr.Issue

mode, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster)
if err := PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventPullRequest, &api.PullRequestPayload{
Action: api.HookIssueEdited,

Check failure on line 744 in services/webhook/notifier.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

File is not properly formatted (gofmt)

Check failure on line 744 in services/webhook/notifier.go

View workflow job for this annotation

GitHub Actions / lint-backend

File is not properly formatted (gofmt)

Check failure on line 744 in services/webhook/notifier.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

File is not properly formatted (gofmt)
Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, pr, doer),
Repository: convert.ToRepo(ctx, issue.Repo, mode),
Sender: convert.ToUser(ctx, doer, nil),
}); err != nil {
log.Error("PrepareWebhooks [pr: %d]: %v", pr.ID, err)
}
}

func (m *webhookNotifier) PullRequestReview(ctx context.Context, pr *issues_model.PullRequest, review *issues_model.Review, comment *issues_model.Comment, mentions []*user_model.User) {
var reviewHookType webhook_module.HookEventType

Expand Down
8 changes: 8 additions & 0 deletions templates/swagger/v1_json.tmpl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading