Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: pass client to objects in an array #409

Merged
merged 1 commit into from
Apr 23, 2019

Conversation

glaubinix
Copy link

Hello,

I encountered an issue where I use a mock client of recurly during test runs where the client does not get passed down to all sub objects.

For instance if I fetch an invoice like this:

$invoice = \Recurly_Invoice::get($invoiceNumber, new MockClient());

if this gets called for an invoice that contains credit_payments then all the \Recurly_Stub object on the individual \Recurly_CreditPayment do not have the MockClient set as _client but instead have the default \RecurlyClient set.

@@ -399,6 +399,9 @@ protected static function __parseXmlToObject($node, &$object)
// has element children, drop in and continue parsing
$new_obj = Recurly_Resource::__createNodeObject($node);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it'd be better to just pass client into __createNodeObject as a parameter since most of the time we assign it afterwards. Then we'd avoid needing to check if it's an instance of Recurly_Base.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I updated the pull request.

@@ -366,7 +364,7 @@ protected static function __parseXmlToObject($node, &$object)
continue;
}

$new_obj = Recurly_Resource::__createNodeObject($node);
$new_obj = Recurly_Resource::__createNodeObject($node, $object->_client);
Copy link

@aaron-junot aaron-junot Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One reason the tests are failing right now is because in this context, $object can be an array (see line 360) which wouldn't have a _client property.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I addressed the issue and the tests are now passing.

Copy link

@aaron-junot aaron-junot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

@aaron-junot aaron-junot merged commit f4750a4 into recurly:master Apr 23, 2019
@glaubinix glaubinix deleted the f/pass-client branch July 6, 2019 19:03
@bhelx bhelx added the V2 V2 Client label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V2 V2 Client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants