diff options
Diffstat (limited to 'models/issues')
28 files changed, 725 insertions, 770 deletions
diff --git a/models/issues/comment.go b/models/issues/comment.go index a248708820..4fdb0c1808 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "html/template" + "slices" "strconv" "unicode/utf8" @@ -19,8 +20,6 @@ import ( 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/gitrepo" - "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/references" @@ -198,12 +197,7 @@ func (t CommentType) HasMailReplySupport() bool { } func (t CommentType) CountedAsConversation() bool { - for _, ct := range ConversationCountedCommentType() { - if t == ct { - return true - } - } - return false + return slices.Contains(ConversationCountedCommentType(), t) } // ConversationCountedCommentType returns the comment types that are counted as a conversation @@ -616,7 +610,7 @@ func UpdateCommentAttachments(ctx context.Context, c *Comment, uuids []string) e if err != nil { return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) } - for i := 0; i < len(attachments); i++ { + for i := range attachments { attachments[i].IssueID = c.IssueID attachments[i].CommentID = c.ID if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { @@ -721,7 +715,8 @@ func (c *Comment) LoadReactions(ctx context.Context, repo *repo_model.Repository return nil } -func (c *Comment) loadReview(ctx context.Context) (err error) { +// LoadReview loads the associated review +func (c *Comment) LoadReview(ctx context.Context) (err error) { if c.ReviewID == 0 { return nil } @@ -738,11 +733,6 @@ func (c *Comment) loadReview(ctx context.Context) (err error) { return nil } -// LoadReview loads the associated review -func (c *Comment) LoadReview(ctx context.Context) error { - return c.loadReview(ctx) -} - // DiffSide returns "previous" if Comment.Line is a LOC of the previous changes and "proposed" if it is a LOC of the proposed changes. func (c *Comment) DiffSide() string { if c.Line < 0 { @@ -774,41 +764,6 @@ func (c *Comment) CodeCommentLink(ctx context.Context) string { return fmt.Sprintf("%s/files#%s", c.Issue.Link(), c.HashTag()) } -// LoadPushCommits Load push commits -func (c *Comment) LoadPushCommits(ctx context.Context) (err error) { - if c.Content == "" || c.Commits != nil || c.Type != CommentTypePullRequestPush { - return nil - } - - var data PushActionContent - - err = json.Unmarshal([]byte(c.Content), &data) - if err != nil { - return err - } - - c.IsForcePush = data.IsForcePush - - if c.IsForcePush { - if len(data.CommitIDs) != 2 { - return nil - } - c.OldCommit = data.CommitIDs[0] - c.NewCommit = data.CommitIDs[1] - } else { - gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, c.Issue.Repo) - if err != nil { - return err - } - defer closer.Close() - - c.Commits = git_model.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo) - c.CommitsNum = int64(len(c.Commits)) - } - - return err -} - // CreateComment creates comment with context func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, err error) { ctx, committer, err := db.TxContext(ctx) @@ -897,7 +852,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment } if comment.ReviewID != 0 { if comment.Review == nil { - if err := comment.loadReview(ctx); err != nil { + if err := comment.LoadReview(ctx); err != nil { return err } } diff --git a/models/issues/comment_code.go b/models/issues/comment_code.go index 67a77ceb13..55e67a1243 100644 --- a/models/issues/comment_code.go +++ b/models/issues/comment_code.go @@ -5,6 +5,7 @@ package issues import ( "context" + "strconv" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/renderhelper" @@ -86,8 +87,10 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu ids = append(ids, comment.ReviewID) } } - if err := e.In("id", ids).Find(&reviews); err != nil { - return nil, err + if len(ids) > 0 { + if err := e.In("id", ids).Find(&reviews); err != nil { + return nil, err + } } n := 0 @@ -112,7 +115,9 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu } var err error - rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo) + rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{ + FootnoteContextID: strconv.FormatInt(comment.ID, 10), + }) if comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content); err != nil { return nil, err } diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index c483ada75a..f6c485449f 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -57,10 +57,7 @@ func (comments CommentList) loadLabels(ctx context.Context) error { commentLabels := make(map[int64]*Label, len(labelIDs)) left := len(labelIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := db.GetEngine(ctx). In("id", labelIDs[:limit]). Rows(new(Label)) @@ -107,10 +104,7 @@ func (comments CommentList) loadMilestones(ctx context.Context) error { milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs)) left := len(milestoneIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) err := db.GetEngine(ctx). In("id", milestoneIDs[:limit]). Find(&milestoneMaps) @@ -146,10 +140,7 @@ func (comments CommentList) loadOldMilestones(ctx context.Context) error { milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs)) left := len(milestoneIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) err := db.GetEngine(ctx). In("id", milestoneIDs[:limit]). Find(&milestoneMaps) @@ -184,10 +175,7 @@ func (comments CommentList) loadAssignees(ctx context.Context) error { assignees := make(map[int64]*user_model.User, len(assigneeIDs)) left := len(assigneeIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := db.GetEngine(ctx). In("id", assigneeIDs[:limit]). Rows(new(user_model.User)) @@ -256,10 +244,7 @@ func (comments CommentList) LoadIssues(ctx context.Context) error { issues := make(map[int64]*Issue, len(issueIDs)) left := len(issueIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := db.GetEngine(ctx). In("id", issueIDs[:limit]). Rows(new(Issue)) @@ -313,10 +298,7 @@ func (comments CommentList) loadDependentIssues(ctx context.Context) error { issues := make(map[int64]*Issue, len(issueIDs)) left := len(issueIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := e. In("id", issueIDs[:limit]). Rows(new(Issue)) @@ -392,10 +374,7 @@ func (comments CommentList) LoadAttachments(ctx context.Context) (err error) { commentsIDs := comments.getAttachmentCommentIDs() left := len(commentsIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := db.GetEngine(ctx). In("comment_id", commentsIDs[:limit]). Rows(new(repo_model.Attachment)) diff --git a/models/issues/comment_test.go b/models/issues/comment_test.go index ae0bc3ce17..c08e3b970d 100644 --- a/models/issues/comment_test.go +++ b/models/issues/comment_test.go @@ -34,10 +34,10 @@ func TestCreateComment(t *testing.T) { assert.NoError(t, err) then := time.Now().Unix() - assert.EqualValues(t, issues_model.CommentTypeComment, comment.Type) - assert.EqualValues(t, "Hello", comment.Content) - assert.EqualValues(t, issue.ID, comment.IssueID) - assert.EqualValues(t, doer.ID, comment.PosterID) + assert.Equal(t, issues_model.CommentTypeComment, comment.Type) + assert.Equal(t, "Hello", comment.Content) + assert.Equal(t, issue.ID, comment.IssueID) + assert.Equal(t, doer.ID, comment.PosterID) unittest.AssertInt64InRange(t, now, then, int64(comment.CreatedUnix)) unittest.AssertExistsAndLoadBean(t, comment) // assert actually added to DB @@ -58,9 +58,9 @@ func Test_UpdateCommentAttachment(t *testing.T) { assert.NoError(t, err) attachment2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: attachment.ID}) - assert.EqualValues(t, attachment.Name, attachment2.Name) - assert.EqualValues(t, comment.ID, attachment2.CommentID) - assert.EqualValues(t, comment.IssueID, attachment2.IssueID) + assert.Equal(t, attachment.Name, attachment2.Name) + assert.Equal(t, comment.ID, attachment2.CommentID) + assert.Equal(t, comment.IssueID, attachment2.IssueID) } func TestFetchCodeComments(t *testing.T) { @@ -111,7 +111,7 @@ func TestMigrate_InsertIssueComments(t *testing.T) { assert.NoError(t, err) issueModified := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) - assert.EqualValues(t, issue.NumComments+1, issueModified.NumComments) + assert.Equal(t, issue.NumComments+1, issueModified.NumComments) unittest.CheckConsistencyFor(t, &issues_model.Issue{}) } @@ -122,5 +122,5 @@ func Test_UpdateIssueNumComments(t *testing.T) { assert.NoError(t, issues_model.UpdateIssueNumComments(db.DefaultContext, issue2.ID)) issue2 = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) - assert.EqualValues(t, 1, issue2.NumComments) + assert.Equal(t, 1, issue2.NumComments) } diff --git a/models/issues/issue.go b/models/issues/issue.go index 564a9fb835..a86d50ca9d 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -10,6 +10,7 @@ import ( "html/template" "regexp" "slices" + "strconv" "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" @@ -17,6 +18,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -96,7 +98,7 @@ type Issue struct { // TODO: RemoveIssueRef: see "repo/issue/branch_selector_field.tmpl" Ref string - PinOrder int `xorm:"DEFAULT 0"` + PinOrder int `xorm:"-"` // 0 means not loaded, -1 means loaded but not pinned DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"` @@ -290,6 +292,23 @@ func (issue *Issue) LoadMilestone(ctx context.Context) (err error) { return nil } +func (issue *Issue) LoadPinOrder(ctx context.Context) error { + if issue.PinOrder != 0 { + return nil + } + issuePin, err := GetIssuePin(ctx, issue) + if err != nil && !db.IsErrNotExist(err) { + return err + } + + if issuePin != nil { + issue.PinOrder = issuePin.PinOrder + } else { + issue.PinOrder = -1 + } + return nil +} + // LoadAttributes loads the attribute of this issue. func (issue *Issue) LoadAttributes(ctx context.Context) (err error) { if err = issue.LoadRepo(ctx); err != nil { @@ -329,6 +348,10 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) { return err } + if err = issue.LoadPinOrder(ctx); err != nil { + return err + } + if err = issue.Comments.LoadAttributes(ctx); err != nil { return err } @@ -341,6 +364,14 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) { return issue.loadReactions(ctx) } +// IsPinned returns if a Issue is pinned +func (issue *Issue) IsPinned() bool { + if issue.PinOrder == 0 { + setting.PanicInDevOrTesting("issue's pinorder has not been loaded") + } + return issue.PinOrder > 0 +} + func (issue *Issue) ResetAttributesLoaded() { issue.isLabelsLoaded = false issue.isMilestoneLoaded = false @@ -501,6 +532,45 @@ func GetIssueByIndex(ctx context.Context, repoID, index int64) (*Issue, error) { return issue, nil } +func isPullToCond(isPull optional.Option[bool]) builder.Cond { + if isPull.Has() { + return builder.Eq{"is_pull": isPull.Value()} + } + return builder.NewCond() +} + +func FindLatestUpdatedIssues(ctx context.Context, repoID int64, isPull optional.Option[bool], pageSize int) (IssueList, error) { + issues := make([]*Issue, 0, pageSize) + err := db.GetEngine(ctx).Where("repo_id = ?", repoID). + And(isPullToCond(isPull)). + OrderBy("updated_unix DESC"). + Limit(pageSize). + Find(&issues) + return issues, err +} + +func FindIssuesSuggestionByKeyword(ctx context.Context, repoID int64, keyword string, isPull optional.Option[bool], excludedID int64, pageSize int) (IssueList, error) { + cond := builder.NewCond() + if excludedID > 0 { + cond = cond.And(builder.Neq{"`id`": excludedID}) + } + + // It seems that GitHub searches both title and content (maybe sorting by the search engine's ranking system?) + // The first PR (https://github.com/go-gitea/gitea/pull/32327) uses "search indexer" to search "name(title) + content" + // But it seems that searching "content" (especially LIKE by DB engine) generates worse (unusable) results. + // So now (https://github.com/go-gitea/gitea/pull/33538) it only searches "name(title)", leave the improvements to the future. + cond = cond.And(db.BuildCaseInsensitiveLike("`name`", keyword)) + + issues := make([]*Issue, 0, pageSize) + err := db.GetEngine(ctx).Where("repo_id = ?", repoID). + And(isPullToCond(isPull)). + And(cond). + OrderBy("updated_unix DESC, `index` DESC"). + Limit(pageSize). + Find(&issues) + return issues, err +} + // GetIssueWithAttrsByIndex returns issue by index in a repository. func GetIssueWithAttrsByIndex(ctx context.Context, repoID, index int64) (*Issue, error) { issue, err := GetIssueByIndex(ctx, repoID, index) @@ -526,6 +596,9 @@ func GetIssueByID(ctx context.Context, id int64) (*Issue, error) { // If keepOrder is true, the order of the returned issues will be the same as the given IDs. func GetIssuesByIDs(ctx context.Context, issueIDs []int64, keepOrder ...bool) (IssueList, error) { issues := make([]*Issue, 0, len(issueIDs)) + if len(issueIDs) == 0 { + return issues, nil + } if err := db.GetEngine(ctx).In("id", issueIDs).Find(&issues); err != nil { return nil, err @@ -680,190 +753,6 @@ func (issue *Issue) HasOriginalAuthor() bool { return issue.OriginalAuthor != "" && issue.OriginalAuthorID != 0 } -var ErrIssueMaxPinReached = util.NewInvalidArgumentErrorf("the max number of pinned issues has been readched") - -// IsPinned returns if a Issue is pinned -func (issue *Issue) IsPinned() bool { - return issue.PinOrder != 0 -} - -// Pin pins a Issue -func (issue *Issue) Pin(ctx context.Context, user *user_model.User) error { - // If the Issue is already pinned, we don't need to pin it twice - if issue.IsPinned() { - return nil - } - - var maxPin int - _, err := db.GetEngine(ctx).SQL("SELECT MAX(pin_order) FROM issue WHERE repo_id = ? AND is_pull = ?", issue.RepoID, issue.IsPull).Get(&maxPin) - if err != nil { - return err - } - - // Check if the maximum allowed Pins reached - if maxPin >= setting.Repository.Issue.MaxPinned { - return ErrIssueMaxPinReached - } - - _, err = db.GetEngine(ctx).Table("issue"). - Where("id = ?", issue.ID). - Update(map[string]any{ - "pin_order": maxPin + 1, - }) - if err != nil { - return err - } - - // Add the pin event to the history - opts := &CreateCommentOptions{ - Type: CommentTypePin, - Doer: user, - Repo: issue.Repo, - Issue: issue, - } - if _, err = CreateComment(ctx, opts); err != nil { - return err - } - - return nil -} - -// UnpinIssue unpins a Issue -func (issue *Issue) Unpin(ctx context.Context, user *user_model.User) error { - // If the Issue is not pinned, we don't need to unpin it - if !issue.IsPinned() { - return nil - } - - // This sets the Pin for all Issues that come after the unpined Issue to the correct value - _, err := db.GetEngine(ctx).Exec("UPDATE issue SET pin_order = pin_order - 1 WHERE repo_id = ? AND is_pull = ? AND pin_order > ?", issue.RepoID, issue.IsPull, issue.PinOrder) - if err != nil { - return err - } - - _, err = db.GetEngine(ctx).Table("issue"). - Where("id = ?", issue.ID). - Update(map[string]any{ - "pin_order": 0, - }) - if err != nil { - return err - } - - // Add the unpin event to the history - opts := &CreateCommentOptions{ - Type: CommentTypeUnpin, - Doer: user, - Repo: issue.Repo, - Issue: issue, - } - if _, err = CreateComment(ctx, opts); err != nil { - return err - } - - return nil -} - -// PinOrUnpin pins or unpins a Issue -func (issue *Issue) PinOrUnpin(ctx context.Context, user *user_model.User) error { - if !issue.IsPinned() { - return issue.Pin(ctx, user) - } - - return issue.Unpin(ctx, user) -} - -// MovePin moves a Pinned Issue to a new Position -func (issue *Issue) MovePin(ctx context.Context, newPosition int) error { - // If the Issue is not pinned, we can't move them - if !issue.IsPinned() { - return nil - } - - if newPosition < 1 { - return fmt.Errorf("The Position can't be lower than 1") - } - - dbctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - var maxPin int - _, err = db.GetEngine(dbctx).SQL("SELECT MAX(pin_order) FROM issue WHERE repo_id = ? AND is_pull = ?", issue.RepoID, issue.IsPull).Get(&maxPin) - if err != nil { - return err - } - - // If the new Position bigger than the current Maximum, set it to the Maximum - if newPosition > maxPin+1 { - newPosition = maxPin + 1 - } - - // Lower the Position of all Pinned Issue that came after the current Position - _, err = db.GetEngine(dbctx).Exec("UPDATE issue SET pin_order = pin_order - 1 WHERE repo_id = ? AND is_pull = ? AND pin_order > ?", issue.RepoID, issue.IsPull, issue.PinOrder) - if err != nil { - return err - } - - // Higher the Position of all Pinned Issues that comes after the new Position - _, err = db.GetEngine(dbctx).Exec("UPDATE issue SET pin_order = pin_order + 1 WHERE repo_id = ? AND is_pull = ? AND pin_order >= ?", issue.RepoID, issue.IsPull, newPosition) - if err != nil { - return err - } - - _, err = db.GetEngine(dbctx).Table("issue"). - Where("id = ?", issue.ID). - Update(map[string]any{ - "pin_order": newPosition, - }) - if err != nil { - return err - } - - return committer.Commit() -} - -// GetPinnedIssues returns the pinned Issues for the given Repo and type -func GetPinnedIssues(ctx context.Context, repoID int64, isPull bool) (IssueList, error) { - issues := make(IssueList, 0) - - err := db.GetEngine(ctx). - Table("issue"). - Where("repo_id = ?", repoID). - And("is_pull = ?", isPull). - And("pin_order > 0"). - OrderBy("pin_order"). - Find(&issues) - if err != nil { - return nil, err - } - - err = issues.LoadAttributes(ctx) - if err != nil { - return nil, err - } - - return issues, nil -} - -// IsNewPinAllowed returns if a new Issue or Pull request can be pinned -func IsNewPinAllowed(ctx context.Context, repoID int64, isPull bool) (bool, error) { - var maxPin int - _, err := db.GetEngine(ctx).SQL("SELECT COUNT(pin_order) FROM issue WHERE repo_id = ? AND is_pull = ? AND pin_order > 0", repoID, isPull).Get(&maxPin) - if err != nil { - return false, err - } - - return maxPin < setting.Repository.Issue.MaxPinned, nil -} - -// IsErrIssueMaxPinReached returns if the error is, that the User can't pin more Issues -func IsErrIssueMaxPinReached(err error) bool { - return err == ErrIssueMaxPinReached -} - // InsertIssues insert issues to database func InsertIssues(ctx context.Context, issues ...*Issue) error { ctx, committer, err := db.TxContext(ctx) @@ -927,7 +816,7 @@ func ChangeIssueTimeEstimate(ctx context.Context, issue *Issue, doer *user_model Doer: doer, Repo: issue.Repo, Issue: issue, - Content: fmt.Sprintf("%d", timeEstimate), + Content: strconv.FormatInt(timeEstimate, 10), } if _, err := CreateComment(ctx, opts); err != nil { return fmt.Errorf("createComment: %w", err) diff --git a/models/issues/issue_label.go b/models/issues/issue_label.go index 10fc821454..f082079e07 100644 --- a/models/issues/issue_label.go +++ b/models/issues/issue_label.go @@ -206,6 +206,7 @@ func DeleteIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *use } issue.Labels = nil + issue.isLabelsLoaded = false return issue.LoadLabels(ctx) } diff --git a/models/issues/issue_list.go b/models/issues/issue_list.go index 02fd330f0a..26b93189b8 100644 --- a/models/issues/issue_list.go +++ b/models/issues/issue_list.go @@ -42,10 +42,7 @@ func (issues IssueList) LoadRepositories(ctx context.Context) (repo_model.Reposi repoMaps := make(map[int64]*repo_model.Repository, len(repoIDs)) left := len(repoIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) err := db.GetEngine(ctx). In("id", repoIDs[:limit]). Find(&repoMaps) @@ -116,10 +113,7 @@ func (issues IssueList) LoadLabels(ctx context.Context) error { issueIDs := issues.getIssueIDs() left := len(issueIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := db.GetEngine(ctx).Table("label"). Join("LEFT", "issue_label", "issue_label.label_id = label.id"). In("issue_label.issue_id", issueIDs[:limit]). @@ -171,10 +165,7 @@ func (issues IssueList) LoadMilestones(ctx context.Context) error { milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs)) left := len(milestoneIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) err := db.GetEngine(ctx). In("id", milestoneIDs[:limit]). Find(&milestoneMaps) @@ -203,10 +194,7 @@ func (issues IssueList) LoadProjects(ctx context.Context) error { } for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) projects := make([]*projectWithIssueID, 0, limit) err := db.GetEngine(ctx). @@ -245,10 +233,7 @@ func (issues IssueList) LoadAssignees(ctx context.Context) error { issueIDs := issues.getIssueIDs() left := len(issueIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := db.GetEngine(ctx).Table("issue_assignees"). Join("INNER", "`user`", "`user`.id = `issue_assignees`.assignee_id"). In("`issue_assignees`.issue_id", issueIDs[:limit]).OrderBy(user_model.GetOrderByName()). @@ -306,10 +291,7 @@ func (issues IssueList) LoadPullRequests(ctx context.Context) error { pullRequestMaps := make(map[int64]*PullRequest, len(issuesIDs)) left := len(issuesIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := db.GetEngine(ctx). In("issue_id", issuesIDs[:limit]). Rows(new(PullRequest)) @@ -354,10 +336,7 @@ func (issues IssueList) LoadAttachments(ctx context.Context) (err error) { issuesIDs := issues.getIssueIDs() left := len(issuesIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := db.GetEngine(ctx). In("issue_id", issuesIDs[:limit]). Rows(new(repo_model.Attachment)) @@ -399,10 +378,7 @@ func (issues IssueList) loadComments(ctx context.Context, cond builder.Cond) (er issuesIDs := issues.getIssueIDs() left := len(issuesIDs) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) rows, err := db.GetEngine(ctx).Table("comment"). Join("INNER", "issue", "issue.id = comment.issue_id"). In("issue.id", issuesIDs[:limit]). @@ -466,10 +442,7 @@ func (issues IssueList) loadTotalTrackedTimes(ctx context.Context) (err error) { left := len(ids) for left > 0 { - limit := db.DefaultMaxInSize - if left < limit { - limit = left - } + limit := min(left, db.DefaultMaxInSize) // select issue_id, sum(time) from tracked_time where issue_id in (<issue ids in current page>) group by issue_id rows, err := db.GetEngine(ctx).Table("tracked_time"). @@ -506,6 +479,39 @@ func (issues IssueList) loadTotalTrackedTimes(ctx context.Context) (err error) { return nil } +func (issues IssueList) LoadPinOrder(ctx context.Context) error { + if len(issues) == 0 { + return nil + } + + issueIDs := container.FilterSlice(issues, func(issue *Issue) (int64, bool) { + return issue.ID, issue.PinOrder == 0 + }) + if len(issueIDs) == 0 { + return nil + } + issuePins, err := GetIssuePinsByIssueIDs(ctx, issueIDs) + if err != nil { + return err + } + + for _, issue := range issues { + if issue.PinOrder != 0 { + continue + } + for _, pin := range issuePins { + if pin.IssueID == issue.ID { + issue.PinOrder = pin.PinOrder + break + } + } + if issue.PinOrder == 0 { + issue.PinOrder = -1 + } + } + return nil +} + // loadAttributes loads all attributes, expect for attachments and comments func (issues IssueList) LoadAttributes(ctx context.Context) error { if _, err := issues.LoadRepositories(ctx); err != nil { diff --git a/models/issues/issue_list_test.go b/models/issues/issue_list_test.go index 9069e1012d..5b4d2ca5ab 100644 --- a/models/issues/issue_list_test.go +++ b/models/issues/issue_list_test.go @@ -27,7 +27,7 @@ func TestIssueList_LoadRepositories(t *testing.T) { assert.NoError(t, err) assert.Len(t, repos, 2) for _, issue := range issueList { - assert.EqualValues(t, issue.RepoID, issue.Repo.ID) + assert.Equal(t, issue.RepoID, issue.Repo.ID) } } @@ -41,28 +41,28 @@ func TestIssueList_LoadAttributes(t *testing.T) { assert.NoError(t, issueList.LoadAttributes(db.DefaultContext)) for _, issue := range issueList { - assert.EqualValues(t, issue.RepoID, issue.Repo.ID) + assert.Equal(t, issue.RepoID, issue.Repo.ID) for _, label := range issue.Labels { - assert.EqualValues(t, issue.RepoID, label.RepoID) + assert.Equal(t, issue.RepoID, label.RepoID) unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: label.ID}) } if issue.PosterID > 0 { - assert.EqualValues(t, issue.PosterID, issue.Poster.ID) + assert.Equal(t, issue.PosterID, issue.Poster.ID) } if issue.AssigneeID > 0 { - assert.EqualValues(t, issue.AssigneeID, issue.Assignee.ID) + assert.Equal(t, issue.AssigneeID, issue.Assignee.ID) } if issue.MilestoneID > 0 { - assert.EqualValues(t, issue.MilestoneID, issue.Milestone.ID) + assert.Equal(t, issue.MilestoneID, issue.Milestone.ID) } if issue.IsPull { - assert.EqualValues(t, issue.ID, issue.PullRequest.IssueID) + assert.Equal(t, issue.ID, issue.PullRequest.IssueID) } for _, attachment := range issue.Attachments { - assert.EqualValues(t, issue.ID, attachment.IssueID) + assert.Equal(t, issue.ID, attachment.IssueID) } for _, comment := range issue.Comments { - assert.EqualValues(t, issue.ID, comment.IssueID) + assert.Equal(t, issue.ID, comment.IssueID) } if issue.ID == int64(1) { assert.Equal(t, int64(400), issue.TotalTrackedTime) diff --git a/models/issues/issue_lock.go b/models/issues/issue_lock.go index b21629b529..fa0d128f74 100644 --- a/models/issues/issue_lock.go +++ b/models/issues/issue_lock.go @@ -12,8 +12,14 @@ import ( // IssueLockOptions defines options for locking and/or unlocking an issue/PR type IssueLockOptions struct { - Doer *user_model.User - Issue *Issue + Doer *user_model.User + Issue *Issue + + // Reason is the doer-provided comment message for the locked issue + // GitHub doesn't support changing the "reasons" by config file, so GitHub has pre-defined "reason" enum values. + // Gitea is not like GitHub, it allows site admin to define customized "reasons" in the config file. + // So the API caller might not know what kind of "reasons" are valid, and the customized reasons are not translatable. + // To make things clear and simple: doer have the chance to use any reason they like, we do not do validation. Reason string } diff --git a/models/issues/issue_pin.go b/models/issues/issue_pin.go new file mode 100644 index 0000000000..ae6195b05d --- /dev/null +++ b/models/issues/issue_pin.go @@ -0,0 +1,246 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issues + +import ( + "context" + "errors" + "sort" + + "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" +) + +type IssuePin struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"UNIQUE(s) NOT NULL"` + IssueID int64 `xorm:"UNIQUE(s) NOT NULL"` + IsPull bool `xorm:"NOT NULL"` + PinOrder int `xorm:"DEFAULT 0"` +} + +var ErrIssueMaxPinReached = util.NewInvalidArgumentErrorf("the max number of pinned issues has been readched") + +// IsErrIssueMaxPinReached returns if the error is, that the User can't pin more Issues +func IsErrIssueMaxPinReached(err error) bool { + return err == ErrIssueMaxPinReached +} + +func init() { + db.RegisterModel(new(IssuePin)) +} + +func GetIssuePin(ctx context.Context, issue *Issue) (*IssuePin, error) { + pin := new(IssuePin) + has, err := db.GetEngine(ctx). + Where("repo_id = ?", issue.RepoID). + And("issue_id = ?", issue.ID).Get(pin) + if err != nil { + return nil, err + } else if !has { + return nil, db.ErrNotExist{ + Resource: "IssuePin", + ID: issue.ID, + } + } + return pin, nil +} + +func GetIssuePinsByIssueIDs(ctx context.Context, issueIDs []int64) ([]IssuePin, error) { + var pins []IssuePin + if err := db.GetEngine(ctx).In("issue_id", issueIDs).Find(&pins); err != nil { + return nil, err + } + return pins, nil +} + +// Pin pins a Issue +func PinIssue(ctx context.Context, issue *Issue, user *user_model.User) error { + return db.WithTx(ctx, func(ctx context.Context) error { + pinnedIssuesNum, err := getPinnedIssuesNum(ctx, issue.RepoID, issue.IsPull) + if err != nil { + return err + } + + // Check if the maximum allowed Pins reached + if pinnedIssuesNum >= setting.Repository.Issue.MaxPinned { + return ErrIssueMaxPinReached + } + + pinnedIssuesMaxPinOrder, err := getPinnedIssuesMaxPinOrder(ctx, issue.RepoID, issue.IsPull) + if err != nil { + return err + } + + if _, err = db.GetEngine(ctx).Insert(&IssuePin{ + RepoID: issue.RepoID, + IssueID: issue.ID, + IsPull: issue.IsPull, + PinOrder: pinnedIssuesMaxPinOrder + 1, + }); err != nil { + return err + } + + // Add the pin event to the history + _, err = CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypePin, + Doer: user, + Repo: issue.Repo, + Issue: issue, + }) + return err + }) +} + +// UnpinIssue unpins a Issue +func UnpinIssue(ctx context.Context, issue *Issue, user *user_model.User) error { + return db.WithTx(ctx, func(ctx context.Context) error { + // This sets the Pin for all Issues that come after the unpined Issue to the correct value + cnt, err := db.GetEngine(ctx).Where("issue_id=?", issue.ID).Delete(new(IssuePin)) + if err != nil { + return err + } + if cnt == 0 { + return nil + } + + // Add the unpin event to the history + _, err = CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypeUnpin, + Doer: user, + Repo: issue.Repo, + Issue: issue, + }) + return err + }) +} + +func getPinnedIssuesNum(ctx context.Context, repoID int64, isPull bool) (int, error) { + var pinnedIssuesNum int + _, err := db.GetEngine(ctx).SQL("SELECT count(pin_order) FROM issue_pin WHERE repo_id = ? AND is_pull = ?", repoID, isPull).Get(&pinnedIssuesNum) + return pinnedIssuesNum, err +} + +func getPinnedIssuesMaxPinOrder(ctx context.Context, repoID int64, isPull bool) (int, error) { + var maxPinnedIssuesMaxPinOrder int + _, err := db.GetEngine(ctx).SQL("SELECT max(pin_order) FROM issue_pin WHERE repo_id = ? AND is_pull = ?", repoID, isPull).Get(&maxPinnedIssuesMaxPinOrder) + return maxPinnedIssuesMaxPinOrder, err +} + +// MovePin moves a Pinned Issue to a new Position +func MovePin(ctx context.Context, issue *Issue, newPosition int) error { + if newPosition < 1 { + return errors.New("The Position can't be lower than 1") + } + + issuePin, err := GetIssuePin(ctx, issue) + if err != nil { + return err + } + if issuePin.PinOrder == newPosition { + return nil + } + + return db.WithTx(ctx, func(ctx context.Context) error { + if issuePin.PinOrder > newPosition { // move the issue to a lower position + _, err = db.GetEngine(ctx).Exec("UPDATE issue_pin SET pin_order = pin_order + 1 WHERE repo_id = ? AND is_pull = ? AND pin_order >= ? AND pin_order < ?", issue.RepoID, issue.IsPull, newPosition, issuePin.PinOrder) + } else { // move the issue to a higher position + // Lower the Position of all Pinned Issue that came after the current Position + _, err = db.GetEngine(ctx).Exec("UPDATE issue_pin SET pin_order = pin_order - 1 WHERE repo_id = ? AND is_pull = ? AND pin_order > ? AND pin_order <= ?", issue.RepoID, issue.IsPull, issuePin.PinOrder, newPosition) + } + if err != nil { + return err + } + + _, err = db.GetEngine(ctx). + Table("issue_pin"). + Where("id = ?", issuePin.ID). + Update(map[string]any{ + "pin_order": newPosition, + }) + return err + }) +} + +func GetPinnedIssueIDs(ctx context.Context, repoID int64, isPull bool) ([]int64, error) { + var issuePins []IssuePin + if err := db.GetEngine(ctx). + Table("issue_pin"). + Where("repo_id = ?", repoID). + And("is_pull = ?", isPull). + Find(&issuePins); err != nil { + return nil, err + } + + sort.Slice(issuePins, func(i, j int) bool { + return issuePins[i].PinOrder < issuePins[j].PinOrder + }) + + var ids []int64 + for _, pin := range issuePins { + ids = append(ids, pin.IssueID) + } + return ids, nil +} + +func GetIssuePinsByRepoID(ctx context.Context, repoID int64, isPull bool) ([]*IssuePin, error) { + var pins []*IssuePin + if err := db.GetEngine(ctx).Where("repo_id = ? AND is_pull = ?", repoID, isPull).Find(&pins); err != nil { + return nil, err + } + return pins, nil +} + +// GetPinnedIssues returns the pinned Issues for the given Repo and type +func GetPinnedIssues(ctx context.Context, repoID int64, isPull bool) (IssueList, error) { + issuePins, err := GetIssuePinsByRepoID(ctx, repoID, isPull) + if err != nil { + return nil, err + } + if len(issuePins) == 0 { + return IssueList{}, nil + } + ids := make([]int64, 0, len(issuePins)) + for _, pin := range issuePins { + ids = append(ids, pin.IssueID) + } + + issues := make(IssueList, 0, len(ids)) + if err := db.GetEngine(ctx).In("id", ids).Find(&issues); err != nil { + return nil, err + } + for _, issue := range issues { + for _, pin := range issuePins { + if pin.IssueID == issue.ID { + issue.PinOrder = pin.PinOrder + break + } + } + if (!setting.IsProd || setting.IsInTesting) && issue.PinOrder == 0 { + panic("It should not happen that a pinned Issue has no PinOrder") + } + } + sort.Slice(issues, func(i, j int) bool { + return issues[i].PinOrder < issues[j].PinOrder + }) + + if err = issues.LoadAttributes(ctx); err != nil { + return nil, err + } + + return issues, nil +} + +// IsNewPinAllowed returns if a new Issue or Pull request can be pinned +func IsNewPinAllowed(ctx context.Context, repoID int64, isPull bool) (bool, error) { + var maxPin int + _, err := db.GetEngine(ctx).SQL("SELECT COUNT(pin_order) FROM issue_pin WHERE repo_id = ? AND is_pull = ?", repoID, isPull).Get(&maxPin) + if err != nil { + return false, err + } + + return maxPin < setting.Repository.Issue.MaxPinned, nil +} diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index c4515fd898..0185244783 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -38,13 +38,30 @@ func (issue *Issue) projectID(ctx context.Context) int64 { } // ProjectColumnID return project column id if issue was assigned to one -func (issue *Issue) ProjectColumnID(ctx context.Context) int64 { +func (issue *Issue) ProjectColumnID(ctx context.Context) (int64, error) { var ip project_model.ProjectIssue has, err := db.GetEngine(ctx).Where("issue_id=?", issue.ID).Get(&ip) - if err != nil || !has { - return 0 + if err != nil { + return 0, err + } else if !has { + return 0, nil + } + 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 ip.ProjectColumnID + return result, nil } // LoadIssuesFromColumn load issues assigned to this column @@ -59,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 } @@ -77,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 { @@ -110,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 } diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index f1cd125d49..79bd6a19b0 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -21,14 +21,16 @@ import ( "xorm.io/xorm" ) +const ScopeSortPrefix = "scope-" + // IssuesOptions represents options of an issue. -type IssuesOptions struct { //nolint +type IssuesOptions struct { //nolint:revive // export stutter Paginator *db.ListOptions RepoIDs []int64 // overwrites RepoCond if the length is not 0 AllPublic bool // include also all public repositories RepoCond builder.Cond - AssigneeID optional.Option[int64] - PosterID optional.Option[int64] + AssigneeID string // "(none)" or "(any)" or a user ID + PosterID string // "(none)" or "(any)" or a user ID MentionedID int64 ReviewRequestedID int64 ReviewedID int64 @@ -49,9 +51,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. @@ -70,11 +72,24 @@ func (o *IssuesOptions) Copy(edit ...func(options *IssuesOptions)) *IssuesOption // applySorts sort an issues-related session based on the provided // sortType string func applySorts(sess *xorm.Session, sortType string, priorityRepoID int64) { + // Since this sortType is dynamically created, it has to be treated specially. + if after, ok := strings.CutPrefix(sortType, ScopeSortPrefix); ok { + scope := after + sess.Join("LEFT", "issue_label", "issue.id = issue_label.issue_id") + // "exclusive_order=0" means "no order is set", so exclude it from the JOIN criteria and then "LEFT JOIN" result is also null + sess.Join("LEFT", "label", "label.id = issue_label.label_id AND label.exclusive_order <> 0 AND label.name LIKE ?", scope+"/%") + // Use COALESCE to make sure we sort NULL last regardless of backend DB (2147483647 == max int) + sess.OrderBy("COALESCE(label.exclusive_order, 2147483647) ASC").Desc("issue.id") + return + } + switch sortType { case "oldest": sess.Asc("issue.created_unix").Asc("issue.id") case "recentupdate": sess.Desc("issue.updated_unix").Desc("issue.created_unix").Desc("issue.id") + case "recentclose": + sess.Desc("issue.closed_unix").Desc("issue.created_unix").Desc("issue.id") case "leastupdate": sess.Asc("issue.updated_unix").Asc("issue.created_unix").Asc("issue.id") case "mostcomment": @@ -273,8 +288,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())) } } @@ -321,20 +340,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 ), ) } @@ -352,26 +371,25 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organizati return cond } -func applyAssigneeCondition(sess *xorm.Session, assigneeID optional.Option[int64]) { +func applyAssigneeCondition(sess *xorm.Session, assigneeID string) { // old logic: 0 is also treated as "not filtering assignee", because the "assignee" was read as FormInt64 - if !assigneeID.Has() || assigneeID.Value() == 0 { - return - } - if assigneeID.Value() == db.NoConditionID { + if assigneeID == "(none)" { sess.Where("issue.id NOT IN (SELECT issue_id FROM issue_assignees)") - } else { + } else if assigneeID == "(any)" { + sess.Where("issue.id IN (SELECT issue_id FROM issue_assignees)") + } else if assigneeIDInt64, _ := strconv.ParseInt(assigneeID, 10, 64); assigneeIDInt64 > 0 { sess.Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id"). - And("issue_assignees.assignee_id = ?", assigneeID.Value()) + And("issue_assignees.assignee_id = ?", assigneeIDInt64) } } -func applyPosterCondition(sess *xorm.Session, posterID optional.Option[int64]) { - if !posterID.Has() { - return - } - // poster doesn't need to support db.NoConditionID(-1), so just use the value as-is - if posterID.Has() { - sess.And("issue.poster_id=?", posterID.Value()) +func applyPosterCondition(sess *xorm.Session, posterID string) { + // Actually every issue has a poster. + // The "(none)" is for internal usage only: when doer tries to search non-existing user as poster, use "(none)" to return empty result. + if posterID == "(none)" { + sess.And("issue.poster_id=0") + } else if posterIDInt64, _ := strconv.ParseInt(posterID, 10, 64); posterIDInt64 > 0 { + sess.And("issue.poster_id=?", posterIDInt64) } } diff --git a/models/issues/issue_stats.go b/models/issues/issue_stats.go index 9ef9347a16..adedaa3d3a 100644 --- a/models/issues/issue_stats.go +++ b/models/issues/issue_stats.go @@ -94,10 +94,7 @@ func GetIssueStats(ctx context.Context, opts *IssuesOptions) (*IssueStats, error // ids in a temporary table and join from them. accum := &IssueStats{} for i := 0; i < len(opts.IssueIDs); { - chunk := i + MaxQueryParameters - if chunk > len(opts.IssueIDs) { - chunk = len(opts.IssueIDs) - } + chunk := min(i+MaxQueryParameters, len(opts.IssueIDs)) stats, err := getIssueStatsChunk(ctx, opts, opts.IssueIDs[i:chunk]) if err != nil { return nil, err @@ -107,7 +104,7 @@ func GetIssueStats(ctx context.Context, opts *IssuesOptions) (*IssueStats, error accum.YourRepositoriesCount += stats.YourRepositoriesCount accum.AssignCount += stats.AssignCount accum.CreateCount += stats.CreateCount - accum.OpenCount += stats.MentionCount + accum.MentionCount += stats.MentionCount accum.ReviewRequestedCount += stats.ReviewRequestedCount accum.ReviewedCount += stats.ReviewedCount i = chunk diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index dbbb1e4179..1c5db55bbc 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -4,8 +4,8 @@ package issues_test import ( - "context" "fmt" + "slices" "sort" "sync" "testing" @@ -16,7 +16,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" @@ -143,8 +142,8 @@ func TestUpdateIssueCols(t *testing.T) { then := time.Now().Unix() updatedIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: issue.ID}) - assert.EqualValues(t, newTitle, updatedIssue.Title) - assert.EqualValues(t, prevContent, updatedIssue.Content) + assert.Equal(t, newTitle, updatedIssue.Title) + assert.Equal(t, prevContent, updatedIssue.Content) unittest.AssertInt64InRange(t, now, then, int64(updatedIssue.UpdatedUnix)) } @@ -156,7 +155,7 @@ func TestIssues(t *testing.T) { }{ { issues_model.IssuesOptions{ - AssigneeID: optional.Some(int64(1)), + AssigneeID: "1", SortType: "oldest", }, []int64{1, 6}, @@ -203,7 +202,7 @@ func TestIssues(t *testing.T) { assert.NoError(t, err) if assert.Len(t, issues, len(test.ExpectedIssueIDs)) { for i, issue := range issues { - assert.EqualValues(t, test.ExpectedIssueIDs[i], issue.ID) + assert.Equal(t, test.ExpectedIssueIDs[i], issue.ID) } } } @@ -236,10 +235,10 @@ func testInsertIssue(t *testing.T, title, content string, expectIndex int64) *is has, err := db.GetEngine(db.DefaultContext).ID(issue.ID).Get(&newIssue) assert.NoError(t, err) assert.True(t, has) - assert.EqualValues(t, issue.Title, newIssue.Title) - assert.EqualValues(t, issue.Content, newIssue.Content) + assert.Equal(t, issue.Title, newIssue.Title) + assert.Equal(t, issue.Content, newIssue.Content) if expectIndex > 0 { - assert.EqualValues(t, expectIndex, newIssue.Index) + assert.Equal(t, expectIndex, newIssue.Index) } }) return &newIssue @@ -272,8 +271,8 @@ func TestIssue_ResolveMentions(t *testing.T) { for i, user := range resolved { ids[i] = user.ID } - sort.Slice(ids, func(i, j int) bool { return ids[i] < ids[j] }) - assert.EqualValues(t, expected, ids) + slices.Sort(ids) + assert.Equal(t, expected, ids) } // Public repo, existing user @@ -294,7 +293,7 @@ func TestResourceIndex(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) var wg sync.WaitGroup - for i := 0; i < 100; i++ { + for i := range 100 { wg.Add(1) go func(i int) { testInsertIssue(t, fmt.Sprintf("issue %d", i+1), "my issue", 0) @@ -316,7 +315,7 @@ func TestCorrectIssueStats(t *testing.T) { issueAmount := issues_model.MaxQueryParameters + 10 var wg sync.WaitGroup - for i := 0; i < issueAmount; i++ { + for i := range issueAmount { wg.Add(1) go func(i int) { testInsertIssue(t, fmt.Sprintf("Issue %d", i+1), "Bugs are nasty", 0) @@ -326,7 +325,7 @@ func TestCorrectIssueStats(t *testing.T) { wg.Wait() // Now we will get all issueID's that match the "Bugs are nasty" query. - issues, err := issues_model.Issues(context.TODO(), &issues_model.IssuesOptions{ + issues, err := issues_model.Issues(t.Context(), &issues_model.IssuesOptions{ Paginator: &db.ListOptions{ PageSize: issueAmount, }, @@ -394,28 +393,28 @@ func TestIssueLoadAttributes(t *testing.T) { for _, issue := range issueList { assert.NoError(t, issue.LoadAttributes(db.DefaultContext)) - assert.EqualValues(t, issue.RepoID, issue.Repo.ID) + assert.Equal(t, issue.RepoID, issue.Repo.ID) for _, label := range issue.Labels { - assert.EqualValues(t, issue.RepoID, label.RepoID) + assert.Equal(t, issue.RepoID, label.RepoID) unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: label.ID}) } if issue.PosterID > 0 { - assert.EqualValues(t, issue.PosterID, issue.Poster.ID) + assert.Equal(t, issue.PosterID, issue.Poster.ID) } if issue.AssigneeID > 0 { - assert.EqualValues(t, issue.AssigneeID, issue.Assignee.ID) + assert.Equal(t, issue.AssigneeID, issue.Assignee.ID) } if issue.MilestoneID > 0 { - assert.EqualValues(t, issue.MilestoneID, issue.Milestone.ID) + assert.Equal(t, issue.MilestoneID, issue.Milestone.ID) } if issue.IsPull { - assert.EqualValues(t, issue.ID, issue.PullRequest.IssueID) + assert.Equal(t, issue.ID, issue.PullRequest.IssueID) } for _, attachment := range issue.Attachments { - assert.EqualValues(t, issue.ID, attachment.IssueID) + assert.Equal(t, issue.ID, attachment.IssueID) } for _, comment := range issue.Comments { - assert.EqualValues(t, issue.ID, comment.IssueID) + assert.Equal(t, issue.ID, comment.IssueID) } if issue.ID == int64(1) { assert.Equal(t, int64(400), issue.TotalTrackedTime) diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 7b3fe04aa5..9b99787e3b 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -5,16 +5,14 @@ package issues import ( "context" + "errors" "fmt" "strings" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/organization" - "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" - project_model "code.gitea.io/gitea/models/project" repo_model "code.gitea.io/gitea/models/repo" - system_model "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" @@ -306,7 +304,7 @@ func UpdateIssueAttachments(ctx context.Context, issueID int64, uuids []string) if err != nil { return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) } - for i := 0; i < len(attachments); i++ { + for i := range attachments { attachments[i].IssueID = issueID if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) @@ -386,10 +384,10 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue } if opts.Issue.Index <= 0 { - return fmt.Errorf("no issue index provided") + return errors.New("no issue index provided") } if opts.Issue.ID > 0 { - return fmt.Errorf("issue exist") + return errors.New("issue exist") } if _, err := e.Insert(opts.Issue); err != nil { @@ -611,7 +609,7 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u unittype = unit.TypePullRequests } for _, team := range teams { - if team.AccessMode >= perm.AccessModeAdmin { + if team.HasAdminAccess() { checked = append(checked, team.ID) resolved[issue.Repo.Owner.LowerName+"/"+team.LowerName] = true continue @@ -715,137 +713,13 @@ func UpdateReactionsMigrationsByType(ctx context.Context, gitServiceType api.Git return err } -// DeleteIssuesByRepoID deletes issues by repositories id -func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []string, err error) { - // MariaDB has a performance bug: https://jira.mariadb.org/browse/MDEV-16289 - // so here it uses "DELETE ... WHERE IN" with pre-queried IDs. - sess := db.GetEngine(ctx) - - for { - issueIDs := make([]int64, 0, db.DefaultMaxInSize) - - err := sess.Table(&Issue{}).Where("repo_id = ?", repoID).OrderBy("id").Limit(db.DefaultMaxInSize).Cols("id").Find(&issueIDs) - if err != nil { - return nil, err - } - - if len(issueIDs) == 0 { - break - } - - // Delete content histories - _, err = sess.In("issue_id", issueIDs).Delete(&ContentHistory{}) - if err != nil { - return nil, err - } - - // Delete comments and attachments - _, err = sess.In("issue_id", issueIDs).Delete(&Comment{}) - if err != nil { - return nil, err - } - - // Dependencies for issues in this repository - _, err = sess.In("issue_id", issueIDs).Delete(&IssueDependency{}) - if err != nil { - return nil, err - } - - // Delete dependencies for issues in other repositories - _, err = sess.In("dependency_id", issueIDs).Delete(&IssueDependency{}) - if err != nil { - return nil, err - } - - _, err = sess.In("issue_id", issueIDs).Delete(&IssueUser{}) - if err != nil { - return nil, err - } - - _, err = sess.In("issue_id", issueIDs).Delete(&Reaction{}) - if err != nil { - return nil, err - } - - _, err = sess.In("issue_id", issueIDs).Delete(&IssueWatch{}) - if err != nil { - return nil, err - } - - _, err = sess.In("issue_id", issueIDs).Delete(&Stopwatch{}) - if err != nil { - return nil, err - } - - _, err = sess.In("issue_id", issueIDs).Delete(&TrackedTime{}) - if err != nil { - return nil, err - } - - _, err = sess.In("issue_id", issueIDs).Delete(&project_model.ProjectIssue{}) - if err != nil { - return nil, err - } - - _, err = sess.In("dependent_issue_id", issueIDs).Delete(&Comment{}) - if err != nil { - return nil, err - } - - var attachments []*repo_model.Attachment - err = sess.In("issue_id", issueIDs).Find(&attachments) - if err != nil { - return nil, err - } - - for j := range attachments { - attachmentPaths = append(attachmentPaths, attachments[j].RelativePath()) - } - - _, err = sess.In("issue_id", issueIDs).Delete(&repo_model.Attachment{}) - if err != nil { - return nil, err - } - - _, err = sess.In("id", issueIDs).Delete(&Issue{}) - if err != nil { - return nil, err - } - } - - return attachmentPaths, err -} - -// DeleteOrphanedIssues delete issues without a repo -func DeleteOrphanedIssues(ctx context.Context) error { - var attachmentPaths []string - err := db.WithTx(ctx, func(ctx context.Context) error { - var ids []int64 - - if err := db.GetEngine(ctx).Table("issue").Distinct("issue.repo_id"). - Join("LEFT", "repository", "issue.repo_id=repository.id"). - Where(builder.IsNull{"repository.id"}).GroupBy("issue.repo_id"). - Find(&ids); err != nil { - return err - } - - for i := range ids { - paths, err := DeleteIssuesByRepoID(ctx, ids[i]) - if err != nil { - return err - } - attachmentPaths = append(attachmentPaths, paths...) - } - - return nil - }) - if err != nil { - return err - } - - // Remove issue attachment files. - for i := range attachmentPaths { - system_model.RemoveAllWithNotice(ctx, "Delete issue attachment", attachmentPaths[i]) +func GetOrphanedIssueRepoIDs(ctx context.Context) ([]int64, error) { + var repoIDs []int64 + if err := db.GetEngine(ctx).Table("issue").Distinct("issue.repo_id"). + Join("LEFT", "repository", "issue.repo_id=repository.id"). + Where(builder.IsNull{"repository.id"}). + Find(&repoIDs); err != nil { + return nil, err } - return nil + return repoIDs, nil } diff --git a/models/issues/label.go b/models/issues/label.go index b9d24bbe99..cfbe100926 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -87,6 +87,7 @@ type Label struct { OrgID int64 `xorm:"INDEX"` Name string Exclusive bool + ExclusiveOrder int `xorm:"DEFAULT 0"` // 0 means no exclusive order Description string Color string `xorm:"VARCHAR(7)"` NumIssues int @@ -236,7 +237,7 @@ func UpdateLabel(ctx context.Context, l *Label) error { } l.Color = color - return updateLabelCols(ctx, l, "name", "description", "color", "exclusive", "archived_unix") + return updateLabelCols(ctx, l, "name", "description", "color", "exclusive", "exclusive_order", "archived_unix") } // DeleteLabel delete a label @@ -299,6 +300,9 @@ func GetLabelByID(ctx context.Context, labelID int64) (*Label, error) { // GetLabelsByIDs returns a list of labels by IDs func GetLabelsByIDs(ctx context.Context, labelIDs []int64, cols ...string) ([]*Label, error) { labels := make([]*Label, 0, len(labelIDs)) + if len(labelIDs) == 0 { + return labels, nil + } return labels, db.GetEngine(ctx).Table("label"). In("id", labelIDs). Asc("name"). @@ -375,6 +379,9 @@ func BuildLabelNamesIssueIDsCondition(labelNames []string) *builder.Builder { // it silently ignores label IDs that do not belong to the repository. func GetLabelsInRepoByIDs(ctx context.Context, repoID int64, labelIDs []int64) ([]*Label, error) { labels := make([]*Label, 0, len(labelIDs)) + if len(labelIDs) == 0 { + return labels, nil + } return labels, db.GetEngine(ctx). Where("repo_id = ?", repoID). In("id", labelIDs). @@ -447,6 +454,9 @@ func GetLabelInOrgByID(ctx context.Context, orgID, labelID int64) (*Label, error // it silently ignores label IDs that do not belong to the organization. func GetLabelsInOrgByIDs(ctx context.Context, orgID int64, labelIDs []int64) ([]*Label, error) { labels := make([]*Label, 0, len(labelIDs)) + if len(labelIDs) == 0 { + return labels, nil + } return labels, db.GetEngine(ctx). Where("org_id = ?", orgID). In("id", labelIDs). diff --git a/models/issues/label_test.go b/models/issues/label_test.go index 185fa11bbc..226036d543 100644 --- a/models/issues/label_test.go +++ b/models/issues/label_test.go @@ -20,7 +20,7 @@ func TestLabel_CalOpenIssues(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1}) label.CalOpenIssues() - assert.EqualValues(t, 2, label.NumOpenIssues) + assert.Equal(t, 2, label.NumOpenIssues) } func TestLabel_LoadSelectedLabelsAfterClick(t *testing.T) { @@ -154,7 +154,7 @@ func TestGetLabelsByRepoID(t *testing.T) { assert.NoError(t, err) assert.Len(t, labels, len(expectedIssueIDs)) for i, label := range labels { - assert.EqualValues(t, expectedIssueIDs[i], label.ID) + assert.Equal(t, expectedIssueIDs[i], label.ID) } } testSuccess(1, "leastissues", []int64{2, 1}) @@ -221,7 +221,7 @@ func TestGetLabelsByOrgID(t *testing.T) { assert.NoError(t, err) assert.Len(t, labels, len(expectedIssueIDs)) for i, label := range labels { - assert.EqualValues(t, expectedIssueIDs[i], label.ID) + assert.Equal(t, expectedIssueIDs[i], label.ID) } } testSuccess(3, "leastissues", []int64{3, 4}) @@ -267,10 +267,10 @@ func TestUpdateLabel(t *testing.T) { label.Name = update.Name assert.NoError(t, issues_model.UpdateLabel(db.DefaultContext, update)) newLabel := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1}) - assert.EqualValues(t, label.ID, newLabel.ID) - assert.EqualValues(t, label.Color, newLabel.Color) - assert.EqualValues(t, label.Name, newLabel.Name) - assert.EqualValues(t, label.Description, newLabel.Description) + assert.Equal(t, label.ID, newLabel.ID) + assert.Equal(t, label.Color, newLabel.Color) + assert.Equal(t, label.Name, newLabel.Name) + assert.Equal(t, label.Description, newLabel.Description) assert.EqualValues(t, 0, newLabel.ArchivedUnix) unittest.CheckConsistencyFor(t, &issues_model.Label{}, &repo_model.Repository{}) } @@ -313,7 +313,7 @@ func TestNewIssueLabel(t *testing.T) { Content: "1", }) label = unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 2}) - assert.EqualValues(t, prevNumIssues+1, label.NumIssues) + assert.Equal(t, prevNumIssues+1, label.NumIssues) // re-add existing IssueLabel assert.NoError(t, issues_model.NewIssueLabel(db.DefaultContext, issue, label, doer)) @@ -366,11 +366,11 @@ func TestNewIssueLabels(t *testing.T) { }) unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: label1.ID}) label1 = unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1}) - assert.EqualValues(t, 3, label1.NumIssues) - assert.EqualValues(t, 1, label1.NumClosedIssues) + assert.Equal(t, 3, label1.NumIssues) + assert.Equal(t, 1, label1.NumClosedIssues) label2 = unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 2}) - assert.EqualValues(t, 1, label2.NumIssues) - assert.EqualValues(t, 1, label2.NumClosedIssues) + assert.Equal(t, 1, label2.NumIssues) + assert.Equal(t, 1, label2.NumClosedIssues) // corner case: test empty slice assert.NoError(t, issues_model.NewIssueLabels(db.DefaultContext, issue, []*issues_model.Label{}, doer)) @@ -408,8 +408,8 @@ func TestDeleteIssueLabel(t *testing.T) { LabelID: labelID, }, `content=''`) label = unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: labelID}) - assert.EqualValues(t, expectedNumIssues, label.NumIssues) - assert.EqualValues(t, expectedNumClosedIssues, label.NumClosedIssues) + assert.Equal(t, expectedNumIssues, label.NumIssues) + assert.Equal(t, expectedNumClosedIssues, label.NumClosedIssues) } testSuccess(1, 1, 2) testSuccess(2, 5, 2) diff --git a/models/issues/milestone_test.go b/models/issues/milestone_test.go index 28cd0c028b..f73355c27d 100644 --- a/models/issues/milestone_test.go +++ b/models/issues/milestone_test.go @@ -69,7 +69,7 @@ func TestGetMilestonesByRepoID(t *testing.T) { assert.Len(t, milestones, n) for _, milestone := range milestones { - assert.EqualValues(t, repoID, milestone.RepoID) + assert.Equal(t, repoID, milestone.RepoID) } } test(1, api.StateOpen) @@ -327,7 +327,7 @@ func TestUpdateMilestone(t *testing.T) { milestone.Content = "newMilestoneContent" assert.NoError(t, issues_model.UpdateMilestone(db.DefaultContext, milestone, milestone.IsClosed)) milestone = unittest.AssertExistsAndLoadBean(t, &issues_model.Milestone{ID: 1}) - assert.EqualValues(t, "newMilestoneName", milestone.Name) + assert.Equal(t, "newMilestoneName", milestone.Name) unittest.CheckConsistencyFor(t, &issues_model.Milestone{}) } @@ -364,7 +364,7 @@ func TestMigrate_InsertMilestones(t *testing.T) { assert.NoError(t, err) unittest.AssertExistsAndLoadBean(t, ms) repoModified := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) - assert.EqualValues(t, repo.NumMilestones+1, repoModified.NumMilestones) + assert.Equal(t, repo.NumMilestones+1, repoModified.NumMilestones) unittest.CheckConsistencyFor(t, &issues_model.Milestone{}) } diff --git a/models/issues/pull.go b/models/issues/pull.go index e3af00224d..0ff32e2473 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -6,10 +6,10 @@ package issues import ( "context" + "errors" "fmt" "io" "regexp" - "strconv" "strings" "code.gitea.io/gitea/models/db" @@ -103,27 +103,6 @@ const ( PullRequestStatusAncestor ) -func (status PullRequestStatus) String() string { - switch status { - case PullRequestStatusConflict: - return "CONFLICT" - case PullRequestStatusChecking: - return "CHECKING" - case PullRequestStatusMergeable: - return "MERGEABLE" - case PullRequestStatusManuallyMerged: - return "MANUALLY_MERGED" - case PullRequestStatusError: - return "ERROR" - case PullRequestStatusEmpty: - return "EMPTY" - case PullRequestStatusAncestor: - return "ANCESTOR" - default: - return strconv.Itoa(int(status)) - } -} - // PullRequestFlow the flow of pull request type PullRequestFlow int @@ -670,12 +649,6 @@ func GetAllUnmergedAgitPullRequestByPoster(ctx context.Context, uid int64) ([]*P return pulls, err } -// Update updates all fields of pull request. -func (pr *PullRequest) Update(ctx context.Context) error { - _, err := db.GetEngine(ctx).ID(pr.ID).AllCols().Update(pr) - return err -} - // UpdateCols updates specific fields of pull request. func (pr *PullRequest) UpdateCols(ctx context.Context, cols ...string) error { _, err := db.GetEngine(ctx).ID(pr.ID).Cols(cols...).Update(pr) @@ -732,7 +705,7 @@ func (pr *PullRequest) GetWorkInProgressPrefix(ctx context.Context) string { // UpdateCommitDivergence update Divergence of a pull request func (pr *PullRequest) UpdateCommitDivergence(ctx context.Context, ahead, behind int) error { if pr.ID == 0 { - return fmt.Errorf("pull ID is 0") + return errors.New("pull ID is 0") } pr.CommitsAhead = ahead pr.CommitsBehind = behind @@ -925,7 +898,7 @@ func ParseCodeOwnersLine(ctx context.Context, tokens []string) (*CodeOwnerRule, if strings.Contains(user, "/") { s := strings.Split(user, "/") if len(s) != 2 { - warnings = append(warnings, fmt.Sprintf("incorrect codeowner group: %s", user)) + warnings = append(warnings, "incorrect codeowner group: "+user) continue } orgName := s[0] @@ -933,12 +906,12 @@ func ParseCodeOwnersLine(ctx context.Context, tokens []string) (*CodeOwnerRule, org, err := org_model.GetOrgByName(ctx, orgName) if err != nil { - warnings = append(warnings, fmt.Sprintf("incorrect codeowner organization: %s", user)) + warnings = append(warnings, "incorrect codeowner organization: "+user) continue } teams, err := org.LoadTeams(ctx) if err != nil { - warnings = append(warnings, fmt.Sprintf("incorrect codeowner team: %s", user)) + warnings = append(warnings, "incorrect codeowner team: "+user) continue } @@ -950,7 +923,7 @@ func ParseCodeOwnersLine(ctx context.Context, tokens []string) (*CodeOwnerRule, } else { u, err := user_model.GetUserByName(ctx, user) if err != nil { - warnings = append(warnings, fmt.Sprintf("incorrect codeowner user: %s", user)) + warnings = append(warnings, "incorrect codeowner user: "+user) continue } rule.Users = append(rule.Users, u) diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go index 1ddb94e566..84f9f6166d 100644 --- a/models/issues/pull_list.go +++ b/models/issues/pull_list.go @@ -28,11 +28,16 @@ type PullRequestsOptions struct { Labels []int64 MilestoneID int64 PosterID int64 + BaseBranch string } func listPullRequestStatement(ctx context.Context, baseRepoID int64, opts *PullRequestsOptions) *xorm.Session { sess := db.GetEngine(ctx).Where("pull_request.base_repo_id=?", baseRepoID) + if opts.BaseBranch != "" { + sess.And("pull_request.base_branch=?", opts.BaseBranch) + } + sess.Join("INNER", "issue", "pull_request.issue_id = issue.id") switch opts.State { case "closed", "open": @@ -56,7 +61,7 @@ func listPullRequestStatement(ctx context.Context, baseRepoID int64, opts *PullR } // GetUnmergedPullRequestsByHeadInfo returns all pull requests that are open and has not been merged -func GetUnmergedPullRequestsByHeadInfo(ctx context.Context, repoID int64, branch string) ([]*PullRequest, error) { +func GetUnmergedPullRequestsByHeadInfo(ctx context.Context, repoID int64, branch string) (PullRequestList, error) { prs := make([]*PullRequest, 0, 2) sess := db.GetEngine(ctx). Join("INNER", "issue", "issue.id = pull_request.issue_id"). @@ -111,7 +116,7 @@ func HasUnmergedPullRequestsByHeadInfo(ctx context.Context, repoID int64, branch // GetUnmergedPullRequestsByBaseInfo returns all pull requests that are open and has not been merged // by given base information (repo and branch). -func GetUnmergedPullRequestsByBaseInfo(ctx context.Context, repoID int64, branch string) ([]*PullRequest, error) { +func GetUnmergedPullRequestsByBaseInfo(ctx context.Context, repoID int64, branch string) (PullRequestList, error) { prs := make([]*PullRequest, 0, 2) return prs, db.GetEngine(ctx). Where("base_repo_id=? AND base_branch=? AND has_merged=? AND issue.is_closed=?", @@ -147,7 +152,8 @@ func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptio applySorts(findSession, opts.SortType, 0) findSession = db.SetSessionPagination(findSession, opts) prs := make([]*PullRequest, 0, opts.PageSize) - return prs, maxResults, findSession.Find(&prs) + found := findSession.Find(&prs) + return prs, maxResults, found } // PullRequestList defines a list of pull requests diff --git a/models/issues/pull_list_test.go b/models/issues/pull_list_test.go index c7a898ca4e..eb2de006d6 100644 --- a/models/issues/pull_list_test.go +++ b/models/issues/pull_list_test.go @@ -16,11 +16,11 @@ import ( func TestPullRequestList_LoadAttributes(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - prs := []*issues_model.PullRequest{ + prs := issues_model.PullRequestList{ unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}), unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}), } - assert.NoError(t, issues_model.PullRequestList(prs).LoadAttributes(db.DefaultContext)) + assert.NoError(t, prs.LoadAttributes(db.DefaultContext)) for _, pr := range prs { assert.NotNil(t, pr.Issue) assert.Equal(t, pr.IssueID, pr.Issue.ID) @@ -32,26 +32,26 @@ func TestPullRequestList_LoadAttributes(t *testing.T) { func TestPullRequestList_LoadReviewCommentsCounts(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - prs := []*issues_model.PullRequest{ + prs := issues_model.PullRequestList{ unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}), unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}), } - reviewComments, err := issues_model.PullRequestList(prs).LoadReviewCommentsCounts(db.DefaultContext) + reviewComments, err := prs.LoadReviewCommentsCounts(db.DefaultContext) assert.NoError(t, err) assert.Len(t, reviewComments, 2) for _, pr := range prs { - assert.EqualValues(t, 1, reviewComments[pr.IssueID]) + assert.Equal(t, 1, reviewComments[pr.IssueID]) } } func TestPullRequestList_LoadReviews(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - prs := []*issues_model.PullRequest{ + prs := issues_model.PullRequestList{ unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}), unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}), } - reviewList, err := issues_model.PullRequestList(prs).LoadReviews(db.DefaultContext) + reviewList, err := prs.LoadReviews(db.DefaultContext) assert.NoError(t, err) // 1, 7, 8, 9, 10, 22 assert.Len(t, reviewList, 6) diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go index 090659864a..39efaa5792 100644 --- a/models/issues/pull_test.go +++ b/models/issues/pull_test.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestPullRequest_LoadAttributes(t *testing.T) { @@ -76,6 +77,47 @@ func TestPullRequestsNewest(t *testing.T) { } } +func TestPullRequests_Closed_RecentSortType(t *testing.T) { + // Issue ID | Closed At. | Updated At + // 2 | 1707270001 | 1707270001 + // 3 | 1707271000 | 1707279999 + // 11 | 1707279999 | 1707275555 + tests := []struct { + sortType string + expectedIssueIDOrder []int64 + }{ + {"recentupdate", []int64{3, 11, 2}}, + {"recentclose", []int64{11, 3, 2}}, + } + + assert.NoError(t, unittest.PrepareTestDatabase()) + _, err := db.Exec(db.DefaultContext, "UPDATE issue SET closed_unix = 1707270001, updated_unix = 1707270001, is_closed = true WHERE id = 2") + require.NoError(t, err) + _, err = db.Exec(db.DefaultContext, "UPDATE issue SET closed_unix = 1707271000, updated_unix = 1707279999, is_closed = true WHERE id = 3") + require.NoError(t, err) + _, err = db.Exec(db.DefaultContext, "UPDATE issue SET closed_unix = 1707279999, updated_unix = 1707275555, is_closed = true WHERE id = 11") + require.NoError(t, err) + + for _, test := range tests { + t.Run(test.sortType, func(t *testing.T) { + prs, _, err := issues_model.PullRequests(db.DefaultContext, 1, &issues_model.PullRequestsOptions{ + ListOptions: db.ListOptions{ + Page: 1, + }, + State: "closed", + SortType: test.sortType, + }) + require.NoError(t, err) + + if assert.Len(t, prs, len(test.expectedIssueIDOrder)) { + for i := range test.expectedIssueIDOrder { + assert.Equal(t, test.expectedIssueIDOrder[i], prs[i].IssueID) + } + } + }) + } +} + func TestLoadRequestedReviewers(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) @@ -206,19 +248,6 @@ func TestGetPullRequestByIssueID(t *testing.T) { assert.True(t, issues_model.IsErrPullRequestNotExist(err)) } -func TestPullRequest_Update(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}) - pr.BaseBranch = "baseBranch" - pr.HeadBranch = "headBranch" - pr.Update(db.DefaultContext) - - pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) - assert.Equal(t, "baseBranch", pr.BaseBranch) - assert.Equal(t, "headBranch", pr.HeadBranch) - unittest.CheckConsistencyFor(t, pr) -} - func TestPullRequest_UpdateCols(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) pr := &issues_model.PullRequest{ @@ -285,7 +314,7 @@ func TestDeleteOrphanedObjects(t *testing.T) { countAfter, err := db.GetEngine(db.DefaultContext).Count(&issues_model.PullRequest{}) assert.NoError(t, err) - assert.EqualValues(t, countBefore, countAfter) + assert.Equal(t, countBefore, countAfter) } func TestParseCodeOwnersLine(t *testing.T) { @@ -318,7 +347,7 @@ func TestGetApprovers(t *testing.T) { setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly = false approvers := pr.GetApprovers(db.DefaultContext) expected := "Reviewed-by: User Five <user5@example.com>\nReviewed-by: Org Six <org6@example.com>\n" - assert.EqualValues(t, expected, approvers) + assert.Equal(t, expected, approvers) } func TestGetPullRequestByMergedCommit(t *testing.T) { diff --git a/models/issues/reaction.go b/models/issues/reaction.go index 11b3c6be20..f24001fd23 100644 --- a/models/issues/reaction.go +++ b/models/issues/reaction.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "fmt" + "strings" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" @@ -321,6 +322,11 @@ func valuesUser(m map[int64]*user_model.User) []*user_model.User { return values } +// newMigrationOriginalUser creates and returns a fake user for external user +func newMigrationOriginalUser(name string) *user_model.User { + return &user_model.User{ID: 0, Name: name, LowerName: strings.ToLower(name)} +} + // LoadUsers loads reactions' all users func (list ReactionList) LoadUsers(ctx context.Context, repo *repo_model.Repository) ([]*user_model.User, error) { if len(list) == 0 { @@ -338,7 +344,7 @@ func (list ReactionList) LoadUsers(ctx context.Context, repo *repo_model.Reposit for _, reaction := range list { if reaction.OriginalAuthor != "" { - reaction.User = user_model.NewReplaceUser(fmt.Sprintf("%s(%s)", reaction.OriginalAuthor, repo.OriginalServiceType.Name())) + reaction.User = newMigrationOriginalUser(fmt.Sprintf("%s(%s)", reaction.OriginalAuthor, repo.OriginalServiceType.Name())) } else if user, ok := userMaps[reaction.UserID]; ok { reaction.User = user } else { diff --git a/models/issues/review.go b/models/issues/review.go index 3e787273be..71fdb7456f 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -5,6 +5,7 @@ package issues import ( "context" + "errors" "fmt" "slices" "strings" @@ -374,7 +375,7 @@ func CreateReview(ctx context.Context, opts CreateReviewOptions) (*Review, error review.Type = ReviewTypeRequest review.ReviewerTeamID = opts.ReviewerTeam.ID } else { - return nil, fmt.Errorf("provide either reviewer or reviewer team") + return nil, errors.New("provide either reviewer or reviewer team") } if _, err := sess.Insert(review); err != nil { @@ -663,7 +664,7 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo } if review != nil { - // skip it when reviewer hase been request to review + // skip it when reviewer has been request to review if review.Type == ReviewTypeRequest { return nil, committer.Commit() // still commit the transaction, or committer.Close() will rollback it, even if it's a reused transaction. } @@ -930,17 +931,19 @@ func MarkConversation(ctx context.Context, comment *Comment, doer *user_model.Us } // CanMarkConversation Add or remove Conversation mark for a code comment permission check -// the PR writer , offfcial reviewer and poster can do it +// the PR writer , official reviewer and poster can do it func CanMarkConversation(ctx context.Context, issue *Issue, doer *user_model.User) (permResult bool, err error) { if doer == nil || issue == nil { - return false, fmt.Errorf("issue or doer is nil") + return false, errors.New("issue or doer is nil") } + if err = issue.LoadRepo(ctx); err != nil { + return false, err + } + if issue.Repo.IsArchived { + return false, nil + } if doer.ID != issue.PosterID { - if err = issue.LoadRepo(ctx); err != nil { - return false, err - } - p, err := access_model.GetUserRepoPermission(ctx, issue.Repo, doer) if err != nil { return false, err @@ -970,11 +973,11 @@ func DeleteReview(ctx context.Context, r *Review) error { defer committer.Close() if r.ID == 0 { - return fmt.Errorf("review is not allowed to be 0") + return errors.New("review is not allowed to be 0") } if r.Type == ReviewTypeRequest { - return fmt.Errorf("review request can not be deleted using this method") + return errors.New("review request can not be deleted using this method") } opts := FindCommentsOptions{ diff --git a/models/issues/review_list.go b/models/issues/review_list.go index 928f24fb2d..bbb8c489fa 100644 --- a/models/issues/review_list.go +++ b/models/issues/review_list.go @@ -22,7 +22,7 @@ type ReviewList []*Review // LoadReviewers loads reviewers func (reviews ReviewList) LoadReviewers(ctx context.Context) error { reviewerIDs := make([]int64, len(reviews)) - for i := 0; i < len(reviews); i++ { + for i := range reviews { reviewerIDs[i] = reviews[i].ReviewerID } reviewers, err := user_model.GetPossibleUserByIDs(ctx, reviewerIDs) diff --git a/models/issues/stopwatch.go b/models/issues/stopwatch.go index 7c05a3a883..761b8f91a0 100644 --- a/models/issues/stopwatch.go +++ b/models/issues/stopwatch.go @@ -5,7 +5,6 @@ package issues import ( "context" - "fmt" "time" "code.gitea.io/gitea/models/db" @@ -15,20 +14,6 @@ import ( "code.gitea.io/gitea/modules/util" ) -// ErrIssueStopwatchNotExist represents an error that stopwatch is not exist -type ErrIssueStopwatchNotExist struct { - UserID int64 - IssueID int64 -} - -func (err ErrIssueStopwatchNotExist) Error() string { - return fmt.Sprintf("issue stopwatch doesn't exist[uid: %d, issue_id: %d", err.UserID, err.IssueID) -} - -func (err ErrIssueStopwatchNotExist) Unwrap() error { - return util.ErrNotExist -} - // Stopwatch represents a stopwatch for time tracking. type Stopwatch struct { ID int64 `xorm:"pk autoincr"` @@ -55,13 +40,11 @@ func getStopwatch(ctx context.Context, userID, issueID int64) (sw *Stopwatch, ex return sw, exists, err } -// UserIDCount is a simple coalition of UserID and Count type UserStopwatch struct { UserID int64 StopWatches []*Stopwatch } -// GetUIDsAndNotificationCounts between the two provided times func GetUIDsAndStopwatch(ctx context.Context) ([]*UserStopwatch, error) { sws := []*Stopwatch{} if err := db.GetEngine(ctx).Where("issue_id != 0").Find(&sws); err != nil { @@ -87,7 +70,7 @@ func GetUIDsAndStopwatch(ctx context.Context) ([]*UserStopwatch, error) { return res, nil } -// GetUserStopwatches return list of all stopwatches of a user +// GetUserStopwatches return list of the user's all stopwatches func GetUserStopwatches(ctx context.Context, userID int64, listOptions db.ListOptions) ([]*Stopwatch, error) { sws := make([]*Stopwatch, 0, 8) sess := db.GetEngine(ctx).Where("stopwatch.user_id = ?", userID) @@ -102,7 +85,7 @@ func GetUserStopwatches(ctx context.Context, userID int64, listOptions db.ListOp return sws, nil } -// CountUserStopwatches return count of all stopwatches of a user +// CountUserStopwatches return count of the user's all stopwatches func CountUserStopwatches(ctx context.Context, userID int64) (int64, error) { return db.GetEngine(ctx).Where("user_id = ?", userID).Count(&Stopwatch{}) } @@ -136,43 +119,21 @@ func HasUserStopwatch(ctx context.Context, userID int64) (exists bool, sw *Stopw return exists, sw, issue, err } -// FinishIssueStopwatchIfPossible if stopwatch exist then finish it otherwise ignore -func FinishIssueStopwatchIfPossible(ctx context.Context, user *user_model.User, issue *Issue) error { - _, exists, err := getStopwatch(ctx, user.ID, issue.ID) - if err != nil { - return err - } - if !exists { - return nil - } - return FinishIssueStopwatch(ctx, user, issue) -} - -// CreateOrStopIssueStopwatch create an issue stopwatch if it's not exist, otherwise finish it -func CreateOrStopIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { - _, exists, err := getStopwatch(ctx, user.ID, issue.ID) - if err != nil { - return err - } - if exists { - return FinishIssueStopwatch(ctx, user, issue) - } - return CreateIssueStopwatch(ctx, user, issue) -} - -// FinishIssueStopwatch if stopwatch exist then finish it otherwise return an error -func FinishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { +// FinishIssueStopwatch if stopwatch exists, then finish it. +func FinishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) (ok bool, err error) { sw, exists, err := getStopwatch(ctx, user.ID, issue.ID) if err != nil { - return err + return false, err + } else if !exists { + return false, nil } - if !exists { - return ErrIssueStopwatchNotExist{ - UserID: user.ID, - IssueID: issue.ID, - } + if err = finishIssueStopwatch(ctx, user, issue, sw); err != nil { + return false, err } + return true, nil +} +func finishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue, sw *Stopwatch) error { // Create tracked time out of the time difference between start date and actual date timediff := time.Now().Unix() - int64(sw.CreatedUnix) @@ -184,14 +145,12 @@ func FinishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Iss Time: timediff, } - if err := db.Insert(ctx, tt); err != nil { + if err := issue.LoadRepo(ctx); err != nil { return err } - - if err := issue.LoadRepo(ctx); err != nil { + if err := db.Insert(ctx, tt); err != nil { return err } - if _, err := CreateComment(ctx, &CreateCommentOptions{ Doer: user, Issue: issue, @@ -202,83 +161,65 @@ func FinishIssueStopwatch(ctx context.Context, user *user_model.User, issue *Iss }); err != nil { return err } - _, err = db.DeleteByBean(ctx, sw) + _, err := db.DeleteByBean(ctx, sw) return err } -// CreateIssueStopwatch creates a stopwatch if not exist, otherwise return an error -func CreateIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { - if err := issue.LoadRepo(ctx); err != nil { - return err - } - - // if another stopwatch is running: stop it - exists, _, otherIssue, err := HasUserStopwatch(ctx, user.ID) - if err != nil { - return err - } - if exists { - if err := FinishIssueStopwatch(ctx, user, otherIssue); err != nil { - return err +// CreateIssueStopwatch creates a stopwatch if the issue doesn't have the user's stopwatch. +// It also stops any other stopwatch that might be running for the user. +func CreateIssueStopwatch(ctx context.Context, user *user_model.User, issue *Issue) (ok bool, err error) { + { // if another issue's stopwatch is running: stop it; if this issue has a stopwatch: return an error. + exists, otherStopWatch, otherIssue, err := HasUserStopwatch(ctx, user.ID) + if err != nil { + return false, err + } + if exists { + if otherStopWatch.IssueID == issue.ID { + // don't allow starting stopwatch for the same issue + return false, nil + } + // stop the other issue's stopwatch + if err = finishIssueStopwatch(ctx, user, otherIssue, otherStopWatch); err != nil { + return false, err + } } } - // Create stopwatch - sw := &Stopwatch{ - UserID: user.ID, - IssueID: issue.ID, + if err = issue.LoadRepo(ctx); err != nil { + return false, err } - - if err := db.Insert(ctx, sw); err != nil { - return err + if err = db.Insert(ctx, &Stopwatch{UserID: user.ID, IssueID: issue.ID}); err != nil { + return false, err } - - if err := issue.LoadRepo(ctx); err != nil { - return err - } - - if _, err := CreateComment(ctx, &CreateCommentOptions{ + if _, err = CreateComment(ctx, &CreateCommentOptions{ Doer: user, Issue: issue, Repo: issue.Repo, Type: CommentTypeStartTracking, }); err != nil { - return err + return false, err } - - return nil + return true, nil } // CancelStopwatch removes the given stopwatch and logs it into issue's timeline. -func CancelStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - if err := cancelStopwatch(ctx, user, issue); err != nil { - return err - } - return committer.Commit() -} - -func cancelStopwatch(ctx context.Context, user *user_model.User, issue *Issue) error { - e := db.GetEngine(ctx) - sw, exists, err := getStopwatch(ctx, user.ID, issue.ID) - if err != nil { - return err - } - - if exists { - if _, err := e.Delete(sw); err != nil { +func CancelStopwatch(ctx context.Context, user *user_model.User, issue *Issue) (ok bool, err error) { + err = db.WithTx(ctx, func(ctx context.Context) error { + e := db.GetEngine(ctx) + sw, exists, err := getStopwatch(ctx, user.ID, issue.ID) + if err != nil { return err + } else if !exists { + return nil } - if err := issue.LoadRepo(ctx); err != nil { + if err = issue.LoadRepo(ctx); err != nil { return err } - - if _, err := CreateComment(ctx, &CreateCommentOptions{ + if _, err = e.Delete(sw); err != nil { + return err + } + if _, err = CreateComment(ctx, &CreateCommentOptions{ Doer: user, Issue: issue, Repo: issue.Repo, @@ -286,6 +227,8 @@ func cancelStopwatch(ctx context.Context, user *user_model.User, issue *Issue) e }); err != nil { return err } - } - return nil + ok = true + return nil + }) + return ok, err } diff --git a/models/issues/stopwatch_test.go b/models/issues/stopwatch_test.go index a1bf9dc931..6333c10234 100644 --- a/models/issues/stopwatch_test.go +++ b/models/issues/stopwatch_test.go @@ -10,7 +10,6 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/timeutil" "github.com/stretchr/testify/assert" ) @@ -18,26 +17,22 @@ import ( func TestCancelStopwatch(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - user1, err := user_model.GetUserByID(db.DefaultContext, 1) - assert.NoError(t, err) - - issue1, err := issues_model.GetIssueByID(db.DefaultContext, 1) - assert.NoError(t, err) - issue2, err := issues_model.GetIssueByID(db.DefaultContext, 2) - assert.NoError(t, err) + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + issue1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) - err = issues_model.CancelStopwatch(db.DefaultContext, user1, issue1) + ok, err := issues_model.CancelStopwatch(db.DefaultContext, user1, issue1) assert.NoError(t, err) + assert.True(t, ok) unittest.AssertNotExistsBean(t, &issues_model.Stopwatch{UserID: user1.ID, IssueID: issue1.ID}) + unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{Type: issues_model.CommentTypeCancelTracking, PosterID: user1.ID, IssueID: issue1.ID}) - _ = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{Type: issues_model.CommentTypeCancelTracking, PosterID: user1.ID, IssueID: issue1.ID}) - - assert.NoError(t, issues_model.CancelStopwatch(db.DefaultContext, user1, issue2)) + ok, err = issues_model.CancelStopwatch(db.DefaultContext, user1, issue1) + assert.NoError(t, err) + assert.False(t, ok) } func TestStopwatchExists(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - assert.True(t, issues_model.StopwatchExists(db.DefaultContext, 1, 1)) assert.False(t, issues_model.StopwatchExists(db.DefaultContext, 1, 2)) } @@ -58,21 +53,35 @@ func TestHasUserStopwatch(t *testing.T) { func TestCreateOrStopIssueStopwatch(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - user2, err := user_model.GetUserByID(db.DefaultContext, 2) + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + issue1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + issue3 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3}) + + // create a new stopwatch + ok, err := issues_model.CreateIssueStopwatch(db.DefaultContext, user4, issue1) assert.NoError(t, err) - org3, err := user_model.GetUserByID(db.DefaultContext, 3) + assert.True(t, ok) + unittest.AssertExistsAndLoadBean(t, &issues_model.Stopwatch{UserID: user4.ID, IssueID: issue1.ID}) + // should not create a second stopwatch for the same issue + ok, err = issues_model.CreateIssueStopwatch(db.DefaultContext, user4, issue1) assert.NoError(t, err) - - issue1, err := issues_model.GetIssueByID(db.DefaultContext, 1) + assert.False(t, ok) + // on a different issue, it will finish the existing stopwatch and create a new one + ok, err = issues_model.CreateIssueStopwatch(db.DefaultContext, user4, issue3) assert.NoError(t, err) - issue2, err := issues_model.GetIssueByID(db.DefaultContext, 2) + assert.True(t, ok) + unittest.AssertNotExistsBean(t, &issues_model.Stopwatch{UserID: user4.ID, IssueID: issue1.ID}) + unittest.AssertExistsAndLoadBean(t, &issues_model.Stopwatch{UserID: user4.ID, IssueID: issue3.ID}) + + // user2 already has a stopwatch in test fixture + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + issue2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) + ok, err = issues_model.FinishIssueStopwatch(db.DefaultContext, user2, issue2) assert.NoError(t, err) - - assert.NoError(t, issues_model.CreateOrStopIssueStopwatch(db.DefaultContext, org3, issue1)) - sw := unittest.AssertExistsAndLoadBean(t, &issues_model.Stopwatch{UserID: 3, IssueID: 1}) - assert.LessOrEqual(t, sw.CreatedUnix, timeutil.TimeStampNow()) - - assert.NoError(t, issues_model.CreateOrStopIssueStopwatch(db.DefaultContext, user2, issue2)) - unittest.AssertNotExistsBean(t, &issues_model.Stopwatch{UserID: 2, IssueID: 2}) - unittest.AssertExistsAndLoadBean(t, &issues_model.TrackedTime{UserID: 2, IssueID: 2}) + assert.True(t, ok) + unittest.AssertNotExistsBean(t, &issues_model.Stopwatch{UserID: user2.ID, IssueID: issue2.ID}) + unittest.AssertExistsAndLoadBean(t, &issues_model.TrackedTime{UserID: user2.ID, IssueID: issue2.ID}) + ok, err = issues_model.FinishIssueStopwatch(db.DefaultContext, user2, issue2) + assert.NoError(t, err) + assert.False(t, ok) } diff --git a/models/issues/tracked_time.go b/models/issues/tracked_time.go index ea404d36cd..2afbe272ed 100644 --- a/models/issues/tracked_time.go +++ b/models/issues/tracked_time.go @@ -350,10 +350,7 @@ func GetIssueTotalTrackedTime(ctx context.Context, opts *IssuesOptions, isClosed // we get the statistics in smaller chunks and get accumulates var accum int64 for i := 0; i < len(opts.IssueIDs); { - chunk := i + MaxQueryParameters - if chunk > len(opts.IssueIDs) { - chunk = len(opts.IssueIDs) - } + chunk := min(i+MaxQueryParameters, len(opts.IssueIDs)) time, err := getIssueTotalTrackedTimeChunk(ctx, opts, isClosed, opts.IssueIDs[i:chunk]) if err != nil { return 0, err |