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

Already on GitHub? Sign in to your account

build_source in payment dependent on hash order #981

Closed
rounders opened this Issue Jan 17, 2012 · 6 comments

Comments

Projects
None yet
5 participants
Contributor

rounders commented Jan 17, 2012

the build_source method in the Spree::Payment class can fail to build the source association depending on the order of the Hash passed into Payment.new and depending on the version of Ruby and how it handles hash ordering.

Whether or not payment_method is nil within the build_source method depends on whether the payment_method setter was called before the source_attributes setter and this can depend on the order of the Hash passed into Payment.new

Please see the following gist for an example test that I ran against spree 1.0.0.rc2 under various ruby versions:
https://gist.github.com/1625808

@rounders rounders added a commit to rounders/spree that referenced this issue Jan 17, 2012

@rounders rounders fix building of source association [#981] 0a3a2d6
Contributor

rounders commented Jan 17, 2012

I think something like the following would work to fix this issue:
rounders/spree@0a3a2d6

but wanted some feedback first. I'm not sure what the best way to test this. The sample test that I included in the gist in the above comment does not consistently succeed or fail with some versions of ruby.

Member

joneslee85 commented Jan 21, 2012

@rounders would this be rather an issue of rails?

Contributor

rounders commented Jan 21, 2012

i think that is probably open for debate :-)

The problem, as i see it, is that the build_source method assumes that the payment_method setter has already been called. And unfortunately that isn't something you can rely on because it depends on the order of the attributes (in the hash) given to Payment.new and ultimately in what order ruby happens to hand those attributes to the initializer.

So I think ultimately the bug is in the build_source method, not in rails.

Owner

schof commented Jan 24, 2012

I agree this needs to be fixed on our end. Investigating this now.

Member

radar commented Jan 25, 2012

@schof: Does that commit close out #981?

@schof schof closed this in d624c4c Jan 29, 2012

Contributor

Locke23rus commented Mar 1, 2012

After fixing this issue was added a new bug. Order doesn't have validation errors for empty credit card number and code.

@Locke23rus Locke23rus added a commit to Locke23rus/spree that referenced this issue Mar 2, 2012

@Locke23rus Locke23rus Fixed regression in #981. Payment can be saved without validation sou…
…rce.

[Fixes #981]
02fae55

@rounders rounders added a commit to 7543417/spree that referenced this issue Jul 11, 2012

@rounders rounders backport github issue #981 to spree 0.11
spree#981
This bug was preventing this version of spree from
working on ruby 1.8.7-p357 and newer.
f7f5396
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment