* fix bug on pull requests when transfer head repository * add migration and fix lint * fix tests and add a cache check on LoadBaseRepotags/v1.10.0-rc2
@@ -6,7 +6,6 @@ | |||
index: 2 | |||
head_repo_id: 1 | |||
base_repo_id: 1 | |||
head_user_name: user1 | |||
head_branch: branch1 | |||
base_branch: master | |||
merge_base: 1234567890abcdef | |||
@@ -21,7 +20,6 @@ | |||
index: 3 | |||
head_repo_id: 1 | |||
base_repo_id: 1 | |||
head_user_name: user1 | |||
head_branch: branch2 | |||
base_branch: master | |||
merge_base: fedcba9876543210 | |||
@@ -35,7 +33,6 @@ | |||
index: 1 | |||
head_repo_id: 11 | |||
base_repo_id: 10 | |||
head_user_name: user13 | |||
head_branch: branch2 | |||
base_branch: master | |||
merge_base: 0abcb056019adb83 |
@@ -258,6 +258,8 @@ var migrations = []Migration{ | |||
NewMigration("update migration repositories' service type", updateMigrationServiceTypes), | |||
// v101 -> v102 | |||
NewMigration("change length of some external login users columns", changeSomeColumnsLengthOfExternalLoginUser), | |||
// v102 -> v103 | |||
NewMigration("update migration repositories' service type", dropColumnHeadUserNameOnPullRequest), | |||
} | |||
// Migrate database to current version |
@@ -0,0 +1,15 @@ | |||
// Copyright 2019 The Gitea Authors. All rights reserved. | |||
// Use of this source code is governed by a MIT-style | |||
// license that can be found in the LICENSE file. | |||
package migrations | |||
import ( | |||
"github.com/go-xorm/xorm" | |||
) | |||
func dropColumnHeadUserNameOnPullRequest(x *xorm.Engine) error { | |||
sess := x.NewSession() | |||
defer sess.Close() | |||
return dropTableColumns(sess, "pull_request", "head_user_name") | |||
} |
@@ -66,7 +66,6 @@ type PullRequest struct { | |||
HeadRepo *Repository `xorm:"-"` | |||
BaseRepoID int64 `xorm:"INDEX"` | |||
BaseRepo *Repository `xorm:"-"` | |||
HeadUserName string | |||
HeadBranch string | |||
BaseBranch string | |||
ProtectedBranch *ProtectedBranch `xorm:"-"` | |||
@@ -79,6 +78,15 @@ type PullRequest struct { | |||
MergedUnix timeutil.TimeStamp `xorm:"updated INDEX"` | |||
} | |||
// MustHeadUserName returns the HeadRepo's username if failed return blank | |||
func (pr *PullRequest) MustHeadUserName() string { | |||
if err := pr.LoadHeadRepo(); err != nil { | |||
log.Error("LoadHeadRepo: %v", err) | |||
return "" | |||
} | |||
return pr.HeadRepo.MustOwnerName() | |||
} | |||
// Note: don't try to get Issue because will end up recursive querying. | |||
func (pr *PullRequest) loadAttributes(e Engine) (err error) { | |||
if pr.HasMerged && pr.Merger == nil { | |||
@@ -102,6 +110,10 @@ func (pr *PullRequest) LoadAttributes() error { | |||
// LoadBaseRepo loads pull request base repository from database | |||
func (pr *PullRequest) LoadBaseRepo() error { | |||
if pr.BaseRepo == nil { | |||
if pr.HeadRepoID == pr.BaseRepoID && pr.HeadRepo != nil { | |||
pr.BaseRepo = pr.HeadRepo | |||
return nil | |||
} | |||
var repo Repository | |||
if has, err := x.ID(pr.BaseRepoID).Get(&repo); err != nil { | |||
return err | |||
@@ -113,6 +125,24 @@ func (pr *PullRequest) LoadBaseRepo() error { | |||
return nil | |||
} | |||
// LoadHeadRepo loads pull request head repository from database | |||
func (pr *PullRequest) LoadHeadRepo() error { | |||
if pr.HeadRepo == nil { | |||
if pr.HeadRepoID == pr.BaseRepoID && pr.BaseRepo != nil { | |||
pr.HeadRepo = pr.BaseRepo | |||
return nil | |||
} | |||
var repo Repository | |||
if has, err := x.ID(pr.HeadRepoID).Get(&repo); err != nil { | |||
return err | |||
} else if !has { | |||
return ErrRepoNotExist{ID: pr.BaseRepoID} | |||
} | |||
pr.HeadRepo = &repo | |||
} | |||
return nil | |||
} | |||
// LoadIssue loads issue information from database | |||
func (pr *PullRequest) LoadIssue() (err error) { | |||
return pr.loadIssue(x) | |||
@@ -152,7 +182,7 @@ func (pr *PullRequest) GetDefaultMergeMessage() string { | |||
return "" | |||
} | |||
} | |||
return fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.HeadUserName, pr.HeadRepo.Name, pr.BaseBranch) | |||
return fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.MustHeadUserName(), pr.HeadRepo.Name, pr.BaseBranch) | |||
} | |||
// GetDefaultSquashMessage returns default message used when squash and merging pull request | |||
@@ -1159,18 +1189,6 @@ func checkForInvalidation(requests PullRequestList, repoID int64, doer *User, br | |||
return nil | |||
} | |||
// ChangeUsernameInPullRequests changes the name of head_user_name | |||
func ChangeUsernameInPullRequests(oldUserName, newUserName string) error { | |||
pr := PullRequest{ | |||
HeadUserName: strings.ToLower(newUserName), | |||
} | |||
_, err := x. | |||
Cols("head_user_name"). | |||
Where("head_user_name = ?", strings.ToLower(oldUserName)). | |||
Update(pr) | |||
return err | |||
} | |||
// checkAndUpdateStatus checks if pull request is possible to leaving checking status, | |||
// and set to be either conflict or mergeable. | |||
func (pr *PullRequest) checkAndUpdateStatus() { |
@@ -232,20 +232,6 @@ func TestPullRequestList_LoadAttributes(t *testing.T) { | |||
// TODO TestAddTestPullRequestTask | |||
func TestChangeUsernameInPullRequests(t *testing.T) { | |||
assert.NoError(t, PrepareTestDatabase()) | |||
const newUsername = "newusername" | |||
assert.NoError(t, ChangeUsernameInPullRequests("user1", newUsername)) | |||
prs := make([]*PullRequest, 0, 10) | |||
assert.NoError(t, x.Where("head_user_name = ?", newUsername).Find(&prs)) | |||
assert.Len(t, prs, 2) | |||
for _, pr := range prs { | |||
assert.Equal(t, newUsername, pr.HeadUserName) | |||
} | |||
CheckConsistencyFor(t, &PullRequest{}) | |||
} | |||
func TestPullRequest_IsWorkInProgress(t *testing.T) { | |||
assert.NoError(t, PrepareTestDatabase()) | |||
@@ -994,10 +994,6 @@ func ChangeUserName(u *User, newUserName string) (err error) { | |||
return ErrUserAlreadyExist{newUserName} | |||
} | |||
if err = ChangeUsernameInPullRequests(u.Name, newUserName); err != nil { | |||
return fmt.Errorf("ChangeUsernameInPullRequests: %v", err) | |||
} | |||
// Do not fail if directory does not exist | |||
if err = os.Rename(UserPath(u.Name), UserPath(newUserName)); err != nil && !os.IsNotExist(err) { | |||
return fmt.Errorf("Rename user directory: %v", err) |
@@ -579,14 +579,13 @@ func (g *GiteaLocalUploader) newPullRequest(pr *base.PullRequest) (*models.PullR | |||
} | |||
var pullRequest = models.PullRequest{ | |||
HeadRepoID: g.repo.ID, | |||
HeadBranch: head, | |||
HeadUserName: g.repoOwner, | |||
BaseRepoID: g.repo.ID, | |||
BaseBranch: pr.Base.Ref, | |||
MergeBase: pr.Base.SHA, | |||
Index: pr.Number, | |||
HasMerged: pr.Merged, | |||
HeadRepoID: g.repo.ID, | |||
HeadBranch: head, | |||
BaseRepoID: g.repo.ID, | |||
BaseBranch: pr.Base.Ref, | |||
MergeBase: pr.Base.SHA, | |||
Index: pr.Number, | |||
HasMerged: pr.Merged, | |||
Issue: &issue, | |||
} |
@@ -190,7 +190,7 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption | |||
) | |||
// Get repo/branch information | |||
headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch := parseCompareInfo(ctx, form) | |||
_, headRepo, headGitRepo, compareInfo, baseBranch, headBranch := parseCompareInfo(ctx, form) | |||
if ctx.Written() { | |||
return | |||
} | |||
@@ -265,15 +265,14 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption | |||
DeadlineUnix: deadlineUnix, | |||
} | |||
pr := &models.PullRequest{ | |||
HeadRepoID: headRepo.ID, | |||
BaseRepoID: repo.ID, | |||
HeadUserName: headUser.Name, | |||
HeadBranch: headBranch, | |||
BaseBranch: baseBranch, | |||
HeadRepo: headRepo, | |||
BaseRepo: repo, | |||
MergeBase: compareInfo.MergeBase, | |||
Type: models.PullRequestGitea, | |||
HeadRepoID: headRepo.ID, | |||
BaseRepoID: repo.ID, | |||
HeadBranch: headBranch, | |||
BaseBranch: baseBranch, | |||
HeadRepo: headRepo, | |||
BaseRepo: repo, | |||
MergeBase: compareInfo.MergeBase, | |||
Type: models.PullRequestGitea, | |||
} | |||
// Get all assignee IDs |
@@ -272,12 +272,12 @@ func checkPullInfo(ctx *context.Context) *models.Issue { | |||
} | |||
func setMergeTarget(ctx *context.Context, pull *models.PullRequest) { | |||
if ctx.Repo.Owner.Name == pull.HeadUserName { | |||
if ctx.Repo.Owner.Name == pull.MustHeadUserName() { | |||
ctx.Data["HeadTarget"] = pull.HeadBranch | |||
} else if pull.HeadRepo == nil { | |||
ctx.Data["HeadTarget"] = pull.HeadUserName + ":" + pull.HeadBranch | |||
ctx.Data["HeadTarget"] = pull.MustHeadUserName() + ":" + pull.HeadBranch | |||
} else { | |||
ctx.Data["HeadTarget"] = pull.HeadUserName + "/" + pull.HeadRepo.Name + ":" + pull.HeadBranch | |||
ctx.Data["HeadTarget"] = pull.MustHeadUserName() + "/" + pull.HeadRepo.Name + ":" + pull.HeadBranch | |||
} | |||
ctx.Data["BaseTarget"] = pull.BaseBranch | |||
} | |||
@@ -440,7 +440,7 @@ func ViewPullCommits(ctx *context.Context) { | |||
ctx.NotFound("ViewPullCommits", nil) | |||
return | |||
} | |||
ctx.Data["Username"] = pull.HeadUserName | |||
ctx.Data["Username"] = pull.MustHeadUserName() | |||
ctx.Data["Reponame"] = pull.HeadRepo.Name | |||
commits = prInfo.Commits | |||
} | |||
@@ -512,7 +512,7 @@ func ViewPullFiles(ctx *context.Context) { | |||
return | |||
} | |||
headRepoPath := models.RepoPath(pull.HeadUserName, pull.HeadRepo.Name) | |||
headRepoPath := pull.HeadRepo.RepoPath() | |||
headGitRepo, err := git.OpenRepository(headRepoPath) | |||
if err != nil { | |||
@@ -531,8 +531,8 @@ func ViewPullFiles(ctx *context.Context) { | |||
endCommitID = headCommitID | |||
gitRepo = headGitRepo | |||
headTarget = path.Join(pull.HeadUserName, pull.HeadRepo.Name) | |||
ctx.Data["Username"] = pull.HeadUserName | |||
headTarget = path.Join(pull.MustHeadUserName(), pull.HeadRepo.Name) | |||
ctx.Data["Username"] = pull.MustHeadUserName() | |||
ctx.Data["Reponame"] = pull.HeadRepo.Name | |||
} | |||
@@ -754,15 +754,14 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm) | |||
Content: form.Content, | |||
} | |||
pullRequest := &models.PullRequest{ | |||
HeadRepoID: headRepo.ID, | |||
BaseRepoID: repo.ID, | |||
HeadUserName: headUser.Name, | |||
HeadBranch: headBranch, | |||
BaseBranch: baseBranch, | |||
HeadRepo: headRepo, | |||
BaseRepo: repo, | |||
MergeBase: prInfo.MergeBase, | |||
Type: models.PullRequestGitea, | |||
HeadRepoID: headRepo.ID, | |||
BaseRepoID: repo.ID, | |||
HeadBranch: headBranch, | |||
BaseBranch: baseBranch, | |||
HeadRepo: headRepo, | |||
BaseRepo: repo, | |||
MergeBase: prInfo.MergeBase, | |||
Type: models.PullRequestGitea, | |||
} | |||
// FIXME: check error in the case two people send pull request at almost same time, give nice error prompt | |||
// instead of 500. |
@@ -65,7 +65,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor | |||
} | |||
}() | |||
headRepoPath := models.RepoPath(pr.HeadUserName, pr.HeadRepo.Name) | |||
headRepoPath := pr.HeadRepo.RepoPath() | |||
if err := git.InitRepository(tmpBasePath, false); err != nil { | |||
return fmt.Errorf("git init: %v", err) | |||
@@ -260,14 +260,17 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor | |||
} | |||
} | |||
headUser, err := models.GetUserByName(pr.HeadUserName) | |||
var headUser *models.User | |||
err = pr.HeadRepo.GetOwner() | |||
if err != nil { | |||
if !models.IsErrUserNotExist(err) { | |||
log.Error("Can't find user: %s for head repository - %v", pr.HeadUserName, err) | |||
log.Error("Can't find user: %d for head repository - %v", pr.HeadRepo.OwnerID, err) | |||
return err | |||
} | |||
log.Error("Can't find user: %s for head repository - defaulting to doer: %s - %v", pr.HeadUserName, doer.Name, err) | |||
log.Error("Can't find user: %d for head repository - defaulting to doer: %s - %v", pr.HeadRepo.OwnerID, doer.Name, err) | |||
headUser = doer | |||
} else { | |||
headUser = pr.HeadRepo.Owner | |||
} | |||
env := models.FullPushingEnvironment( |