Skip to content

Commit

Permalink
Prevent double validations on save/save! (#70)
Browse files Browse the repository at this point in the history
* Prevent double validations on save/save!

Originally, calling `form.save/!` would cause model validations to fire
off twice.

The first time when the form's validations are being run (as the form
has a validation to ensure that all `models` are valid`)

The second time is when the `models` are looped over and they each
receive `save!`.

For validations that are done with in-memory data, such as `validate
:attribute, presence: true`, this isn't a big deal. But this could be
problematic for any validation that is dependent on the state of the
database (it will effectively hit the database twice).

This instead passes `validate: false` into the saving of models so that
their validations aren't run a second time.

* Update README to reflect line count
  • Loading branch information
thewatts committed Jul 15, 2021
1 parent bfb108d commit 56e6bbc
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 1 deletion.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ If you want to learn more about Form Objects you can check out [these great arti

### Why YAAF?

- It is [69 lines long](https://github.com/rootstrap/yaaf/blob/master/lib/yaaf/form.rb#L69). As you can imagine, we did no magic in such a few lines of code, we just leveraged Rails modules in order to provide our form objects with a Rails-like behavior. You can review the code, it's easy to understand.
- It is [71 lines long](https://github.com/rootstrap/yaaf/blob/master/lib/yaaf/form.rb#L71). As you can imagine, we did no magic in such a few lines of code, we just leveraged Rails modules in order to provide our form objects with a Rails-like behavior. You can review the code, it's easy to understand.

- It provides a similar API to `ActiveModel` models so you can treat them interchangeably.

Expand Down
2 changes: 2 additions & 0 deletions lib/yaaf/form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ def save_in_transaction(options)
end

def save_models(options)
options.merge!(validate: false)

models.map { |model| model.save!(options) }
end

Expand Down
6 changes: 6 additions & 0 deletions spec/support/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
class User < ActiveRecord::Base
validates :name, presence: true

validate :custom_validation

def custom_validation
'A validation method used in the specs'
end
end
34 changes: 34 additions & 0 deletions spec/yaaf/model_validations_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

RSpec.describe 'Model validations' do
let(:form) { WithValidationCallbacksForm.new(args) }
let(:email) { 'test@example.com' }
let(:name) { 'John' }
let(:args) do
{ email: email, name: name, before_counter: 0, after_counter: 0 }
end

context 'by default' do
let(:options) { {} }

%i[save save!].each do |persistence_method|
it "calling #{persistence_method} runs the model validations only once" do
expect(form.user).to receive(:custom_validation).once

form.send(persistence_method, options)
end
end
end

context 'with validate: false' do
let(:options) { { validate: false } }

%i[save save!].each do |persistence_method|
it "calling #{persistence_method} doesn't run model validations" do
expect(form.user).not_to receive(:custom_validation)

form.send(persistence_method, options)
end
end
end
end

0 comments on commit 56e6bbc

Please sign in to comment.