Skip to content

Post updated event serialization#1598

Merged
mike182uk merged 3 commits intomainfrom
cursor/BER-3164-post-updated-event-serialization-d792
Feb 18, 2026
Merged

Post updated event serialization#1598
mike182uk merged 3 commits intomainfrom
cursor/BER-3164-post-updated-event-serialization-d792

Conversation

@mike182uk
Copy link
Member

Refactor PostUpdatedEvent to implement SerializableEvent to comply with ADR-0011.


Linear Issue: BER-3164

Open in Cursor Open in Web

- Update PostUpdatedEvent to implement SerializableEvent interface
- Change event to store postId instead of full Post entity
- Add toJSON() and fromJSON() methods for serialization
- Update KnexPostRepository to emit event with postId
- Update FediverseBridge to lookup Post from repository
- Register PostUpdatedEvent with EventSerializer
- Add unit tests for serialization/deserialization
- Update existing tests to use new API

Co-authored-by: Michael Barrett <mike182uk@users.noreply.github.com>
@cursor
Copy link

cursor bot commented Feb 18, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@coderabbitai
Copy link

coderabbitai bot commented Feb 18, 2026

Walkthrough

This pull request refactors PostUpdatedEvent to support serialization by replacing the full Post object parameter with a numeric postId. The event class now implements SerializableEvent with toJSON() and fromJSON() methods. Consumers like FediverseBridge are updated to accept a KnexPostRepository dependency and fetch posts by ID from the repository, with guards added for missing or non-internal posts. The event is registered with the EventSerializer in app.ts, and comprehensive test coverage is added for the new serialization logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Serializable post reposted event #1541: Applies the same refactoring pattern—converting post events to use postId for serializability while updating consumers to inject KnexPostRepository, fetch posts by ID, and guard against missing posts.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: refactoring PostUpdatedEvent to implement SerializableEvent with serialization support.
Description check ✅ Passed The description clearly explains the refactoring of PostUpdatedEvent to implement SerializableEvent per ADR-0011, with relevant context linking to the Linear issue.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cursor/BER-3164-post-updated-event-serialization-d792

Comment @coderabbitai help to get the list of available commands and usage tips.

return this.postId;
}

static getName(): string {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cursoragent lets add the non-static version like in the other events whilst we are here

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking a look!

Open in Cursor Open in Web


import { PostUpdatedEvent } from '@/post/post-updated.event';

describe('PostUpdatedEvent', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cursoragent These tests should be consistent with the other events

Copy link

@cursor cursor bot Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restructured the PostUpdatedEvent tests to be consistent with other event tests:

  • Added nested describe blocks for each method (getName, getPostId, toJSON, fromJSON, round-trip serialization)
  • Added test case for postId being null
  • Added round-trip serialization test
  • Using .toBe() instead of .toEqual() for primitive comparisons

Open in Cursor Open in Web

cursoragent and others added 2 commits February 18, 2026 10:32
Adds instance method that delegates to static method, consistent
with other event classes like PostLikedEvent.

Co-authored-by: Michael Barrett <mike182uk@users.noreply.github.com>
Use nested describe blocks for each method, consistent with
PostLikedEvent and other event test files.

Co-authored-by: Michael Barrett <mike182uk@users.noreply.github.com>
@mike182uk mike182uk marked this pull request as ready for review February 18, 2026 11:09
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/activitypub/fediverse-bridge.ts (1)

22-22: Repository injection pattern is consistent with codebase conventions.

The guideline "repositories should not be used directly" is noted, but this pattern is established across the codebase: NotificationEventService, PostInteractionCountsService, FeedUpdateService, GhostPostService, PostService, controllers, and other event handlers all inject and use KnexPostRepository directly. There is no PostService layer providing an alternative.

The implementation correctly guards against null and non-internal authors. The postRepository parameter name matches the DI registration at src/configuration/registrations.ts:303, satisfying Awilix CLASSIC mode requirements.

If the team intends to enforce repository abstraction, it would require broader refactoring beyond this change.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/activitypub/fediverse-bridge.ts` at line 22, The current use of
KnexPostRepository in src/activitypub/fediverse-bridge.ts is consistent with the
codebase pattern, so do not refactor to a service now; keep the constructor
parameter named postRepository and continue injecting KnexPostRepository
directly, ensure the existing null checks and non-internal author guards around
usages remain intact, and make no change unless you plan a broader
repository-abstraction refactor across
NotificationEventService/PostInteractionCountsService/FeedUpdateService/GhostPostService/PostService
and related DI registrations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/activitypub/fediverse-bridge.ts`:
- Line 22: The current use of KnexPostRepository in
src/activitypub/fediverse-bridge.ts is consistent with the codebase pattern, so
do not refactor to a service now; keep the constructor parameter named
postRepository and continue injecting KnexPostRepository directly, ensure the
existing null checks and non-internal author guards around usages remain intact,
and make no change unless you plan a broader repository-abstraction refactor
across
NotificationEventService/PostInteractionCountsService/FeedUpdateService/GhostPostService/PostService
and related DI registrations.

@mike182uk mike182uk merged commit ed6d7b1 into main Feb 18, 2026
12 checks passed
@mike182uk mike182uk deleted the cursor/BER-3164-post-updated-event-serialization-d792 branch February 18, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments