laravel / cashier-mollie

MIT License
376 stars 63 forks source link

Interval calculation #41

Open robindirksen1 opened 4 years ago

robindirksen1 commented 4 years ago

So for this example I have a plan configured with an interval of 1 month and made a subscription.

What I expect:

When I start a subscription for a user on 2019-08-30, I want to bill them on the 30 off every month.

Here I made a example (the billing dates are marked with a green box):

calendar-view-expected

What the packages does:

The package billing-day is changing when the month doesn't have (in this case) 30 or more days in a month.

Here I made a example of what the package is doing (startdate = 2018-08-30) (the billing dates are marked with a green box):

calendar-view-actual

Proposal

I think we should make this as an optional item in the plan configuration, that way the enduser can decide when to use this option in their plans or use the default carbon behaviour.

Also, I've discussed this issue with @vernondegoede about how they handle this in their Subscription API.

Used code

When starting on 2019-08-30

//source how laravel/cashier-mollie handles: https://github.com/laravel/cashier-mollie/blob/b1256489456a5f1e71e3af1de150df7d4a9fb08a/src/Subscription.php#L423

$run_timer = 12;
$tmp = $now = Carbon::parse('2019-08-30');

for($x = 1; $x <= $run_timer; $x++) {
    $tmp = $tmp->copy();

    $tmp->modify('+ 1 month');

    dump($tmp);
}

image

When starting on 2018-08-30 (one year before)

//source how laravel/cashier-mollie handles: https://github.com/laravel/cashier-mollie/blob/b1256489456a5f1e71e3af1de150df7d4a9fb08a/src/Subscription.php#L423

$run_timer = 12;
$tmp = $now = Carbon::parse('2018-08-30');

for($x = 1; $x <= $run_timer; $x++) {
    $tmp = $tmp->copy();

    $tmp->modify('+ 1 month');

    dump($tmp);
}

image

robindirksen1 commented 4 years ago

cc @driesvints @bobbybouwmann as discussed at LaraconEU

bobbybouwmann commented 4 years ago

@robindirksen1 I was going over this and I thought it makes sense that this is broken because februari can have only 28 days. It doesn't make sense that the other months don't work and that the calculations for the other months are broken, but that's a whole other story.

What happens when you always put it on the 28th when it's 29th, 30th or 31st? This should fix in general you problem and makes sense when accounting februari as well!

robindirksen1 commented 4 years ago

What happens when you always put it on the 28th when it's 29th, 30th or 31st? This should fix in general you problem and makes sense when accounting februari as well!

Yes that would work, but then nobody can have a subscription that runs from the last days (29,30,31) in a month. When you subscribe to something, let's says Laravel Forge, your subscription runs from the day you started and not a few days before or after that.

robindirksen1 commented 4 years ago

Another proposal I've:

Add a method that returns a collection Carbon instances so you can debug/show information about when the next billing day(s) for that specific subscription are.

sandervanhooft commented 4 years ago

Thanks @robindirksen1

Some good bits in here, we can work in a swappable action class (optionally override this per subscription plan) once the v1 has released.

mmachatschek commented 4 years ago

I was looking into this a little more. Currently I don't see a good solution here because the new billing cycle is calculated with ->modify() (as stated above) which basically can have any string that can be handled by the DateTime::class e.g. ->modify('+2 day first sunday''). This is a bad example but could be set as interval on integrations.

My suggestion would be to allow a more specific interval but also allow the old version e.g.

// 'interval'=> '1 month' //still valid
'interval' => [
    'value' => 1,
    'period' => 'month', //day, month, year
   // 'fixed' => true,
],

This way we could call the corresponding ->addPeriodWithoutOverflow() method, and let Carbon handle the correct interval Date. So the next cycle would not result in a completely different month.

With the fixed entry you can force the next billing cycle to be on the same day or the correspondig last day in the next month.

I made a draft PR for this #194 What do you guys think?