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

Broken test for payment in allocation_spec #131

Closed
wsmoak opened this issue May 27, 2016 · 2 comments
Closed

Broken test for payment in allocation_spec #131

wsmoak opened this issue May 27, 2016 · 2 comments

Comments

@wsmoak
Copy link
Contributor

wsmoak commented May 27, 2016

Adding the Chargify::Payment resource for external payments in #128 has broken the assertion at

https://github.com/chargify/chargify_api_ares/blob/master/spec/resources/allocation_spec.rb#L87

expect(result.first.payment).to be_a Chargify::Allocation::Payment

Previously, it seems like ActiveResource was "inventing" a nested Chargify::Allocation::Payment (that doesn't really exist...). Now that there ​_is_​ a Chargify::Payment, it uses that.

A Chargify::Payment is an external payment, which records something that happened outside Chargify, like receiving a paper check in the mail. See: https://docs.chargify.com/api-payments

The 'Payment' within an 'Allocation' is really a Chargify::Transaction with a type of payment. See: https://docs.chargify.com/api-transactions

@wsmoak
Copy link
Contributor Author

wsmoak commented May 27, 2016

To restore the original behavior, it seems that defining a 'Payment' class inside 'Allocation' is enough to get ActiveResource to make the payment a Chargify::Allocation::Payment again. I'm still working with it to make sure that doesn't break anything else.

Thanks to @cmoel for help debugging this!

wsmoak added a commit that referenced this issue May 27, 2016
This is needed so that ActiveResource will not use Chargify::Payment
when there is a Payment inside an Allocation.

This Payment is an output-only attribute of an Allocation.

Fixes #131
@wsmoak
Copy link
Contributor Author

wsmoak commented May 28, 2016

WIP on branch https://github.com/chargify/chargify_api_ares/tree/fix_allocation_spec2

This adds a Payment class inside Allocation, and ActiveResource "sees" it and uses it instead of the other Chargify::Payment.

The response differs slightly -- the Chargify::Allocation::Payment does not have the 'persisted' and 'prefix_options' attributes that seem to be on all the other objects whose classes extend from ActiveResource::Base.

Making the inner Payment class inherit from < Base does not make those attributes show up either.

I'm still working on getting the entire test suite (including the remote tests) to run locally to see if this breaks anything else.

wsmoak added a commit that referenced this issue May 29, 2016
This is needed so that ActiveResource will not use Chargify::Payment
when there is a Payment inside an Allocation.

This Payment is an output-only attribute of an Allocation.

The Payment class should inherit from Base so that its initialize method is defined properly.

Fixes #131
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant