railsgsoc / actionform

Create nested forms with ease.
MIT License
293 stars 35 forks source link

Assign form attributes #8

Open vad4msiu opened 10 years ago

vad4msiu commented 10 years ago

I have form:

class Forms::Publishers::CreateForm < ActiveForm::Base
  attributes :name
  validates :name, presence: true
end

Then in a controller I submit params:

form = Forms::Publishers::CreateForm.new(publisher)
form.submit(params)

But if params contain not name key (example: foo) I have error: NoMethodError: undefined methodfoo=' for #. It force my to useparams.slice(...)orstrong_parameters` gem. Why not just ignore attributes which don't defined? I thought form should receive any params but assign only defined?

m-Peter commented 9 years ago

You're correct. The functionality you describe is the desired. I'll work on it.

kaspth commented 9 years ago

Actually I think how it works currently is the desired way. Library users are expected to use strong parameters.

cc @guilleiguaran

andrey-skat commented 9 years ago

@kaspth Why to use strong parameters if you are using form objects? We already list all allowed attributes in attributes of ActiveForm. Why we need duplicate these attributes in controller?

vad4msiu commented 9 years ago

:+1: I agree with the comment above

kaspth commented 9 years ago

I don't mind the duplication concern expressed here as I'm not DRY religious (not saying you are).

To me it feels odd that a form specifies its accepted attributes. Rails moved away from attr_accessible and this kind of introduces it again. That's part of a controller and why Strong Parameters was added.

On the other hand, specifying attributes does bring a certain clarity to the form.

What do you think, @m-Peter?

andrey-skat commented 9 years ago

attr_accessible and attributes in form object it's not the same thing.

I concerned about duplication not because DRY, but because keeping in sync many attributes between form and controller bored.

m-Peter commented 9 years ago

I share the same opinion with @andrey-skat . I had a hard time trying to setup the nested hierarchy nature that strong_parameters require.

m-Peter commented 9 years ago

There is a WIP in this commit: https://github.com/rails/activeform/commit/8e847e7a4206d1e3cb94527e54806e2032cef954 . Let me know your thoughts about it.

kaspth commented 9 years ago

@andrey-skat I know they functionally aren't the same, but effectively they are for what we're talking about.

@m-Peter I've left some comments for you on that linked commit.

m-Peter commented 9 years ago

@kaspth Thanks for your help! I've applied them and will update the commit.