Added more fields to Account Entity #1499
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR adds three ActivityPub URL fields— Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/account/account.repository.knex.ts (4)
253-268: Missing new fields in SELECT clause.The
getBySitequery doesn't select the new fields (ap_outbox_url,ap_following_url,ap_liked_url,ap_public_key,ap_private_key), butmapRowToAccountEntityexpects them. This will result inundefinedvalues for these fields on returned entities.🔧 Proposed fix
const accountRow = await this.db('accounts') .where('id', user.account_id) .select( 'accounts.id', 'accounts.uuid', 'accounts.username', 'accounts.name', 'accounts.bio', 'accounts.url', 'accounts.avatar_url', 'accounts.banner_image_url', 'accounts.ap_id', 'accounts.ap_followers_url', 'accounts.ap_inbox_url', + 'accounts.ap_outbox_url', + 'accounts.ap_following_url', + 'accounts.ap_liked_url', + 'accounts.ap_public_key', + 'accounts.ap_private_key', 'accounts.custom_fields', ) .first();
282-295: Missing new fields in SELECT clause.Same issue as
getBySite- thegetByApIdquery doesn't select the new fields required bymapRowToAccountEntity.🔧 Proposed fix
.select( 'accounts.id', 'accounts.uuid', 'accounts.username', 'accounts.name', 'accounts.bio', 'accounts.url', 'accounts.avatar_url', 'accounts.banner_image_url', 'accounts.ap_id', 'accounts.ap_followers_url', 'accounts.ap_inbox_url', + 'accounts.ap_outbox_url', + 'accounts.ap_following_url', + 'accounts.ap_liked_url', + 'accounts.ap_public_key', + 'accounts.ap_private_key', 'accounts.custom_fields', 'users.site_id', )
310-324: Missing new fields in SELECT clause.Same issue -
getByIddoesn't select the new fields.🔧 Proposed fix
.select( 'accounts.id', 'accounts.uuid', 'accounts.username', 'accounts.name', 'accounts.bio', 'accounts.url', 'accounts.avatar_url', 'accounts.banner_image_url', 'accounts.custom_fields', 'accounts.ap_id', 'accounts.ap_followers_url', 'accounts.ap_inbox_url', + 'accounts.ap_outbox_url', + 'accounts.ap_following_url', + 'accounts.ap_liked_url', + 'accounts.ap_public_key', + 'accounts.ap_private_key', 'users.site_id', )
341-355: Missing new fields in SELECT clause.Same issue -
getByInboxUrldoesn't select the new fields.🔧 Proposed fix
.select( 'accounts.id', 'accounts.uuid', 'accounts.username', 'accounts.name', 'accounts.bio', 'accounts.url', 'accounts.avatar_url', 'accounts.banner_image_url', 'accounts.ap_id', 'accounts.ap_followers_url', 'accounts.ap_inbox_url', + 'accounts.ap_outbox_url', + 'accounts.ap_following_url', + 'accounts.ap_liked_url', + 'accounts.ap_public_key', + 'accounts.ap_private_key', 'accounts.custom_fields', 'users.site_id', )
♻️ Duplicate comments (3)
src/account/account.entity.ts (1)
373-375: Consider keepingAccountDraftDataunexported.A past review noted this type may not be used externally. If it's only consumed internally (e.g., by
AccountEntity.draft), keeping it unexported reduces the public API surface.#!/bin/bash # Check if AccountDraftData is imported/used outside of account.entity.ts rg -n "AccountDraftData" --type=ts -g '!src/account/account.entity.ts'src/post/content.unit.test.ts (1)
264-280: Tests don't need to be async.These test functions are marked
asyncbut don't contain anyawaitexpressions. Thepreparer.prepare()call is synchronous. While thebeforeEachcorrectly usesasync/awaitfor account creation, the individual test functions don't need to be async.src/post/post.repository.knex.ts (1)
137-141: Private key loading concern already raised.The security concern about loading
ap_private_keyinto the entity for all post queries has been raised in the past review comments. As noted there, the risk is accidentally leaking private keys through API responses. Consider whether the private key is actually needed in these post-fetching contexts, or if it could be loaded separately only when required (e.g., in the keypair dispatcher).
🧹 Nitpick comments (1)
src/helpers/activitypub/activity.unit.test.ts (1)
64-101: Consider refactoring remaining tests to use test helpers for consistency.The
buildCreateActivityAndObjectFromPostandbuildUpdateActivityAndObjectFromPosttest blocks still use theObject.createpattern whilebuildAnnounceActivityForPostnow uses the test helpers. For long-term maintainability and consistency, consider migrating these tests to usecreateTestInternalAccount/createTestExternalAccountas well.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/account/account.entity.tssrc/account/account.entity.unit.test.tssrc/account/account.repository.knex.tssrc/feed/feed-update.service.unit.test.tssrc/helpers/activitypub/activity.unit.test.tssrc/http/api/feed.unit.test.tssrc/http/api/helpers/post.unit.test.tssrc/http/api/post.controller.unit.test.tssrc/http/host-data-context-loader.tssrc/post/content.unit.test.tssrc/post/post.repository.knex.tssrc/test/account-entity-test-helpers.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Never use direct string comparisons for ActivityPub IDs; use SHA256 hash lookups instead - see ADR-0009
Always use the Result helper functions (isError,getError,getValue) with Result types instead of destructuring directly
Dependency injection parameter names must match registration names in Awilix CLASSIC injection mode
Routes should be defined using decorators (@APIRoute,@RequireRoles) rather than direct registration
Prefer class-based architecture with dependency injection over function factories for handlers
Repositories should not be used directly; they should be used through services
Controllers should be lean and delegate to services where appropriate
Views can talk directly to the database if necessary but should not be responsible for any business logic
Prefer immutable entities that generate domain events over mutable entities with dirty flags
Prefer error objects with context over string literal errors in Result types
Avoid mutable entities with dirty flags; use immutable entities with domain events instead
Files:
src/http/host-data-context-loader.tssrc/helpers/activitypub/activity.unit.test.tssrc/test/account-entity-test-helpers.tssrc/account/account.repository.knex.tssrc/post/post.repository.knex.tssrc/post/content.unit.test.tssrc/http/api/helpers/post.unit.test.tssrc/account/account.entity.unit.test.tssrc/http/api/post.controller.unit.test.tssrc/http/api/feed.unit.test.tssrc/account/account.entity.tssrc/feed/feed-update.service.unit.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
All unit & integration test files should have the prefix
.test.ts
Files:
src/helpers/activitypub/activity.unit.test.tssrc/post/content.unit.test.tssrc/http/api/helpers/post.unit.test.tssrc/account/account.entity.unit.test.tssrc/http/api/post.controller.unit.test.tssrc/http/api/feed.unit.test.tssrc/feed/feed-update.service.unit.test.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Unit test files should use the file extension
.unit.test.tsto indicate the test type
Files:
src/helpers/activitypub/activity.unit.test.tssrc/post/content.unit.test.tssrc/http/api/helpers/post.unit.test.tssrc/account/account.entity.unit.test.tssrc/http/api/post.controller.unit.test.tssrc/http/api/feed.unit.test.tssrc/feed/feed-update.service.unit.test.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 737
File: migrate/migrations/000054_populate-outboxes-table.up.sql:56-63
Timestamp: 2025-05-27T09:09:22.110Z
Learning: In the outboxes table migration for Ghost ActivityPub, the author_id field always refers to the author of the original post, not the person performing the outbox action. For reposts (outbox_type = 1), author_id should be posts.author_id (original author), while user_id captures who made the repost via the joins through accounts and users tables.
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 602
File: src/http/api/follow.ts:80-88
Timestamp: 2025-05-01T07:06:59.816Z
Learning: In the ActivityPub codebase, Account entities use the `apId` property when referencing ActivityPub IDs, not `ap_id`.
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1329
File: .github/renovate.json5:31-34
Timestamp: 2025-09-25T12:39:45.755Z
Learning: The jobs/update-external-accounts directory was removed from the TryGhost/ActivityPub repository and no longer exists, so Renovate configuration exclusions referencing this path are no longer needed.
📚 Learning: 2025-09-25T12:39:45.755Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1329
File: .github/renovate.json5:31-34
Timestamp: 2025-09-25T12:39:45.755Z
Learning: The jobs/update-external-accounts directory was removed from the TryGhost/ActivityPub repository and no longer exists, so Renovate configuration exclusions referencing this path are no longer needed.
Applied to files:
src/helpers/activitypub/activity.unit.test.tssrc/post/post.repository.knex.tssrc/account/account.entity.ts
📚 Learning: 2025-06-16T15:43:24.787Z
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 858
File: features/support/content.js:24-27
Timestamp: 2025-06-16T15:43:24.787Z
Learning: In features/support/content.js, the publishArticle() and publishNote() helper functions handle endpoints with different return types: the webhook endpoint returns the post object directly, while the note action endpoint returns a wrapper object with a `post` property. Both helpers are designed to normalize these differences by returning the full post object.
Applied to files:
src/helpers/activitypub/activity.unit.test.ts
📚 Learning: 2025-04-24T10:41:15.124Z
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 566
File: src/account/account.entity.ts:110-123
Timestamp: 2025-04-24T10:41:15.124Z
Learning: In the Account entity's `updateProfile` method, the current implementation using a helper function that checks for undefined values is type-safe at runtime, even though TypeScript might not perfectly infer this due to the function's return type declaration.
Applied to files:
src/test/account-entity-test-helpers.tssrc/post/content.unit.test.tssrc/http/api/helpers/post.unit.test.tssrc/account/account.entity.unit.test.tssrc/http/api/post.controller.unit.test.tssrc/http/api/feed.unit.test.tssrc/feed/feed-update.service.unit.test.ts
📚 Learning: 2025-05-27T09:09:22.110Z
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 737
File: migrate/migrations/000054_populate-outboxes-table.up.sql:56-63
Timestamp: 2025-05-27T09:09:22.110Z
Learning: In the outboxes table migration for Ghost ActivityPub, the author_id field always refers to the author of the original post, not the person performing the outbox action. For reposts (outbox_type = 1), author_id should be posts.author_id (original author), while user_id captures who made the repost via the joins through accounts and users tables.
Applied to files:
src/post/post.repository.knex.ts
📚 Learning: 2025-05-01T07:06:59.816Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 602
File: src/http/api/follow.ts:80-88
Timestamp: 2025-05-01T07:06:59.816Z
Learning: In the ActivityPub codebase, Account entities use the `apId` property when referencing ActivityPub IDs, not `ap_id`.
Applied to files:
src/post/post.repository.knex.tssrc/account/account.entity.ts
📚 Learning: 2025-07-16T05:20:47.031Z
Learnt from: vershwal
Repo: TryGhost/ActivityPub PR: 1050
File: migrate/migrations/000061_add-ghost-ap-post-mappings-table.up.sql:8-10
Timestamp: 2025-07-16T05:20:47.031Z
Learning: In the TryGhost/ActivityPub codebase, the established pattern for large string fields like ap_id is to use hash-based lookups via generated SHA-256 hash columns with UNIQUE constraints. This approach provides fast fixed-size binary lookups instead of slower string comparisons, while the unique constraint on the hash effectively ensures uniqueness of the original string field due to SHA-256's cryptographic properties. This pattern is used in both the posts table and ghost_ap_post_mappings table.
Applied to files:
src/post/post.repository.knex.ts
📚 Learning: 2025-11-25T10:54:15.904Z
Learnt from: CR
Repo: TryGhost/ActivityPub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:54:15.904Z
Learning: Applies to src/**/*.{ts,tsx} : Avoid mutable entities with dirty flags; use immutable entities with domain events instead
Applied to files:
src/account/account.entity.unit.test.tssrc/feed/feed-update.service.unit.test.ts
🧬 Code graph analysis (9)
src/helpers/activitypub/activity.unit.test.ts (2)
src/post/post.entity.ts (1)
Post(143-687)src/test/account-entity-test-helpers.ts (2)
createTestExternalAccount(101-131)createTestInternalAccount(72-99)
src/test/account-entity-test-helpers.ts (1)
src/account/account.entity.ts (2)
draft(165-202)AccountEntity(79-322)
src/account/account.repository.knex.ts (2)
src/account/account.entity.ts (2)
AccountEntity(79-322)draft(165-202)src/core/url.ts (1)
parseURL(1-7)
src/post/post.repository.knex.ts (1)
src/core/url.ts (1)
parseURL(1-7)
src/http/api/helpers/post.unit.test.ts (1)
src/test/account-entity-test-helpers.ts (1)
createTestInternalAccount(72-99)
src/account/account.entity.unit.test.ts (2)
src/test/account-entity-test-helpers.ts (1)
createTestInternalAccount(72-99)src/account/account.entity.ts (2)
AccountEntity(79-322)draft(165-202)
src/http/api/post.controller.unit.test.ts (1)
src/test/account-entity-test-helpers.ts (1)
createTestInternalAccount(72-99)
src/http/api/feed.unit.test.ts (1)
src/test/account-entity-test-helpers.ts (1)
createTestInternalAccount(72-99)
src/feed/feed-update.service.unit.test.ts (1)
src/test/account-entity-test-helpers.ts (1)
createTestInternalAccount(72-99)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (31)
src/account/account.entity.ts (4)
3-4: LGTM!The import of
exportJwkfrom@fedify/fedifyis appropriate for serializing CryptoKey objects to JWK format for storage.
30-34: LGTM!The new ActivityPub fields (
apOutbox,apFollowing,apLiked,apPublicKey,apPrivateKey) are well-typed and appropriately nullable where external accounts may not have private keys.
92-96: LGTM!Constructor parameters correctly extended with the new ActivityPub fields, maintaining consistency with the interface.
135-163: LGTM!The
fromDraftmethod is correctly converted to async to accommodate the asyncexportJwkcalls. The key serialization pattern usingJSON.stringify(await exportJwk(...))is consistent and properly handles the nullableapPrivateKey.src/account/account.repository.knex.ts (4)
35-39: LGTM!The
AccountRowinterface is correctly extended with the new ActivityPub fields.
64-72: LGTM!The insert statement correctly includes the new fields and properly serializes the keys using
exportJwk.
95-95: LGTM!Correctly awaiting the now-async
AccountEntity.fromDraftmethod.
385-389: LGTM!The mapping correctly includes the new fields. However, this depends on the queries actually selecting these columns, which is addressed in the previous comments.
src/test/account-entity-test-helpers.ts (3)
1-1: LGTM!Import added correctly for key serialization.
88-98: LGTM!The key serialization pattern using
exportJwkis consistent withAccountEntity.fromDraft. This helper effectively reduces boilerplate across tests.
123-130: LGTM!External accounts correctly serialize only the public key and set
apPrivateKeytonull, matching the expected behavior that external accounts don't have private keys.src/http/api/post.controller.unit.test.ts (3)
5-5: LGTM!Appropriate use of type-only import since
AccountEntityis only used as a type annotation in this file.
21-21: LGTM!Switching to
createTestInternalAccountsimplifies the test setup by encapsulating the draft creation and key serialization logic.
96-106: LGTM!Clean refactor using the new async test helper. The
beforeEachcorrectly awaits the asynccreateTestInternalAccountcall.src/feed/feed-update.service.unit.test.ts (4)
5-5: LGTM!Type-only import is appropriate since
AccountEntityis used for type annotations and casting.
18-18: LGTM!Import of the new test helper aligns with the PR-wide refactoring pattern.
41-50: LGTM!Test setup correctly uses the async helper and awaits the result.
218-229: LGTM!Blocked account test correctly uses the async helper for creating test accounts.
src/helpers/activitypub/activity.unit.test.ts (1)
328-364: Good adoption of test helpers for the Announce activity tests.The refactored setup using
createTestExternalAccountandcreateTestInternalAccountwithPost.createFromDatais cleaner and aligns with the PR's goal of centralizing test fixture creation. The asyncbeforeEachcorrectly awaits the helper functions.src/post/content.unit.test.ts (1)
247-262: Test helper setup is correctly implemented.The
beforeEachproperly awaits the asynccreateTestExternalAccounthelper and provides appropriate test data. The type-only import ofAccountEntityis appropriate since the entity is not directly instantiated here.src/http/api/feed.unit.test.ts (1)
125-144: Clean migration to test helper pattern.The refactored
beforeEachcorrectly usesasync/awaitwithcreateTestInternalAccount. The type-only import ofAccountEntityis appropriate since the entity is only used for type annotation on line 25.src/account/account.entity.unit.test.ts (2)
74-104: Appropriate use of lower-level API for testingfromDraft.These tests correctly use
createInternalAccountDraftDataandAccountEntity.draftto test the entity creation path directly, rather than using the higher-levelcreateTestInternalAccounthelper. This is the right approach for unit testing thefromDraftmethod itself. TheawaitonAccountEntity.fromDraftcorrectly handles the now-async method.
516-547: Block/unblock tests correctly create separate account instances.The tests properly create distinct account entities with different IDs (1 and 2) to test blocking/unblocking another account. This ensures the domain logic is tested correctly.
src/http/host-data-context-loader.ts (3)
44-48: New ActivityPub fields correctly added to the query.The SQL selection includes all the new ActivityPub-related fields (
ap_outbox_url,ap_following_url,ap_liked_url,ap_public_key,ap_private_key) following the existing naming convention.
86-91: Field mapping tocreateFromRowis consistent.The new fields are correctly passed to
createFromRowwith the appropriate snake_case naming convention matching the database columns.
89-90: No action required. The code correctly ensures private key handling aligns with account type expectations. TheHostDataContextLoaderloads accounts joined with users and sites, which guaranteesisInternal = true(computed assite_id !== null). Private keys are present only for these internal accounts, as expected.src/post/post.repository.knex.ts (3)
68-72: Interface additions are consistent with query selections.The new fields follow the established naming pattern (prefixing author fields with
author_ap_*). The types are appropriate: URLs arestring | nulland will be parsed withparseURL, while keys usestringandstring | null.
957-961: Field mapping is consistent with the new interface.The mapping correctly uses
parseURLfor URL fields (apOutbox,apFollowing,apLiked) and passes string values directly for keys (apPublicKey,apPrivateKey). This aligns with theAccountEntity.createsignature updates.
1053-1057: Outbox query mirrors getByQuery changes.The same fields are selected in
getOutboxForAccountas ingetByQuery, maintaining consistency. The same private key concern applies here.src/http/api/helpers/post.unit.test.ts (2)
5-18: Clean refactoring to use the new test helper.The migration from manual
AccountEntity.draft+AccountEntity.createtocreateTestInternalAccountreduces test boilerplate and centralizes account creation logic. The function correctly returns the Promise without redundantasync/await, and all callers properly await it.
20-32: Tests properly adapted to async account creation.All test cases correctly use
asyncfunctions andawait createAuthor(), ensuring the Promise resolves before proceeding with assertions.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 3
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/http/host-data-context-loader.ts`:
- Around line 84-86: Repository query methods getByApId, getById, and
getByInboxUrl don't select the newly added AP URL columns, so createFromRow /
mapRowToAccountEntity receive nulls; update each method's SELECT clause to
include ap_outbox_url, ap_following_url, and ap_liked_url (the same column names
used when mapping to createFromRow/mapRowToAccountEntity) so those fields are
populated when accounts are fetched.
🧹 Nitpick comments (1)
src/account/account.entity.unit.test.ts (1)
21-785: Consider extracting common test fixtures to reduce duplication.The test data setup is highly repetitive across tests (same host, URL patterns, etc.). Consider creating a shared default configuration object that tests can spread and override only when needed:
const defaultAccountOverrides = { host: new URL('http://example.com'), username: 'testuser', name: 'Test User', bio: 'Test Bio', url: new URL('http://example.com/url'), avatarUrl: new URL('http://example.com/avatar.png'), bannerImageUrl: new URL('http://example.com/banner.png'), customFields: null, }; // Usage in tests: const account = await createTestInternalAccount(1, { ...defaultAccountOverrides, name: 'Custom Name', // only override what's relevant });This would reduce boilerplate while keeping tests explicit about what matters for each case.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/account/account.entity.tssrc/account/account.entity.unit.test.tssrc/account/account.repository.knex.tssrc/http/host-data-context-loader.tssrc/post/content.unit.test.tssrc/post/post.repository.knex.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/account/account.repository.knex.ts
- src/post/content.unit.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Never use direct string comparisons for ActivityPub IDs; use SHA256 hash lookups instead - see ADR-0009
Always use the Result helper functions (isError,getError,getValue) with Result types instead of destructuring directly
Dependency injection parameter names must match registration names in Awilix CLASSIC injection mode
Routes should be defined using decorators (@APIRoute,@RequireRoles) rather than direct registration
Prefer class-based architecture with dependency injection over function factories for handlers
Repositories should not be used directly; they should be used through services
Controllers should be lean and delegate to services where appropriate
Views can talk directly to the database if necessary but should not be responsible for any business logic
Prefer immutable entities that generate domain events over mutable entities with dirty flags
Prefer error objects with context over string literal errors in Result types
Avoid mutable entities with dirty flags; use immutable entities with domain events instead
Files:
src/account/account.entity.tssrc/account/account.entity.unit.test.tssrc/http/host-data-context-loader.tssrc/post/post.repository.knex.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
All unit & integration test files should have the prefix
.test.ts
Files:
src/account/account.entity.unit.test.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Unit test files should use the file extension
.unit.test.tsto indicate the test type
Files:
src/account/account.entity.unit.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 602
File: src/http/api/follow.ts:80-88
Timestamp: 2025-05-01T07:06:59.816Z
Learning: In the ActivityPub codebase, Account entities use the `apId` property when referencing ActivityPub IDs, not `ap_id`.
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 737
File: migrate/migrations/000054_populate-outboxes-table.up.sql:56-63
Timestamp: 2025-05-27T09:09:22.110Z
Learning: In the outboxes table migration for Ghost ActivityPub, the author_id field always refers to the author of the original post, not the person performing the outbox action. For reposts (outbox_type = 1), author_id should be posts.author_id (original author), while user_id captures who made the repost via the joins through accounts and users tables.
📚 Learning: 2025-05-01T07:06:59.816Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 602
File: src/http/api/follow.ts:80-88
Timestamp: 2025-05-01T07:06:59.816Z
Learning: In the ActivityPub codebase, Account entities use the `apId` property when referencing ActivityPub IDs, not `ap_id`.
Applied to files:
src/account/account.entity.tssrc/post/post.repository.knex.ts
📚 Learning: 2025-04-24T10:41:15.124Z
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 566
File: src/account/account.entity.ts:110-123
Timestamp: 2025-04-24T10:41:15.124Z
Learning: In the Account entity's `updateProfile` method, the current implementation using a helper function that checks for undefined values is type-safe at runtime, even though TypeScript might not perfectly infer this due to the function's return type declaration.
Applied to files:
src/account/account.entity.tssrc/account/account.entity.unit.test.ts
📚 Learning: 2025-05-27T09:09:22.110Z
Learnt from: allouis
Repo: TryGhost/ActivityPub PR: 737
File: migrate/migrations/000054_populate-outboxes-table.up.sql:56-63
Timestamp: 2025-05-27T09:09:22.110Z
Learning: In the outboxes table migration for Ghost ActivityPub, the author_id field always refers to the author of the original post, not the person performing the outbox action. For reposts (outbox_type = 1), author_id should be posts.author_id (original author), while user_id captures who made the repost via the joins through accounts and users tables.
Applied to files:
src/post/post.repository.knex.ts
📚 Learning: 2025-07-16T05:20:47.031Z
Learnt from: vershwal
Repo: TryGhost/ActivityPub PR: 1050
File: migrate/migrations/000061_add-ghost-ap-post-mappings-table.up.sql:8-10
Timestamp: 2025-07-16T05:20:47.031Z
Learning: In the TryGhost/ActivityPub codebase, the established pattern for large string fields like ap_id is to use hash-based lookups via generated SHA-256 hash columns with UNIQUE constraints. This approach provides fast fixed-size binary lookups instead of slower string comparisons, while the unique constraint on the hash effectively ensures uniqueness of the original string field due to SHA-256's cryptographic properties. This pattern is used in both the posts table and ghost_ap_post_mappings table.
Applied to files:
src/post/post.repository.knex.ts
📚 Learning: 2025-09-25T12:39:45.755Z
Learnt from: mike182uk
Repo: TryGhost/ActivityPub PR: 1329
File: .github/renovate.json5:31-34
Timestamp: 2025-09-25T12:39:45.755Z
Learning: The jobs/update-external-accounts directory was removed from the TryGhost/ActivityPub repository and no longer exists, so Renovate configuration exclusions referencing this path are no longer needed.
Applied to files:
src/post/post.repository.knex.ts
🧬 Code graph analysis (2)
src/account/account.entity.unit.test.ts (1)
src/test/account-entity-test-helpers.ts (1)
createTestInternalAccount(70-90)
src/post/post.repository.knex.ts (1)
src/core/url.ts (1)
parseURL(1-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: build / Build Docker Images (linux/amd64, amd64)
🔇 Additional comments (21)
src/account/account.entity.unit.test.ts (9)
16-19: LGTM!The import structure is clean, correctly importing both helpers:
createInternalAccountDraftDatafor tests that specifically verify draft behavior, andcreateTestInternalAccountfor all other test cases.
22-72: LGTM!The refactored tests correctly use
async/awaitwithcreateTestInternalAccountand maintain the same test coverage forapIdandapFollowersURL generation.
74-147: LGTM!The
fromDrafttests appropriately continue using the draft-based approach (createInternalAccountDraftData→AccountEntity.draft→AccountEntity.fromDraft) since they specifically verify that workflow. This is the correct design choice.
149-193: LGTM!Tests for
getApIdForPostcorrectly verify URL generation for bothArticleandNotepost types using the new helper.
195-493: LGTM!The
updateProfiletests provide comprehensive coverage including individual property updates, multiple simultaneous updates, null value handling, and proper event emission verification. The refactoring to usecreateTestInternalAccountis cleanly applied.
496-600: LGTM!Block and unblock tests correctly verify self-operation prevention (empty events) and proper event emission for cross-account operations. The two-account setup with distinct IDs is correctly implemented.
602-656: LGTM!Domain block and unblock tests correctly verify event emission with proper domain URL and account ID verification through the event getters.
658-762: LGTM!Follow and unfollow tests mirror the block/unblock pattern correctly, verifying self-operation prevention and proper event emission for cross-account operations.
764-785: LGTM!The
readAllNotificationstest correctly verifies event emission with the expected account ID.src/account/account.entity.ts (7)
28-30: LGTM! New ActivityPub fields added consistently to Account interface.The three new fields (
apOutbox,apFollowing,apLiked) follow the existing pattern for nullable URL fields likeapFollowersandapInbox.
50-69: LGTM! AccountDraft updated with new AP fields.The draft interface correctly includes all three new fields with the appropriate
URL | nulltype.
76-94: LGTM! Constructor updated with new fields.The immutable entity pattern is maintained with the three new readonly properties.
105-125: LGTM! Static create method propagates new fields correctly.The
createmethod properly passes through the new AP fields from the data object to the constructor.
127-148: LGTM! fromDraft method correctly initializes new fields from draft.The draft-to-entity construction path properly maps the new AP fields.
150-187: LGTM! Draft computation logic follows established URL patterns.The URL construction for internal accounts (
/.ghost/activitypub/outbox/index, etc.) is consistent with existing endpoints likeapFollowersandapInbox. External accounts correctly pass through the provided URLs.
339-358: LGTM! ExternalAccountDraftData type updated with new fields.The type correctly requires the new AP URL fields for external accounts.
src/http/host-data-context-loader.ts (1)
44-46: LGTM! Database query extended to select new AP URL fields.The new columns (
account_ap_outbox_url,account_ap_following_url,account_ap_liked_url) follow the existing naming convention.src/post/post.repository.knex.ts (4)
68-70: LGTM! PostRow interface extended with new author AP URL fields.The interface correctly adds the three new string | null fields matching the database column types.
135-137: LGTM! Query selects new AP URL columns.The
getByQuerymethod correctly selects the new account AP URL fields with appropriate aliases.
940-957: LGTM! AccountEntity creation includes new AP URL fields.The mapping correctly uses
parseURLfor the new fields, consistent with the existing pattern forapFollowersandapInbox. TheparseURLutility safely handles null values.
1047-1049: LGTM! Outbox query selects new AP URL columns.The
getOutboxForAccountmethod correctly includes the new author AP URL fields in its select list, consistent with thegetByQuerymethod.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
ref https://linear.app/ghost/issue/BER-3012
siteService.getSiteByHostandaccountService.getDefaultAccountForSitein dispatchers.js, as they create 3 database queries for each dispatcher call