From 680623f1c6f46857f8fba4abcfe394586d3583f8 Mon Sep 17 00:00:00 2001 From: Bretton Date: Tue, 27 Jan 2026 09:48:02 -0800 Subject: [PATCH] refactor(api): remove redundant content fields from view models Align PostView and CommentView with atProto patterns by accessing content through the Record object instead of redundant top-level fields. This matches the Bluesky approach where the record contains the authoritative content. Changes: - Remove content field from CommentView (access via Record.Content) - Remove title/text fields from PostView (access via Record) - Update lexicons to remove redundant field definitions - Update comment_service, feed_repo_base, post_repo to stop setting removed fields - Add test for deleted comments with nil Record - Add helper functions in integration tests to extract content from Record Co-Authored-By: Claude Opus 4.5 --- .../api/handlers/actor/get_comments_test.go | 109 +++++++++++++++++- .../social/coves/community/comment/defs.json | 6 +- .../social/coves/community/post/get.json | 6 - internal/core/comments/comment_service.go | 4 - .../core/comments/comment_service_test.go | 5 +- internal/core/comments/view_models.go | 1 - internal/core/posts/post.go | 2 - internal/db/postgres/feed_repo_base.go | 4 - internal/db/postgres/post_repo.go | 6 - tests/integration/author_posts_e2e_test.go | 22 +++- tests/integration/blob_upload_e2e_test.go | 17 ++- tests/integration/feed_test.go | 33 ++++-- 12 files changed, 170 insertions(+), 45 deletions(-) diff --git a/internal/api/handlers/actor/get_comments_test.go b/internal/api/handlers/actor/get_comments_test.go index f1ebc15..e8adea0 100644 --- a/internal/api/handlers/actor/get_comments_test.go +++ b/internal/api/handlers/actor/get_comments_test.go @@ -131,7 +131,11 @@ func TestGetCommentsHandler_Success(t *testing.T) { { URI: "at://did:plc:testuser/social.coves.community.comment/abc123", CID: "bafytest123", - Content: "Test comment content", + Record: &comments.CommentRecord{ + Type: "social.coves.community.comment", + Content: "Test comment content", + CreatedAt: createdAt, + }, CreatedAt: createdAt, IndexedAt: indexedAt, Author: &posts.AuthorView{ @@ -176,8 +180,18 @@ func TestGetCommentsHandler_Success(t *testing.T) { t.Errorf("Expected correct comment URI, got '%s'", response.Comments[0].URI) } - if response.Comments[0].Content != "Test comment content" { - t.Errorf("Expected correct comment content, got '%s'", response.Comments[0].Content) + // After JSON marshal/unmarshal, Record becomes map[string]interface{} instead + // of the original *CommentRecord type because json.Unmarshal doesn't preserve + // Go struct types for interface{} fields. + if response.Comments[0].Record == nil { + t.Fatal("Expected Record to be non-nil after JSON round-trip") + } + record, ok := response.Comments[0].Record.(map[string]interface{}) + if !ok { + t.Fatalf("Expected Record to be map[string]interface{}, got %T", response.Comments[0].Record) + } + if record["content"] != "Test comment content" { + t.Errorf("Expected correct comment content, got '%s'", record["content"]) } } @@ -623,3 +637,92 @@ func TestGetCommentsHandler_ResolutionFailedError_Returns500(t *testing.T) { t.Errorf("Expected error 'InternalServerError', got '%s'", response.Error) } } + +func TestGetCommentsHandler_DeletedComment_NilRecord(t *testing.T) { + // Test that deleted comments are properly serialized with nil Record at the API layer. + // This verifies the JSON response correctly handles deleted comments where content + // has been removed but the comment shell remains for thread continuity. + createdAt := time.Now().Format(time.RFC3339) + indexedAt := time.Now().Format(time.RFC3339) + deletedAt := time.Now().Format(time.RFC3339) + deletionReason := "User deleted" + + mockComments := &mockCommentService{ + getActorCommentsFunc: func(ctx context.Context, req *comments.GetActorCommentsRequest) (*comments.GetActorCommentsResponse, error) { + return &comments.GetActorCommentsResponse{ + Comments: []*comments.CommentView{ + { + URI: "at://did:plc:testuser/social.coves.community.comment/deleted123", + CID: "bafydeleted", + Record: nil, // Deleted comments have nil Record + IsDeleted: true, + DeletedAt: &deletedAt, + DeletionReason: &deletionReason, + CreatedAt: createdAt, + IndexedAt: indexedAt, + Author: &posts.AuthorView{ + DID: "did:plc:testuser", + Handle: "test.user", + }, + Post: &comments.CommentRef{ + URI: "at://did:plc:community/social.coves.community.post/parent123", + CID: "bafyparent", + }, + Stats: &comments.CommentStats{ + Upvotes: 0, + Downvotes: 0, + Score: 0, + ReplyCount: 0, + }, + }, + }, + }, nil + }, + } + mockUsers := &mockUserServiceForComments{} + mockVotes := &mockVoteServiceForComments{} + + handler := NewGetCommentsHandler(mockComments, mockUsers, mockVotes) + + req := httptest.NewRequest(http.MethodGet, "/xrpc/social.coves.actor.getComments?actor=did:plc:testuser", nil) + rec := httptest.NewRecorder() + + handler.HandleGetComments(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("Expected status 200, got %d", rec.Code) + } + + var response comments.GetActorCommentsResponse + if err := json.NewDecoder(rec.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + if len(response.Comments) != 1 { + t.Fatalf("Expected 1 comment, got %d", len(response.Comments)) + } + + deletedComment := response.Comments[0] + + // Verify deleted comment fields + if !deletedComment.IsDeleted { + t.Error("Expected IsDeleted to be true for deleted comment") + } + + if deletedComment.Record != nil { + t.Errorf("Expected Record to be nil for deleted comment, got %T", deletedComment.Record) + } + + if deletedComment.DeletedAt == nil || *deletedComment.DeletedAt != deletedAt { + t.Errorf("Expected DeletedAt to be %s, got %v", deletedAt, deletedComment.DeletedAt) + } + + if deletedComment.DeletionReason == nil || *deletedComment.DeletionReason != deletionReason { + t.Errorf("Expected DeletionReason to be %s, got %v", deletionReason, deletedComment.DeletionReason) + } + + // Verify author info is still present (for attribution even on deleted comments) + if deletedComment.Author == nil || deletedComment.Author.DID != "did:plc:testuser" { + t.Error("Expected deleted comment to retain author information") + } +} diff --git a/internal/atproto/lexicon/social/coves/community/comment/defs.json b/internal/atproto/lexicon/social/coves/community/comment/defs.json index 6aed983..2da348c 100644 --- a/internal/atproto/lexicon/social/coves/community/comment/defs.json +++ b/internal/atproto/lexicon/social/coves/community/comment/defs.json @@ -5,7 +5,7 @@ "commentView": { "type": "object", "description": "Base view for a single comment with voting, stats, and viewer state", - "required": ["uri", "cid", "author", "record", "post", "content", "createdAt", "indexedAt", "stats"], + "required": ["uri", "cid", "author", "record", "post", "createdAt", "indexedAt", "stats"], "properties": { "uri": { "type": "string", @@ -36,10 +36,6 @@ "ref": "#commentRef", "description": "Reference to parent comment if this is a nested reply" }, - "content": { - "type": "string", - "description": "Comment text content" - }, "embed": { "type": "union", "description": "Embedded content in the comment (images or quoted post)", diff --git a/internal/atproto/lexicon/social/coves/community/post/get.json b/internal/atproto/lexicon/social/coves/community/post/get.json index 015bbbb..a9669f6 100644 --- a/internal/atproto/lexicon/social/coves/community/post/get.json +++ b/internal/atproto/lexicon/social/coves/community/post/get.json @@ -66,12 +66,6 @@ "type": "ref", "ref": "#communityRef" }, - "title": { - "type": "string" - }, - "text": { - "type": "string" - }, "embed": { "type": "union", "description": "Embedded content (images, video, link preview, or quoted post)", diff --git a/internal/core/comments/comment_service.go b/internal/core/comments/comment_service.go index 5db3dad..192a6ff 100644 --- a/internal/core/comments/comment_service.go +++ b/internal/core/comments/comment_service.go @@ -426,7 +426,6 @@ func (s *commentService) buildCommentView( Record: commentRecord, Post: postRef, Parent: parentRef, - Content: comment.Content, Embed: embed, CreatedAt: comment.CreatedAt.Format(time.RFC3339), IndexedAt: comment.IndexedAt.Format(time.RFC3339), @@ -486,7 +485,6 @@ func (s *commentService) buildDeletedCommentView(comment *Comment) *CommentView Record: nil, // No record for deleted comments Post: postRef, Parent: parentRef, - Content: "", // Blanked content Embed: nil, CreatedAt: comment.CreatedAt.Format(time.RFC3339), IndexedAt: comment.IndexedAt.Format(time.RFC3339), @@ -975,8 +973,6 @@ func (s *commentService) buildPostView(ctx context.Context, post *posts.Post, vi Author: authorView, Record: postRecord, Community: communityRef, - Title: post.Title, - Text: post.Content, CreatedAt: post.CreatedAt, IndexedAt: post.IndexedAt, EditedAt: post.EditedAt, diff --git a/internal/core/comments/comment_service_test.go b/internal/core/comments/comment_service_test.go index c409c3a..212270a 100644 --- a/internal/core/comments/comment_service_test.go +++ b/internal/core/comments/comment_service_test.go @@ -939,7 +939,7 @@ func TestCommentService_buildThreadViews_IncludesDeletedCommentsAsPlaceholders(t assert.Equal(t, deletedComment.URI, result[0].Comment.URI) assert.True(t, result[0].Comment.IsDeleted) assert.Equal(t, DeletionReasonAuthor, *result[0].Comment.DeletionReason) - assert.Empty(t, result[0].Comment.Content) + assert.Nil(t, result[0].Comment.Record) // Deleted comments have nil record // Second comment should be the normal one assert.Equal(t, normalComment.URI, result[1].Comment.URI) @@ -1031,7 +1031,8 @@ func TestCommentService_buildCommentView_BasicFields(t *testing.T) { // Verify basic fields assert.Equal(t, commentURI, result.URI) assert.Equal(t, comment.CID, result.CID) - assert.Equal(t, comment.Content, result.Content) + record := result.Record.(*CommentRecord) + assert.Equal(t, comment.Content, record.Content) assert.NotNil(t, result.Author) assert.Equal(t, "did:plc:commenter123", result.Author.DID) assert.Equal(t, "commenter.test", result.Author.Handle) diff --git a/internal/core/comments/view_models.go b/internal/core/comments/view_models.go index 942f4fc..4ab419e 100644 --- a/internal/core/comments/view_models.go +++ b/internal/core/comments/view_models.go @@ -16,7 +16,6 @@ type CommentView struct { Post *CommentRef `json:"post"` Parent *CommentRef `json:"parent,omitempty"` Stats *CommentStats `json:"stats"` - Content string `json:"content"` CreatedAt string `json:"createdAt"` IndexedAt string `json:"indexedAt"` URI string `json:"uri"` diff --git a/internal/core/posts/post.go b/internal/core/posts/post.go index 58eb9a3..d5c6cd3 100644 --- a/internal/core/posts/post.go +++ b/internal/core/posts/post.go @@ -91,8 +91,6 @@ type PostView struct { Embed interface{} `json:"embed,omitempty"` Language *string `json:"language,omitempty"` EditedAt *time.Time `json:"editedAt,omitempty"` - Title *string `json:"title,omitempty"` - Text *string `json:"text,omitempty"` Viewer *ViewerState `json:"viewer,omitempty"` Author *AuthorView `json:"author"` Stats *PostStats `json:"stats,omitempty"` diff --git a/internal/db/postgres/feed_repo_base.go b/internal/db/postgres/feed_repo_base.go index e370a52..3755ed6 100644 --- a/internal/db/postgres/feed_repo_base.go +++ b/internal/db/postgres/feed_repo_base.go @@ -358,10 +358,6 @@ func (r *feedRepoBase) scanFeedPost(rows *sql.Rows) (*posts.PostView, float64, e } postView.Community = &communityRef - // Set optional fields - postView.Title = nullStringPtr(title) - postView.Text = nullStringPtr(content) - // Parse facets JSON into local variable (will be added to record below) // Log errors but continue - a single malformed post shouldn't break the entire feed var facetArray []interface{} diff --git a/internal/db/postgres/post_repo.go b/internal/db/postgres/post_repo.go index 4c9de33..699c5f9 100644 --- a/internal/db/postgres/post_repo.go +++ b/internal/db/postgres/post_repo.go @@ -346,12 +346,6 @@ func (r *postgresPostRepo) scanAuthorPost(rows *sql.Rows) (*posts.PostView, erro postView.Community = &communityRef // Set optional fields - if title.Valid { - postView.Title = &title.String - } - if content.Valid { - postView.Text = &content.String - } if editedAt.Valid { postView.EditedAt = &editedAt.Time } diff --git a/tests/integration/author_posts_e2e_test.go b/tests/integration/author_posts_e2e_test.go index f55bf31..f59e128 100644 --- a/tests/integration/author_posts_e2e_test.go +++ b/tests/integration/author_posts_e2e_test.go @@ -25,6 +25,24 @@ import ( "github.com/pressly/goose/v3" ) +// getPostTitleFromView extracts title from PostView.Record. +// Fails the test if Record structure is invalid (should not happen in valid responses). +func getPostTitleFromView(t *testing.T, pv *posts.PostView) string { + t.Helper() + if pv.Record == nil { + t.Fatalf("getPostTitleFromView: Record is nil for post URI %s", pv.URI) + } + record, ok := pv.Record.(map[string]interface{}) + if !ok { + t.Fatalf("getPostTitleFromView: Record is %T, not map[string]interface{}", pv.Record) + } + title, ok := record["title"].(string) + if !ok { + t.Fatalf("getPostTitleFromView: title field missing or not string, Record: %+v", record) + } + return title +} + // TestGetAuthorPosts_E2E_Success tests the full author posts flow with real PDS // Flow: Create user on PDS → Create posts → Query via XRPC → Verify response func TestGetAuthorPosts_E2E_Success(t *testing.T) { @@ -629,8 +647,8 @@ func TestGetAuthorPosts_WithJetstreamIndexing(t *testing.T) { } if len(response.Feed) > 0 && response.Feed[0].Post != nil { - title := response.Feed[0].Post.Title - if title == nil || *title != "Jetstream Indexed Post" { + title := getPostTitleFromView(t, response.Feed[0].Post) + if title != "Jetstream Indexed Post" { t.Errorf("Expected title 'Jetstream Indexed Post', got %v", title) } } diff --git a/tests/integration/blob_upload_e2e_test.go b/tests/integration/blob_upload_e2e_test.go index 495e317..e77c6a2 100644 --- a/tests/integration/blob_upload_e2e_test.go +++ b/tests/integration/blob_upload_e2e_test.go @@ -164,12 +164,23 @@ func TestBlobUpload_E2E_PostWithImages(t *testing.T) { // STEP 6: Verify blob URL transformation in feed responses // This is what the feed handler would do before returning to client + // Build the record as the feed repos do + record := map[string]interface{}{ + "$type": "social.coves.community.post", + "createdAt": indexedPost.CreatedAt.Format(time.RFC3339), + } + if indexedPost.Title != nil { + record["title"] = *indexedPost.Title + } + if indexedPost.Content != nil { + record["content"] = *indexedPost.Content + } + postView := &posts.PostView{ URI: indexedPost.URI, CID: indexedPost.CID, - Title: indexedPost.Title, - Text: indexedPost.Content, // Content maps to Text in PostView - Embed: embedMap, // Use parsed embed map + Record: record, + Embed: embedMap, // Use parsed embed map CreatedAt: indexedPost.CreatedAt, Community: &posts.CommunityRef{ DID: community.DID, diff --git a/tests/integration/feed_test.go b/tests/integration/feed_test.go index 4b49d03..8930883 100644 --- a/tests/integration/feed_test.go +++ b/tests/integration/feed_test.go @@ -4,6 +4,7 @@ import ( "Coves/internal/api/handlers/communityFeed" "Coves/internal/core/communities" "Coves/internal/core/communityFeeds" + "Coves/internal/core/posts" "Coves/internal/db/postgres" "context" "encoding/json" @@ -17,6 +18,24 @@ import ( "github.com/stretchr/testify/require" ) +// getPostTitle extracts title from PostView.Record. +// Fails the test if Record structure is invalid (should not happen in valid responses). +func getPostTitle(t *testing.T, pv *posts.PostView) string { + t.Helper() + if pv.Record == nil { + t.Fatalf("getPostTitle: Record is nil for post URI %s", pv.URI) + } + record, ok := pv.Record.(map[string]interface{}) + if !ok { + t.Fatalf("getPostTitle: Record is %T, not map[string]interface{}", pv.Record) + } + title, ok := record["title"].(string) + if !ok { + t.Fatalf("getPostTitle: title field missing or not string, Record: %+v", record) + } + return title +} + // TestGetCommunityFeed_Hot tests hot feed sorting algorithm func TestGetCommunityFeed_Hot(t *testing.T) { if testing.Short() { @@ -150,7 +169,7 @@ func TestGetCommunityFeed_Top_WithTimeframe(t *testing.T) { assert.Len(t, response.Feed, 2) // Verify top-ranked post (highest score) - assert.Equal(t, "2 hours old", *response.Feed[0].Post.Title) + assert.Equal(t, "2 hours old", getPostTitle(t, response.Feed[0].Post)) assert.Equal(t, 100, response.Feed[0].Post.Stats.Score) }) @@ -169,7 +188,7 @@ func TestGetCommunityFeed_Top_WithTimeframe(t *testing.T) { assert.Len(t, response.Feed, 3) // Highest score should be first - assert.Equal(t, "2 days old", *response.Feed[0].Post.Title) + assert.Equal(t, "2 days old", getPostTitle(t, response.Feed[0].Post)) assert.Equal(t, 200, response.Feed[0].Post.Stats.Score) }) } @@ -227,9 +246,9 @@ func TestGetCommunityFeed_New(t *testing.T) { assert.Len(t, response.Feed, 3) // Verify chronological order (newest first) - assert.Equal(t, "Newest post", *response.Feed[0].Post.Title) - assert.Equal(t, "Middle post", *response.Feed[1].Post.Title) - assert.Equal(t, "Oldest post", *response.Feed[2].Post.Title) + assert.Equal(t, "Newest post", getPostTitle(t, response.Feed[0].Post)) + assert.Equal(t, "Middle post", getPostTitle(t, response.Feed[1].Post)) + assert.Equal(t, "Oldest post", getPostTitle(t, response.Feed[2].Post)) } // TestGetCommunityFeed_Pagination tests cursor-based pagination @@ -580,7 +599,7 @@ func TestGetCommunityFeed_HotPaginationBug(t *testing.T) { // The highest hot_rank post should be first (recent with low-medium score) firstPostURI := page1.Feed[0].Post.URI - t.Logf("Page 1 - First post: %s (URI: %s)", *page1.Feed[0].Post.Title, firstPostURI) + t.Logf("Page 1 - First post: %s (URI: %s)", getPostTitle(t, page1.Feed[0].Post), firstPostURI) t.Logf("Page 1 - Cursor: %s", *page1.Cursor) // Page 2: Use cursor - this is where the bug would occur @@ -606,7 +625,7 @@ func TestGetCommunityFeed_HotPaginationBug(t *testing.T) { seenURIs := map[string]bool{firstPostURI: true} for _, p := range page2.Feed { allURIs = append(allURIs, p.Post.URI) - t.Logf("Page 2 - Post: %s (URI: %s)", *p.Post.Title, p.Post.URI) + t.Logf("Page 2 - Post: %s (URI: %s)", getPostTitle(t, p.Post), p.Post.URI) // Check for duplicates if seenURIs[p.Post.URI] { t.Errorf("Duplicate post found: %s", p.Post.URI) -- 2.51.2