From 10644d6dd7574b031118bf01b2bd737017230ffd Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Tue, 31 Jan 2017 20:31:35 -0500 Subject: [PATCH] Bug fixes and unit tests for models/issue_label (#802) --- models/fixtures/comment.yml | 7 + models/fixtures/issue.yml | 10 ++ models/fixtures/issue_label.yml | 14 ++ models/fixtures/label.yml | 15 ++ models/issue.go | 4 +- models/issue_label.go | 19 +-- models/issue_label_test.go | 244 +++++++++++++++++++++++++++++ routers/api/v1/repo/issue_label.go | 2 +- 8 files changed, 301 insertions(+), 14 deletions(-) create mode 100644 models/fixtures/comment.yml create mode 100644 models/fixtures/issue_label.yml create mode 100644 models/fixtures/label.yml create mode 100644 models/issue_label_test.go diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml new file mode 100644 index 000000000..e38952ac5 --- /dev/null +++ b/models/fixtures/comment.yml @@ -0,0 +1,7 @@ +- + id: 1 + type: 7 # label + poster_id: 2 + issue_id: 1 + label_id: 1 + content: "1" diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index dbd435939..8bcadb0aa 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -44,3 +44,13 @@ content: content4 is_closed: true is_pull: false + +- + id: 5 + repo_id: 1 + index: 4 + poster_id: 2 + name: issue5 + content: content5 + is_closed: true + is_pull: false diff --git a/models/fixtures/issue_label.yml b/models/fixtures/issue_label.yml new file mode 100644 index 000000000..49d5a95d0 --- /dev/null +++ b/models/fixtures/issue_label.yml @@ -0,0 +1,14 @@ +- + id: 1 + issue_id: 1 + label_id: 1 + +- + id: 2 + issue_id: 5 + label_id: 2 + +- + id: 3 + issue_id: 2 + label_id: 1 diff --git a/models/fixtures/label.yml b/models/fixtures/label.yml new file mode 100644 index 000000000..5336342b1 --- /dev/null +++ b/models/fixtures/label.yml @@ -0,0 +1,15 @@ +- + id: 1 + repo_id: 1 + name: label1 + color: '#abcdef' + num_issues: 2 + num_closed_issues: 0 + +- + id: 2 + repo_id: 1 + name: label2 + color: '#000000' + num_issues: 1 + num_closed_issues: 1 diff --git a/models/issue.go b/models/issue.go index 0102656f0..1727da1d5 100644 --- a/models/issue.go +++ b/models/issue.go @@ -345,7 +345,7 @@ func (issue *Issue) getLabels(e Engine) (err error) { } func (issue *Issue) removeLabel(e *xorm.Session, doer *User, label *Label) error { - return deleteIssueLabel(e, doer, issue, label) + return deleteIssueLabel(e, issue, label, doer) } // RemoveLabel removes a label from issue by given ID. @@ -360,7 +360,7 @@ func (issue *Issue) RemoveLabel(doer *User, label *Label) error { return ErrLabelNotExist{} } - if err := DeleteIssueLabel(issue, doer, label); err != nil { + if err := DeleteIssueLabel(issue, label, doer); err != nil { return err } diff --git a/models/issue_label.go b/models/issue_label.go index 02397f146..970b3fcc4 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -114,7 +114,7 @@ func getLabelInRepoByName(e Engine, repoID int64, labelName string) (*Label, err Name: labelName, RepoID: repoID, } - has, err := x.Get(l) + has, err := e.Get(l) if err != nil { return nil, err } else if !has { @@ -135,7 +135,7 @@ func getLabelInRepoByID(e Engine, repoID, labelID int64) (*Label, error) { ID: labelID, RepoID: repoID, } - has, err := x.Get(l) + has, err := e.Get(l) if err != nil { return nil, err } else if !has { @@ -355,17 +355,14 @@ func getIssueLabels(e Engine, issueID int64) ([]*IssueLabel, error) { Find(&issueLabels) } -// GetIssueLabels returns all issue-label relations of given issue by ID. -func GetIssueLabels(issueID int64) ([]*IssueLabel, error) { - return getIssueLabels(x, issueID) -} - -func deleteIssueLabel(e *xorm.Session, doer *User, issue *Issue, label *Label) (err error) { - if _, err = e.Delete(&IssueLabel{ +func deleteIssueLabel(e *xorm.Session, issue *Issue, label *Label, doer *User) (err error) { + if count, err := e.Delete(&IssueLabel{ IssueID: issue.ID, LabelID: label.ID, }); err != nil { return err + } else if count == 0 { + return nil } if err = issue.loadRepo(e); err != nil { @@ -384,14 +381,14 @@ func deleteIssueLabel(e *xorm.Session, doer *User, issue *Issue, label *Label) ( } // DeleteIssueLabel deletes issue-label relation. -func DeleteIssueLabel(issue *Issue, doer *User, label *Label) (err error) { +func DeleteIssueLabel(issue *Issue, label *Label, doer *User) (err error) { sess := x.NewSession() defer sessionRelease(sess) if err = sess.Begin(); err != nil { return err } - if err = deleteIssueLabel(sess, doer, issue, label); err != nil { + if err = deleteIssueLabel(sess, issue, label, doer); err != nil { return err } diff --git a/models/issue_label_test.go b/models/issue_label_test.go new file mode 100644 index 000000000..a1cdb8c9b --- /dev/null +++ b/models/issue_label_test.go @@ -0,0 +1,244 @@ +// 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 models + +import ( + "html/template" + "testing" + + api "code.gitea.io/sdk/gitea" + + "github.com/stretchr/testify/assert" +) + +// TODO TestGetLabelTemplateFile + +func TestLabel_APIFormat(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label := AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) + assert.Equal(t, api.Label{ + ID: label.ID, + Name: label.Name, + Color: "abcdef", + }, *label.APIFormat()) +} + +func TestLabel_CalOpenIssues(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label := AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) + label.CalOpenIssues() + assert.EqualValues(t, 2, label.NumOpenIssues) +} + +func TestLabel_ForegroundColor(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label := AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) + assert.Equal(t, template.CSS("#000"), label.ForegroundColor()) + + label = AssertExistsAndLoadBean(t, &Label{ID: 2}).(*Label) + assert.Equal(t, template.CSS("#fff"), label.ForegroundColor()) +} + +func TestNewLabels(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + labels := []*Label{ + {RepoID: 2, Name: "labelName2", Color: "#123456"}, + {RepoID: 3, Name: "labelName3", Color: "#234567"}, + } + for _, label := range labels { + AssertNotExistsBean(t, label) + } + assert.NoError(t, NewLabels(labels...)) + for _, label := range labels { + AssertExistsAndLoadBean(t, label) + } +} + +func TestGetLabelByID(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label, err := GetLabelByID(1) + assert.NoError(t, err) + assert.EqualValues(t, 1, label.ID) + + _, err = GetLabelByID(NonexistentID) + assert.True(t, IsErrLabelNotExist(err)) +} + +func TestGetLabelInRepoByName(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label, err := GetLabelInRepoByName(1, "label1") + assert.NoError(t, err) + assert.EqualValues(t, 1, label.ID) + assert.Equal(t, "label1", label.Name) + + _, err = GetLabelInRepoByName(1, "") + assert.True(t, IsErrLabelNotExist(err)) + + _, err = GetLabelInRepoByName(NonexistentID, "nonexistent") + assert.True(t, IsErrLabelNotExist(err)) +} + +func TestGetLabelInRepoByID(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label, err := GetLabelInRepoByID(1, 1) + assert.NoError(t, err) + assert.EqualValues(t, 1, label.ID) + + _, err = GetLabelInRepoByID(1, -1) + assert.True(t, IsErrLabelNotExist(err)) + + _, err = GetLabelInRepoByID(NonexistentID, NonexistentID) + assert.True(t, IsErrLabelNotExist(err)) +} + +func TestGetLabelsInRepoByIDs(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + labels, err := GetLabelsInRepoByIDs(1, []int64{1, 2, NonexistentID}) + assert.NoError(t, err) + assert.Len(t, labels, 2) + assert.EqualValues(t, 1, labels[0].ID) + assert.EqualValues(t, 2, labels[1].ID) +} + +func TestGetLabelsByRepoID(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + testSuccess := func(repoID int64, sortType string, expectedIssueIDs []int64) { + labels, err := GetLabelsByRepoID(repoID, sortType) + assert.NoError(t, err) + assert.Len(t, labels, len(expectedIssueIDs)) + for i, label := range labels { + assert.EqualValues(t, expectedIssueIDs[i], label.ID) + } + } + testSuccess(1, "leastissues", []int64{2, 1}) + testSuccess(1, "mostissues", []int64{1, 2}) + testSuccess(1, "reversealphabetically", []int64{2, 1}) + testSuccess(1, "default", []int64{1, 2}) +} + +func TestGetLabelsByIssueID(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + labels, err := GetLabelsByIssueID(1) + assert.NoError(t, err) + assert.Len(t, labels, 1) + assert.EqualValues(t, 1, labels[0].ID) + + labels, err = GetLabelsByIssueID(NonexistentID) + assert.NoError(t, err) + assert.Len(t, labels, 0) +} + +func TestUpdateLabel(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label := AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) + label.Color = "#ffff00" + label.Name = "newLabelName" + assert.NoError(t, UpdateLabel(label)) + newLabel := AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) + assert.Equal(t, *label, *newLabel) +} + +func TestDeleteLabel(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label := AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) + assert.NoError(t, DeleteLabel(label.RepoID, label.ID)) + AssertNotExistsBean(t, &Label{ID: label.ID, RepoID: label.RepoID}) + + assert.NoError(t, DeleteLabel(label.RepoID, label.ID)) + AssertNotExistsBean(t, &Label{ID: label.ID, RepoID: label.RepoID}) + + assert.NoError(t, DeleteLabel(NonexistentID, NonexistentID)) +} + +func TestHasIssueLabel(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + assert.True(t, HasIssueLabel(1, 1)) + assert.False(t, HasIssueLabel(1, 2)) + assert.False(t, HasIssueLabel(NonexistentID, NonexistentID)) +} + +func TestNewIssueLabel(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label := AssertExistsAndLoadBean(t, &Label{ID: 2}).(*Label) + issue := AssertExistsAndLoadBean(t, &Issue{ID: 1}).(*Issue) + doer := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + + // add new IssueLabel + prevNumIssues := label.NumIssues + assert.NoError(t, NewIssueLabel(issue, label, doer)) + AssertExistsAndLoadBean(t, &IssueLabel{IssueID: issue.ID, LabelID: label.ID}) + AssertExistsAndLoadBean(t, &Comment{ + Type: CommentTypeLabel, + PosterID: doer.ID, + IssueID: issue.ID, + LabelID: label.ID, + Content: "1", + }) + assert.EqualValues(t, prevNumIssues+1, label.NumIssues) + + // re-add existing IssueLabel + assert.NoError(t, NewIssueLabel(issue, label, doer)) +} + +func TestNewIssueLabels(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label1 := AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) + label2 := AssertExistsAndLoadBean(t, &Label{ID: 2}).(*Label) + issue := AssertExistsAndLoadBean(t, &Issue{ID: 5}).(*Issue) + doer := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + + assert.NoError(t, NewIssueLabels(issue, []*Label{label1, label2}, doer)) + AssertExistsAndLoadBean(t, &IssueLabel{IssueID: issue.ID, LabelID: label1.ID}) + AssertExistsAndLoadBean(t, &Comment{ + Type: CommentTypeLabel, + PosterID: doer.ID, + IssueID: issue.ID, + LabelID: label1.ID, + Content: "1", + }) + AssertExistsAndLoadBean(t, &IssueLabel{IssueID: issue.ID, LabelID: label1.ID}) + label1 = AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) + assert.EqualValues(t, 3, label1.NumIssues) + assert.EqualValues(t, 1, label1.NumClosedIssues) + label2 = AssertExistsAndLoadBean(t, &Label{ID: 2}).(*Label) + assert.EqualValues(t, 1, label2.NumIssues) + assert.EqualValues(t, 1, label2.NumClosedIssues) + + // corner case: test empty slice + assert.NoError(t, NewIssueLabels(issue, []*Label{}, doer)) +} + +func TestDeleteIssueLabel(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + testSuccess := func(labelID, issueID, doerID int64) { + label := AssertExistsAndLoadBean(t, &Label{ID: labelID}).(*Label) + issue := AssertExistsAndLoadBean(t, &Issue{ID: issueID}).(*Issue) + doer := AssertExistsAndLoadBean(t, &User{ID: doerID}).(*User) + + expectedNumIssues := label.NumIssues + expectedNumClosedIssues := label.NumClosedIssues + if BeanExists(t, &IssueLabel{IssueID: issueID, LabelID: labelID}) { + expectedNumIssues-- + if issue.IsClosed { + expectedNumClosedIssues-- + } + } + + assert.NoError(t, DeleteIssueLabel(issue, label, doer)) + AssertNotExistsBean(t, &IssueLabel{IssueID: issueID, LabelID: labelID}) + AssertExistsAndLoadBean(t, &Comment{ + Type: CommentTypeLabel, + PosterID: doerID, + IssueID: issueID, + LabelID: labelID, + }, `content=""`) + label = AssertExistsAndLoadBean(t, &Label{ID: labelID}).(*Label) + assert.EqualValues(t, expectedNumIssues, label.NumIssues) + assert.EqualValues(t, expectedNumClosedIssues, label.NumClosedIssues) + } + testSuccess(1, 1, 2) + testSuccess(2, 5, 2) + testSuccess(1, 1, 2) // delete non-existent IssueLabel +} diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index 20de41b75..d94d66671 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -98,7 +98,7 @@ func DeleteIssueLabel(ctx *context.APIContext) { return } - if err := models.DeleteIssueLabel(issue, ctx.User, label); err != nil { + if err := models.DeleteIssueLabel(issue, label, ctx.User); err != nil { ctx.Error(500, "DeleteIssueLabel", err) return }