[Refactor] move APIFormat() of Issue and Label to convert package (#10423)

* Label: delete .APIFormat() and use convert.ToLabel()

* move issue APIFormat too

* add missing one

* move TEST too

* handle error -> return empty APIIssue

* Apply suggestions from code review

Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>

4 files changed, 142 insertions, 13 deletions

diff --git a/modules/convert/issue.go b/modules/convert/issue.go
index d88a742dbb..d0985b6be1 100644
--- a/modules/convert/issue.go
+++ b/modules/convert/issue.go
@@ -5,10 +5,96 @@
 package convert
 
 import (
+	"strings"
+
 	"code.gitea.io/gitea/models"
 	api "code.gitea.io/gitea/modules/structs"
 )
 
+// ToAPIIssue converts an Issue to API format
+// it assumes some fields assigned with values:
+// Required - Poster, Labels,
+// Optional - Milestone, Assignee, PullRequest
+func ToAPIIssue(issue *models.Issue) *api.Issue {
+	if err := issue.LoadLabels(); err != nil {
+		return &api.Issue{}
+	}
+	if err := issue.LoadPoster(); err != nil {
+		return &api.Issue{}
+	}
+	if err := issue.LoadRepo(); err != nil {
+		return &api.Issue{}
+	}
+
+	apiIssue := &api.Issue{
+		ID:       issue.ID,
+		URL:      issue.APIURL(),
+		HTMLURL:  issue.HTMLURL(),
+		Index:    issue.Index,
+		Poster:   issue.Poster.APIFormat(),
+		Title:    issue.Title,
+		Body:     issue.Content,
+		Labels:   ToLabelList(issue.Labels),
+		State:    issue.State(),
+		Comments: issue.NumComments,
+		Created:  issue.CreatedUnix.AsTime(),
+		Updated:  issue.UpdatedUnix.AsTime(),
+	}
+
+	apiIssue.Repo = &api.RepositoryMeta{
+		ID:       issue.Repo.ID,
+		Name:     issue.Repo.Name,
+		Owner:    issue.Repo.OwnerName,
+		FullName: issue.Repo.FullName(),
+	}
+
+	if issue.ClosedUnix != 0 {
+		apiIssue.Closed = issue.ClosedUnix.AsTimePtr()
+	}
+
+	if err := issue.LoadMilestone(); err != nil {
+		return &api.Issue{}
+	}
+	if issue.Milestone != nil {
+		apiIssue.Milestone = issue.Milestone.APIFormat()
+	}
+
+	if err := issue.LoadAssignees(); err != nil {
+		return &api.Issue{}
+	}
+	if len(issue.Assignees) > 0 {
+		for _, assignee := range issue.Assignees {
+			apiIssue.Assignees = append(apiIssue.Assignees, assignee.APIFormat())
+		}
+		apiIssue.Assignee = issue.Assignees[0].APIFormat() // For compatibility, we're keeping the first assignee as `apiIssue.Assignee`
+	}
+	if issue.IsPull {
+		if err := issue.LoadPullRequest(); err != nil {
+			return &api.Issue{}
+		}
+		apiIssue.PullRequest = &api.PullRequestMeta{
+			HasMerged: issue.PullRequest.HasMerged,
+		}
+		if issue.PullRequest.HasMerged {
+			apiIssue.PullRequest.Merged = issue.PullRequest.MergedUnix.AsTimePtr()
+		}
+	}
+	if issue.DeadlineUnix != 0 {
+		apiIssue.Deadline = issue.DeadlineUnix.AsTimePtr()
+	}
+
+	return apiIssue
+}
+
+// ToAPIIssueList converts an IssueList to API format
+func ToAPIIssueList(il models.IssueList) []*api.Issue {
+	result := make([]*api.Issue, len(il))
+	for i := range il {
+		result[i] = ToAPIIssue(il[i])
+	}
+	return result
+}
+
 // ToTrackedTime converts TrackedTime to API format
 func ToTrackedTime(t *models.TrackedTime) (apiT *api.TrackedTime) {
 	apiT = &api.TrackedTime{
@@ -20,7 +106,7 @@ func ToTrackedTime(t *models.TrackedTime) (apiT *api.TrackedTime) {
 		Created:  t.Created,
 	}
 	if t.Issue != nil {
-		apiT.Issue = t.Issue.APIFormat()
+		apiT.Issue = ToAPIIssue(t.Issue)
 	}
 	if t.User != nil {
 		apiT.UserName = t.User.Name
@@ -36,3 +122,22 @@ func ToTrackedTimeList(tl models.TrackedTimeList) api.TrackedTimeList {
 	}
 	return result
 }
+
+// ToLabel converts Label to API format
+func ToLabel(label *models.Label) *api.Label {
+	return &api.Label{
+		ID:          label.ID,
+		Name:        label.Name,
+		Color:       strings.TrimLeft(label.Color, "#"),
+		Description: label.Description,
+	}
+}
+
+// ToLabelList converts list of Label to API format
+func ToLabelList(labels []*models.Label) []*api.Label {
+	result := make([]*api.Label, len(labels))
+	for i := range labels {
+		result[i] = ToLabel(labels[i])
+	}
+	return result
+}
diff --git a/modules/convert/issue_test.go b/modules/convert/issue_test.go
new file mode 100644
index 0000000000..a7286d0766
--- /dev/null
+++ b/modules/convert/issue_test.go
@@ -0,0 +1,24 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package convert
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models"
+	api "code.gitea.io/gitea/modules/structs"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLabel_ToLabel(t *testing.T) {
+	assert.NoError(t, models.PrepareTestDatabase())
+	label := models.AssertExistsAndLoadBean(t, &models.Label{ID: 1}).(*models.Label)
+	assert.Equal(t, &api.Label{
+		ID:    label.ID,
+		Name:  label.Name,
+		Color: "abcdef",
+	}, ToLabel(label))
+}
diff --git a/modules/convert/pull.go b/modules/convert/pull.go
index fa22977d02..cf2bf88aa8 100644
--- a/modules/convert/pull.go
+++ b/modules/convert/pull.go
@@ -31,7 +31,7 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
 		return nil
 	}
 
-	apiIssue := pr.Issue.APIFormat()
+	apiIssue := ToAPIIssue(pr.Issue)
 	if pr.BaseRepo == nil {
 		pr.BaseRepo, err = models.GetRepositoryByID(pr.BaseRepoID)
 		if err != nil {
diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go
index c1795d537b..f598d5b1f7 100644
--- a/modules/notification/webhook/webhook.go
+++ b/modules/notification/webhook/webhook.go
@@ -59,7 +59,7 @@ func (m *webhookNotifier) NotifyIssueClearLabels(doer *models.User, issue *model
 		err = webhook_module.PrepareWebhooks(issue.Repo, models.HookEventIssues, &api.IssuePayload{
 			Action:     api.HookIssueLabelCleared,
 			Index:      issue.Index,
-			Issue:      issue.APIFormat(),
+			Issue:      convert.ToAPIIssue(issue),
 			Repository: issue.Repo.APIFormat(mode),
 			Sender:     doer.APIFormat(),
 		})
@@ -155,7 +155,7 @@ func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *mo
 		mode, _ := models.AccessLevelUnit(doer, issue.Repo, models.UnitTypeIssues)
 		apiIssue := &api.IssuePayload{
 			Index:      issue.Index,
-			Issue:      issue.APIFormat(),
+			Issue:      convert.ToAPIIssue(issue),
 			Repository: issue.Repo.APIFormat(mode),
 			Sender:     doer.APIFormat(),
 		}
@@ -202,7 +202,7 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(doer *models.User, issue *model
 					From: oldTitle,
 				},
 			},
-			Issue:      issue.APIFormat(),
+			Issue:      convert.ToAPIIssue(issue),
 			Repository: issue.Repo.APIFormat(mode),
 			Sender:     issue.Poster.APIFormat(),
 		})
@@ -237,7 +237,7 @@ func (m *webhookNotifier) NotifyIssueChangeStatus(doer *models.User, issue *mode
 	} else {
 		apiIssue := &api.IssuePayload{
 			Index:      issue.Index,
-			Issue:      issue.APIFormat(),
+			Issue:      convert.ToAPIIssue(issue),
 			Repository: issue.Repo.APIFormat(mode),
 			Sender:     doer.APIFormat(),
 		}
@@ -267,7 +267,7 @@ func (m *webhookNotifier) NotifyNewIssue(issue *models.Issue) {
 	if err := webhook_module.PrepareWebhooks(issue.Repo, models.HookEventIssues, &api.IssuePayload{
 		Action:     api.HookIssueOpened,
 		Index:      issue.Index,
-		Issue:      issue.APIFormat(),
+		Issue:      convert.ToAPIIssue(issue),
 		Repository: issue.Repo.APIFormat(mode),
 		Sender:     issue.Poster.APIFormat(),
 	}); err != nil {
@@ -327,7 +327,7 @@ func (m *webhookNotifier) NotifyIssueChangeContent(doer *models.User, issue *mod
 					From: oldContent,
 				},
 			},
-			Issue:      issue.APIFormat(),
+			Issue:      convert.ToAPIIssue(issue),
 			Repository: issue.Repo.APIFormat(mode),
 			Sender:     doer.APIFormat(),
 		})
@@ -355,7 +355,7 @@ func (m *webhookNotifier) NotifyUpdateComment(doer *models.User, c *models.Comme
 	mode, _ := models.AccessLevel(doer, c.Issue.Repo)
 	if err := webhook_module.PrepareWebhooks(c.Issue.Repo, models.HookEventIssueComment, &api.IssueCommentPayload{
 		Action:  api.HookIssueCommentEdited,
-		Issue:   c.Issue.APIFormat(),
+		Issue:   convert.ToAPIIssue(c.Issue),
 		Comment: c.APIFormat(),
 		Changes: &api.ChangesPayload{
 			Body: &api.ChangesFromPayload{
@@ -375,7 +375,7 @@ func (m *webhookNotifier) NotifyCreateIssueComment(doer *models.User, repo *mode
 	mode, _ := models.AccessLevel(doer, repo)
 	if err := webhook_module.PrepareWebhooks(repo, models.HookEventIssueComment, &api.IssueCommentPayload{
 		Action:     api.HookIssueCommentCreated,
-		Issue:      issue.APIFormat(),
+		Issue:      convert.ToAPIIssue(issue),
 		Comment:    comment.APIFormat(),
 		Repository: repo.APIFormat(mode),
 		Sender:     doer.APIFormat(),
@@ -404,7 +404,7 @@ func (m *webhookNotifier) NotifyDeleteComment(doer *models.User, comment *models
 
 	if err := webhook_module.PrepareWebhooks(comment.Issue.Repo, models.HookEventIssueComment, &api.IssueCommentPayload{
 		Action:     api.HookIssueCommentDeleted,
-		Issue:      comment.Issue.APIFormat(),
+		Issue:      convert.ToAPIIssue(comment.Issue),
 		Comment:    comment.APIFormat(),
 		Repository: comment.Issue.Repo.APIFormat(mode),
 		Sender:     doer.APIFormat(),
@@ -449,7 +449,7 @@ func (m *webhookNotifier) NotifyIssueChangeLabels(doer *models.User, issue *mode
 		err = webhook_module.PrepareWebhooks(issue.Repo, models.HookEventIssues, &api.IssuePayload{
 			Action:     api.HookIssueLabelUpdated,
 			Index:      issue.Index,
-			Issue:      issue.APIFormat(),
+			Issue:      convert.ToAPIIssue(issue),
 			Repository: issue.Repo.APIFormat(mode),
 			Sender:     doer.APIFormat(),
 		})
@@ -491,7 +491,7 @@ func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *m
 		err = webhook_module.PrepareWebhooks(issue.Repo, models.HookEventIssues, &api.IssuePayload{
 			Action:     hookAction,
 			Index:      issue.Index,
-			Issue:      issue.APIFormat(),
+			Issue:      convert.ToAPIIssue(issue),
 			Repository: issue.Repo.APIFormat(mode),
 			Sender:     doer.APIFormat(),
 		})