payolapayments / payola

Drop-in Rails engine for accepting payments with Stripe
http://www.payola.io
Other
818 stars 154 forks source link

set optional:true on all non-presence validated belongs_to associations #306

Closed JeremiahChurch closed 7 years ago

JeremiahChurch commented 7 years ago

This fixes #259 - with rails 5.1 this is now a breaking issue in upgrading from 5.0 -> 5.1.

Any belongs_to that didn't have presence validation already on the model now has optional: true & and the dummy app config default is enabled as well.

Hopefully I didn't miss anything...

CLAassistant commented 7 years ago

CLA assistant check
All committers have signed the CLA.

peterkeen commented 7 years ago

Thanks! Looks like it's failing tests though.

On Sun, Jun 11, 2017 at 1:57 PM, CLAassistant notifications@github.com wrote:

[image: CLA assistant check] https://cla-assistant.io/payolapayments/payola?pullRequest=306 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement https://cla-assistant.io/payolapayments/payola?pullRequest=306 before we can accept your contribution. You have signed the CLA already but the status is still pending? Let us recheck https://cla-assistant.io/check/payolapayments/payola?pullRequest=306 it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/payolapayments/payola/pull/306#issuecomment-307645665, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA2mVbEzIE5ipE9sUBxTXRpW2YjDMkXks5sDCp_gaJpZM4N2doP .

JeremiahChurch commented 7 years ago

gah, of course the older versions - what's your strategy on this gem for backwards support? check the rails version and add the optional key if rails > 5 or new version/branch?

I'll look at those other failures...

peterkeen commented 7 years ago

I don't have a strategy right now, but ideally we maintain support for 4.0 onwards.

On Sun, Jun 11, 2017 at 2:31 PM, Jeremiah notifications@github.com wrote:

gah, of course the older versions - what's your strategy on this gem for backwards support? check the rails version and add the optional key if rails > 5 or new version/branch?

I'll look at those other failures...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/payolapayments/payola/pull/306#issuecomment-307647722, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA2mcLdsAdQVAFw4mh-czRxr-y7lLYWks5sDDKcgaJpZM4N2doP .

JeremiahChurch commented 7 years ago

It's ugly but it passes now :-)

The failures against 5.x sort of raise a question to me - the model validations are checking the attributes for product_id/type & plan_id/type rather than validating the presence of the actual model - the specs are likewise only building the attributes vs the related product/plan. that required me to add the optional: true to the product & plan associations of subscription & sale - I wanted to limit the scope of changes to only the minimal needed - I'm guessing that validation scheme is intentional?

peterkeen commented 7 years ago

Thank you!