Skip to content
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

Fix project issues list and counting #33594

Merged
merged 21 commits into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from 13 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
40 changes: 21 additions & 19 deletions models/issues/issue_project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -61,11 +76,11 @@ func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, opts *Is
}

if b.Default {
issues, err := Issues(ctx, &IssuesOptions{
ProjectColumnID: db.NoConditionID,
ProjectID: b.ProjectID,
SortType: "project-column-sorting",
})
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
}
Expand All @@ -79,19 +94,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 {
Expand All @@ -112,7 +114,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
}
Expand Down
24 changes: 14 additions & 10 deletions models/issues/issue_search.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ type IssuesOptions struct { //nolint
// prioritize issues from this repo
PriorityRepoID int64
IsArchived optional.Option[bool]
Org *organization.Organization // issues permission scope
Team *organization.Team // issues permission scope
User *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.
Expand Down Expand Up @@ -273,8 +273,12 @@ 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.Owner != nil {
sess.And(repo_model.UserOwnedRepoCond(opts.Owner.ID))
}

if opts.Doer != nil && !opts.Doer.IsAdmin {
sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.Doer.ID, opts.Owner, opts.Team, opts.IsPull.Value()))
}
}

Expand Down Expand Up @@ -321,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
),
)
}
Expand Down
47 changes: 30 additions & 17 deletions models/project/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand All @@ -57,20 +59,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).
Expand Down Expand Up @@ -192,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
}
Expand Down Expand Up @@ -257,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).
Expand All @@ -270,6 +258,31 @@ func (p *Project) GetDefaultColumn(ctx context.Context) (*Column, error) {
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Desc(id) in the called method seems incorrect.
Shouldn't it be Asc(sorting, id) instead?
(even if it might be potentially minimally breaking if someone misconfigured their projects)

Copy link
Member Author

@lunny lunny Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's already OrderBy("sorting, id")?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@wxiaoguang wxiaoguang Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it should change the existing logic in getDefaultColumn

The old logic means this: use the latest default one. It doesn't care about "sorting" or not. There should be only one "default" column, the "order by" is just a fallback.

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 {
column.Default = true
if _, err := db.GetEngine(ctx).ID(column.ID).Cols("`default`").Update(&column); err != nil {
return nil, err
}
return &column, nil
}

// create a default column if none is found
column = Column{
Expand Down
6 changes: 3 additions & 3 deletions models/project/column_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ 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)
assert.Equal(t, "Done", column.Title)

projectWithMultipleDefaults, err := GetProjectByID(db.DefaultContext, 6)
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)
Expand Down
43 changes: 0 additions & 43 deletions models/project/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util"
)

Expand All @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions models/project/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
4 changes: 2 additions & 2 deletions modules/indexer/issues/db/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m
UpdatedBeforeUnix: options.UpdatedBeforeUnix.Value(),
PriorityRepoID: 0,
IsArchived: options.IsArchived,
Org: nil,
Owner: nil,
Team: nil,
User: nil,
Doer: nil,
}

if len(options.MilestoneIDs) == 1 && options.MilestoneIDs[0] == 0 {
Expand Down
20 changes: 18 additions & 2 deletions routers/web/org/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -327,6 +332,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 {
Expand All @@ -340,14 +349,21 @@ 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{
opts := issues_model.IssuesOptions{
LabelIDs: labelIDs,
AssigneeID: optional.Some(assigneeID),
})
Owner: project.Owner,
Doer: ctx.Doer,
}

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)
Expand Down
11 changes: 10 additions & 1 deletion routers/web/repo/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -312,14 +317,18 @@ 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.LoadIssuesFromProject(ctx, project, &issues_model.IssuesOptions{
RepoIDs: []int64{ctx.Repo.Repository.ID},
LabelIDs: labelIDs,
AssigneeID: optional.Some(assigneeID),
})
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][]*repo_model.Attachment)
Expand Down
4 changes: 2 additions & 2 deletions routers/web/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Doer: ctx.Doer,
}
// --------------------------------------------------------------------------
// Build opts (IssuesOptions), which contains filter information.
Expand All @@ -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())
Expand Down
Loading
Loading