Refactor comments to support markup types (#707)

Added a `markup` column in the `comments` table using
a migration script.
When creating or updating a comment, the default markup
is used if none was provided.
When showing comments, the default markup is also returned
if none was found in the database (ie, for older comments).

Signed-off-by: Xavier Coulon <xcoulon@redhat.com>

comment/comment.go

@@ -21,6 +21,7 @@ type Comment struct {
 	ParentID  string
 	CreatedBy uuid.UUID `sql:"type:uuid"` // Belongs To Identity
 	Body      string
+	Markup    string
 }
 
 // Repository describes interactions with comments

comment/comment_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/almighty/almighty-core/comment"
 	"github.com/almighty/almighty-core/gormsupport"
 	"github.com/almighty/almighty-core/gormsupport/cleaner"
+	"github.com/almighty/almighty-core/rendering"
 	"github.com/almighty/almighty-core/resource"
 	uuid "github.com/satori/go.uuid"
 	"github.com/stretchr/testify/suite"
@@ -36,20 +37,17 @@ func (test *TestCommentRepository) TearDownTest() {
 func (test *TestCommentRepository) TestCreateComment() {
 	t := test.T()
 	resource.Require(t, resource.Database)
-
 	repo := comment.NewCommentRepository(test.DB)
-
 	c := &comment.Comment{
 		ParentID:  "A",
 		Body:      "Test A",
+		Markup:    rendering.SystemMarkupMarkdown,
 		CreatedBy: uuid.NewV4(),
 	}
-
 	repo.Create(context.Background(), c)
 	if c.ID == uuid.Nil {
 		t.Errorf("Comment was not created, ID nil")
 	}
-
 	if c.CreatedAt.After(time.Now()) {
 		t.Errorf("Comment was not created, CreatedAt after Now()?")
 	}
@@ -58,22 +56,20 @@ func (test *TestCommentRepository) TestCreateComment() {
 func (test *TestCommentRepository) TestSaveComment() {
 	t := test.T()
 	resource.Require(t, resource.Database)
-
 	repo := comment.NewCommentRepository(test.DB)
-
 	parentID := "AA"
 	c := &comment.Comment{
 		ParentID:  parentID,
 		Body:      "Test AA",
+		Markup:    rendering.SystemMarkupPlainText,
 		CreatedBy: uuid.NewV4(),
 	}
-
 	repo.Create(context.Background(), c)
 	if c.ID == uuid.Nil {
 		t.Errorf("Comment was not created, ID nil")
 	}
-
 	c.Body = "Test AB"
+	c.Markup = rendering.SystemMarkupMarkdown
 	repo.Save(context.Background(), c)
 
 	offset := 0
@@ -88,7 +84,7 @@ func (test *TestCommentRepository) TestSaveComment() {
 	}
 
 	c1 := cl[0]
-	if c1.Body != "Test AB" {
+	if c1.Body != "Test AB" || c1.Markup != rendering.SystemMarkupMarkdown {
 		t.Error("List returned unexpected comment")
 	}
 

comments.go

@@ -8,6 +8,7 @@ import (
 	"github.com/almighty/almighty-core/comment"
 	"github.com/almighty/almighty-core/jsonapi"
 	"github.com/almighty/almighty-core/login"
+	"github.com/almighty/almighty-core/rendering"
 	"github.com/almighty/almighty-core/rest"
 	"github.com/goadesign/goa"
 )
@@ -60,6 +61,7 @@ func (c *CommentsController) Update(ctx *app.UpdateCommentsContext) error {
 		}
 
 		cm.Body = *ctx.Payload.Data.Attributes.Body
+		cm.Markup = rendering.NilSafeGetMarkup(ctx.Payload.Data.Attributes.Markup)
 		cm, err = appl.Comments().Save(ctx.Context, cm)
 		if err != nil {
 			return jsonapi.JSONErrorResponse(ctx, err)
@@ -109,11 +111,13 @@ func ConvertCommentResourceID(request *goa.RequestData, comment *comment.Comment
 // ConvertComment converts between internal and external REST representation
 func ConvertComment(request *goa.RequestData, comment *comment.Comment, additional ...CommentConvertFunc) *app.Comment {
 	selfURL := rest.AbsoluteURL(request, app.CommentsHref(comment.ID))
+	markup := rendering.NilSafeGetMarkup(&comment.Markup)
 	c := &app.Comment{
 		Type: "comments",
 		ID:   &comment.ID,
 		Attributes: &app.CommentAttributes{
 			Body:      &comment.Body,
+			Markup:    &markup,
 			CreatedAt: &comment.CreatedAt,
 		},
 		Relationships: &app.CommentRelations{

comments_blackbox_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/almighty/almighty-core/gormapplication"
 	"github.com/almighty/almighty-core/gormsupport"
 	"github.com/almighty/almighty-core/gormsupport/cleaner"
+	"github.com/almighty/almighty-core/rendering"
 	"github.com/almighty/almighty-core/resource"
 	testsupport "github.com/almighty/almighty-core/test"
 	almtoken "github.com/almighty/almighty-core/token"
@@ -45,6 +46,10 @@ func (s *CommentsSuite) TearDownTest() {
 	s.clean()
 }
 
+var markdownMarkup = rendering.SystemMarkupMarkdown
+var plaintextMarkup = rendering.SystemMarkupPlainText
+var defaultMarkup = rendering.SystemMarkupDefault
+
 func (s *CommentsSuite) unsecuredController() (*goa.Service, *CommentsController) {
 	svc := goa.New("Comments-service-test")
 	commentsCtrl := NewCommentsController(svc, s.db)
@@ -67,7 +72,7 @@ func (s *CommentsSuite) createWorkItem(identity account.Identity) string {
 		Data: &app.WorkItem2{
 			Type: APIStringTypeWorkItem,
 			Attributes: map[string]interface{}{
-				workitem.SystemTitle: "Title",
+				workitem.SystemTitle: "work item title",
 				workitem.SystemState: workitem.SystemStateNew},
 			Relationships: &app.WorkItemRelationships{
 				BaseType: &app.RelationBaseType{
@@ -86,101 +91,126 @@ func (s *CommentsSuite) createWorkItem(identity account.Identity) string {
 	return workitemId
 }
 
-// createWorkItemComment creates a workitem comment that will be used to perform the comment operations during the tests.
-func (s *CommentsSuite) createWorkItemComment(identity account.Identity, workitemId string) uuid.UUID {
-	createWorkItemCommentPayload := app.CreateWorkItemCommentsPayload{
+func newCreateWorkItemCommentsPayload(body string, markup *string) *app.CreateWorkItemCommentsPayload {
+	return &app.CreateWorkItemCommentsPayload{
 		Data: &app.CreateComment{
 			Type: "comments",
 			Attributes: &app.CreateCommentAttributes{
-				Body: "a comment",
+				Body:   body,
+				Markup: markup,
+			},
+		},
+	}
+}
+
+func newUpdateCommentsPayload(body string, markup *string) *app.UpdateCommentsPayload {
+	return &app.UpdateCommentsPayload{
+		Data: &app.Comment{
+			Type: "comments",
+			Attributes: &app.CommentAttributes{
+				Body:   &body,
+				Markup: markup,
 			},
 		},
 	}
+}
+
+// createWorkItemComment creates a workitem comment that will be used to perform the comment operations during the tests.
+func (s *CommentsSuite) createWorkItemComment(identity account.Identity, workitemId string, body string, markup *string) uuid.UUID {
+	createWorkItemCommentPayload := newCreateWorkItemCommentsPayload(body, markup)
 	userSvc, _, workitemCommentsCtrl, _ := s.securedControllers(identity)
-	_, comment := test.CreateWorkItemCommentsOK(s.T(), userSvc.Context, userSvc, workitemCommentsCtrl, workitemId,
-		&createWorkItemCommentPayload)
+	_, comment := test.CreateWorkItemCommentsOK(s.T(), userSvc.Context, userSvc, workitemCommentsCtrl, workitemId, createWorkItemCommentPayload)
 	commentId := *comment.Data.ID
 	s.T().Log(fmt.Sprintf("Created comment with id %v", commentId))
 	return commentId
 }
 
+func validateComment(t *testing.T, result *app.CommentSingle, expectedBody string, expectedMarkup string) {
+	require.NotNil(t, result)
+	require.NotNil(t, result.Data)
+	assert.NotNil(t, result.Data.ID)
+	assert.NotNil(t, result.Data.Type)
+	require.NotNil(t, result.Data.Attributes)
+	require.NotNil(t, result.Data.Attributes.Body)
+	assert.Equal(t, expectedBody, *result.Data.Attributes.Body)
+	require.NotNil(t, result.Data.Attributes.Markup)
+	assert.Equal(t, expectedMarkup, *result.Data.Attributes.Markup)
+	require.NotNil(t, result.Data.Relationships)
+	require.NotNil(t, result.Data.Relationships.CreatedBy)
+	require.NotNil(t, result.Data.Relationships.CreatedBy.Data)
+	require.NotNil(t, result.Data.Relationships.CreatedBy.Data.ID)
+	assert.Equal(t, testsupport.TestIdentity.ID, *result.Data.Relationships.CreatedBy.Data.ID)
+}
+
 func (s *CommentsSuite) TestShowCommentWithoutAuth() {
 	// given
 	workitemId := s.createWorkItem(testsupport.TestIdentity)
-	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId)
+	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId, "body", &markdownMarkup)
 	// when
 	userSvc, commentsCtrl := s.unsecuredController()
 	_, result := test.ShowCommentsOK(s.T(), userSvc.Context, userSvc, commentsCtrl, commentId)
 	// then
-	validateComment(s.T(), result)
+	validateComment(s.T(), result, "body", rendering.SystemMarkupMarkdown)
 }
 
-func (s *CommentsSuite) TestShowCommentWithAuth() {
+func (s *CommentsSuite) TestShowCommentWithoutAuthWithMarkup() {
 	// given
 	workitemId := s.createWorkItem(testsupport.TestIdentity)
-	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId)
+	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId, "body", nil)
 	// when
-	userSvc, _, _, commentsCtrl := s.securedControllers(testsupport.TestIdentity)
+	userSvc, commentsCtrl := s.unsecuredController()
 	_, result := test.ShowCommentsOK(s.T(), userSvc.Context, userSvc, commentsCtrl, commentId)
 	// then
-	validateComment(s.T(), result)
+	validateComment(s.T(), result, "body", rendering.SystemMarkupPlainText)
 }
 
-func validateComment(t *testing.T, result *app.CommentSingle) {
-	require.NotNil(t, result)
-	require.NotNil(t, result.Data)
-	assert.NotNil(t, result.Data.ID)
-	assert.NotNil(t, result.Data.Type)
-	require.NotNil(t, result.Data.Attributes)
-	require.NotNil(t, result.Data.Attributes.Body)
-	assert.Equal(t, "a comment", *result.Data.Attributes.Body)
-	require.NotNil(t, result.Data.Relationships)
-	require.NotNil(t, result.Data.Relationships.CreatedBy)
-	require.NotNil(t, result.Data.Relationships.CreatedBy.Data)
-	require.NotNil(t, result.Data.Relationships.CreatedBy.Data.ID)
-	assert.Equal(t, testsupport.TestIdentity.ID, *result.Data.Relationships.CreatedBy.Data.ID)
+func (s *CommentsSuite) TestShowCommentWithAuth() {
+	// given
+	workitemId := s.createWorkItem(testsupport.TestIdentity)
+	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId, "body", &plaintextMarkup)
+	// when
+	userSvc, _, _, commentsCtrl := s.securedControllers(testsupport.TestIdentity)
+	_, result := test.ShowCommentsOK(s.T(), userSvc.Context, userSvc, commentsCtrl, commentId)
+	// then
+	validateComment(s.T(), result, "body", rendering.SystemMarkupPlainText)
 }
 
 func (s *CommentsSuite) TestUpdateCommentWithoutAuth() {
 	// given
 	workitemId := s.createWorkItem(testsupport.TestIdentity)
-	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId)
+	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId, "body", &plaintextMarkup)
 	// when
-	updatedCommentBody := "An updated comment"
-	updateCommentPayload := app.UpdateCommentsPayload{
-		Data: &app.Comment{
-			Type: "comments",
-			Attributes: &app.CommentAttributes{
-				Body: &updatedCommentBody,
-			},
-		},
-	}
+	updateCommentPayload := newUpdateCommentsPayload("updated body", &markdownMarkup)
 	userSvc, commentsCtrl := s.unsecuredController()
-	test.UpdateCommentsUnauthorized(s.T(), userSvc.Context, userSvc, commentsCtrl, commentId, &updateCommentPayload)
+	test.UpdateCommentsUnauthorized(s.T(), userSvc.Context, userSvc, commentsCtrl, commentId, updateCommentPayload)
 }
 
-func (s *CommentsSuite) TestUpdateCommentWithSameAuthenticatedUser() {
+func (s *CommentsSuite) TestUpdateCommentWithSameUserWithOtherMarkup() {
 	// given
 	workitemId := s.createWorkItem(testsupport.TestIdentity)
-	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId)
+	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId, "body", &plaintextMarkup)
 	// when
-	updatedCommentBody := "An updated comment"
-	updateCommentPayload := app.UpdateCommentsPayload{
-		Data: &app.Comment{
-			Type: "comments",
-			Attributes: &app.CommentAttributes{
-				Body: &updatedCommentBody,
-			},
-		},
-	}
+	updateCommentPayload := newUpdateCommentsPayload("updated body", &markdownMarkup)
+	userSvc, _, _, commentsCtrl := s.securedControllers(testsupport.TestIdentity)
+	_, result := test.UpdateCommentsOK(s.T(), userSvc.Context, userSvc, commentsCtrl, commentId, updateCommentPayload)
+	validateComment(s.T(), result, "updated body", rendering.SystemMarkupMarkdown)
+}
+
+func (s *CommentsSuite) TestUpdateCommentWithSameUserWithNilMarkup() {
+	// given
+	workitemId := s.createWorkItem(testsupport.TestIdentity)
+	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId, "body", &plaintextMarkup)
+	// when
+	updateCommentPayload := newUpdateCommentsPayload("updated body", nil)
 	userSvc, _, _, commentsCtrl := s.securedControllers(testsupport.TestIdentity)
-	test.UpdateCommentsOK(s.T(), userSvc.Context, userSvc, commentsCtrl, commentId, &updateCommentPayload)
+	_, result := test.UpdateCommentsOK(s.T(), userSvc.Context, userSvc, commentsCtrl, commentId, updateCommentPayload)
+	validateComment(s.T(), result, "updated body", rendering.SystemMarkupDefault)
 }
 
-func (s *CommentsSuite) TestUpdateCommentWithOtherAuthenticatedUser() {
+func (s *CommentsSuite) TestUpdateCommentWithOtherUser() {
 	// given
 	workitemId := s.createWorkItem(testsupport.TestIdentity)
-	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId)
+	commentId := s.createWorkItemComment(testsupport.TestIdentity, workitemId, "body", &plaintextMarkup)
 	// when
 	updatedCommentBody := "An updated comment"
 	updateCommentPayload := app.UpdateCommentsPayload{

design/comments.go

@@ -36,6 +36,9 @@ var commentAttributes = a.Type("CommentAttributes", func() {
 	a.Attribute("body", d.String, "The comment body", func() {
 		a.Example("This is really interesting")
 	})
+	a.Attribute("markup", d.String, "The comment markup associated with the body", func() {
+		a.Example("Markdown")
+	})
 })
 
 var createCommentAttributes = a.Type("CreateCommentAttributes", func() {
@@ -44,6 +47,9 @@ var createCommentAttributes = a.Type("CreateCommentAttributes", func() {
 		a.MinLength(1) // Empty comment not allowed
 		a.Example("This is really interesting")
 	})
+	a.Attribute("markup", d.String, "The comment markup associated with the body", func() {
+		a.Example("Markdown")
+	})
 	a.Required("body")
 })
 

migration/migration.go

@@ -145,6 +145,9 @@ func getMigrations() migrations {
 	// Version 22
 	m = append(m, steps{executeSQLFile("022-work-item-description-update.sql")})
 
+	// Version 23
+	m = append(m, steps{executeSQLFile("023-comment-markup.sql")})
+
 	// Version N
 	//
 	// In order to add an upgrade, simply append an array of MigrationFunc to the

migration/sql-files/023-comment-markup.sql

@@ -0,0 +1,2 @@
+-- add a 'markup' column in the 'comments' table
+alter table comments add column markup text; 

rendering/markup_content.go

@@ -67,3 +67,11 @@ func NewMarkupContentFromValue(value interface{}) *MarkupContent {
 		return nil
 	}
 }
+
+// NilSafeGetMarkup returns the given markup if it is not nil nor empty, otherwise it returns the default markup
+func NilSafeGetMarkup(markup *string) string {
+	if markup != nil && *markup != "" {
+		return *markup
+	}
+	return SystemMarkupDefault
+}

rendering/markup_content_test.go

@@ -0,0 +1,33 @@
+package rendering
+
+import "testing"
+import "github.com/stretchr/testify/assert"
+import "github.com/stretchr/testify/require"
+
+func TestGetDefaultMarkupFromNil(t *testing.T) {
+	// when
+	result := NilSafeGetMarkup(nil)
+	// then
+	require.NotNil(t, result)
+	assert.Equal(t, SystemMarkupDefault, result)
+}
+
+func TestGetMarkupFromValue(t *testing.T) {
+	// given
+	markup := SystemMarkupMarkdown
+	// when
+	result := NilSafeGetMarkup(&markup)
+	// then
+	require.NotNil(t, result)
+	assert.Equal(t, markup, result)
+}
+
+func TestGetMarkupFromEmptyValue(t *testing.T) {
+	// given
+	markup := ""
+	// when
+	result := NilSafeGetMarkup(&markup)
+	// then
+	require.NotNil(t, result)
+	assert.Equal(t, SystemMarkupDefault, result)
+}