Convert RecurlyHostedRoutes to be a sub-class of RouteSubscriberBase implementing the alterRoutes method in the same way that that RecurlyRouteSubscriber is currently implemented.

CommentFileSizeAuthor
#6 convert_recurly_hosted-2663608-6.patch6.02 KBadamzimmermann
#6 interdiff.txt2.11 KBadamzimmermann
#4 convert_recurly_hosted-2663608-4.patch3.95 KBmarkdorison
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markdorison created an issue. See original summary.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
CreditAttribution: colan at Colan Schwartz Consulting commented

What part of the model should we follow? What needs to change? I think we need a description here.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Issue summary: View changes

Updated issue summary.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Priority: Normal » Minor
Status: Active » Needs review
FileSize
convert_recurly_hosted-2663608-4.patch3.95 KB

No functional changes. Testing should consist of looking for regressions.

adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Assigned: Unassigned » adamzimmermann
adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Assigned: adamzimmermann » Unassigned
Status: Needs review » Needs work
FileSize
interdiff.txt2.11 KB
convert_recurly_hosted-2663608-6.patch6.02 KB

I tested this by going directly to (/user/1/subscription/billing) and it threw an error: Recoverable fatal error: Argument 1 passed to recurly_hosted_account_manage_url() must be of the type array, object given,. So I checked out what was calling recurly_hosted_account_manage_url() and found it was passing in an object as the error claimed. So I changed the type hinting for the recurly_hosted_account_manage_url() function.

Below are the two locations that call recurly_hosted_account_manage_url(), and neither seem to pass in an array.

if ($url = recurly_hosted_account_manage_url($recurly_account, TRUE)) {

return recurly_hosted_account_manage_url($context['account'], TRUE);

Once I fixed that, this redirected successfully. However, the URL it sent me to was a 404. Is that a known issue, or am I testing this incorrectly?

The edit buttons at "/admin/config/services/recurly/subscription-plans" seem to work just fine.

Perhaps this is the issue? Should RecurlyHostedAccountRedirectController be calling recurly_hosted_account_manage_url? or should it be calling recurly_hosted_subscription_plan_purchase_url?

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented

@adamzimmermann

I tested this by going directly to (/user/1/subscription/billing) and it threw an error: Recoverable fatal error: Argument 1 passed to recurly_hosted_account_manage_url() must be of the type array, object given,.

Did you try this on a user other than user 1? The user 1 issue may be seperate and nothing to do with this patch?

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented

@adamzimmermann Please ignore my previous comment.

I tested this by going directly to (/user/1/subscription/billing) and it threw an error:

However, the URL it sent me to was a 404.

I actually get a 404 with this patch and an exception without it. Improvement??

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs work » Needs review
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs review » Needs work
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs work » Needs review
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs review » Reviewed & tested by the community

I believe this patch fixes the issue at hand but has exposed a new (previously existing) issue documented in #2869882: recurly_hosted account manage url returns 404.

I am going to commit this patch and address the new bug in it's own issue.

  • markdorison committed 12bbacd on 8.x-1.x 
    Issue #2663608 by adamzimmermann, markdorison: Convert recurly_hosted...
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.