Skip to content

feat: add aggregate identity handling to use Commanded.Aggregate.Identity protocol - #43

Merged
yordis merged 1 commit into
mainfrom
add-protocol-1
Jan 13, 2026
Merged

yordis merged 1 commit into
mainfrom
add-protocol-1

Conversation

@yordis

@yordis yordis commented Jan 13, 2026

Copy link
Copy Markdown
Member

No description provided.

@coderabbitai

coderabbitai Bot commented Jan 13, 2026 •

Copy link
Copy Markdown

Walkthrough

A new Commanded.Aggregate.Identity protocol converts aggregate identities to stream ID strings, replacing direct String.Chars usage. The protocol includes automatic fallback to String.Chars for backwards compatibility. Documentation, middleware, and tests are updated to support the new protocol.

Changes

Cohort / File(s) Summary
Documentation Updates
guides/explanations/commands.md, guides/explanations/fork-differences.md
Updated and expanded documentation describing the new Commanded.Aggregate.Identity protocol, its fallback behavior to String.Chars, usage patterns, and backwards compatibility considerations.
Protocol Definition
lib/commanded/aggregate/identity.ex
New public protocol Commanded.Aggregate.Identity with to_stream_id/1 callback and default implementation for Any type delegating to to_string/1. Uses @fallback_to_any for backwards compatibility.
Middleware Integration
lib/commanded/middleware/extract_aggregate_identity.ex
Updated ExtractAggregateIdentity to use Identity.to_stream_id/1 instead of String.Chars conversion, maintaining error handling behavior.
Test Coverage
test/commands/custom_identity_routing_test.exs
Updated existing tests to use new protocol; added backwards-compatibility tests with LegacyAccountNumber and LegacyIdentityRouter modules demonstrating fallback to String.Chars.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A protocol hops in, clean and bright,
Aggregate IDs now convert just right,
Old String.Chars won't be forgotten—
Fallback keeps it well begotten,
Backwards safe, the stream flows tight! 🌊

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether it relates to the changeset. Add a pull request description explaining the purpose, benefits, and migration path for adopting the new Commanded.Aggregate.Identity protocol.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: introducing a new Commanded.Aggregate.Identity protocol for aggregate identity handling, which is the primary purpose of this changeset.

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

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f44dcf1 and 0d45675.

📒 Files selected for processing (5)
  • guides/explanations/commands.md
  • guides/explanations/fork-differences.md
  • lib/commanded/aggregate/identity.ex
  • lib/commanded/middleware/extract_aggregate_identity.ex
  • test/commands/custom_identity_routing_test.exs
🧰 Additional context used
🧬 Code graph analysis (2)
lib/commanded/middleware/extract_aggregate_identity.ex (1)
lib/commanded/aggregate/identity.ex (1)
  • to_stream_id (44-46)
test/commands/custom_identity_routing_test.exs (2)
lib/commanded/aggregate/identity.ex (1)
  • to_stream_id (44-46)
lib/application.ex (1)
  • dispatch (381-385)
⏰ 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). (3)
  • GitHub Check: Quality Assurance (1.19.x, 27)
  • GitHub Check: Cursor Bugbot
  • GitHub Check: Cursor Bugbot
🔇 Additional comments (9)
lib/commanded/aggregate/identity.ex (2)

1-34: LGTM! Well-designed protocol with clear semantics.

The protocol definition is clean with proper @fallback_to_any true for backward compatibility. The documentation clearly explains the purpose and provides a good usage example.


36-47: LGTM! Correct fallback implementation.

The Any implementation properly delegates to to_string/1, ensuring that existing types implementing String.Chars will continue to work without modification.

lib/commanded/middleware/extract_aggregate_identity.ex (2)

9-9: LGTM!

Clean alias addition for the new protocol.


49-57: LGTM! Correct integration with the new protocol.

The middleware properly uses Identity.to_stream_id/1 and the rescue block correctly handles the case where both the Identity protocol and the String.Chars fallback are not implemented (the Protocol.UndefinedError will be raised by to_string/1 in the Any fallback).

guides/explanations/commands.md (2)

162-173: LGTM! Clear documentation update.

The documentation accurately describes the new protocol and provides a consistent example matching the protocol definition.


188-191: LGTM! Helpful backwards compatibility note.

The info callout clearly communicates the fallback behavior for existing implementations.

guides/explanations/fork-differences.md (1)

174-217: LGTM! Excellent rationale section.

The documentation clearly explains the need for a dedicated protocol by illustrating the separation of concerns between API response formatting (String.Chars) and event store stream IDs (Commanded.Aggregate.Identity). This is valuable context for users deciding which protocol to implement.

test/commands/custom_identity_routing_test.exs (2)

13-16: LGTM!

Clean implementation of the new Commanded.Aggregate.Identity protocol for the test struct.


65-97: LGTM! Comprehensive backwards compatibility test.

Good test coverage for the String.Chars fallback path. Using a different separator (- vs :) clearly demonstrates that the correct protocol implementation is being invoked.


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

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

@cursor

cursor Bot commented Jan 13, 2026 •

Copy link
Copy Markdown

PR Summary

Introduces a dedicated protocol for aggregate identity conversion and wires it into command dispatch.

  • Added Commanded.Aggregate.Identity protocol with @fallback_to_any true and default Any impl delegating to String.Chars
  • Updated ExtractAggregateIdentity middleware to call Identity.to_stream_id/1 instead of to_string/1
  • Documentation updated in guides/explanations/commands.md and fork-differences.md with examples and migration notes
  • Tests added/updated to cover protocol usage, invalid identities, and String.Chars backward compatibility

Written by Cursor Bugbot for commit 0d45675. This will update automatically on new commits. Configure here.

@yordis
yordis force-pushed the add-protocol-1 branch 2 times, most recently from 2a11ae0 to e4d569e Compare January 13, 2026 07:50
…ntity` protocol

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis marked this pull request as ready for review January 13, 2026 07:51
@yordis
yordis merged commit a0eb2cb into main Jan 13, 2026
5 checks passed
@yordis
yordis deleted the add-protocol-1 branch January 13, 2026 08:04
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.

1 participant