Skip to content

refactor(core): promote Money value object to BuildingBlocks and adopt in Billing top-ups #1316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marcelo-maciel wants to merge 3 commits into
base: main
Choose a base branch
from
Open

refactor(core): promote Money value object to BuildingBlocks and adopt in Billing top-ups #1316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94127a5
refactor(core): promote Money value object to BuildingBlocks and enri…
marcelo-maciel Jun 26, 2026
7864674
fix(catalog): enforce non-negative price on aggregate and sync model …
marcelo-maciel Jun 26, 2026
b5d2d6b
refactor(core): trim speculative Money arithmetic pending a real caller
marcelo-maciel Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions ...s/Catalog/Modules.Catalog/Domain/Money.cs → src/BuildingBlocks/Core/Domain/Money.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace FSH.Modules.Catalog.Domain;
namespace FSH.Framework.Core.Domain;

public sealed record Money
Comment on lines +1 to +3

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Get approval before changing BuildingBlocks

This adds a shared domain primitive under src/BuildingBlocks, but AGENTS.md golden rule 4 says “Do NOT modify src/BuildingBlocks without explicit approval” and .agents/rules/buildingblocks-protection.md says to “Confirm explicit approval.” I don't see that approval in this reviewed change/context; because Core is consumed by every module, this should not land until the approval is recorded or the type stays module-local.

Useful? React with 👍 / 👎.

@marcelo-maciel marcelo-maciel Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged. This BuildingBlocks change is deliberate, not incidental: it promotes Money to a shared domain primitive, stays dependency-free, and passes the BuildingBlocks independence/immutability/layering arch tests. Whether the golden-rule-4 approval bar is satisfied is a maintainer call, so flagging it here for explicit sign-off rather than self-approving.

{
Expand All @@ -8,10 +8,6 @@ public sealed record Money
public Money(decimal amount, string currency)
{
ArgumentException.ThrowIfNullOrWhiteSpace(currency);
if (amount < 0)
{
throw new ArgumentOutOfRangeException(nameof(amount), "Amount cannot be negative.");
Comment on lines -11 to -13

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve Catalog price non-negativity

Removing this guard makes the shared Money accept negative amounts, but Product.Create/ChangePrice still accept a Money directly and do not check Price.Amount; any catalog import, seed, test helper, or internal caller that bypasses the two command validators can now persist a negative product price. Move the non-negative price invariant into the Catalog aggregate before deleting the Catalog-specific Money guard.

Useful? React with 👍 / 👎.

@marcelo-maciel marcelo-maciel Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Resolved in 7864674. The promoted Money now allows signed amounts (ledger semantics), so the non-negative price invariant moved onto the Catalog aggregate: Product.Create and ChangePrice throw ArgumentOutOfRangeException when price.Amount < 0, so any caller that bypasses the command validators (seed, import, test helpers) can no longer persist a negative price. Added unit tests for both paths.

}
Amount = amount;
Currency = currency.ToUpperInvariant();
}
Expand Down
Loading
Loading