Rename ActiveModel::Model to ActiveModel::BasicModel

[cgriego]

This module provides the sets of features required to support the ActiveModel API and no more. Its naming is meant to be reminiscent of Ruby's Object and BasicObject.

[josevalim]

👎 for removing the initialize method implementation. I prefer an implementation that is practical than one that fits a conceptual purpose of mapping to BasicObject.

[cgriego]

@josevalim I've added back the initialize method as ActiveModel::MassAssignment. It's anemic for now but will grow as people find need for mass assignment security and the other ActiveRecord like methods (attributes=, assign_attributes). This is such a common piece of code that there's no reason to believe that people needing mass assignment functionality also need to support the ActiveModel API, provided by BasicModel. For example, this presentation repeats the code twice in classes that never interface with ActionPack.

[josevalim]

MassAssignment is being re-evaluated for Rails 4. That said, let's put a
halt on this patch until we define how MassAssignment is going to look
like.

[cgriego]

@josevalim Is mass assignment still being re-evaluated for Rails 4?

[josevalim]

Yes, we will like move Rails to use something like strong_parameters.

[cgriego]

Ok, so strong_parameters in its current form depends on the model class calling sanitize_for_mass_assignment, which nothing in ActiveModel does today. An ActiveModel::MassAssignment module would be the best place to add that feature for use in non-ActiveRecord models.

@josevalim Is there anything else pending the acceptance of this pull request?

[steveklabnik]

This doesn't merge cleanly any more, and will need a rebase.

[cgriego]

@steveklabnik I've rebased, it should merge cleanly now.

[steveklabnik]

GitHub still says no, though maybe it's caching is a bit out of date. I know @tenderlove was talking about some weirdness on Twitter today...

[cgriego]

@steveklabnik I've just merged the branch cleanly with the current master locally. Does GitHub agree now?

[josevalim]

It merges cleanly now but something is wrong, it says we have more than 250 commits to be merged.

[vendethiel]

It's been merged, merging then other comments, and became a whole mess ._.

[cgriego]

@josevalim Every way I check it looks clean on my end. I think GitHub got confused. I can submit as a new pull request if you like but I'd hate for it to sit as long as this one has.

[steveklabnik]

You should do this:

$ git checkout master
$ git fetch upstream
$ git reset --hard upstream/master
$ git checkout rename-model-to-basic-model
$ git reset --hard upstream/master
$ git cherry-pick SOME_SHA
<repeat for all commits>
$ git push origin rename-model-to-basic-model --force

Where SOME_SHA is the sha of the commit you want. Or commits. I forget if there are multiple.

I'd be happy to help walk you through this if you're unsure about it. Basically just resetting EVERYTHING to make sure it's current with master and making only the changes on your branch. GitHub should automatically update the pull request.

[cgriego]

Rebasing again seems to have fixed GitHub's view of the pull request. But for the record, the tree was clean.

* 7c95be5 (rails/master, rails/ambiguous_controller_fix) Fix generators to help with ambiguous `ApplicationController` issue
* [abbreviated]
* 47971a1 Merge pull request #6360 from bcardarella/logger_debug_fix
| * 8125042 (rename-model-to-basic-model) Added ActiveModel mass assignment back in as ActiveModel::MassAssignment
| * 3ecdda7 Rename ActiveModel::Model to ActiveModel::BasicModel
|/
* 251b22f Don't need to force size to nil

[frodsan]

👎 Is this split really worthwhile?

[frodsan]

@cgriego Could you please rebase it?

[rafaelfranca]

I'm fine with the rename. But I don't like the idea of the MassAssignment module. I don't see why we need to split.

[frodsan]

This is a duplicate of #8064. I'm closing here to continue the discussion there.

@cgriego Thanks!!!