HQD-159: rename InstallmentWasStarted → InstallmentWasCharged, fire e…

[bladeroot]

…very month

Previously the event fired only on the first billing month (exact match with formula since). If billing missed that month, the Sale was never created.

Now the event fires for every month within the installment period. HandleInstallmentWasCharged (in hiapi-legacy) is already idempotent: it checks whether a Sale with the correct till-date already exists and skips creation if so.

Renamed to InstallmentWasCharged to reflect the actual semantics — one charge processed, not necessarily the start of the installment.

Summary by CodeRabbit

• Bug Fixes

  • Improved installment charge tracking to ensure consistent recording across all applicable installment periods.

• Documentation

  • Updated documentation to reflect changes in installment processing event recording.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR renames the domain event InstallmentWasStarted to InstallmentWasCharged and refactors the installment modifier to improve first-month determination logic and ensure consistent event recording across all installment charges.

Changes

Installment Event Rename and Modifier Refactoring

Layer / File(s)	Summary
Event class rename and cleanup src/charge/modifiers/event/InstallmentWasCharged.php	Domain event class renamed from InstallmentWasStarted to InstallmentWasCharged with the same base classes and interface implementation; PHPDoc comment for jsonSerialize() removed while the method implementation remains unchanged.
Installment modifier logic and imports src/charge/modifiers/Installment.php	Event imports updated to use InstallmentWasCharged; the isFirstMonthInInstallmentPassed helper is replaced with isFirstMonthAfterInstallmentPassed using till and term-based offsets; charge creation logic simplified to always route through createInstallmentStartingCharge, ensuring the charged event is recorded for all in-period installment charges.
Unit and Behat test expectations tests/unit/charge/modifiers/InstallmentTest.php, tests/behat/Installment.feature, tests/behat/Combination.feature	Unit test and Behat scenarios updated to expect InstallmentWasCharged instead of InstallmentWasStarted across installment examples (08.2018, 01.2024, zero-price) and combination interaction scenarios; all event assertions and example rows reflect the renamed event and updated charge recording behavior.
Documentation updates docs/overview.md, docs/formula-and-modifiers.md	Domain event references in the API directory map and installment constraints documentation updated from InstallmentWasStarted to InstallmentWasCharged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit hops through time with glee,
Renaming events from "started" to "charged" decree!
Helper methods dance with till in embrace,
Installing precision all over the place. 🐰⚡

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main changes: renaming InstallmentWasStarted to InstallmentWasCharged and the behavior change to fire the event every month. It directly summarizes the primary objective of the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/charge/modifiers/event/InstallmentWasCharged.php (1)

41-46: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Serialize the billing month as part of the event payload.

InstallmentWasCharged can now be emitted multiple times for the same price_id / part_id, but jsonSerialize() still makes every month look identical. Any consumer that persists, deduplicates, or replays the JSON form loses which installment month was actually charged. Include the charge month/time, or another stable period identifier, in the serialized payload.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/charge/modifiers/event/InstallmentWasCharged.php` around lines 41 - 46,
The jsonSerialize() output for InstallmentWasCharged omits any stable period
identifier so multiple events with the same price_id/part_id are
indistinguishable; update InstallmentWasCharged::jsonSerialize() to include a
billing-period field (e.g. "charged_at" as an ISO‑8601 timestamp or
"billing_month" as YYYY‑MM) derived from the Charge object (use the appropriate
accessor on $this->charge such as
getChargedAt()/getChargeDate()/getBillingPeriod()/getPeriodStart() and format to
a stable string) so consumers can tell which installment month was charged.
Ensure the new key is present alongside 'price_id' and 'part_id' and uses a
consistent, parseable format.

src/charge/modifiers/Installment.php (1)

105-119: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize the end-date check to billing-month granularity.

$time is always normalized to first day of this month midnight, but this helper compares it to raw till / term->addTo($since) timestamps and only treats an exact same-day diff as a match. For installments that start or end mid-month, the first post-installment billing month will never satisfy this check, so InstallmentWasFinished is skipped. Compare against the same normalized month boundary used in modifyCharge().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/charge/modifiers/Installment.php` around lines 105 - 119, In
isFirstMonthAfterInstallmentPassed, normalize the compared boundaries to the
same "first day of month midnight" granularity as $time instead of relying on
day-exact diff checks: when using $this->getTill()->getValue() or
$this->getTerm()->addTo($since->getValue()), convert those DateTimeImmutable
values to the month boundary (e.g. first day of that month at 00:00) before
doing the <= or equality comparison, and replace the diff('%a') === '0' checks
with direct comparisons against the normalized boundary; use the same
normalization helper used in modifyCharge() or implement a small local
normalization call and reference getSince, getTill, getTerm and term->addTo(...)
to locate the places to change.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/charge/modifiers/event/InstallmentWasCharged.php`:
- Around line 41-46: The jsonSerialize() output for InstallmentWasCharged omits
any stable period identifier so multiple events with the same price_id/part_id
are indistinguishable; update InstallmentWasCharged::jsonSerialize() to include
a billing-period field (e.g. "charged_at" as an ISO‑8601 timestamp or
"billing_month" as YYYY‑MM) derived from the Charge object (use the appropriate
accessor on $this->charge such as
getChargedAt()/getChargeDate()/getBillingPeriod()/getPeriodStart() and format to
a stable string) so consumers can tell which installment month was charged.
Ensure the new key is present alongside 'price_id' and 'part_id' and uses a
consistent, parseable format.

In `@src/charge/modifiers/Installment.php`:
- Around line 105-119: In isFirstMonthAfterInstallmentPassed, normalize the
compared boundaries to the same "first day of month midnight" granularity as
$time instead of relying on day-exact diff checks: when using
$this->getTill()->getValue() or $this->getTerm()->addTo($since->getValue()),
convert those DateTimeImmutable values to the month boundary (e.g. first day of
that month at 00:00) before doing the <= or equality comparison, and replace the
diff('%a') === '0' checks with direct comparisons against the normalized
boundary; use the same normalization helper used in modifyCharge() or implement
a small local normalization call and reference getSince, getTill, getTerm and
term->addTo(...) to locate the places to change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 985cc3a9-a6dd-4988-a026-7733041e3f7d

📥 Commits

Reviewing files that changed from the base of the PR and between 7301ba7 and 62ee9fd.

📒 Files selected for processing (7)

• docs/formula-and-modifiers.md
• docs/overview.md
• src/charge/modifiers/Installment.php
• src/charge/modifiers/event/InstallmentWasCharged.php
• tests/behat/Combination.feature
• tests/behat/Installment.feature
• tests/unit/charge/modifiers/InstallmentTest.php