[ING-68] feat/multi-entity): adapt dunning campaign flow for multi entity billing#5515
[ING-68] feat/multi-entity): adapt dunning campaign flow for multi entity billing#5515lovrocolic wants to merge 3 commits into
Conversation
lovrocolic
changed the title
ancorcruz
left a comment
ancorcruz
left a comment
There was a problem hiding this comment.
I understand reading the code that we run the dunning for the customer across all billing entities (with a single counter per currency) but we split the amounts billing by BE. is this intended?
If that is the case, looks all good except self billed invoices must be excluded from dunning (pre existing bug I'm afraid).
| end | ||
|
|
||
| def overdue_invoices | ||
| def overdue_invoices_in_currency |
There was a problem hiding this comment.
If I'm not wrong this also should exclude the self-billed invoices from the overdue ones.
|
|
||
| def dunning_campaign_threshold_reached? | ||
| overdue_invoices.sum(:total_amount_cents) >= dunning_campaign_threshold.amount_cents | ||
| overdue_invoices_in_currency.sum(:total_amount_cents) >= dunning_campaign_threshold.amount_cents |
There was a problem hiding this comment.
I'm a bit confuse here...
We compute all the overdue invoices of the customer across billing entities with a given currency to check if the threshold has been reached. However, we run the dunning campaign scoped to each billing entity + currency.... then a customer with overdue invoices in EUR, for example 10k in billing_entity_1 and 5k in billing_entity_2, given a dunning threshold of 12k EUR, it will run two dunning processes for this customer one for the 10k overdue in BE_1 and another one with 5k in BE_2. Is this right?
There was a problem hiding this comment.
I would say yes. BE affects only the billing side, not the tresholds
|
|
||
| custom_campaign = customer.applied_dunning_campaign | ||
| default_campaign = billing_entity.applied_dunning_campaign | ||
| default_campaign = customer.billing_entity.applied_dunning_campaign |
There was a problem hiding this comment.
why consider the customer default campaign the one defined as one of its billing entities' default campaing instead of the given billing entity default campaign? Does this make sense?
There was a problem hiding this comment.
if this is intentional it would be great to add a quick comment as insight for the next reader...
There was a problem hiding this comment.
A dunning campaign is a customer-level concept — it's resolved as customer.applied_dunning_campaign ||
customer.billing_entity.applied_dunning_campaign. The per-BE iteration only scopes invoices, not which campaign applies. Note BulkProcessService#applicable_dunning_campaign (line 71) uses the exact same resolution, so they're consistent.
| entity_ids = customer.invoices.non_self_billed.payment_overdue | ||
| .where(currency: currency).distinct.pluck(:billing_entity_id) | ||
| customer.organization.billing_entities.where(id: entity_ids) |
There was a problem hiding this comment.
should we filter invoices by ready_for_payment_processing: true as well? We do in ProcessAttemptService#overdue_invoices
There was a problem hiding this comment.
Applied it, thanks!
| def dunning_campaign_threshold_reached? | ||
| overdue_invoices.sum(:total_amount_cents) >= dunning_campaign_threshold.amount_cents | ||
| overdue_invoices_in_currency.sum(:total_amount_cents) >= dunning_campaign_threshold.amount_cents | ||
| end |
There was a problem hiding this comment.
we run this query for each BE of the customer, it is always the same query as it is across all BE, we can run this one level above and pass the result to the service saving a DB query for each BE.
There was a problem hiding this comment.
I improved it a bit with memoization, but IMO not worth passing result around since in most cases, BE is the same and in rare cases we would have max. few billing entities. Let me know if you still think we should improve it more
lovrocolic
force-pushed
the
feat-dunning-multi-entity
branch
from
8b1d466 to
46b52f8
Compare
lovrocolic
force-pushed
the
feat-dunning-multi-entity
branch
from
abbe64b to
5a5aaff
Compare
2 participants
Context
Currently, one customer can be assigned to only one billing entity. With
multi-billing-entitiesfeature, it will be possible to assign any billing entity to customer's billing object, like subscription or wallet.Description
This PR adapts dunning campaign flow for multi entity billing.