Billing period on recurring orders for prepaid subscriptions should be for the next period, not the current one

Created on 8 December 2020, about 4 years ago
Updated 24 July 2024, 5 months ago


Problem/Motivation

Printed and/or emailed receipts on recurring orders on prepaid orders have the payment period of the previous or first payment period conected to them. This makes the receipt a bit confusing fo the customer.

Steps to reproduce

  • Create and attach a prepaid billing schedule to a product.
  • Purchase the poduct
  • Add the payment period of the order to the template of the receipt.
  • View the receipt

Proposed resolution

Let's make prepaid and postpaid draft orders behave dfferrently.

postpaid

The postpaid orders can continue working as they do now, with the cron job picking up the drafts where the end date has passed to close and generate new drafts for the next period.

prepaid

In the case of a prepaid billing schedule we can instead close and generate a draft for the next billing period when the start of the orders billing period has passed. This makes the order be closed and any emails be sent at the start of tthe period when the money is collected. Another change needed for this to happen is that the first recurring order should be treated as the trial order and it's price should be set to 0.

🐛 Bug report
Status

Needs work

Version

1.0

Component

Code

Created by

🇸🇪Sweden auth

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States rszrama

    +1'ing this issue, as we've just encountered it. The scenario is exactly the same: we're using a prepaid billing schedule and are seeing draft recurring orders that appear to be for the previous billing period, the one that was already prepaid. That said, I'm not sure I fully understand the proposed solution or latest patch.

    The original post states that "the first recurring order should be treated as the trial order and its price should be set to 0." This is either imprecise or just not correct for all scenarios. In our case, we still charge the customer the current month's prorated fee upon checkout, so it would not be appropriate for either the initial order or the first recurring order to be set to $0.

    Additionally, the patch separates the single Cron::enqueueOrders() function into separate functions for prepaid vs. postpaid billing schedules. I don't see why this is necessary. Can we not just use a single query that uses condition groups to find those prepaid orders whose start date has passed and postpaid orders whose end date has passed? This will reduce code duplication, given we have to perform the same checks on the loaded orders for cancellation regardless.

    At the end of the day, what we need to see are:

    1. Subscriptions with prepaid billing schedules: my draft recurring order should be for the billing period that ends after my current one. Thus, if I subscribe today with a fixed monthly billing schedule that renews on the 1st of the month, my draft recurring order should show that it is for August 1 - September 1.
    2. Subscriptions with postpaid billing schedules: my draft recurring order should be for the current billing period. Thus, if I subscribe today with a fixed monthly billing schedule that renews on the 1st of the month, my draft recurring order should show that it is for July 1 - August 1.

    There are some nuances we need to explore with respect to trial periods, but the gist of the fix for the initial order placement is that prepaid billing schedules should result in the next billing period being generated after the first is. (In the patch, I'm not sure I follow the logic of the change for this in RecurringOrderManager::startRecurring().)

    I'm also not sure what we should do for the billing period with to fixed prepaid schedules and free trials, as the adjusted start date code is a little funky ... but maybe the answer is that free trials in these cases just aren't super appropriate? If a subscription always renews on the 1st of the month, what's the use case for a free trial vs. a 100% off coupon for the first month or something? 🧐

  • 🇮🇱Israel jsacksick

    @rszrama: There is currently nothing we could query on to simply adapt the query. There is a billing period field on the order, and that just holds the start and the end date, nothing else.

    The subscription references the billing schedule entity, and the billing type (prepaid vs postpaid) is stored on that config entity, so unfortunately, nothing we can query on.

    I believe this is why the current logic works as is... We always close recurring orders at the end of the billing period... I wonder if we shouldn't find a way to simply "cheat" and just display a different billing period. This is far from a perfect solution, but it'd solve the display problem I suppose...

  • 🇮🇱Israel jsacksick

    So actually revising what I said in #13.

    What we could do is a following, loop on all billing schedules, and separate prepaid from postpaid billing schedules. We essentially need an array containing prepaid billing schedule IDS and another one containing postpaid billing schedule IDS.

    Then, assuming we fix the initial billing period, we could adjust the query to do the following:

    • Create a first "AND" condition group to have a condition on the both the end date and a condition to get orders referencing postpaid billing schedules.
    • Create a second AND condition group to have a condition on the start date AND a reference to a "prepaid" billing schedule.
    • Then an OR condition group to get either recurring orders with a billing period end date in the past referencing postpaid billing schedules OR recurring orders with a billing period start date in the past AND referencing a prepaid billing schedule.

    But what about existing orders? This would be a breaking change as we wouldn't bill existing orders properly unfortunately.

  • 🇮🇱Israel jsacksick

    I think we need an update hook updating all opened recurring draft orders referencing prepaid billing schedules, to ensure their billing periods is adjusted. I'm just hoping that doing this isn't going to cause a billing cycle to be skipped or something.

  • 🇳🇿New Zealand john pitcairn

    This makes me very nervous, and not just about the possibility of skipping a cycle. All our orders are prepaid.

    If we only update draft order billing periods, pushing them out by 1 cycle, then we will have a "missing" billing period in the subscription. At some point admin will spot that and freak out. The customer may think they've "skipped" a billing period and request support. And it may screw up any reporting or other operations that use the billing period.

    I think we'd need to update all prepaid orders, no? Ouch.

  • 🇳🇿New Zealand john pitcairn

    Can't help but thinking we have a naming problem here, ie we call it the "billing period", but it's always used as if it were post-paid – so it is not the same as the subscription period for pre-paid subscriptions.

  • 🇮🇱Israel jsacksick

    We use billing period because that is how it's called in the code everywhere...
    We may have to update all prepaid orders yes, I understand this is making you nervous, however this has to be fixed as the current situation is really confusing and is far from ideal...

  • 🇳🇿New Zealand john pitcairn

    Sure. Was just wondering about keeping it as is and adding a calculated subscription_period field, using that for display, email etc. So the current assumptions and data would not change.

  • 🇺🇸United States rszrama

    I do think there's some small risk here that subscriptions payments could be impacted, but I'm struggling to conceive the hypothetical scenario.

    For example, let's say the "close order" queue was populated and then the module gets updated before all queued items can be processed. The update then updates all those draft recurring orders to use the new billing period... well, there's no double checking in the queue processor to see if it's really the right time to close the order. They'd close like normal.

    Additionally, let's say the order got updated right around the time it was supposed to be enqueued for closing. (Or let's say the queue processor actually did double check to make sure it's the right time for an order to close.) Well, the coordinated change to the logic regarding when a prepaid subscription's draft recurring order should close (switching from the start to the end of the billing period) will be in place. No harm, no foul.

    I think the concerns (closing previously enqueued orders vs. updating draft orders) are separate enough that there's little risk. Maybe we just need to make sure we leave non-draft recurring orders for prepaid subscriptions alone ... that will create some apparent billing duplication, but that's easily explained if it becomes a customer service issue.

    Can anyone else think of a scenario where a payment may actually be double charged or skipped? Maybe we just need to ensure the enqueueing of an order for closure can't happen twice? (But even then, if closure was attempted a second time, the default implementation in RecurringOrderManager will skip the order if it is completed or paid.)

  • 🇳🇿New Zealand john pitcairn

    OK fair enough. Update drafts only.

    I guess we can run our own update to fix past completed/failed orders after all orders in needs_payment at initial update time have either completed or failed. So we have consistent data.

  • 🇳🇴Norway zaporylie

    Can anyone else think of a scenario where a payment may actually be double charged or skipped? Maybe we just need to ensure the enqueueing of an order for closure can't happen twice? (But even then, if closure was attempted a second time, the default implementation in RecurringOrderManager will skip the order if it is completed or paid.)

    I think there's a risk in the billing period being updated after the order renewal job is triggered but before the order closing job commences. It can happen as advancedqueue doesn't guarantee in which order jobs are executed. In such a scenario, the new draft will cover the same dates as the order destined to be closed, with a modified billing period. This could be avoided if commerce_recurring_order_renew checks if the order is closed before attempting a renewal.

    I believe we'd need to have a change record if we do update to billing periods.

    Finally, while limiting the scope for updated orders to drafts only, perhaps making it possible for site owners to decide whether they want all the orders or only draft orders (default) would be a good strategy here.

Production build 0.71.5 2024