From 6548c8080898d27d1267b296dfea408dece24f32 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 24 Mar 2020 18:44:26 -0400 Subject: [PATCH 01/41] Add organization wide labels Implement organization wide labels similar to organization wide webhooks. This lets you create individual labels for organizations that can be used for all repos under that organization (so being able to reuse the same label across multiple repos). This makes it possible for small organizations with many repos to use labels effectively. Fixes #7406 --- integrations/api_issue_label_test.go | 71 ++++++ models/error.go | 28 ++- models/fixtures/issue_label.yml | 5 + models/fixtures/label.yml | 19 ++ models/issue.go | 4 +- models/issue_label.go | 195 +++++++++++++- models/issue_label_test.go | 97 ++++++- modules/repository/create.go | 2 +- modules/test/context_tests.go | 4 +- options/locale/locale_en-US.ini | 5 + routers/api/v1/api.go | 7 + routers/api/v1/org/label.go | 238 ++++++++++++++++++ routers/api/v1/repo/issue_label.go | 2 +- routers/api/v1/repo/label.go | 6 +- routers/org/org_labels.go | 113 +++++++++ routers/org/setting.go | 14 ++ routers/repo/issue.go | 31 ++- routers/repo/issue_label.go | 41 ++- routers/routes/routes.go | 8 + services/issue/label.go | 5 +- templates/org/settings/labels.tmpl | 29 +++ templates/org/settings/navbar.tmpl | 3 + templates/repo/issue/labels.tmpl | 170 +------------ .../repo/issue/labels/edit_delete_label.tmpl | 58 +++++ templates/repo/issue/labels/label_list.tmpl | 97 +++++++ .../issue/labels/label_load_template.tmpl | 30 +++ templates/repo/issue/labels/label_new.tmpl | 27 ++ templates/repo/issue/new_form.tmpl | 8 + .../repo/issue/view_content/sidebar.tmpl | 10 + templates/swagger/v1_json.tmpl | 183 ++++++++++++++ web_src/js/index.js | 69 ++--- web_src/less/_organization.less | 41 +++ web_src/less/_repository.less | 23 +- 33 files changed, 1404 insertions(+), 239 deletions(-) create mode 100644 routers/api/v1/org/label.go create mode 100644 routers/org/org_labels.go create mode 100644 templates/org/settings/labels.tmpl create mode 100644 templates/repo/issue/labels/edit_delete_label.tmpl create mode 100644 templates/repo/issue/labels/label_list.tmpl create mode 100644 templates/repo/issue/labels/label_load_template.tmpl create mode 100644 templates/repo/issue/labels/label_new.tmpl diff --git a/integrations/api_issue_label_test.go b/integrations/api_issue_label_test.go index 6cdb3a0dad6db..b9e5066ad6778 100644 --- a/integrations/api_issue_label_test.go +++ b/integrations/api_issue_label_test.go @@ -134,3 +134,74 @@ func TestAPIReplaceIssueLabels(t *testing.T) { models.AssertCount(t, &models.IssueLabel{IssueID: issue.ID}, 1) models.AssertExistsAndLoadBean(t, &models.IssueLabel{IssueID: issue.ID, LabelID: label.ID}) } + +func TestAPIModifyOrgLabels(t *testing.T) { + assert.NoError(t, models.LoadFixtures()) + + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 3}).(*models.Repository) + owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + user := "user1" + session := loginUser(t, user) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/orgs/%s/labels?token=%s", owner.Name, token) + + // CreateLabel + req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateLabelOption{ + Name: "TestL 1", + Color: "abcdef", + Description: "test label", + }) + resp := session.MakeRequest(t, req, http.StatusCreated) + apiLabel := new(api.Label) + DecodeJSON(t, resp, &apiLabel) + dbLabel := models.AssertExistsAndLoadBean(t, &models.Label{ID: apiLabel.ID, OrgID: owner.ID}).(*models.Label) + assert.EqualValues(t, dbLabel.Name, apiLabel.Name) + assert.EqualValues(t, strings.TrimLeft(dbLabel.Color, "#"), apiLabel.Color) + + req = NewRequestWithJSON(t, "POST", urlStr, &api.CreateLabelOption{ + Name: "TestL 2", + Color: "#123456", + Description: "jet another test label", + }) + session.MakeRequest(t, req, http.StatusCreated) + req = NewRequestWithJSON(t, "POST", urlStr, &api.CreateLabelOption{ + Name: "WrongTestL", + Color: "#12345g", + }) + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + + //ListLabels + req = NewRequest(t, "GET", urlStr) + resp = session.MakeRequest(t, req, http.StatusOK) + var apiLabels []*api.Label + DecodeJSON(t, resp, &apiLabels) + assert.Len(t, apiLabels, 2) + + //GetLabel + singleURLStr := fmt.Sprintf("/api/v1/orgs/%s/labels/%d?token=%s", owner.Name, dbLabel.ID, token) + req = NewRequest(t, "GET", singleURLStr) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiLabel) + assert.EqualValues(t, strings.TrimLeft(dbLabel.Color, "#"), apiLabel.Color) + + //EditLabel + newName := "LabelNewName" + newColor := "09876a" + newColorWrong := "09g76a" + req = NewRequestWithJSON(t, "PATCH", singleURLStr, &api.EditLabelOption{ + Name: &newName, + Color: &newColor, + }) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiLabel) + assert.EqualValues(t, newColor, apiLabel.Color) + req = NewRequestWithJSON(t, "PATCH", singleURLStr, &api.EditLabelOption{ + Color: &newColorWrong, + }) + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + + //DeleteLabel + req = NewRequest(t, "DELETE", singleURLStr) + resp = session.MakeRequest(t, req, http.StatusNoContent) + +} diff --git a/models/error.go b/models/error.go index 09fb4ebcaf0f8..9b1bed47a7633 100644 --- a/models/error.go +++ b/models/error.go @@ -1549,22 +1549,38 @@ func (err ErrTrackedTimeNotExist) Error() string { // |_______ (____ /___ /\___ >____/ // \/ \/ \/ \/ -// ErrLabelNotExist represents a "LabelNotExist" kind of error. -type ErrLabelNotExist struct { +// ErrRepoLabelNotExist represents a "LabelNotExist" kind of error. +type ErrRepoLabelNotExist struct { LabelID int64 RepoID int64 } -// IsErrLabelNotExist checks if an error is a ErrLabelNotExist. -func IsErrLabelNotExist(err error) bool { - _, ok := err.(ErrLabelNotExist) +// IsErrRepoLabelNotExist checks if an error is a ErrLabelNotExist. +func IsErrRepoLabelNotExist(err error) bool { + _, ok := err.(ErrRepoLabelNotExist) return ok } -func (err ErrLabelNotExist) Error() string { +func (err ErrRepoLabelNotExist) Error() string { return fmt.Sprintf("label does not exist [label_id: %d, repo_id: %d]", err.LabelID, err.RepoID) } +// ErrOrgLabelNotExist represents a "LabelNotExist" kind of error. +type ErrOrgLabelNotExist struct { + LabelID int64 + OrgID int64 +} + +// IsErrOrgLabelNotExist checks if an error is a ErrLabelNotExist. +func IsErrOrgLabelNotExist(err error) bool { + _, ok := err.(ErrOrgLabelNotExist) + return ok +} + +func (err ErrOrgLabelNotExist) Error() string { + return fmt.Sprintf("label does not exist [label_id: %d, org_id: %d]", err.LabelID, err.OrgID) +} + // _____ .__.__ __ // / \ |__| | ____ _______/ |_ ____ ____ ____ // / \ / \| | | _/ __ \ / ___/\ __\/ _ \ / \_/ __ \ diff --git a/models/fixtures/issue_label.yml b/models/fixtures/issue_label.yml index 49d5a95d020f3..f4ecb1f923232 100644 --- a/models/fixtures/issue_label.yml +++ b/models/fixtures/issue_label.yml @@ -12,3 +12,8 @@ id: 3 issue_id: 2 label_id: 1 + +- + id: 4 + issue_id: 2 + label_id: 4 diff --git a/models/fixtures/label.yml b/models/fixtures/label.yml index 5336342b1cf89..b35203baff0f3 100644 --- a/models/fixtures/label.yml +++ b/models/fixtures/label.yml @@ -1,6 +1,7 @@ - id: 1 repo_id: 1 + org_id: 0 name: label1 color: '#abcdef' num_issues: 2 @@ -9,7 +10,25 @@ - id: 2 repo_id: 1 + org_id: 0 name: label2 color: '#000000' num_issues: 1 num_closed_issues: 1 +- + id: 3 + repo_id: 0 + org_id: 1 + name: orglabel3 + color: '#abcdef' + num_issues: 0 + num_closed_issues: 0 + +- + id: 4 + repo_id: 0 + org_id: 1 + name: orglabel4 + color: '#000000' + num_issues: 1 + num_closed_issues: 0 \ No newline at end of file diff --git a/models/issue.go b/models/issue.go index 03af32700dfe3..d309c11853707 100644 --- a/models/issue.go +++ b/models/issue.go @@ -459,7 +459,7 @@ func (issue *Issue) ClearLabels(doer *User) (err error) { return err } if !perm.CanWriteIssuesOrPulls(issue.IsPull) { - return ErrLabelNotExist{} + return ErrRepoLabelNotExist{} } if err = issue.clearLabels(sess, doer); err != nil { @@ -894,7 +894,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { for _, label := range labels { // Silently drop invalid labels. - if label.RepoID != opts.Repo.ID { + if label.RepoID != opts.Repo.ID && label.OrgID != opts.Repo.Owner.ID { continue } diff --git a/models/issue_label.go b/models/issue_label.go index c111afb2ff0fa..38a17b917f890 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -23,6 +23,7 @@ var LabelColorPattern = regexp.MustCompile("^#[0-9a-fA-F]{6}$") type Label struct { ID int64 `xorm:"pk autoincr"` RepoID int64 `xorm:"INDEX"` + OrgID int64 `xorm:"INDEX"` Name string Description string Color string `xorm:"VARCHAR(7)"` @@ -145,7 +146,7 @@ func LoadLabelsFormatted(labelTemplate string) (string, error) { return strings.Join(labels, ", "), err } -func initializeLabels(e Engine, repoID int64, labelTemplate string) error { +func initializeLabels(e Engine, id int64, labelTemplate string, isOrg bool) error { list, err := GetLabelTemplateFile(labelTemplate) if err != nil { return ErrIssueLabelTemplateLoad{labelTemplate, err} @@ -154,11 +155,15 @@ func initializeLabels(e Engine, repoID int64, labelTemplate string) error { labels := make([]*Label, len(list)) for i := 0; i < len(list); i++ { labels[i] = &Label{ - RepoID: repoID, Name: list[i][0], Description: list[i][2], Color: list[i][1], } + if isOrg { + labels[i].OrgID = id + } else { + labels[i].RepoID = id + } } for _, label := range labels { if err = newLabel(e, label); err != nil { @@ -169,8 +174,8 @@ func initializeLabels(e Engine, repoID int64, labelTemplate string) error { } // InitializeLabels adds a label set to a repository using a template -func InitializeLabels(ctx DBContext, repoID int64, labelTemplate string) error { - return initializeLabels(ctx.e, repoID, labelTemplate) +func InitializeLabels(ctx DBContext, repoID int64, labelTemplate string, isOrg bool) error { + return initializeLabels(ctx.e, repoID, labelTemplate, isOrg) } func newLabel(e Engine, label *Label) error { @@ -209,7 +214,7 @@ func NewLabels(labels ...*Label) error { // and can return arbitrary label with any valid ID. func getLabelInRepoByName(e Engine, repoID int64, labelName string) (*Label, error) { if len(labelName) == 0 { - return nil, ErrLabelNotExist{0, repoID} + return nil, ErrRepoLabelNotExist{0, repoID} } l := &Label{ @@ -220,7 +225,7 @@ func getLabelInRepoByName(e Engine, repoID int64, labelName string) (*Label, err if err != nil { return nil, err } else if !has { - return nil, ErrLabelNotExist{0, l.RepoID} + return nil, ErrRepoLabelNotExist{0, l.RepoID} } return l, nil } @@ -230,7 +235,7 @@ func getLabelInRepoByName(e Engine, repoID int64, labelName string) (*Label, err // and can return arbitrary label with any valid ID. func getLabelInRepoByID(e Engine, repoID, labelID int64) (*Label, error) { if labelID <= 0 { - return nil, ErrLabelNotExist{labelID, repoID} + return nil, ErrRepoLabelNotExist{labelID, repoID} } l := &Label{ @@ -241,7 +246,7 @@ func getLabelInRepoByID(e Engine, repoID, labelID int64) (*Label, error) { if err != nil { return nil, err } else if !has { - return nil, ErrLabelNotExist{l.ID, l.RepoID} + return nil, ErrRepoLabelNotExist{l.ID, l.RepoID} } return l, nil } @@ -325,6 +330,129 @@ func GetLabelsByRepoID(repoID int64, sortType string, listOptions ListOptions) ( return getLabelsByRepoID(x, repoID, sortType, listOptions) } +// ________ +// \_____ \_______ ____ +// / | \_ __ \/ ___\ +// / | \ | \/ /_/ > +// \_______ /__| \___ / +// \/ /_____/ + +// getLabelInOrgByName returns a label by Name in given organization +// If pass orgID as 0, then ORM will ignore limitation of organization +// and can return arbitrary label with any valid ID. +func getLabelInOrgByName(e Engine, orgID int64, labelName string) (*Label, error) { + if len(labelName) == 0 { + return nil, ErrOrgLabelNotExist{0, orgID} // FIX ME! + } + + l := &Label{ + Name: labelName, + OrgID: orgID, + } + has, err := e.Get(l) + if err != nil { + return nil, err + } else if !has { + return nil, ErrOrgLabelNotExist{0, l.OrgID} + } + return l, nil +} + +// getLabelInOrgByID returns a label by ID in given organization. +// If pass orgID as 0, then ORM will ignore limitation of organization +// and can return arbitrary label with any valid ID. +func getLabelInOrgByID(e Engine, orgID, labelID int64) (*Label, error) { + if labelID <= 0 { + return nil, ErrOrgLabelNotExist{labelID, orgID} + } + + l := &Label{ + ID: labelID, + OrgID: orgID, + } + has, err := e.Get(l) + if err != nil { + return nil, err + } else if !has { + return nil, ErrOrgLabelNotExist{l.ID, l.OrgID} + } + return l, nil +} + +// GetLabelInOrgByName returns a label by name in given organization. +func GetLabelInOrgByName(orgID int64, labelName string) (*Label, error) { + return getLabelInOrgByName(x, orgID, labelName) +} + +// GetLabelIDsInOrgByNames returns a list of labelIDs by names in a given +// organization. +// it silently ignores label names that do not belong to the organization. +func GetLabelIDsInOrgByNames(orgID int64, labelNames []string) ([]int64, error) { + labelIDs := make([]int64, 0, len(labelNames)) + return labelIDs, x.Table("label"). + Where("org_id = ?", orgID). + In("name", labelNames). + Asc("name"). + Cols("id"). + Find(&labelIDs) +} + +// GetLabelIDsInOrgsByNames returns a list of labelIDs by names in one of the given +// organization. +// it silently ignores label names that do not belong to the organization. +func GetLabelIDsInOrgsByNames(orgIDs []int64, labelNames []string) ([]int64, error) { + labelIDs := make([]int64, 0, len(labelNames)) + return labelIDs, x.Table("label"). + In("org_id", orgIDs). + In("name", labelNames). + Asc("name"). + Cols("id"). + Find(&labelIDs) +} + +// GetLabelInOrgByID returns a label by ID in given organization. +func GetLabelInOrgByID(orgID, labelID int64) (*Label, error) { + return getLabelInOrgByID(x, orgID, labelID) +} + +// GetLabelsInOrgByIDs returns a list of labels by IDs in given organization, +// it silently ignores label IDs that do not belong to the organization. +func GetLabelsInOrgByIDs(orgID int64, labelIDs []int64) ([]*Label, error) { + labels := make([]*Label, 0, len(labelIDs)) + return labels, x. + Where("org_id = ?", orgID). + In("id", labelIDs). + Asc("name"). + Find(&labels) +} + +func getLabelsByOrgID(e Engine, orgID int64, sortType string, listOptions ListOptions) ([]*Label, error) { + labels := make([]*Label, 0, 10) + sess := e.Where("org_id = ?", orgID) + + switch sortType { + case "reversealphabetically": + sess.Desc("name") + case "leastissues": + sess.Asc("num_issues") + case "mostissues": + sess.Desc("num_issues") + default: + sess.Asc("name") + } + + if listOptions.Page != 0 { + sess = listOptions.setSessionPagination(sess) + } + + return labels, sess.Find(&labels) +} + +// GetLabelsByOrgID returns all labels that belong to given organization by ID. +func GetLabelsByOrgID(orgID int64, sortType string, listOptions ListOptions) ([]*Label, error) { + return getLabelsByOrgID(x, orgID, sortType, listOptions) +} + func getLabelsByIssueID(e Engine, issueID int64) ([]*Label, error) { var labels []*Label return labels, e.Where("issue_label.issue_id = ?", issueID). @@ -364,11 +492,54 @@ func UpdateLabel(l *Label) error { return updateLabel(x, l) } -// DeleteLabel delete a label of given repository. -func DeleteLabel(repoID, labelID int64) error { - _, err := GetLabelInRepoByID(repoID, labelID) +// DeleteLabel delete a label of given repository or organiization. +func DeleteLabel(id, labelID int64, isOrg bool) error { + var err error + if isOrg { + _, err = GetLabelInOrgByID(id, labelID) + if err != nil { + if IsErrRepoLabelNotExist(err) { + return nil + } + return err + } + } else { + _, err = GetLabelInRepoByID(id, labelID) + if err != nil { + if IsErrRepoLabelNotExist(err) { + return nil + } + return err + } + } + + sess := x.NewSession() + defer sess.Close() + if err = sess.Begin(); err != nil { + return err + } + + if _, err = sess.ID(labelID).Delete(new(Label)); err != nil { + return err + } else if _, err = sess. + Where("label_id = ?", labelID). + Delete(new(IssueLabel)); err != nil { + return err + } + + // Clear label id in comment table + if _, err = sess.Where("label_id = ?", labelID).Cols("label_id").Update(&Comment{}); err != nil { + return err + } + + return sess.Commit() +} + +// DeleteOrgLabel delete a label of given organization. +func DeleteOrgLabel(orgID, labelID int64) error { + _, err := GetLabelInOrgByID(orgID, labelID) if err != nil { - if IsErrLabelNotExist(err) { + if IsErrRepoLabelNotExist(err) { return nil } return err diff --git a/models/issue_label_test.go b/models/issue_label_test.go index 6f51473fcbdda..2fbfeb4c9a58d 100644 --- a/models/issue_label_test.go +++ b/models/issue_label_test.go @@ -55,7 +55,7 @@ func TestGetLabelByID(t *testing.T) { assert.EqualValues(t, 1, label.ID) _, err = GetLabelByID(NonexistentID) - assert.True(t, IsErrLabelNotExist(err)) + assert.True(t, IsErrRepoLabelNotExist(err)) } func TestGetLabelInRepoByName(t *testing.T) { @@ -66,10 +66,10 @@ func TestGetLabelInRepoByName(t *testing.T) { assert.Equal(t, "label1", label.Name) _, err = GetLabelInRepoByName(1, "") - assert.True(t, IsErrLabelNotExist(err)) + assert.True(t, IsErrRepoLabelNotExist(err)) _, err = GetLabelInRepoByName(NonexistentID, "nonexistent") - assert.True(t, IsErrLabelNotExist(err)) + assert.True(t, IsErrRepoLabelNotExist(err)) } func TestGetLabelInRepoByNames(t *testing.T) { @@ -103,10 +103,10 @@ func TestGetLabelInRepoByID(t *testing.T) { assert.EqualValues(t, 1, label.ID) _, err = GetLabelInRepoByID(1, -1) - assert.True(t, IsErrLabelNotExist(err)) + assert.True(t, IsErrRepoLabelNotExist(err)) _, err = GetLabelInRepoByID(NonexistentID, NonexistentID) - assert.True(t, IsErrLabelNotExist(err)) + assert.True(t, IsErrRepoLabelNotExist(err)) } func TestGetLabelsInRepoByIDs(t *testing.T) { @@ -135,6 +135,87 @@ func TestGetLabelsByRepoID(t *testing.T) { testSuccess(1, "default", []int64{1, 2}) } +// Org vrsions + +func TestGetLabelInOrgByName(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label, err := GetLabelInOrgByName(1, "orglabel3") + assert.NoError(t, err) + assert.EqualValues(t, 3, label.ID) + assert.Equal(t, "orglabel3", label.Name) + + _, err = GetLabelInOrgByName(1, "") + assert.True(t, IsErrOrgLabelNotExist(err)) + + _, err = GetLabelInOrgByName(NonexistentID, "nonexistent") + assert.True(t, IsErrOrgLabelNotExist(err)) +} + +func TestGetLabelInOrgByNames(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + labelIDs, err := GetLabelIDsInOrgByNames(1, []string{"orglabel3", "orglabel4"}) + assert.NoError(t, err) + + assert.Len(t, labelIDs, 2) + + assert.Equal(t, int64(3), labelIDs[0]) + assert.Equal(t, int64(4), labelIDs[1]) +} + +func TestGetLabelInOrgByNamesDiscardsNonExistentLabels(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + // orglabel99 doesn't exists.. See labels.yml + labelIDs, err := GetLabelIDsInOrgByNames(1, []string{"orglabel3", "orglabel4", "orglabel99"}) + assert.NoError(t, err) + + assert.Len(t, labelIDs, 2) + + assert.Equal(t, int64(3), labelIDs[0]) + assert.Equal(t, int64(4), labelIDs[1]) + assert.NoError(t, err) +} + +func TestGetLabelInOrgByID(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + label, err := GetLabelInOrgByID(1, 3) + assert.NoError(t, err) + assert.EqualValues(t, 3, label.ID) + + _, err = GetLabelInOrgByID(1, -1) + assert.True(t, IsErrOrgLabelNotExist(err)) + + _, err = GetLabelInOrgByID(NonexistentID, NonexistentID) + assert.True(t, IsErrOrgLabelNotExist(err)) +} + +func TestGetLabelsInOrgByIDs(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + labels, err := GetLabelsInOrgByIDs(1, []int64{3, 4, NonexistentID}) + assert.NoError(t, err) + if assert.Len(t, labels, 2) { + assert.EqualValues(t, 3, labels[0].ID) + assert.EqualValues(t, 4, labels[1].ID) + } +} + +func TestGetLabelsByOrgID(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + testSuccess := func(orgID int64, sortType string, expectedIssueIDs []int64) { + labels, err := GetLabelsByOrgID(orgID, sortType, ListOptions{}) + 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{3, 4}) + testSuccess(1, "mostissues", []int64{3, 4}) + testSuccess(1, "reversealphabetically", []int64{4, 3}) + testSuccess(1, "default", []int64{3, 4}) +} + +// + func TestGetLabelsByIssueID(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) labels, err := GetLabelsByIssueID(1) @@ -162,13 +243,13 @@ func TestUpdateLabel(t *testing.T) { func TestDeleteLabel(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) label := AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) - assert.NoError(t, DeleteLabel(label.RepoID, label.ID)) + assert.NoError(t, DeleteLabel(label.RepoID, label.ID, false)) AssertNotExistsBean(t, &Label{ID: label.ID, RepoID: label.RepoID}) - assert.NoError(t, DeleteLabel(label.RepoID, label.ID)) + assert.NoError(t, DeleteLabel(label.RepoID, label.ID, false)) AssertNotExistsBean(t, &Label{ID: label.ID, RepoID: label.RepoID}) - assert.NoError(t, DeleteLabel(NonexistentID, NonexistentID)) + assert.NoError(t, DeleteLabel(NonexistentID, NonexistentID, false)) CheckConsistencyFor(t, &Label{}, &Repository{}) } diff --git a/modules/repository/create.go b/modules/repository/create.go index 255bf097317ca..5c0aae30da6bb 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -58,7 +58,7 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (_ *m // Initialize Issue Labels if selected if len(opts.IssueLabels) > 0 { - if err = models.InitializeLabels(ctx, repo.ID, opts.IssueLabels); err != nil { + if err = models.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil { return fmt.Errorf("InitializeLabels: %v", err) } } diff --git a/modules/test/context_tests.go b/modules/test/context_tests.go index f9f0ec5d42c93..af47369ee1a7c 100644 --- a/modules/test/context_tests.go +++ b/modules/test/context_tests.go @@ -45,8 +45,10 @@ func MockContext(t *testing.T, path string) *context.Context { func LoadRepo(t *testing.T, ctx *context.Context, repoID int64) { ctx.Repo = &context.Repository{} ctx.Repo.Repository = models.AssertExistsAndLoadBean(t, &models.Repository{ID: repoID}).(*models.Repository) - ctx.Repo.RepoLink = ctx.Repo.Repository.Link() var err error + ctx.Repo.Owner, err = models.GetUserByID(ctx.Repo.Repository.OwnerID) + assert.NoError(t, err) + ctx.Repo.RepoLink = ctx.Repo.Repository.Link() ctx.Repo.Permission, err = models.GetUserRepoPermission(ctx.Repo.Repository, ctx.User) assert.NoError(t, err) } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 483970e032846..2fcf90bb7e096 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -725,6 +725,8 @@ tags = Tags issues = Issues pulls = Pull Requests labels = Labels +org.labels = Organization level labels that can be used with all repositories under this organization +org.labels.manage = manage milestones = Milestones commits = Commits commit = Commit @@ -1692,6 +1694,9 @@ settings.delete_org_title = Delete Organization settings.delete_org_desc = This organization will be deleted permanently. Continue? settings.hooks_desc = Add webhooks which will be triggered for all repositories under this organization. +settings.labels_desc = Add labels which can be used on issues for all repositories under this organization. +settings.labels = Labels + members.membership_visibility = Membership Visibility: members.public = Visible members.public_helper = make hidden diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index eee9440574d5f..e5bb98033b97a 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -861,6 +861,13 @@ func RegisterRoutes(m *macaron.Macaron) { Post(reqOrgOwnership(), bind(api.CreateTeamOption{}), org.CreateTeam) m.Get("/search", org.SearchTeam) }, reqOrgMembership()) + m.Group("/labels", func() { + m.Get("", org.ListLabels) + m.Post("", reqToken(), reqOrgOwnership(), bind(api.CreateLabelOption{}), org.CreateLabel) + m.Combo("/:id").Get(org.GetLabel). + Patch(reqToken(), reqOrgOwnership(), bind(api.EditLabelOption{}), org.EditLabel). + Delete(reqToken(), reqOrgOwnership(), org.DeleteLabel) + }) m.Group("/hooks", func() { m.Combo("").Get(org.ListHooks). Post(bind(api.CreateHookOption{}), org.CreateHook) diff --git a/routers/api/v1/org/label.go b/routers/api/v1/org/label.go new file mode 100644 index 0000000000000..13d3eaf0f0b2f --- /dev/null +++ b/routers/api/v1/org/label.go @@ -0,0 +1,238 @@ +// Copyright 2016 The Gogs Authors. All rights reserved. +// Copyright 2018 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 org + +import ( + "fmt" + "net/http" + "strconv" + "strings" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/convert" + api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/routers/api/v1/utils" +) + +// ListLabels list all the labels of an organization +func ListLabels(ctx *context.APIContext) { + // swagger:operation GET /orgs/{org}/labels organization orgListLabels + // --- + // summary: List an organization's labels + // produces: + // - application/json + // parameters: + // - name: org + // in: path + // description: name of the organization + // type: string + // required: true + // - name: page + // in: query + // description: page number of results to return (1-based) + // type: integer + // - name: limit + // in: query + // description: page size of results, maximum page size is 50 + // type: integer + // responses: + // "200": + // "$ref": "#/responses/LabelList" + + labels, err := models.GetLabelsByOrgID(ctx.Org.Organization.ID, ctx.Query("sort"), utils.GetListOptions(ctx)) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetLabelsByOrgID", err) + return + } + + ctx.JSON(http.StatusOK, convert.ToLabelList(labels)) +} + +// CreateLabel create a label for a repository +func CreateLabel(ctx *context.APIContext, form api.CreateLabelOption) { + // swagger:operation POST /orgs/{org}/labels organization orgCreateLabel + // --- + // summary: Create a label for an organization + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: org + // in: path + // description: name of the organization + // type: string + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/CreateLabelOption" + // responses: + // "201": + // "$ref": "#/responses/Label" + // "422": + // "$ref": "#/responses/validationError" + + form.Color = strings.Trim(form.Color, " ") + if len(form.Color) == 6 { + form.Color = "#" + form.Color + } + if !models.LabelColorPattern.MatchString(form.Color) { + ctx.Error(http.StatusUnprocessableEntity, "ColorPattern", fmt.Errorf("bad color code: %s", form.Color)) + return + } + + label := &models.Label{ + Name: form.Name, + Color: form.Color, + OrgID: ctx.Org.Organization.ID, + Description: form.Description, + } + if err := models.NewLabel(label); err != nil { + ctx.Error(http.StatusInternalServerError, "NewLabel", err) + return + } + ctx.JSON(http.StatusCreated, convert.ToLabel(label)) +} + +// GetLabel get label by organization and label id +func GetLabel(ctx *context.APIContext) { + // swagger:operation GET /orgs/{org}/labels/{id} organization orgGetLabel + // --- + // summary: Get a single label + // produces: + // - application/json + // parameters: + // - name: org + // in: path + // description: name of the organization + // type: string + // required: true + // - name: id + // in: path + // description: id of the label to get + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/Label" + + var ( + label *models.Label + err error + ) + strID := ctx.Params(":id") + if intID, err2 := strconv.ParseInt(strID, 10, 64); err2 != nil { + label, err = models.GetLabelInOrgByName(ctx.Org.Organization.ID, strID) + } else { + label, err = models.GetLabelInOrgByID(ctx.Org.Organization.ID, intID) + } + if err != nil { + if models.IsErrOrgLabelNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(http.StatusInternalServerError, "GetLabelByOrgID", err) + } + return + } + + ctx.JSON(http.StatusOK, convert.ToLabel(label)) +} + +// EditLabel modify a label for an Organization +func EditLabel(ctx *context.APIContext, form api.EditLabelOption) { + // swagger:operation PATCH /orgs/{org}/labels/{id} organization orgEditLabel + // --- + // summary: Update a label + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: org + // in: path + // description: name of the organization + // type: string + // required: true + // - name: id + // in: path + // description: id of the label to edit + // type: integer + // format: int64 + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/EditLabelOption" + // responses: + // "200": + // "$ref": "#/responses/Label" + // "422": + // "$ref": "#/responses/validationError" + + label, err := models.GetLabelInOrgByID(ctx.Org.Organization.ID, ctx.ParamsInt64(":id")) + if err != nil { + if models.IsErrOrgLabelNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(http.StatusInternalServerError, "GetLabelByRepoID", err) + } + return + } + + if form.Name != nil { + label.Name = *form.Name + } + if form.Color != nil { + label.Color = strings.Trim(*form.Color, " ") + if len(label.Color) == 6 { + label.Color = "#" + label.Color + } + if !models.LabelColorPattern.MatchString(label.Color) { + ctx.Error(http.StatusUnprocessableEntity, "ColorPattern", fmt.Errorf("bad color code: %s", label.Color)) + return + } + } + if form.Description != nil { + label.Description = *form.Description + } + if err := models.UpdateLabel(label); err != nil { + ctx.ServerError("UpdateLabel", err) + return + } + ctx.JSON(http.StatusOK, convert.ToLabel(label)) +} + +// DeleteLabel delete a label for an organization +func DeleteLabel(ctx *context.APIContext) { + // swagger:operation DELETE /orgs/{org}/labels/{id} organization orgDeleteLabel + // --- + // summary: Delete a label + // parameters: + // - name: org + // in: path + // description: name of the organization + // type: string + // required: true + // - name: id + // in: path + // description: id of the label to delete + // type: integer + // format: int64 + // required: true + // responses: + // "204": + // "$ref": "#/responses/empty" + + if err := models.DeleteLabel(ctx.Org.Organization.ID, ctx.ParamsInt64(":id"), true); err != nil { + ctx.Error(http.StatusInternalServerError, "DeleteLabel", err) + return + } + + ctx.Status(http.StatusNoContent) +} diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index 808989126593e..ea51e77d47aa1 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -173,7 +173,7 @@ func DeleteIssueLabel(ctx *context.APIContext) { label, err := models.GetLabelInRepoByID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")) if err != nil { - if models.IsErrLabelNotExist(err) { + if models.IsErrRepoLabelNotExist(err) { ctx.Error(http.StatusUnprocessableEntity, "", err) } else { ctx.Error(http.StatusInternalServerError, "GetLabelInRepoByID", err) diff --git a/routers/api/v1/repo/label.go b/routers/api/v1/repo/label.go index 95dbbc955101b..7f023ca429b11 100644 --- a/routers/api/v1/repo/label.go +++ b/routers/api/v1/repo/label.go @@ -96,7 +96,7 @@ func GetLabel(ctx *context.APIContext) { label, err = models.GetLabelInRepoByID(ctx.Repo.Repository.ID, intID) } if err != nil { - if models.IsErrLabelNotExist(err) { + if models.IsErrRepoLabelNotExist(err) { ctx.NotFound() } else { ctx.Error(http.StatusInternalServerError, "GetLabelByRepoID", err) @@ -197,7 +197,7 @@ func EditLabel(ctx *context.APIContext, form api.EditLabelOption) { label, err := models.GetLabelInRepoByID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")) if err != nil { - if models.IsErrLabelNotExist(err) { + if models.IsErrRepoLabelNotExist(err) { ctx.NotFound() } else { ctx.Error(http.StatusInternalServerError, "GetLabelByRepoID", err) @@ -254,7 +254,7 @@ func DeleteLabel(ctx *context.APIContext) { // "204": // "$ref": "#/responses/empty" - if err := models.DeleteLabel(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")); err != nil { + if err := models.DeleteLabel(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id"), false); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteLabel", err) return } diff --git a/routers/org/org_labels.go b/routers/org/org_labels.go new file mode 100644 index 0000000000000..ac0ef0ed5a9a1 --- /dev/null +++ b/routers/org/org_labels.go @@ -0,0 +1,113 @@ +// Copyright 2020 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 org + +import ( + //"strings" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/auth" + //"code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/context" + //"code.gitea.io/gitea/modules/log" + //"code.gitea.io/gitea/modules/setting" + // userSetting "code.gitea.io/gitea/routers/user/setting" +) + +// RetrieveLabels find all the labels of an organization +func RetrieveLabels(ctx *context.Context) { + labels, err := models.GetLabelsByOrgID(ctx.Org.Organization.ID, ctx.Query("sort"), models.ListOptions{}) + if err != nil { + ctx.ServerError("RetrieveLabels.GetLabels", err) + return + } + for _, l := range labels { + l.CalOpenIssues() + } + ctx.Data["Labels"] = labels + ctx.Data["NumLabels"] = len(labels) + ctx.Data["SortType"] = ctx.Query("sort") +} + +// NewLabel create new label for repository +func NewLabel(ctx *context.Context, form auth.CreateLabelForm) { + println("are we here???") + ctx.Data["Title"] = ctx.Tr("repo.labels") + ctx.Data["PageIsLabels"] = true + + if ctx.HasError() { + ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) + ctx.Redirect(ctx.Org.OrgLink + "/settings/labels") + return + } + + l := &models.Label{ + OrgID: ctx.Org.Organization.ID, + Name: form.Title, + Description: form.Description, + Color: form.Color, + } + if err := models.NewLabel(l); err != nil { + ctx.ServerError("NewLabel", err) + return + } + ctx.Redirect(ctx.Org.OrgLink + "/settings/labels") +} + +// UpdateLabel update a label's name and color +func UpdateLabel(ctx *context.Context, form auth.CreateLabelForm) { + l, err := models.GetLabelByID(form.ID) + if err != nil { + switch { + case models.IsErrOrgLabelNotExist(err): + ctx.Error(404) + default: + ctx.ServerError("UpdateLabel", err) + } + return + } + + l.Name = form.Title + l.Description = form.Description + l.Color = form.Color + if err := models.UpdateLabel(l); err != nil { + ctx.ServerError("UpdateLabel", err) + return + } + ctx.Redirect(ctx.Org.OrgLink + "/settings/labels") +} + +// DeleteLabel delete a label +func DeleteLabel(ctx *context.Context) { + if err := models.DeleteLabel(ctx.Org.Organization.ID, ctx.QueryInt64("id"), true); err != nil { + ctx.Flash.Error("DeleteLabel: " + err.Error()) + } else { + ctx.Flash.Success(ctx.Tr("repo.issues.label_deletion_success")) + } + + ctx.JSON(200, map[string]interface{}{ + "redirect": ctx.Org.OrgLink + "/settings/labels", + }) +} + +// InitializeLabels init labels for an organization +func InitializeLabels(ctx *context.Context, form auth.InitializeLabelsForm) { + if ctx.HasError() { + ctx.Redirect(ctx.Repo.RepoLink + "/labels") + return + } + + if err := models.InitializeLabels(models.DefaultDBContext(), ctx.Org.Organization.ID, form.TemplateName, true); err != nil { + if models.IsErrIssueLabelTemplateLoad(err) { + originalErr := err.(models.ErrIssueLabelTemplateLoad).OriginalError + ctx.Flash.Error(ctx.Tr("repo.issues.label_templates.fail_to_load_file", form.TemplateName, originalErr)) + ctx.Redirect(ctx.Org.OrgLink + "/settings/labels") + return + } + ctx.ServerError("InitializeLabels", err) + return + } + ctx.Redirect(ctx.Org.OrgLink + "/settings/labels") +} diff --git a/routers/org/setting.go b/routers/org/setting.go index 3b6e1245878ad..6694b0401ec2f 100644 --- a/routers/org/setting.go +++ b/routers/org/setting.go @@ -24,6 +24,8 @@ const ( tplSettingsDelete base.TplName = "org/settings/delete" // tplSettingsHooks template path for render hook settings tplSettingsHooks base.TplName = "org/settings/hooks" + // + tplSettingsLabels base.TplName = "org/settings/labels" ) // Settings render the main settings page @@ -177,3 +179,15 @@ func DeleteWebhook(ctx *context.Context) { "redirect": ctx.Org.OrgLink + "/settings/hooks", }) } + +// Labels render organization labels page +func Labels(ctx *context.Context) { + ctx.Data["Title"] = ctx.Tr("repo.labels") + ctx.Data["IsOrg"] = true + ctx.Data["PageIsSettingsLabels"] = true + ctx.Data["PageIsLabels"] = true + ctx.Data["RequireMinicolors"] = true + ctx.Data["RequireTribute"] = true + ctx.Data["LabelTemplates"] = models.LabelTemplates + ctx.HTML(200, tplSettingsLabels) +} diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 9cc6ea1dfa3b3..05f2e6f5c4796 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -377,6 +377,17 @@ func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository, isPull boo return nil } ctx.Data["Labels"] = labels + if repo.Owner.IsOrganization() { + orgLabels, err := models.GetLabelsByOrgID(repo.Owner.ID, ctx.Query("sort"), models.ListOptions{}) + if err != nil { + return nil + } + + ctx.Data["OrgLabels"] = orgLabels + for _, v := range orgLabels { + labels = append(labels, v) + } + } RetrieveRepoMilestonesAndAssignees(ctx, repo) if ctx.Written() { @@ -593,6 +604,7 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) { Content: form.Content, Ref: form.Ref, } + if err := issue_service.NewIssue(repo, issue, labelIDs, attachments, assigneeIDs); err != nil { if models.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(400, "UserDoesNotHaveAccessToRepo", err.Error()) @@ -761,6 +773,24 @@ func ViewIssue(ctx *context.Context) { ctx.ServerError("GetLabelsByRepoID", err) return } + ctx.Data["Labels"] = labels + + if repo.Owner.IsOrganization() { + orgLabels, err := models.GetLabelsByOrgID(repo.Owner.ID, ctx.Query("sort"), models.ListOptions{}) + if err != nil { + ctx.ServerError("GetLabelsByOrgID", err) + return + } + for _, l := range orgLabels { + l.CalOpenIssues() + } + ctx.Data["OrgLabels"] = orgLabels + + for _, v := range orgLabels { + labels = append(labels, v) + } + } + hasSelected := false for i := range labels { if labelIDMark[labels[i].ID] { @@ -769,7 +799,6 @@ func ViewIssue(ctx *context.Context) { } } ctx.Data["HasSelectedLabel"] = hasSelected - ctx.Data["Labels"] = labels // Check milestone and assignee. if ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { diff --git a/routers/repo/issue_label.go b/routers/repo/issue_label.go index 8ac9b8d336867..a3bb19c3c281e 100644 --- a/routers/repo/issue_label.go +++ b/routers/repo/issue_label.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" issue_service "code.gitea.io/gitea/services/issue" ) @@ -35,7 +36,7 @@ func InitializeLabels(ctx *context.Context, form auth.InitializeLabelsForm) { return } - if err := models.InitializeLabels(models.DefaultDBContext(), ctx.Repo.Repository.ID, form.TemplateName); err != nil { + if err := models.InitializeLabels(models.DefaultDBContext(), ctx.Repo.Repository.ID, form.TemplateName, false); err != nil { if models.IsErrIssueLabelTemplateLoad(err) { originalErr := err.(models.ErrIssueLabelTemplateLoad).OriginalError ctx.Flash.Error(ctx.Tr("repo.issues.label_templates.fail_to_load_file", form.TemplateName, originalErr)) @@ -48,17 +49,47 @@ func InitializeLabels(ctx *context.Context, form auth.InitializeLabelsForm) { ctx.Redirect(ctx.Repo.RepoLink + "/labels") } -// RetrieveLabels find all the labels of a repository +// RetrieveLabels find all the labels of a repository and organization func RetrieveLabels(ctx *context.Context) { labels, err := models.GetLabelsByRepoID(ctx.Repo.Repository.ID, ctx.Query("sort"), models.ListOptions{}) if err != nil { ctx.ServerError("RetrieveLabels.GetLabels", err) return } + for _, l := range labels { l.CalOpenIssues() } + ctx.Data["Labels"] = labels + + if ctx.Repo.Owner.IsOrganization() { + orgLabels, err := models.GetLabelsByOrgID(ctx.Repo.Owner.ID, ctx.Query("sort"), models.ListOptions{}) + if err != nil { + ctx.ServerError("GetLabelsByOrgID", err) + return + } + for _, l := range orgLabels { + l.CalOpenIssues() + } + ctx.Data["OrgLabels"] = orgLabels + + org, err := models.GetOrgByName(ctx.Repo.Owner.LowerName) + if err != nil { + ctx.ServerError("GetOrgByName", err) + return + } + if ctx.User != nil { + ctx.Org.IsOwner, err = org.IsOwnedBy(ctx.User.ID) + if err != nil { + ctx.ServerError("org.IsOwnedBy", err) + return + } + ctx.Org.OrgLink = setting.AppSubURL + "/org/" + org.LowerName + ctx.Data["IsOrganizationOwner"] = ctx.Org.IsOwner + ctx.Data["OrganizationLink"] = ctx.Org.OrgLink + } + } ctx.Data["NumLabels"] = len(labels) ctx.Data["SortType"] = ctx.Query("sort") } @@ -92,7 +123,7 @@ func UpdateLabel(ctx *context.Context, form auth.CreateLabelForm) { l, err := models.GetLabelByID(form.ID) if err != nil { switch { - case models.IsErrLabelNotExist(err): + case models.IsErrRepoLabelNotExist(err): ctx.Error(404) default: ctx.ServerError("UpdateLabel", err) @@ -112,7 +143,7 @@ func UpdateLabel(ctx *context.Context, form auth.CreateLabelForm) { // DeleteLabel delete a label func DeleteLabel(ctx *context.Context) { - if err := models.DeleteLabel(ctx.Repo.Repository.ID, ctx.QueryInt64("id")); err != nil { + if err := models.DeleteLabel(ctx.Repo.Repository.ID, ctx.QueryInt64("id"), false); err != nil { ctx.Flash.Error("DeleteLabel: " + err.Error()) } else { ctx.Flash.Success(ctx.Tr("repo.issues.label_deletion_success")) @@ -141,7 +172,7 @@ func UpdateIssueLabel(ctx *context.Context) { case "attach", "detach", "toggle": label, err := models.GetLabelByID(ctx.QueryInt64("id")) if err != nil { - if models.IsErrLabelNotExist(err) { + if models.IsErrRepoLabelNotExist(err) { ctx.Error(404, "GetLabelByID") } else { ctx.ServerError("GetLabelByID", err) diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 093edcd920583..00c321c15d090 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -588,6 +588,14 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/feishu/:id", bindIgnErr(auth.NewFeishuHookForm{}), repo.FeishuHooksEditPost) }) + m.Group("/labels", func() { + m.Get("", org.RetrieveLabels, org.Labels) + m.Post("/new", bindIgnErr(auth.CreateLabelForm{}), org.NewLabel) + m.Post("/edit", bindIgnErr(auth.CreateLabelForm{}), org.UpdateLabel) + m.Post("/delete", org.DeleteLabel) + m.Post("/initialize", bindIgnErr(auth.InitializeLabelsForm{}), org.InitializeLabels) + }) + m.Route("/delete", "GET,POST", org.SettingsDelete) }) }, context.OrgAssignment(true, true)) diff --git a/services/issue/label.go b/services/issue/label.go index d2c1cd6ec5a0c..c8ef9e9536640 100644 --- a/services/issue/label.go +++ b/services/issue/label.go @@ -51,7 +51,10 @@ func RemoveLabel(issue *models.Issue, doer *models.User, label *models.Label) er return err } if !perm.CanWriteIssuesOrPulls(issue.IsPull) { - return models.ErrLabelNotExist{} + if label.OrgID > 0 { + return models.ErrOrgLabelNotExist{} + } + return models.ErrRepoLabelNotExist{} } if err := models.DeleteIssueLabel(issue, label, doer); err != nil { diff --git a/templates/org/settings/labels.tmpl b/templates/org/settings/labels.tmpl new file mode 100644 index 0000000000000..c1bff02ca5853 --- /dev/null +++ b/templates/org/settings/labels.tmpl @@ -0,0 +1,29 @@ +{{template "base/head" .}} +
+ {{template "org/header" .}} +
+
+ {{template "org/settings/navbar" .}} +
+
+
+ {{$.i18n.Tr "org.settings.labels_desc" | Str2html}} +
+
+
+
{{.i18n.Tr "repo.issues.new_label"}}
+
+
+
+
+ {{template "repo/issue/labels/label_new" .}} + {{template "base/alert" .}} + {{template "repo/issue/labels/label_list" .}} +
+
+
+
+ + +{{template "repo/issue/labels/edit_delete_label" .}} +{{template "base/footer" .}} \ No newline at end of file diff --git a/templates/org/settings/navbar.tmpl b/templates/org/settings/navbar.tmpl index 09fca5d7f6639..b1804b9833bde 100644 --- a/templates/org/settings/navbar.tmpl +++ b/templates/org/settings/navbar.tmpl @@ -7,6 +7,9 @@ {{.i18n.Tr "repo.settings.hooks"}} + + {{.i18n.Tr "repo.labels"}} + {{.i18n.Tr "org.settings.delete"}} diff --git a/templates/repo/issue/labels.tmpl b/templates/repo/issue/labels.tmpl index 4719c8f1fb2b7..d3df3b5944579 100644 --- a/templates/repo/issue/labels.tmpl +++ b/templates/repo/issue/labels.tmpl @@ -10,173 +10,17 @@ {{end}} - {{if not .Repository.IsArchived}} -
-
- {{.CsrfTokenHtml}} -
-
-
- -
-
-
-
- -
-
-
- -
-
- {{template "repo/issue/label_precolors"}} -
-
-
{{.i18n.Tr "repo.milestones.cancel"}}
- -
-
-
-
- {{end}}
- -
- -
- - {{.i18n.Tr "repo.issues.filter_sort"}} - - -
- {{.i18n.Tr "repo.issues.label.filter_sort.alphabetically"}} - {{.i18n.Tr "repo.issues.label.filter_sort.reverse_alphabetically"}} - {{.i18n.Tr "repo.milestones.filter_sort.least_issues"}} - {{.i18n.Tr "repo.milestones.filter_sort.most_issues"}} -
-
-
+ {{if and (or .CanWriteIssues .CanWritePulls) (not .Repository.IsArchived)}} + {{template "repo/issue/labels/label_new" .}} + {{end}} {{template "base/alert" .}} -
{{.i18n.Tr "repo.issues.label_count" .NumLabels}}
-
- {{if and (or $.CanWriteIssues $.CanWritePulls) (eq .NumLabels 0) (not $.Repository.IsArchived) }} -
-
-
- -

{{.i18n.Tr "repo.issues.label_templates.info"}}

-
-
- {{.CsrfTokenHtml}} -
-
- -
{{.i18n.Tr "repo.issues.label_templates.helper"}}
-
- {{range $template, $labels := .LabelTemplates}} -
{{$template}}
({{$labels}})
- {{end}} -
-
-
- -
-
-
-
- {{end}} - -
- - {{range .Labels}} -
  • -
    -
    -
    {{svg "octicon-tag" 16}} {{.Name}}
    -
    -
    - {{.Description}} -
    -
    - {{svg "octicon-issue-opened" 16}} {{$.i18n.Tr "repo.issues.label_open_issues" .NumOpenIssues}} -
    -
    - {{if and (not $.Repository.IsArchived) (or $.CanWriteIssues $.CanWritePulls)}} - {{svg "octicon-trashcan" 16}} {{$.i18n.Tr "repo.issues.label_delete"}} - {{svg "octicon-pencil" 16}} {{$.i18n.Tr "repo.issues.label_edit"}} - {{end}} -
    -
    -
    • - {{end}} -
    + {{template "repo/issue/labels/label_list" .}} -{{if and (or .CanWriteIssues .CanWritePulls) (not .Repository.IsArchived)}} -
    -
    - - {{.i18n.Tr "repo.issues.label_deletion"}} -
    -
    -

    {{.i18n.Tr "repo.issues.label_deletion_desc"}}

    -
    -
    -
    - - {{.i18n.Tr "modal.no"}} -
    -
    - - {{.i18n.Tr "modal.yes"}} -
    -
    -
    - -
    -
    - {{.i18n.Tr "repo.issues.label_modify"}} -
    -
    -
    - {{.CsrfTokenHtml}} - -
    -
    -
    - -
    -
    -
    -
    - -
    -
    -
    - -
    -
    - {{template "repo/issue/label_precolors"}} -
    -
    -
    -
    -
    -
    - {{.i18n.Tr "modal.no"}} -
    -
    - {{.i18n.Tr "modal.modify"}} - -
    -
    -
    +{{if and (or .CanWriteIssues .CanWritePulls) (not .Repository.IsArchived) }} +{{template "repo/issue/labels/edit_delete_label" .}} {{end}} + {{template "base/footer" .}} diff --git a/templates/repo/issue/labels/edit_delete_label.tmpl b/templates/repo/issue/labels/edit_delete_label.tmpl new file mode 100644 index 0000000000000..6d7fb6b99254e --- /dev/null +++ b/templates/repo/issue/labels/edit_delete_label.tmpl @@ -0,0 +1,58 @@ +
    +
    + + {{.i18n.Tr "repo.issues.label_deletion"}} +
    +
    +

    {{.i18n.Tr "repo.issues.label_deletion_desc"}}

    +
    +
    +
    + + {{.i18n.Tr "modal.no"}} +
    +
    + + {{.i18n.Tr "modal.yes"}} +
    +
    +
    + +
    +
    + {{.i18n.Tr "repo.issues.label_modify"}} +
    +
    +
    + {{.CsrfTokenHtml}} + +
    +
    +
    + +
    +
    +
    +
    + +
    +
    +
    + +
    +
    + {{template "repo/issue/label_precolors"}} +
    +
    +
    +
    +
    +
    + {{.i18n.Tr "modal.no"}} +
    +
    + {{.i18n.Tr "modal.modify"}} + +
    +
    +
    \ No newline at end of file diff --git a/templates/repo/issue/labels/label_list.tmpl b/templates/repo/issue/labels/label_list.tmpl new file mode 100644 index 0000000000000..e022b3bde3d38 --- /dev/null +++ b/templates/repo/issue/labels/label_list.tmpl @@ -0,0 +1,97 @@ +

    + {{.i18n.Tr "repo.issues.label_count" .NumLabels}} +
    +
    + +
    + + {{.i18n.Tr "repo.issues.filter_sort"}} + + +
    + {{.i18n.Tr "repo.issues.label.filter_sort.alphabetically"}} + {{.i18n.Tr "repo.issues.label.filter_sort.reverse_alphabetically"}} + {{.i18n.Tr "repo.milestones.filter_sort.least_issues"}} + {{.i18n.Tr "repo.milestones.filter_sort.most_issues"}} +
    +
    +
    +
    +

    + +
    +
    + {{if and (not $.IsOrg) (or $.CanWriteIssues $.CanWritePulls) (eq .NumLabels 0) (not $.Repository.IsArchived) }} + {{template "repo/issue/labels/label_load_template" .}} +
    + {{else if and ($.IsOrg) (eq .NumLabels 0)}} + {{template "repo/issue/labels/label_load_template" .}} + {{end}} + {{range .Labels}} +
  • +
    +
    +
    {{svg "octicon-tag" 16}} {{.Name}}
    +
    +
    +
    + {{.Description}} +
    +
    +
    + {{if $.IsOrg}} + {{svg "octicon-issue-opened" 16}} {{$.i18n.Tr "repo.issues.label_open_issues" .NumOpenIssues}} + {{else}} + {{svg "octicon-issue-opened" 16}} {{$.i18n.Tr "repo.issues.label_open_issues" .NumOpenIssues}} + {{end}} +
    +
    + {{if and (not $.IsOrg ) (not $.Repository.IsArchived) (or $.CanWriteIssues $.CanWritePulls)}} + {{svg "octicon-trashcan" 16}} {{$.i18n.Tr "repo.issues.label_delete"}} + {{svg "octicon-pencil" 16}} {{$.i18n.Tr "repo.issues.label_edit"}} + {{else if $.IsOrg}} + {{svg "octicon-trashcan" 16}} {{$.i18n.Tr "repo.issues.label_delete"}} + {{svg "octicon-pencil" 16}} {{$.i18n.Tr "repo.issues.label_edit"}} + {{end}} +
    +
    +
    • + {{end}} + {{if not .IsOrg}} +
  • +
    +
    + {{$.i18n.Tr "repo.org.labels" | Str2html}} + {{if .IsOrganizationOwner}} + ({{$.i18n.Tr "repo.org.labels.manage"}}): + {{end}} +
    +
    +
    • + {{if (not $.IsOrg)}} +
    + {{range .OrgLabels}} +
  • +
    +
    +
    {{svg "octicon-tag" 16}} {{.Name}}
    +
    +
    +
    + {{.Description}} +
    +
    +
    + {{svg "octicon-issue-opened" 16}} {{$.i18n.Tr "repo.issues.label_open_issues" .NumOpenIssues}} +
    +
    +
    +
    +
    • + {{end}} +
    + {{end}} + {{end}} +
    +
    + diff --git a/templates/repo/issue/labels/label_load_template.tmpl b/templates/repo/issue/labels/label_load_template.tmpl new file mode 100644 index 0000000000000..299f22e61fd14 --- /dev/null +++ b/templates/repo/issue/labels/label_load_template.tmpl @@ -0,0 +1,30 @@ +
    +
    +
    + +

    {{.i18n.Tr "repo.issues.label_templates.info"}}

    +
    +
    + {{.CsrfTokenHtml}} +
    +
    + +
    {{.i18n.Tr "repo.issues.label_templates.helper"}}
    +
    + {{range $template, $labels := .LabelTemplates}} +
    {{$template}}
    ({{$labels}})
    + {{end}} +
    +
    +
    + +
    +
    +
    +
    \ No newline at end of file diff --git a/templates/repo/issue/labels/label_new.tmpl b/templates/repo/issue/labels/label_new.tmpl new file mode 100644 index 0000000000000..aa14579c57ca8 --- /dev/null +++ b/templates/repo/issue/labels/label_new.tmpl @@ -0,0 +1,27 @@ +
    +
    + {{.CsrfTokenHtml}} +
    +
    +
    + +
    +
    +
    +
    + +
    +
    +
    + +
    +
    + {{template "repo/issue/label_precolors"}} +
    +
    +
    {{.i18n.Tr "repo.milestones.cancel"}}
    + +
    +
    +
    +
    \ No newline at end of file diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 524b849c1444f..4df49d9c8ba5a 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -49,6 +49,11 @@ {{svg "octicon-check" 16}} {{.Name}} {{if .Description }}
    {{.Description}}{{end}}     {{end}} +
    + {{range .OrgLabels}} + {{svg "octicon-check" 16}} {{.Name}} + {{if .Description }}
    {{.Description}}{{end}}     + {{end}}
    @@ -56,6 +61,9 @@ {{range .Labels}} {{.Name}} {{end}} + {{range .OrgLabels}} + {{.Name}} + {{end}}
    diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 1d1f1916da0bd..d0275c23f4aa7 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -15,6 +15,11 @@ {{svg "octicon-check" 16}} {{.Name}} {{if .Description }}
    {{.Description}}{{end}}     {{end}} +
    + {{range .OrgLabels}} + {{svg "octicon-check" 16}} {{.Name}} + {{if .Description }}
    {{.Description}}{{end}}     + {{end}}
    @@ -23,6 +28,11 @@
    {{.Name}}
    + {{end}} + {{range .OrgLabels}} +
    + {{.Name}} +
    {{end}}
    diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 07d760212ea0f..90c60c0eedf90 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -981,6 +981,189 @@ } } }, + "/orgs/{org}/labels": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "organization" + ], + "summary": "List an organization's labels", + "operationId": "orgListLabels", + "parameters": [ + { + "type": "string", + "description": "name of the organization", + "name": "org", + "in": "path", + "required": true + }, + { + "type": "integer", + "description": "page number of results to return (1-based)", + "name": "page", + "in": "query" + }, + { + "type": "integer", + "description": "page size of results, maximum page size is 50", + "name": "limit", + "in": "query" + } + ], + "responses": { + "200": { + "$ref": "#/responses/LabelList" + } + } + }, + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "organization" + ], + "summary": "Create a label for an organization", + "operationId": "orgCreateLabel", + "parameters": [ + { + "type": "string", + "description": "name of the organization", + "name": "org", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/CreateLabelOption" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/Label" + }, + "422": { + "$ref": "#/responses/validationError" + } + } + } + }, + "/orgs/{org}/labels/{id}": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "organization" + ], + "summary": "Get a single label", + "operationId": "orgGetLabel", + "parameters": [ + { + "type": "string", + "description": "name of the organization", + "name": "org", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the label to get", + "name": "id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/Label" + } + } + }, + "delete": { + "tags": [ + "organization" + ], + "summary": "Delete a label", + "operationId": "orgDeleteLabel", + "parameters": [ + { + "type": "string", + "description": "name of the organization", + "name": "org", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the label to delete", + "name": "id", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + } + } + }, + "patch": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "organization" + ], + "summary": "Update a label", + "operationId": "orgEditLabel", + "parameters": [ + { + "type": "string", + "description": "name of the organization", + "name": "org", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the label to edit", + "name": "id", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/EditLabelOption" + } + } + ], + "responses": { + "200": { + "$ref": "#/responses/Label" + }, + "422": { + "$ref": "#/responses/validationError" + } + } + } + }, "/orgs/{org}/members": { "get": { "produces": [ diff --git a/web_src/js/index.js b/web_src/js/index.js index 2db5b08b8bee0..a0e410c133e96 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -125,6 +125,39 @@ function initBranchSelector() { }); } +function initLabelEdit() { +// Create label + const $newLabelPanel = $('.new-label.segment'); + $('.new-label.button').click(() => { + $newLabelPanel.show(); + }); + $('.new-label.segment .cancel').click(() => { + $newLabelPanel.hide(); + }); + + $('.color-picker').each(function () { + $(this).minicolors(); + }); + $('.precolors .color').click(function () { + const color_hex = $(this).data('color-hex'); + $('.color-picker').val(color_hex); + $('.minicolors-swatch-color').css('background-color', color_hex); + }); + $('.edit-label-button').click(function () { + $('#label-modal-id').val($(this).data('id')); + $('.edit-label .new-label-input').val($(this).data('title')); + $('.edit-label .new-label-desc-input').val($(this).data('description')); + $('.edit-label .color-picker').val($(this).data('color')); + $('.minicolors-swatch-color').css('background-color', $(this).data('color')); + $('.edit-label.modal').modal({ + onApprove() { + $('.edit-label.form').submit(); + } + }).modal('show'); + return false; + }); +} + function updateIssuesMeta(url, action, issueIds, elementId) { return new Promise(((resolve) => { $.ajax({ @@ -697,36 +730,7 @@ async function initRepository() { // Labels if ($('.repository.labels').length > 0) { - // Create label - const $newLabelPanel = $('.new-label.segment'); - $('.new-label.button').click(() => { - $newLabelPanel.show(); - }); - $('.new-label.segment .cancel').click(() => { - $newLabelPanel.hide(); - }); - - $('.color-picker').each(function () { - $(this).minicolors(); - }); - $('.precolors .color').click(function () { - const color_hex = $(this).data('color-hex'); - $('.color-picker').val(color_hex); - $('.minicolors-swatch-color').css('background-color', color_hex); - }); - $('.edit-label-button').click(function () { - $('#label-modal-id').val($(this).data('id')); - $('.edit-label .new-label-input').val($(this).data('title')); - $('.edit-label .new-label-desc-input').val($(this).data('description')); - $('.edit-label .color-picker').val($(this).data('color')); - $('.minicolors-swatch-color').css('background-color', $(this).data('color')); - $('.edit-label.modal').modal({ - onApprove() { - $('.edit-label.form').submit(); - } - }).modal('show'); - return false; - }); + initLabelEdit(); } // Milestones @@ -1757,6 +1761,11 @@ function initOrganization() { } }); } + + // Labels + if ($('.organization.settings.labels').length > 0) { + initLabelEdit(); + } } function initUserSettings() { diff --git a/web_src/less/_organization.less b/web_src/less/_organization.less index 6071604cbcd63..5a72017c2f5bb 100644 --- a/web_src/less/_organization.less +++ b/web_src/less/_organization.less @@ -168,4 +168,45 @@ height: 60px; } } + + &.settings { + .labelspage { + list-style: none; + padding-top: 0; + + .item { + margin-top: 0; + margin-right: -14px; + margin-left: -14px !important; + padding: 10px; + border-bottom: 1px solid #e1e4e8; + border-top: none; + + a { + font-size: 15px; + padding-top: 5px; + padding-right: 10px; + color: #666666; + + &:hover { + color: #000000; + } + + &.open-issues { + margin-right: 30px; + } + } + + .ui.label { + font-size: 1em; + } + } + + .item:last-child { + border-bottom: none; + padding-bottom: 0; + } + + } + } } diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index f570bcd7fd93c..b4e3403acaa50 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -1045,14 +1045,17 @@ } } - .label.list { + .labelspage { list-style: none; - padding-top: 15px; + padding-top: 0; .item { - padding-top: 10px; - padding-bottom: 10px; - border-bottom: 1px dashed #aaaaaa; + margin-top: 0; + margin-right: -14px; + margin-left: -14px; + padding: 10px; + border-bottom: 1px solid #e1e4e8; + border-top: none; a { font-size: 15px; @@ -1073,6 +1076,16 @@ font-size: 1em; } } + + .item:last-child { + border-bottom: none; + padding-bottom: 0; + } + + .orglabel { + opacity: .7; + } + } .milestone.list { From b7dc22ced3cf8c3ef8567847d3f0adb52efa86a3 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 24 Mar 2020 18:58:54 -0400 Subject: [PATCH 02/41] Add migration --- models/migrations/migrations.go | 2 ++ models/migrations/v132.go | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 models/migrations/v132.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 2badb72788fb1..90e93bca20fb9 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -196,6 +196,8 @@ var migrations = []Migration{ NewMigration("Expand webhooks for more granularity", expandWebhooks), // v131 -> v132 NewMigration("Add IsSystemWebhook column to webhooks table", addSystemWebhookColumn), + // v132 -> v133 + NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), } // Migrate database to current version diff --git a/models/migrations/v132.go b/models/migrations/v132.go new file mode 100644 index 0000000000000..8d859d42c05b0 --- /dev/null +++ b/models/migrations/v132.go @@ -0,0 +1,22 @@ +// Copyright 2020 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 migrations + +import ( + "fmt" + + "xorm.io/xorm" +) + +func addOrgIDLabelColumn(x *xorm.Engine) error { + type Label struct { + OrgID int64 `xorm:"INDEX"` + } + + if err := x.Sync2(new(Label)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + return nil +} From 350569a37d804a6876a4330f3b7ff961c00595b0 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 24 Mar 2020 19:02:32 -0400 Subject: [PATCH 03/41] remove comments --- routers/org/org_labels.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/routers/org/org_labels.go b/routers/org/org_labels.go index ac0ef0ed5a9a1..c4898afeb7a2a 100644 --- a/routers/org/org_labels.go +++ b/routers/org/org_labels.go @@ -5,15 +5,9 @@ package org import ( - //"strings" - "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth" - //"code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" - //"code.gitea.io/gitea/modules/log" - //"code.gitea.io/gitea/modules/setting" - // userSetting "code.gitea.io/gitea/routers/user/setting" ) // RetrieveLabels find all the labels of an organization @@ -33,7 +27,6 @@ func RetrieveLabels(ctx *context.Context) { // NewLabel create new label for repository func NewLabel(ctx *context.Context, form auth.CreateLabelForm) { - println("are we here???") ctx.Data["Title"] = ctx.Tr("repo.labels") ctx.Data["PageIsLabels"] = true From c562eef957681c8004a216a19c3ca221ba762b08 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 24 Mar 2020 19:32:18 -0400 Subject: [PATCH 04/41] fix tests --- models/issue_label_test.go | 2 +- models/issue_list_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/models/issue_label_test.go b/models/issue_label_test.go index 2fbfeb4c9a58d..fd04f89dd9bd8 100644 --- a/models/issue_label_test.go +++ b/models/issue_label_test.go @@ -209,7 +209,7 @@ func TestGetLabelsByOrgID(t *testing.T) { } } testSuccess(1, "leastissues", []int64{3, 4}) - testSuccess(1, "mostissues", []int64{3, 4}) + testSuccess(1, "mostissues", []int64{4, 3}) testSuccess(1, "reversealphabetically", []int64{4, 3}) testSuccess(1, "default", []int64{3, 4}) } diff --git a/models/issue_list_test.go b/models/issue_list_test.go index f5a91702f25dd..c9c39332c7a8e 100644 --- a/models/issue_list_test.go +++ b/models/issue_list_test.go @@ -34,7 +34,6 @@ func TestIssueList_LoadAttributes(t *testing.T) { setting.Service.EnableTimetracking = true issueList := IssueList{ AssertExistsAndLoadBean(t, &Issue{ID: 1}).(*Issue), - AssertExistsAndLoadBean(t, &Issue{ID: 2}).(*Issue), AssertExistsAndLoadBean(t, &Issue{ID: 4}).(*Issue), } From 140d87d9f9208c48ead4d4c555698581a9e40b34 Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Tue, 24 Mar 2020 22:59:31 -0400 Subject: [PATCH 05/41] Update options/locale/locale_en-US.ini Removed unused translation string --- options/locale/locale_en-US.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 2fcf90bb7e096..6417e06b228ac 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1695,7 +1695,6 @@ settings.delete_org_desc = This organization will be deleted permanently. Contin settings.hooks_desc = Add webhooks which will be triggered for all repositories under this organization. settings.labels_desc = Add labels which can be used on issues for all repositories under this organization. -settings.labels = Labels members.membership_visibility = Membership Visibility: members.public = Visible From 00729a240e0bdbca65e5738197707c15cc9a5dd3 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Wed, 25 Mar 2020 17:43:13 -0400 Subject: [PATCH 06/41] show org labels in issue search label filter --- routers/repo/issue.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 05f2e6f5c4796..1a97984961fb3 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -259,6 +259,20 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB ctx.ServerError("GetLabelsByRepoID", err) return } + + if repo.Owner.IsOrganization() { + orgLabels, err := models.GetLabelsByOrgID(repo.Owner.ID, ctx.Query("sort"), models.ListOptions{}) + if err != nil { + ctx.ServerError("GetLabelsByOrgID", err) + return + } + + ctx.Data["OrgLabels"] = orgLabels + for _, v := range orgLabels { + labels = append(labels, v) + } + } + for _, l := range labels { l.LoadSelectedLabelsAfterClick(labelIDs) } From 8af0f6c7dda1f4a70106d66252c2d0e0217e7e81 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Fri, 27 Mar 2020 01:11:45 -0400 Subject: [PATCH 07/41] Use more clear var name --- routers/org/setting.go | 4 +--- templates/repo/issue/labels/label_list.tmpl | 14 +++++++------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/routers/org/setting.go b/routers/org/setting.go index 6694b0401ec2f..c626f9f4c9728 100644 --- a/routers/org/setting.go +++ b/routers/org/setting.go @@ -183,9 +183,7 @@ func DeleteWebhook(ctx *context.Context) { // Labels render organization labels page func Labels(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.labels") - ctx.Data["IsOrg"] = true - ctx.Data["PageIsSettingsLabels"] = true - ctx.Data["PageIsLabels"] = true + ctx.Data["PageIsOrgSettingsLabels"] = true ctx.Data["RequireMinicolors"] = true ctx.Data["RequireTribute"] = true ctx.Data["LabelTemplates"] = models.LabelTemplates diff --git a/templates/repo/issue/labels/label_list.tmpl b/templates/repo/issue/labels/label_list.tmpl index e022b3bde3d38..51e1016e6feb0 100644 --- a/templates/repo/issue/labels/label_list.tmpl +++ b/templates/repo/issue/labels/label_list.tmpl @@ -21,10 +21,10 @@
    - {{if and (not $.IsOrg) (or $.CanWriteIssues $.CanWritePulls) (eq .NumLabels 0) (not $.Repository.IsArchived) }} + {{if and (not $.PageIsOrgSettingsLabels) (or $.CanWriteIssues $.CanWritePulls) (eq .NumLabels 0) (not $.Repository.IsArchived) }} {{template "repo/issue/labels/label_load_template" .}}
    - {{else if and ($.IsOrg) (eq .NumLabels 0)}} + {{else if and ($.PageIsOrgSettingsLabels) (eq .NumLabels 0)}} {{template "repo/issue/labels/label_load_template" .}} {{end}} {{range .Labels}} @@ -39,17 +39,17 @@
    - {{if $.IsOrg}} + {{if $.PageIsOrgSettingsLabels}} {{svg "octicon-issue-opened" 16}} {{$.i18n.Tr "repo.issues.label_open_issues" .NumOpenIssues}} {{else}} {{svg "octicon-issue-opened" 16}} {{$.i18n.Tr "repo.issues.label_open_issues" .NumOpenIssues}} {{end}}
    - {{if and (not $.IsOrg ) (not $.Repository.IsArchived) (or $.CanWriteIssues $.CanWritePulls)}} + {{if and (not $.PageIsOrgSettingsLabels ) (not $.Repository.IsArchived) (or $.CanWriteIssues $.CanWritePulls)}} {{svg "octicon-trashcan" 16}} {{$.i18n.Tr "repo.issues.label_delete"}} {{svg "octicon-pencil" 16}} {{$.i18n.Tr "repo.issues.label_edit"}} - {{else if $.IsOrg}} + {{else if $.PageIsOrgSettingsLabels}} {{svg "octicon-trashcan" 16}} {{$.i18n.Tr "repo.issues.label_delete"}} {{svg "octicon-pencil" 16}} {{$.i18n.Tr "repo.issues.label_edit"}} {{end}} @@ -57,7 +57,7 @@
    {{end}} - {{if not .IsOrg}} + {{if and (not .PageIsOrgSettingsLabels) (.OrgLabels) }}
  • @@ -68,7 +68,7 @@
    • - {{if (not $.IsOrg)}} + {{if (not $.PageIsOrgSettingsLabels)}}
    {{range .OrgLabels}}
  • From 101d4e7aa8160a90237ca0350fdb34f71eb1e76e Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Fri, 27 Mar 2020 01:59:24 -0400 Subject: [PATCH 08/41] rename migration after merge from master --- models/migrations/migrations.go | 4 ++-- models/migrations/v133.go | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 models/migrations/v133.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index f1bbd20054ec6..986fcfde45d56 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -198,8 +198,8 @@ var migrations = []Migration{ NewMigration("Add IsSystemWebhook column to webhooks table", addSystemWebhookColumn), // v132 -> v133 NewMigration("Add Branch Protection Protected Files Column", addBranchProtectionProtectedFilesColumn), - // v132 -> v134 - NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), + // v132 -> v134 + NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), } // Migrate database to current version diff --git a/models/migrations/v133.go b/models/migrations/v133.go new file mode 100644 index 0000000000000..8d859d42c05b0 --- /dev/null +++ b/models/migrations/v133.go @@ -0,0 +1,22 @@ +// Copyright 2020 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 migrations + +import ( + "fmt" + + "xorm.io/xorm" +) + +func addOrgIDLabelColumn(x *xorm.Engine) error { + type Label struct { + OrgID int64 `xorm:"INDEX"` + } + + if err := x.Sync2(new(Label)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + return nil +} From 33ca43e385bf3b4ce284d7b8d6cdbab00971194e Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Fri, 27 Mar 2020 02:00:40 -0400 Subject: [PATCH 09/41] comment typo --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 986fcfde45d56..7d20b589a65b6 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -198,7 +198,7 @@ var migrations = []Migration{ NewMigration("Add IsSystemWebhook column to webhooks table", addSystemWebhookColumn), // v132 -> v133 NewMigration("Add Branch Protection Protected Files Column", addBranchProtectionProtectedFilesColumn), - // v132 -> v134 + // v133 -> v134 NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), } From 13fadae071770d1ec330bc413aacfe20be73d9a4 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Fri, 27 Mar 2020 10:15:39 -0400 Subject: [PATCH 10/41] update migration again after rebase with master --- models/migrations/migrations.go | 4 ++-- models/migrations/v133.go | 2 +- models/migrations/v134.go | 22 ++++++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 models/migrations/v134.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index edd2c0ea9e176..e858db4872421 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -200,8 +200,8 @@ var migrations = []Migration{ NewMigration("Add Branch Protection Protected Files Column", addBranchProtectionProtectedFilesColumn), // v133 -> v134 NewMigration("Add EmailHash Table", addEmailHashTable), - // v134 -> v135 - NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), + // v134 -> v135 + NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), } // Migrate database to current version diff --git a/models/migrations/v133.go b/models/migrations/v133.go index 0cc939a811148..ea0411d470bef 100644 --- a/models/migrations/v133.go +++ b/models/migrations/v133.go @@ -13,4 +13,4 @@ func addEmailHashTable(x *xorm.Engine) error { Email string `xorm:"UNIQUE NOT NULL"` } return x.Sync2(new(EmailHash)) -} \ No newline at end of file +} diff --git a/models/migrations/v134.go b/models/migrations/v134.go new file mode 100644 index 0000000000000..8d859d42c05b0 --- /dev/null +++ b/models/migrations/v134.go @@ -0,0 +1,22 @@ +// Copyright 2020 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 migrations + +import ( + "fmt" + + "xorm.io/xorm" +) + +func addOrgIDLabelColumn(x *xorm.Engine) error { + type Label struct { + OrgID int64 `xorm:"INDEX"` + } + + if err := x.Sync2(new(Label)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + return nil +} From a82a29b67f4b269854777605a622eee28f5674a3 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Fri, 27 Mar 2020 15:17:08 -0400 Subject: [PATCH 11/41] check for orgID <=0 per guillep2k review --- models/issue_label.go | 20 ++++++++++++++------ models/issue_label_test.go | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 38a17b917f890..982606efddaaa 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -338,11 +338,12 @@ func GetLabelsByRepoID(repoID int64, sortType string, listOptions ListOptions) ( // \/ /_____/ // getLabelInOrgByName returns a label by Name in given organization -// If pass orgID as 0, then ORM will ignore limitation of organization -// and can return arbitrary label with any valid ID. func getLabelInOrgByName(e Engine, orgID int64, labelName string) (*Label, error) { if len(labelName) == 0 { - return nil, ErrOrgLabelNotExist{0, orgID} // FIX ME! + return nil, ErrOrgLabelNotExist{0, orgID} + } + if orgID <= 0 { + return nil, ErrOrgLabelNotExist{0, orgID} } l := &Label{ @@ -359,12 +360,13 @@ func getLabelInOrgByName(e Engine, orgID int64, labelName string) (*Label, error } // getLabelInOrgByID returns a label by ID in given organization. -// If pass orgID as 0, then ORM will ignore limitation of organization -// and can return arbitrary label with any valid ID. func getLabelInOrgByID(e Engine, orgID, labelID int64) (*Label, error) { if labelID <= 0 { return nil, ErrOrgLabelNotExist{labelID, orgID} } + if orgID <= 0 { + return nil, ErrOrgLabelNotExist{labelID, orgID} + } l := &Label{ ID: labelID, @@ -386,9 +388,12 @@ func GetLabelInOrgByName(orgID int64, labelName string) (*Label, error) { // GetLabelIDsInOrgByNames returns a list of labelIDs by names in a given // organization. -// it silently ignores label names that do not belong to the organization. func GetLabelIDsInOrgByNames(orgID int64, labelNames []string) ([]int64, error) { + if orgID <= 0 { + return nil, ErrOrgLabelNotExist{0, orgID} + } labelIDs := make([]int64, 0, len(labelNames)) + return labelIDs, x.Table("label"). Where("org_id = ?", orgID). In("name", labelNames). @@ -427,6 +432,9 @@ func GetLabelsInOrgByIDs(orgID int64, labelIDs []int64) ([]*Label, error) { } func getLabelsByOrgID(e Engine, orgID int64, sortType string, listOptions ListOptions) ([]*Label, error) { + if orgID <= 0 { + return nil, ErrOrgLabelNotExist{0, orgID} + } labels := make([]*Label, 0, 10) sess := e.Where("org_id = ?", orgID) diff --git a/models/issue_label_test.go b/models/issue_label_test.go index fd04f89dd9bd8..f970d769ba96e 100644 --- a/models/issue_label_test.go +++ b/models/issue_label_test.go @@ -147,6 +147,12 @@ func TestGetLabelInOrgByName(t *testing.T) { _, err = GetLabelInOrgByName(1, "") assert.True(t, IsErrOrgLabelNotExist(err)) + _, err = GetLabelInOrgByName(0, "orglabel3") + assert.True(t, IsErrOrgLabelNotExist(err)) + + _, err = GetLabelInOrgByName(-1, "orglabel3") + assert.True(t, IsErrOrgLabelNotExist(err)) + _, err = GetLabelInOrgByName(NonexistentID, "nonexistent") assert.True(t, IsErrOrgLabelNotExist(err)) } @@ -184,6 +190,12 @@ func TestGetLabelInOrgByID(t *testing.T) { _, err = GetLabelInOrgByID(1, -1) assert.True(t, IsErrOrgLabelNotExist(err)) + _, err = GetLabelInOrgByID(0, 3) + assert.True(t, IsErrOrgLabelNotExist(err)) + + _, err = GetLabelInOrgByID(-1, 3) + assert.True(t, IsErrOrgLabelNotExist(err)) + _, err = GetLabelInOrgByID(NonexistentID, NonexistentID) assert.True(t, IsErrOrgLabelNotExist(err)) } @@ -212,6 +224,15 @@ func TestGetLabelsByOrgID(t *testing.T) { testSuccess(1, "mostissues", []int64{4, 3}) testSuccess(1, "reversealphabetically", []int64{4, 3}) testSuccess(1, "default", []int64{3, 4}) + + + var err error + _, err = GetLabelsByOrgID(0, "leastissues", ListOptions{}) + assert.True(t, IsErrOrgLabelNotExist(err)) + + _, err = GetLabelsByOrgID(-1, "leastissues", ListOptions{}) + assert.True(t, IsErrOrgLabelNotExist(err)) + } // From 27209eebe61ea6c5660c553c67b816e441e8e188 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Fri, 27 Mar 2020 15:17:26 -0400 Subject: [PATCH 12/41] fmt --- models/issue_label_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/issue_label_test.go b/models/issue_label_test.go index f970d769ba96e..4dc1be64a3b93 100644 --- a/models/issue_label_test.go +++ b/models/issue_label_test.go @@ -225,7 +225,6 @@ func TestGetLabelsByOrgID(t *testing.T) { testSuccess(1, "reversealphabetically", []int64{4, 3}) testSuccess(1, "default", []int64{3, 4}) - var err error _, err = GetLabelsByOrgID(0, "leastissues", ListOptions{}) assert.True(t, IsErrOrgLabelNotExist(err)) From eae47240192361abd437a1d1c67551f987e917c0 Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Fri, 27 Mar 2020 15:36:29 -0400 Subject: [PATCH 13/41] Apply suggestions from code review Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com> --- models/issue_label.go | 2 +- routers/org/org_labels.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 982606efddaaa..28e0260f41b35 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -547,7 +547,7 @@ func DeleteLabel(id, labelID int64, isOrg bool) error { func DeleteOrgLabel(orgID, labelID int64) error { _, err := GetLabelInOrgByID(orgID, labelID) if err != nil { - if IsErrRepoLabelNotExist(err) { + if IsErrOrgLabelNotExist(err) { return nil } return err diff --git a/routers/org/org_labels.go b/routers/org/org_labels.go index c4898afeb7a2a..7be26e94719e5 100644 --- a/routers/org/org_labels.go +++ b/routers/org/org_labels.go @@ -25,7 +25,7 @@ func RetrieveLabels(ctx *context.Context) { ctx.Data["SortType"] = ctx.Query("sort") } -// NewLabel create new label for repository +// NewLabel create new label for organization func NewLabel(ctx *context.Context, form auth.CreateLabelForm) { ctx.Data["Title"] = ctx.Tr("repo.labels") ctx.Data["PageIsLabels"] = true From a68f6f83baa8a7c8758483c79c4fdfbc5c7eacb1 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Fri, 27 Mar 2020 15:42:58 -0400 Subject: [PATCH 14/41] remove unused code --- routers/repo/issue.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 1a97984961fb3..25e143db81a6e 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -795,9 +795,6 @@ func ViewIssue(ctx *context.Context) { ctx.ServerError("GetLabelsByOrgID", err) return } - for _, l := range orgLabels { - l.CalOpenIssues() - } ctx.Data["OrgLabels"] = orgLabels for _, v := range orgLabels { From 3a0b210f31ee7eef7cc5bde9aec2b1fb5aa978c5 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Fri, 27 Mar 2020 15:49:46 -0400 Subject: [PATCH 15/41] Make sure RepoID is 0 when searching orgID per code review --- models/issue_label.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 28e0260f41b35..b569032b1d2c3 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -347,8 +347,9 @@ func getLabelInOrgByName(e Engine, orgID int64, labelName string) (*Label, error } l := &Label{ - Name: labelName, - OrgID: orgID, + Name: labelName, + RepoID: 0, + OrgID: orgID, } has, err := e.Get(l) if err != nil { @@ -369,8 +370,9 @@ func getLabelInOrgByID(e Engine, orgID, labelID int64) (*Label, error) { } l := &Label{ - ID: labelID, - OrgID: orgID, + ID: labelID, + RepoID: 0, + OrgID: orgID, } has, err := e.Get(l) if err != nil { From f4afe502de9fae7ba5e9cc7e53c7163c11a71f2f Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sat, 28 Mar 2020 17:53:55 -0400 Subject: [PATCH 16/41] more changes/code review requests --- models/error.go | 23 ++- models/issue.go | 23 ++- models/issue_label.go | 233 +++++++++++++++-------------- models/issue_label_test.go | 10 +- routers/api/v1/org/label.go | 5 +- routers/api/v1/repo/issue.go | 1 + routers/api/v1/repo/issue_label.go | 10 +- routers/api/v1/repo/label.go | 2 +- routers/org/org_labels.go | 4 +- routers/repo/issue_label.go | 4 +- templates/repo/issue/new_form.tmpl | 2 +- 11 files changed, 178 insertions(+), 139 deletions(-) diff --git a/models/error.go b/models/error.go index 2f1b6b67570a6..2357de1130b10 100644 --- a/models/error.go +++ b/models/error.go @@ -1568,13 +1568,13 @@ func (err ErrTrackedTimeNotExist) Error() string { // |_______ (____ /___ /\___ >____/ // \/ \/ \/ \/ -// ErrRepoLabelNotExist represents a "LabelNotExist" kind of error. +// ErrRepoLabelNotExist represents a "RepoLabelNotExist" kind of error. type ErrRepoLabelNotExist struct { LabelID int64 RepoID int64 } -// IsErrRepoLabelNotExist checks if an error is a ErrLabelNotExist. +// IsErrRepoLabelNotExist checks if an error is a RepoErrLabelNotExist. func IsErrRepoLabelNotExist(err error) bool { _, ok := err.(ErrRepoLabelNotExist) return ok @@ -1584,13 +1584,13 @@ func (err ErrRepoLabelNotExist) Error() string { return fmt.Sprintf("label does not exist [label_id: %d, repo_id: %d]", err.LabelID, err.RepoID) } -// ErrOrgLabelNotExist represents a "LabelNotExist" kind of error. +// ErrOrgLabelNotExist represents a "OrgLabelNotExist" kind of error. type ErrOrgLabelNotExist struct { LabelID int64 OrgID int64 } -// IsErrOrgLabelNotExist checks if an error is a ErrLabelNotExist. +// IsErrOrgLabelNotExist checks if an error is a OrgErrLabelNotExist. func IsErrOrgLabelNotExist(err error) bool { _, ok := err.(ErrOrgLabelNotExist) return ok @@ -1600,6 +1600,21 @@ func (err ErrOrgLabelNotExist) Error() string { return fmt.Sprintf("label does not exist [label_id: %d, org_id: %d]", err.LabelID, err.OrgID) } +// ErrLabelNotExist represents a "LabelNotExist" kind of error. +type ErrLabelNotExist struct { + LabelID int64 +} + +// IsErrLabelNotExist checks if an error is a ErrLabelNotExist. +func IsErrLabelNotExist(err error) bool { + _, ok := err.(ErrLabelNotExist) + return ok +} + +func (err ErrLabelNotExist) Error() string { + return fmt.Sprintf("label does not exist [label_id: %d]", err.LabelID) +} + // _____ .__.__ __ // / \ |__| | ____ _______/ |_ ____ ____ ____ // / \ / \| | | _/ __ \ / ___/\ __\/ _ \ / \_/ __ \ diff --git a/models/issue.go b/models/issue.go index d309c11853707..6720ec73636be 100644 --- a/models/issue.go +++ b/models/issue.go @@ -894,7 +894,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { for _, label := range labels { // Silently drop invalid labels. - if label.RepoID != opts.Repo.ID && label.OrgID != opts.Repo.Owner.ID { + if label.RepoID != opts.Repo.ID && label.OrgID != opts.Repo.OwnerID { continue } @@ -1057,6 +1057,8 @@ type IssuesOptions struct { IssueIDs []int64 // prioritize issues from this repo PriorityRepoID int64 + //Search for issues that contain any of the label ids above rather than all + HasAnyLabel bool } // sortIssuesSession sort an issues-related session based on the provided @@ -1144,12 +1146,19 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } if opts.LabelIDs != nil { - for i, labelID := range opts.LabelIDs { - if labelID > 0 { - sess.Join("INNER", fmt.Sprintf("issue_label il%d", i), - fmt.Sprintf("issue.id = il%[1]d.issue_id AND il%[1]d.label_id = %[2]d", i, labelID)) - } else { - sess.Where("issue.id not in (select issue_id from issue_label where label_id = ?)", -labelID) + // API Issues Search + if opts.HasAnyLabel { + sess.Join("INNER", []string{"issue_label", "il"}, "il.issue_id = issue.id"). + In("il.label_id", opts.LabelIDs) + } else { + // Repo Issue List Search + for i, labelID := range opts.LabelIDs { + if labelID > 0 { + sess.Join("INNER", fmt.Sprintf("issue_label il%d", i), + fmt.Sprintf("issue.id = il%[1]d.issue_id AND il%[1]d.label_id = %[2]d", i, labelID)) + } else { + sess.Where("issue.id not in (select issue_id from issue_label where label_id = ?)", -labelID) + } } } } diff --git a/models/issue_label.go b/models/issue_label.go index b569032b1d2c3..b48a6b4d85050 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -107,6 +107,20 @@ func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64) label.QueryString = strings.Join(labelQuerySlice, ",") } +func (label *Label) BelongsToOrg() bool { + if label.OrgID > 0 { + return true + } + return false +} + +func (label *Label) BelongsToRepo() bool { + if label.RepoID > 0 { + return true + } + return false +} + // ForegroundColor calculates the text color for labels based // on their background color. func (label *Label) ForegroundColor() template.CSS { @@ -127,6 +141,8 @@ func (label *Label) ForegroundColor() template.CSS { return template.CSS("#000") } +// LABEL + func loadLabels(labelTemplate string) ([]string, error) { list, err := GetLabelTemplateFile(labelTemplate) if err != nil { @@ -183,7 +199,7 @@ func newLabel(e Engine, label *Label) error { return err } -// NewLabel creates a new label for a repository +// NewLabel creates a new label func NewLabel(label *Label) error { if !LabelColorPattern.MatchString(label.Color) { return fmt.Errorf("bad color code: %s", label.Color) @@ -191,7 +207,7 @@ func NewLabel(label *Label) error { return newLabel(x, label) } -// NewLabels creates new labels for a repository. +// NewLabels creates new labels func NewLabels(labels ...*Label) error { sess := x.NewSession() defer sess.Close() @@ -209,11 +225,91 @@ func NewLabels(labels ...*Label) error { return sess.Commit() } +// UpdateLabel updates label information. +func UpdateLabel(l *Label) error { + if !LabelColorPattern.MatchString(l.Color) { + return fmt.Errorf("bad color code: %s", l.Color) + } + return updateLabel(x, l) +} + +// DeleteLabel delete a label +func DeleteLabel(id, labelID int64) error { + + label, err := GetLabelByID(labelID) + if err != nil { + if IsErrLabelNotExist(err) { + return nil + } + return err + } + + sess := x.NewSession() + defer sess.Close() + if err = sess.Begin(); err != nil { + return err + } + + if label.BelongsToOrg() { + sess.And("org_id = ?", id) + } else { + sess.And("repo_id = ?", id) + } + + if _, err = sess.ID(labelID).Delete(new(Label)); err != nil { + return err + } else if _, err = sess. + Where("label_id = ?", labelID). + Delete(new(IssueLabel)); err != nil { + return err + } + + // Clear label id in comment table //FIX LOOK INTO + if _, err = sess.Where("label_id = ?", labelID).Cols("label_id").Update(&Comment{}); err != nil { + return err + } + + return sess.Commit() +} + +// getLabelByID returns a label by label id +func getLabelByID(e Engine, labelID int64) (*Label, error) { + if labelID <= 0 { + return nil, ErrLabelNotExist{labelID} + } + + l := &Label{ + ID: labelID, + } + has, err := e.Get(l) + if err != nil { + return nil, err + } else if !has { + return nil, ErrLabelNotExist{l.ID} + } + return l, nil +} + +// GetLabelByID returns a label by given ID. +func GetLabelByID(id int64) (*Label, error) { + return getLabelByID(x, id) +} + +// GetLabelsByIDs returns a list of labels by IDs +func GetLabelsByIDs(labelIDs []int64) ([]*Label, error) { + labels := make([]*Label, 0, len(labelIDs)) + return labels, x.Table("label"). + In("id", labelIDs). + Asc("name"). + Cols("id"). + Find(&labels) +} + +// REPO + // getLabelInRepoByName returns a label by Name in given repository. -// If pass repoID as 0, then ORM will ignore limitation of repository -// and can return arbitrary label with any valid ID. func getLabelInRepoByName(e Engine, repoID int64, labelName string) (*Label, error) { - if len(labelName) == 0 { + if len(labelName) == 0 || repoID <= 0 { return nil, ErrRepoLabelNotExist{0, repoID} } @@ -231,10 +327,8 @@ func getLabelInRepoByName(e Engine, repoID int64, labelName string) (*Label, err } // getLabelInRepoByID returns a label by ID in given repository. -// If pass repoID as 0, then ORM will ignore limitation of repository -// and can return arbitrary label with any valid ID. func getLabelInRepoByID(e Engine, repoID, labelID int64) (*Label, error) { - if labelID <= 0 { + if labelID <= 0 || repoID <= 0 { return nil, ErrRepoLabelNotExist{labelID, repoID} } @@ -251,11 +345,6 @@ func getLabelInRepoByID(e Engine, repoID, labelID int64) (*Label, error) { return l, nil } -// GetLabelByID returns a label by given ID. -func GetLabelByID(id int64) (*Label, error) { - return getLabelInRepoByID(x, 0, id) -} - // GetLabelInRepoByName returns a label by name in given repository. func GetLabelInRepoByName(repoID int64, labelName string) (*Label, error) { return getLabelInRepoByName(x, repoID, labelName) @@ -274,13 +363,23 @@ func GetLabelIDsInRepoByNames(repoID int64, labelNames []string) ([]int64, error Find(&labelIDs) } -// GetLabelIDsInReposByNames returns a list of labelIDs by names in one of the given -// repositories. -// it silently ignores label names that do not belong to the repository. +// GetLabelIDsInReposByNames returns a list of labelIDs that are availabble to repositories +// based on name. This will check for organization labels the repo has access to as well +// and include them in the results func GetLabelIDsInReposByNames(repoIDs []int64, labelNames []string) ([]int64, error) { + + ownerIDs := make([]int64, 0, len(repoIDs)) + x.Table("repository"). + In("repository.id", repoIDs). + Cols("owner_id"). + Distinct("owner_id"). + Join("INNER", "org_user", "`org_user`.org_id = `repository`.owner_id"). + Find(&ownerIDs) + labelIDs := make([]int64, 0, len(labelNames)) return labelIDs, x.Table("label"). In("repo_id", repoIDs). + Or(builder.In("org_id", ownerIDs)). In("name", labelNames). Asc("name"). Cols("id"). @@ -304,6 +403,9 @@ func GetLabelsInRepoByIDs(repoID int64, labelIDs []int64) ([]*Label, error) { } func getLabelsByRepoID(e Engine, repoID int64, sortType string, listOptions ListOptions) ([]*Label, error) { + if repoID <= 0 { + return nil, ErrRepoLabelNotExist{0, repoID} + } labels := make([]*Label, 0, 10) sess := e.Where("repo_id = ?", repoID) @@ -339,10 +441,7 @@ func GetLabelsByRepoID(repoID int64, sortType string, listOptions ListOptions) ( // getLabelInOrgByName returns a label by Name in given organization func getLabelInOrgByName(e Engine, orgID int64, labelName string) (*Label, error) { - if len(labelName) == 0 { - return nil, ErrOrgLabelNotExist{0, orgID} - } - if orgID <= 0 { + if len(labelName) == 0 || orgID <= 0 { return nil, ErrOrgLabelNotExist{0, orgID} } @@ -362,17 +461,14 @@ func getLabelInOrgByName(e Engine, orgID int64, labelName string) (*Label, error // getLabelInOrgByID returns a label by ID in given organization. func getLabelInOrgByID(e Engine, orgID, labelID int64) (*Label, error) { - if labelID <= 0 { - return nil, ErrOrgLabelNotExist{labelID, orgID} - } - if orgID <= 0 { + if labelID <= 0 || orgID <= 0 { return nil, ErrOrgLabelNotExist{labelID, orgID} } l := &Label{ - ID: labelID, + ID: labelID, RepoID: 0, - OrgID: orgID, + OrgID: orgID, } has, err := e.Get(l) if err != nil { @@ -463,6 +559,8 @@ func GetLabelsByOrgID(orgID int64, sortType string, listOptions ListOptions) ([] return getLabelsByOrgID(x, orgID, sortType, listOptions) } +//ISSUE + func getLabelsByIssueID(e Engine, issueID int64) ([]*Label, error) { var labels []*Label return labels, e.Where("issue_label.issue_id = ?", issueID). @@ -494,89 +592,6 @@ func updateLabel(e Engine, l *Label) error { return err } -// UpdateLabel updates label information. -func UpdateLabel(l *Label) error { - if !LabelColorPattern.MatchString(l.Color) { - return fmt.Errorf("bad color code: %s", l.Color) - } - return updateLabel(x, l) -} - -// DeleteLabel delete a label of given repository or organiization. -func DeleteLabel(id, labelID int64, isOrg bool) error { - var err error - if isOrg { - _, err = GetLabelInOrgByID(id, labelID) - if err != nil { - if IsErrRepoLabelNotExist(err) { - return nil - } - return err - } - } else { - _, err = GetLabelInRepoByID(id, labelID) - if err != nil { - if IsErrRepoLabelNotExist(err) { - return nil - } - return err - } - } - - sess := x.NewSession() - defer sess.Close() - if err = sess.Begin(); err != nil { - return err - } - - if _, err = sess.ID(labelID).Delete(new(Label)); err != nil { - return err - } else if _, err = sess. - Where("label_id = ?", labelID). - Delete(new(IssueLabel)); err != nil { - return err - } - - // Clear label id in comment table - if _, err = sess.Where("label_id = ?", labelID).Cols("label_id").Update(&Comment{}); err != nil { - return err - } - - return sess.Commit() -} - -// DeleteOrgLabel delete a label of given organization. -func DeleteOrgLabel(orgID, labelID int64) error { - _, err := GetLabelInOrgByID(orgID, labelID) - if err != nil { - if IsErrOrgLabelNotExist(err) { - return nil - } - return err - } - - sess := x.NewSession() - defer sess.Close() - if err = sess.Begin(); err != nil { - return err - } - - if _, err = sess.ID(labelID).Delete(new(Label)); err != nil { - return err - } else if _, err = sess. - Where("label_id = ?", labelID). - Delete(new(IssueLabel)); err != nil { - return err - } - - // Clear label id in comment table - if _, err = sess.Where("label_id = ?", labelID).Cols("label_id").Update(&Comment{}); err != nil { - return err - } - - return sess.Commit() -} - // .___ .____ ___. .__ // | | ______ ________ __ ____ | | _____ \_ |__ ____ | | // | |/ ___// ___/ | \_/ __ \| | \__ \ | __ \_/ __ \| | diff --git a/models/issue_label_test.go b/models/issue_label_test.go index 4dc1be64a3b93..9f395ca6fb3ab 100644 --- a/models/issue_label_test.go +++ b/models/issue_label_test.go @@ -55,7 +55,7 @@ func TestGetLabelByID(t *testing.T) { assert.EqualValues(t, 1, label.ID) _, err = GetLabelByID(NonexistentID) - assert.True(t, IsErrRepoLabelNotExist(err)) + assert.True(t, IsErrLabelNotExist(err)) } func TestGetLabelInRepoByName(t *testing.T) { @@ -263,13 +263,13 @@ func TestUpdateLabel(t *testing.T) { func TestDeleteLabel(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) label := AssertExistsAndLoadBean(t, &Label{ID: 1}).(*Label) - assert.NoError(t, DeleteLabel(label.RepoID, label.ID, false)) + 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, false)) - AssertNotExistsBean(t, &Label{ID: label.ID, RepoID: label.RepoID}) + assert.NoError(t, DeleteLabel(label.RepoID, label.ID)) + AssertNotExistsBean(t, &Label{ID: label.ID}) - assert.NoError(t, DeleteLabel(NonexistentID, NonexistentID, false)) + assert.NoError(t, DeleteLabel(NonexistentID, NonexistentID)) CheckConsistencyFor(t, &Label{}, &Repository{}) } diff --git a/routers/api/v1/org/label.go b/routers/api/v1/org/label.go index 13d3eaf0f0b2f..c5fb262a309a1 100644 --- a/routers/api/v1/org/label.go +++ b/routers/api/v1/org/label.go @@ -1,5 +1,4 @@ -// Copyright 2016 The Gogs Authors. All rights reserved. -// Copyright 2018 The Gitea Authors. All rights reserved. +// Copyright 2020 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. @@ -229,7 +228,7 @@ func DeleteLabel(ctx *context.APIContext) { // "204": // "$ref": "#/responses/empty" - if err := models.DeleteLabel(ctx.Org.Organization.ID, ctx.ParamsInt64(":id"), true); err != nil { + if err := models.DeleteLabel(ctx.Org.Organization.ID, ctx.ParamsInt64(":id")); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteLabel", err) return } diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 2f08ba6ea60f1..34e46aa7b61c7 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -161,6 +161,7 @@ func SearchIssues(ctx *context.APIContext) { SortType: "priorityrepo", PriorityRepoID: ctx.QueryInt64("priority_repo_id"), IsPull: isPull, + HasAnyLabel: true, }) } diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index ea51e77d47aa1..8b2a1988fab40 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -171,12 +171,12 @@ func DeleteIssueLabel(ctx *context.APIContext) { return } - label, err := models.GetLabelInRepoByID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")) + label, err := models.GetLabelByID(ctx.ParamsInt64(":id")) if err != nil { - if models.IsErrRepoLabelNotExist(err) { + if models.IsErrLabelNotExist(err) { ctx.Error(http.StatusUnprocessableEntity, "", err) } else { - ctx.Error(http.StatusInternalServerError, "GetLabelInRepoByID", err) + ctx.Error(http.StatusInternalServerError, "GetLabelByID", err) } return } @@ -308,9 +308,9 @@ func prepareForReplaceOrAdd(ctx *context.APIContext, form api.IssueLabelsOption) return } - labels, err = models.GetLabelsInRepoByIDs(ctx.Repo.Repository.ID, form.Labels) + labels, err = models.GetLabelsByIDs(form.Labels) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetLabelsInRepoByIDs", err) + ctx.Error(http.StatusInternalServerError, "GetLabelsByIDs", err) return } diff --git a/routers/api/v1/repo/label.go b/routers/api/v1/repo/label.go index 7f023ca429b11..5f70e744077cf 100644 --- a/routers/api/v1/repo/label.go +++ b/routers/api/v1/repo/label.go @@ -254,7 +254,7 @@ func DeleteLabel(ctx *context.APIContext) { // "204": // "$ref": "#/responses/empty" - if err := models.DeleteLabel(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id"), false); err != nil { + if err := models.DeleteLabel(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteLabel", err) return } diff --git a/routers/org/org_labels.go b/routers/org/org_labels.go index 7be26e94719e5..e5b9d9ddeef8a 100644 --- a/routers/org/org_labels.go +++ b/routers/org/org_labels.go @@ -51,7 +51,7 @@ func NewLabel(ctx *context.Context, form auth.CreateLabelForm) { // UpdateLabel update a label's name and color func UpdateLabel(ctx *context.Context, form auth.CreateLabelForm) { - l, err := models.GetLabelByID(form.ID) + l, err := models.GetLabelInOrgByID(ctx.Org.Organization.ID, form.ID) if err != nil { switch { case models.IsErrOrgLabelNotExist(err): @@ -74,7 +74,7 @@ func UpdateLabel(ctx *context.Context, form auth.CreateLabelForm) { // DeleteLabel delete a label func DeleteLabel(ctx *context.Context) { - if err := models.DeleteLabel(ctx.Org.Organization.ID, ctx.QueryInt64("id"), true); err != nil { + if err := models.DeleteLabel(ctx.Org.Organization.ID, ctx.QueryInt64("id")); err != nil { ctx.Flash.Error("DeleteLabel: " + err.Error()) } else { ctx.Flash.Success(ctx.Tr("repo.issues.label_deletion_success")) diff --git a/routers/repo/issue_label.go b/routers/repo/issue_label.go index a3bb19c3c281e..602573ac25a66 100644 --- a/routers/repo/issue_label.go +++ b/routers/repo/issue_label.go @@ -120,7 +120,7 @@ func NewLabel(ctx *context.Context, form auth.CreateLabelForm) { // UpdateLabel update a label's name and color func UpdateLabel(ctx *context.Context, form auth.CreateLabelForm) { - l, err := models.GetLabelByID(form.ID) + l, err := models.GetLabelInRepoByID(ctx.Repo.Repository.ID, form.ID) if err != nil { switch { case models.IsErrRepoLabelNotExist(err): @@ -143,7 +143,7 @@ func UpdateLabel(ctx *context.Context, form auth.CreateLabelForm) { // DeleteLabel delete a label func DeleteLabel(ctx *context.Context) { - if err := models.DeleteLabel(ctx.Repo.Repository.ID, ctx.QueryInt64("id"), false); err != nil { + if err := models.DeleteLabel(ctx.Repo.Repository.ID, ctx.QueryInt64("id")); err != nil { ctx.Flash.Error("DeleteLabel: " + err.Error()) } else { ctx.Flash.Success(ctx.Tr("repo.issues.label_deletion_success")) diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 4df49d9c8ba5a..a53bbdc685aba 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -38,7 +38,7 @@ {{template "repo/issue/branch_selector_field" .}} -
    +
    {{.i18n.Tr "repo.issues.new.labels"}} {{svg "octicon-gear" 16}} From 835ee9504b276759a126bbe32fed1ad02207ea9b Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sat, 28 Mar 2020 18:08:49 -0400 Subject: [PATCH 17/41] More descriptive translation var per code review --- options/locale/locale_en-US.ini | 5 +++-- templates/repo/issue/labels/label_list.tmpl | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ea680e17d7b84..92589a0066984 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -725,8 +725,9 @@ tags = Tags issues = Issues pulls = Pull Requests labels = Labels -org.labels = Organization level labels that can be used with all repositories under this organization -org.labels.manage = manage +org_labels_desc = Organization level labels that can be used with all repositories under this organization +org_labels_desc_manage = manage + milestones = Milestones commits = Commits commit = Commit diff --git a/templates/repo/issue/labels/label_list.tmpl b/templates/repo/issue/labels/label_list.tmpl index 51e1016e6feb0..d15dd4467a4cb 100644 --- a/templates/repo/issue/labels/label_list.tmpl +++ b/templates/repo/issue/labels/label_list.tmpl @@ -61,9 +61,9 @@
  • - {{$.i18n.Tr "repo.org.labels" | Str2html}} + {{$.i18n.Tr "repo.org_labels_desc" | Str2html}} {{if .IsOrganizationOwner}} - ({{$.i18n.Tr "repo.org.labels.manage"}}): + ({{$.i18n.Tr "repo.org_labels_desc_manage"}}): {{end}}
    From 503998b935529bef0cd9aa3bd5de9217fcb4bf9a Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sat, 28 Mar 2020 18:45:23 -0400 Subject: [PATCH 18/41] func description/delete comment when issue label deleted instead of hiding it --- models/issue_label.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/issue_label.go b/models/issue_label.go index b48a6b4d85050..b51476e13a764 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -107,6 +107,7 @@ func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64) label.QueryString = strings.Join(labelQuerySlice, ",") } +// BelongsToOrg returns true if label is an organization label func (label *Label) BelongsToOrg() bool { if label.OrgID > 0 { return true @@ -114,6 +115,7 @@ func (label *Label) BelongsToOrg() bool { return false } +// BelongsToRepo returns true if label is a repository label func (label *Label) BelongsToRepo() bool { if label.RepoID > 0 { return true @@ -265,7 +267,7 @@ func DeleteLabel(id, labelID int64) error { } // Clear label id in comment table //FIX LOOK INTO - if _, err = sess.Where("label_id = ?", labelID).Cols("label_id").Update(&Comment{}); err != nil { + if _, err = sess.Where("label_id = ?", labelID).Cols("label_id").Delete(&Comment{}); err != nil { return err } From 007ff15b5c33d8a7e480e7ee7280024b4bdcf7d0 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sat, 28 Mar 2020 18:47:28 -0400 Subject: [PATCH 19/41] remove comment --- models/issue_label.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_label.go b/models/issue_label.go index b51476e13a764..608d55690795c 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -266,7 +266,7 @@ func DeleteLabel(id, labelID int64) error { return err } - // Clear label id in comment table //FIX LOOK INTO + // delete comments about now deleted label_id if _, err = sess.Where("label_id = ?", labelID).Cols("label_id").Delete(&Comment{}); err != nil { return err } From e4714fe9ae13f8ac5fb310fae294fe1789e04376 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sat, 28 Mar 2020 20:23:10 -0400 Subject: [PATCH 20/41] only use issues in that repo when calculating number of open issues for org label on repo label page --- models/issue_label.go | 49 +++++++++++++++------ routers/repo/compare.go | 2 - routers/repo/issue_label.go | 2 +- templates/org/settings/navbar.tmpl | 2 +- templates/repo/issue/labels/label_list.tmpl | 2 +- 5 files changed, 38 insertions(+), 19 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 608d55690795c..f4141763dcdf7 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -21,19 +21,20 @@ var LabelColorPattern = regexp.MustCompile("^#[0-9a-fA-F]{6}$") // Label represents a label of repository for issues. type Label struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX"` - OrgID int64 `xorm:"INDEX"` - Name string - Description string - Color string `xorm:"VARCHAR(7)"` - NumIssues int - NumClosedIssues int - NumOpenIssues int `xorm:"-"` - IsChecked bool `xorm:"-"` - QueryString string `xorm:"-"` - IsSelected bool `xorm:"-"` - IsExcluded bool `xorm:"-"` + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX"` + OrgID int64 `xorm:"INDEX"` + Name string + Description string + Color string `xorm:"VARCHAR(7)"` + NumIssues int + NumClosedIssues int + NumOpenIssues int `xorm:"-"` + NumOpenRepoIssues int64 `xorm:"-"` + IsChecked bool `xorm:"-"` + QueryString string `xorm:"-"` + IsSelected bool `xorm:"-"` + IsExcluded bool `xorm:"-"` } // GetLabelTemplateFile loads the label template file by given name, @@ -80,11 +81,31 @@ func GetLabelTemplateFile(name string) ([][3]string, error) { return list, nil } -// CalOpenIssues calculates the open issues of label. +// CalOpenIssues returns the open issues of label. func (label *Label) CalOpenIssues() { label.NumOpenIssues = label.NumIssues - label.NumClosedIssues } +// CalOpenOrgIssues calculates the open issues of a label for a specific repo +func (label *Label) CalOpenOrgIssues(repoID, labelID int64) { + repoIDs := []int64{repoID} + labelIDs := []int64{labelID} + + counts, _ := CountIssuesByRepo(&IssuesOptions{ + RepoIDs: repoIDs, + LabelIDs: labelIDs, + }) + + for _, count := range counts { + label.NumOpenRepoIssues = count + } +} + +// CalOpenIssuesInRepo calculates the open issues of label inside a repo. +func (label *Label) CalOpenIssuesInRepo(repoID int64) { + label.NumOpenIssues = label.NumIssues - label.NumClosedIssues +} + // LoadSelectedLabelsAfterClick calculates the set of selected labels when a label is clicked func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64) { var labelQuerySlice []string diff --git a/routers/repo/compare.go b/routers/repo/compare.go index 4769a4da1aa78..87b66dc7fb539 100644 --- a/routers/repo/compare.go +++ b/routers/repo/compare.go @@ -335,7 +335,6 @@ func PrepareCompareDiff( } else { title = headBranch } - ctx.Data["title"] = title ctx.Data["Username"] = headUser.Name ctx.Data["Reponame"] = headRepo.Name @@ -422,7 +421,6 @@ func CompareDiff(ctx *context.Context) { beforeCommitID := ctx.Data["BeforeCommitID"].(string) afterCommitID := ctx.Data["AfterCommitID"].(string) - ctx.Data["Title"] = "Comparing " + base.ShortSha(beforeCommitID) + "..." + base.ShortSha(afterCommitID) ctx.Data["IsRepoToolbarCommits"] = true diff --git a/routers/repo/issue_label.go b/routers/repo/issue_label.go index 602573ac25a66..16638404e380a 100644 --- a/routers/repo/issue_label.go +++ b/routers/repo/issue_label.go @@ -70,7 +70,7 @@ func RetrieveLabels(ctx *context.Context) { return } for _, l := range orgLabels { - l.CalOpenIssues() + l.CalOpenOrgIssues(ctx.Repo.Repository.ID, l.ID) } ctx.Data["OrgLabels"] = orgLabels diff --git a/templates/org/settings/navbar.tmpl b/templates/org/settings/navbar.tmpl index b1804b9833bde..63114b056ed42 100644 --- a/templates/org/settings/navbar.tmpl +++ b/templates/org/settings/navbar.tmpl @@ -7,7 +7,7 @@ {{.i18n.Tr "repo.settings.hooks"}} - + {{.i18n.Tr "repo.labels"}} diff --git a/templates/repo/issue/labels/label_list.tmpl b/templates/repo/issue/labels/label_list.tmpl index d15dd4467a4cb..ce33d76fc1e54 100644 --- a/templates/repo/issue/labels/label_list.tmpl +++ b/templates/repo/issue/labels/label_list.tmpl @@ -82,7 +82,7 @@
    • - {{svg "octicon-issue-opened" 16}} {{$.i18n.Tr "repo.issues.label_open_issues" .NumOpenIssues}} + {{svg "octicon-issue-opened" 16}} {{$.i18n.Tr "repo.issues.label_open_issues" .NumOpenRepoIssues}}
    From 70529e2f62648f7c0d9d0b5e9fbee51b92f08584 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sun, 29 Mar 2020 01:28:58 -0400 Subject: [PATCH 21/41] Add integration test for IssuesSearch API with labels --- integrations/api_issue_test.go | 54 ++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/integrations/api_issue_test.go b/integrations/api_issue_test.go index 30fbda173c0b4..9873dbcc3e047 100644 --- a/integrations/api_issue_test.go +++ b/integrations/api_issue_test.go @@ -173,3 +173,57 @@ func TestAPISearchIssue(t *testing.T) { DecodeJSON(t, resp, &apiIssues) assert.Len(t, apiIssues, 1) } + +func TestAPISearchIssueWithLabel(t *testing.T) { + defer prepareTestEnv(t)() + + session := loginUser(t, "user1") + token := getTokenForLoggedInUser(t, session) + + link, _ := url.Parse("/api/v1/repos/issues/search") + req := NewRequest(t, "GET", link.String()) + resp := session.MakeRequest(t, req, http.StatusOK) + var apiIssues []*api.Issue + DecodeJSON(t, resp, &apiIssues) + + assert.Len(t, apiIssues, 9) + + query := url.Values{} + query.Add("token", token) + link.RawQuery = query.Encode() + req = NewRequest(t, "GET", link.String()) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiIssues) + assert.Len(t, apiIssues, 9) + + query.Add("labels", "label1") + link.RawQuery = query.Encode() + req = NewRequest(t, "GET", link.String()) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiIssues) + assert.Len(t, apiIssues, 2) + + // multiple labels + query.Add("labels", "label1%2Clabel2") + link.RawQuery = query.Encode() + req = NewRequest(t, "GET", link.String()) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiIssues) + assert.Len(t, apiIssues, 2) + + // an org label + query.Add("labels", "orglabel4") + link.RawQuery = query.Encode() + req = NewRequest(t, "GET", link.String()) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiIssues) + assert.Len(t, apiIssues, 2) + + // org and repo label + query.Add("labels", "label1%2Corglabel4") + link.RawQuery = query.Encode() + req = NewRequest(t, "GET", link.String()) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiIssues) + assert.Len(t, apiIssues, 2) +} From 2604c39cafbc35bcaec229e09c09edacf974573d Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sun, 29 Mar 2020 01:38:29 -0400 Subject: [PATCH 22/41] remove unused function --- models/issue_label.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index f4141763dcdf7..9f335a1f4111d 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -101,11 +101,6 @@ func (label *Label) CalOpenOrgIssues(repoID, labelID int64) { } } -// CalOpenIssuesInRepo calculates the open issues of label inside a repo. -func (label *Label) CalOpenIssuesInRepo(repoID int64) { - label.NumOpenIssues = label.NumIssues - label.NumClosedIssues -} - // LoadSelectedLabelsAfterClick calculates the set of selected labels when a label is clicked func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64) { var labelQuerySlice []string From 72b4c5c5d9b67c253ebfe83b9f41e7804fd0b893 Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Sun, 29 Mar 2020 01:56:42 -0400 Subject: [PATCH 23/41] Update models/issue_label.go Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com> --- models/issue_label.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_label.go b/models/issue_label.go index 9f335a1f4111d..455efc827ce03 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -381,7 +381,7 @@ func GetLabelIDsInRepoByNames(repoID int64, labelNames []string) ([]int64, error Find(&labelIDs) } -// GetLabelIDsInReposByNames returns a list of labelIDs that are availabble to repositories +// GetLabelIDsInReposByNames returns a list of labelIDs that are available to repositories // based on name. This will check for organization labels the repo has access to as well // and include them in the results func GetLabelIDsInReposByNames(repoIDs []int64, labelNames []string) ([]int64, error) { From 8c1f393cdab26a8f50935090b91d7fa016f75ff8 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sun, 29 Mar 2020 03:11:05 -0400 Subject: [PATCH 24/41] Use subquery in GetLabelIDsInReposByNames --- models/issue_label.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 455efc827ce03..89af4f2035f3f 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -385,22 +385,19 @@ func GetLabelIDsInRepoByNames(repoID int64, labelNames []string) ([]int64, error // based on name. This will check for organization labels the repo has access to as well // and include them in the results func GetLabelIDsInReposByNames(repoIDs []int64, labelNames []string) ([]int64, error) { - - ownerIDs := make([]int64, 0, len(repoIDs)) - x.Table("repository"). - In("repository.id", repoIDs). - Cols("owner_id"). - Distinct("owner_id"). - Join("INNER", "org_user", "`org_user`.org_id = `repository`.owner_id"). - Find(&ownerIDs) + var subQuery = builder. + Select("org_id"). + From("org_user"). + Join("INNER", "repository", "org_user.org_id = repository.owner_id"). + Where(builder.In("repository.id", repoIDs)) labelIDs := make([]int64, 0, len(labelNames)) return labelIDs, x.Table("label"). In("repo_id", repoIDs). - Or(builder.In("org_id", ownerIDs)). + Or(builder.In("org_id", subQuery)). In("name", labelNames). Asc("name"). - Cols("id"). + Cols("label.id"). Find(&labelIDs) } From a8517abfd16791c76844927dae4e21f4287f53b0 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sun, 29 Mar 2020 04:49:32 -0400 Subject: [PATCH 25/41] Fix tests to use correct orgID --- integrations/api_issue_test.go | 10 +++++----- models/fixtures/label.yml | 4 ++-- models/issue_label_test.go | 22 +++++++++++----------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/integrations/api_issue_test.go b/integrations/api_issue_test.go index 9873dbcc3e047..666c4bf6c93f2 100644 --- a/integrations/api_issue_test.go +++ b/integrations/api_issue_test.go @@ -204,7 +204,7 @@ func TestAPISearchIssueWithLabel(t *testing.T) { assert.Len(t, apiIssues, 2) // multiple labels - query.Add("labels", "label1%2Clabel2") + query.Set("labels", "label1,label2") link.RawQuery = query.Encode() req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) @@ -212,18 +212,18 @@ func TestAPISearchIssueWithLabel(t *testing.T) { assert.Len(t, apiIssues, 2) // an org label - query.Add("labels", "orglabel4") + query.Set("labels", "orglabel4") link.RawQuery = query.Encode() req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 2) + assert.Len(t, apiIssues, 1) // org and repo label - query.Add("labels", "label1%2Corglabel4") + query.Set("labels", "label1,orglabel4") link.RawQuery = query.Encode() req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 2) + assert.Len(t, apiIssues, 3) } diff --git a/models/fixtures/label.yml b/models/fixtures/label.yml index b35203baff0f3..2af577ef5b946 100644 --- a/models/fixtures/label.yml +++ b/models/fixtures/label.yml @@ -18,7 +18,7 @@ - id: 3 repo_id: 0 - org_id: 1 + org_id: 3 name: orglabel3 color: '#abcdef' num_issues: 0 @@ -27,7 +27,7 @@ - id: 4 repo_id: 0 - org_id: 1 + org_id: 3 name: orglabel4 color: '#000000' num_issues: 1 diff --git a/models/issue_label_test.go b/models/issue_label_test.go index 9f395ca6fb3ab..8afba779e0135 100644 --- a/models/issue_label_test.go +++ b/models/issue_label_test.go @@ -139,12 +139,12 @@ func TestGetLabelsByRepoID(t *testing.T) { func TestGetLabelInOrgByName(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - label, err := GetLabelInOrgByName(1, "orglabel3") + label, err := GetLabelInOrgByName(3, "orglabel3") assert.NoError(t, err) assert.EqualValues(t, 3, label.ID) assert.Equal(t, "orglabel3", label.Name) - _, err = GetLabelInOrgByName(1, "") + _, err = GetLabelInOrgByName(3, "") assert.True(t, IsErrOrgLabelNotExist(err)) _, err = GetLabelInOrgByName(0, "orglabel3") @@ -159,7 +159,7 @@ func TestGetLabelInOrgByName(t *testing.T) { func TestGetLabelInOrgByNames(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - labelIDs, err := GetLabelIDsInOrgByNames(1, []string{"orglabel3", "orglabel4"}) + labelIDs, err := GetLabelIDsInOrgByNames(3, []string{"orglabel3", "orglabel4"}) assert.NoError(t, err) assert.Len(t, labelIDs, 2) @@ -171,7 +171,7 @@ func TestGetLabelInOrgByNames(t *testing.T) { func TestGetLabelInOrgByNamesDiscardsNonExistentLabels(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) // orglabel99 doesn't exists.. See labels.yml - labelIDs, err := GetLabelIDsInOrgByNames(1, []string{"orglabel3", "orglabel4", "orglabel99"}) + labelIDs, err := GetLabelIDsInOrgByNames(3, []string{"orglabel3", "orglabel4", "orglabel99"}) assert.NoError(t, err) assert.Len(t, labelIDs, 2) @@ -183,11 +183,11 @@ func TestGetLabelInOrgByNamesDiscardsNonExistentLabels(t *testing.T) { func TestGetLabelInOrgByID(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - label, err := GetLabelInOrgByID(1, 3) + label, err := GetLabelInOrgByID(3, 3) assert.NoError(t, err) assert.EqualValues(t, 3, label.ID) - _, err = GetLabelInOrgByID(1, -1) + _, err = GetLabelInOrgByID(3, -1) assert.True(t, IsErrOrgLabelNotExist(err)) _, err = GetLabelInOrgByID(0, 3) @@ -202,7 +202,7 @@ func TestGetLabelInOrgByID(t *testing.T) { func TestGetLabelsInOrgByIDs(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - labels, err := GetLabelsInOrgByIDs(1, []int64{3, 4, NonexistentID}) + labels, err := GetLabelsInOrgByIDs(3, []int64{3, 4, NonexistentID}) assert.NoError(t, err) if assert.Len(t, labels, 2) { assert.EqualValues(t, 3, labels[0].ID) @@ -220,10 +220,10 @@ func TestGetLabelsByOrgID(t *testing.T) { assert.EqualValues(t, expectedIssueIDs[i], label.ID) } } - testSuccess(1, "leastissues", []int64{3, 4}) - testSuccess(1, "mostissues", []int64{4, 3}) - testSuccess(1, "reversealphabetically", []int64{4, 3}) - testSuccess(1, "default", []int64{3, 4}) + testSuccess(3, "leastissues", []int64{3, 4}) + testSuccess(3, "mostissues", []int64{4, 3}) + testSuccess(3, "reversealphabetically", []int64{4, 3}) + testSuccess(3, "default", []int64{3, 4}) var err error _, err = GetLabelsByOrgID(0, "leastissues", ListOptions{}) From e5809c4f8b62dec3434046f8173c405f0f70becb Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Sun, 29 Mar 2020 05:06:57 -0400 Subject: [PATCH 26/41] fix more tests --- integrations/api_issue_label_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/api_issue_label_test.go b/integrations/api_issue_label_test.go index b9e5066ad6778..ddcfdd6615892 100644 --- a/integrations/api_issue_label_test.go +++ b/integrations/api_issue_label_test.go @@ -175,7 +175,7 @@ func TestAPIModifyOrgLabels(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusOK) var apiLabels []*api.Label DecodeJSON(t, resp, &apiLabels) - assert.Len(t, apiLabels, 2) + assert.Len(t, apiLabels, 4) //GetLabel singleURLStr := fmt.Sprintf("/api/v1/orgs/%s/labels/%d?token=%s", owner.Name, dbLabel.ID, token) From a01f7a9dfb8fa70d45123647ae147650c458da97 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Mon, 30 Mar 2020 09:53:27 -0400 Subject: [PATCH 27/41] IssuesSearch api now uses new BuildLabelNamesIssueIDsCondition. Add a few more tests as well --- integrations/api_issue_test.go | 15 ++++++++++++--- models/issue_label.go | 20 -------------------- routers/api/v1/repo/issue.go | 5 ++--- 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/integrations/api_issue_test.go b/integrations/api_issue_test.go index 666c4bf6c93f2..2a6f137747991 100644 --- a/integrations/api_issue_test.go +++ b/integrations/api_issue_test.go @@ -130,7 +130,7 @@ func TestAPIEditIssue(t *testing.T) { assert.Equal(t, title, issueAfter.Title) } -func TestAPISearchIssue(t *testing.T) { +func TestAPISearchIssues(t *testing.T) { defer prepareTestEnv(t)() session := loginUser(t, "user2") @@ -174,7 +174,7 @@ func TestAPISearchIssue(t *testing.T) { assert.Len(t, apiIssues, 1) } -func TestAPISearchIssueWithLabel(t *testing.T) { +func TestAPISearchIssuesWithLabels(t *testing.T) { defer prepareTestEnv(t)() session := loginUser(t, "user1") @@ -220,10 +220,19 @@ func TestAPISearchIssueWithLabel(t *testing.T) { assert.Len(t, apiIssues, 1) // org and repo label + query.Set("labels", "label2,orglabel4") + query.Add("state", "all") + link.RawQuery = query.Encode() + req = NewRequest(t, "GET", link.String()) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiIssues) + assert.Len(t, apiIssues, 2) + + // org and repo label which share the same issue query.Set("labels", "label1,orglabel4") link.RawQuery = query.Encode() req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 3) + assert.Len(t, apiIssues, 2) } diff --git a/models/issue_label.go b/models/issue_label.go index 30eda0ac4f813..2868c68368330 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -392,26 +392,6 @@ func BuildLabelNamesIssueIDsCondition(labelNames []string) *builder.Builder { GroupBy("issue_label.issue_id") } -// GetLabelIDsInReposByNames returns a list of labelIDs that are available to repositories -// based on name. This will check for organization labels the repo has access to as well -// and include them in the results -func GetLabelIDsInReposByNames(repoIDs []int64, labelNames []string) ([]int64, error) { - var subQuery = builder. - Select("org_id"). - From("org_user"). - Join("INNER", "repository", "org_user.org_id = repository.owner_id"). - Where(builder.In("repository.id", repoIDs)) - - labelIDs := make([]int64, 0, len(labelNames)) - return labelIDs, x.Table("label"). - In("repo_id", repoIDs). - Or(builder.In("org_id", subQuery)). - In("name", labelNames). - Asc("name"). - Cols("label.id"). - Find(&labelIDs) -} - // GetLabelInRepoByID returns a label by ID in given repository. func GetLabelInRepoByID(repoID, labelID int64) (*Label, error) { return getLabelInRepoByID(x, repoID, labelID) diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index c90e46081ace7..ef2bd6310268f 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -152,7 +152,7 @@ func SearchIssues(ctx *context.APIContext) { Page: ctx.QueryInt("page"), PageSize: setting.UI.IssuePagingNum, }, - + RepoIDs: repoIDs, IsClosed: isClosed, IssueIDs: issueIDs, @@ -160,8 +160,7 @@ func SearchIssues(ctx *context.APIContext) { SortType: "priorityrepo", PriorityRepoID: ctx.QueryInt64("priority_repo_id"), IsPull: isPull, - HasAnyLabel: true, - + HasAnyLabel: true, }) } From 47d1b8ec9adc0c8b6f21cfcd97e233380bd93f03 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Mon, 30 Mar 2020 12:03:56 -0400 Subject: [PATCH 28/41] update comment for clarity --- models/issue.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/issue.go b/models/issue.go index 37ec6e5a4eddb..64fb5cae342a2 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1148,12 +1148,12 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } if opts.LabelIDs != nil { - // API Issues Search + // API Issues Search should return issues with any provided label if opts.HasAnyLabel { sess.Join("INNER", []string{"issue_label", "il"}, "il.issue_id = issue.id"). In("il.label_id", opts.LabelIDs) } else { - // Repo Issue List Search + // Repo Issue List Search only returns issues that have all provided labels for i, labelID := range opts.LabelIDs { if labelID > 0 { sess.Join("INNER", fmt.Sprintf("issue_label il%d", i), From d17c8c2adc30eae9533a8939e3d4d629a02d3a92 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Mon, 30 Mar 2020 12:11:20 -0400 Subject: [PATCH 29/41] Revert previous code change now that we can use the new BuildLabelNamesIssueIDsCondition --- models/issue.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/models/issue.go b/models/issue.go index 64fb5cae342a2..339e8ca9c9dc2 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1148,19 +1148,12 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } if opts.LabelIDs != nil { - // API Issues Search should return issues with any provided label - if opts.HasAnyLabel { - sess.Join("INNER", []string{"issue_label", "il"}, "il.issue_id = issue.id"). - In("il.label_id", opts.LabelIDs) - } else { - // Repo Issue List Search only returns issues that have all provided labels - for i, labelID := range opts.LabelIDs { - if labelID > 0 { - sess.Join("INNER", fmt.Sprintf("issue_label il%d", i), - fmt.Sprintf("issue.id = il%[1]d.issue_id AND il%[1]d.label_id = %[2]d", i, labelID)) - } else { - sess.Where("issue.id not in (select issue_id from issue_label where label_id = ?)", -labelID) - } + for i, labelID := range opts.LabelIDs { + if labelID > 0 { + sess.Join("INNER", fmt.Sprintf("issue_label il%d", i), + fmt.Sprintf("issue.id = il%[1]d.issue_id AND il%[1]d.label_id = %[2]d", i, labelID)) + } else { + sess.Where("issue.id not in (select issue_id from issue_label where label_id = ?)", -labelID) } } } From 59f0586bd46a410ec8b4e5c127e38dd5478029f8 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Mon, 30 Mar 2020 17:32:51 -0400 Subject: [PATCH 30/41] Don't sort repos by date in IssuesSearch API After much debugging I've found a strange issue where in some cases MySQL will return a different result than other enigines if a query is sorted by a null collumn. For example with our integration test data where we don't set updated_unix in repository fixtures: SELECT `id`, `owner_id`, `owner_name`, `lower_name`, `name`, `description`, `website`, `original_service_type`, `original_url`, `default_branch`, `num_watches`, `num_stars`, `num_forks`, `num_issues`, `num_closed_issues`, `num_pulls`, `num_closed_pulls`, `num_milestones`, `num_closed_milestones`, `is_private`, `is_empty`, `is_archived`, `is_mirror`, `status`, `is_fork`, `fork_id`, `is_template`, `template_id`, `size`, `is_fsck_enabled`, `close_issues_via_commit_in_any_branch`, `topics`, `avatar`, `created_unix`, `updated_unix` FROM `repository` ORDER BY updated_unix DESC LIMIT 15 OFFSET 45 Returns different results for MySQL than other engines. However, the similar query: SELECT `id`, `owner_id`, `owner_name`, `lower_name`, `name`, `description`, `website`, `original_service_type`, `original_url`, `default_branch`, `num_watches`, `num_stars`, `num_forks`, `num_issues`, `num_closed_issues`, `num_pulls`, `num_closed_pulls`, `num_milestones`, `num_closed_milestones`, `is_private`, `is_empty`, `is_archived`, `is_mirror`, `status`, `is_fork`, `fork_id`, `is_template`, `template_id`, `size`, `is_fsck_enabled`, `close_issues_via_commit_in_any_branch`, `topics`, `avatar`, `created_unix`, `updated_unix` FROM `repository` ORDER BY updated_unix DESC LIMIT 15 OFFSET 30 Returns the same results. This causes integration tests to fail on MySQL in certain cases but would never show up in a real installation. Since this API call always returns issues based on the optionally provided repo_priority_id or the issueID itself, there is no change to results by changing the repo sorting method used to get ids earlier in the function. --- routers/api/v1/repo/issue.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index ef2bd6310268f..d6a4e5274d15e 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -81,7 +81,9 @@ func SearchIssues(ctx *context.APIContext) { AllPublic: true, TopicOnly: false, Collaborate: util.OptionalBoolNone, - OrderBy: models.SearchOrderByRecentUpdated, + // This needs to be a column that is not nil in fixtures or + // MySQL will return different results when sorting by null in some cases + OrderBy: models.SearchOrderByAlphabetically, Actor: ctx.User, } if ctx.IsSigned { From a5bb0336ceab12832432492656c24932bfc60ddd Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Mon, 30 Mar 2020 17:54:57 -0400 Subject: [PATCH 31/41] linter is back! --- models/issue_label.go | 10 ++-------- routers/api/v1/repo/issue.go | 6 +++--- routers/repo/issue.go | 12 +++--------- 3 files changed, 8 insertions(+), 20 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 2868c68368330..892821703214c 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -125,18 +125,12 @@ func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64) // BelongsToOrg returns true if label is an organization label func (label *Label) BelongsToOrg() bool { - if label.OrgID > 0 { - return true - } - return false + return label.OrgID > 0 } // BelongsToRepo returns true if label is a repository label func (label *Label) BelongsToRepo() bool { - if label.RepoID > 0 { - return true - } - return false + return label.RepoID > 0 } // ForegroundColor calculates the text color for labels based diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index d6a4e5274d15e..f7f6445ce54aa 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -82,9 +82,9 @@ func SearchIssues(ctx *context.APIContext) { TopicOnly: false, Collaborate: util.OptionalBoolNone, // This needs to be a column that is not nil in fixtures or - // MySQL will return different results when sorting by null in some cases - OrderBy: models.SearchOrderByAlphabetically, - Actor: ctx.User, + // MySQL will return different results when sorting by null in some cases + OrderBy: models.SearchOrderByAlphabetically, + Actor: ctx.User, } if ctx.IsSigned { opts.Private = true diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 25e143db81a6e..6dbf9cf5c8639 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -268,9 +268,7 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB } ctx.Data["OrgLabels"] = orgLabels - for _, v := range orgLabels { - labels = append(labels, v) - } + labels = append(labels, orgLabels...) } for _, l := range labels { @@ -398,9 +396,7 @@ func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository, isPull boo } ctx.Data["OrgLabels"] = orgLabels - for _, v := range orgLabels { - labels = append(labels, v) - } + labels = append(labels, orgLabels...) } RetrieveRepoMilestonesAndAssignees(ctx, repo) @@ -797,9 +793,7 @@ func ViewIssue(ctx *context.Context) { } ctx.Data["OrgLabels"] = orgLabels - for _, v := range orgLabels { - labels = append(labels, v) - } + labels = append(labels, orgLabels...) } hasSelected := false From e4ed7ecf5b57f22421b168536b3d3e42664e2ba6 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 31 Mar 2020 00:36:09 -0400 Subject: [PATCH 32/41] code review --- models/issue_label.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 892821703214c..3e08c0943229e 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -446,9 +446,8 @@ func getLabelInOrgByName(e Engine, orgID int64, labelName string) (*Label, error } l := &Label{ - Name: labelName, - RepoID: 0, - OrgID: orgID, + Name: labelName, + OrgID: orgID, } has, err := e.Get(l) if err != nil { @@ -466,9 +465,8 @@ func getLabelInOrgByID(e Engine, orgID, labelID int64) (*Label, error) { } l := &Label{ - ID: labelID, - RepoID: 0, - OrgID: orgID, + ID: labelID, + OrgID: orgID, } has, err := e.Get(l) if err != nil { From 893650b0ec651e12f82d2615ee8f138ddd14eebc Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 31 Mar 2020 00:39:11 -0400 Subject: [PATCH 33/41] remove now unused option --- models/issue.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/models/issue.go b/models/issue.go index 339e8ca9c9dc2..db8991095dea7 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1059,8 +1059,6 @@ type IssuesOptions struct { IssueIDs []int64 // prioritize issues from this repo PriorityRepoID int64 - //Search for issues that contain any of the label ids above rather than all - HasAnyLabel bool } // sortIssuesSession sort an issues-related session based on the provided From 7e406222434c101907edf4c8791649960ca6b387 Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Tue, 31 Mar 2020 00:54:54 -0400 Subject: [PATCH 34/41] Fix newline at end of files --- models/fixtures/label.yml | 3 ++- templates/org/settings/labels.tmpl | 2 +- templates/repo/issue/labels/edit_delete_label.tmpl | 3 ++- templates/repo/issue/labels/label_load_template.tmpl | 2 +- templates/repo/issue/labels/label_new.tmpl | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/models/fixtures/label.yml b/models/fixtures/label.yml index 2af577ef5b946..3ad82eebedcf6 100644 --- a/models/fixtures/label.yml +++ b/models/fixtures/label.yml @@ -31,4 +31,5 @@ name: orglabel4 color: '#000000' num_issues: 1 - num_closed_issues: 0 \ No newline at end of file + num_closed_issues: 0 + diff --git a/templates/org/settings/labels.tmpl b/templates/org/settings/labels.tmpl index c1bff02ca5853..ed31890df5398 100644 --- a/templates/org/settings/labels.tmpl +++ b/templates/org/settings/labels.tmpl @@ -26,4 +26,4 @@ {{template "repo/issue/labels/edit_delete_label" .}} -{{template "base/footer" .}} \ No newline at end of file +{{template "base/footer" .}} diff --git a/templates/repo/issue/labels/edit_delete_label.tmpl b/templates/repo/issue/labels/edit_delete_label.tmpl index 6d7fb6b99254e..1ddfd39ee400a 100644 --- a/templates/repo/issue/labels/edit_delete_label.tmpl +++ b/templates/repo/issue/labels/edit_delete_label.tmpl @@ -55,4 +55,5 @@ - \ No newline at end of file + + diff --git a/templates/repo/issue/labels/label_load_template.tmpl b/templates/repo/issue/labels/label_load_template.tmpl index 299f22e61fd14..76ee77658a01b 100644 --- a/templates/repo/issue/labels/label_load_template.tmpl +++ b/templates/repo/issue/labels/label_load_template.tmpl @@ -27,4 +27,4 @@ - \ No newline at end of file + diff --git a/templates/repo/issue/labels/label_new.tmpl b/templates/repo/issue/labels/label_new.tmpl index aa14579c57ca8..15a6c029a3185 100644 --- a/templates/repo/issue/labels/label_new.tmpl +++ b/templates/repo/issue/labels/label_new.tmpl @@ -24,4 +24,4 @@ - \ No newline at end of file + From 235d42067f097a064d424c36e48e83d042db9bd0 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 31 Mar 2020 01:49:44 -0400 Subject: [PATCH 35/41] more unused code --- routers/api/v1/repo/issue.go | 1 - 1 file changed, 1 deletion(-) diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index f7f6445ce54aa..217c97c69b1ca 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -162,7 +162,6 @@ func SearchIssues(ctx *context.APIContext) { SortType: "priorityrepo", PriorityRepoID: ctx.QueryInt64("priority_repo_id"), IsPull: isPull, - HasAnyLabel: true, }) } From 0013cd314f2e1f2cc0d29f98cbe49f0be5db5700 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 31 Mar 2020 09:52:55 -0400 Subject: [PATCH 36/41] update to master --- models/migrations/migrations.go | 2 +- models/migrations/v134.go | 2 +- models/migrations/v135.go | 22 ++++++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 models/migrations/v135.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index acb381f04001b..847cd75d521f6 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -202,7 +202,7 @@ var migrations = []Migration{ NewMigration("Add EmailHash Table", addEmailHashTable), // v134 -> v135 NewMigration("Refix merge base for merged pull requests", refixMergeBase), - // v135 -> 136 + // v135 -> 136 NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), } diff --git a/models/migrations/v134.go b/models/migrations/v134.go index 2a27e90fe83c6..527cbafe07ae4 100644 --- a/models/migrations/v134.go +++ b/models/migrations/v134.go @@ -93,4 +93,4 @@ func refixMergeBase(x *xorm.Engine) error { } } return nil -} \ No newline at end of file +} diff --git a/models/migrations/v135.go b/models/migrations/v135.go new file mode 100644 index 0000000000000..8d859d42c05b0 --- /dev/null +++ b/models/migrations/v135.go @@ -0,0 +1,22 @@ +// Copyright 2020 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 migrations + +import ( + "fmt" + + "xorm.io/xorm" +) + +func addOrgIDLabelColumn(x *xorm.Engine) error { + type Label struct { + OrgID int64 `xorm:"INDEX"` + } + + if err := x.Sync2(new(Label)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + return nil +} From f74a1b8d53f7f16df32ef65d9b225f97b5cd8484 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 31 Mar 2020 10:38:08 -0400 Subject: [PATCH 37/41] check for matching ids before query --- models/issue_label.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 3e08c0943229e..0eb75b6a833a9 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -262,10 +262,11 @@ func DeleteLabel(id, labelID int64) error { return err } - if label.BelongsToOrg() { - sess.And("org_id = ?", id) - } else { - sess.And("repo_id = ?", id) + if label.BelongsToOrg() && label.OrgID != id { + return nil + } + if label.BelongsToRepo() && label.RepoID != id { + return nil } if _, err = sess.ID(labelID).Delete(new(Label)); err != nil { From 551f3b570129c8d14f66bb9e08a69b11242a7a6d Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Tue, 31 Mar 2020 14:45:48 -0400 Subject: [PATCH 38/41] Update models/issue_label.go Co-Authored-By: 6543 <6543@obermui.de> --- models/issue_label.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_label.go b/models/issue_label.go index 0eb75b6a833a9..210d79aff3586 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -97,7 +97,7 @@ func (label *Label) CalOpenOrgIssues(repoID, labelID int64) { }) for _, count := range counts { - label.NumOpenRepoIssues = count + label.NumOpenRepoIssues += count } } From d39f3032d8c6dd97aba8f567b7adfbaca4ecdd83 Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Tue, 31 Mar 2020 14:48:41 -0400 Subject: [PATCH 39/41] Update models/issue_label.go --- models/issue_label.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_label.go b/models/issue_label.go index 210d79aff3586..944835ca9fa13 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -81,7 +81,7 @@ func GetLabelTemplateFile(name string) ([][3]string, error) { return list, nil } -// CalOpenIssues returns the open issues of label. +// CalOpenIssues sets the number of open issues of a label based on the already stored number of closed issues. func (label *Label) CalOpenIssues() { label.NumOpenIssues = label.NumIssues - label.NumClosedIssues } From 0bddcf776275ae4801afea0ffa8de7953c704e23 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Tue, 31 Mar 2020 15:11:22 -0400 Subject: [PATCH 40/41] update comments --- models/issue_label.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 944835ca9fa13..cb9c307e2b279 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -153,7 +153,11 @@ func (label *Label) ForegroundColor() template.CSS { return template.CSS("#000") } -// LABEL +// .____ ___. .__ +// | | _____ \_ |__ ____ | | +// | | \__ \ | __ \_/ __ \| | +// | |___ / __ \| \_\ \ ___/| |__ +// >_______ (____ /___ /\___ >____/ func loadLabels(labelTemplate string) ([]string, error) { list, err := GetLabelTemplateFile(labelTemplate) @@ -318,7 +322,12 @@ func GetLabelsByIDs(labelIDs []int64) ([]*Label, error) { Find(&labels) } -// REPO +// __________ .__ __ +// \______ \ ____ ______ ____ _____|__|/ |_ ___________ ___.__. +// | _// __ \\____ \ / _ \/ ___/ \ __\/ _ \_ __ < | | +// | | \ ___/| |_> > <_> )___ \| || | ( <_> ) | \/\___ | +// |____|_ /\___ > __/ \____/____ >__||__| \____/|__| / ____| +// \/ \/|__| \/ \/ // getLabelInRepoByName returns a label by Name in given repository. func getLabelInRepoByName(e Engine, repoID int64, labelName string) (*Label, error) { @@ -558,7 +567,12 @@ func GetLabelsByOrgID(orgID int64, sortType string, listOptions ListOptions) ([] return getLabelsByOrgID(x, orgID, sortType, listOptions) } -//ISSUE +// .___ +// | | ______ ________ __ ____ +// | |/ ___// ___/ | \_/ __ \ +// | |\___ \ \___ \| | /\ ___/ +// |___/____ >____ >____/ \___ | +// \/ \/ \/ func getLabelsByIssueID(e Engine, issueID int64) ([]*Label, error) { var labels []*Label From 69cf00ecd436f73fae445fc60a014f22b115cf03 Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Tue, 31 Mar 2020 15:13:10 -0400 Subject: [PATCH 41/41] Update routers/org/setting.go --- routers/org/setting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/org/setting.go b/routers/org/setting.go index c626f9f4c9728..348d8cc8d84e0 100644 --- a/routers/org/setting.go +++ b/routers/org/setting.go @@ -24,7 +24,7 @@ const ( tplSettingsDelete base.TplName = "org/settings/delete" // tplSettingsHooks template path for render hook settings tplSettingsHooks base.TplName = "org/settings/hooks" - // + // tplSettingsLabels template path for render labels settings tplSettingsLabels base.TplName = "org/settings/labels" )