create comment API (#137)

internal/api/controller/pullreq/comment_create.go

@@ -0,0 +1,115 @@
+// Copyright 2022 Harness Inc. All rights reserved.
+// Use of this source code is governed by the Polyform Free Trial License
+// that can be found in the LICENSE.md file for this repository.
+
+package pullreq
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"time"
+
+	"github.com/harness/gitness/internal/api/usererror"
+	"github.com/harness/gitness/internal/auth"
+	"github.com/harness/gitness/internal/store"
+	"github.com/harness/gitness/types"
+	"github.com/harness/gitness/types/enum"
+)
+
+type CommentCreateInput struct {
+	ParentID int64  `json:"parent_id"`
+	Text     string `json:"text"`
+}
+
+// CommentCreate creates a new pull request comment (pull request activity, type=comment).
+func (c *Controller) CommentCreate(
+	ctx context.Context,
+	session *auth.Session,
+	repoRef string,
+	prNum int64,
+	in *CommentCreateInput,
+) (*types.PullReqActivity, error) {
+	repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoEdit)
+	if err != nil {
+		return nil, fmt.Errorf("failed to acquire access to repo: %w", err)
+	}
+
+	pr, err := c.pullreqStore.FindByNumber(ctx, repo.ID, prNum)
+	if err != nil {
+		return nil, fmt.Errorf("failed to find pull request by number: %w", err)
+	}
+
+	act := getCommentActivity(session, pr, in)
+
+	if in.ParentID != 0 {
+		var parentAct *types.PullReqActivity
+		parentAct, err = c.checkIsReplyable(ctx, pr, in.ParentID)
+		if err != nil {
+			return nil, err
+		}
+		err = c.writeReplyActivity(ctx, parentAct, act)
+	} else {
+		err = c.writeActivity(ctx, pr, act)
+	}
+	if err != nil {
+		return nil, fmt.Errorf("failed to create comment: %w", err)
+	}
+
+	return act, nil
+}
+
+func (c *Controller) checkIsReplyable(ctx context.Context,
+	pr *types.PullReq, parentID int64) (*types.PullReqActivity, error) {
+	// make sure the parent comment exists, belongs to the same PR and isn't itself a reply
+	parentAct, err := c.pullreqActivityStore.Find(ctx, parentID)
+	if errors.Is(err, store.ErrResourceNotFound) || parentAct == nil {
+		return nil, usererror.BadRequest("Parent pull request activity not found.")
+	}
+	if err != nil {
+		return nil, fmt.Errorf("failed to find parent pull request activity: %w", err)
+	}
+
+	if parentAct.PullReqID != pr.ID || parentAct.RepoID != pr.TargetRepoID {
+		return nil, usererror.BadRequest("Parent pull request activity doesn't belong to the same pull request.")
+	}
+
+	if !parentAct.IsReplyable() {
+		return nil, usererror.BadRequest("Can't create a reply to the specified entry.")
+	}
+
+	return parentAct, nil
+}
+
+func getCommentActivity(session *auth.Session, pr *types.PullReq, in *CommentCreateInput) *types.PullReqActivity {
+	now := time.Now().UnixMilli()
+	act := &types.PullReqActivity{
+		ID:         0, // Will be populated in the data layer
+		Version:    0,
+		CreatedBy:  session.Principal.ID,
+		Created:    now,
+		Updated:    now,
+		Edited:     now,
+		Deleted:    nil,
+		RepoID:     pr.TargetRepoID,
+		PullReqID:  pr.ID,
+		Order:      0, // Will be filled in writeActivity/writeReplyActivity
+		SubOrder:   0, // Will be filled in writeReplyActivity
+		ReplySeq:   0,
+		Type:       enum.PullReqActivityTypeComment,
+		Kind:       enum.PullReqActivityKindComment,
+		Text:       in.Text,
+		Payload:    nil,
+		Metadata:   nil,
+		ResolvedBy: nil,
+		Resolved:   nil,
+		Author: types.PrincipalInfo{
+			ID:    session.Principal.ID,
+			UID:   session.Principal.UID,
+			Name:  session.Principal.DisplayName,
+			Email: session.Principal.Email,
+		},
+	}
+
+	return act
+}

internal/api/controller/pullreq/controller.go

@@ -19,7 +19,6 @@ import (
 	"github.com/harness/gitness/types/enum"
 
 	"github.com/jmoiron/sqlx"
-	"github.com/rs/zerolog/log"
 )
 
 type Controller struct {
@@ -90,23 +89,45 @@ func (c *Controller) getRepoCheckAccess(ctx context.Context,
 	return repo, nil
 }
 
-func (c *Controller) writeActivity(ctx context.Context,
-	pr *types.PullReq, act *types.PullReqActivity) (*types.PullReq, *types.PullReqActivity) {
+// writeActivity updates the PR's activity sequence number (using the optimistic locking mechanism),
+// sets the correct Order value and writes the activity to the database.
+// Even if the writing fails, the updating of the sequence number can succeed.
+func (c *Controller) writeActivity(ctx context.Context, pr *types.PullReq, act *types.PullReqActivity) error {
 	prUpd, err := c.pullreqStore.UpdateActivitySeq(ctx, pr)
 	if err != nil {
-		// non-critical error
-		log.Err(err).Msg("failed to get pull request activity number")
-		return pr, nil
+		return fmt.Errorf("failed to get pull request activity number: %w", err)
 	}
 
+	*pr = *prUpd // update the pull request object
+
 	act.Order = prUpd.ActivitySeq
 
 	err = c.pullreqActivityStore.Create(ctx, act)
 	if err != nil {
-		// non-critical error
-		log.Err(err).Msg("failed to create pull request activity")
-		return prUpd, nil
+		return fmt.Errorf("failed to create pull request activity: %w", err)
 	}
 
-	return prUpd, act
+	return nil
+}
+
+// writeReplyActivity updates the parent activity's reply sequence number (using the optimistic locking mechanism),
+// sets the correct Order and SubOrder values and writes the activity to the database.
+// Even if the writing fails, the updating of the sequence number can succeed.
+func (c *Controller) writeReplyActivity(ctx context.Context, parent, act *types.PullReqActivity) error {
+	parentUpd, err := c.pullreqActivityStore.UpdateReplySeq(ctx, parent)
+	if err != nil {
+		return fmt.Errorf("failed to get pull request activity number: %w", err)
+	}
+
+	*parent = *parentUpd // update the parent pull request activity object
+
+	act.Order = parentUpd.Order
+	act.SubOrder = parentUpd.ReplySeq
+
+	err = c.pullreqActivityStore.Create(ctx, act)
+	if err != nil {
+		return fmt.Errorf("failed to create pull request activity: %w", err)
+	}
+
+	return nil
 }

internal/api/controller/pullreq/update.go

@@ -16,6 +16,8 @@ import (
 	"github.com/harness/gitness/internal/store/database/dbtx"
 	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/enum"
+
+	"github.com/rs/zerolog/log"
 )
 
 type UpdateInput struct {
@@ -85,7 +87,11 @@ func (c *Controller) Update(ctx context.Context,
 
 	// Write a row to the pull request activity
 	if activity != nil {
-		pr, activity = c.writeActivity(ctx, pr, activity)
+		err = c.writeActivity(ctx, pr, activity)
+		if err != nil {
+			// non-critical error
+			log.Err(err).Msg("failed to write pull req activity")
+		}
 	}
 
 	return pr, nil

internal/api/handler/pullreq/comment_create.go

@@ -0,0 +1,49 @@
+// Copyright 2022 Harness Inc. All rights reserved.
+// Use of this source code is governed by the Polyform Free Trial License
+// that can be found in the LICENSE.md file for this repository.
+
+package pullreq
+
+import (
+	"encoding/json"
+	"net/http"
+
+	"github.com/harness/gitness/internal/api/controller/pullreq"
+	"github.com/harness/gitness/internal/api/render"
+	"github.com/harness/gitness/internal/api/request"
+)
+
+// HandleCommentCreate is an HTTP handler for creating a new pull request comment or a reply to a comment.
+func HandleCommentCreate(pullreqCtrl *pullreq.Controller) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		ctx := r.Context()
+		session, _ := request.AuthSessionFrom(ctx)
+
+		repoRef, err := request.GetRepoRefFromPath(r)
+		if err != nil {
+			render.TranslatedUserError(w, err)
+			return
+		}
+
+		pullreqNumber, err := request.GetPullReqNumberFromPath(r)
+		if err != nil {
+			render.TranslatedUserError(w, err)
+			return
+		}
+
+		in := new(pullreq.CommentCreateInput)
+		err = json.NewDecoder(r.Body).Decode(in)
+		if err != nil {
+			render.BadRequestf(w, "Invalid Request Body: %s.", err)
+			return
+		}
+
+		comment, err := pullreqCtrl.CommentCreate(ctx, session, repoRef, pullreqNumber, in)
+		if err != nil {
+			render.TranslatedUserError(w, err)
+			return
+		}
+
+		render.JSON(w, http.StatusOK, comment)
+	}
+}

internal/api/openapi/pullreq.go

@@ -44,6 +44,11 @@ type listPullReqActivitiesRequest struct {
 	pullReqRequest
 }
 
+type commentPullReqRequest struct {
+	pullReqRequest
+	pullreq.CommentCreateInput
+}
+
 var queryParameterQueryPullRequest = openapi3.ParameterOrRef{
 	Parameter: &openapi3.Parameter{
 		Name:        request.QueryParamQuery,
@@ -274,4 +279,16 @@ func pullReqOperations(reflector *openapi3.Reflector) {
 	_ = reflector.SetJSONResponse(&listPullReqActivities, new(usererror.Error), http.StatusForbidden)
 	_ = reflector.Spec.AddOperation(http.MethodGet,
 		"/repos/{repoRef}/pullreq/{pullreq_number}/activities", listPullReqActivities)
+
+	commentPullReq := openapi3.Operation{}
+	commentPullReq.WithTags("pullreq")
+	commentPullReq.WithMapOfAnything(map[string]interface{}{"operationId": "commentPullReq"})
+	_ = reflector.SetRequest(&commentPullReq, new(commentPullReqRequest), http.MethodPost)
+	_ = reflector.SetJSONResponse(&commentPullReq, new(types.PullReqActivity), http.StatusOK)
+	_ = reflector.SetJSONResponse(&commentPullReq, new(usererror.Error), http.StatusBadRequest)
+	_ = reflector.SetJSONResponse(&commentPullReq, new(usererror.Error), http.StatusInternalServerError)
+	_ = reflector.SetJSONResponse(&commentPullReq, new(usererror.Error), http.StatusUnauthorized)
+	_ = reflector.SetJSONResponse(&commentPullReq, new(usererror.Error), http.StatusForbidden)
+	_ = reflector.Spec.AddOperation(http.MethodPost,
+		"/repos/{repoRef}/pullreq/{pullreq_number}/comment", commentPullReq)
 }

internal/router/api.go

@@ -209,6 +209,7 @@ func setupPullReq(r chi.Router, pullreqCtrl *pullreq.Controller) {
 			r.Get("/", handlerpullreq.HandleFind(pullreqCtrl))
 			r.Put("/", handlerpullreq.HandleUpdate(pullreqCtrl))
 			r.Get("/activities", handlerpullreq.HandleListActivities(pullreqCtrl))
+			r.Post("/comment", handlerpullreq.HandleCommentCreate(pullreqCtrl))
 		})
 	})
 }

internal/store/database/pullreq_activity.go

@@ -102,8 +102,8 @@ const (
 	pullreqActivitySelectBase = `
 	SELECT` + pullreqActivityColumns + `
 	FROM pullreq_activities
-	INNER JOIN principals author on author.principal_id = pullreq_created_by
-	LEFT JOIN principals resolver on resolver.principal_id = journal_pullreq_merged_by`
+	INNER JOIN principals author on author.principal_id = pullreq_activity_created_by
+	LEFT JOIN principals resolver on resolver.principal_id = pullreq_activity_resolved_by`
 )
 
 // Find finds the pull request activity by id.

types/pullreq.go

@@ -86,6 +86,15 @@ type PullReqActivity struct {
 	Resolver *PrincipalInfo `json:"resolver"`
 }
 
+func (a *PullReqActivity) IsReplyable() bool {
+	return (a.Type == enum.PullReqActivityTypeComment || a.Type == enum.PullReqActivityTypeCodeComment) &&
+		a.SubOrder == 0
+}
+
+func (a *PullReqActivity) IsReply() bool {
+	return a.SubOrder > 0
+}
+
 // PullReqActivityFilter stores pull request activity query parameters.
 type PullReqActivityFilter struct {
 	Since int64 `json:"since"`