diff --git a/server/core/internal/db/document_comment.go b/server/core/internal/db/document_comment.go index 5a3692bb..5d6bfa5f 100644 --- a/server/core/internal/db/document_comment.go +++ b/server/core/internal/db/document_comment.go @@ -20,6 +20,7 @@ func (a *agent) InsertDocumentComment(ctx context.Context, c comment.Comment) er "fk_branch_id": c.BranchID, "resolved": c.Resolved, "fk_resolved_by": c.ResolvedBy, + "resolved_at": c.ResolvedAt, "anchor_block_id": c.AnchorBlockID, "content": c.Content, "diff_deletion_context": c.DiffDeletionContext, @@ -112,6 +113,7 @@ func (a *agent) UpdateDocumentComment(ctx context.Context, c comment.Comment) er "content": c.Content, "resolved": c.Resolved, "fk_resolved_by": c.ResolvedBy, + "resolved_at": c.ResolvedAt, "anchor_block_id": c.AnchorBlockID, "updated_at": c.UpdatedAt, }). @@ -193,12 +195,14 @@ func (a *agent) ReplaceDocumentComment(ctx context.Context, c comment.Comment) e return err } -// FetchDocumentCommentsByBranchID retrieves all comments for a specific branch along with their replies. -func (a *agent) FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string) ([]comment.Comment, error) { +// FetchDocumentCommentsByBranchID retrieves the comments of a specific branch +// in the given resolved state along with their replies. +func (a *agent) FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]comment.Comment, error) { q, args := a.selectDocumentComment(a.builder.Select()). Where(sq.Eq{ "document_comments.fk_branch_id": branchID, "document_comments.fk_organization_id": organizationID, + "document_comments.resolved": resolved, }). OrderBy("document_comments.created_at DESC"). MustSql() @@ -278,6 +282,7 @@ func (a *agent) selectDocumentComment(b sq.SelectBuilder) sq.SelectBuilder { `document_comments.fk_organization_id AS "fk_organization_id"`, `document_comments.fk_branch_id AS "fk_branch_id"`, `document_comments.fk_resolved_by AS "fk_resolved_by"`, + `document_comments.resolved_at AS "resolved_at"`, `document_comments.resolved AS "resolved"`, `document_comments.anchor_block_id AS "anchor_block_id"`, `document_comments.content AS "content"`, diff --git a/server/core/internal/db/document_comment_test.go b/server/core/internal/db/document_comment_test.go index ded271ce..a9b0a753 100644 --- a/server/core/internal/db/document_comment_test.go +++ b/server/core/internal/db/document_comment_test.go @@ -65,6 +65,7 @@ func prepDocumentComments(t *testing.T, db *DB, count int, fn func(int, *comment "fk_branch_id": c.BranchID, "resolved": c.Resolved, "fk_resolved_by": c.ResolvedBy, + "resolved_at": c.ResolvedAt, "anchor_block_id": c.AnchorBlockID, "content": c.Content, "diff_deletion_context": c.DiffDeletionContext, @@ -350,6 +351,7 @@ func Test_agent_UpdateDocumentComment(t *testing.T) { } c.Resolved = true c.ResolvedBy = null.StringFrom(users[0]) + c.ResolvedAt = null.TimeFrom(timeutil.Now().Truncate(time.Second)) c.AnchorBlockID = null.StringFrom("block-2") c.UpdatedAt = null.TimeFrom(timeutil.Now().Truncate(time.Second)) c.Replies = []comment.Reply{} @@ -628,7 +630,7 @@ func Test_agent_FetchDocumentCommentsByBranchID(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - res, err := db.FetchDocumentCommentsByBranchID(ctx, xid.New(), "org-id") + res, err := db.FetchDocumentCommentsByBranchID(ctx, xid.New(), "org-id", false) require.Error(t, err) assert.Nil(t, res) }) @@ -645,7 +647,7 @@ func Test_agent_FetchDocumentCommentsByBranchID(t *testing.T) { mock.ExpectQuery("SELECT .* FROM document_comment_replies"). WillReturnError(assert.AnError) - res, err := a.FetchDocumentCommentsByBranchID(context.Background(), xid.New(), "org-id") + res, err := a.FetchDocumentCommentsByBranchID(context.Background(), xid.New(), "org-id", false) assert.Equal(t, assert.AnError, err) assert.Nil(t, res) }) @@ -656,15 +658,24 @@ func Test_agent_FetchDocumentCommentsByBranchID(t *testing.T) { db := prepTempDB(t) // success - no comments - res, err := db.FetchDocumentCommentsByBranchID(context.Background(), xid.New(), "non-existent-org-id") + res, err := db.FetchDocumentCommentsByBranchID(context.Background(), xid.New(), "non-existent-org-id", false) require.NoError(t, err) assert.Empty(t, res) // success - newest comment first, replies in creation order and // attached to the comment they belong to rather than the first // one; a comment with no replies carries an empty slice, not - // nil, matching FetchDocumentComment + // nil, matching FetchDocumentComment; a resolved comment of the + // same branch is left out comments := prepDocumentComments(t, db, 3, nil) + resolved := prepDocumentComments(t, db, 1, func(_ int, c *comment.Comment) { + c.BranchID = comments[0].BranchID + c.DocumentID = comments[0].DocumentID + c.OrganizationID = comments[0].OrganizationID + c.Resolved = true + c.ResolvedAt = null.TimeFrom(timeutil.Now().Truncate(time.Second)) + c.Replies = make([]comment.Reply, 0) + }) comments[0].Replies = prepDocumentCommentReplies(t, db, 2, func(_ int, r *comment.Reply) { r.CommentID = comments[0].ID r.OrganizationID = comments[0].OrganizationID @@ -675,9 +686,14 @@ func Test_agent_FetchDocumentCommentsByBranchID(t *testing.T) { }) comments[2].Replies = make([]comment.Reply, 0) - res, err = db.FetchDocumentCommentsByBranchID(context.Background(), comments[0].BranchID, comments[0].OrganizationID) + res, err = db.FetchDocumentCommentsByBranchID(context.Background(), comments[0].BranchID, comments[0].OrganizationID, false) assert.NoError(t, err) testutil.AssertFilterEqual(t, comments, res) + + // success - only the resolved comment + res, err = db.FetchDocumentCommentsByBranchID(context.Background(), comments[0].BranchID, comments[0].OrganizationID, true) + assert.NoError(t, err) + testutil.AssertFilterEqual(t, resolved, res) }) } @@ -726,7 +742,7 @@ func Test_agent_DeleteDocumentCommentsByBranchID(t *testing.T) { return } - res, err := db.FetchDocumentCommentsByBranchID(context.Background(), c.Comments[0].BranchID, c.Comments[0].OrganizationID) + res, err := db.FetchDocumentCommentsByBranchID(context.Background(), c.Comments[0].BranchID, c.Comments[0].OrganizationID, false) require.NoError(t, err) assert.Empty(t, res) }) diff --git a/server/core/internal/db/migrations/001_initial.sql b/server/core/internal/db/migrations/001_initial.sql index 75c316d5..fa7f8932 100644 --- a/server/core/internal/db/migrations/001_initial.sql +++ b/server/core/internal/db/migrations/001_initial.sql @@ -328,6 +328,7 @@ CREATE TABLE document_comments ( fk_user_id TEXT REFERENCES users ON DELETE SET NULL, fk_organization_id TEXT NOT NULL REFERENCES organizations ON DELETE CASCADE, fk_resolved_by TEXT REFERENCES users ON DELETE SET NULL, + resolved_at TIMESTAMP, anchor_block_id TEXT, resolved BOOLEAN NOT NULL DEFAULT FALSE, content JSONB NOT NULL, diff --git a/server/core/internal/document/comment/comment.go b/server/core/internal/document/comment/comment.go index 7a4fb511..7cadabc6 100644 --- a/server/core/internal/document/comment/comment.go +++ b/server/core/internal/document/comment/comment.go @@ -14,8 +14,13 @@ import ( "github.com/rs/xid" ) -// ErrMissingContent is returned when a comment or reply carries no content. -var ErrMissingContent = errutil.New(http.StatusBadRequest, "comment.missing_content", "comment content is required") +var ( + // ErrMissingContent is returned when a comment or reply carries no content. + ErrMissingContent = errutil.New(http.StatusBadRequest, "comment.missing_content", "comment content is required") + + // ErrMissingStatus is returned when a status update carries no resolved flag. + ErrMissingStatus = errutil.New(http.StatusBadRequest, "comment.missing_status", "comment resolved status is required") +) // Comment represents a comment on a document. type Comment struct { @@ -43,6 +48,9 @@ type Comment struct { // ResolvedBy is the identifier for the user who resolved this comment. ResolvedBy null.String `json:"resolvedBy" db:"fk_resolved_by"` + // ResolvedAt is the timestamp when the comment was resolved. + ResolvedAt null.Time `json:"resolvedAt" db:"resolved_at"` + // Content is the content of the comment. Content Content `json:"content" db:"content"` @@ -159,6 +167,7 @@ func (c Comment) Resolve(resolvedBy string) Comment { nc := c nc.Resolved = true nc.ResolvedBy = null.StringFrom(resolvedBy) + nc.ResolvedAt = null.TimeFrom(timeutil.Now()) return nc } @@ -168,6 +177,7 @@ func (c Comment) Unresolve() Comment { nc := c nc.Resolved = false nc.ResolvedBy = null.String{} + nc.ResolvedAt = null.Time{} return nc } @@ -242,6 +252,21 @@ func (inp Input) Validate() error { return nil } +// StatusInput is the input structure for a comment status change. +type StatusInput struct { + // Resolved is the resolved state to store. + Resolved null.Bool `json:"resolved"` +} + +// Validate checks whether the input carries the resolved flag. +func (inp StatusInput) Validate() error { + if !inp.Resolved.Valid { + return ErrMissingStatus + } + + return nil +} + // ReplyInput is the input structure for a reply. type ReplyInput struct { // Content is the content of the reply. diff --git a/server/core/internal/document/comment/comment_test.go b/server/core/internal/document/comment/comment_test.go index 6d9c0052..07810c1a 100644 --- a/server/core/internal/document/comment/comment_test.go +++ b/server/core/internal/document/comment/comment_test.go @@ -88,6 +88,7 @@ func Test_Comment_Resolve(t *testing.T) { resolved := c.Resolve("resolver") assert.True(t, resolved.Resolved) assert.Equal(t, null.StringFrom("resolver"), resolved.ResolvedBy) + assert.True(t, resolved.ResolvedAt.Valid) assert.Equal(t, c.BranchID, resolved.BranchID) assert.Equal(t, c.Content, resolved.Content) assert.Equal(t, c.DiffDeletionContext, resolved.DiffDeletionContext) @@ -101,6 +102,7 @@ func Test_Comment_Unresolve(t *testing.T) { unresolved := c.Resolve("resolver").Unresolve() assert.False(t, unresolved.Resolved) assert.False(t, unresolved.ResolvedBy.Valid) + assert.False(t, unresolved.ResolvedAt.Valid) assert.Equal(t, c.BranchID, unresolved.BranchID) assert.Equal(t, c.Content, unresolved.Content) } @@ -166,6 +168,34 @@ func Test_Reply_ApplyUpdate(t *testing.T) { assert.True(t, nr.UpdatedAt.Valid) } +func Test_StatusInput_Validate(t *testing.T) { + t.Parallel() + + cc := map[string]struct { + Input StatusInput + Err error + }{ + "Missing resolved flag": { + Input: StatusInput{}, + Err: ErrMissingStatus, + }, + "Resolved": { + Input: StatusInput{Resolved: null.BoolFrom(true)}, + }, + "Unresolved": { + Input: StatusInput{Resolved: null.BoolFrom(false)}, + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, c.Err, c.Input.Validate()) + }) + } +} + func Test_Content_Scan(t *testing.T) { t.Parallel() diff --git a/server/core/internal/server/0mock_db_test.go b/server/core/internal/server/0mock_db_test.go index 32f84392..77a907bd 100644 --- a/server/core/internal/server/0mock_db_test.go +++ b/server/core/internal/server/0mock_db_test.go @@ -130,7 +130,7 @@ var _ DB = &DBMock{} // FetchDocumentCommentReplyFunc: func(ctx context.Context, id xid.ID, commentID xid.ID, organizationID string) (*comment.Reply, error) { // panic("mock out the FetchDocumentCommentReply method") // }, -// FetchDocumentCommentsByBranchIDFunc: func(ctx context.Context, branchID xid.ID, organizationID string) ([]comment.Comment, error) { +// FetchDocumentCommentsByBranchIDFunc: func(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]comment.Comment, error) { // panic("mock out the FetchDocumentCommentsByBranchID method") // }, // FetchDocumentFileFunc: func(ctx context.Context, id string, organizationID string) (*file.File, error) { @@ -417,7 +417,7 @@ type DBMock struct { FetchDocumentCommentReplyFunc func(ctx context.Context, id xid.ID, commentID xid.ID, organizationID string) (*comment.Reply, error) // FetchDocumentCommentsByBranchIDFunc mocks the FetchDocumentCommentsByBranchID method. - FetchDocumentCommentsByBranchIDFunc func(ctx context.Context, branchID xid.ID, organizationID string) ([]comment.Comment, error) + FetchDocumentCommentsByBranchIDFunc func(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]comment.Comment, error) // FetchDocumentFileFunc mocks the FetchDocumentFile method. FetchDocumentFileFunc func(ctx context.Context, id string, organizationID string) (*file.File, error) @@ -909,6 +909,8 @@ type DBMock struct { BranchID xid.ID // OrganizationID is the organizationID argument value. OrganizationID string + // Resolved is the resolved argument value. + Resolved bool } // FetchDocumentFile holds details about calls to the FetchDocumentFile method. FetchDocumentFile []struct { @@ -2932,15 +2934,17 @@ func (mock *DBMock) FetchDocumentCommentReplyCalls() []struct { } // FetchDocumentCommentsByBranchID calls FetchDocumentCommentsByBranchIDFunc. -func (mock *DBMock) FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string) ([]comment.Comment, error) { +func (mock *DBMock) FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]comment.Comment, error) { callInfo := struct { Ctx context.Context BranchID xid.ID OrganizationID string + Resolved bool }{ Ctx: ctx, BranchID: branchID, OrganizationID: organizationID, + Resolved: resolved, } mock.lockFetchDocumentCommentsByBranchID.Lock() mock.calls.FetchDocumentCommentsByBranchID = append(mock.calls.FetchDocumentCommentsByBranchID, callInfo) @@ -2952,7 +2956,7 @@ func (mock *DBMock) FetchDocumentCommentsByBranchID(ctx context.Context, branchI ) return commentsOut, errOut } - return mock.FetchDocumentCommentsByBranchIDFunc(ctx, branchID, organizationID) + return mock.FetchDocumentCommentsByBranchIDFunc(ctx, branchID, organizationID, resolved) } // FetchDocumentCommentsByBranchIDCalls gets all the calls that were made to FetchDocumentCommentsByBranchID. @@ -2963,11 +2967,13 @@ func (mock *DBMock) FetchDocumentCommentsByBranchIDCalls() []struct { Ctx context.Context BranchID xid.ID OrganizationID string + Resolved bool } { var calls []struct { Ctx context.Context BranchID xid.ID OrganizationID string + Resolved bool } mock.lockFetchDocumentCommentsByBranchID.RLock() calls = mock.calls.FetchDocumentCommentsByBranchID diff --git a/server/core/internal/server/internal/document/comment/0mock_db_test.go b/server/core/internal/server/internal/document/comment/0mock_db_test.go index 0f1a7f5f..080d19c3 100644 --- a/server/core/internal/server/internal/document/comment/0mock_db_test.go +++ b/server/core/internal/server/internal/document/comment/0mock_db_test.go @@ -43,7 +43,7 @@ var _ DB = &DBMock{} // FetchDocumentCommentReplyFunc: func(ctx context.Context, id xid.ID, commentID xid.ID, organizationID string) (*commentCore.Reply, error) { // panic("mock out the FetchDocumentCommentReply method") // }, -// FetchDocumentCommentsByBranchIDFunc: func(ctx context.Context, branchID xid.ID, organizationID string) ([]commentCore.Comment, error) { +// FetchDocumentCommentsByBranchIDFunc: func(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]commentCore.Comment, error) { // panic("mock out the FetchDocumentCommentsByBranchID method") // }, // FetchDocumentMaintainersFunc: func(ctx context.Context, documentID xid.ID, organizationID string) ([]string, error) { @@ -93,7 +93,7 @@ type DBMock struct { FetchDocumentCommentReplyFunc func(ctx context.Context, id xid.ID, commentID xid.ID, organizationID string) (*commentCore.Reply, error) // FetchDocumentCommentsByBranchIDFunc mocks the FetchDocumentCommentsByBranchID method. - FetchDocumentCommentsByBranchIDFunc func(ctx context.Context, branchID xid.ID, organizationID string) ([]commentCore.Comment, error) + FetchDocumentCommentsByBranchIDFunc func(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]commentCore.Comment, error) // FetchDocumentMaintainersFunc mocks the FetchDocumentMaintainers method. FetchDocumentMaintainersFunc func(ctx context.Context, documentID xid.ID, organizationID string) ([]string, error) @@ -192,6 +192,8 @@ type DBMock struct { BranchID xid.ID // OrganizationID is the organizationID argument value. OrganizationID string + // Resolved is the resolved argument value. + Resolved bool } // FetchDocumentMaintainers holds details about calls to the FetchDocumentMaintainers method. FetchDocumentMaintainers []struct { @@ -571,15 +573,17 @@ func (mock *DBMock) FetchDocumentCommentReplyCalls() []struct { } // FetchDocumentCommentsByBranchID calls FetchDocumentCommentsByBranchIDFunc. -func (mock *DBMock) FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string) ([]commentCore.Comment, error) { +func (mock *DBMock) FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]commentCore.Comment, error) { callInfo := struct { Ctx context.Context BranchID xid.ID OrganizationID string + Resolved bool }{ Ctx: ctx, BranchID: branchID, OrganizationID: organizationID, + Resolved: resolved, } mock.lockFetchDocumentCommentsByBranchID.Lock() mock.calls.FetchDocumentCommentsByBranchID = append(mock.calls.FetchDocumentCommentsByBranchID, callInfo) @@ -591,7 +595,7 @@ func (mock *DBMock) FetchDocumentCommentsByBranchID(ctx context.Context, branchI ) return commentsOut, errOut } - return mock.FetchDocumentCommentsByBranchIDFunc(ctx, branchID, organizationID) + return mock.FetchDocumentCommentsByBranchIDFunc(ctx, branchID, organizationID, resolved) } // FetchDocumentCommentsByBranchIDCalls gets all the calls that were made to FetchDocumentCommentsByBranchID. @@ -602,11 +606,13 @@ func (mock *DBMock) FetchDocumentCommentsByBranchIDCalls() []struct { Ctx context.Context BranchID xid.ID OrganizationID string + Resolved bool } { var calls []struct { Ctx context.Context BranchID xid.ID OrganizationID string + Resolved bool } mock.lockFetchDocumentCommentsByBranchID.RLock() calls = mock.calls.FetchDocumentCommentsByBranchID diff --git a/server/core/internal/server/internal/document/comment/0mock_tx_test.go b/server/core/internal/server/internal/document/comment/0mock_tx_test.go index 5e047120..08c8809c 100644 --- a/server/core/internal/server/internal/document/comment/0mock_tx_test.go +++ b/server/core/internal/server/internal/document/comment/0mock_tx_test.go @@ -43,7 +43,7 @@ var _ Tx = &TxMock{} // FetchDocumentCommentReplyFunc: func(ctx context.Context, id xid.ID, commentID xid.ID, organizationID string) (*commentCore.Reply, error) { // panic("mock out the FetchDocumentCommentReply method") // }, -// FetchDocumentCommentsByBranchIDFunc: func(ctx context.Context, branchID xid.ID, organizationID string) ([]commentCore.Comment, error) { +// FetchDocumentCommentsByBranchIDFunc: func(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]commentCore.Comment, error) { // panic("mock out the FetchDocumentCommentsByBranchID method") // }, // FetchDocumentMaintainersFunc: func(ctx context.Context, documentID xid.ID, organizationID string) ([]string, error) { @@ -96,7 +96,7 @@ type TxMock struct { FetchDocumentCommentReplyFunc func(ctx context.Context, id xid.ID, commentID xid.ID, organizationID string) (*commentCore.Reply, error) // FetchDocumentCommentsByBranchIDFunc mocks the FetchDocumentCommentsByBranchID method. - FetchDocumentCommentsByBranchIDFunc func(ctx context.Context, branchID xid.ID, organizationID string) ([]commentCore.Comment, error) + FetchDocumentCommentsByBranchIDFunc func(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]commentCore.Comment, error) // FetchDocumentMaintainersFunc mocks the FetchDocumentMaintainers method. FetchDocumentMaintainersFunc func(ctx context.Context, documentID xid.ID, organizationID string) ([]string, error) @@ -194,6 +194,8 @@ type TxMock struct { BranchID xid.ID // OrganizationID is the organizationID argument value. OrganizationID string + // Resolved is the resolved argument value. + Resolved bool } // FetchDocumentMaintainers holds details about calls to the FetchDocumentMaintainers method. FetchDocumentMaintainers []struct { @@ -568,15 +570,17 @@ func (mock *TxMock) FetchDocumentCommentReplyCalls() []struct { } // FetchDocumentCommentsByBranchID calls FetchDocumentCommentsByBranchIDFunc. -func (mock *TxMock) FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string) ([]commentCore.Comment, error) { +func (mock *TxMock) FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]commentCore.Comment, error) { callInfo := struct { Ctx context.Context BranchID xid.ID OrganizationID string + Resolved bool }{ Ctx: ctx, BranchID: branchID, OrganizationID: organizationID, + Resolved: resolved, } mock.lockFetchDocumentCommentsByBranchID.Lock() mock.calls.FetchDocumentCommentsByBranchID = append(mock.calls.FetchDocumentCommentsByBranchID, callInfo) @@ -588,7 +592,7 @@ func (mock *TxMock) FetchDocumentCommentsByBranchID(ctx context.Context, branchI ) return commentsOut, errOut } - return mock.FetchDocumentCommentsByBranchIDFunc(ctx, branchID, organizationID) + return mock.FetchDocumentCommentsByBranchIDFunc(ctx, branchID, organizationID, resolved) } // FetchDocumentCommentsByBranchIDCalls gets all the calls that were made to FetchDocumentCommentsByBranchID. @@ -599,11 +603,13 @@ func (mock *TxMock) FetchDocumentCommentsByBranchIDCalls() []struct { Ctx context.Context BranchID xid.ID OrganizationID string + Resolved bool } { var calls []struct { Ctx context.Context BranchID xid.ID OrganizationID string + Resolved bool } mock.lockFetchDocumentCommentsByBranchID.RLock() calls = mock.calls.FetchDocumentCommentsByBranchID diff --git a/server/core/internal/server/internal/document/comment/http.go b/server/core/internal/server/internal/document/comment/http.go index f851383a..5d2a76b7 100644 --- a/server/core/internal/server/internal/document/comment/http.go +++ b/server/core/internal/server/internal/document/comment/http.go @@ -18,9 +18,15 @@ import ( "github.com/rs/xid" ) -// ErrBranchMismatch is returned when the requested branch does not belong to -// the document identified by the request path. -var ErrBranchMismatch = errutil.New(http.StatusNotFound, "document.branch_mismatch", "branch does not belong to the document") +var ( + // ErrBranchMismatch is returned when the requested branch does not belong to + // the document identified by the request path. + ErrBranchMismatch = errutil.New(http.StatusNotFound, "document.branch_mismatch", "branch does not belong to the document") + + // ErrInvalidResolvedFilter is returned when the resolved query parameter + // is neither "true" nor "false". + ErrInvalidResolvedFilter = errutil.New(http.StatusBadRequest, "comment.invalid_resolved_filter", "resolved must be true or false") +) // Handler holds dependencies required for document comment operations. type Handler struct { @@ -283,8 +289,10 @@ func (h *Handler) FetchDocumentComment(w http.ResponseWriter, r *http.Request) { ) } -// FetchDocumentComments handles the retrieval of all comments for a document branch. -// Requires a "branchId" query parameter to identify which branch's comments to return. +// FetchDocumentComments handles the retrieval of the comments of a document +// branch. Requires a "branchId" query parameter to identify which branch's +// comments to return; the optional "resolved" parameter selects resolved +// comments instead of the open ones returned by default. func (h *Handler) FetchDocumentComments(w http.ResponseWriter, r *http.Request) { session, ok := auth.RequireSession(h.log, w, r) if !ok { @@ -303,6 +311,17 @@ func (h *Handler) FetchDocumentComments(w http.ResponseWriter, r *http.Request) return } + var resolved bool + + switch r.URL.Query().Get("resolved") { + case "", "false": + case "true": + resolved = true + default: + httpserver.RespondError(h.log, w, ErrInvalidResolvedFilter) + return + } + branchDoc, err := h.db.FetchDocumentByBranchID(r.Context(), branchID, session.ActiveOrganizationID) if err != nil { httpserver.RespondError(h.log, w, err) @@ -314,7 +333,7 @@ func (h *Handler) FetchDocumentComments(w http.ResponseWriter, r *http.Request) return } - comments, err := h.db.FetchDocumentCommentsByBranchID(r.Context(), branchID, session.ActiveOrganizationID) + comments, err := h.db.FetchDocumentCommentsByBranchID(r.Context(), branchID, session.ActiveOrganizationID, resolved) if err != nil { httpserver.RespondError(h.log, w, err) return @@ -476,8 +495,9 @@ func (h *Handler) UpdateDocumentCommentReply(w http.ResponseWriter, r *http.Requ ) } -// ResolveDocumentComment handles marking a comment as resolved. -func (h *Handler) ResolveDocumentComment(w http.ResponseWriter, r *http.Request) { +// UpdateDocumentCommentStatus handles marking a comment as resolved or +// unresolved. +func (h *Handler) UpdateDocumentCommentStatus(w http.ResponseWriter, r *http.Request) { session, ok := auth.RequireSession(h.log, w, r) if !ok { return @@ -501,25 +521,45 @@ func (h *Handler) ResolveDocumentComment(w http.ResponseWriter, r *http.Request) return } - // resolving currently destroys the comment and its replies, so it needs - // the same ownership guard the delete route applies. - if !c.UserID.Valid || c.UserID.String != session.UserID { - httpserver.RespondError(h.log, w, httpserver.ErrNotPermitted) - return - } + var si commentCore.StatusInput - if err := h.db.DeleteDocumentComment(r.Context(), commentID, documentID, session.ActiveOrganizationID); err != nil { + if err := httpserver.DecodeJSON(r, &si); err != nil { httpserver.RespondError(h.log, w, err) return } - // TODO: Once comment history is implemented, soft delete (resolve) the - // comment via c.Resolve and publish an update message instead of - // removing it. + if err := si.Validate(); err != nil { + httpserver.RespondError(h.log, w, err) + return + } + + if c.Resolved == si.Resolved.Bool { + httpserver.Respond( + h.log, + w, + c, + http.StatusOK, + ) + + return + } + + var nc commentCore.Comment + + if si.Resolved.Bool { + nc = c.Resolve(session.UserID) + } else { + nc = c.Unresolve() + } + + if err := h.db.UpdateDocumentComment(r.Context(), nc); err != nil { + httpserver.RespondError(h.log, w, err) + return + } if h.comments.changeCallback != nil { h.comments.changeCallback(session.ActiveOrganizationID, documentID, ChangeMessage{ - Type: ChangeTypeDeleted, + Type: ChangeTypeUpdated, CommentID: commentID, }) } @@ -527,60 +567,11 @@ func (h *Handler) ResolveDocumentComment(w http.ResponseWriter, r *http.Request) httpserver.Respond( h.log, w, - c, + nc, http.StatusOK, ) } -// TODO: Uncomment once comment history is implemented. -// UnresolveDocumentComment handles marking a comment as unresolved. -// func (h *Handler) UnresolveDocumentComment(w http.ResponseWriter, r *http.Request) { -// session, err := auth.ExtractSessionFromContext(r.Context()) -// if err != nil { -// httpserver.RespondError(h.log, w, err) -// return -// } -// -// documentID, err := httpserver.ExtractNamedID(r, "documentId") -// if err != nil { -// httpserver.RespondError(h.log, w, err) -// return -// } -// -// commentID, err := httpserver.ExtractNamedID(r, "commentId") -// if err != nil { -// httpserver.RespondError(h.log, w, err) -// return -// } -// -// c, err := h.db.FetchDocumentComment(r.Context(), commentID, documentID, session.ActiveOrganizationID) -// if err != nil { -// httpserver.RespondError(h.log, w, err) -// return -// } -// -// nc := c.Unresolve() -// -// if err := h.db.UpdateDocumentComment(r.Context(), nc); err != nil { -// httpserver.RespondError(h.log, w, err) -// return -// } -// -// if h.comments.changeCallback != nil { -// h.comments.changeCallback(session.ActiveOrganizationID, documentID, ChangeMessage{ -// Type: ChangeTypeUpdated, -// CommentID: commentID, -// }) -// } -// -// httpserver.Respond( -// h.log, -// w, -// nc, -// http.StatusOK, -// ) -//} - // DeleteDocumentComment handles the deletion of a comment. // If the comment has replies, the first reply is promoted to become the main comment. // If there are no replies, the comment is deleted. @@ -769,8 +760,9 @@ type CommentsDBAgent interface { // FetchDocumentComment should fetch a comment by its ID along with all its replies. FetchDocumentComment(ctx context.Context, id, documentID xid.ID, organizationID string) (*commentCore.Comment, error) - // FetchDocumentCommentsByBranchID should fetch all comments for a branch with their replies. - FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string) ([]commentCore.Comment, error) + // FetchDocumentCommentsByBranchID should fetch the comments of a branch in + // the given resolved state with their replies. + FetchDocumentCommentsByBranchID(ctx context.Context, branchID xid.ID, organizationID string, resolved bool) ([]commentCore.Comment, error) // UpdateDocumentComment should update an existing comment. UpdateDocumentComment(ctx context.Context, c commentCore.Comment) error diff --git a/server/core/internal/server/internal/document/comment/http_test.go b/server/core/internal/server/internal/document/comment/http_test.go index 461aaa3d..7c80ff7a 100644 --- a/server/core/internal/server/internal/document/comment/http_test.go +++ b/server/core/internal/server/internal/document/comment/http_test.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/guregu/null/v5" documentCore "github.com/oxynote/oxynote/server/core/internal/document" @@ -475,6 +476,7 @@ func Test_Handler_FetchDocumentComments(t *testing.T) { OmitDoc bool Query string RespCode int + Resolved bool }{ "No session in context": { DB: &DBMock{}, @@ -493,6 +495,11 @@ func Test_Handler_FetchDocumentComments(t *testing.T) { Query: "?branchId=bogus", RespCode: http.StatusBadRequest, }, + "Invalid resolved query parameter": { + DB: &DBMock{}, + Query: "?branchId=" + _branchID.String() + "&resolved=bogus", + RespCode: http.StatusBadRequest, + }, "Branch document fetch error": { DB: &DBMock{ FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) { @@ -513,7 +520,7 @@ func Test_Handler_FetchDocumentComments(t *testing.T) { }, "Comments fetch error": { DB: &DBMock{ - FetchDocumentCommentsByBranchIDFunc: func(context.Context, xid.ID, string) ([]commentCore.Comment, error) { + FetchDocumentCommentsByBranchIDFunc: func(context.Context, xid.ID, string, bool) ([]commentCore.Comment, error) { return nil, errors.New("boom") }, }, @@ -522,13 +529,32 @@ func Test_Handler_FetchDocumentComments(t *testing.T) { }, "Successful fetch": { DB: &DBMock{ - FetchDocumentCommentsByBranchIDFunc: func(context.Context, xid.ID, string) ([]commentCore.Comment, error) { + FetchDocumentCommentsByBranchIDFunc: func(context.Context, xid.ID, string, bool) ([]commentCore.Comment, error) { return []commentCore.Comment{*storedComment("u2")}, nil }, }, Query: "?branchId=" + _branchID.String(), RespCode: http.StatusOK, }, + "Successful fetch of open comments": { + DB: &DBMock{ + FetchDocumentCommentsByBranchIDFunc: func(context.Context, xid.ID, string, bool) ([]commentCore.Comment, error) { + return []commentCore.Comment{*storedComment("u2")}, nil + }, + }, + Query: "?branchId=" + _branchID.String() + "&resolved=false", + RespCode: http.StatusOK, + }, + "Successful fetch of resolved comments": { + DB: &DBMock{ + FetchDocumentCommentsByBranchIDFunc: func(context.Context, xid.ID, string, bool) ([]commentCore.Comment, error) { + return []commentCore.Comment{*storedComment("u2")}, nil + }, + }, + Query: "?branchId=" + _branchID.String() + "&resolved=true", + RespCode: http.StatusOK, + Resolved: true, + }, } for cn, c := range cc { @@ -570,6 +596,7 @@ func Test_Handler_FetchDocumentComments(t *testing.T) { ff := c.DB.FetchDocumentCommentsByBranchIDCalls() require.Len(t, ff, 1) assert.Equal(t, _branchID, ff[0].BranchID) + assert.Equal(t, c.Resolved, ff[0].Resolved) } }) } @@ -797,23 +824,37 @@ func Test_Handler_UpdateDocumentCommentReply(t *testing.T) { } } -func Test_Handler_ResolveDocumentComment(t *testing.T) { +func Test_Handler_UpdateDocumentCommentStatus(t *testing.T) { + // resolvedComment builds a stored comment already resolved by u2. + resolvedComment := func() *commentCore.Comment { + c := storedComment("u2") + c.Resolved = true + c.ResolvedBy = null.StringFrom("u2") + c.ResolvedAt = null.TimeFrom(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)) + + return c + } + cc := map[string]struct { DB *DBMock NoSession bool OmitDoc bool + Body string RespCode int - Deleted int + Updated int Changes int + Check func(t *testing.T, c commentCore.Comment) }{ "No session in context": { DB: &DBMock{}, NoSession: true, + Body: `{"resolved":true}`, RespCode: http.StatusUnauthorized, }, "Missing document ID parameter": { DB: &DBMock{}, OmitDoc: true, + Body: `{"resolved":true}`, RespCode: http.StatusNotFound, }, "Comment fetch error": { @@ -822,37 +863,89 @@ func Test_Handler_ResolveDocumentComment(t *testing.T) { return nil, errors.New("boom") }, }, + Body: `{"resolved":true}`, RespCode: http.StatusInternalServerError, }, - "Another user's comment": { + "Invalid JSON body": { DB: &DBMock{ FetchDocumentCommentFunc: func(context.Context, xid.ID, xid.ID, string) (*commentCore.Comment, error) { return storedComment("u2"), nil }, }, - RespCode: http.StatusForbidden, + Body: "{", + RespCode: http.StatusBadRequest, }, - "Comment deletion error": { + "Missing resolved field": { DB: &DBMock{ FetchDocumentCommentFunc: func(context.Context, xid.ID, xid.ID, string) (*commentCore.Comment, error) { - return storedComment("u1"), nil + return storedComment("u2"), nil }, - DeleteDocumentCommentFunc: func(context.Context, xid.ID, xid.ID, string) error { + }, + Body: `{}`, + RespCode: http.StatusBadRequest, + }, + "Comment update error": { + DB: &DBMock{ + FetchDocumentCommentFunc: func(context.Context, xid.ID, xid.ID, string) (*commentCore.Comment, error) { + return storedComment("u2"), nil + }, + UpdateDocumentCommentFunc: func(context.Context, commentCore.Comment) error { return errors.New("boom") }, }, + Body: `{"resolved":true}`, RespCode: http.StatusInternalServerError, - Deleted: 1, + Updated: 1, }, - "Successful resolution": { + "Successful resolution of another user's comment": { DB: &DBMock{ FetchDocumentCommentFunc: func(context.Context, xid.ID, xid.ID, string) (*commentCore.Comment, error) { - return storedComment("u1"), nil + return storedComment("u2"), nil }, }, + Body: `{"resolved":true}`, RespCode: http.StatusOK, - Deleted: 1, + Updated: 1, Changes: 1, + Check: func(t *testing.T, c commentCore.Comment) { + assert.True(t, c.Resolved) + assert.Equal(t, null.StringFrom("u1"), c.ResolvedBy) + assert.True(t, c.ResolvedAt.Valid) + }, + }, + "Successful unresolution": { + DB: &DBMock{ + FetchDocumentCommentFunc: func(context.Context, xid.ID, xid.ID, string) (*commentCore.Comment, error) { + return resolvedComment(), nil + }, + }, + Body: `{"resolved":false}`, + RespCode: http.StatusOK, + Updated: 1, + Changes: 1, + Check: func(t *testing.T, c commentCore.Comment) { + assert.False(t, c.Resolved) + assert.False(t, c.ResolvedBy.Valid) + assert.False(t, c.ResolvedAt.Valid) + }, + }, + "Already resolved": { + DB: &DBMock{ + FetchDocumentCommentFunc: func(context.Context, xid.ID, xid.ID, string) (*commentCore.Comment, error) { + return resolvedComment(), nil + }, + }, + Body: `{"resolved":true}`, + RespCode: http.StatusOK, + }, + "Already unresolved": { + DB: &DBMock{ + FetchDocumentCommentFunc: func(context.Context, xid.ID, xid.ID, string) (*commentCore.Comment, error) { + return storedComment("u2"), nil + }, + }, + Body: `{"resolved":false}`, + RespCode: http.StatusOK, }, } @@ -861,18 +954,30 @@ func Test_Handler_ResolveDocumentComment(t *testing.T) { t.Parallel() cpt := &capturedChange{} - hdl := newTestHandler(c.DB, &fakePublisher{}, cpt) + pub := &fakePublisher{} + hdl := newTestHandler(c.DB, pub, cpt) rec := httptest.NewRecorder() - hdl.ResolveDocumentComment(rec, newRequest(http.MethodPost, "", c.NoSession, c.OmitDoc, false, true)) + hdl.UpdateDocumentCommentStatus(rec, newRequest(http.MethodPut, c.Body, c.NoSession, c.OmitDoc, false, true)) assert.Equal(t, c.RespCode, rec.Code) - assert.Len(t, c.DB.DeleteDocumentCommentCalls(), c.Deleted) + assert.Empty(t, pub.calls) + + uu := c.DB.UpdateDocumentCommentCalls() + require.Len(t, uu, c.Updated) require.Len(t, cpt.calls, c.Changes) if c.Changes > 0 { - assert.Equal(t, ChangeTypeDeleted, cpt.calls[0].Type) + assert.Equal(t, ChangeTypeUpdated, cpt.calls[0].Type) + } + + if c.Check != nil { + c.Check(t, uu[0].C) + } + + if c.RespCode == http.StatusOK { + assert.Contains(t, rec.Body.String(), _commentID.String()) } }) } diff --git a/server/core/internal/server/router.go b/server/core/internal/server/router.go index 38e74fd3..985c71fe 100644 --- a/server/core/internal/server/router.go +++ b/server/core/internal/server/router.go @@ -346,11 +346,9 @@ func (s *Server) router() chi.Router { sssr.Get("/", s.handlers.comment.FetchDocumentComments) sssr.Post("/", s.handlers.comment.CreateDocumentComment) sssr.Route("/{commentId}", func(ssssr chi.Router) { - // TODO: Uncomment this once comment history is implemented. - // ssssr.Put("/unresolve", s.handlers.document.UnresolveDocumentComment) ssssr.Get("/", s.handlers.comment.FetchDocumentComment) ssssr.Put("/", s.handlers.comment.UpdateDocumentComment) - ssssr.Put("/resolve", s.handlers.comment.ResolveDocumentComment) + ssssr.Put("/status", s.handlers.comment.UpdateDocumentCommentStatus) ssssr.Delete("/", s.handlers.comment.DeleteDocumentComment) ssssr.Route("/replies", func(sssssr chi.Router) { sssssr.Post("/", s.handlers.comment.CreateDocumentCommentReply) diff --git a/web/app/components/editor/comments/CommentRenderer.nuxt.test.ts b/web/app/components/editor/comments/CommentRenderer.nuxt.test.ts index 913fb82d..6198ed02 100644 --- a/web/app/components/editor/comments/CommentRenderer.nuxt.test.ts +++ b/web/app/components/editor/comments/CommentRenderer.nuxt.test.ts @@ -367,7 +367,7 @@ describe("", { concurrent: false }, () => { seedComments([serverComment()]) const calls = mockEndpoint( "PUT", - `/api/documents/${DOCUMENT_ID}/comments/${COMMENT_ID}/resolve`, + `/api/documents/${DOCUMENT_ID}/comments/${COMMENT_ID}/status`, () => null, ) const wrapper = await mountRenderer(editor) @@ -382,6 +382,7 @@ describe("", { concurrent: false }, () => { await vi.waitFor(() => { expect(calls).toHaveLength(1) }, WAIT_FOR_OPTIONS) + expect(calls[0]?.body).toEqual({ resolved: true }) expect(popoverOpen(wrapper)).toBe(false) }) @@ -391,7 +392,7 @@ describe("", { concurrent: false }, () => { seedComments([serverComment()]) mockEndpoint( "PUT", - `/api/documents/${DOCUMENT_ID}/comments/${COMMENT_ID}/resolve`, + `/api/documents/${DOCUMENT_ID}/comments/${COMMENT_ID}/status`, (_c, event) => { setResponseStatus(event, 500) diff --git a/web/app/components/editor/comments/CommentRenderer.vue b/web/app/components/editor/comments/CommentRenderer.vue index 942143c2..4b14c3dc 100644 --- a/web/app/components/editor/comments/CommentRenderer.vue +++ b/web/app/components/editor/comments/CommentRenderer.vue @@ -997,10 +997,11 @@ async function resolveComment() { closePopover() try { - await commentAPI.updateDocumentCommentResolveByCommentId.mutateAsync({ + await commentAPI.updateDocumentCommentStatusByCommentId.mutateAsync({ docId: editorStore.activeDocumentId, branchId: editorStore.activeBranchId, commentId: commentId, + req: { resolved: true }, }) } catch { showToastMessage("error", t("editor.comment-thread.errors.delete-failed")) diff --git a/web/app/composables/api/useDocumentCommentAPI.nuxt.test.ts b/web/app/composables/api/useDocumentCommentAPI.nuxt.test.ts index 04626933..150c4881 100644 --- a/web/app/composables/api/useDocumentCommentAPI.nuxt.test.ts +++ b/web/app/composables/api/useDocumentCommentAPI.nuxt.test.ts @@ -35,7 +35,7 @@ const COMMENTS_KEY = ["documents", DOC_ID, "comments", BRANCH_ID] as const const COMMENTS_URL = `/api/documents/${DOC_ID}/comments` const COMMENT_URL = `${COMMENTS_URL}/${COMMENT_ID}` -const RESOLVE_URL = `${COMMENT_URL}/resolve` +const STATUS_URL = `${COMMENT_URL}/status` const REPLIES_URL = `${COMMENT_URL}/replies` const REPLY_URL = `${REPLIES_URL}/${REPLY_ID}` @@ -182,14 +182,15 @@ describe("useDocumentCommentAPI", { concurrent: false }, () => { }), }, { - name: "updateDocumentCommentResolveByCommentId", + name: "updateDocumentCommentStatusByCommentId", method: "PUT", - url: `/api/documents/${NON_XID_ID}/comments/${COMMENT_ID}/resolve`, + url: `/api/documents/${NON_XID_ID}/comments/${COMMENT_ID}/status`, invoke: (api: DocumentCommentAPI) => - api.updateDocumentCommentResolveByCommentId.mutateAsync({ + api.updateDocumentCommentStatusByCommentId.mutateAsync({ docId: NON_XID_ID, branchId: BRANCH_ID, commentId: COMMENT_ID, + req: { resolved: true }, }), }, { @@ -573,7 +574,7 @@ describe("useDocumentCommentAPI", { concurrent: false }, () => { }) }) - describe("updateDocumentCommentResolveByCommentId", () => { + describe("updateDocumentCommentStatusByCommentId", () => { it("removes the resolved comment optimistically and refetches once the resolve succeeds", async ({ expect, }) => { @@ -584,7 +585,7 @@ describe("useDocumentCommentAPI", { concurrent: false }, () => { const serverComments = [makeComment(COMMENT_ID_2)] const getCalls = mockEndpoint("GET", COMMENTS_URL, () => serverComments) let commentsAtRequest: DocumentCommentsResponse | undefined - const putCalls = mockEndpoint("PUT", RESOLVE_URL, () => { + const putCalls = mockEndpoint("PUT", STATUS_URL, () => { commentsAtRequest = readComments() return {} @@ -592,13 +593,44 @@ describe("useDocumentCommentAPI", { concurrent: false }, () => { const api = makeCommentAPI() runInApp(() => api.useFetchDocumentCommentsByDocId(DOC_ID, BRANCH_ID)) - await api.updateDocumentCommentResolveByCommentId.mutateAsync({ + await api.updateDocumentCommentStatusByCommentId.mutateAsync({ docId: DOC_ID, branchId: BRANCH_ID, commentId: COMMENT_ID, + req: { resolved: true }, }) expect(putCalls).toHaveLength(1) + expect(putCalls[0]?.body).toEqual({ resolved: true }) + expect(commentsAtRequest).toEqual([other]) + expect(getCalls).toHaveLength(1) + expect(readComments()).toEqual(serverComments) + }) + + it("keeps the cached list when unresolving", async ({ expect }) => { + seedAuth() + const other = makeComment(COMMENT_ID_2) + seedComments([other]) + const serverComments = [makeComment(COMMENT_ID), other] + const getCalls = mockEndpoint("GET", COMMENTS_URL, () => serverComments) + let commentsAtRequest: DocumentCommentsResponse | undefined + const putCalls = mockEndpoint("PUT", STATUS_URL, () => { + commentsAtRequest = readComments() + + return {} + }) + const api = makeCommentAPI() + runInApp(() => api.useFetchDocumentCommentsByDocId(DOC_ID, BRANCH_ID)) + + await api.updateDocumentCommentStatusByCommentId.mutateAsync({ + docId: DOC_ID, + branchId: BRANCH_ID, + commentId: COMMENT_ID, + req: { resolved: false }, + }) + + expect(putCalls).toHaveLength(1) + expect(putCalls[0]?.body).toEqual({ resolved: false }) expect(commentsAtRequest).toEqual([other]) expect(getCalls).toHaveLength(1) expect(readComments()).toEqual(serverComments) @@ -611,17 +643,18 @@ describe("useDocumentCommentAPI", { concurrent: false }, () => { const target = makeComment(COMMENT_ID) seedComments([target]) const getCalls = mockEndpoint("GET", COMMENTS_URL, () => []) - const putCalls = mockEndpoint("PUT", RESOLVE_URL, () => { + const putCalls = mockEndpoint("PUT", STATUS_URL, () => { throw createError({ statusCode: 500 }) }) const api = makeCommentAPI() runInApp(() => api.useFetchDocumentCommentsByDocId(DOC_ID, BRANCH_ID)) await expect( - api.updateDocumentCommentResolveByCommentId.mutateAsync({ + api.updateDocumentCommentStatusByCommentId.mutateAsync({ docId: DOC_ID, branchId: BRANCH_ID, commentId: COMMENT_ID, + req: { resolved: true }, }), ).rejects.toThrow() @@ -642,7 +675,7 @@ describe("useDocumentCommentAPI", { concurrent: false }, () => { seedAuth() seedComments([makeComment(COMMENT_ID), makeComment(COMMENT_ID_2)]) const getCalls = mockEndpoint("GET", COMMENTS_URL, () => []) - const putCalls = mockEndpoint("PUT", RESOLVE_URL, () => { + const putCalls = mockEndpoint("PUT", STATUS_URL, () => { putReached() return new Promise((_resolve, reject) => { @@ -652,10 +685,11 @@ describe("useDocumentCommentAPI", { concurrent: false }, () => { const api = makeCommentAPI() runInApp(() => api.useFetchDocumentCommentsByDocId(DOC_ID, BRANCH_ID)) - const pending = api.updateDocumentCommentResolveByCommentId.mutateAsync({ + const pending = api.updateDocumentCommentStatusByCommentId.mutateAsync({ docId: DOC_ID, branchId: BRANCH_ID, commentId: COMMENT_ID, + req: { resolved: true }, }) await putReachedSignal diff --git a/web/app/composables/api/useDocumentCommentAPI.ts b/web/app/composables/api/useDocumentCommentAPI.ts index 94746bcd..ccc32015 100644 --- a/web/app/composables/api/useDocumentCommentAPI.ts +++ b/web/app/composables/api/useDocumentCommentAPI.ts @@ -82,6 +82,7 @@ export default function () { userId: userId, resolved: false, resolvedBy: null, + resolvedAt: null, content: req.content, createdAt: new Date(), updatedAt: null, @@ -227,15 +228,17 @@ export default function () { }, }) - const updateDocumentCommentResolveByCommentId = useMutation({ + const updateDocumentCommentStatusByCommentId = useMutation({ onMutate: ({ docId, branchId, commentId, + req, }: { docId: string branchId: string commentId: string + req: DocumentCommentStatusUpdateRequest }) => { if (!isXid(docId) || !isXid(branchId) || !isXid(commentId)) { // optimisticInserts use nanoid @@ -248,9 +251,13 @@ export default function () { ) const newComments = clone(oldComments) ?? [] - const index = newComments.findIndex((h) => h.id === commentId) - if (index !== -1) { - newComments.splice(index, 1) + // the cached list holds open comments only, so a resolved one + // leaves it and an unresolved one is not in it to begin with + if (req.resolved) { + const index = newComments.findIndex((h) => h.id === commentId) + if (index !== -1) { + newComments.splice(index, 1) + } } queryCache.setQueryData(key, newComments) @@ -262,10 +269,12 @@ export default function () { docId, branchId, commentId, + req, }: { docId: string branchId: string commentId: string + req: DocumentCommentStatusUpdateRequest }) => { if (!isXid(docId) || !isXid(branchId) || !isXid(commentId)) { // optimisticInserts use nanoid @@ -273,9 +282,10 @@ export default function () { } await $coreAPIClient( - `/api/documents/${docId}/comments/${commentId}/resolve`, + `/api/documents/${docId}/comments/${commentId}/status`, { method: "PUT", + body: req, }, ) }, @@ -747,7 +757,7 @@ export default function () { useFetchDocumentCommentsByDocId, createDocumentCommentByDocId, updateDocumentCommentByCommentId, - updateDocumentCommentResolveByCommentId, + updateDocumentCommentStatusByCommentId, deleteDocumentCommentByCommentId, createDocumentCommentReplyByCommentId, updateDocumentCommentReplyByReplyId, diff --git a/web/app/utils/api/comment.ts b/web/app/utils/api/comment.ts index d80de7ca..feb8c676 100644 --- a/web/app/utils/api/comment.ts +++ b/web/app/utils/api/comment.ts @@ -13,6 +13,7 @@ export interface DocumentComment { userId: string resolved: boolean resolvedBy?: string | null + resolvedAt?: Date | string | null content: Record createdAt: Date | string updatedAt?: Date | string | null @@ -120,5 +121,9 @@ export type DocumentCommentReplyCreateResponse = DocumentCommentReply export type DocumentCommentUpdateRequest = DocumentCommentCreateRequest +export interface DocumentCommentStatusUpdateRequest { + resolved: boolean +} + export type DocumentCommentReplyUpdateRequest = DocumentCommentReplyCreateRequest