stephskardal / rails_admin_import

Rails Admin Import functionality
http://www.endpoint.com/
MIT License
160 stars 122 forks source link

Optionally disabling model validations on import #59

Closed dmitrypol closed 6 years ago

dmitrypol commented 8 years ago

My models have many validations (which I normally need). However, when I am loading data via RA Import I sometimes do not need to enforce them.

It could be a useful feature to allow user to explicitly select a checkbox to disable validations just for this one upload (with appropriate text warning). Assuming the biz user knows what he/she is doing.

monkbroc commented 8 years ago

I didn't comment on this one yet. This is a divisive topic in Rails: where should model validations be used. I tend to use model validations for data integrity only and use operations to enforce business rules. So in principle I'm against turning off validations during bulk import since I feel this would come back to bite me later when the application crashes with invalid associations, blank fields, etc.

I have turned off some validations on a case-by-case basis like requiring a password for a Devise user during import.

class User < ActiveRecord::Base
  # Non-persistent attribute to allow creating a new user without a password
  # Password will be set by the user by following a link in the invitation email
  attr_accessor :allow_blank_password

  def password_required?
    allow_blank_password ? false : super
  end

  # Don't require a password when importing users
  def before_import_save(record)
    self.allow_blank_password = true
  end
end
dmitrypol commented 8 years ago

Yeah, I am very torn about the concept as well as I am big user of validations (had to deal with corrupted data too many times). On one hand it's useful to give a power user functionality to bypass certain safety checks, on the other hand it's dangerous.

To get around it I implemented before hooks where I just set dummy values. I like your approach of disabling validations code and only in before_import hook.

monkbroc commented 6 years ago

I added a note to the readme about this