From e6f719c6c68460d42151d829f0f64eed49ea2b26 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 13 Feb 2025 14:48:31 -0800 Subject: [PATCH 01/17] Fix project issues list --- models/issues/issue_project.go | 6 ++ models/project/column.go | 14 ---- models/project/issue.go | 43 ------------ models/project/project.go | 3 + routers/web/org/projects.go | 8 ++- routers/web/repo/projects.go | 2 +- services/projects/issue.go | 117 +++++++++++++++++++++++++++++++++ 7 files changed, 134 insertions(+), 59 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index f520604321c9d..f446d55af1452 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -55,6 +55,9 @@ func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, opts *Is o.ProjectColumnID = b.ID o.ProjectID = b.ProjectID o.SortType = "project-column-sorting" + o.AllPublic = opts.AllPublic + o.User = opts.User + o.Org = opts.Org })) if err != nil { return nil, err @@ -65,6 +68,9 @@ func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, opts *Is ProjectColumnID: db.NoConditionID, ProjectID: b.ProjectID, SortType: "project-column-sorting", + AllPublic: opts.AllPublic, + User: opts.User, + Org: opts.Org, }) if err != nil { return nil, err diff --git a/models/project/column.go b/models/project/column.go index 222f44859928e..f6d6614004796 100644 --- a/models/project/column.go +++ b/models/project/column.go @@ -57,20 +57,6 @@ func (Column) TableName() string { return "project_board" // TODO: the legacy table name should be project_column } -// NumIssues return counter of all issues assigned to the column -func (c *Column) NumIssues(ctx context.Context) int { - total, err := db.GetEngine(ctx).Table("project_issue"). - Where("project_id=?", c.ProjectID). - And("project_board_id=?", c.ID). - GroupBy("issue_id"). - Cols("issue_id"). - Count() - if err != nil { - return 0 - } - return int(total) -} - func (c *Column) GetIssues(ctx context.Context) ([]*ProjectIssue, error) { issues := make([]*ProjectIssue, 0, 5) if err := db.GetEngine(ctx).Where("project_id=?", c.ProjectID). diff --git a/models/project/issue.go b/models/project/issue.go index b4347a9c2b4c6..98eed2a2137b5 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -8,7 +8,6 @@ import ( "fmt" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" ) @@ -34,48 +33,6 @@ func deleteProjectIssuesByProjectID(ctx context.Context, projectID int64) error return err } -// NumIssues return counter of all issues assigned to a project -func (p *Project) NumIssues(ctx context.Context) int { - c, err := db.GetEngine(ctx).Table("project_issue"). - Where("project_id=?", p.ID). - GroupBy("issue_id"). - Cols("issue_id"). - Count() - if err != nil { - log.Error("NumIssues: %v", err) - return 0 - } - return int(c) -} - -// NumClosedIssues return counter of closed issues assigned to a project -func (p *Project) NumClosedIssues(ctx context.Context) int { - c, err := db.GetEngine(ctx).Table("project_issue"). - Join("INNER", "issue", "project_issue.issue_id=issue.id"). - Where("project_issue.project_id=? AND issue.is_closed=?", p.ID, true). - Cols("issue_id"). - Count() - if err != nil { - log.Error("NumClosedIssues: %v", err) - return 0 - } - return int(c) -} - -// NumOpenIssues return counter of open issues assigned to a project -func (p *Project) NumOpenIssues(ctx context.Context) int { - c, err := db.GetEngine(ctx).Table("project_issue"). - Join("INNER", "issue", "project_issue.issue_id=issue.id"). - Where("project_issue.project_id=? AND issue.is_closed=?", p.ID, false). - Cols("issue_id"). - Count() - if err != nil { - log.Error("NumOpenIssues: %v", err) - return 0 - } - return int(c) -} - func (c *Column) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Column) error { if c.ProjectID != newColumn.ProjectID { return fmt.Errorf("columns have to be in the same project") diff --git a/models/project/project.go b/models/project/project.go index 20b5df0b6e4de..d27e0530947fa 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -97,6 +97,9 @@ type Project struct { Type Type RenderedContent template.HTML `xorm:"-"` + NumOpenIssues int64 `xorm:"-"` + NumClosedIssues int64 `xorm:"-"` + NumIssues int64 `xorm:"-"` CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 32da1b41d16d4..9bd3f7c89471f 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -327,6 +327,10 @@ func ViewProject(ctx *context.Context) { ctx.NotFound("", nil) return } + if err := project.LoadOwner(ctx); err != nil { + ctx.ServerError("LoadOwner", err) + return + } columns, err := project.GetColumns(ctx) if err != nil { @@ -340,9 +344,11 @@ func ViewProject(ctx *context.Context) { } assigneeID := ctx.FormInt64("assignee") // TODO: use "optional" but not 0 in the future - issuesMap, err := issues_model.LoadIssuesFromColumnList(ctx, columns, &issues_model.IssuesOptions{ + issuesMap, err := project_service.LoadIssuesFromColumnList(ctx, project, columns, &issues_model.IssuesOptions{ LabelIDs: labelIDs, AssigneeID: optional.Some(assigneeID), + Org: org_model.OrgFromUser(project.Owner), + User: ctx.Doer, }) if err != nil { ctx.ServerError("LoadIssuesOfColumns", err) diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 346132102f6c7..701d9dd6fb721 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -312,7 +312,7 @@ func ViewProject(ctx *context.Context) { assigneeID := ctx.FormInt64("assignee") // TODO: use "optional" but not 0 in the future - issuesMap, err := issues_model.LoadIssuesFromColumnList(ctx, columns, &issues_model.IssuesOptions{ + issuesMap, err := project_service.LoadIssuesFromColumnList(ctx, project, columns, &issues_model.IssuesOptions{ LabelIDs: labelIDs, AssigneeID: optional.Some(assigneeID), }) diff --git a/services/projects/issue.go b/services/projects/issue.go index 6ca0f168060ad..374423ca445f6 100644 --- a/services/projects/issue.go +++ b/services/projects/issue.go @@ -84,3 +84,120 @@ func MoveIssuesOnProjectColumn(ctx context.Context, doer *user_model.User, colum return nil }) } + +func LoadIssuesFromColumnList(ctx context.Context, project *project_model.Project, bs project_model.ColumnList, opts *issues_model.IssuesOptions) (map[int64]issues_model.IssueList, error) { + if project.RepoID > 0 { // repo level project, just load all issues + return issues_model.LoadIssuesFromColumnList(ctx, bs, opts) + } + + // for org level project, we need to filter issues according to the user's access + if opts.User == nil { + opts.AllPublic = true + } + return issues_model.LoadIssuesFromColumnList(ctx, bs, opts) +} + +/* +// NumIssues return counter of all issues assigned to the column +func (c *Column) NumIssues(ctx context.Context) int { + total, err := db.GetEngine(ctx).Table("project_issue"). + Where("project_id=?", c.ProjectID). + And("project_board_id=?", c.ID). + GroupBy("issue_id"). + Cols("issue_id"). + Count() + if err != nil { + return 0 + } + return int(total) +} + +// NumIssues return counter of all issues assigned to a project +func (p *Project) NumIssues(ctx context.Context) int { + c, err := db.GetEngine(ctx).Table("project_issue"). + Where("project_id=?", p.ID). + GroupBy("issue_id"). + Cols("issue_id"). + Count() + if err != nil { + log.Error("NumIssues: %v", err) + return 0 + } + return int(c) +}*/ + +// NumClosedIssues return counter of closed issues assigned to a project +func loadNumClosedIssues(ctx context.Context, p *project_model.Project) error { + cnt, err := db.GetEngine(ctx).Table("project_issue"). + Join("INNER", "issue", "project_issue.issue_id=issue.id"). + Where("project_issue.project_id=? AND issue.is_closed=?", p.ID, true). + Cols("issue_id"). + Count() + if err != nil { + return err + } + p.NumClosedIssues = cnt + return nil +} + +// NumOpenIssues return counter of open issues assigned to a project +func loadNumOpenIssues(ctx context.Context, p *project_model.Project) error { + cnt, err := db.GetEngine(ctx).Table("project_issue"). + Join("INNER", "issue", "project_issue.issue_id=issue.id"). + Where("project_issue.project_id=? AND issue.is_closed=?", p.ID, false). + Cols("issue_id"). + Count() + if err != nil { + return err + } + p.NumOpenIssues = cnt + return nil +} + +func LoadIssueNumbersForProjects(ctx context.Context, projects []*project_model.Project, doer *user_model.User) error { + for _, project := range projects { + if err := LoadIssueNumbersForProject(ctx, project, doer); err != nil { + return err + } + } + return nil +} + +func LoadIssueNumbersForProject(ctx context.Context, project *project_model.Project, doer *user_model.User) error { + if project.OwnerID == 0 { + if err := loadNumClosedIssues(ctx, project); err != nil { + return err + } + if err := loadNumOpenIssues(ctx, project); err != nil { + return err + } + project.NumIssues = project.NumClosedIssues + project.NumOpenIssues + return nil + } + + if err := project.LoadOwner(ctx); err != nil { + return err + } + bs, err := project.GetColumns(ctx) + if err != nil { + return err + } + im, err := LoadIssuesFromColumnList(ctx, project, bs, &issues_model.IssuesOptions{ + User: doer, + Org: org.From(project.Owner), + }) + if err != nil { + return err + } + for _, il := range im { + project.NumIssues += int64(len(il)) + for _, issue := range il { + if issue.IsClosed { + project.NumClosedIssues++ + } else { + project.NumOpenIssues++ + } + } + } + return nil +} From 611862ff7cca2c586a60c87874f0384155b6f782 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 14 Feb 2025 00:07:40 -0800 Subject: [PATCH 02/17] Improve project issues retrieves and numbers --- models/fixtures/project.yml | 12 +++ models/fixtures/project_issue.yml | 12 +++ models/issues/issue_project.go | 34 ++++--- models/issues/issue_search.go | 12 ++- models/project/column.go | 2 + modules/indexer/issues/db/options.go | 3 +- routers/web/org/projects.go | 22 ++++- routers/web/repo/projects.go | 11 ++- routers/web/user/home.go | 2 +- services/projects/issue.go | 116 +++++++++++++--------- services/projects/issue_test.go | 143 +++++++++++++++++++++++++++ services/projects/main_test.go | 16 +++ templates/projects/list.tmpl | 4 +- templates/projects/view.tmpl | 2 +- 14 files changed, 316 insertions(+), 75 deletions(-) create mode 100644 services/projects/issue_test.go create mode 100644 services/projects/main_test.go diff --git a/models/fixtures/project.yml b/models/fixtures/project.yml index 44d87bce04674..526034f8ae3c4 100644 --- a/models/fixtures/project.yml +++ b/models/fixtures/project.yml @@ -69,3 +69,15 @@ type: 2 created_unix: 1688973000 updated_unix: 1688973000 + +- + id: 7 + title: project in an org + owner_id: 3 + repo_id: 0 + is_closed: false + creator_id: 2 + board_type: 1 + type: 2 + created_unix: 1688973000 + updated_unix: 1688973000 diff --git a/models/fixtures/project_issue.yml b/models/fixtures/project_issue.yml index b1af05908aafb..c948952218d90 100644 --- a/models/fixtures/project_issue.yml +++ b/models/fixtures/project_issue.yml @@ -21,3 +21,15 @@ issue_id: 5 project_id: 1 project_board_id: 3 + +- + id: 5 + issue_id: 1 + project_id: 4 + project_board_id: 4 + +- + id: 6 + issue_id: 4 + project_id: 4 + project_board_id: 4 diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index f446d55af1452..b7d43daaf14ea 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -49,6 +49,21 @@ func (issue *Issue) ProjectColumnID(ctx context.Context) (int64, error) { return ip.ProjectColumnID, nil } +func LoadProjectIssueColumnMap(ctx context.Context, projectID, defaultColumnID int64) (map[int64]int64, error) { + issues := make([]project_model.ProjectIssue, 0) + if err := db.GetEngine(ctx).Where("project_id=?", projectID).Find(&issues); err != nil { + return nil, err + } + result := make(map[int64]int64, len(issues)) + for _, issue := range issues { + if issue.ProjectColumnID == 0 { + issue.ProjectColumnID = defaultColumnID + } + result[issue.IssueID] = issue.ProjectColumnID + } + return result, nil +} + // LoadIssuesFromColumn load issues assigned to this column func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, opts *IssuesOptions) (IssueList, error) { issueList, err := Issues(ctx, opts.Copy(func(o *IssuesOptions) { @@ -56,8 +71,9 @@ func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, opts *Is o.ProjectID = b.ProjectID o.SortType = "project-column-sorting" o.AllPublic = opts.AllPublic - o.User = opts.User + o.AccessUser = opts.AccessUser o.Org = opts.Org + o.Owner = opts.Owner })) if err != nil { return nil, err @@ -69,8 +85,9 @@ func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, opts *Is ProjectID: b.ProjectID, SortType: "project-column-sorting", AllPublic: opts.AllPublic, - User: opts.User, + AccessUser: opts.AccessUser, Org: opts.Org, + Owner: opts.Owner, }) if err != nil { return nil, err @@ -85,19 +102,6 @@ func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, opts *Is return issueList, nil } -// LoadIssuesFromColumnList load issues assigned to the columns -func LoadIssuesFromColumnList(ctx context.Context, bs project_model.ColumnList, opts *IssuesOptions) (map[int64]IssueList, error) { - issuesMap := make(map[int64]IssueList, len(bs)) - for i := range bs { - il, err := LoadIssuesFromColumn(ctx, bs[i], opts) - if err != nil { - return nil, err - } - issuesMap[bs[i].ID] = il - } - return issuesMap, nil -} - // IssueAssignOrRemoveProject changes the project associated with an issue // If newProjectID is 0, the issue is removed from the project func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index f1cd125d495c4..f7cd7197a85f0 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -50,8 +50,9 @@ type IssuesOptions struct { //nolint PriorityRepoID int64 IsArchived optional.Option[bool] Org *organization.Organization // issues permission scope + Owner *user_model.User // issues permission scope Team *organization.Team // issues permission scope - User *user_model.User // issues permission scope + AccessUser *user_model.User // issues permission scope } // Copy returns a copy of the options. @@ -273,8 +274,8 @@ func applyConditions(sess *xorm.Session, opts *IssuesOptions) { applyLabelsCondition(sess, opts) - if opts.User != nil { - sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.Value())) + if opts.AccessUser != nil { + sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.AccessUser.ID, opts.Org, opts.Owner, opts.Team, opts.IsPull.Value())) } } @@ -321,7 +322,7 @@ func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Typ } // issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table -func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organization.Organization, team *organization.Team, isPull bool) builder.Cond { +func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organization.Organization, owner *user_model.User, team *organization.Team, isPull bool) builder.Cond { cond := builder.NewCond() unitType := unit.TypeIssues if isPull { @@ -339,6 +340,9 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organizati ) } } else { + if owner != nil { + cond = cond.And(repo_model.UserOwnedRepoCond(owner.ID)) // owned repos + } cond = cond.And( builder.Or( repo_model.UserOwnedRepoCond(userID), // owned repos diff --git a/models/project/column.go b/models/project/column.go index f6d6614004796..c9cbb82ac19e5 100644 --- a/models/project/column.go +++ b/models/project/column.go @@ -48,6 +48,8 @@ type Column struct { ProjectID int64 `xorm:"INDEX NOT NULL"` CreatorID int64 `xorm:"NOT NULL"` + NumIssues int64 `xorm:"-"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` } diff --git a/modules/indexer/issues/db/options.go b/modules/indexer/issues/db/options.go index 42834f6e8863b..c836e0c6ea870 100644 --- a/modules/indexer/issues/db/options.go +++ b/modules/indexer/issues/db/options.go @@ -75,7 +75,8 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m IsArchived: options.IsArchived, Org: nil, Team: nil, - User: nil, + AccessUser: nil, + Owner: nil, } if len(options.MilestoneIDs) == 1 && options.MilestoneIDs[0] == 0 { diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 9bd3f7c89471f..c52061902b721 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -77,6 +77,11 @@ func Projects(ctx *context.Context) { return } + if err := project_service.LoadIssueNumbersForProjects(ctx, projects, ctx.Doer); err != nil { + ctx.ServerError("LoadIssueNumbersForProjects", err) + return + } + opTotal, err := db.Count[project_model.Project](ctx, project_model.SearchOptions{ OwnerID: ctx.ContextUser.ID, IsClosed: optional.Some(!isShowClosed), @@ -344,16 +349,25 @@ func ViewProject(ctx *context.Context) { } assigneeID := ctx.FormInt64("assignee") // TODO: use "optional" but not 0 in the future - issuesMap, err := project_service.LoadIssuesFromColumnList(ctx, project, columns, &issues_model.IssuesOptions{ + opts := issues_model.IssuesOptions{ LabelIDs: labelIDs, AssigneeID: optional.Some(assigneeID), - Org: org_model.OrgFromUser(project.Owner), - User: ctx.Doer, - }) + AccessUser: ctx.Doer, + } + if project.Owner.IsOrganization() { + opts.Org = org_model.OrgFromUser(project.Owner) + } else { + opts.Owner = project.Owner + } + + issuesMap, err := project_service.LoadIssuesFromProject(ctx, project, &opts) if err != nil { ctx.ServerError("LoadIssuesOfColumns", err) return } + for _, column := range columns { + column.NumIssues = int64(len(issuesMap[column.ID])) + } if project.CardType != project_model.CardTypeTextOnly { issuesAttachmentMap := make(map[int64][]*attachment_model.Attachment) diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 701d9dd6fb721..220684f2ea7c4 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -92,6 +92,11 @@ func Projects(ctx *context.Context) { return } + if err := project_service.LoadIssueNumbersForProjects(ctx, projects, ctx.Doer); err != nil { + ctx.ServerError("LoadIssueNumbersForProjects", err) + return + } + for i := range projects { rctx := renderhelper.NewRenderContextRepoComment(ctx, repo) projects[i].RenderedContent, err = markdown.RenderString(rctx, projects[i].Description) @@ -312,7 +317,8 @@ func ViewProject(ctx *context.Context) { assigneeID := ctx.FormInt64("assignee") // TODO: use "optional" but not 0 in the future - issuesMap, err := project_service.LoadIssuesFromColumnList(ctx, project, columns, &issues_model.IssuesOptions{ + issuesMap, err := project_service.LoadIssuesFromProject(ctx, project, &issues_model.IssuesOptions{ + RepoIDs: []int64{ctx.Repo.Repository.ID}, LabelIDs: labelIDs, AssigneeID: optional.Some(assigneeID), }) @@ -320,6 +326,9 @@ func ViewProject(ctx *context.Context) { ctx.ServerError("LoadIssuesOfColumns", err) return } + for _, column := range columns { + column.NumIssues = int64(len(issuesMap[column.ID])) + } if project.CardType != project_model.CardTypeTextOnly { issuesAttachmentMap := make(map[int64][]*repo_model.Attachment) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index ff9334da6e2fa..24eb39ac0e590 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -417,7 +417,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { IsPull: optional.Some(isPullList), SortType: sortType, IsArchived: optional.Some(false), - User: ctx.Doer, + AccessUser: ctx.Doer, } // -------------------------------------------------------------------------- // Build opts (IssuesOptions), which contains filter information. diff --git a/services/projects/issue.go b/services/projects/issue.go index 374423ca445f6..0c9485e90f79e 100644 --- a/services/projects/issue.go +++ b/services/projects/issue.go @@ -9,8 +9,10 @@ import ( "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" + org_model "code.gitea.io/gitea/models/organization" project_model "code.gitea.io/gitea/models/project" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/optional" ) // MoveIssuesOnProjectColumn moves or keeps issues in a column and sorts them inside that column @@ -85,46 +87,49 @@ func MoveIssuesOnProjectColumn(ctx context.Context, doer *user_model.User, colum }) } -func LoadIssuesFromColumnList(ctx context.Context, project *project_model.Project, bs project_model.ColumnList, opts *issues_model.IssuesOptions) (map[int64]issues_model.IssueList, error) { - if project.RepoID > 0 { // repo level project, just load all issues - return issues_model.LoadIssuesFromColumnList(ctx, bs, opts) +// LoadIssuesFromProject load issues assigned to the project +func LoadIssuesFromProject(ctx context.Context, project *project_model.Project, opts *issues_model.IssuesOptions) (map[int64]issues_model.IssueList, error) { + issueList, err := issues_model.Issues(ctx, opts.Copy(func(o *issues_model.IssuesOptions) { + o.ProjectID = project.ID + o.SortType = "project-column-sorting" + o.AllPublic = opts.AllPublic + o.AccessUser = opts.AccessUser + o.Org = opts.Org + o.Owner = opts.Owner + o.LabelIDs = opts.LabelIDs + o.AssigneeID = opts.AssigneeID + })) + if err != nil { + return nil, err } - // for org level project, we need to filter issues according to the user's access - if opts.User == nil { - opts.AllPublic = true + if err := issueList.LoadComments(ctx); err != nil { + return nil, err } - return issues_model.LoadIssuesFromColumnList(ctx, bs, opts) -} -/* -// NumIssues return counter of all issues assigned to the column -func (c *Column) NumIssues(ctx context.Context) int { - total, err := db.GetEngine(ctx).Table("project_issue"). - Where("project_id=?", c.ProjectID). - And("project_board_id=?", c.ID). - GroupBy("issue_id"). - Cols("issue_id"). - Count() + defaultColumn, err := project.GetDefaultColumn(ctx) if err != nil { - return 0 + return nil, err } - return int(total) -} -// NumIssues return counter of all issues assigned to a project -func (p *Project) NumIssues(ctx context.Context) int { - c, err := db.GetEngine(ctx).Table("project_issue"). - Where("project_id=?", p.ID). - GroupBy("issue_id"). - Cols("issue_id"). - Count() + issueColumnMap, err := issues_model.LoadProjectIssueColumnMap(ctx, project.ID, defaultColumn.ID) if err != nil { - log.Error("NumIssues: %v", err) - return 0 + return nil, err + } + + results := make(map[int64]issues_model.IssueList) + for _, issue := range issueList { + projectColumnID, ok := issueColumnMap[issue.ID] + if !ok { + continue + } + if _, ok := results[projectColumnID]; !ok { + results[projectColumnID] = make(issues_model.IssueList, 0) + } + results[projectColumnID] = append(results[projectColumnID], issue) } - return int(c) -}*/ + return results, nil +} // NumClosedIssues return counter of closed issues assigned to a project func loadNumClosedIssues(ctx context.Context, p *project_model.Project) error { @@ -164,6 +169,7 @@ func LoadIssueNumbersForProjects(ctx context.Context, projects []*project_model. } func LoadIssueNumbersForProject(ctx context.Context, project *project_model.Project, doer *user_model.User) error { + // for repository project, just get the numbers if project.OwnerID == 0 { if err := loadNumClosedIssues(ctx, project); err != nil { return err @@ -175,29 +181,47 @@ func LoadIssueNumbersForProject(ctx context.Context, project *project_model.Proj return nil } + // for user or org projects, we need to check access permissions + opts := issues_model.IssuesOptions{ + ProjectID: project.ID, + AccessUser: doer, + AllPublic: doer == nil, + } if err := project.LoadOwner(ctx); err != nil { return err } - bs, err := project.GetColumns(ctx) + if project.Owner.IsOrganization() { + opts.Org = org_model.OrgFromUser(project.Owner) + } else { + opts.Owner = project.Owner + } + + var err error + project.NumOpenIssues, err = issues_model.CountIssues(ctx, opts.Copy(func(o *issues_model.IssuesOptions) { + o.ProjectID = opts.ProjectID + o.AllPublic = opts.AllPublic + o.AccessUser = opts.AccessUser + o.Org = opts.Org + o.Owner = opts.Owner + o.IsClosed = optional.Some(false) + })) if err != nil { return err } - im, err := LoadIssuesFromColumnList(ctx, project, bs, &issues_model.IssuesOptions{ - User: doer, - Org: org.From(project.Owner), - }) + + project.NumClosedIssues, err = issues_model.CountIssues(ctx, opts.Copy(func(o *issues_model.IssuesOptions) { + o.ProjectID = opts.ProjectID + o.AllPublic = opts.AllPublic + o.AccessUser = opts.AccessUser + o.Org = opts.Org + o.Owner = opts.Owner + o.IsClosed = optional.Some(true) + })) if err != nil { return err } - for _, il := range im { - project.NumIssues += int64(len(il)) - for _, issue := range il { - if issue.IsClosed { - project.NumClosedIssues++ - } else { - project.NumOpenIssues++ - } - } - } + + project.NumIssues = project.NumClosedIssues + project.NumOpenIssues + return nil } diff --git a/services/projects/issue_test.go b/services/projects/issue_test.go new file mode 100644 index 0000000000000..2a6c2bb744d65 --- /dev/null +++ b/services/projects/issue_test.go @@ -0,0 +1,143 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package project + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + issues_model "code.gitea.io/gitea/models/issues" + org_model "code.gitea.io/gitea/models/organization" + project_model "code.gitea.io/gitea/models/project" + repo_model "code.gitea.io/gitea/models/repository" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" +) + +func Test_Projects(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + org3 := unittest.AssertExistsAndLoadBean(t, &org_model.Organization{ID: 3}) + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + + t.Run("User projects", func(t *testing.T) { + projects, err := db.Find[project_model.Project](db.DefaultContext, project_model.SearchOptions{ + OwnerID: user2.ID, + }) + assert.NoError(t, err) + assert.Len(t, projects, 3) + assert.EqualValues(t, 4, projects[0].ID) + + t.Run("Authenticated user", func(t *testing.T) { + columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ + Owner: user2, + AccessUser: user2, + }) + assert.NoError(t, err) + assert.Len(t, columnIssues, 1) // 4 has 2 issues, 6 will not contains here because 0 issues + assert.Len(t, columnIssues[4], 2) // user2 can visit both issues, one from public repository one from private repository + }) + + t.Run("Anonymous user", func(t *testing.T) { + columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ + AllPublic: true, + }) + assert.NoError(t, err) + assert.Len(t, columnIssues, 1) + assert.Len(t, columnIssues[4], 1) // anonymous user can only visit public repo issues + }) + + t.Run("Authenticated user with no permission to the private repo", func(t *testing.T) { + columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ + Owner: user2, + AccessUser: user4, + }) + assert.NoError(t, err) + assert.Len(t, columnIssues, 1) + assert.Len(t, columnIssues[4], 1) // user4 can only visit public repo issues + }) + }) + + t.Run("Org projects", func(t *testing.T) { + projects, err := db.Find[project_model.Project](db.DefaultContext, project_model.SearchOptions{ + OwnerID: org3.ID, + }) + assert.NoError(t, err) + assert.Len(t, projects, 1) + assert.EqualValues(t, 7, projects[0].ID) + + t.Run("Authenticated user", func(t *testing.T) { + columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ + Org: org3, + AccessUser: user2, + }) + assert.NoError(t, err) + assert.Len(t, columnIssues, 1) // 4 has 2 issues, 6 will not contains here because 0 issues + assert.Len(t, columnIssues[4], 2) // user2 can visit both issues, one from public repository one from private repository + }) + + t.Run("Anonymous user", func(t *testing.T) { + columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ + AllPublic: true, + }) + assert.NoError(t, err) + assert.Len(t, columnIssues, 1) + assert.Len(t, columnIssues[4], 1) // anonymous user can only visit public repo issues + }) + + t.Run("Authenticated user with no permission to the private repo", func(t *testing.T) { + columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ + Org: org3, + AccessUser: user4, + }) + assert.NoError(t, err) + assert.Len(t, columnIssues, 1) + assert.Len(t, columnIssues[4], 1) // user4 can only visit public repo issues + }) + }) + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + t.Run("Repository projects", func(t *testing.T) { + projects, err := db.Find[project_model.Project](db.DefaultContext, project_model.SearchOptions{ + RepoID: repo1.ID, + }) + assert.NoError(t, err) + assert.Len(t, projects, 1) + assert.EqualValues(t, 1, projects[0].ID) + + t.Run("Authenticated user", func(t *testing.T) { + columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ + RepoIDs: []int64{repo1.ID}, + AccessUser: user2, + }) + assert.NoError(t, err) + assert.Len(t, columnIssues, 1) // 4 has 2 issues, 6 will not contains here because 0 issues + assert.Len(t, columnIssues[4], 2) // user2 can visit both issues, one from public repository one from private repository + }) + + t.Run("Anonymous user", func(t *testing.T) { + columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ + RepoIDs: []int64{repo1.ID}, + AllPublic: true, + }) + assert.NoError(t, err) + assert.Len(t, columnIssues, 1) + assert.Len(t, columnIssues[4], 1) // anonymous user can only visit public repo issues + }) + + t.Run("Authenticated user with no permission to the private repo", func(t *testing.T) { + columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ + RepoIDs: []int64{repo1.ID}, + AccessUser: user4, + }) + assert.NoError(t, err) + assert.Len(t, columnIssues, 1) + assert.Len(t, columnIssues[4], 1) // user4 can only visit public repo issues + }) + }) +} diff --git a/services/projects/main_test.go b/services/projects/main_test.go new file mode 100644 index 0000000000000..45703e1ce5a89 --- /dev/null +++ b/services/projects/main_test.go @@ -0,0 +1,16 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package project + +import ( + "testing" + + _ "code.gitea.io/gitea/models/actions" + _ "code.gitea.io/gitea/models/activities" + "code.gitea.io/gitea/models/unittest" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m) +} diff --git a/templates/projects/list.tmpl b/templates/projects/list.tmpl index 7c75585bf7320..5d40653dc672c 100644 --- a/templates/projects/list.tmpl +++ b/templates/projects/list.tmpl @@ -54,11 +54,11 @@
{{svg "octicon-issue-opened" 14}} - {{ctx.Locale.PrettyNumber (.NumOpenIssues ctx)}} {{ctx.Locale.Tr "repo.issues.open_title"}} + {{ctx.Locale.PrettyNumber .NumOpenIssues}} {{ctx.Locale.Tr "repo.issues.open_title"}}
{{svg "octicon-check" 14}} - {{ctx.Locale.PrettyNumber (.NumClosedIssues ctx)}} {{ctx.Locale.Tr "repo.issues.closed_title"}} + {{ctx.Locale.PrettyNumber .NumClosedIssues}} {{ctx.Locale.Tr "repo.issues.closed_title"}}
{{if and $.CanWriteProjects (not $.Repository.IsArchived)}} diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 2550cae69c2bc..d60943a3de405 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -70,7 +70,7 @@
- {{.NumIssues ctx}} + {{.NumIssues}}
{{.Title}}
{{if $canWriteProject}} From 374e11e929cba9ffc681bdd95de769d189692934 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 14 Feb 2025 00:14:26 -0800 Subject: [PATCH 03/17] Fix checks --- services/projects/issue_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/projects/issue_test.go b/services/projects/issue_test.go index 2a6c2bb744d65..d50487a326a5c 100644 --- a/services/projects/issue_test.go +++ b/services/projects/issue_test.go @@ -10,7 +10,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" org_model "code.gitea.io/gitea/models/organization" project_model "code.gitea.io/gitea/models/project" - repo_model "code.gitea.io/gitea/models/repository" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" From 134f6915cab217d79ded81b132b21ea6cf1e6629 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 14 Feb 2025 18:14:04 -0800 Subject: [PATCH 04/17] Adjust tests --- models/fixtures/project.yml | 12 ----- models/fixtures/project_issue.yml | 12 ----- models/issues/issue_search.go | 11 ++-- services/projects/issue_test.go | 89 ++++++++++++++++++++++++------- 4 files changed, 78 insertions(+), 46 deletions(-) diff --git a/models/fixtures/project.yml b/models/fixtures/project.yml index 526034f8ae3c4..44d87bce04674 100644 --- a/models/fixtures/project.yml +++ b/models/fixtures/project.yml @@ -69,15 +69,3 @@ type: 2 created_unix: 1688973000 updated_unix: 1688973000 - -- - id: 7 - title: project in an org - owner_id: 3 - repo_id: 0 - is_closed: false - creator_id: 2 - board_type: 1 - type: 2 - created_unix: 1688973000 - updated_unix: 1688973000 diff --git a/models/fixtures/project_issue.yml b/models/fixtures/project_issue.yml index c948952218d90..b1af05908aafb 100644 --- a/models/fixtures/project_issue.yml +++ b/models/fixtures/project_issue.yml @@ -21,15 +21,3 @@ issue_id: 5 project_id: 1 project_board_id: 3 - -- - id: 5 - issue_id: 1 - project_id: 4 - project_board_id: 4 - -- - id: 6 - issue_id: 4 - project_id: 4 - project_board_id: 4 diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index f7cd7197a85f0..2fab4c7fb72d2 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -274,7 +274,13 @@ func applyConditions(sess *xorm.Session, opts *IssuesOptions) { applyLabelsCondition(sess, opts) - if opts.AccessUser != nil { + if opts.Org != nil { + sess.And(repo_model.UserOwnedRepoCond(opts.Org.ID)) + } else if opts.Owner != nil { + sess.And(repo_model.UserOwnedRepoCond(opts.Owner.ID)) + } + + if opts.AccessUser != nil && !opts.AccessUser.IsAdmin { sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.AccessUser.ID, opts.Org, opts.Owner, opts.Team, opts.IsPull.Value())) } } @@ -340,9 +346,6 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organizati ) } } else { - if owner != nil { - cond = cond.And(repo_model.UserOwnedRepoCond(owner.ID)) // owned repos - } cond = cond.And( builder.Or( repo_model.UserOwnedRepoCond(userID), // owned repos diff --git a/services/projects/issue_test.go b/services/projects/issue_test.go index d50487a326a5c..3e5d184082f38 100644 --- a/services/projects/issue_test.go +++ b/services/projects/issue_test.go @@ -20,11 +20,26 @@ import ( func Test_Projects(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) + userAdmin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) org3 := unittest.AssertExistsAndLoadBean(t, &org_model.Organization{ID: 3}) user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) t.Run("User projects", func(t *testing.T) { + err := db.Insert(db.DefaultContext, &project_model.ProjectIssue{ + ProjectID: 4, + IssueID: 1, + ProjectColumnID: 4, + }) + assert.NoError(t, err) + + err = db.Insert(db.DefaultContext, &project_model.ProjectIssue{ + ProjectID: 4, + IssueID: 4, + ProjectColumnID: 4, + }) + assert.NoError(t, err) + projects, err := db.Find[project_model.Project](db.DefaultContext, project_model.SearchOptions{ OwnerID: user2.ID, }) @@ -63,21 +78,54 @@ func Test_Projects(t *testing.T) { }) t.Run("Org projects", func(t *testing.T) { + project1 := project_model.Project{ + Title: "project in an org", + OwnerID: org3.ID, + Type: project_model.TypeOrganization, + TemplateType: project_model.TemplateTypeBasicKanban, + } + err := project_model.NewProject(db.DefaultContext, &project1) + assert.NoError(t, err) + + column1 := project_model.Column{ + Title: "column 1", + ProjectID: project1.ID, + } + err = project_model.NewColumn(db.DefaultContext, &column1) + assert.NoError(t, err) + + column2 := project_model.Column{ + Title: "column 2", + ProjectID: project1.ID, + } + err = project_model.NewColumn(db.DefaultContext, &column2) + assert.NoError(t, err) + + // issue 6 belongs to private repo 3 under org 3 + issue6 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 6}) + err = issues_model.IssueAssignOrRemoveProject(db.DefaultContext, issue6, user2, project1.ID, column1.ID) + assert.NoError(t, err) + + // issue 16 belongs to public repo 16 under org 3 + issue16 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 16}) + err = issues_model.IssueAssignOrRemoveProject(db.DefaultContext, issue16, user2, project1.ID, column1.ID) + assert.NoError(t, err) + projects, err := db.Find[project_model.Project](db.DefaultContext, project_model.SearchOptions{ OwnerID: org3.ID, }) assert.NoError(t, err) assert.Len(t, projects, 1) - assert.EqualValues(t, 7, projects[0].ID) + assert.EqualValues(t, project1.ID, projects[0].ID) t.Run("Authenticated user", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ Org: org3, - AccessUser: user2, + AccessUser: userAdmin, }) assert.NoError(t, err) - assert.Len(t, columnIssues, 1) // 4 has 2 issues, 6 will not contains here because 0 issues - assert.Len(t, columnIssues[4], 2) // user2 can visit both issues, one from public repository one from private repository + assert.Len(t, columnIssues, 1) // column1 has 2 issues, 6 will not contains here because 0 issues + assert.Len(t, columnIssues[column1.ID], 2) // user2 can visit both issues, one from public repository one from private repository }) t.Run("Anonymous user", func(t *testing.T) { @@ -86,23 +134,23 @@ func Test_Projects(t *testing.T) { }) assert.NoError(t, err) assert.Len(t, columnIssues, 1) - assert.Len(t, columnIssues[4], 1) // anonymous user can only visit public repo issues + assert.Len(t, columnIssues[column1.ID], 1) // anonymous user can only visit public repo issues }) t.Run("Authenticated user with no permission to the private repo", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ Org: org3, - AccessUser: user4, + AccessUser: user2, }) assert.NoError(t, err) assert.Len(t, columnIssues, 1) - assert.Len(t, columnIssues[4], 1) // user4 can only visit public repo issues + assert.Len(t, columnIssues[column1.ID], 1) // user4 can only visit public repo issues }) }) - repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - t.Run("Repository projects", func(t *testing.T) { + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + projects, err := db.Find[project_model.Project](db.DefaultContext, project_model.SearchOptions{ RepoID: repo1.ID, }) @@ -113,31 +161,36 @@ func Test_Projects(t *testing.T) { t.Run("Authenticated user", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ RepoIDs: []int64{repo1.ID}, - AccessUser: user2, + AccessUser: userAdmin, }) assert.NoError(t, err) - assert.Len(t, columnIssues, 1) // 4 has 2 issues, 6 will not contains here because 0 issues - assert.Len(t, columnIssues[4], 2) // user2 can visit both issues, one from public repository one from private repository + assert.Len(t, columnIssues, 3) + assert.Len(t, columnIssues[1], 2) + assert.Len(t, columnIssues[2], 1) + assert.Len(t, columnIssues[3], 1) }) t.Run("Anonymous user", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ - RepoIDs: []int64{repo1.ID}, AllPublic: true, }) assert.NoError(t, err) - assert.Len(t, columnIssues, 1) - assert.Len(t, columnIssues[4], 1) // anonymous user can only visit public repo issues + assert.Len(t, columnIssues, 3) + assert.Len(t, columnIssues[1], 2) + assert.Len(t, columnIssues[2], 1) + assert.Len(t, columnIssues[3], 1) }) t.Run("Authenticated user with no permission to the private repo", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ RepoIDs: []int64{repo1.ID}, - AccessUser: user4, + AccessUser: user2, }) assert.NoError(t, err) - assert.Len(t, columnIssues, 1) - assert.Len(t, columnIssues[4], 1) // user4 can only visit public repo issues + assert.Len(t, columnIssues, 3) + assert.Len(t, columnIssues[1], 2) + assert.Len(t, columnIssues[2], 1) + assert.Len(t, columnIssues[3], 1) }) }) } From 7344a1e26b84e7a2feeefedeb49fc152007a49b8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 14 Feb 2025 20:51:34 -0800 Subject: [PATCH 05/17] Fix lint --- models/issues/issue_search.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index 2fab4c7fb72d2..ccde51aa82500 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -281,7 +281,7 @@ func applyConditions(sess *xorm.Session, opts *IssuesOptions) { } if opts.AccessUser != nil && !opts.AccessUser.IsAdmin { - sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.AccessUser.ID, opts.Org, opts.Owner, opts.Team, opts.IsPull.Value())) + sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.AccessUser.ID, opts.Org, opts.Team, opts.IsPull.Value())) } } @@ -328,7 +328,7 @@ func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Typ } // issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table -func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organization.Organization, owner *user_model.User, team *organization.Team, isPull bool) builder.Cond { +func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organization.Organization, team *organization.Team, isPull bool) builder.Cond { cond := builder.NewCond() unitType := unit.TypeIssues if isPull { From a845cd15f4a6d5e0c31f000f9001a2e08c3a209d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 14 Feb 2025 21:14:30 -0800 Subject: [PATCH 06/17] Fix checks --- services/projects/main_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/projects/main_test.go b/services/projects/main_test.go index 45703e1ce5a89..d39c82a140e5e 100644 --- a/services/projects/main_test.go +++ b/services/projects/main_test.go @@ -6,9 +6,10 @@ package project import ( "testing" + "code.gitea.io/gitea/models/unittest" + _ "code.gitea.io/gitea/models/actions" _ "code.gitea.io/gitea/models/activities" - "code.gitea.io/gitea/models/unittest" ) func TestMain(m *testing.M) { From 5878ceb259c31bf8ee6a65e4ecb036a6c246d938 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 15 Feb 2025 12:12:12 -0800 Subject: [PATCH 07/17] Cleanup tests resources --- services/projects/issue_test.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/services/projects/issue_test.go b/services/projects/issue_test.go index 3e5d184082f38..653b100b7513c 100644 --- a/services/projects/issue_test.go +++ b/services/projects/issue_test.go @@ -26,19 +26,29 @@ func Test_Projects(t *testing.T) { user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) t.Run("User projects", func(t *testing.T) { - err := db.Insert(db.DefaultContext, &project_model.ProjectIssue{ + pi1 := project_model.ProjectIssue{ ProjectID: 4, IssueID: 1, ProjectColumnID: 4, - }) + } + err := db.Insert(db.DefaultContext, &pi1) assert.NoError(t, err) + defer func() { + _, err = db.DeleteByID[project_model.ProjectIssue](db.DefaultContext, pi1.ID) + assert.NoError(t, err) + }() - err = db.Insert(db.DefaultContext, &project_model.ProjectIssue{ + pi2 := project_model.ProjectIssue{ ProjectID: 4, IssueID: 4, ProjectColumnID: 4, - }) + } + err = db.Insert(db.DefaultContext, &pi2) assert.NoError(t, err) + defer func() { + _, err = db.DeleteByID[project_model.ProjectIssue](db.DefaultContext, pi2.ID) + assert.NoError(t, err) + }() projects, err := db.Find[project_model.Project](db.DefaultContext, project_model.SearchOptions{ OwnerID: user2.ID, @@ -86,6 +96,10 @@ func Test_Projects(t *testing.T) { } err := project_model.NewProject(db.DefaultContext, &project1) assert.NoError(t, err) + defer func() { + err := project_model.DeleteProjectByID(db.DefaultContext, project1.ID) + assert.NoError(t, err) + }() column1 := project_model.Column{ Title: "column 1", From 2f2ccd9a3bd36e88614445156c25b2f02a8a4c5a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 15 Feb 2025 15:15:28 -0800 Subject: [PATCH 08/17] Fix get default column --- models/issues/issue_project.go | 2 +- models/project/column.go | 27 ++++++++++++++++++++++++--- models/project/column_test.go | 4 ++-- services/projects/issue.go | 2 +- tests/integration/project_test.go | 2 +- 5 files changed, 29 insertions(+), 8 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index b7d43daaf14ea..29a82e6956c46 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -122,7 +122,7 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo return util.NewPermissionDeniedErrorf("issue %d can't be accessed by project %d", issue.ID, newProject.ID) } if newColumnID == 0 { - newDefaultColumn, err := newProject.GetDefaultColumn(ctx) + newDefaultColumn, err := newProject.MustDefaultColumn(ctx) if err != nil { return err } diff --git a/models/project/column.go b/models/project/column.go index c9cbb82ac19e5..85c53921c1578 100644 --- a/models/project/column.go +++ b/models/project/column.go @@ -180,7 +180,7 @@ func deleteColumnByID(ctx context.Context, columnID int64) error { if err != nil { return err } - defaultColumn, err := project.GetDefaultColumn(ctx) + defaultColumn, err := project.MustDefaultColumn(ctx) if err != nil { return err } @@ -245,8 +245,8 @@ func (p *Project) GetColumns(ctx context.Context) (ColumnList, error) { return columns, nil } -// GetDefaultColumn return default column and ensure only one exists -func (p *Project) GetDefaultColumn(ctx context.Context) (*Column, error) { +// getDefaultColumn return default column and ensure only one exists +func (p *Project) getDefaultColumn(ctx context.Context) (*Column, error) { var column Column has, err := db.GetEngine(ctx). Where("project_id=? AND `default` = ?", p.ID, true). @@ -255,6 +255,27 @@ func (p *Project) GetDefaultColumn(ctx context.Context) (*Column, error) { return nil, err } + if has { + return &column, nil + } + return nil, ErrProjectColumnNotExist{ColumnID: 0} +} + +// MustDefaultColumn returns the default column for a project, get the first one if exist one and creating one if it does not exist +func (p *Project) MustDefaultColumn(ctx context.Context) (*Column, error) { + c, err := p.getDefaultColumn(ctx) + if err != nil && !IsErrProjectColumnNotExist(err) { + return nil, err + } + if c != nil { + return c, nil + } + + var column Column + has, err := db.GetEngine(ctx).Where("project_id=?", p.ID).OrderBy("sorting, id").Get(&column) + if err != nil { + return nil, err + } if has { return &column, nil } diff --git a/models/project/column_test.go b/models/project/column_test.go index 566667e45d133..791f18e7298e1 100644 --- a/models/project/column_test.go +++ b/models/project/column_test.go @@ -20,7 +20,7 @@ func TestGetDefaultColumn(t *testing.T) { assert.NoError(t, err) // check if default column was added - column, err := projectWithoutDefault.GetDefaultColumn(db.DefaultContext) + column, err := projectWithoutDefault.MustDefaultColumn(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(5), column.ProjectID) assert.Equal(t, "Uncategorized", column.Title) @@ -29,7 +29,7 @@ func TestGetDefaultColumn(t *testing.T) { assert.NoError(t, err) // check if multiple defaults were removed - column, err = projectWithMultipleDefaults.GetDefaultColumn(db.DefaultContext) + column, err = projectWithMultipleDefaults.MustDefaultColumn(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(6), column.ProjectID) assert.Equal(t, int64(9), column.ID) diff --git a/services/projects/issue.go b/services/projects/issue.go index 0c9485e90f79e..2c5282123af39 100644 --- a/services/projects/issue.go +++ b/services/projects/issue.go @@ -107,7 +107,7 @@ func LoadIssuesFromProject(ctx context.Context, project *project_model.Project, return nil, err } - defaultColumn, err := project.GetDefaultColumn(ctx) + defaultColumn, err := project.MustDefaultColumn(ctx) if err != nil { return nil, err } diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index cdff9aa2fdc62..111356b1da220 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -78,7 +78,7 @@ func TestMoveRepoProjectColumns(t *testing.T) { columnsAfter, err := project1.GetColumns(db.DefaultContext) assert.NoError(t, err) - assert.Len(t, columns, 3) + assert.Len(t, columnsAfter, 3) assert.EqualValues(t, columns[1].ID, columnsAfter[0].ID) assert.EqualValues(t, columns[2].ID, columnsAfter[1].ID) assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) From 113350320f7bab3d452e8304b8e1e30a661f8ed8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 15 Feb 2025 15:57:28 -0800 Subject: [PATCH 09/17] Fix test --- models/project/column.go | 4 ++++ models/project/column_test.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/models/project/column.go b/models/project/column.go index 85c53921c1578..874a8b9ae145c 100644 --- a/models/project/column.go +++ b/models/project/column.go @@ -277,6 +277,10 @@ func (p *Project) MustDefaultColumn(ctx context.Context) (*Column, error) { return nil, err } if has { + column.Default = true + if _, err := db.GetEngine(ctx).ID(column.ID).Cols("`default`").Update(&column); err != nil { + return nil, err + } return &column, nil } diff --git a/models/project/column_test.go b/models/project/column_test.go index 791f18e7298e1..3bf7681aca220 100644 --- a/models/project/column_test.go +++ b/models/project/column_test.go @@ -23,7 +23,7 @@ func TestGetDefaultColumn(t *testing.T) { column, err := projectWithoutDefault.MustDefaultColumn(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(5), column.ProjectID) - assert.Equal(t, "Uncategorized", column.Title) + assert.Equal(t, "Done", column.Title) projectWithMultipleDefaults, err := GetProjectByID(db.DefaultContext, 6) assert.NoError(t, err) From b63e059c5f7f1589e85147d9638325a2024ba738 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 16 Feb 2025 12:20:45 -0800 Subject: [PATCH 10/17] Improvements --- models/issues/issue_project.go | 18 ++++----------- models/issues/issue_search.go | 25 +++++++++----------- modules/indexer/issues/db/options.go | 5 ++-- routers/web/org/projects.go | 8 ++----- routers/web/user/home.go | 4 ++-- services/projects/issue.go | 34 ++++++---------------------- services/projects/issue_test.go | 24 ++++++++++---------- 7 files changed, 41 insertions(+), 77 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 29a82e6956c46..01852447834c7 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -70,25 +70,17 @@ func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, opts *Is o.ProjectColumnID = b.ID o.ProjectID = b.ProjectID o.SortType = "project-column-sorting" - o.AllPublic = opts.AllPublic - o.AccessUser = opts.AccessUser - o.Org = opts.Org - o.Owner = opts.Owner })) if err != nil { return nil, err } if b.Default { - issues, err := Issues(ctx, &IssuesOptions{ - ProjectColumnID: db.NoConditionID, - ProjectID: b.ProjectID, - SortType: "project-column-sorting", - AllPublic: opts.AllPublic, - AccessUser: opts.AccessUser, - Org: opts.Org, - Owner: opts.Owner, - }) + issues, err := Issues(ctx, opts.Copy(func(o *IssuesOptions) { + o.ProjectColumnID = db.NoConditionID + o.ProjectID = b.ProjectID + o.SortType = "project-column-sorting" + })) if err != nil { return nil, err } diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index ccde51aa82500..43f2d0c6cfd2a 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -49,10 +49,9 @@ type IssuesOptions struct { //nolint // prioritize issues from this repo PriorityRepoID int64 IsArchived optional.Option[bool] - Org *organization.Organization // issues permission scope - Owner *user_model.User // issues permission scope - Team *organization.Team // issues permission scope - AccessUser *user_model.User // issues permission scope + Owner *user_model.User // issues permission scope, it could be an organization or a user + Team *organization.Team // issues permission scope + Doer *user_model.User // issues permission scope } // Copy returns a copy of the options. @@ -274,14 +273,12 @@ func applyConditions(sess *xorm.Session, opts *IssuesOptions) { applyLabelsCondition(sess, opts) - if opts.Org != nil { - sess.And(repo_model.UserOwnedRepoCond(opts.Org.ID)) - } else if opts.Owner != nil { + if opts.Owner != nil { sess.And(repo_model.UserOwnedRepoCond(opts.Owner.ID)) } - if opts.AccessUser != nil && !opts.AccessUser.IsAdmin { - sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.AccessUser.ID, opts.Org, opts.Team, opts.IsPull.Value())) + if opts.Doer != nil { + sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.Doer.ID, opts.Owner, opts.Team, opts.IsPull.Value())) } } @@ -328,20 +325,20 @@ func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Typ } // issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table -func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organization.Organization, team *organization.Team, isPull bool) builder.Cond { +func issuePullAccessibleRepoCond(repoIDstr string, userID int64, owner *user_model.User, team *organization.Team, isPull bool) builder.Cond { cond := builder.NewCond() unitType := unit.TypeIssues if isPull { unitType = unit.TypePullRequests } - if org != nil { + if owner != nil && owner.IsOrganization() { if team != nil { - cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) // special team member repos + cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, owner.ID, team.ID, unitType)) // special team member repos } else { cond = cond.And( builder.Or( - repo_model.UserOrgUnitRepoCond(repoIDstr, userID, org.ID, unitType), // team member repos - repo_model.UserOrgPublicUnitRepoCond(userID, org.ID), // user org public non-member repos, TODO: check repo has issues + repo_model.UserOrgUnitRepoCond(repoIDstr, userID, owner.ID, unitType), // team member repos + repo_model.UserOrgPublicUnitRepoCond(userID, owner.ID), // user org public non-member repos, TODO: check repo has issues ), ) } diff --git a/modules/indexer/issues/db/options.go b/modules/indexer/issues/db/options.go index c836e0c6ea870..87ce398a202d0 100644 --- a/modules/indexer/issues/db/options.go +++ b/modules/indexer/issues/db/options.go @@ -73,10 +73,9 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m UpdatedBeforeUnix: options.UpdatedBeforeUnix.Value(), PriorityRepoID: 0, IsArchived: options.IsArchived, - Org: nil, - Team: nil, - AccessUser: nil, Owner: nil, + Team: nil, + Doer: nil, } if len(options.MilestoneIDs) == 1 && options.MilestoneIDs[0] == 0 { diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index c52061902b721..cbbc51d8e9282 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -352,12 +352,8 @@ func ViewProject(ctx *context.Context) { opts := issues_model.IssuesOptions{ LabelIDs: labelIDs, AssigneeID: optional.Some(assigneeID), - AccessUser: ctx.Doer, - } - if project.Owner.IsOrganization() { - opts.Org = org_model.OrgFromUser(project.Owner) - } else { - opts.Owner = project.Owner + Owner: project.Owner, + Doer: ctx.Doer, } issuesMap, err := project_service.LoadIssuesFromProject(ctx, project, &opts) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 24eb39ac0e590..12c0b36678660 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -417,7 +417,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { IsPull: optional.Some(isPullList), SortType: sortType, IsArchived: optional.Some(false), - AccessUser: ctx.Doer, + Doer: ctx.Doer, } // -------------------------------------------------------------------------- // Build opts (IssuesOptions), which contains filter information. @@ -429,7 +429,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { // Get repository IDs where User/Org/Team has access. if ctx.Org != nil && ctx.Org.Organization != nil { - opts.Org = ctx.Org.Organization + opts.Owner = ctx.Org.Organization.AsUser() opts.Team = ctx.Org.Team issue.PrepareFilterIssueLabels(ctx, 0, ctx.Org.Organization.AsUser()) diff --git a/services/projects/issue.go b/services/projects/issue.go index 2c5282123af39..1d63bbe0912b3 100644 --- a/services/projects/issue.go +++ b/services/projects/issue.go @@ -9,7 +9,6 @@ import ( "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" - org_model "code.gitea.io/gitea/models/organization" project_model "code.gitea.io/gitea/models/project" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/optional" @@ -92,12 +91,6 @@ func LoadIssuesFromProject(ctx context.Context, project *project_model.Project, issueList, err := issues_model.Issues(ctx, opts.Copy(func(o *issues_model.IssuesOptions) { o.ProjectID = project.ID o.SortType = "project-column-sorting" - o.AllPublic = opts.AllPublic - o.AccessUser = opts.AccessUser - o.Org = opts.Org - o.Owner = opts.Owner - o.LabelIDs = opts.LabelIDs - o.AssigneeID = opts.AssigneeID })) if err != nil { return nil, err @@ -181,28 +174,20 @@ func LoadIssueNumbersForProject(ctx context.Context, project *project_model.Proj return nil } - // for user or org projects, we need to check access permissions - opts := issues_model.IssuesOptions{ - ProjectID: project.ID, - AccessUser: doer, - AllPublic: doer == nil, - } if err := project.LoadOwner(ctx); err != nil { return err } - if project.Owner.IsOrganization() { - opts.Org = org_model.OrgFromUser(project.Owner) - } else { - opts.Owner = project.Owner + + // for user or org projects, we need to check access permissions + opts := issues_model.IssuesOptions{ + ProjectID: project.ID, + Doer: doer, + AllPublic: doer == nil, + Owner: project.Owner, } var err error project.NumOpenIssues, err = issues_model.CountIssues(ctx, opts.Copy(func(o *issues_model.IssuesOptions) { - o.ProjectID = opts.ProjectID - o.AllPublic = opts.AllPublic - o.AccessUser = opts.AccessUser - o.Org = opts.Org - o.Owner = opts.Owner o.IsClosed = optional.Some(false) })) if err != nil { @@ -210,11 +195,6 @@ func LoadIssueNumbersForProject(ctx context.Context, project *project_model.Proj } project.NumClosedIssues, err = issues_model.CountIssues(ctx, opts.Copy(func(o *issues_model.IssuesOptions) { - o.ProjectID = opts.ProjectID - o.AllPublic = opts.AllPublic - o.AccessUser = opts.AccessUser - o.Org = opts.Org - o.Owner = opts.Owner o.IsClosed = optional.Some(true) })) if err != nil { diff --git a/services/projects/issue_test.go b/services/projects/issue_test.go index 653b100b7513c..b6f0b1dae11a7 100644 --- a/services/projects/issue_test.go +++ b/services/projects/issue_test.go @@ -59,8 +59,8 @@ func Test_Projects(t *testing.T) { t.Run("Authenticated user", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ - Owner: user2, - AccessUser: user2, + Owner: user2, + Doer: user2, }) assert.NoError(t, err) assert.Len(t, columnIssues, 1) // 4 has 2 issues, 6 will not contains here because 0 issues @@ -78,8 +78,8 @@ func Test_Projects(t *testing.T) { t.Run("Authenticated user with no permission to the private repo", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ - Owner: user2, - AccessUser: user4, + Owner: user2, + Doer: user4, }) assert.NoError(t, err) assert.Len(t, columnIssues, 1) @@ -134,8 +134,8 @@ func Test_Projects(t *testing.T) { t.Run("Authenticated user", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ - Org: org3, - AccessUser: userAdmin, + Owner: org3.AsUser(), + Doer: userAdmin, }) assert.NoError(t, err) assert.Len(t, columnIssues, 1) // column1 has 2 issues, 6 will not contains here because 0 issues @@ -153,8 +153,8 @@ func Test_Projects(t *testing.T) { t.Run("Authenticated user with no permission to the private repo", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ - Org: org3, - AccessUser: user2, + Owner: org3.AsUser(), + Doer: user2, }) assert.NoError(t, err) assert.Len(t, columnIssues, 1) @@ -174,8 +174,8 @@ func Test_Projects(t *testing.T) { t.Run("Authenticated user", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ - RepoIDs: []int64{repo1.ID}, - AccessUser: userAdmin, + RepoIDs: []int64{repo1.ID}, + Doer: userAdmin, }) assert.NoError(t, err) assert.Len(t, columnIssues, 3) @@ -197,8 +197,8 @@ func Test_Projects(t *testing.T) { t.Run("Authenticated user with no permission to the private repo", func(t *testing.T) { columnIssues, err := LoadIssuesFromProject(db.DefaultContext, projects[0], &issues_model.IssuesOptions{ - RepoIDs: []int64{repo1.ID}, - AccessUser: user2, + RepoIDs: []int64{repo1.ID}, + Doer: user2, }) assert.NoError(t, err) assert.Len(t, columnIssues, 3) From 0b82fed350c20d4e2d667d2276fd7bc09ba9ad33 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 16 Feb 2025 12:54:56 -0800 Subject: [PATCH 11/17] Fix test --- models/issues/issue_search.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index 43f2d0c6cfd2a..694b918755dda 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -277,7 +277,7 @@ func applyConditions(sess *xorm.Session, opts *IssuesOptions) { sess.And(repo_model.UserOwnedRepoCond(opts.Owner.ID)) } - if opts.Doer != nil { + if opts.Doer != nil && !opts.Doer.IsAdmin { sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.Doer.ID, opts.Owner, opts.Team, opts.IsPull.Value())) } } From 25ebf7caaad676ae11e7c0cf1418eef54367ab38 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 16 Feb 2025 17:48:44 -0800 Subject: [PATCH 12/17] Apply suggestions from code review Co-authored-by: delvh --- models/project/column.go | 4 +++- services/projects/issue.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/models/project/column.go b/models/project/column.go index 874a8b9ae145c..5f581b58804cb 100644 --- a/models/project/column.go +++ b/models/project/column.go @@ -261,7 +261,9 @@ func (p *Project) getDefaultColumn(ctx context.Context) (*Column, error) { return nil, ErrProjectColumnNotExist{ColumnID: 0} } -// MustDefaultColumn returns the default column for a project, get the first one if exist one and creating one if it does not exist +// MustDefaultColumn returns the default column for a project. +// If one exists, it is returned +// If none exists, the first column will be elevated to the default column of this project func (p *Project) MustDefaultColumn(ctx context.Context) (*Column, error) { c, err := p.getDefaultColumn(ctx) if err != nil && !IsErrProjectColumnNotExist(err) { diff --git a/services/projects/issue.go b/services/projects/issue.go index 1d63bbe0912b3..090d19d2f4700 100644 --- a/services/projects/issue.go +++ b/services/projects/issue.go @@ -86,7 +86,7 @@ func MoveIssuesOnProjectColumn(ctx context.Context, doer *user_model.User, colum }) } -// LoadIssuesFromProject load issues assigned to the project +// LoadIssuesFromProject load issues assigned to each project column inside the given project func LoadIssuesFromProject(ctx context.Context, project *project_model.Project, opts *issues_model.IssuesOptions) (map[int64]issues_model.IssueList, error) { issueList, err := issues_model.Issues(ctx, opts.Copy(func(o *issues_model.IssuesOptions) { o.ProjectID = project.ID From 5a106cf8c6b11df443f5a9f2c69232f65d741116 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 16 Feb 2025 17:54:56 -0800 Subject: [PATCH 13/17] Use a different sort when getting default column of project --- models/project/column.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/project/column.go b/models/project/column.go index 874a8b9ae145c..4042d2598fdbc 100644 --- a/models/project/column.go +++ b/models/project/column.go @@ -250,7 +250,7 @@ func (p *Project) getDefaultColumn(ctx context.Context) (*Column, error) { var column Column has, err := db.GetEngine(ctx). Where("project_id=? AND `default` = ?", p.ID, true). - Desc("id").Get(&column) + OrderBy("sorting, id").Get(&column) if err != nil { return nil, err } From e85927659054e3871c5d4e7c4796cd36dc22b08e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 16 Feb 2025 19:23:22 -0800 Subject: [PATCH 14/17] Fix test because of get default logic change --- models/project/column_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/project/column_test.go b/models/project/column_test.go index 3bf7681aca220..f547e4582061a 100644 --- a/models/project/column_test.go +++ b/models/project/column_test.go @@ -32,7 +32,7 @@ func TestGetDefaultColumn(t *testing.T) { column, err = projectWithMultipleDefaults.MustDefaultColumn(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(6), column.ProjectID) - assert.Equal(t, int64(9), column.ID) + assert.Equal(t, int64(8), column.ID) // sorted by sorting, id 8 is the first default column // set 8 as default column assert.NoError(t, SetDefaultColumn(db.DefaultContext, column.ProjectID, 8)) From 92dedaf026137bf1b72d8cb640c9b57512e491d4 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 17 Feb 2025 11:25:14 +0800 Subject: [PATCH 15/17] Update models/project/column_test.go --- models/project/column_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/project/column_test.go b/models/project/column_test.go index f547e4582061a..c64418eef9956 100644 --- a/models/project/column_test.go +++ b/models/project/column_test.go @@ -32,7 +32,7 @@ func TestGetDefaultColumn(t *testing.T) { column, err = projectWithMultipleDefaults.MustDefaultColumn(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(6), column.ProjectID) - assert.Equal(t, int64(8), column.ID) // sorted by sorting, id 8 is the first default column + assert.Equal(t, int64(8), column.ID) // sorted by "sorting, id", id 8 is the first default column // set 8 as default column assert.NoError(t, SetDefaultColumn(db.DefaultContext, column.ProjectID, 8)) From 74c9b7b13ab229d29ff266162c1336a0370b76d6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 17 Feb 2025 11:29:48 +0800 Subject: [PATCH 16/17] Update models/project/column.go --- models/project/column.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/project/column.go b/models/project/column.go index b9816980f1148..5f581b58804cb 100644 --- a/models/project/column.go +++ b/models/project/column.go @@ -250,7 +250,7 @@ func (p *Project) getDefaultColumn(ctx context.Context) (*Column, error) { var column Column has, err := db.GetEngine(ctx). Where("project_id=? AND `default` = ?", p.ID, true). - OrderBy("sorting, id").Get(&column) + Desc("id").Get(&column) if err != nil { return nil, err } From 3670a2c633d8fe8a1d98652c44467eed2e036813 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 17 Feb 2025 12:03:13 +0800 Subject: [PATCH 17/17] Update models/project/column_test.go --- models/project/column_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/project/column_test.go b/models/project/column_test.go index c64418eef9956..66db23a3e429c 100644 --- a/models/project/column_test.go +++ b/models/project/column_test.go @@ -32,7 +32,7 @@ func TestGetDefaultColumn(t *testing.T) { column, err = projectWithMultipleDefaults.MustDefaultColumn(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(6), column.ProjectID) - assert.Equal(t, int64(8), column.ID) // sorted by "sorting, id", id 8 is the first default column + assert.Equal(t, int64(9), column.ID) // there are 2 default columns in the test data, use the latest one // set 8 as default column assert.NoError(t, SetDefaultColumn(db.DefaultContext, column.ProjectID, 8))