Skip to content

Post created event serialization#1599

Open
mike182uk wants to merge 7 commits intomainfrom
cursor/BER-3174-post-created-event-serialization-d22a
Open

Post created event serialization#1599
mike182uk wants to merge 7 commits intomainfrom
cursor/BER-3174-post-created-event-serialization-d22a

Conversation

@mike182uk
Copy link
Member

Refactor PostCreatedEvent to implement SerializableEvent to comply with ADR-0011 for serializable domain events.


Linear Issue: BER-3174

Open in Cursor Open in Web

…(ADR-0011)

- Updated PostCreatedEvent to only contain postId instead of full Post entity
- Implemented toJSON() and fromJSON() methods for serialization
- Updated event publisher in post.repository.knex.ts to pass postId
- Updated FediverseBridge, FeedUpdateService, and NotificationEventService consumers to look up post from repository
- Registered PostCreatedEvent with EventSerializer in app.ts
- Added unit tests for serialization/deserialization
- Updated existing tests to use the new PostCreatedEvent signature

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

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

coderabbitai bot commented Feb 18, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cursor/BER-3174-post-created-event-serialization-d22a

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


import { PostCreatedEvent } from '@/post/post-created.event';

describe('PostCreatedEvent', () => {
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 should be consistent with the other events

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

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 also add the non static version of this whilst we are here

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.

This has already been addressed in my last commit. The PostCreatedEvent now has both the static and instance getName() methods:

getName(): string {
    return PostCreatedEvent.getName();
}

static getName(): string {
    return 'post.created';
}

The tests have also been updated to verify both versions work correctly. The comment you're seeing was on the older diff before I made the changes.

Open in Cursor Open in Web

- Add instance getName() method to PostCreatedEvent
- Restructure tests to match the pattern used by other events
- Add tests for getName (static and instance), getPostId, toJSON, fromJSON, and round-trip serialization
- Add null validation test for fromJSON

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

const post = Object.create(Post);
post.id = 'post-123';
post.id = 111;
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 this should probably be 456 if we are following the existing pattern. String references below containing post-123 should also be updated (e.g new URL('https://example.com/post/456');

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

author.isInternal = false;

const post = Object.create(Post);
post.id = 222;
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 this should probably be 456 if we are following the existing pattern

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


vi.mocked(postRepository.getById).mockResolvedValue(null);

const event = new PostCreatedEvent(999);
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 this should probably be 123 if we are following the existing pattern

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

cursoragent and others added 2 commits February 19, 2026 09:17
- Changed post.id from 111 to 456 in Article test to follow existing pattern
- Updated URL references from post-123 to 456

Co-authored-by: Michael Barrett <mike182uk@users.noreply.github.com>
- Updated external accounts test to use post.id = 456
- Updated post not found test to use 456

Co-authored-by: Michael Barrett <mike182uk@users.noreply.github.com>
audience: Audience.FollowersOnly,
});
// biome-ignore lint/suspicious/noExplicitAny: Test helper to set post id
(post as any).id = 124;
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 this should probably be 123

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

audience: Audience.Direct,
});
// biome-ignore lint/suspicious/noExplicitAny: Test helper to set post id
(post as any).id = 125;
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 this should probably be 123

Copy link

@cursor cursor bot Feb 19, 2026

Choose a reason for hiding this comment

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

This has already been addressed in my last commit. All the post IDs in the feed-update.service.unit.test.ts file are now set to 123:

  • Line 101: (post as any).id = 123;
  • Line 125: (post as any).id = 123;
  • Line 144: new PostCreatedEvent(123)
  • Line 148: expect(postRepository.getById).toHaveBeenCalledWith(123);

The comment you're seeing was on the older diff before I made the changes.

Open in Cursor Open in Web

cursoragent and others added 2 commits February 19, 2026 09:19
Follow existing pattern where 123 is used for IDs in not-found scenarios

Co-authored-by: Michael Barrett <mike182uk@users.noreply.github.com>
All PostCreatedEvent tests now use 123 for the post ID

Co-authored-by: Michael Barrett <mike182uk@users.noreply.github.com>
@mike182uk mike182uk marked this pull request as ready for review February 19, 2026 09:23
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