Skip to content

Commit 74731c3

Browse files
lunnylunny
authored
Move some issue methods as functions (#19255)
* Move some issue methods as functions * Fix bug
1 parent bd97736 commit 74731c3

19 files changed

+63
-84
lines changed

‎models/issue.go

Lines changed: 32 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -457,23 +457,6 @@ func (issue *Issue) IsPoster(uid int64) bool {
457457
return issue.OriginalAuthorID == 0 && issue.PosterID == uid
458458
}
459459

460-
func (issue *Issue) hasLabel(e db.Engine, labelID int64) bool {
461-
return hasIssueLabel(e, issue.ID, labelID)
462-
}
463-
464-
// HasLabel returns true if issue has been labeled by given ID.
465-
func (issue *Issue) HasLabel(labelID int64) bool {
466-
return issue.hasLabel(db.GetEngine(db.DefaultContext), labelID)
467-
}
468-
469-
func (issue *Issue) addLabel(ctx context.Context, label *Label, doer *user_model.User) error {
470-
return newIssueLabel(ctx, issue, label, doer)
471-
}
472-
473-
func (issue *Issue) addLabels(ctx context.Context, labels []*Label, doer *user_model.User) error {
474-
return newIssueLabels(ctx, issue, labels, doer)
475-
}
476-
477460
func (issue *Issue) getLabels(e db.Engine) (err error) {
478461
if len(issue.Labels) > 0 {
479462
return nil
@@ -486,27 +469,23 @@ func (issue *Issue) getLabels(e db.Engine) (err error) {
486469
return nil
487470
}
488471

489-
func (issue *Issue) removeLabel(ctx context.Context, doer *user_model.User, label *Label) error {
490-
return deleteIssueLabel(ctx, issue, label, doer)
491-
}
492-
493-
func (issue *Issue) clearLabels(ctx context.Context, doer *user_model.User) (err error) {
472+
func clearIssueLabels(ctx context.Context, issue *Issue, doer *user_model.User) (err error) {
494473
if err = issue.getLabels(db.GetEngine(ctx)); err != nil {
495474
return fmt.Errorf("getLabels: %v", err)
496475
}
497476

498477
for i := range issue.Labels {
499-
if err = issue.removeLabel(ctx, doer, issue.Labels[i]); err != nil {
478+
if err = deleteIssueLabel(ctx, issue, issue.Labels[i], doer); err != nil {
500479
return fmt.Errorf("removeLabel: %v", err)
501480
}
502481
}
503482

504483
return nil
505484
}
506485

507-
// ClearLabels removes all issue labels as the given user.
486+
// ClearIssueLabels removes all issue labels as the given user.
508487
// Triggers appropriate WebHooks, if any.
509-
func (issue *Issue) ClearLabels(doer *user_model.User) (err error) {
488+
func ClearIssueLabels(issue *Issue, doer *user_model.User) (err error) {
510489
ctx, committer, err := db.TxContext()
511490
if err != nil {
512491
return err
@@ -527,7 +506,7 @@ func (issue *Issue) ClearLabels(doer *user_model.User) (err error) {
527506
return ErrRepoLabelNotExist{}
528507
}
529508

530-
if err = issue.clearLabels(ctx, doer); err != nil {
509+
if err = clearIssueLabels(ctx, issue, doer); err != nil {
531510
return err
532511
}
533512

@@ -552,9 +531,9 @@ func (ts labelSorter) Swap(i, j int) {
552531
[]*Label(ts)[i], []*Label(ts)[j] = []*Label(ts)[j], []*Label(ts)[i]
553532
}
554533

555-
// ReplaceLabels removes all current labels and add new labels to the issue.
534+
// ReplaceIssueLabels removes all current labels and add new labels to the issue.
556535
// Triggers appropriate WebHooks, if any.
557-
func (issue *Issue) ReplaceLabels(labels []*Label, doer *user_model.User) (err error) {
536+
func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (err error) {
558537
ctx, committer, err := db.TxContext()
559538
if err != nil {
560539
return err
@@ -601,13 +580,13 @@ func (issue *Issue) ReplaceLabels(labels []*Label, doer *user_model.User) (err e
601580
toRemove = append(toRemove, issue.Labels[removeIndex:]...)
602581

603582
if len(toAdd) > 0 {
604-
if err = issue.addLabels(ctx, toAdd, doer); err != nil {
583+
if err = newIssueLabels(ctx, issue, toAdd, doer); err != nil {
605584
return fmt.Errorf("addLabels: %v", err)
606585
}
607586
}
608587

609588
for _, l := range toRemove {
610-
if err = issue.removeLabel(ctx, doer, l); err != nil {
589+
if err = deleteIssueLabel(ctx, issue, l, doer); err != nil {
611590
return fmt.Errorf("removeLabel: %v", err)
612591
}
613592
}
@@ -636,7 +615,7 @@ func updateIssueCols(ctx context.Context, issue *Issue, cols ...string) error {
636615
return nil
637616
}
638617

639-
func (issue *Issue) changeStatus(ctx context.Context, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
618+
func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
640619
// Reload the issue
641620
currentIssue, err := getIssueByID(db.GetEngine(ctx), issue.ID)
642621
if err != nil {
@@ -656,10 +635,10 @@ func (issue *Issue) changeStatus(ctx context.Context, doer *user_model.User, isC
656635
}
657636

658637
issue.IsClosed = isClosed
659-
return issue.doChangeStatus(ctx, doer, isMergePull)
638+
return doChangeIssueStatus(ctx, issue, doer, isMergePull)
660639
}
661640

662-
func (issue *Issue) doChangeStatus(ctx context.Context, doer *user_model.User, isMergePull bool) (*Comment, error) {
641+
func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isMergePull bool) (*Comment, error) {
663642
e := db.GetEngine(ctx)
664643
// Check for open dependencies
665644
if issue.IsClosed && issue.Repo.IsDependenciesEnabledCtx(ctx) {
@@ -701,7 +680,7 @@ func (issue *Issue) doChangeStatus(ctx context.Context, doer *user_model.User, i
701680
}
702681
}
703682

704-
if err := issue.updateClosedNum(ctx); err != nil {
683+
if err := updateIssueClosedNum(ctx, issue); err != nil {
705684
return nil, err
706685
}
707686

@@ -721,8 +700,8 @@ func (issue *Issue) doChangeStatus(ctx context.Context, doer *user_model.User, i
721700
})
722701
}
723702

724-
// ChangeStatus changes issue status to open or closed.
725-
func (issue *Issue) ChangeStatus(doer *user_model.User, isClosed bool) (*Comment, error) {
703+
// ChangeIssueStatus changes issue status to open or closed.
704+
func ChangeIssueStatus(issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) {
726705
ctx, committer, err := db.TxContext()
727706
if err != nil {
728707
return nil, err
@@ -736,7 +715,7 @@ func (issue *Issue) ChangeStatus(doer *user_model.User, isClosed bool) (*Comment
736715
return nil, err
737716
}
738717

739-
comment, err := issue.changeStatus(ctx, doer, isClosed, false)
718+
comment, err := changeIssueStatus(ctx, issue, doer, isClosed, false)
740719
if err != nil {
741720
return nil, err
742721
}
@@ -748,8 +727,8 @@ func (issue *Issue) ChangeStatus(doer *user_model.User, isClosed bool) (*Comment
748727
return comment, nil
749728
}
750729

751-
// ChangeTitle changes the title of this issue, as the given user.
752-
func (issue *Issue) ChangeTitle(doer *user_model.User, oldTitle string) (err error) {
730+
// ChangeIssueTitle changes the title of this issue, as the given user.
731+
func ChangeIssueTitle(issue *Issue, doer *user_model.User, oldTitle string) (err error) {
753732
ctx, committer, err := db.TxContext()
754733
if err != nil {
755734
return err
@@ -782,8 +761,8 @@ func (issue *Issue) ChangeTitle(doer *user_model.User, oldTitle string) (err err
782761
return committer.Commit()
783762
}
784763

785-
// ChangeRef changes the branch of this issue, as the given user.
786-
func (issue *Issue) ChangeRef(doer *user_model.User, oldRef string) (err error) {
764+
// ChangeIssueRef changes the branch of this issue, as the given user.
765+
func ChangeIssueRef(issue *Issue, doer *user_model.User, oldRef string) (err error) {
787766
ctx, committer, err := db.TxContext()
788767
if err != nil {
789768
return err
@@ -840,8 +819,8 @@ func AddDeletePRBranchComment(doer *user_model.User, repo *repo_model.Repository
840819
return committer.Commit()
841820
}
842821

843-
// UpdateAttachments update attachments by UUIDs for the issue
844-
func (issue *Issue) UpdateAttachments(uuids []string) (err error) {
822+
// UpdateIssueAttachments update attachments by UUIDs for the issue
823+
func UpdateIssueAttachments(issueID int64, uuids []string) (err error) {
845824
ctx, committer, err := db.TxContext()
846825
if err != nil {
847826
return err
@@ -852,16 +831,16 @@ func (issue *Issue) UpdateAttachments(uuids []string) (err error) {
852831
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %v", uuids, err)
853832
}
854833
for i := 0; i < len(attachments); i++ {
855-
attachments[i].IssueID = issue.ID
834+
attachments[i].IssueID = issueID
856835
if err := repo_model.UpdateAttachmentCtx(ctx, attachments[i]); err != nil {
857836
return fmt.Errorf("update attachment [id: %d]: %v", attachments[i].ID, err)
858837
}
859838
}
860839
return committer.Commit()
861840
}
862841

863-
// ChangeContent changes issue content, as the given user.
864-
func (issue *Issue) ChangeContent(doer *user_model.User, content string) (err error) {
842+
// ChangeIssueContent changes issue content, as the given user.
843+
func ChangeIssueContent(issue *Issue, doer *user_model.User, content string) (err error) {
865844
ctx, committer, err := db.TxContext()
866845
if err != nil {
867846
return err
@@ -1034,7 +1013,7 @@ func newIssue(ctx context.Context, doer *user_model.User, opts NewIssueOptions)
10341013
continue
10351014
}
10361015

1037-
if err = opts.Issue.addLabel(ctx, label, opts.Issue.Poster); err != nil {
1016+
if err = newIssueLabel(ctx, opts.Issue, label, opts.Issue.Poster); err != nil {
10381017
return fmt.Errorf("addLabel [id: %d]: %v", label.ID, err)
10391018
}
10401019
}
@@ -2011,7 +1990,7 @@ func UpdateIssueByAPI(issue *Issue, doer *user_model.User) (statusChangeComment
20111990
}
20121991

20131992
if currentIssue.IsClosed != issue.IsClosed {
2014-
statusChangeComment, err = issue.doChangeStatus(ctx, doer, false)
1993+
statusChangeComment, err = doChangeIssueStatus(ctx, issue, doer, false)
20151994
if err != nil {
20161995
return nil, false, err
20171996
}
@@ -2235,7 +2214,7 @@ func (issue *Issue) BlockingDependencies() ([]*DependencyInfo, error) {
22352214
return issue.getBlockingDependencies(db.GetEngine(db.DefaultContext))
22362215
}
22372216

2238-
func (issue *Issue) updateClosedNum(ctx context.Context) (err error) {
2217+
func updateIssueClosedNum(ctx context.Context, issue *Issue) (err error) {
22392218
if issue.IsPull {
22402219
err = repoStatsCorrectNumClosed(ctx, issue.RepoID, true, "num_closed_pulls")
22412220
} else {
@@ -2245,9 +2224,9 @@ func (issue *Issue) updateClosedNum(ctx context.Context) (err error) {
22452224
}
22462225

22472226
// FindAndUpdateIssueMentions finds users mentioned in the given content string, and saves them in the database.
2248-
func (issue *Issue) FindAndUpdateIssueMentions(ctx context.Context, doer *user_model.User, content string) (mentions []*user_model.User, err error) {
2227+
func FindAndUpdateIssueMentions(ctx context.Context, issue *Issue, doer *user_model.User, content string) (mentions []*user_model.User, err error) {
22492228
rawMentions := references.FindAllMentionsMarkdown(content)
2250-
mentions, err = issue.ResolveMentionsByVisibility(ctx, doer, rawMentions)
2229+
mentions, err = ResolveIssueMentionsByVisibility(ctx, issue, doer, rawMentions)
22512230
if err != nil {
22522231
return nil, fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
22532232
}
@@ -2257,9 +2236,9 @@ func (issue *Issue) FindAndUpdateIssueMentions(ctx context.Context, doer *user_m
22572236
return
22582237
}
22592238

2260-
// ResolveMentionsByVisibility returns the users mentioned in an issue, removing those that
2239+
// ResolveIssueMentionsByVisibility returns the users mentioned in an issue, removing those that
22612240
// don't have access to reading it. Teams are expanded into their users, but organizations are ignored.
2262-
func (issue *Issue) ResolveMentionsByVisibility(ctx context.Context, doer *user_model.User, mentions []string) (users []*user_model.User, err error) {
2241+
func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *user_model.User, mentions []string) (users []*user_model.User, err error) {
22632242
if len(mentions) == 0 {
22642243
return
22652244
}

‎models/issue_assignees.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,15 +92,15 @@ func clearAssigneeByUserID(sess db.Engine, userID int64) (err error) {
9292
return
9393
}
9494

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

103-
removed, comment, err = issue.toggleAssignee(ctx, doer, assigneeID, false)
103+
removed, comment, err = toggleIssueAssignee(ctx, issue, doer, assigneeID, false)
104104
if err != nil {
105105
return false, nil, err
106106
}
@@ -112,7 +112,7 @@ func (issue *Issue) ToggleAssignee(doer *user_model.User, assigneeID int64) (rem
112112
return removed, comment, nil
113113
}
114114

115-
func (issue *Issue) toggleAssignee(ctx context.Context, doer *user_model.User, assigneeID int64, isCreate bool) (removed bool, comment *Comment, err error) {
115+
func toggleIssueAssignee(ctx context.Context, issue *Issue, doer *user_model.User, assigneeID int64, isCreate bool) (removed bool, comment *Comment, err error) {
116116
sess := db.GetEngine(ctx)
117117
removed, err = toggleUserAssignee(sess, issue, assigneeID)
118118
if err != nil {

‎models/issue_assignees_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,17 @@ func TestUpdateAssignee(t *testing.T) {
2323
// Assign multiple users
2424
user2, err := user_model.GetUserByID(2)
2525
assert.NoError(t, err)
26-
_, _, err = issue.ToggleAssignee(&user_model.User{ID: 1}, user2.ID)
26+
_, _, err = ToggleIssueAssignee(issue, &user_model.User{ID: 1}, user2.ID)
2727
assert.NoError(t, err)
2828

2929
user3, err := user_model.GetUserByID(3)
3030
assert.NoError(t, err)
31-
_, _, err = issue.ToggleAssignee(&user_model.User{ID: 1}, user3.ID)
31+
_, _, err = ToggleIssueAssignee(issue, &user_model.User{ID: 1}, user3.ID)
3232
assert.NoError(t, err)
3333

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

3939
// Check if he got removed

‎models/issue_comment.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
858858
}
859859
}
860860
case CommentTypeReopen, CommentTypeClose:
861-
if err = opts.Issue.updateClosedNum(ctx); err != nil {
861+
if err = updateIssueClosedNum(ctx, opts.Issue); err != nil {
862862
return err
863863
}
864864
}

‎models/issue_dependency_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestCreateIssueDependency(t *testing.T) {
4848
assert.False(t, left)
4949

5050
// Close #2 and check again
51-
_, err = issue2.ChangeStatus(user1, true)
51+
_, err = ChangeIssueStatus(issue2, user1, true)
5252
assert.NoError(t, err)
5353

5454
left, err = IssueNoDependenciesLeft(issue1)

‎models/issue_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestIssue_ReplaceLabels(t *testing.T) {
3434
for i, labelID := range labelIDs {
3535
labels[i] = unittest.AssertExistsAndLoadBean(t, &Label{ID: labelID, RepoID: repo.ID}).(*Label)
3636
}
37-
assert.NoError(t, issue.ReplaceLabels(labels, doer))
37+
assert.NoError(t, ReplaceIssueLabels(issue, labels, doer))
3838
unittest.AssertCount(t, &IssueLabel{IssueID: issueID}, len(labelIDs))
3939
for _, labelID := range labelIDs {
4040
unittest.AssertExistsAndLoadBean(t, &IssueLabel{IssueID: issueID, LabelID: labelID})
@@ -116,7 +116,7 @@ func TestIssue_ClearLabels(t *testing.T) {
116116
assert.NoError(t, unittest.PrepareTestDatabase())
117117
issue := unittest.AssertExistsAndLoadBean(t, &Issue{ID: test.issueID}).(*Issue)
118118
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: test.doerID}).(*user_model.User)
119-
assert.NoError(t, issue.ClearLabels(doer))
119+
assert.NoError(t, ClearIssueLabels(issue, doer))
120120
unittest.AssertNotExistsBean(t, &IssueLabel{IssueID: test.issueID})
121121
}
122122
}
@@ -459,7 +459,7 @@ func TestIssue_ResolveMentions(t *testing.T) {
459459
r := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: o.ID, LowerName: repo}).(*repo_model.Repository)
460460
issue := &Issue{RepoID: r.ID}
461461
d := unittest.AssertExistsAndLoadBean(t, &user_model.User{LowerName: doer}).(*user_model.User)
462-
resolved, err := issue.ResolveMentionsByVisibility(db.DefaultContext, d, mentions)
462+
resolved, err := ResolveIssueMentionsByVisibility(db.DefaultContext, issue, d, mentions)
463463
assert.NoError(t, err)
464464
ids := make([]int64, len(resolved))
465465
for i, user := range resolved {

‎models/issue_xref_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func TestXRef_NeuterCrossReferences(t *testing.T) {
8383

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

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

104104
pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))

‎models/pull.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ func (pr *PullRequest) SetMerged() (bool, error) {
420420
return false, err
421421
}
422422

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

‎routers/api/v1/repo/pull.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ func EditPullRequest(ctx *context.APIContext) {
560560
labels = append(labels, orgLabels...)
561561
}
562562

563-
if err = issue.ReplaceLabels(labels, ctx.Doer); err != nil {
563+
if err = models.ReplaceIssueLabels(issue, labels, ctx.Doer); err != nil {
564564
ctx.Error(http.StatusInternalServerError, "ReplaceLabelsError", err)
565565
return
566566
}

‎routers/web/repo/issue.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2595,7 +2595,7 @@ func updateAttachments(item interface{}, files []string) error {
25952595
if len(files) > 0 {
25962596
switch content := item.(type) {
25972597
case *models.Issue:
2598-
err = content.UpdateAttachments(files)
2598+
err = models.UpdateIssueAttachments(content.ID, files)
25992599
case *models.Comment:
26002600
err = content.UpdateAttachments(files)
26012601
default:

‎routers/web/repo/issue_label.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ func UpdateIssueLabel(ctx *context.Context) {
191191
// detach if any issues already have label, otherwise attach
192192
action = "attach"
193193
for _, issue := range issues {
194-
if issue.HasLabel(label.ID) {
194+
if models.HasIssueLabel(issue.ID, label.ID) {
195195
action = "detach"
196196
break
197197
}

0 commit comments

Comments
 (0)