diff --git a/integrations/api_issue_label_test.go b/integrations/api_issue_label_test.go index ddcfdd661..64c3515dd 100644 --- a/integrations/api_issue_label_test.go +++ b/integrations/api_issue_label_test.go @@ -91,22 +91,22 @@ func TestAPIAddIssueLabels(t *testing.T) { repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) issue := models.AssertExistsAndLoadBean(t, &models.Issue{RepoID: repo.ID}).(*models.Issue) - label := models.AssertExistsAndLoadBean(t, &models.Label{RepoID: repo.ID}).(*models.Label) + _ = models.AssertExistsAndLoadBean(t, &models.Label{RepoID: repo.ID, ID: 2}).(*models.Label) owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) session := loginUser(t, owner.Name) token := getTokenForLoggedInUser(t, session) urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/labels?token=%s", - owner.Name, repo.Name, issue.Index, token) + repo.OwnerName, repo.Name, issue.Index, token) req := NewRequestWithJSON(t, "POST", urlStr, &api.IssueLabelsOption{ - Labels: []int64{label.ID}, + Labels: []int64{1, 2}, }) resp := session.MakeRequest(t, req, http.StatusOK) var apiLabels []*api.Label DecodeJSON(t, resp, &apiLabels) assert.Len(t, apiLabels, models.GetCount(t, &models.IssueLabel{IssueID: issue.ID})) - models.AssertExistsAndLoadBean(t, &models.IssueLabel{IssueID: issue.ID, LabelID: label.ID}) + models.AssertExistsAndLoadBean(t, &models.IssueLabel{IssueID: issue.ID, LabelID: 2}) } func TestAPIReplaceIssueLabels(t *testing.T) { diff --git a/models/issue.go b/models/issue.go index 1b634ed9e..3a7a0cd41 100644 --- a/models/issue.go +++ b/models/issue.go @@ -513,6 +513,10 @@ func (issue *Issue) ReplaceLabels(labels []*Label, doer *User) (err error) { return err } + if err = issue.loadRepo(sess); err != nil { + return err + } + if err = issue.loadLabels(sess); err != nil { return err } @@ -527,10 +531,18 @@ func (issue *Issue) ReplaceLabels(labels []*Label, doer *User) (err error) { addLabel := labels[addIndex] removeLabel := issue.Labels[removeIndex] if addLabel.ID == removeLabel.ID { + // Silently drop invalid labels + if removeLabel.RepoID != issue.RepoID && removeLabel.OrgID != issue.Repo.OwnerID { + toRemove = append(toRemove, removeLabel) + } + addIndex++ removeIndex++ } else if addLabel.ID < removeLabel.ID { - toAdd = append(toAdd, addLabel) + // Only add if the label is valid + if addLabel.RepoID == issue.RepoID || addLabel.OrgID == issue.Repo.OwnerID { + toAdd = append(toAdd, addLabel) + } addIndex++ } else { toRemove = append(toRemove, removeLabel) diff --git a/models/issue_label.go b/models/issue_label.go index 54b286fe7..1b5cfd88d 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -321,7 +321,7 @@ func GetLabelsByIDs(labelIDs []int64) ([]*Label, error) { return labels, x.Table("label"). In("id", labelIDs). Asc("name"). - Cols("id"). + Cols("id", "repo_id", "org_id"). Find(&labels) } @@ -632,6 +632,8 @@ func HasIssueLabel(issueID, labelID int64) bool { return hasIssueLabel(x, issueID, labelID) } +// newIssueLabel this function creates a new label it does not check if the label is valid for the issue +// YOU MUST CHECK THIS BEFORE THIS FUNCTION func newIssueLabel(e *xorm.Session, issue *Issue, label *Label, doer *User) (err error) { if _, err = e.Insert(&IssueLabel{ IssueID: issue.ID, @@ -671,6 +673,15 @@ func NewIssueLabel(issue *Issue, label *Label, doer *User) (err error) { return err } + if err = issue.loadRepo(sess); err != nil { + return err + } + + // Do NOT add invalid labels + if issue.RepoID != label.RepoID && issue.Repo.OwnerID != label.OrgID { + return nil + } + if err = newIssueLabel(sess, issue, label, doer); err != nil { return err } @@ -683,13 +694,19 @@ func NewIssueLabel(issue *Issue, label *Label, doer *User) (err error) { return sess.Commit() } +// newIssueLabels add labels to an issue. It will check if the labels are valid for the issue func newIssueLabels(e *xorm.Session, issue *Issue, labels []*Label, doer *User) (err error) { - for i := range labels { - if hasIssueLabel(e, issue.ID, labels[i].ID) { + if err = issue.loadRepo(e); err != nil { + return err + } + for _, label := range labels { + // Don't add already present labels and invalid labels + if hasIssueLabel(e, issue.ID, label.ID) || + (label.RepoID != issue.RepoID && label.OrgID != issue.Repo.OwnerID) { continue } - if err = newIssueLabel(e, issue, labels[i], doer); err != nil { + if err = newIssueLabel(e, issue, label, doer); err != nil { return fmt.Errorf("newIssueLabel: %v", err) } } diff --git a/models/repo_transfer.go b/models/repo_transfer.go index 9f8fd649b..273dca1c5 100644 --- a/models/repo_transfer.go +++ b/models/repo_transfer.go @@ -325,6 +325,33 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e } } + // Delete labels that belong to the old organization and comments that added these labels + if oldOwner.IsOrganization() { + if _, err := sess.Exec(`DELETE FROM issue_label WHERE issue_label.id IN ( + SELECT il_too.id FROM ( + SELECT il_too_too.id + FROM issue_label AS il_too_too + INNER JOIN label ON il_too_too.id = label.id + INNER JOIN issue on issue.id = il_too_too.issue_id + WHERE + issue.repo_id = ? AND (issue.repo_id != label.repo_id OR (label.repo_id = 0 AND label.org_id != ?)) + ) AS il_too )`, repo.ID, newOwner.ID); err != nil { + return fmt.Errorf("Unable to remove old org labels: %v", err) + } + + if _, err := sess.Exec(`DELETE FROM comment WHERE comment.id IN ( + SELECT il_too.id FROM ( + SELECT com.id + FROM comment AS com + INNER JOIN label ON com.label_id = label.id + INNER JOIN issue on issue.id = com.issue_id + WHERE + com.type = ? AND issue.repo_id = ? AND (issue.repo_id != label.repo_id OR (label.repo_id = 0 AND label.org_id != ?)) + ) AS il_too)`, CommentTypeLabel, repo.ID, newOwner.ID); err != nil { + return fmt.Errorf("Unable to remove old org label comments: %v", err) + } + } + // Rename remote repository to new path and delete local copy. dir := UserPath(newOwner.Name)