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

Move some issue methods as functions #19255

Merged
merged 4 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
85 changes: 32 additions & 53 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,23 +456,6 @@ func (issue *Issue) IsPoster(uid int64) bool {
return issue.OriginalAuthorID == 0 && issue.PosterID == uid
}

func (issue *Issue) hasLabel(e db.Engine, labelID int64) bool {
return hasIssueLabel(e, issue.ID, labelID)
}

// HasLabel returns true if issue has been labeled by given ID.
func (issue *Issue) HasLabel(labelID int64) bool {
return issue.hasLabel(db.GetEngine(db.DefaultContext), labelID)
}

func (issue *Issue) addLabel(ctx context.Context, label *Label, doer *user_model.User) error {
return newIssueLabel(ctx, issue, label, doer)
}

func (issue *Issue) addLabels(ctx context.Context, labels []*Label, doer *user_model.User) error {
return newIssueLabels(ctx, issue, labels, doer)
}

func (issue *Issue) getLabels(e db.Engine) (err error) {
if len(issue.Labels) > 0 {
return nil
Expand All @@ -485,27 +468,23 @@ func (issue *Issue) getLabels(e db.Engine) (err error) {
return nil
}

func (issue *Issue) removeLabel(ctx context.Context, doer *user_model.User, label *Label) error {
return deleteIssueLabel(ctx, issue, label, doer)
}

func (issue *Issue) clearLabels(ctx context.Context, doer *user_model.User) (err error) {
func clearIssueLabels(ctx context.Context, issue *Issue, doer *user_model.User) (err error) {
if err = issue.getLabels(db.GetEngine(ctx)); err != nil {
return fmt.Errorf("getLabels: %v", err)
}

for i := range issue.Labels {
if err = issue.removeLabel(ctx, doer, issue.Labels[i]); err != nil {
if err = deleteIssueLabel(ctx, issue, issue.Labels[i], doer); err != nil {
return fmt.Errorf("removeLabel: %v", err)
}
}

return nil
}

// ClearLabels removes all issue labels as the given user.
// ClearIssueLabels removes all issue labels as the given user.
// Triggers appropriate WebHooks, if any.
func (issue *Issue) ClearLabels(doer *user_model.User) (err error) {
func ClearIssueLabels(issue *Issue, doer *user_model.User) (err error) {
ctx, committer, err := db.TxContext()
if err != nil {
return err
Expand All @@ -526,7 +505,7 @@ func (issue *Issue) ClearLabels(doer *user_model.User) (err error) {
return ErrRepoLabelNotExist{}
}

if err = issue.clearLabels(ctx, doer); err != nil {
if err = clearIssueLabels(ctx, issue, doer); err != nil {
return err
}

Expand All @@ -551,9 +530,9 @@ func (ts labelSorter) Swap(i, j int) {
[]*Label(ts)[i], []*Label(ts)[j] = []*Label(ts)[j], []*Label(ts)[i]
}

// ReplaceLabels removes all current labels and add new labels to the issue.
// ReplaceIssueLabels removes all current labels and add new labels to the issue.
// Triggers appropriate WebHooks, if any.
func (issue *Issue) ReplaceLabels(labels []*Label, doer *user_model.User) (err error) {
func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (err error) {
ctx, committer, err := db.TxContext()
if err != nil {
return err
Expand Down Expand Up @@ -600,13 +579,13 @@ func (issue *Issue) ReplaceLabels(labels []*Label, doer *user_model.User) (err e
toRemove = append(toRemove, issue.Labels[removeIndex:]...)

if len(toAdd) > 0 {
if err = issue.addLabels(ctx, toAdd, doer); err != nil {
if err = newIssueLabels(ctx, issue, toAdd, doer); err != nil {
return fmt.Errorf("addLabels: %v", err)
}
}

for _, l := range toRemove {
if err = issue.removeLabel(ctx, doer, l); err != nil {
if err = deleteIssueLabel(ctx, issue, l, doer); err != nil {
return fmt.Errorf("removeLabel: %v", err)
}
}
Expand Down Expand Up @@ -635,7 +614,7 @@ func updateIssueCols(ctx context.Context, issue *Issue, cols ...string) error {
return nil
}

func (issue *Issue) changeStatus(ctx context.Context, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
// Reload the issue
currentIssue, err := getIssueByID(db.GetEngine(ctx), issue.ID)
if err != nil {
Expand All @@ -655,10 +634,10 @@ func (issue *Issue) changeStatus(ctx context.Context, doer *user_model.User, isC
}

issue.IsClosed = isClosed
return issue.doChangeStatus(ctx, doer, isMergePull)
return doChangeIssueStatus(ctx, issue, doer, isMergePull)
}

func (issue *Issue) doChangeStatus(ctx context.Context, doer *user_model.User, isMergePull bool) (*Comment, error) {
func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isMergePull bool) (*Comment, error) {
e := db.GetEngine(ctx)
// Check for open dependencies
if issue.IsClosed && issue.Repo.IsDependenciesEnabledCtx(ctx) {
Expand Down Expand Up @@ -700,7 +679,7 @@ func (issue *Issue) doChangeStatus(ctx context.Context, doer *user_model.User, i
}
}

if err := issue.updateClosedNum(ctx); err != nil {
if err := updateIssueClosedNum(ctx, issue); err != nil {
return nil, err
}

Expand All @@ -720,8 +699,8 @@ func (issue *Issue) doChangeStatus(ctx context.Context, doer *user_model.User, i
})
}

// ChangeStatus changes issue status to open or closed.
func (issue *Issue) ChangeStatus(doer *user_model.User, isClosed bool) (*Comment, error) {
// ChangeIssueStatus changes issue status to open or closed.
func ChangeIssueStatus(issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) {
ctx, committer, err := db.TxContext()
if err != nil {
return nil, err
Expand All @@ -735,7 +714,7 @@ func (issue *Issue) ChangeStatus(doer *user_model.User, isClosed bool) (*Comment
return nil, err
}

comment, err := issue.changeStatus(ctx, doer, isClosed, false)
comment, err := changeIssueStatus(ctx, issue, doer, isClosed, false)
if err != nil {
return nil, err
}
Expand All @@ -747,8 +726,8 @@ func (issue *Issue) ChangeStatus(doer *user_model.User, isClosed bool) (*Comment
return comment, nil
}

// ChangeTitle changes the title of this issue, as the given user.
func (issue *Issue) ChangeTitle(doer *user_model.User, oldTitle string) (err error) {
// ChangeIssueTitle changes the title of this issue, as the given user.
func ChangeIssueTitle(issue *Issue, doer *user_model.User, oldTitle string) (err error) {
ctx, committer, err := db.TxContext()
if err != nil {
return err
Expand Down Expand Up @@ -781,8 +760,8 @@ func (issue *Issue) ChangeTitle(doer *user_model.User, oldTitle string) (err err
return committer.Commit()
}

// ChangeRef changes the branch of this issue, as the given user.
func (issue *Issue) ChangeRef(doer *user_model.User, oldRef string) (err error) {
// ChangeIssueRef changes the branch of this issue, as the given user.
func ChangeIssueRef(issue *Issue, doer *user_model.User, oldRef string) (err error) {
ctx, committer, err := db.TxContext()
if err != nil {
return err
Expand Down Expand Up @@ -839,8 +818,8 @@ func AddDeletePRBranchComment(doer *user_model.User, repo *repo_model.Repository
return committer.Commit()
}

// UpdateAttachments update attachments by UUIDs for the issue
func (issue *Issue) UpdateAttachments(uuids []string) (err error) {
// UpdateIssueAttachments update attachments by UUIDs for the issue
func UpdateIssueAttachments(issueID int64, uuids []string) (err error) {
ctx, committer, err := db.TxContext()
if err != nil {
return err
Expand All @@ -851,16 +830,16 @@ func (issue *Issue) UpdateAttachments(uuids []string) (err error) {
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %v", uuids, err)
}
for i := 0; i < len(attachments); i++ {
attachments[i].IssueID = issue.ID
attachments[i].IssueID = issueID
if err := repo_model.UpdateAttachmentCtx(ctx, attachments[i]); err != nil {
return fmt.Errorf("update attachment [id: %d]: %v", attachments[i].ID, err)
}
}
return committer.Commit()
}

// ChangeContent changes issue content, as the given user.
func (issue *Issue) ChangeContent(doer *user_model.User, content string) (err error) {
// ChangeIssueContent changes issue content, as the given user.
func ChangeIssueContent(issue *Issue, doer *user_model.User, content string) (err error) {
ctx, committer, err := db.TxContext()
if err != nil {
return err
Expand Down Expand Up @@ -1033,7 +1012,7 @@ func newIssue(ctx context.Context, doer *user_model.User, opts NewIssueOptions)
continue
}

if err = opts.Issue.addLabel(ctx, label, opts.Issue.Poster); err != nil {
if err = newIssueLabel(ctx, opts.Issue, label, opts.Issue.Poster); err != nil {
return fmt.Errorf("addLabel [id: %d]: %v", label.ID, err)
}
}
Expand Down Expand Up @@ -2010,7 +1989,7 @@ func UpdateIssueByAPI(issue *Issue, doer *user_model.User) (statusChangeComment
}

if currentIssue.IsClosed != issue.IsClosed {
statusChangeComment, err = issue.doChangeStatus(ctx, doer, false)
statusChangeComment, err = doChangeIssueStatus(ctx, issue, doer, false)
if err != nil {
return nil, false, err
}
Expand Down Expand Up @@ -2234,7 +2213,7 @@ func (issue *Issue) BlockingDependencies() ([]*DependencyInfo, error) {
return issue.getBlockingDependencies(db.GetEngine(db.DefaultContext))
}

func (issue *Issue) updateClosedNum(ctx context.Context) (err error) {
func updateIssueClosedNum(ctx context.Context, issue *Issue) (err error) {
if issue.IsPull {
err = repoStatsCorrectNumClosed(ctx, issue.RepoID, true, "num_closed_pulls")
} else {
Expand All @@ -2244,9 +2223,9 @@ func (issue *Issue) updateClosedNum(ctx context.Context) (err error) {
}

// FindAndUpdateIssueMentions finds users mentioned in the given content string, and saves them in the database.
func (issue *Issue) FindAndUpdateIssueMentions(ctx context.Context, doer *user_model.User, content string) (mentions []*user_model.User, err error) {
func FindAndUpdateIssueMentions(ctx context.Context, issue *Issue, doer *user_model.User, content string) (mentions []*user_model.User, err error) {
rawMentions := references.FindAllMentionsMarkdown(content)
mentions, err = issue.ResolveMentionsByVisibility(ctx, doer, rawMentions)
mentions, err = ResolveIssueMentionsByVisibility(ctx, issue, doer, rawMentions)
if err != nil {
return nil, fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
}
Expand All @@ -2256,9 +2235,9 @@ func (issue *Issue) FindAndUpdateIssueMentions(ctx context.Context, doer *user_m
return
}

// ResolveMentionsByVisibility returns the users mentioned in an issue, removing those that
// ResolveIssueMentionsByVisibility returns the users mentioned in an issue, removing those that
// don't have access to reading it. Teams are expanded into their users, but organizations are ignored.
func (issue *Issue) ResolveMentionsByVisibility(ctx context.Context, doer *user_model.User, mentions []string) (users []*user_model.User, err error) {
func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *user_model.User, mentions []string) (users []*user_model.User, err error) {
if len(mentions) == 0 {
return
}
Expand Down
8 changes: 4 additions & 4 deletions models/issue_assignees.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,15 @@ func clearAssigneeByUserID(sess db.Engine, userID int64) (err error) {
return
}

// ToggleAssignee changes a user between assigned and not assigned for this issue, and make issue comment for it.
func (issue *Issue) ToggleAssignee(doer *user_model.User, assigneeID int64) (removed bool, comment *Comment, err error) {
// ToggleIssueAssignee changes a user between assigned and not assigned for this issue, and make issue comment for it.
func ToggleIssueAssignee(issue *Issue, doer *user_model.User, assigneeID int64) (removed bool, comment *Comment, err error) {
ctx, committer, err := db.TxContext()
if err != nil {
return false, nil, err
}
defer committer.Close()

removed, comment, err = issue.toggleAssignee(ctx, doer, assigneeID, false)
removed, comment, err = toggleIssueAssignee(ctx, issue, doer, assigneeID, false)
if err != nil {
return false, nil, err
}
Expand All @@ -112,7 +112,7 @@ func (issue *Issue) ToggleAssignee(doer *user_model.User, assigneeID int64) (rem
return removed, comment, nil
}

func (issue *Issue) toggleAssignee(ctx context.Context, doer *user_model.User, assigneeID int64, isCreate bool) (removed bool, comment *Comment, err error) {
func toggleIssueAssignee(ctx context.Context, issue *Issue, doer *user_model.User, assigneeID int64, isCreate bool) (removed bool, comment *Comment, err error) {
sess := db.GetEngine(ctx)
removed, err = toggleUserAssignee(sess, issue, assigneeID)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions models/issue_assignees_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@ func TestUpdateAssignee(t *testing.T) {
// Assign multiple users
user2, err := user_model.GetUserByID(2)
assert.NoError(t, err)
_, _, err = issue.ToggleAssignee(&user_model.User{ID: 1}, user2.ID)
_, _, err = ToggleIssueAssignee(issue, &user_model.User{ID: 1}, user2.ID)
assert.NoError(t, err)

user3, err := user_model.GetUserByID(3)
assert.NoError(t, err)
_, _, err = issue.ToggleAssignee(&user_model.User{ID: 1}, user3.ID)
_, _, err = ToggleIssueAssignee(issue, &user_model.User{ID: 1}, user3.ID)
assert.NoError(t, err)

user1, err := user_model.GetUserByID(1) // This user is already assigned (see the definition in fixtures), so running UpdateAssignee should unassign him
assert.NoError(t, err)
_, _, err = issue.ToggleAssignee(&user_model.User{ID: 1}, user1.ID)
_, _, err = ToggleIssueAssignee(issue, &user_model.User{ID: 1}, user1.ID)
assert.NoError(t, err)

// Check if he got removed
Expand Down
2 changes: 1 addition & 1 deletion models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
}
}
case CommentTypeReopen, CommentTypeClose:
if err = opts.Issue.updateClosedNum(ctx); err != nil {
if err = updateIssueClosedNum(ctx, opts.Issue); err != nil {
return err
}
}
Expand Down
2 changes: 1 addition & 1 deletion models/issue_dependency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestCreateIssueDependency(t *testing.T) {
assert.False(t, left)

// Close #2 and check again
_, err = issue2.ChangeStatus(user1, true)
_, err = ChangeIssueStatus(issue2, user1, true)
assert.NoError(t, err)

left, err = IssueNoDependenciesLeft(issue1)
Expand Down
6 changes: 3 additions & 3 deletions models/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestIssue_ReplaceLabels(t *testing.T) {
for i, labelID := range labelIDs {
labels[i] = unittest.AssertExistsAndLoadBean(t, &Label{ID: labelID, RepoID: repo.ID}).(*Label)
}
assert.NoError(t, issue.ReplaceLabels(labels, doer))
assert.NoError(t, ReplaceIssueLabels(issue, labels, doer))
unittest.AssertCount(t, &IssueLabel{IssueID: issueID}, len(labelIDs))
for _, labelID := range labelIDs {
unittest.AssertExistsAndLoadBean(t, &IssueLabel{IssueID: issueID, LabelID: labelID})
Expand Down Expand Up @@ -116,7 +116,7 @@ func TestIssue_ClearLabels(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &Issue{ID: test.issueID}).(*Issue)
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: test.doerID}).(*user_model.User)
assert.NoError(t, issue.ClearLabels(doer))
assert.NoError(t, ClearIssueLabels(issue, doer))
unittest.AssertNotExistsBean(t, &IssueLabel{IssueID: test.issueID})
}
}
Expand Down Expand Up @@ -459,7 +459,7 @@ func TestIssue_ResolveMentions(t *testing.T) {
r := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: o.ID, LowerName: repo}).(*repo_model.Repository)
issue := &Issue{RepoID: r.ID}
d := unittest.AssertExistsAndLoadBean(t, &user_model.User{LowerName: doer}).(*user_model.User)
resolved, err := issue.ResolveMentionsByVisibility(db.DefaultContext, d, mentions)
resolved, err := ResolveIssueMentionsByVisibility(db.DefaultContext, issue, d, mentions)
assert.NoError(t, err)
ids := make([]int64, len(resolved))
for i, user := range resolved {
Expand Down
4 changes: 2 additions & 2 deletions models/issue_xref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestXRef_NeuterCrossReferences(t *testing.T) {

d := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User)
i.Title = "title2, no mentions"
assert.NoError(t, i.ChangeTitle(d, title))
assert.NoError(t, ChangeIssueTitle(i, d, title))

ref = unittest.AssertExistsAndLoadBean(t, &Comment{IssueID: itarget.ID, RefIssueID: i.ID, RefCommentID: 0}).(*Comment)
assert.Equal(t, CommentTypeIssueRef, ref.Type)
Expand All @@ -98,7 +98,7 @@ func TestXRef_ResolveCrossReferences(t *testing.T) {
i1 := testCreateIssue(t, 1, 2, "title1", "content1", false)
i2 := testCreateIssue(t, 1, 2, "title2", "content2", false)
i3 := testCreateIssue(t, 1, 2, "title3", "content3", false)
_, err := i3.ChangeStatus(d, true)
_, err := ChangeIssueStatus(i3, d, true)
assert.NoError(t, err)

pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))
Expand Down
2 changes: 1 addition & 1 deletion models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ func (pr *PullRequest) SetMerged() (bool, error) {
return false, err
}

if _, err := pr.Issue.changeStatus(ctx, pr.Merger, true, true); err != nil {
if _, err := changeIssueStatus(ctx, pr.Issue, pr.Merger, true, true); err != nil {
return false, fmt.Errorf("Issue.changeStatus: %v", err)
}

Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ func EditPullRequest(ctx *context.APIContext) {
labels = append(labels, orgLabels...)
}

if err = issue.ReplaceLabels(labels, ctx.Doer); err != nil {
if err = models.ReplaceIssueLabels(issue, labels, ctx.Doer); err != nil {
ctx.Error(http.StatusInternalServerError, "ReplaceLabelsError", err)
return
}
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -2594,7 +2594,7 @@ func updateAttachments(item interface{}, files []string) error {
if len(files) > 0 {
switch content := item.(type) {
case *models.Issue:
err = content.UpdateAttachments(files)
err = models.UpdateIssueAttachments(content.ID, files)
case *models.Comment:
err = content.UpdateAttachments(files)
default:
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/issue_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func UpdateIssueLabel(ctx *context.Context) {
// detach if any issues already have label, otherwise attach
action = "attach"
for _, issue := range issues {
if issue.HasLabel(label.ID) {
if models.HasIssueLabel(issue.ID, label.ID) {
action = "detach"
break
}
Expand Down
Loading