diff --git a/internal/api/controller/pullreq/comment_create.go b/internal/api/controller/pullreq/comment_create.go new file mode 100644 index 000000000..27843a46a --- /dev/null +++ b/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 +} diff --git a/internal/api/controller/pullreq/controller.go b/internal/api/controller/pullreq/controller.go index 860f4c698..61a65c5c9 100644 --- a/internal/api/controller/pullreq/controller.go +++ b/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 } diff --git a/internal/api/controller/pullreq/update.go b/internal/api/controller/pullreq/update.go index 1576ded13..852ac5a3c 100644 --- a/internal/api/controller/pullreq/update.go +++ b/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 diff --git a/internal/api/handler/pullreq/comment_create.go b/internal/api/handler/pullreq/comment_create.go new file mode 100644 index 000000000..188e7a4ca --- /dev/null +++ b/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) + } +} diff --git a/internal/api/openapi/pullreq.go b/internal/api/openapi/pullreq.go index 3b8270690..93927f230 100644 --- a/internal/api/openapi/pullreq.go +++ b/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) } diff --git a/internal/router/api.go b/internal/router/api.go index 4115bf59c..a4af2d734 100644 --- a/internal/router/api.go +++ b/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)) }) }) } diff --git a/internal/store/database/pullreq_activity.go b/internal/store/database/pullreq_activity.go index bd899c0cf..d5f1211d1 100644 --- a/internal/store/database/pullreq_activity.go +++ b/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. diff --git a/types/pullreq.go b/types/pullreq.go index 1f5096c24..c2cd6b79a 100644 --- a/types/pullreq.go +++ b/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"`