From 280f4bebbf5883eef94d8a84b21575393ae5e0e1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 19 Oct 2019 17:48:29 +0800 Subject: [PATCH] Move issue label operations to issue service package (#8553) * Move issue label operations to issue service package * fix test * fix fmt --- models/issue.go | 88 ----------------------------- models/issue_test.go | 47 ---------------- routers/api/v1/repo/issue_label.go | 2 +- routers/repo/issue_label.go | 4 +- services/issue/label.go | 90 ++++++++++++++++++++++++++++++ services/issue/label_test.go | 59 ++++++++++++++++++++ services/issue/main_test.go | 16 ++++++ 7 files changed, 168 insertions(+), 138 deletions(-) create mode 100644 services/issue/label_test.go create mode 100644 services/issue/main_test.go diff --git a/models/issue.go b/models/issue.go index 525152552..688a412d8 100644 --- a/models/issue.go +++ b/models/issue.go @@ -428,52 +428,6 @@ func (issue *Issue) HasLabel(labelID int64) bool { return issue.hasLabel(x, labelID) } -func (issue *Issue) sendLabelUpdatedWebhook(doer *User) { - var err error - - if err = issue.loadRepo(x); err != nil { - log.Error("loadRepo: %v", err) - return - } - - if err = issue.loadPoster(x); err != nil { - log.Error("loadPoster: %v", err) - return - } - - mode, _ := AccessLevel(issue.Poster, issue.Repo) - if issue.IsPull { - if err = issue.loadPullRequest(x); err != nil { - log.Error("loadPullRequest: %v", err) - return - } - if err = issue.PullRequest.LoadIssue(); err != nil { - log.Error("LoadIssue: %v", err) - return - } - err = PrepareWebhooks(issue.Repo, HookEventPullRequest, &api.PullRequestPayload{ - Action: api.HookIssueLabelUpdated, - Index: issue.Index, - PullRequest: issue.PullRequest.APIFormat(), - Repository: issue.Repo.APIFormat(AccessModeNone), - Sender: doer.APIFormat(), - }) - } else { - err = PrepareWebhooks(issue.Repo, HookEventIssues, &api.IssuePayload{ - Action: api.HookIssueLabelUpdated, - Index: issue.Index, - Issue: issue.APIFormat(), - Repository: issue.Repo.APIFormat(mode), - Sender: doer.APIFormat(), - }) - } - if err != nil { - log.Error("PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err) - } else { - go HookQueue.Add(issue.RepoID) - } -} - // ReplyReference returns tokenized address to use for email reply headers func (issue *Issue) ReplyReference() string { var path string @@ -490,30 +444,10 @@ func (issue *Issue) addLabel(e *xorm.Session, label *Label, doer *User) error { return newIssueLabel(e, issue, label, doer) } -// AddLabel adds a new label to the issue. -func (issue *Issue) AddLabel(doer *User, label *Label) error { - if err := NewIssueLabel(issue, label, doer); err != nil { - return err - } - - issue.sendLabelUpdatedWebhook(doer) - return nil -} - func (issue *Issue) addLabels(e *xorm.Session, labels []*Label, doer *User) error { return newIssueLabels(e, issue, labels, doer) } -// AddLabels adds a list of new labels to the issue. -func (issue *Issue) AddLabels(doer *User, labels []*Label) error { - if err := NewIssueLabels(issue, labels, doer); err != nil { - return err - } - - issue.sendLabelUpdatedWebhook(doer) - return nil -} - func (issue *Issue) getLabels(e Engine) (err error) { if len(issue.Labels) > 0 { return nil @@ -530,28 +464,6 @@ func (issue *Issue) removeLabel(e *xorm.Session, doer *User, label *Label) error return deleteIssueLabel(e, issue, label, doer) } -// RemoveLabel removes a label from issue by given ID. -func (issue *Issue) RemoveLabel(doer *User, label *Label) error { - if err := issue.loadRepo(x); err != nil { - return err - } - - perm, err := GetUserRepoPermission(issue.Repo, doer) - if err != nil { - return err - } - if !perm.CanWriteIssuesOrPulls(issue.IsPull) { - return ErrLabelNotExist{} - } - - if err := DeleteIssueLabel(issue, label, doer); err != nil { - return err - } - - issue.sendLabelUpdatedWebhook(doer) - return nil -} - func (issue *Issue) clearLabels(e *xorm.Session, doer *User) (err error) { if err = issue.getLabels(e); err != nil { return fmt.Errorf("getLabels: %v", err) diff --git a/models/issue_test.go b/models/issue_test.go index 0be3f6880..7614c2f55 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -84,53 +84,6 @@ func TestGetParticipantsByIssueID(t *testing.T) { checkParticipants(1, []int{5}) } -func TestIssue_AddLabel(t *testing.T) { - var tests = []struct { - issueID int64 - labelID int64 - doerID int64 - }{ - {1, 2, 2}, // non-pull-request, not-already-added label - {1, 1, 2}, // non-pull-request, already-added label - {2, 2, 2}, // pull-request, not-already-added label - {2, 1, 2}, // pull-request, already-added label - } - for _, test := range tests { - assert.NoError(t, PrepareTestDatabase()) - issue := AssertExistsAndLoadBean(t, &Issue{ID: test.issueID}).(*Issue) - label := AssertExistsAndLoadBean(t, &Label{ID: test.labelID}).(*Label) - doer := AssertExistsAndLoadBean(t, &User{ID: test.doerID}).(*User) - assert.NoError(t, issue.AddLabel(doer, label)) - AssertExistsAndLoadBean(t, &IssueLabel{IssueID: test.issueID, LabelID: test.labelID}) - } -} - -func TestIssue_AddLabels(t *testing.T) { - var tests = []struct { - issueID int64 - labelIDs []int64 - doerID int64 - }{ - {1, []int64{1, 2}, 2}, // non-pull-request - {1, []int64{}, 2}, // non-pull-request, empty - {2, []int64{1, 2}, 2}, // pull-request - {2, []int64{}, 1}, // pull-request, empty - } - for _, test := range tests { - assert.NoError(t, PrepareTestDatabase()) - issue := AssertExistsAndLoadBean(t, &Issue{ID: test.issueID}).(*Issue) - labels := make([]*Label, len(test.labelIDs)) - for i, labelID := range test.labelIDs { - labels[i] = AssertExistsAndLoadBean(t, &Label{ID: labelID}).(*Label) - } - doer := AssertExistsAndLoadBean(t, &User{ID: test.doerID}).(*User) - assert.NoError(t, issue.AddLabels(doer, labels)) - for _, labelID := range test.labelIDs { - AssertExistsAndLoadBean(t, &IssueLabel{IssueID: test.issueID, LabelID: labelID}) - } - } -} - func TestIssue_ClearLabels(t *testing.T) { var tests = []struct { issueID int64 diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index 7bd76c24c..e41c1512e 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -117,7 +117,7 @@ func AddIssueLabels(ctx *context.APIContext, form api.IssueLabelsOption) { return } - if err = issue.AddLabels(ctx.User, labels); err != nil { + if err = issue_service.AddLabels(issue, ctx.User, labels); err != nil { ctx.Error(500, "AddLabels", err) return } diff --git a/routers/repo/issue_label.go b/routers/repo/issue_label.go index 53c37e2e9..02568f77a 100644 --- a/routers/repo/issue_label.go +++ b/routers/repo/issue_label.go @@ -162,14 +162,14 @@ func UpdateIssueLabel(ctx *context.Context) { if action == "attach" { for _, issue := range issues { - if err = issue.AddLabel(ctx.User, label); err != nil { + if err = issue_service.AddLabel(issue, ctx.User, label); err != nil { ctx.ServerError("AddLabel", err) return } } } else { for _, issue := range issues { - if err = issue.RemoveLabel(ctx.User, label); err != nil { + if err = issue_service.RemoveLabel(issue, ctx.User, label); err != nil { ctx.ServerError("RemoveLabel", err) return } diff --git a/services/issue/label.go b/services/issue/label.go index 4af8c2b97..b393e5d38 100644 --- a/services/issue/label.go +++ b/services/issue/label.go @@ -6,9 +6,57 @@ package issue import ( "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" + api "code.gitea.io/gitea/modules/structs" ) +func sendLabelUpdatedWebhook(issue *models.Issue, doer *models.User) { + var err error + + if err = issue.LoadRepo(); err != nil { + log.Error("LoadRepo: %v", err) + return + } + + if err = issue.LoadPoster(); err != nil { + log.Error("LoadPoster: %v", err) + return + } + + mode, _ := models.AccessLevel(issue.Poster, issue.Repo) + if issue.IsPull { + if err = issue.LoadPullRequest(); err != nil { + log.Error("loadPullRequest: %v", err) + return + } + if err = issue.PullRequest.LoadIssue(); err != nil { + log.Error("LoadIssue: %v", err) + return + } + err = models.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{ + Action: api.HookIssueLabelUpdated, + Index: issue.Index, + PullRequest: issue.PullRequest.APIFormat(), + Repository: issue.Repo.APIFormat(models.AccessModeNone), + Sender: doer.APIFormat(), + }) + } else { + err = models.PrepareWebhooks(issue.Repo, models.HookEventIssues, &api.IssuePayload{ + Action: api.HookIssueLabelUpdated, + Index: issue.Index, + Issue: issue.APIFormat(), + Repository: issue.Repo.APIFormat(mode), + Sender: doer.APIFormat(), + }) + } + if err != nil { + log.Error("PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err) + } else { + go models.HookQueue.Add(issue.RepoID) + } +} + // ClearLabels clears all of an issue's labels func ClearLabels(issue *models.Issue, doer *models.User) (err error) { if err = issue.ClearLabels(doer); err != nil { @@ -19,3 +67,45 @@ func ClearLabels(issue *models.Issue, doer *models.User) (err error) { return nil } + +// AddLabel adds a new label to the issue. +func AddLabel(issue *models.Issue, doer *models.User, label *models.Label) error { + if err := models.NewIssueLabel(issue, label, doer); err != nil { + return err + } + + sendLabelUpdatedWebhook(issue, doer) + return nil +} + +// AddLabels adds a list of new labels to the issue. +func AddLabels(issue *models.Issue, doer *models.User, labels []*models.Label) error { + if err := models.NewIssueLabels(issue, labels, doer); err != nil { + return err + } + + sendLabelUpdatedWebhook(issue, doer) + return nil +} + +// RemoveLabel removes a label from issue by given ID. +func RemoveLabel(issue *models.Issue, doer *models.User, label *models.Label) error { + if err := issue.LoadRepo(); err != nil { + return err + } + + perm, err := models.GetUserRepoPermission(issue.Repo, doer) + if err != nil { + return err + } + if !perm.CanWriteIssuesOrPulls(issue.IsPull) { + return models.ErrLabelNotExist{} + } + + if err := models.DeleteIssueLabel(issue, label, doer); err != nil { + return err + } + + sendLabelUpdatedWebhook(issue, doer) + return nil +} diff --git a/services/issue/label_test.go b/services/issue/label_test.go new file mode 100644 index 000000000..065202894 --- /dev/null +++ b/services/issue/label_test.go @@ -0,0 +1,59 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package issue + +import ( + "testing" + + "code.gitea.io/gitea/models" + "github.com/stretchr/testify/assert" +) + +func TestIssue_AddLabels(t *testing.T) { + var tests = []struct { + issueID int64 + labelIDs []int64 + doerID int64 + }{ + {1, []int64{1, 2}, 2}, // non-pull-request + {1, []int64{}, 2}, // non-pull-request, empty + {2, []int64{1, 2}, 2}, // pull-request + {2, []int64{}, 1}, // pull-request, empty + } + for _, test := range tests { + assert.NoError(t, models.PrepareTestDatabase()) + issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: test.issueID}).(*models.Issue) + labels := make([]*models.Label, len(test.labelIDs)) + for i, labelID := range test.labelIDs { + labels[i] = models.AssertExistsAndLoadBean(t, &models.Label{ID: labelID}).(*models.Label) + } + doer := models.AssertExistsAndLoadBean(t, &models.User{ID: test.doerID}).(*models.User) + assert.NoError(t, AddLabels(issue, doer, labels)) + for _, labelID := range test.labelIDs { + models.AssertExistsAndLoadBean(t, &models.IssueLabel{IssueID: test.issueID, LabelID: labelID}) + } + } +} + +func TestIssue_AddLabel(t *testing.T) { + var tests = []struct { + issueID int64 + labelID int64 + doerID int64 + }{ + {1, 2, 2}, // non-pull-request, not-already-added label + {1, 1, 2}, // non-pull-request, already-added label + {2, 2, 2}, // pull-request, not-already-added label + {2, 1, 2}, // pull-request, already-added label + } + for _, test := range tests { + assert.NoError(t, models.PrepareTestDatabase()) + issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: test.issueID}).(*models.Issue) + label := models.AssertExistsAndLoadBean(t, &models.Label{ID: test.labelID}).(*models.Label) + doer := models.AssertExistsAndLoadBean(t, &models.User{ID: test.doerID}).(*models.User) + assert.NoError(t, AddLabel(issue, doer, label)) + models.AssertExistsAndLoadBean(t, &models.IssueLabel{IssueID: test.issueID, LabelID: test.labelID}) + } +} diff --git a/services/issue/main_test.go b/services/issue/main_test.go new file mode 100644 index 000000000..b056678a4 --- /dev/null +++ b/services/issue/main_test.go @@ -0,0 +1,16 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package issue + +import ( + "path/filepath" + "testing" + + "code.gitea.io/gitea/models" +) + +func TestMain(m *testing.M) { + models.MainTest(m, filepath.Join("..", "..")) +}