apotonick / disposable

Decorators on top of your ORM layer.
MIT License
171 stars 39 forks source link

Using 0.1.10 with Reform causes form field values to change to Uber::Options::Values #10

Closed laynemcnish closed 9 years ago

laynemcnish commented 9 years ago

Hello,

We did a full bundle update today while upgrading our Rails project and ran into the following issue: We are using Reform v2.0.3 and with the full update, Disposable was bumped to v0.1.10. In an #update, the form calls #validate on a param which changes all but one of the form fields to Uber::Options::Values. I have included the controller, form and test outputs below.

Report Controller:

  def update
    authorize! :manage, 'Report'
    report = Report.find(params[:id])
    form = RemoteReportForm.new(report)

    if form.validate(params[:remote_report])
      form.save do |hash|
        report.update_attributes('report' => hash.stringify_keys)
      end
      redirect_to report_path(params[:id])
    else
      render :edit
    end
  end

Form:

class RemoteReportForm < Reform::Form
  property :id
  property :name
  property :notes
  property :a_ref

  validates :name, presence: true

  module RemoteReportFormExtensions
    def name=(name)
      self.w_ref = Website.where(name: name).try(:first).try(:id)
      super(name)
    end
  end

  collection :anchors, populate_if_empty: Reports::Anchor do
    property :id
    property :name
    property :w_ref
    property :_destroy

    include RemoteReportFormExtensions

    collection :feeders, populate_if_empty: Reports::Feeder do
      property :id
      property :name
      property :w_ref
      property :_destroy

      include RemoteReportFormExtensions

    end
  end
end

Test:

    describe 'PUT update' do
      it 'should update the report' do
        expected = {
          'report' => {
            'name' => 'New name',
            'id' => 23,
            'notes' => 'These are some notes',
            'a_ref' => 56,
            'anchors' => []
          }
        }
        put :update, id: 23, remote_report: { name: 'New name' }
        expect(report).to have_received(:update_attributes).with(expected)
      end
    end

Output: form before #validate is called:

=> #<RemoteReportForm:0x007fe01f5cd698
 @_changes={},
 @fields={"id"=>23, "name"=>"One", "notes"=>"These are some notes", "a_ref"=>56, "anchors"=>[]},
 @mapper=#<#<Class:0x007fe01f5ccb80>:0x007fe01c2edc28 @model=#<InstanceDouble(Report) (anonymous)>>, @model=#<InstanceDouble(Report) (anonymous)>>

form after #validate is called:

form.validate(params[:remote_report])
form
=> #<RemoteReportForm:0x007fe004b74288
 @_changes={"id"=>true, "name"=>true, "notes"=>true, "account_ref"=>true},
 @errors=#<Reform::Contract::Errors:0x007fe00488b238 @base=#<RemoteReportForm:0x007fe004b74288 ...>, @messages={}>,
 @fields=
  {"id"=>#<Uber::Options::Value:0x007fe01c149930 @callable=false, @dynamic=false, @method=false, @proc=false, @value=nil>,
   "name"=>"New name",
   "notes"=>#<Uber::Options::Value:0x007fe01f3a5078 @callable=false, @dynamic=false, @method=false, @proc=false, @value=nil>,
   "account_ref"=>#<Uber::Options::Value:0x007fe01f33c460 @callable=false, @dynamic=false, @method=false, @proc=false, @value=nil>,
   "anchors"=>[]},
 @mapper=#<#<Class:0x007fe01f5ccb80>:0x007fe004b741c0 @model=#<InstanceDouble(Report) (anonymous)>>,
 @model=#<InstanceDouble(Report) (anonymous)>>

As you can see, before #validate is called, the form retains it's original form values and after validation, they are changed to Uber::Options::Values. The only form field that does not change is the 'name' which is also the only attribute with a validation in the RemoteReportForm.

apotonick commented 9 years ago

Sorry, you have no idea how stupid I felt when I saw this! :blush: I ran tests for the new Disposable against all my other gems but not Reform haha.

apotonick commented 9 years ago

BTW, thanks for the very clear bug report.

Your controller/form looks as if you want to give Trailblazer a go. The Operation pattern encapsulates service logic and the form, your controller will end up as a very lean endpoint. You will love it.