Fix wrong display of recently pushed notification (#25812)
There's a bug in #25715: If user pushed a commit into another repo with same branch name, the no-related repo will display the recently pushed notification incorrectly. It is simple to fix this, we should match the repo id in the sql query.  The latest commit is 2 weeks ago.  The notification comes from another repo with same branch name:  After: In forked repo:  New PR Link will redirect to the original repo:  In the original repo:  New PR Link:  In the same repo:  New PR Link:  08/15 Update: Follow #26257, added permission check and logic fix mentioned in https://github.com/go-gitea/gitea/pull/26257#discussion_r1294085203 2024/04/25 Update: Fix #30611 --------- Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
yp05327
GitHub
parent
9c8c9ff6d1
commit
daf2a4c047
26 changed files with 508 additions and 72 deletions
|
|
@ -10,9 +10,11 @@ import (
|
|||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
repo_model "code.gitea.io/gitea/models/repo"
|
||||
"code.gitea.io/gitea/models/unit"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
"code.gitea.io/gitea/modules/git"
|
||||
"code.gitea.io/gitea/modules/log"
|
||||
"code.gitea.io/gitea/modules/optional"
|
||||
"code.gitea.io/gitea/modules/timeutil"
|
||||
"code.gitea.io/gitea/modules/util"
|
||||
|
||||
|
|
@ -102,8 +104,9 @@ func (err ErrBranchesEqual) Unwrap() error {
|
|||
// for pagination, keyword search and filtering
|
||||
type Branch struct {
|
||||
ID int64
|
||||
RepoID int64 `xorm:"UNIQUE(s)"`
|
||||
Name string `xorm:"UNIQUE(s) NOT NULL"` // git's ref-name is case-sensitive internally, however, in some databases (mssql, mysql, by default), it's case-insensitive at the moment
|
||||
RepoID int64 `xorm:"UNIQUE(s)"`
|
||||
Repo *repo_model.Repository `xorm:"-"`
|
||||
Name string `xorm:"UNIQUE(s) NOT NULL"` // git's ref-name is case-sensitive internally, however, in some databases (mssql, mysql, by default), it's case-insensitive at the moment
|
||||
CommitID string
|
||||
CommitMessage string `xorm:"TEXT"` // it only stores the message summary (the first line)
|
||||
PusherID int64
|
||||
|
|
@ -139,6 +142,14 @@ func (b *Branch) LoadPusher(ctx context.Context) (err error) {
|
|||
return err
|
||||
}
|
||||
|
||||
func (b *Branch) LoadRepo(ctx context.Context) (err error) {
|
||||
if b.Repo != nil || b.RepoID == 0 {
|
||||
return nil
|
||||
}
|
||||
b.Repo, err = repo_model.GetRepositoryByID(ctx, b.RepoID)
|
||||
return err
|
||||
}
|
||||
|
||||
func init() {
|
||||
db.RegisterModel(new(Branch))
|
||||
db.RegisterModel(new(RenamedBranch))
|
||||
|
|
@ -400,24 +411,111 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str
|
|||
return committer.Commit()
|
||||
}
|
||||
|
||||
// FindRecentlyPushedNewBranches return at most 2 new branches pushed by the user in 6 hours which has no opened PRs created
|
||||
// except the indicate branch
|
||||
func FindRecentlyPushedNewBranches(ctx context.Context, repoID, userID int64, excludeBranchName string) (BranchList, error) {
|
||||
branches := make(BranchList, 0, 2)
|
||||
subQuery := builder.Select("head_branch").From("pull_request").
|
||||
InnerJoin("issue", "issue.id = pull_request.issue_id").
|
||||
Where(builder.Eq{
|
||||
"pull_request.head_repo_id": repoID,
|
||||
"issue.is_closed": false,
|
||||
})
|
||||
err := db.GetEngine(ctx).
|
||||
Where("pusher_id=? AND is_deleted=?", userID, false).
|
||||
And("name <> ?", excludeBranchName).
|
||||
And("repo_id = ?", repoID).
|
||||
And("commit_time >= ?", time.Now().Add(-time.Hour*6).Unix()).
|
||||
NotIn("name", subQuery).
|
||||
OrderBy("branch.commit_time DESC").
|
||||
Limit(2).
|
||||
Find(&branches)
|
||||
return branches, err
|
||||
type FindRecentlyPushedNewBranchesOptions struct {
|
||||
Repo *repo_model.Repository
|
||||
BaseRepo *repo_model.Repository
|
||||
CommitAfterUnix int64
|
||||
MaxCount int
|
||||
}
|
||||
|
||||
type RecentlyPushedNewBranch struct {
|
||||
BranchDisplayName string
|
||||
BranchLink string
|
||||
BranchCompareURL string
|
||||
CommitTime timeutil.TimeStamp
|
||||
}
|
||||
|
||||
// FindRecentlyPushedNewBranches return at most 2 new branches pushed by the user in 2 hours which has no opened PRs created
|
||||
// if opts.CommitAfterUnix is 0, we will find the branches that were committed to in the last 2 hours
|
||||
// if opts.ListOptions is not set, we will only display top 2 latest branch
|
||||
func FindRecentlyPushedNewBranches(ctx context.Context, doer *user_model.User, opts *FindRecentlyPushedNewBranchesOptions) ([]*RecentlyPushedNewBranch, error) {
|
||||
if doer == nil {
|
||||
return []*RecentlyPushedNewBranch{}, nil
|
||||
}
|
||||
|
||||
// find all related repo ids
|
||||
repoOpts := repo_model.SearchRepoOptions{
|
||||
Actor: doer,
|
||||
Private: true,
|
||||
AllPublic: false, // Include also all public repositories of users and public organisations
|
||||
AllLimited: false, // Include also all public repositories of limited organisations
|
||||
Fork: optional.Some(true),
|
||||
ForkFrom: opts.BaseRepo.ID,
|
||||
Archived: optional.Some(false),
|
||||
}
|
||||
repoCond := repo_model.SearchRepositoryCondition(&repoOpts).And(repo_model.AccessibleRepositoryCondition(doer, unit.TypeCode))
|
||||
if opts.Repo.ID == opts.BaseRepo.ID {
|
||||
// should also include the base repo's branches
|
||||
repoCond = repoCond.Or(builder.Eq{"id": opts.BaseRepo.ID})
|
||||
} else {
|
||||
// in fork repo, we only detect the fork repo's branch
|
||||
repoCond = repoCond.And(builder.Eq{"id": opts.Repo.ID})
|
||||
}
|
||||
repoIDs := builder.Select("id").From("repository").Where(repoCond)
|
||||
|
||||
if opts.CommitAfterUnix == 0 {
|
||||
opts.CommitAfterUnix = time.Now().Add(-time.Hour * 2).Unix()
|
||||
}
|
||||
|
||||
baseBranch, err := GetBranch(ctx, opts.BaseRepo.ID, opts.BaseRepo.DefaultBranch)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// find all related branches, these branches may already created PRs, we will check later
|
||||
var branches []*Branch
|
||||
if err := db.GetEngine(ctx).
|
||||
Where(builder.And(
|
||||
builder.Eq{
|
||||
"pusher_id": doer.ID,
|
||||
"is_deleted": false,
|
||||
},
|
||||
builder.Gte{"commit_time": opts.CommitAfterUnix},
|
||||
builder.In("repo_id", repoIDs),
|
||||
// newly created branch have no changes, so skip them
|
||||
builder.Neq{"commit_id": baseBranch.CommitID},
|
||||
)).
|
||||
OrderBy(db.SearchOrderByRecentUpdated.String()).
|
||||
Find(&branches); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
newBranches := make([]*RecentlyPushedNewBranch, 0, len(branches))
|
||||
if opts.MaxCount == 0 {
|
||||
// by default we display 2 recently pushed new branch
|
||||
opts.MaxCount = 2
|
||||
}
|
||||
for _, branch := range branches {
|
||||
// whether branch have already created PR
|
||||
count, err := db.GetEngine(ctx).Table("pull_request").
|
||||
// we should not only use branch name here, because if there are branches with same name in other repos,
|
||||
// we can not detect them correctly
|
||||
Where(builder.Eq{"head_repo_id": branch.RepoID, "head_branch": branch.Name}).Count()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// if no PR, we add to the result
|
||||
if count == 0 {
|
||||
if err := branch.LoadRepo(ctx); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
branchDisplayName := branch.Name
|
||||
if branch.Repo.ID != opts.BaseRepo.ID && branch.Repo.ID != opts.Repo.ID {
|
||||
branchDisplayName = fmt.Sprintf("%s:%s", branch.Repo.FullName(), branchDisplayName)
|
||||
}
|
||||
newBranches = append(newBranches, &RecentlyPushedNewBranch{
|
||||
BranchDisplayName: branchDisplayName,
|
||||
BranchLink: fmt.Sprintf("%s/src/branch/%s", branch.Repo.Link(), util.PathEscapeSegments(branch.Name)),
|
||||
BranchCompareURL: branch.Repo.ComposeBranchCompareURL(opts.BaseRepo, branch.Name),
|
||||
CommitTime: branch.CommitTime,
|
||||
})
|
||||
}
|
||||
if len(newBranches) == opts.MaxCount {
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
return newBranches, nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import (
|
|||
"context"
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
repo_model "code.gitea.io/gitea/models/repo"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
"code.gitea.io/gitea/modules/container"
|
||||
"code.gitea.io/gitea/modules/optional"
|
||||
|
|
@ -59,6 +60,24 @@ func (branches BranchList) LoadPusher(ctx context.Context) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
func (branches BranchList) LoadRepo(ctx context.Context) error {
|
||||
ids := container.FilterSlice(branches, func(branch *Branch) (int64, bool) {
|
||||
return branch.RepoID, branch.RepoID > 0 && branch.Repo == nil
|
||||
})
|
||||
|
||||
reposMap := make(map[int64]*repo_model.Repository, len(ids))
|
||||
if err := db.GetEngine(ctx).In("id", ids).Find(&reposMap); err != nil {
|
||||
return err
|
||||
}
|
||||
for _, branch := range branches {
|
||||
if branch.RepoID <= 0 || branch.Repo != nil {
|
||||
continue
|
||||
}
|
||||
branch.Repo = reposMap[branch.RepoID]
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type FindBranchOptions struct {
|
||||
db.ListOptions
|
||||
RepoID int64
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue