Skip to content

Uniform an abstract notification layer for ui, mail, action, webhook and indexer #4001

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.

Sign up for GitHub

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

Closed
wants to merge 14 commits into from
Closed

Uniform an abstract notification layer for ui, mail, action, webhook and indexer #4001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
46b14ab
uniform an abstract notification layer for ui, mail, action, webhook,…
lunny May 21, 2018
57295ab
fix lint
lunny May 21, 2018
f300273
fix lint
lunny May 21, 2018
9c02a54
fix imports
lunny May 21, 2018
5f3d1a7
rename to notifier
lunny May 21, 2018
1cf129b
add deletecomment notify
lunny May 21, 2018
76d97a0
add more webhooks
lunny May 22, 2018
ef32ba6
more events
lunny May 23, 2018
c81535c
fix tests
lunny May 23, 2018
f3a25d4
more events
lunny Jul 9, 2018
26675b8
fix lint
lunny Jul 12, 2018
5a78ce9
fix tests
lunny Jul 13, 2018
8b6c568
fix imports
lunny Jul 13, 2018
08ef597
more notification
lunny Aug 2, 2018
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
10 changes: 3 additions & 7 deletions models/action.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,8 +719,9 @@ func TransferRepoAction(doer, oldOwner *User, repo *Repository) error {
return transferRepoAction(x, doer, oldOwner, repo)
}

func mergePullRequestAction(e Engine, doer *User, repo *Repository, issue *Issue) error {
return notifyWatchers(e, &Action{
// MergePullRequestAction adds new action for merging pull request.
func MergePullRequestAction(doer *User, repo *Repository, issue *Issue) error {
return notifyWatchers(x, &Action{
ActUserID: doer.ID,
ActUser: doer,
OpType: ActionMergePullRequest,
Expand All @@ -731,11 +732,6 @@ func mergePullRequestAction(e Engine, doer *User, repo *Repository, issue *Issue
})
}

// MergePullRequestAction adds new action for merging pull request.
func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue) error {
return mergePullRequestAction(x, actUser, repo, pull)
}

// GetFeedsOptions options for retrieving feeds
type GetFeedsOptions struct {
RequestedUser *User
Expand Down
203 changes: 10 additions & 193 deletions models/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ func (issue *Issue) IsOverdue() bool {
return util.TimeStampNow() >= issue.DeadlineUnix
}

// LoadRepo loads repository
func (issue *Issue) LoadRepo() error {
return issue.loadRepo(x)
}

func (issue *Issue) loadRepo(e Engine) (err error) {
if issue.Repo == nil {
issue.Repo, err = getRepositoryByID(e, issue.RepoID)
Expand All @@ -105,14 +110,9 @@ func (issue *Issue) IsTimetrackerEnabled() bool {
return issue.Repo.IsTimetrackerEnabled()
}

// GetPullRequest returns the issue pull request
func (issue *Issue) GetPullRequest() (pr *PullRequest, err error) {
if !issue.IsPull {
return nil, fmt.Errorf("Issue is not a pull request")
}

pr, err = getPullRequestByIssueID(x, issue.ID)
return
// LoadPullRequest loads the issue pull request
func (issue *Issue) LoadPullRequest() error {
return issue.loadPullRequest(x)
}

func (issue *Issue) loadLabels(e Engine) (err error) {
Expand Down Expand Up @@ -368,52 +368,6 @@ func (issue *Issue) HasLabel(labelID int64) bool {
return issue.hasLabel(x, labelID)
}

func (issue *Issue) sendLabelUpdatedWebhook(doer *User) {
var err error

if err = issue.loadRepo(x); err != nil {
log.Error(4, "loadRepo: %v", err)
return
}

if err = issue.loadPoster(x); err != nil {
log.Error(4, "loadPoster: %v", err)
return
}

mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
if issue.IsPull {
if err = issue.loadPullRequest(x); err != nil {
log.Error(4, "loadPullRequest: %v", err)
return
}
if err = issue.PullRequest.LoadIssue(); err != nil {
log.Error(4, "LoadIssue: %v", err)
return
}
err = PrepareWebhooks(issue.Repo, HookEventPullRequest, &api.PullRequestPayload{
Action: api.HookIssueLabelUpdated,
Index: issue.Index,
PullRequest: issue.PullRequest.APIFormat(),
Repository: issue.Repo.APIFormat(AccessModeNone),
Sender: doer.APIFormat(),
})
} else {
err = PrepareWebhooks(issue.Repo, HookEventIssues, &api.IssuePayload{
Action: api.HookIssueLabelUpdated,
Index: issue.Index,
Issue: issue.APIFormat(),
Repository: issue.Repo.APIFormat(mode),
Sender: doer.APIFormat(),
})
}
if err != nil {
log.Error(4, "PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
} else {
go HookQueue.Add(issue.RepoID)
}
}

func (issue *Issue) addLabel(e *xorm.Session, label *Label, doer *User) error {
return newIssueLabel(e, issue, label, doer)
}
Expand All @@ -424,7 +378,6 @@ func (issue *Issue) AddLabel(doer *User, label *Label) error {
return err
}

issue.sendLabelUpdatedWebhook(doer)
return nil
}

Expand All @@ -438,7 +391,6 @@ func (issue *Issue) AddLabels(doer *User, labels []*Label) error {
return err
}

issue.sendLabelUpdatedWebhook(doer)
return nil
}

Expand Down Expand Up @@ -474,7 +426,6 @@ func (issue *Issue) RemoveLabel(doer *User, label *Label) error {
return err
}

issue.sendLabelUpdatedWebhook(doer)
return nil
}

Expand Down Expand Up @@ -521,39 +472,6 @@ func (issue *Issue) ClearLabels(doer *User) (err error) {
return fmt.Errorf("Commit: %v", err)
}

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

mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
if issue.IsPull {
err = issue.PullRequest.LoadIssue()
if err != nil {
log.Error(4, "LoadIssue: %v", err)
return
}
err = PrepareWebhooks(issue.Repo, HookEventPullRequest, &api.PullRequestPayload{
Action: api.HookIssueLabelCleared,
Index: issue.Index,
PullRequest: issue.PullRequest.APIFormat(),
Repository: issue.Repo.APIFormat(mode),
Sender: doer.APIFormat(),
})
} else {
err = PrepareWebhooks(issue.Repo, HookEventIssues, &api.IssuePayload{
Action: api.HookIssueLabelCleared,
Index: issue.Index,
Issue: issue.APIFormat(),
Repository: issue.Repo.APIFormat(mode),
Sender: doer.APIFormat(),
})
}
if err != nil {
log.Error(4, "PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
} else {
go HookQueue.Add(issue.RepoID)
}

return nil
}

Expand Down Expand Up @@ -718,6 +636,7 @@ func (issue *Issue) ChangeStatus(doer *User, repo *Repository, isClosed bool) (e
return fmt.Errorf("Commit: %v", err)
}

// FIX: this code should be removed and add notification service out of models
mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
if issue.IsPull {
// Merge pull request calls issue.changeStatus so we need to handle separately.
Expand Down Expand Up @@ -780,42 +699,6 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
return err
}

mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
if issue.IsPull {
issue.PullRequest.Issue = issue
err = PrepareWebhooks(issue.Repo, HookEventPullRequest, &api.PullRequestPayload{
Action: api.HookIssueEdited,
Index: issue.Index,
Changes: &api.ChangesPayload{
Title: &api.ChangesFromPayload{
From: oldTitle,
},
},
PullRequest: issue.PullRequest.APIFormat(),
Repository: issue.Repo.APIFormat(mode),
Sender: doer.APIFormat(),
})
} else {
err = PrepareWebhooks(issue.Repo, HookEventIssues, &api.IssuePayload{
Action: api.HookIssueEdited,
Index: issue.Index,
Changes: &api.ChangesPayload{
Title: &api.ChangesFromPayload{
From: oldTitle,
},
},
Issue: issue.APIFormat(),
Repository: issue.Repo.APIFormat(mode),
Sender: issue.Poster.APIFormat(),
})
}

if err != nil {
log.Error(4, "PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
} else {
go HookQueue.Add(issue.RepoID)
}

return nil
}

Expand All @@ -839,48 +722,12 @@ func AddDeletePRBranchComment(doer *User, repo *Repository, issueID int64, branc

// ChangeContent changes issue content, as the given user.
func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
oldContent := issue.Content
issue.Content = content

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

mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
if issue.IsPull {
issue.PullRequest.Issue = issue
err = PrepareWebhooks(issue.Repo, HookEventPullRequest, &api.PullRequestPayload{
Action: api.HookIssueEdited,
Index: issue.Index,
Changes: &api.ChangesPayload{
Body: &api.ChangesFromPayload{
From: oldContent,
},
},
PullRequest: issue.PullRequest.APIFormat(),
Repository: issue.Repo.APIFormat(mode),
Sender: doer.APIFormat(),
})
} else {
err = PrepareWebhooks(issue.Repo, HookEventIssues, &api.IssuePayload{
Action: api.HookIssueEdited,
Index: issue.Index,
Changes: &api.ChangesPayload{
Body: &api.ChangesFromPayload{
From: oldContent,
},
},
Issue: issue.APIFormat(),
Repository: issue.Repo.APIFormat(mode),
Sender: doer.APIFormat(),
})
}
if err != nil {
log.Error(4, "PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
} else {
go HookQueue.Add(issue.RepoID)
}

return nil
}

Expand Down Expand Up @@ -964,7 +811,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {

// Insert the assignees
for _, assigneeID := range opts.AssigneeIDs {
err = opts.Issue.changeAssignee(e, doer, assigneeID)
_, err = opts.Issue.changeAssignee(e, doer, assigneeID)
if err != nil {
return err
}
Expand Down Expand Up @@ -1049,36 +896,6 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []in
return fmt.Errorf("Commit: %v", err)
}

UpdateIssueIndexer(issue.ID)

if err = NotifyWatchers(&Action{
ActUserID: issue.Poster.ID,
ActUser: issue.Poster,
OpType: ActionCreateIssue,
Content: fmt.Sprintf("%d|%s", issue.Index, issue.Title),
RepoID: repo.ID,
Repo: repo,
IsPrivate: repo.IsPrivate,
}); err != nil {
log.Error(4, "NotifyWatchers: %v", err)
}
if err = issue.MailParticipants(); err != nil {
log.Error(4, "MailParticipants: %v", err)
}

mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
if err = PrepareWebhooks(repo, HookEventIssues, &api.IssuePayload{
Action: api.HookIssueOpened,
Index: issue.Index,
Issue: issue.APIFormat(),
Repository: repo.APIFormat(mode),
Sender: issue.Poster.APIFormat(),
}); err != nil {
log.Error(4, "PrepareWebhooks: %v", err)
} else {
go HookQueue.Add(issue.RepoID)
}

return nil
}

Expand Down
Loading