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

support for ActiveModel::Model alternative to database-backed models only #144

Merged
merged 1 commit into from
Jun 16, 2016

Conversation

brandondees
Copy link
Contributor

related to #143

PhonyRails can be added to ActiveModel::Model as well as ActiveRecord::Base to enable usage in models that are not db-backed. There is also a dependency on ActiveModel::Validations::Callbacks as the normalization currently depends on a before_validate callback.

I am using this version for my project and have extended the basic test coverage to #account for this usage variant. Please provide any relevant critique or feedback.

@coveralls
Copy link

coveralls commented May 11, 2016

Coverage Status

Coverage increased (+0.0004%) to 99.796% when pulling 379cc03 on brandondees:activerecord-freedom into 17a310d on joost:master.

@brandondees
Copy link
Contributor Author

I'll see if I can reproduce this build failure.

@brandondees
Copy link
Contributor Author

I had accidentally removed one of the tested attribute names.

@coveralls
Copy link

coveralls commented May 11, 2016

Coverage Status

Coverage increased (+0.0004%) to 99.796% when pulling e11cc3b on brandondees:activerecord-freedom into 17a310d on joost:master.

@joost
Copy link
Owner

joost commented May 24, 2016

Sorry for the late reply. I like this. However we don't need to include in AR::Base AND AR::Model right?

@sighmin
Copy link

sighmin commented May 27, 2016

@brandondees @joost This is awesome I use form objects when the validations for the endpoint are different for the persisted model itself.

Yeah I don't think you need to include it in AR::Base just AR:Model

Thanks @brandondees 🎉

@brandondees
Copy link
Contributor Author

sorry, I didn't get notifications for your replies...

I didn't want to interfere with existing use cases / tests so I left the existing behavior alone, just extended it to work with plain models.

@sighmin
Copy link

sighmin commented Jun 5, 2016

@brandondees No worries :) As far as I can remember ActiveRecord::Model is included in ActiveRecord::Base.

Keep in mind you can use ActiveModel without ActiveRecord, so just check for the presence of ActiveModel constant and include it in there and we should golden 👍 @joost any changes before merge?

@joost
Copy link
Owner

joost commented Jun 5, 2016

Only remove the duplication. ActiveRecord::Base should not include but inherit from ::Model.

On 05 Jun 2016, at 08:40, Simon van Dyk [email protected] wrote:

@brandondees No worries :) As far as I can remember ActiveRecord::Model is included in ActiveRecord::Base.

Keep in mind you can use ActiveModel without ActiveRecord, so just check for the presence of ActiveModel constant and include it in there and we should golden 👍 @joost any changes before merge?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@joost joost merged commit 454775c into joost:master Jun 16, 2016
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

Successfully merging this pull request may close these issues.

5 participants