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 assignee for issue #808

Merged
merged 3 commits into from
Feb 3, 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
21 changes: 21 additions & 0 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,11 +693,23 @@ func (issue *Issue) ChangeContent(doer *User, content string) (err error) {

// ChangeAssignee changes the Asssignee field of this issue.
func (issue *Issue) ChangeAssignee(doer *User, assigneeID int64) (err error) {
var oldAssigneeID = issue.AssigneeID
issue.AssigneeID = assigneeID
if err = UpdateIssueUserByAssignee(issue); err != nil {
return fmt.Errorf("UpdateIssueUserByAssignee: %v", err)
}

sess := x.NewSession()
defer sess.Close()

if err = issue.loadRepo(sess); err != nil {
return fmt.Errorf("loadRepo: %v", err)
}

if _, err = createAssigneeComment(sess, doer, issue.Repo, issue, oldAssigneeID, assigneeID); err != nil {
return fmt.Errorf("createAssigneeComment: %v", err)
}

issue.Assignee, err = GetUserByID(issue.AssigneeID)
if err != nil && !IsErrUserNotExist(err) {
log.Error(4, "GetUserByID [assignee_id: %v]: %v", issue.AssigneeID, err)
Expand Down Expand Up @@ -788,6 +800,15 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
}
}

if opts.Issue.AssigneeID > 0 {
if err = opts.Issue.loadRepo(e); err != nil {
return err
}
if _, err = createAssigneeComment(e, doer, opts.Issue.Repo, opts.Issue, -1, opts.Issue.AssigneeID); err != nil {
return err
}
}

if opts.IsPull {
_, err = e.Exec("UPDATE `repository` SET num_pulls = num_pulls + 1 WHERE id = ?", opts.Issue.RepoID)
} else {
Expand Down
63 changes: 52 additions & 11 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ const (
CommentTypeLabel
// Milestone changed
CommentTypeMilestone
// Assignees changed
CommentTypeAssignees
Copy link
Member

Choose a reason for hiding this comment

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

Why plural? I would prefer singular for consistency

Copy link
Member Author

Choose a reason for hiding this comment

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

Because currently Gitea only supports one assignee, but github support many. I think Gitea could support many in future. So I keep it plural for the next PR.

Copy link
Member

Choose a reason for hiding this comment

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

Okay

)

// CommentTag defines comment tag type
Expand All @@ -55,17 +57,22 @@ const (

// Comment represents a comment in commit and issue page.
type Comment struct {
ID int64 `xorm:"pk autoincr"`
Type CommentType
PosterID int64 `xorm:"INDEX"`
Poster *User `xorm:"-"`
IssueID int64 `xorm:"INDEX"`
LabelID int64
Label *Label `xorm:"-"`
OldMilestoneID int64
MilestoneID int64
OldMilestone *Milestone `xorm:"-"`
Milestone *Milestone `xorm:"-"`
ID int64 `xorm:"pk autoincr"`
Type CommentType
PosterID int64 `xorm:"INDEX"`
Poster *User `xorm:"-"`
IssueID int64 `xorm:"INDEX"`
LabelID int64
Label *Label `xorm:"-"`
OldMilestoneID int64
MilestoneID int64
OldMilestone *Milestone `xorm:"-"`
Milestone *Milestone `xorm:"-"`
OldAssigneeID int64
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if we didn't have to have both OldMilestoneID/MilestoneID and OldAssigneeID/AssigneeID, since at most one of them will be relevant for any particular issue.

Would it make sense to simply have OldElementID and ElementID fields? If Type==CommentTypeAssignees then OldElementID and ElementID are assignee IDs, if Type==CommentTypeMilestone they are milestone IDs, etc. I see pros and cons, interested to hear your thoughts 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that's a good idea. The names should have meaning for easy maintaining. The memory usage is not very different.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough

AssigneeID int64
Assignee *User `xorm:"-"`
OldAssignee *User `xorm:"-"`

CommitID int64
Line int64
Content string `xorm:"TEXT"`
Expand Down Expand Up @@ -240,6 +247,25 @@ func (c *Comment) LoadMilestone() error {
return nil
}

// LoadAssignees if comment.Type is CommentTypeAssignees, then load assignees
func (c *Comment) LoadAssignees() error {
var err error
if c.OldAssigneeID > 0 {
c.OldAssignee, err = getUserByID(x, c.OldAssigneeID)
if err != nil {
return err
}
}

if c.AssigneeID > 0 {
c.Assignee, err = getUserByID(x, c.AssigneeID)
if err != nil {
return err
}
}
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 Down Expand Up @@ -276,6 +302,8 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
LabelID: LabelID,
OldMilestoneID: opts.OldMilestoneID,
MilestoneID: opts.MilestoneID,
OldAssigneeID: opts.OldAssigneeID,
AssigneeID: opts.AssigneeID,
CommitID: opts.CommitID,
CommitSHA: opts.CommitSHA,
Line: opts.LineNum,
Expand Down Expand Up @@ -416,6 +444,17 @@ func createMilestoneComment(e *xorm.Session, doer *User, repo *Repository, issue
})
}

func createAssigneeComment(e *xorm.Session, doer *User, repo *Repository, issue *Issue, oldAssigneeID, assigneeID int64) (*Comment, error) {
return createComment(e, &CreateCommentOptions{
Type: CommentTypeAssignees,
Doer: doer,
Repo: repo,
Issue: issue,
OldAssigneeID: oldAssigneeID,
AssigneeID: assigneeID,
})
}

// CreateCommentOptions defines options for creating comment
type CreateCommentOptions struct {
Type CommentType
Expand All @@ -426,6 +465,8 @@ type CreateCommentOptions struct {

OldMilestoneID int64
MilestoneID int64
OldAssigneeID int64
AssigneeID int64
CommitID int64
CommitSHA string
LineNum int64
Expand Down
3 changes: 3 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,9 @@ issues.remove_label_at = `removed the <div class="ui label" style="color: %s; ba
issues.add_milestone_at = `added this to the <b>%s</b> milestone %s`
issues.change_milestone_at = `modified the milestone from <b>%s</b> to <b>%s</b> %s`
issues.remove_milestone_at = `removed this from the <b>%s</b> milestone %s`
issues.self_assign_at = `self-assigned this %s`
issues.add_assignee_at = `was assigned by <b>%s</b> %s`
issues.remove_assignee_at = `removed their assignment %s`
issues.open_tab = %d Open
issues.close_tab = %d Closed
issues.filter_label = Label
Expand Down
5 changes: 5 additions & 0 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,11 @@ func ViewIssue(ctx *context.Context) {
ctx.Handle(500, "LoadMilestone", err)
return
}
} else if comment.Type == models.CommentTypeAssignees {
if err = comment.LoadAssignees(); err != nil {
ctx.Handle(500, "LoadAssignees", err)
return
}
}
}

Expand Down
13 changes: 13 additions & 0 deletions templates/repo/issue/view_content.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,19 @@
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
{{if gt .OldMilestoneID 0}}{{if gt .MilestoneID 0}}{{$.i18n.Tr "repo.issues.change_milestone_at" .OldMilestone.Name .Milestone.Name $createdStr | Safe}}{{else}}{{$.i18n.Tr "repo.issues.remove_milestone_at" .OldMilestone.Name $createdStr | Safe}}{{end}}{{else if gt .MilestoneID 0}}{{$.i18n.Tr "repo.issues.add_milestone_at" .Milestone.Name $createdStr | Safe}}{{end}}</span>
</div>
{{else if eq .Type 9}}
<div class="event">
<span class="octicon octicon-primitive-dot"></span>
{{if gt .AssigneeID 0}}{{if eq .Poster.ID .AssigneeID}}<a class="ui avatar image" href="{{.Poster.HomeLink}}">
<img src="{{.Poster.RelAvatarLink}}">
</a> <span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a> {{$.i18n.Tr "repo.issues.self_assign_at" $createdStr | Safe}} </span>
{{else}}<a class="ui avatar image" href="{{.Assignee.HomeLink}}">
<img src="{{.Assignee.RelAvatarLink}}">
</a><span class="text grey"><a href="{{.Assignee.HomeLink}}">{{.Assignee.Name}}</a> {{$.i18n.Tr "repo.issues.add_assignee_at" .Poster.Name $createdStr | Safe}} </span>{{end}}{{else if gt .OldAssigneeID 0}}
<a class="ui avatar image" href="{{.Poster.HomeLink}}">
<img src="{{.Poster.RelAvatarLink}}">
</a> <span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a> {{$.i18n.Tr "repo.issues.remove_assignee_at" $createdStr | Safe}} </span>{{end}}
</div>
{{end}}

{{end}}
Expand Down