Stripe monthly subscriptions are 30 days long instead of one month

Thank you for maintaining this plugin and for all your hard work!

I noticed something odd: When you make a Stripe subscription for x months, it actually makes a subscription for x * 30 days. This can add up and short change a customer by 5.25 days per year.

Here is a relevant thread Monthly billing query, that points out another issue with this. The day a customer is charged changes from month to month. This can be confusing for customers, and can even result in overdraft fees if they have limited funds.

The relevant code is in stripe-utilities.inc.php:377 in the method defined as:
public static function get_plan($shortcode_attrs, $metadata = array())

Specifically, the issue is that the call to the “per_term_2_days()” method converts 1M to 30D and 1Y to 365D.

Stripe supports time periods set in days, weeks, months, or years, like the s2member shortcode, so I do not believe this conversion is needed.

I modified the “get_plan()” method to the code below and tested as working for 23D, 2W, 1M and 1Y. However, I know that this will get overwritten the next time I update s2member.

Would it be possible to please merge this fix into the codebase so that it is in the next version? That would be a huge help!


	public static function get_plan($shortcode_attrs, $metadata = array())
	{
		$input_time = time(); // Initialize.
		$input_vars = get_defined_vars(); // Arguments.

		self::init_stripe_sdk();

		$amount                      = $shortcode_attrs['ra'];
		$currency                    = $shortcode_attrs['cc'];
		$name                        = $shortcode_attrs['desc'];
		$metadata['recurring']       = $shortcode_attrs['rr'] && $shortcode_attrs['rr'] !== 'BN';
		// rrt installments are not managed by Stripe, it's a regular subscription ended by s2 after number of payments.
		// This gets tricky with Jason's shift of first regular to a separate charge when there's an unused trial period.
		$metadata['recurring_times'] = $shortcode_attrs['rr'] && $shortcode_attrs['rrt'] ? (integer)$shortcode_attrs['rrt'] : -1;
		$trial_period_days           = self::per_term_2_days($shortcode_attrs['tp'], $shortcode_attrs['tt']);
		$interval_term = "";
		switch (strtoupper($shortcode_attrs['rt']))
		{
			case 'D':
				$interval_term = "day";
				break;
			case 'W':
				$interval_term = "week";
				break;
			case 'M':
				$interval_term = "month";
				break;
			case 'Y':
				$interval_term = "year";
				break;
		}
		$interval_period = is_numeric($shortcode_attrs['rp'])? (integer)$shortcode_attrs['rp'] : 0;

		// The access is more correct for the product's name, and will avoid duplicate products,
		// but the shortcode's description is probably better in this case...
		// $product_name = trim('level'$shortcode_attrs['level'].':'.$shortcode_attrs['ccaps']);
		$product      = self::get_product($name);

		$plan_id      = 's2_plan_'.md5($amount.$currency.$name.$trial_period_days.$interval_period.$interval_term.serialize($metadata).$GLOBALS['WS_PLUGIN__']['s2member']['o']['pro_stripe_api_statement_description']);

		try // Attempt to get an existing plan; else create a new one.
		{
			try // Try to find an existing plan.
			{
				$plan = \Stripe\Plan::retrieve($plan_id);
			}
			catch(exception $exception) // Else create one.
			{
				$plan = array(
					'id'                => $plan_id,
					'product'           => $product->id,
					'metadata'          => $metadata,
					'amount'            => self::dollar_amount_to_cents($amount, $currency),
					'currency'          => $currency,
					'interval'          => $interval_term,
					'interval_count'    => $interval_period,
					// This condition in the argument below moves the first regular period out of the subscription when there's an unused trial period.
					// Basically, if there's an unused trial, it'll use it, it will always set a trial, even when the site owner didn't mean it.
					// This trial will be "free" in the subscription (trialing...). The period is still charged, but separately.
					// 'trial_period_days' => $trial_period_days ? $trial_period_days : $interval_days,
				);
				// Stop adding the trial for subscriptions that didn't mean to have it.
				// To allow paid trials (initial period, different from the regular ones), create invoice item for it right before the subscription,
				// so it gets charged in the trial's invoice. https://stripe.com/docs/billing/subscriptions/trials
				if($trial_period_days)
					$plan['trial_period_days'] = $trial_period_days;

				$plan = \Stripe\Plan::create($plan);
			}
			self::log_entry(__FUNCTION__, $input_time, $input_vars, time(), $plan);

			return $plan; // Stripe plan object.
		}
		catch(exception $exception)
		{
			self::log_entry(__FUNCTION__, $input_time, $input_vars, time(), $exception);

			return self::error_message($exception);
		}
	}
1 Like

+1 for this as an update. We’ve had to use a bunch of workarounds to make customers happy with this over the years.

Ditto to what Jason said! :slight_smile:

Here are the file changes:

They are relatively minor so I really hope they can be rolled in!

I put in a pull request on GitHub with the changes, and requested clavaque to review it for pulling into the repository. I hope you can please accept the change!

Woah - we just found the same thing!
image

Surely the subscrition for these people is now wrong within Stripe - how will this get fixed???

Thank you so much for bringing this to my attention, guys.

I agree with you, but I have to be careful with this change. Jason wrote the code so it’d change all subscriptions down to days as a common denominator, and there are many parts of the code that expect only days, not months or something else.

So I can’t easily make this change without risking breaking something unknown that may not be noticed in a while, after causing other problems unnoticed. I’ve been reviewing the code, and there are many places to study and many things to change, and all with a risk of breaking something…

What I can see as a simpler and safer change, would be to change the month conversion from 30 days to 31… I still need to review more carefully all the code that would depend on that, but from what I saw, it doesn’t seem it’d break anything.

That, instead of shortchanging the customer by 5 days a year, would give him an extra week. This would be advantageous for the customer and end possible complaints, but then you’re giving away some extra bonus time you could have charged for.

If you guys think that it’d be an acceptable compromise, I can make this change for next the release.

And of course, I’m keeping this in mind for the new integration I want to create in the future, rewriting the whole pro-forms and gateways code.

Thank you for looking into this! If we can’t get the fix right now, then I’ll just use 31D instead of 1M on my end - no need to change 1M from 30D to 31D in the source code.

But really I hope that fixing 1M to 1 calendar month could please be put in the not-to-distant future. It really simplifies things. s2member’s PayPal gateway (which we also use) correctly uses 1M = 1 calendar month. So if Stripe’s 1M is also set to correctly use 1 calendar month, then timings will match between Stripe, PayPal, and customer expectations. No weirdness, nice and clean!

1 Like

There is the additional problem/confusion of knowing which stripe customers are losing the day at the moment, and how to fix their subscription so it works correctly from some point onwards. All the current members with monthly subscriptions need something to happen to fix future payments. Could you look into how that could be done automatically, please?