how to avoid saving empty records on a nested rails form

Question

I'm using the nested_form gem for my AddressBook relation. When the user blanks out the value of an existing Addr, I want to delete that Addr rather than saving with a blank value

class Person < ActiveRecord::Base
  has_many :addrs, dependent: :destroy
  attr_accessible :name, :addrs_attributes
  accepts_nested_attributes_for :addrs, reject_if: :addr_blank, allow_destroy: true

  def addr_blank(a)
    valid? && a[:id].blank? && a[:value].blank? 
  end

class Addr < ActiveRecord::Base
  belongs_to :person
  attr_accessible :kind, :label, :value, :person_id

My :reject_if method works well but it doesn't give me everything I need

valid? keeps my blank Addrs around through validation
a[:id].blank? avoids rejections when the user blanks out and existing record

Now, I need to delete (rather than save) an existing Addr when the user blanks the value. Also, I'm exposing Persons and Addrs via a RESTful API. I see two possible options:

Post process the params hash to add the magical _destroy=1 param. IOW, emulate the user activity of pressing the delete button.
Encapsulate this inside the Addr model such that an update with a blank value is effectively considered a delete.

Based on the advice here is how I implemented it:

people_controller.rb

def update
  @person = Person.find(params[:id])
  @person.destroy_blank_addrs(params[:person])
  respond_to do |format|
  ...

person.rb

def destroy_blank_addrs(person_params)
  if valid? && person_params[:addrs_attributes]
    person_params[:addrs_attributes].each do |addr_params_array|
      addr_params= addr_params_array[1] 
      addr_params[:_destroy] = '1' if !addr_params[:id].blank? && addr_params[:value].blank? 
    end
  end
end

Of the two, use option 1. You don't want "magic" like "if the value of X field is blank then delete the record". — Zabba, Jul 15 '12 at 05:53
@Zabba, I'm refactoring this code 18 months later and you were right. My idea of blanking the value as a "magical" `destroy_blank_addrs` was brain-dead. I also believe any solution that involves directly modifying the `params` array is bad practice. Any post-processing should be done after `assign_attributes` but before `save` — Chris Beck, Feb 14 '15 at 21:51

score 17 · Answer 1 · edited Feb 14 '15 at 19:36

17

accepts_nested_attributes_for :addrs, 
  allow_destroy: true, 
  :reject_if => proc { |att| att[:name].blank? && attr[:description].blank? }

edited Feb 14 '15 at 19:36

eebbesen

4,760
7
46
67

answered Jul 21 '14 at 12:55

Shimaa Marzouk

391
3
9

Kyle Decot · Answer 2 · 2015-05-27T02:43:29.230

10

accepts_nested_attributes_for :addrs, 
  allow_destroy: true, 
  reject_if: -> { |attr| [name, description].any? &:blank? }

edited May 27 '15 at 02:43

answered Jul 21 '14 at 13:01

Kyle Decot

19,418
35
132
245

Very concise. I include a `valid?` test so rows aren't rejected until the user is done resolving validations. – Chris Beck Feb 14 '15 at 22:25

score 5 · Accepted Answer · answered Jul 19 '12 at 22:18

5

A third alternative would be to add an before_save callback on Person that will remove all addresses that are blank. This idea has some merit, but I probably won't go with it.

Out of the two options you present, I will not go with post-processing the params. It will work out, but it is too much work. Besides, to controller code will get a bit messier and I'm a firm believer in a very slim controller.

The easiest option, in my head, is to remove the blank addresses after saving. You can add Person#remove_blank_addresses() and then call it on successful save. You don't need to pass in the params - it can just iterate the addresses and remove the blank ones. It has the disadvantage of creating empty addresses and then destroying them, but you would need that for updating people anyway.

If we're talking about the cleanest solution (in my opinion), I would introduce a third class that would handle all that logic and have the controller delegate to it. The controller would be easy enough to test in isolation and then you can write a model spec that checks all the nitty-gritty details. It is a bit more work and I can't think of a good name right now (PersonUpdater?), but it might be an idea worth thinking about.

answered Jul 19 '12 at 22:18

Stefan Kanev

2,970
19
17

1

Thanks for the thoughtful response, Stefan. A 3rd class is the cleanest solution, but too much effort. The idea of putting records into the db that are available to some async process (even just for an instant) seems wrong. Another idea is to manage it on the front end in JS. Blanking is just another way of pressing the delete button. Then Blanking never makes it into my REST api. – Chris Beck Jul 20 '12 at 14:10
In that case, I would have this code in the controller. It is a bit trickier to test and somehow looks messy in my beliefs on what should be in the controller, but it should be the cleanest solution in your case. – Stefan Kanev Jul 20 '12 at 18:33
Thanks, I went with the controller approach because I felt it was simpler and it adds functionality to my API where consumers can delete Addrs by blanking the value. If you update your answer, I will accept. – Chris Beck Jul 27 '12 at 18:39
now that I'm refactoring this code, moving this into a `ServiceObject` (as you suggested) is the best solution. I named it `PersonTracker` because it also takes care of updating the `activity` log. For more info, see http://railscasts.com/episodes/398-service-objects – Chris Beck Feb 14 '15 at 21:58

score 3 · Answer 4 · edited Jul 14 '15 at 22:30

3

accepts_nested_attributes_for :addrs, 
  allow_destroy: true, 
  reject_if: :all_blank

edited Jul 14 '15 at 22:30

a14m

6,653
7
49
62

answered Jul 13 '15 at 13:23

Oniram

146
1
3

Allows you to specify a Proc or a Symbol pointing to a method that checks whether a record should be built for a certain attribute hash. The hash is passed to the supplied Proc or the method and it should return either true or false. When no :reject_if is specified, a record will be built for all attribute hashes that do not have a _destroy value that evaluates to true. Passing :all_blank instead of a Proc will create a proc that will reject a record where all the attributes are blank excluding any value for _destroy. – hadees Dec 22 '16 at 01:29

how to avoid saving empty records on a nested rails form

4 Answers4

Linked