Skip to content

Conversation

VadymHrechukha
Copy link
Collaborator

@VadymHrechukha VadymHrechukha commented Sep 6, 2025

Summary by CodeRabbit

  • Refactor
    • Expanded internal unit contract to standardize unit identifiers, labels, and equality checks across implementations.
    • Improves consistency for how fractional units are represented and compared internally.
    • No changes to user-facing behavior.

Copy link
Contributor

coderabbitai bot commented Sep 6, 2025

Walkthrough

The interface FractionUnitInterface was updated to add three new public method declarations: name(): string, label(): string, and equals(FractionUnitInterface $other): bool. No other files or runtime logic were changed.

Changes

Cohort / File(s) Summary
Domain Model Interface Update
src/product/Domain/Model/Unit/FractionUnitInterface.php
Added three methods to the interface: name(): string, label(): string, and equals(FractionUnitInterface $other): bool.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble on interfaces by moonlight,
Three new doors to name and show,
A mirror check to know my kind,
Implementers, please say hello.
Hop, hop — the contract grows tonight. 🐰✨


📜 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 b128ccb and 2758971.

📒 Files selected for processing (1)
  • src/product/Domain/Model/Unit/FractionUnitInterface.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/product/Domain/Model/Unit/FractionUnitInterface.php
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link
Contributor

@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.

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/product/Domain/Model/Unit/FractionUnitInterface.php (3)

5-10: DRY the contract: extend UnitInterface instead of redeclaring name().

FractionUnitInterface can inherit name() from UnitInterface to avoid duplication and keep the type hierarchy clear.

Apply:

-interface FractionUnitInterface
+interface FractionUnitInterface extends UnitInterface
 {
-    public function name(): string;
 
     public function label(): string;
 }

9-9: Scope check: should label() belong to all units?

If the Price creation UI renders a dropdown of “units” (not only fractional ones), consider moving label() to UnitInterface for consistency; otherwise the UI will need type checks.

Outside this file (UnitInterface):

 interface UnitInterface
 {
     public function name(): string;
+    public function label(): string;
 }

Then keep FractionUnitInterface focused on fraction-specific concerns.


7-9: Document semantics of name() vs label().

Add PHPDoc to define invariants (e.g., name = stable identifier, label = human-readable, localized) to prevent misuse across UI and persistence.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e28267a and b128ccb.

📒 Files selected for processing (1)
  • src/product/Domain/Model/Unit/FractionUnitInterface.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/product/Domain/Model/Unit/FractionUnitInterface.php (2)
src/product/Domain/Model/TariffTypeInterface.php (2)
  • name (7-7)
  • label (9-9)
src/product/Domain/Model/Unit/UnitInterface.php (1)
  • name (9-9)
🔇 Additional comments (1)
src/product/Domain/Model/Unit/FractionUnitInterface.php (1)

7-9: Heads-up: Adding methods to FractionUnitInterface is a BC break.

No classes in this repository implement FractionUnitInterface; however, any external/consumer implementations must now define name() and label() or risk runtime errors. Verify and update all downstream implementers.

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