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

Hydrating a Resource node object doesn't transfer client instance from Recurly_Pager #254

Closed
cyruscollier opened this issue Jul 22, 2016 · 5 comments · Fixed by #265
Closed
Labels
V2 V2 Client

Comments

@cyruscollier
Copy link

During a singular resource hydration Recurly_Base::__parseResponseToNewObject() is called, which passes the client instance to the newly created resource object. However, when a Recurly_Pager subclass like Recurly_SubscriptionList hydrates all of its nodes and calls Recurly_Base::__createNodeObject(), that method does NOT pass the client instance to the each newly created resource objects. This means if a subclassed or mocked Recurly_Client class is used as the client in Recurly_SubscriptionList, the resulting node objects cannot be used directly with the same expected client behavior.

@drewish
Copy link

drewish commented Jul 22, 2016

Interesting... so seems like __parseXmlToObject should either be assigning _client or __createNodeObject should take client as a parameter and assign it.

Do you have any test code you were using that demonstrates the problem that I could use to build a unit test off of?

@cyruscollier
Copy link
Author

I can do one better than that! Here's a full unit test demonstrating the problem and a patch that solves it. You'll see when this test runs that we get the error Recurly_UnauthorizedError: Your API Key is not authorized to connect to Recurly., which means the test isn't using the Recurly_MockClient when trying to reactivate on a subscription node. I've included a patch for Recurly_Base which passes the client down to all of the child nodes.

SubscriptionNode_Test.php.txt
base.php.diff.txt

cyruscollier added a commit to cyruscollier/recurly-client-php that referenced this issue Jul 25, 2016
@cyruscollier
Copy link
Author

Any updates/thoughts on this?

@drewish
Copy link

drewish commented Aug 10, 2016

hey sorry! i want to get this into the next release. would you be offended if i redo that spec?

@cyruscollier
Copy link
Author

Are you referring to the unit test I wrote? Yeah, do whatever you want with it - it's just for demonstrating the issue.

drewish added a commit that referenced this issue Aug 10, 2016
drewish added a commit that referenced this issue Aug 10, 2016
drewish added a commit that referenced this issue Aug 10, 2016
@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 a pull request may close this issue.

3 participants