Update AccountService.recordAccountFollow to use Account entity

[mike182uk]

refs https://github.com/TryGhost/ActivityPub/pull/441/files#diff-fb5a965e319e4cc068e247181f63852dc961379121ca4cad8a64a70d025866a5

• Updated Account type to have uuid as this was missing
• Introduced asAccountEntity helper that maps Account type to Account entity
• Updated AccountService.recordAccountFollow to accept Account entity instead of Account type

[coderabbitai]

Walkthrough

The pull request updates several parts of the account-related code. The import statements for the Account type have been modified across multiple files, moving from 'account/types' to either './account.entity' or 'account/account.entity'. The changes also include the introduction of a new uuid property to the Account interface and the update of the ExternalAccountData type to exclude the uuid property. A new utility function, asAccountEntity, has been added to standardize account object transformations, and its usage has been integrated into tests and dispatcher functions. Furthermore, the service methods in the AccountService have been revised for consistent UUID generation and type update, especially in the recordAccountFollow and createExternalAccount methods. These adjustments are reflected in both the main service logic and integration tests.

Possibly related PRs

• Added notification for an account being followed #441: Involves changes to the import source for the Account type in the context of the AccountFollowedEvent class, similar to the modifications in this PR.
• Added notification for a post being reposted #447: Updates the import statement for the Account type in the notification module, reflecting a related codebase context.

Suggested reviewers

• allouis
• vershwal

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1271cc and d9b97ca.

📒 Files selected for processing (12)

• src/account/account-followed.event.ts (1 hunks)
• src/account/account.service.integration.test.ts (12 hunks)
• src/account/account.service.ts (3 hunks)
• src/account/types.ts (1 hunks)
• src/account/utils.ts (2 hunks)
• src/activitypub/fediverse-bridge.unit.test.ts (1 hunks)
• src/activitypub/followers.service.integration.test.ts (3 hunks)
• src/dispatchers.ts (3 hunks)
• src/feed/feed.service.integration.test.ts (11 hunks)
• src/notification/notification-event.service.unit.test.ts (1 hunks)
• src/notification/notification.service.integration.test.ts (1 hunks)
• src/notification/notification.service.ts (1 hunks)

🧰 Additional context used

🧬 Code Definitions (4)

src/activitypub/followers.service.integration.test.ts (1)

src/account/utils.ts (1)

• asAccountEntity (87-101)

src/feed/feed.service.integration.test.ts (2)

src/account/account.entity.ts (1)

• Account (25-135)

src/account/utils.ts (1)

• asAccountEntity (87-101)

src/account/account.service.integration.test.ts (1)

src/account/utils.ts (1)

• asAccountEntity (87-101)

src/dispatchers.ts (1)

src/account/utils.ts (1)

• asAccountEntity (87-101)

🔇 Additional comments (20)

src/activitypub/fediverse-bridge.unit.test.ts (1)

57-57: Added uuid property to the account object to align with updated Account type

This change correctly adds the newly required uuid property to the test fixture, which is now part of the Account type as mentioned in the PR description.

src/account/account-followed.event.ts (1)

1-1: Updated import path for Account to use entity-based architecture

The import path has been updated to reflect the architectural change described in the PR, moving from type-based to entity-based account handling.

src/activitypub/followers.service.integration.test.ts (3)

3-3: Added import for new asAccountEntity helper function

This import supports the changes to recordAccountFollow method that now requires Account entities instead of Account types.

---

57-57: Updated avatar URL to use a proper URL format

The avatar URL was updated from a placeholder text to a proper URL format, which is necessary for correct conversion to an Account entity since asAccountEntity attempts to construct a URL object from this value.

---

76-87: Updated recordAccountFollow calls to use asAccountEntity conversion

The modification properly implements the API change described in the PR objectives, where recordAccountFollow now accepts Account entity objects instead of Account types. The asAccountEntity helper function correctly transforms the objects to the expected format.

src/notification/notification-event.service.unit.test.ts (1)

6-6: Updated import path for Account to use entity-based architecture

This change is consistent with the architectural shift described in the PR, updating the import path to use the entity-based approach for account handling.

src/account/types.ts (2)

38-38: Addition of UUID property to Account interface

The addition of the uuid property to the Account interface aligns with the PR objectives and supports the changes to use the Account entity instead of the Account type in the AccountService.recordAccountFollow method.

---

44-47: Update to ExternalAccountData type exclusions

Updating the ExternalAccountData type to exclude the newly added uuid property is consistent with the changes made to the Account interface. This ensures that external account data doesn't require a UUID, which is appropriate if UUIDs are intended to be generated internally.

src/notification/notification.service.ts (1)

3-3: Import path update for Account type

Updating the import path from account/types to account/account.entity aligns with the PR objectives of using the Account entity throughout the codebase. This change is consistent with similar updates in other files.

src/notification/notification.service.integration.test.ts (1)

3-3: Import path update for Account type in test file

The import path change is consistent with the updates in other files and supports the transition to using the Account entity. The tests themselves remain unchanged, indicating that the type changes are backward compatible and don't affect the existing test cases.

src/account/utils.ts (2)

2-3: Updated import statements for Account entity and type

The import statements have been updated to bring in both the Account entity from ./account.entity and to rename the imported Account type from ./types to AccountType. This change supports the new helper function and clearly distinguishes between the entity and type.

---

82-101:

Details

✅ Verification successful

New helper function for converting AccountType to Account entity

The asAccountEntity function provides a standardized way to convert from AccountType to Account entity, supporting the PR objective of updating the AccountService.recordAccountFollow method to use the Account entity.

The function handles nullable URL fields appropriately by conditionally creating URL objects when the string values are present.

---

🏁 Script executed:

#!/bin/bash
# Check for all usages of this new helper function in the codebase

echo "Searching for usages of asAccountEntity function..."
rg -A 3 "asAccountEntity" --type ts

Length of output: 10550

---

Helper Function Verified Successfully

The updated asAccountEntity helper function correctly converts AccountType objects to Account entities while properly handling nullable URL fields. Our grep results confirm its usage across multiple parts of the codebase, specifically in:

• src/dispatchers.ts
• src/feed/feed.service.integration.test.ts
• src/account/account.service.integration.test.ts
• src/activitypub/followers.service.integration.test.ts

This implementation aligns with the PR objective to update AccountService.recordAccountFollow and supports standardized account conversion. No further issues were detected.

src/feed/feed.service.integration.test.ts (1)

3-3: Good improvement - replacing type casting with asAccountEntity

This change improves type safety by replacing unsafe type casting (as unknown as Account) with the asAccountEntity utility function. The utility ensures proper conversion from AccountType to Account entity with appropriate field transformations, reducing the risk of runtime errors.

Also applies to: 158-159, 202-204, 281-283, 297-299, 390-392, 476-478, 607-609, 617-619, 677-679, 733-735, 738-740

src/account/account.service.integration.test.ts (3)

31-31: Good refactoring - consistent use of asAccountEntity

The consistent replacement of direct AccountType usage with asAccountEntity conversion improves the test reliability and alignment with the service's updated method signatures. This ensures the tests properly validate the service behavior with correctly formed Account entities.

Also applies to: 218-221, 252-255, 278-281, 285-288, 320-323, 352-359, 453-464, 534-541, 568-579, 645-652, 675-678

---

91-91: Improved test data format for avatar_url

Changing from a simple string ('Test Site Icon') to a proper URL format makes the test data more realistic and consistent with production expectations, improving test validity.

---

330-331: Better assertion approach - comparing IDs instead of objects

Comparing IDs instead of entire object references is more reliable and less prone to false negatives from unrelated property differences between the objects.

src/dispatchers.ts (1)

26-29: Consistent integration of asAccountEntity in dispatchers

The dispatchers have been properly updated to use the asAccountEntity function when calling recordAccountFollow, ensuring type safety and compatibility with the updated service method signature. This change maintains the expected behavior while improving the robustness of the code.

Also applies to: 169-172, 242-245

src/account/account.service.ts (3)

154-155: Improved UUID consistency in createExternalAccount

Generating the UUID once at the beginning of the method and reusing it for both database insertion and the returned object ensures consistency between the stored record and the API response. This prevents potential discrepancies that could occur if different UUIDs were generated at different points in the method.

Also applies to: 158-158, 165-165

---

176-177: Enhanced type safety with Account entity parameters

Changing the parameter types from AccountType to Account enforces proper structure and validation for objects passed to the recordAccountFollow method. This aligns with the PR objective and improves the robustness of the service.

---

421-421: Complete data retrieval with uuid field in getByInternalId

Adding the uuid field to the return object in getByInternalId ensures consistent data retrieval across all account-related methods. This is necessary to support the Account entity model throughout the codebase.

✨ Finishing Touches

• 📝 Generate Docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai plan to trigger planning for file edits and PR creation.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[mike182uk]

AccountType doesn't have the site data, so we should in theory be safe to leave this as null as anywhere that is using AccountType currently won't be accessing the site. We could pass the site as another argument to the fn if needed?

[mike182uk]

Closing this in favour of modelling the follow / unfollow functionality within the account entity