ruckus / quickbooks-ruby

Quickbooks Online REST API V3 - Ruby
MIT License
374 stars 302 forks source link

BaseModel includes ActiveModel::AttributeAssignment #453

Closed wayne5540 closed 4 years ago

wayne5540 commented 5 years ago

Changes

Add following functions by including ActiveModel::AttributeAssignment:

Notes

Reference

wayne5540 commented 5 years ago

Test breaks because ActiveSupport requires Ruby > 2.2.2. Not caused by this PR.

drewish commented 4 years ago

Might be worth rebasing this PR against master to see if the tests will be happier with all the gem bumps that have happened since then.

wayne5540 commented 4 years ago

Rebased. CI still failed. Found reason.

This PR failed the test with Ruby < 2.3 because ActiveModel::AttributeAssignment was introduced by rails 5+. reference

Run the CI with Ruby verison < 2.3 will use activemode 4.2.11 version which has not yet implemented ActiveModel::AttributeAssignment module.

To make this PR works we need to lock dependency to activemodel > 5.0 which I don't know if it worth.

ruckus commented 4 years ago

Thanks for investigating @wayne5540

No we definitely cannot lock to > 5.0. I'm still on 4.x (just upgraded my massive legacy app from rails3 last week!)

wayne5540 commented 4 years ago

@ruckus Yes, agree! Feel free to close this PR.

ruckus commented 4 years ago

Closing for now. This PR requires activemodel > 5.0