#2246 add HTMLURL to webhook type

- Fill Milestone and Assignee field when available in webhook payload

models/action.go

@@ -520,7 +520,7 @@ func CommitRepoAction(
 			Before:     oldCommitID,
 			After:      newCommitID,
 			CompareURL: setting.AppUrl + commit.CompareURL,
-			Commits:    commit.ToApiPayloadCommits(repo.FullLink()),
+			Commits:    commit.ToApiPayloadCommits(repo.HTMLURL()),
 			Repo:       apiRepo,
 			Pusher:     apiPusher,
 			Sender:     apiPusher,

models/issue.go

@@ -32,23 +32,23 @@ var (
 type Issue struct {
 	ID              int64       `xorm:"pk autoincr"`
 	RepoID          int64       `xorm:"INDEX UNIQUE(repo_index)"`
-	Index           int64       `xorm:"UNIQUE(repo_index)"` // Index in one repository.
-	Title           string      `xorm:"name"`
 	Repo            *Repository `xorm:"-"`
+	Index           int64       `xorm:"UNIQUE(repo_index)"` // Index in one repository.
 	PosterID        int64
 	Poster          *User    `xorm:"-"`
+	Title           string   `xorm:"name"`
+	Content         string   `xorm:"TEXT"`
+	RenderedContent string   `xorm:"-"`
 	Labels          []*Label `xorm:"-"`
 	MilestoneID     int64
 	Milestone       *Milestone `xorm:"-"`
+	Priority        int
 	AssigneeID      int64
 	Assignee        *User `xorm:"-"`
-	IsRead          bool  `xorm:"-"`
-	IsPull          bool  // Indicates whether is a pull request or not.
-	*PullRequest    `xorm:"-"`
 	IsClosed        bool
-	Content         string `xorm:"TEXT"`
-	RenderedContent string `xorm:"-"`
-	Priority        int
+	IsRead          bool         `xorm:"-"`
+	IsPull          bool         // Indicates whether is a pull request or not.
+	PullRequest     *PullRequest `xorm:"-"`
 	NumComments     int
 
 	Deadline     time.Time `xorm:"-"`
@@ -155,6 +155,16 @@ func (issue *Issue) LoadAttributes() error {
 	return issue.loadAttributes(x)
 }
 
+func (issue *Issue) HTMLURL() string {
+	var path string
+	if issue.IsPull {
+		path = "pulls"
+	} else {
+		path = "issues"
+	}
+	return fmt.Sprintf("%s/%s/%d", issue.Repo.HTMLURL(), path, issue.Index)
+}
+
 // State returns string representation of issue status.
 func (i *Issue) State() api.StateType {
 	if i.IsClosed {
@@ -175,11 +185,11 @@ func (issue *Issue) APIFormat() *api.Issue {
 	apiIssue := &api.Issue{
 		ID:       issue.ID,
 		Index:    issue.Index,
-		State:    issue.State(),
+		Poster:   issue.Poster.APIFormat(),
 		Title:    issue.Title,
 		Body:     issue.Content,
-		User:     issue.Poster.APIFormat(),
 		Labels:   apiLabels,
+		State:    issue.State(),
 		Comments: issue.NumComments,
 		Created:  issue.Created,
 		Updated:  issue.Updated,
@@ -208,16 +218,6 @@ func (i *Issue) HashTag() string {
 	return "issue-" + com.ToStr(i.ID)
 }
 
-func (issue *Issue) FullLink() string {
-	var path string
-	if issue.IsPull {
-		path = "pulls"
-	} else {
-		path = "issues"
-	}
-	return fmt.Sprintf("%s/%s/%d", issue.Repo.FullLink(), path, issue.Index)
-}
-
 // IsPoster returns true if given user by ID is the poster.
 func (i *Issue) IsPoster(uid int64) bool {
 	return i.PosterID == uid
@@ -591,16 +591,44 @@ func newIssue(e *xorm.Session, opts NewIssueOptions) (err error) {
 	opts.Issue.Title = strings.TrimSpace(opts.Issue.Title)
 	opts.Issue.Index = opts.Repo.NextIssueIndex()
 
-	if opts.Issue.AssigneeID > 0 {
-		// Silently drop invalid assignee.
-		valid, err := hasAccess(e, &User{ID: opts.Issue.AssigneeID}, opts.Repo, ACCESS_MODE_WRITE)
-		if err != nil {
-			return fmt.Errorf("hasAccess [user_id: %d, repo_id: %d]: %v", opts.Issue.AssigneeID, opts.Repo.ID, err)
-		} else if !valid {
-			opts.Issue.AssigneeID = 0
+	if opts.Issue.MilestoneID > 0 {
+		milestone, err := getMilestoneByID(e, opts.Issue.MilestoneID)
+		if err != nil && !IsErrMilestoneNotExist(err) {
+			return fmt.Errorf("getMilestoneByID: %v", err)
+		}
+
+		// Assume milestone is invalid and drop silently.
+		opts.Issue.MilestoneID = 0
+		if milestone != nil {
+			opts.Issue.MilestoneID = milestone.ID
+			opts.Issue.Milestone = milestone
+			if err = changeMilestoneAssign(e, opts.Issue, -1); err != nil {
+				return err
+			}
 		}
 	}
 
+	if opts.Issue.AssigneeID > 0 {
+		assignee, err := getUserByID(e, opts.Issue.AssigneeID)
+		if err != nil && !IsErrUserNotExist(err) {
+			return fmt.Errorf("getUserByID: %v", err)
+		}
+
+		// Assume assignee is invalid and drop silently.
+		opts.Issue.AssigneeID = 0
+		if assignee != nil {
+			valid, err := hasAccess(e, assignee, opts.Repo, ACCESS_MODE_WRITE)
+			if err != nil {
+				return fmt.Errorf("hasAccess [user_id: %d, repo_id: %d]: %v", assignee.ID, opts.Repo.ID, err)
+			}
+			if valid {
+				opts.Issue.AssigneeID = assignee.ID
+				opts.Issue.Assignee = assignee
+			}
+		}
+	}
+
+	// Milestone and assignee validation should happen before insert actual object.
 	if _, err = e.Insert(opts.Issue); err != nil {
 		return err
 	}
@@ -634,12 +662,6 @@ func newIssue(e *xorm.Session, opts NewIssueOptions) (err error) {
 		}
 	}
 
-	if opts.Issue.MilestoneID > 0 {
-		if err = changeMilestoneAssign(e, opts.Issue, -1); err != nil {
-			return err
-		}
-	}
-
 	if err = newIssueUsers(e, opts.Repo, opts.Issue); err != nil {
 		return err
 	}

models/mail.go

@@ -129,7 +129,7 @@ func SendCollaboratorMail(u, doer *User, repo *Repository) {
 	data := map[string]interface{}{
 		"Subject":  subject,
 		"RepoName": repoName,
-		"Link":     repo.FullLink(),
+		"Link":     repo.HTMLURL(),
 	}
 	body, err := mailRender.HTMLString(string(MAIL_NOTIFY_COLLABORATOR), data)
 	if err != nil {
@@ -153,8 +153,8 @@ func composeTplData(subject, body, link string) map[string]interface{} {
 
 func composeIssueMessage(issue *Issue, doer *User, tplName base.TplName, tos []string, info string) *mailer.Message {
 	subject := issue.MailSubject()
-	body := string(markdown.RenderSpecialLink([]byte(issue.Content), issue.Repo.FullLink(), issue.Repo.ComposeMetas()))
-	data := composeTplData(subject, body, issue.FullLink())
+	body := string(markdown.RenderSpecialLink([]byte(issue.Content), issue.Repo.HTMLURL(), issue.Repo.ComposeMetas()))
+	data := composeTplData(subject, body, issue.HTMLURL())
 	data["Doer"] = doer
 	content, err := mailRender.HTMLString(string(tplName), data)
 	if err != nil {

models/pull.go

@@ -100,6 +100,10 @@ func (pr *PullRequest) LoadAttributes() error {
 }
 
 func (pr *PullRequest) LoadIssue() (err error) {
+	if pr.Issue != nil {
+		return nil
+	}
+
 	pr.Issue, err = GetIssueByID(pr.IssueID)
 	return err
 }
@@ -112,14 +116,15 @@ func (pr *PullRequest) APIFormat() *api.PullRequest {
 	apiPullRequest := &api.PullRequest{
 		ID:        pr.ID,
 		Index:     pr.Index,
-		State:     apiIssue.State,
+		Poster:    apiIssue.Poster,
 		Title:     apiIssue.Title,
 		Body:      apiIssue.Body,
-		User:      apiIssue.User,
 		Labels:    apiIssue.Labels,
 		Milestone: apiIssue.Milestone,
 		Assignee:  apiIssue.Assignee,
+		State:     apiIssue.State,
 		Comments:  apiIssue.Comments,
+		HTMLURL:   pr.Issue.HTMLURL(),
 		HasMerged: pr.HasMerged,
 	}
 
@@ -312,7 +317,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
 		Before:     pr.MergeBase,
 		After:      pr.MergedCommitID,
 		CompareURL: setting.AppUrl + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID),
-		Commits:    ListToPushCommits(l).ToApiPayloadCommits(pr.BaseRepo.FullLink()),
+		Commits:    ListToPushCommits(l).ToApiPayloadCommits(pr.BaseRepo.HTMLURL()),
 		Repo:       pr.BaseRepo.APIFormat(nil),
 		Pusher:     pr.HeadRepo.MustOwner().APIFormat(),
 		Sender:     doer.APIFormat(),

models/repo.go

@@ -233,7 +233,7 @@ func (repo *Repository) FullName() string {
 	return repo.MustOwner().Name + "/" + repo.Name
 }
 
-func (repo *Repository) FullLink() string {
+func (repo *Repository) HTMLURL() string {
 	return setting.AppUrl + repo.FullName()
 }
 
@@ -248,7 +248,7 @@ func (repo *Repository) APIFormat(permission *api.Permission) *api.Repository {
 		Description:   repo.Description,
 		Private:       repo.IsPrivate,
 		Fork:          repo.IsFork,
-		HTMLURL:       repo.FullLink(),
+		HTMLURL:       repo.HTMLURL(),
 		SSHURL:        cloneLink.SSH,
 		CloneURL:      cloneLink.HTTPS,
 		Website:       repo.Website,