Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track labels changed on issue view & resolved #542 #788

Merged
merged 2 commits into from
Jan 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 101 additions & 26 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package models
import (
"errors"
"fmt"
"sort"
"strings"
"time"

Expand Down Expand Up @@ -103,11 +104,17 @@ func (issue *Issue) GetPullRequest() (pr *PullRequest, err error) {
return
}

func (issue *Issue) loadAttributes(e Engine) (err error) {
if err := issue.loadRepo(e); err != nil {
return err
func (issue *Issue) loadLabels(e Engine) (err error) {
if issue.Labels == nil {
issue.Labels, err = getLabelsByIssueID(e, issue.ID)
if err != nil {
return fmt.Errorf("getLabelsByIssueID [%d]: %v", issue.ID, err)
}
}
return nil
}

func (issue *Issue) loadPoster(e Engine) (err error) {
if issue.Poster == nil {
issue.Poster, err = getUserByID(e, issue.PosterID)
if err != nil {
Expand All @@ -120,12 +127,20 @@ func (issue *Issue) loadAttributes(e Engine) (err error) {
return
}
}
return
}

if issue.Labels == nil {
issue.Labels, err = getLabelsByIssueID(e, issue.ID)
if err != nil {
return fmt.Errorf("getLabelsByIssueID [%d]: %v", issue.ID, err)
}
func (issue *Issue) loadAttributes(e Engine) (err error) {
if err = issue.loadRepo(e); err != nil {
return
}

if err = issue.loadPoster(e); err != nil {
return
}

if err = issue.loadLabels(e); err != nil {
return
}

if issue.Milestone == nil && issue.MilestoneID > 0 {
Expand Down Expand Up @@ -289,27 +304,27 @@ func (issue *Issue) sendLabelUpdatedWebhook(doer *User) {
}
}

func (issue *Issue) addLabel(e *xorm.Session, label *Label) error {
return newIssueLabel(e, issue, label)
func (issue *Issue) addLabel(e *xorm.Session, label *Label, doer *User) error {
return newIssueLabel(e, issue, label, doer)
}

// AddLabel adds a new label to the issue.
func (issue *Issue) AddLabel(doer *User, label *Label) error {
if err := NewIssueLabel(issue, label); err != nil {
if err := NewIssueLabel(issue, label, doer); err != nil {
return err
}

issue.sendLabelUpdatedWebhook(doer)
return nil
}

func (issue *Issue) addLabels(e *xorm.Session, labels []*Label) error {
return newIssueLabels(e, issue, labels)
func (issue *Issue) addLabels(e *xorm.Session, labels []*Label, doer *User) error {
return newIssueLabels(e, issue, labels, doer)
}

// AddLabels adds a list of new labels to the issue.
func (issue *Issue) AddLabels(doer *User, labels []*Label) error {
if err := NewIssueLabels(issue, labels); err != nil {
if err := NewIssueLabels(issue, labels, doer); err != nil {
return err
}

Expand All @@ -329,8 +344,8 @@ func (issue *Issue) getLabels(e Engine) (err error) {
return nil
}

func (issue *Issue) removeLabel(e *xorm.Session, label *Label) error {
return deleteIssueLabel(e, issue, label)
func (issue *Issue) removeLabel(e *xorm.Session, doer *User, label *Label) error {
return deleteIssueLabel(e, doer, issue, label)
}

// RemoveLabel removes a label from issue by given ID.
Expand All @@ -345,21 +360,21 @@ func (issue *Issue) RemoveLabel(doer *User, label *Label) error {
return ErrLabelNotExist{}
}

if err := DeleteIssueLabel(issue, label); err != nil {
if err := DeleteIssueLabel(issue, doer, label); err != nil {
return err
}

issue.sendLabelUpdatedWebhook(doer)
return nil
}

func (issue *Issue) clearLabels(e *xorm.Session) (err error) {
func (issue *Issue) clearLabels(e *xorm.Session, doer *User) (err error) {
if err = issue.getLabels(e); err != nil {
return fmt.Errorf("getLabels: %v", err)
}

for i := range issue.Labels {
if err = issue.removeLabel(e, issue.Labels[i]); err != nil {
if err = issue.removeLabel(e, doer, issue.Labels[i]); err != nil {
return fmt.Errorf("removeLabel: %v", err)
}
}
Expand All @@ -386,7 +401,7 @@ func (issue *Issue) ClearLabels(doer *User) (err error) {
return ErrLabelNotExist{}
}

if err = issue.clearLabels(sess); err != nil {
if err = issue.clearLabels(sess, doer); err != nil {
return err
}

Expand Down Expand Up @@ -417,19 +432,75 @@ func (issue *Issue) ClearLabels(doer *User) (err error) {
return nil
}

type labelSorter []*Label

func (ts labelSorter) Len() int {
return len([]*Label(ts))
}

func (ts labelSorter) Less(i, j int) bool {
return []*Label(ts)[i].ID < []*Label(ts)[j].ID
}

func (ts labelSorter) Swap(i, j int) {
[]*Label(ts)[i], []*Label(ts)[j] = []*Label(ts)[j], []*Label(ts)[i]
}

// ReplaceLabels removes all current labels and add new labels to the issue.
// Triggers appropriate WebHooks, if any.
func (issue *Issue) ReplaceLabels(labels []*Label) (err error) {
func (issue *Issue) ReplaceLabels(labels []*Label, doer *User) (err error) {
sess := x.NewSession()
defer sessionRelease(sess)
if err = sess.Begin(); err != nil {
return err
}

if err = issue.clearLabels(sess); err != nil {
return fmt.Errorf("clearLabels: %v", err)
} else if err = issue.addLabels(sess, labels); err != nil {
return fmt.Errorf("addLabels: %v", err)
if err = issue.loadLabels(sess); err != nil {
return err
}

sort.Sort(labelSorter(labels))
sort.Sort(labelSorter(issue.Labels))

var toAdd, toRemove []*Label
for _, l := range labels {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should Sort these first? should make it slightly faster in some cases 🙂

var exist bool
for _, oriLabel := range issue.Labels {
if oriLabel.ID == l.ID {
exist = true
break
}
}
if !exist {
toAdd = append(toAdd, l)
}
}

for _, oriLabel := range issue.Labels {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

var exist bool
for _, l := range labels {
if oriLabel.ID == l.ID {
exist = true
break
}
}
if !exist {
toRemove = append(toRemove, oriLabel)
}
}

if len(toAdd) > 0 {
if err = issue.addLabels(sess, toAdd, doer); err != nil {
return fmt.Errorf("addLabels: %v", err)
}
}

if len(toRemove) > 0 {
for _, l := range toRemove {
if err = issue.removeLabel(sess, doer, l); err != nil {
return fmt.Errorf("removeLabel: %v", err)
}
}
}

return sess.Commit()
Expand Down Expand Up @@ -731,13 +802,17 @@ func newIssue(e *xorm.Session, opts NewIssueOptions) (err error) {
return fmt.Errorf("find all labels [label_ids: %v]: %v", opts.LableIDs, err)
}

if err = opts.Issue.loadPoster(e); err != nil {
return err
}

for _, label := range labels {
// Silently drop invalid labels.
if label.RepoID != opts.Repo.ID {
continue
}

if err = opts.Issue.addLabel(e, label); err != nil {
if err = opts.Issue.addLabel(e, label, opts.Issue.Poster); err != nil {
return fmt.Errorf("addLabel [id: %d]: %v", label.ID, err)
}
}
Expand Down
44 changes: 44 additions & 0 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ const (
CommentTypeCommentRef
// Reference from a pull request
CommentTypePullRef
// Labels changed
CommentTypeLabel
)

// CommentTag defines comment tag type
Expand All @@ -57,6 +59,8 @@ type Comment struct {
Poster *User `xorm:"-"`
IssueID int64 `xorm:"INDEX"`
CommitID int64
LabelID int64
Label *Label `xorm:"-"`
Line int64
Content string `xorm:"TEXT"`
RenderedContent string `xorm:"-"`
Expand Down Expand Up @@ -185,6 +189,21 @@ func (c *Comment) EventTag() string {
return "event-" + com.ToStr(c.ID)
}

// LoadLabel if comment.Type is CommentTypeLabel, then load Label
func (c *Comment) LoadLabel() error {
var label Label
has, err := x.ID(c.LabelID).Get(&label)
if err != nil {
return err
} else if !has {
return ErrLabelNotExist{
LabelID: c.LabelID,
}
}
c.Label = &label
return nil
}

// MailParticipants sends new comment emails to repository watchers
// and mentioned people.
func (c *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (err error) {
Expand All @@ -209,11 +228,16 @@ func (c *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (e
}

func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err error) {
var LabelID int64
if opts.Label != nil {
LabelID = opts.Label.ID
}
comment := &Comment{
Type: opts.Type,
PosterID: opts.Doer.ID,
Poster: opts.Doer,
IssueID: opts.Issue.ID,
LabelID: LabelID,
CommitID: opts.CommitID,
CommitSHA: opts.CommitSHA,
Line: opts.LineNum,
Expand All @@ -223,6 +247,10 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
return nil, err
}

if err = opts.Repo.getOwner(e); err != nil {
return nil, err
}

// Compose comment action, could be plain comment, close or reopen issue/pull request.
// This object will be used to notify watchers in the end of function.
act := &Action{
Expand Down Expand Up @@ -324,12 +352,28 @@ func createStatusComment(e *xorm.Session, doer *User, repo *Repository, issue *I
})
}

func createLabelComment(e *xorm.Session, doer *User, repo *Repository, issue *Issue, label *Label, add bool) (*Comment, error) {
var content string
if add {
content = "1"
}
return createComment(e, &CreateCommentOptions{
Type: CommentTypeLabel,
Doer: doer,
Repo: repo,
Issue: issue,
Label: label,
Content: content,
})
}

// CreateCommentOptions defines options for creating comment
type CreateCommentOptions struct {
Type CommentType
Doer *User
Repo *Repository
Issue *Issue
Label *Label

CommitID int64
CommitSHA string
Expand Down
Loading