Skip to content

Fix wrong display of recently pushed notification (#25812) #31043

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

Merged
merged 4 commits into from
May 23, 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
36 changes: 36 additions & 0 deletions models/fixtures/branch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,39 @@
is_deleted: false
deleted_by_id: 0
deleted_unix: 0

-
id: 5
repo_id: 10
name: 'master'
commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d'
commit_message: 'Initial commit'
commit_time: 1489927679
pusher_id: 12
is_deleted: false
deleted_by_id: 0
deleted_unix: 0

-
id: 6
repo_id: 10
name: 'outdated-new-branch'
commit_id: 'cb24c347e328d83c1e0c3c908a6b2c0a2fcb8a3d'
commit_message: 'add'
commit_time: 1489927679
pusher_id: 12
is_deleted: false
deleted_by_id: 0
deleted_unix: 0

-
id: 14
repo_id: 11
name: 'master'
commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d'
commit_message: 'Initial commit'
commit_time: 1489927679
pusher_id: 13
is_deleted: false
deleted_by_id: 0
deleted_unix: 0
8 changes: 8 additions & 0 deletions models/fixtures/issue_index.yml
Original file line number Diff line number Diff line change
@@ -1,27 +1,35 @@
-
group_id: 1
max_index: 5

-
group_id: 2
max_index: 2

-
group_id: 3
max_index: 2

-
group_id: 10
max_index: 1

-
group_id: 32
max_index: 2

-
group_id: 48
max_index: 1

-
group_id: 42
max_index: 1

-
group_id: 50
max_index: 1

-
group_id: 51
max_index: 1
12 changes: 12 additions & 0 deletions models/fixtures/org_user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,15 @@
uid: 40
org_id: 41
is_public: true

-
id: 21
uid: 12
org_id: 25
is_public: true

-
id: 22
uid: 2
org_id: 35
is_public: true
2 changes: 1 addition & 1 deletion models/fixtures/repository.yml
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@
is_archived: false
is_mirror: false
status: 0
is_fork: false
is_fork: true
fork_id: 10
is_template: false
template_id: 0
Expand Down
22 changes: 22 additions & 0 deletions models/fixtures/team.yml
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,25 @@
num_members: 2
includes_all_repositories: false
can_create_org_repo: false

-
id: 23
org_id: 25
lower_name: owners
name: Owners
authorize: 4 # owner
num_repos: 0
num_members: 1
includes_all_repositories: false
can_create_org_repo: true

-
id: 24
org_id: 35
lower_name: team24
name: team24
authorize: 2 # write
num_repos: 0
num_members: 1
includes_all_repositories: true
can_create_org_repo: false
18 changes: 18 additions & 0 deletions models/fixtures/team_unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -322,3 +322,21 @@
team_id: 22
type: 3
access_mode: 1

-
id: 55
team_id: 18
type: 1 # code
access_mode: 4

-
id: 56
team_id: 23
type: 1 # code
access_mode: 4

-
id: 57
team_id: 24
type: 1 # code
access_mode: 2
12 changes: 12 additions & 0 deletions models/fixtures/team_user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,15 @@
org_id: 41
team_id: 22
uid: 39

-
id: 26
org_id: 25
team_id: 23
uid: 12

-
id: 27
org_id: 35
team_id: 24
uid: 2
8 changes: 4 additions & 4 deletions models/fixtures/user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -918,8 +918,8 @@
num_following: 0
num_stars: 0
num_repos: 0
num_teams: 1
num_members: 1
num_teams: 2
num_members: 2
visibility: 0
repo_admin_change_team_access: false
theme: ""
Expand Down Expand Up @@ -1289,8 +1289,8 @@
num_following: 0
num_stars: 0
num_repos: 0
num_teams: 1
num_members: 1
num_teams: 2
num_members: 2
visibility: 2
repo_admin_change_team_access: false
theme: ""
Expand Down
142 changes: 120 additions & 22 deletions models/git/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
}
19 changes: 19 additions & 0 deletions models/git/branch_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Loading