Bug fixes and unit tests for models/issue_label (#802)

Author: ethantkoenig

models/fixtures/comment.yml

@@ -0,0 +1,7 @@
+-
+  id: 1
+  type: 7 # label
+  poster_id: 2
+  issue_id: 1
+  label_id: 1
+  content: "1"

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

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

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

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
 	}
 

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
 	}
 

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
+}

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
 	}