AHN-2700: find charge by type, object and quantity#108
Conversation
WalkthroughIntroduces quantity-aware charge lookup in BillingContext. Updates findCharge to accept an optional quantity, adds findCharges to collect matching charges by type/target, and modifies chargeWithTarget to pass quantity. Logic iterates collected charges to find one matching the requested quantity or returns the last iterated charge when no exact quantity match exists. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test as Behat Test
participant BC as BillingContext
participant Coll as Charges Collection
Test->>BC: chargeWithTarget(type, target, quantity)
activate BC
BC->>BC: findCharge(type, target, quantity)
activate BC
BC->>BC: findCharges(type, target)
note right of BC: Filter charges by type and target
BC->>Coll: iterate charges
Coll-->>BC: matching charges[]
deactivate BC
alt charges found
loop for each charge in charges[]
BC->>BC: compare usage quantity
end
note right of BC: Return exact quantity match\nelse fallback to last iterated
BC-->>Test: ChargeInterface|null
else none
BC-->>Test: null
end
deactivate BC
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate 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. Comment |
bladeroot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/behat/bootstrap/BillingContext.php (1)
405-419: Regression: null target can never match; also initialize accumulator.findCharges() filters by target unconditionally; when $target is null (e.g., charge() step), no charge can match. Also, pre-initialize $charges for clarity and return []-emptiness consistently.
- public function findCharges($type, $target): ?array - { - foreach ($this->charges as $charge) { - if ($charge->getType()->getName() !== $type) { - continue; - } - if ($charge->getTarget()->getFullName() !== $target) { - continue; - } - - $charges[] = $charge; - } - - return $charges ?? null; - } + public function findCharges($type, $target): ?array + { + $charges = []; + foreach ($this->charges as $charge) { + if ($charge->getType()->getName() !== $type) { + continue; + } + if ($target !== null && $charge->getTarget()->getFullName() !== $target) { + continue; + } + $charges[] = $charge; + } + + return $charges ?: null; + }
🧹 Nitpick comments (1)
tests/behat/bootstrap/BillingContext.php (1)
389-403: Avoid ambiguous fallback; return null when quantity doesn’t match; compare numerically.Current code returns the last iterated charge if no exact quantity match, which can produce misleading assertions. Prefer returning null when a quantity was provided but not found, and compare as floats to avoid string-format mismatches.
- public function findCharge($type, $target, $quantity = null): ?ChargeInterface - { - $charges = $this->findCharges($type, $target); - if ($charges === null) { - return null; - } - - foreach ($charges as $charge) { - if ((string) $quantity === (string) $charge->getUsage()->getQuantity()) { - return $charge; - } - } - - return $charge; - } + public function findCharge($type, $target, $quantity = null): ?ChargeInterface + { + $charges = $this->findCharges($type, $target); + if (empty($charges)) { + return null; + } + + if ($quantity !== null) { + foreach ($charges as $candidate) { + if ((float) $quantity === (float) $candidate->getUsage()->getQuantity()) { + return $candidate; + } + } + return null; + } + + return end($charges) ?: null; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/behat/bootstrap/BillingContext.php(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/behat/bootstrap/BillingContext.php (4)
tests/behat/bootstrap/FactoryBasedBuilder.php (1)
findCharges(226-234)src/charge/Charge.php (1)
getUsage(132-135)src/charge/ChargeInterface.php (1)
getUsage(47-47)src/charge/derivative/ChargeDerivativeQueryInterface.php (1)
getUsage(60-60)
🔇 Additional comments (1)
tests/behat/bootstrap/BillingContext.php (1)
379-387: Quantity propagation looks correct; mind BC when target is null.Passing $quantity into findCharge is good. However, the new findCharges() requires a non-null target to match; calls from charge() pass null, which will now return no matches. See fix below on findCharges().
2 participants
Summary by CodeRabbit