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

@@ -15,6 +15,7 @@ import (
 	"github.com/almighty/almighty-core/gormsupport/cleaner"
 	"github.com/almighty/almighty-core/migration"
 	"github.com/almighty/almighty-core/models"
+	"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,7 +46,7 @@ type CommentsSuite struct {
 	pubKey               *rsa.PublicKey
 	priKey               *rsa.PrivateKey
 	clean                func()
-	commentId            uuid.UUID
+	workitemId           string
 }
 
 func (s *CommentsSuite) SetupSuite() {
@@ -104,55 +105,61 @@ func (s *CommentsSuite) SetupTest() {
 		},
 	}
 	_, wi := test.CreateWorkitemCreated(s.T(), s.userSvc.Context, s.userSvc, s.workitemCtrl, &createWorkitemPayload)
-	workitemId := *wi.Data.ID
-	// now, create a comment for the workitem
+	require.NotNil(s.T(), wi.Data.ID)
+	s.workitemId = *wi.Data.ID
+
+}
+
+func (s *CommentsSuite) TearDownTest() {
+}
+
+func (s *CommentsSuite) createComment(body string, markup *string) uuid.UUID {
 	createWorkItemCommentPayload := app.CreateWorkItemCommentsPayload{
 		Data: &app.CreateComment{
 			Type: "comments",
 			Attributes: &app.CreateCommentAttributes{
-				Body: "a comment",
+				Body:   body,
+				Markup: markup,
 			},
 		},
 	}
-	_, comment := test.CreateWorkItemCommentsOK(s.T(), s.userSvc.Context, s.userSvc, s.workitemCommentsCtrl, workitemId,
+	_, comment := test.CreateWorkItemCommentsOK(s.T(), s.userSvc.Context, s.userSvc, s.workitemCommentsCtrl, s.workitemId,
 		&createWorkItemCommentPayload)
-	s.commentId = *comment.Data.ID
-	s.T().Log(fmt.Sprintf("Created comment with id %v", s.commentId))
-}
-
-func (s *CommentsSuite) TearDownTest() {
+	commentId := *comment.Data.ID
+	s.T().Log(fmt.Sprintf("Created comment with id %v", commentId))
+	return commentId
 }
 
-func (s *CommentsSuite) TestShowCommentWithoutAuth() {
+func (s *CommentsSuite) TestShowCommentWithoutMarkupWithoutAuth() {
 	// given
+	commentId := s.createComment("a comment", nil)
 	commentsCtrl := NewCommentsController(s.defaultSvc, gormapplication.NewGormDB(s.db))
 	// when
-	_, result := test.ShowCommentsOK(s.T(), s.defaultSvc.Context, s.defaultSvc, commentsCtrl, s.commentId)
+	_, result := test.ShowCommentsOK(s.T(), s.defaultSvc.Context, s.defaultSvc, commentsCtrl, commentId)
 	// then
 	require.NotNil(s.T(), result)
 	require.NotNil(s.T(), result.Data)
 	assert.NotNil(s.T(), result.Data.ID)
 	assert.NotNil(s.T(), result.Data.Type)
 	assert.Equal(s.T(), "a comment", *result.Data.Attributes.Body)
+	assert.NotNil(s.T(), result.Data.Attributes.Markup)
+	assert.Equal(s.T(), rendering.SystemMarkupDefault, *result.Data.Attributes.Markup) // default markup is returned
 	assert.Equal(s.T(), account.TestIdentity.ID, *result.Data.Relationships.CreatedBy.Data.ID)
 }
 
-func (s *CommentsSuite) TestShowCommentWithAuth() {
+func (s *CommentsSuite) TestShowCommentWithMarkupWithAuth() {
 	// given
+	markup := rendering.SystemMarkupPlainText
+	commentId := s.createComment("a comment", &markup)
 	commentsCtrl := NewCommentsController(s.userSvc, gormapplication.NewGormDB(s.db))
-	// when
-	_, result := test.ShowCommentsOK(s.T(), s.userSvc.Context, s.userSvc, commentsCtrl, s.commentId)
-	// then
-	require.NotNil(s.T(), result)
-	require.NotNil(s.T(), result.Data)
-	assert.NotNil(s.T(), result.Data.ID)
-	assert.NotNil(s.T(), result.Data.Type)
-	assert.Equal(s.T(), "a comment", *result.Data.Attributes.Body)
-	assert.Equal(s.T(), account.TestIdentity.ID, *result.Data.Relationships.CreatedBy.Data.ID)
+	// when/then
+	test.ShowCommentsOK(s.T(), s.userSvc.Context, s.userSvc, commentsCtrl, commentId)
+	// let's assume that the content of the returned comment is already validated in the test above.
 }
 
 func (s *CommentsSuite) TestUpdateCommentWithoutAuth() {
 	// given
+	commentId := s.createComment("a comment", nil)
 	commentsCtrl := NewCommentsController(s.defaultSvc, gormapplication.NewGormDB(s.db))
 	updatedCommentBody := "An updated comment"
 	updateCommentPayload := app.UpdateCommentsPayload{
@@ -164,27 +171,63 @@ func (s *CommentsSuite) TestUpdateCommentWithoutAuth() {
 		},
 	}
 	// when/then
-	test.UpdateCommentsUnauthorized(s.T(), s.defaultSvc.Context, s.defaultSvc, commentsCtrl, s.commentId, &updateCommentPayload)
+	test.UpdateCommentsUnauthorized(s.T(), s.defaultSvc.Context, s.defaultSvc, commentsCtrl, commentId, &updateCommentPayload)
 }
 
 func (s *CommentsSuite) TestUpdateCommentWithSameAuthenticatedUser() {
 	// given
+	markup := rendering.SystemMarkupPlainText
+	commentId := s.createComment("a comment", &markup)
 	commentsCtrl := NewCommentsController(s.userSvc, gormapplication.NewGormDB(s.db))
 	updatedCommentBody := "An updated comment"
+	updatedCommentMarkup := rendering.SystemMarkupMarkdown
 	updateCommentPayload := app.UpdateCommentsPayload{
 		Data: &app.Comment{
 			Type: "comments",
 			Attributes: &app.CommentAttributes{
-				Body: &updatedCommentBody,
+				Body:   &updatedCommentBody,
+				Markup: &updatedCommentMarkup,
+			},
+		},
+	}
+	// when/then
+	_, result := test.UpdateCommentsOK(s.T(), s.userSvc.Context, s.userSvc, commentsCtrl, commentId, &updateCommentPayload)
+	require.NotNil(s.T(), result)
+	require.NotNil(s.T(), result.Data)
+	require.Equal(s.T(), commentId.String(), result.Data.ID.String())
+	require.NotNil(s.T(), result.Data.Attributes)
+	require.Equal(s.T(), updatedCommentBody, *result.Data.Attributes.Body)
+	require.Equal(s.T(), updatedCommentMarkup, *result.Data.Attributes.Markup)
+}
+
+func (s *CommentsSuite) TestUpdateCommentWithSameAuthenticatedUserAndNilMarkup() {
+	// given
+	markup := rendering.SystemMarkupPlainText
+	commentId := s.createComment("a comment", &markup)
+	commentsCtrl := NewCommentsController(s.userSvc, gormapplication.NewGormDB(s.db))
+	updatedCommentBody := "An updated comment"
+	updateCommentPayload := app.UpdateCommentsPayload{
+		Data: &app.Comment{
+			Type: "comments",
+			Attributes: &app.CommentAttributes{
+				Body:   &updatedCommentBody,
+				Markup: nil,
 			},
 		},
 	}
 	// when/then
-	test.UpdateCommentsOK(s.T(), s.userSvc.Context, s.userSvc, commentsCtrl, s.commentId, &updateCommentPayload)
+	_, result := test.UpdateCommentsOK(s.T(), s.userSvc.Context, s.userSvc, commentsCtrl, commentId, &updateCommentPayload)
+	require.NotNil(s.T(), result)
+	require.NotNil(s.T(), result.Data)
+	require.Equal(s.T(), commentId.String(), result.Data.ID.String())
+	require.NotNil(s.T(), result.Data.Attributes)
+	require.Equal(s.T(), updatedCommentBody, *result.Data.Attributes.Body)
+	require.Equal(s.T(), rendering.SystemMarkupDefault, *result.Data.Attributes.Markup) // default markup is saved and returned
 }
 
 func (s *CommentsSuite) TestUpdateCommentWithOtherAuthenticatedUser() {
 	// given
+	commentId := s.createComment("a comment", nil)
 	commentsCtrl := NewCommentsController(s.userSvc2, gormapplication.NewGormDB(s.db))
 	updatedCommentBody := "An updated comment"
 	updateCommentPayload := app.UpdateCommentsPayload{
@@ -196,5 +239,5 @@ func (s *CommentsSuite) TestUpdateCommentWithOtherAuthenticatedUser() {
 		},
 	}
 	// when/then
-	test.UpdateCommentsUnauthorized(s.T(), s.userSvc2.Context, s.userSvc2, commentsCtrl, s.commentId, &updateCommentPayload)
+	test.UpdateCommentsUnauthorized(s.T(), s.userSvc2.Context, s.userSvc2, commentsCtrl, commentId, &updateCommentPayload)
 }

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)
+}

work-item-comments.go

@@ -1,11 +1,14 @@
 package main
 
 import (
+	"fmt"
+
 	"github.com/almighty/almighty-core/app"
 	"github.com/almighty/almighty-core/application"
 	"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"
 	"github.com/pkg/errors"
@@ -41,12 +44,13 @@ func (c *WorkItemCommentsController) Create(ctx *app.CreateWorkItemCommentsConte
 		if err != nil {
 			return jsonapi.JSONErrorResponse(ctx, goa.ErrUnauthorized(err.Error()))
 		}
-
 		reqComment := ctx.Payload.Data
-
+		fmt.Println(fmt.Sprintf("Processing markup value: '%v'", reqComment.Attributes.Markup))
+		markup := rendering.NilSafeGetMarkup(reqComment.Attributes.Markup)
 		newComment := comment.Comment{
 			ParentID:  ctx.ID,
 			Body:      reqComment.Attributes.Body,
+			Markup:    markup,
 			CreatedBy: currentUserID,
 		}