Undocumented behaviour of #save!

[artur.b...@eestiinternet.ee]

It seems passing `validate: false` to `ActiveRecord::Base#save!` bypasses validation, however doc states that:

 With <tt>save!</tt> validations always run.

https://github.com/rails/rails/blob/5d1402a1011f58b405e42007d3ceed4e122d273e/activerecord/lib/active_record/persistence.rb#L130

Isn't it confusing? Perhaps it's worth triggering some check for this case?

I am using Rails v4.2.5.2, but it could also apply to newest versions.

[Alberto Almagro]

Hi Artur!

Thanks! Good catch! This is indeed confusing. I think the intention of <tt>save!</tt> is to throw an exception if validation fails, while <tt>save</tt>  "only" returns false if it can't persist the record.

I would take one of the following actions to fix this:

1. Update the docs of <tt>save!</tt> so they aren't confusing anymore or

2. Make <tt>save!</tt> run always validations, although validate: false is provided.

I would like someone of the core team tell use which direction they want to take with this.

[nanaya]

Hi,

On Sun, Feb 11, 2018, at 22:24, Alberto Almagro wrote:
> Hi Artur!
>
> Thanks! Good catch! This is indeed confusing. I think the intention of
> <tt>save!</tt> is to throw an exception if validation fails, while
> <tt>save</tt> "only" returns false if it can't persist the record.
>
> I would take one of the following actions to fix this:
> 1. Update the docs of <tt>save!</tt> so they aren't confusing anymore or
> 2. Make <tt>save!</tt> run always validations, although validate: false is
> provided.
>

Here's the code:

def save!(*)
create_or_update || raise(RecordNotSaved.new("Failed to save the record", self))
end

It already ignores the parameter. My quick test also shows it's ignored and the model is validated regardless (5.1.4).

[Alberto Almagro]

Hi nanaya,

with Rails 5.1.4 I can reproduce it:

Having the schema:

  create_table "posts", force: :cascade do |t|

    t.string "title"

    t.string "content"

    t.string "meta_description"

    t.datetime "created_at", null: false

    t.datetime "updated_at", null: false

    t.string "slug"

    t.boolean "public"

    t.string "image"

    t.string "image_teaser"

    t.string "youtube_id"

    t.string "author", default: "María García"

    t.index ["created_at"], name: "index_posts_on_created_at"

    t.index ["slug"], name: "index_posts_on_slug", unique: true

  end

And a model with this validations:

class Post < ApplicationRecord

  validates :title, presence: true, length: { maximum: 100 }

  validates :content, presence: true

  validates :meta_description, presence: true, length: { maximum: 155 }

end

I execute:

irb(main):001:0> p = Post.new

=> #<Post id: nil, title: nil, content: nil, meta_description: nil, created_at: nil, updated_at: nil, slug: nil, public: nil, image: nil, image_teaser: nil, youtube_id: nil, author: "Max Mustermann">

irb(main):002:0> p.save

   (0.3ms)  BEGIN

   (0.4ms)  ROLLBACK

=> false

irb(main):003:0> p.save!

   (0.4ms)  BEGIN

   (0.3ms)  ROLLBACK

ActiveRecord::RecordInvalid: translation missing: es.activerecord.errors.messages.record_invalid

from (irb):3

irb(main):004:0> p.save! validate: false

   (0.3ms)  BEGIN

  SQL (1.1ms)  INSERT INTO "posts" ("created_at", "updated_at") VALUES ($1, $2) RETURNING "id"  [["created_at", "2018-02-11 13:54:05.991094"], ["updated_at", "2018-02-11 13:54:05.991094"]]

   (0.6ms)  COMMIT

=> true

irb(main):005:0> p

=> #<Post id: 32, title: nil, content: nil, meta_description: nil, created_at: "2018-02-11 13:54:05", updated_at: "2018-02-11 13:54:05", slug: nil, public: nil, image: nil, image_teaser: nil, youtube_id: nil, author: "Max Mustermann">

May I open an issue?

Best regards,

Alberto Almagro

[nanaya]

Hi,

On Sun, Feb 11, 2018, at 22:57, Alberto Almagro wrote:
> Hi nanaya,
>
> with Rails 5.1.4 I can reproduce it:
>

Right, looks like I tried wrong thing before. The actual save is in validation.rb and few years ago support for options is (accidentally?) added during a refactor and nobody noticed until now.

[nanaya]

Hi,

On Mon, Feb 12, 2018, at 00:03, nanaya wrote:
>
> Right, looks like I tried wrong thing before. The actual save is in
> validation.rb and few years ago support for options is (accidentally?)
> added during a refactor and nobody noticed until now.
>

Nevermind, it's already fixed in 5.0 and up.

# By default, #save! always runs validations. If any of them fail
# ActiveRecord::RecordInvalid gets raised. However, if you supply
# validate: false, validations are bypassed altogether. See
# ActiveRecord::Validations for more information.

[artur.b...@eestiinternet.ee]

Oh, indeed, https://github.com/rails/rails/blob/5d1402a1011f58b405e42007d3ceed4e122d273e/activerecord/lib/active_record/validations.rb#L42 calls the one I mentioned above. @nanaya, Could you point to a commit where it is fixed? I am able to reproduce it in 5.1.4.

[Artur Beljajev]

https://github.com/rails/rails/issues/31959