Allow users to pause their subscription / go on vacation

Rerolled to apply with latest 4.x dev

The last submitted patch, 28: recurly-pause_subscriptions-2320769-28.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Small change to incorrect variable name.

The last submitted patch, 31: recurly-pause_subscriptions-2320769-31.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

reroll

The last submitted patch, 33: recurly-pause_subscriptions-2320769-33.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

It looks right to me. The patch applies and appears to work. I'm not sure whats going on with the test failures though.

Trying to get the tests to pass.
I'm not very good with tests, but it looks like it is failing in recurly_user_edit_form_submit_redirect(). I Removed the call to the deprecated initialize() method inside getPlans() to make it more in line with the recurly_subscription_plans() function it is replacing. Maybe that will do it?

The last submitted patch, 36: recurly-pause_subscriptions-2320769-36.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Getting closer.

I believe I've tracked down why the remaining tests are failing. If I'm not mistaken, it stems from this change

@@ -58,7 +58,7 @@ class RecurlySubscriptionListController extends RecurlyController {
     // Unlikely that we'd have more than 50 subscriptions, but you never know.
     $per_page = 50;
     $subscription_type = $this->config('recurly.settings')->get('recurly_subscription_display');
-    $state = $subscription_type === 'live' ? ['state' => 'active'] : [];
+    $state = $subscription_type === 'live' ? ['state' => 'live'] : [];

The reason this was changed from "active" to "live", is that paused subscriptions are not considered "active". Those are considered to be in the "future" state. so querying for "live" will return both. see: https://recurly.com/developers/api/v2021-02-25/index.html#operation/list...

Without this, when a subscription is paused it wont show the summary on the subscription page since its only querying active ones.

However, the tests are expecting the param to be "active" and therefore throwing this error

Recurly_NotFoundError: Don't know how to GET '/accounts/abcdef1234567890/subscriptions?per_page=50&state=live' in Drupal\recurly_test_client\RecurlyMockClient->request() (line 277 of modules/contrib/recurly/tests/modules/recurly_test_client/src/RecurlyMockClient.php).

I think this is whats going on. I'm not really up on how tests work, so I could use some assistance in correcting this.

The patch is otherwise functioning as expected.

Removed an invalid call to getClientFromSettings() in RecurlySubscriptionPauseConfirmForm

The last submitted patch, 39: recurly-pause_subscriptions-2320769-39.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

This needs a config setting to enable or disable this feature.

Apologies for the mess I've made.

Unfortunately, I don't know enough about tests to fix whatever is going on with them. But this MR33 applies successfully and works for me. I have been using the code in production for a while now.

Hopefully someone can take a look and shed some light on to whats happening.

loze → closed merge request !33