`update` causing unexpected RecordNotUnique error

ktweeden commented 3 years ago

I recently upgraded from v0.9.0 to 1.0.1 in order to address the deprecation warnings for connection config. It has caused some of my tests to fail in a way that I wasn't expecting given the details outlined in the change log and I think it may be a bug.

I have a model called User which has many Questions. Questions must be unique by name for a given user. We have a controller for adding questions to a user which has a method along the lines of user.update(user_params_with_questions). The params include an array of questions to add.

In the failing test cases case we're attempting to add two new questions with the same name to a user as part of the same update operation. Previously, this returned false but it's now raising a RecordNotUnique error. If I add them sequentially I get the expected behaviour.

I went through a process of progressively upgrading from the original version to pinpoint when the breaking changes were introduced and found that 0.9.4 was the culprit. This is consistent with the stack trace which points to /lib/database_validations/lib/validations.rb:20:increate_or_update'`:

def create_or_update(*args, &block)
      options = args.extract_options!

      if options[:validate] == false
        super
      else
        rescue_from_database_exceptions { super }
      end
    end

So, my assumption is that something is causing the validate option to be interpreted as false and the super behaviour is being called vs the rescue_from_database_exceptions function.

Additional context:

Rails version: 6.1.4.1 Ruby version: 2.6.2

Any ideas about what might be going on here? Or additional behaviour that would help figure it out? Thanks in advance!

djezzzl commented 3 years ago

Hello @ktweeden,

Thank you for using the gem and pointing out the issues you have!

Firstly, let me confirm that the gem was supposed to improve only the Rails standards without breaking the flow. So, we expect, that anything we improve still implies the same behavior.

You're right, that before applying a fix (yes, a fix) that was done on 0.9.4, those methods start to raise errors instead gracefully returning validation errors.

Such actions, as expected, should raise errors based on ActiveRecord decision to not run validations (validate: false). If one day, this decision would change, then the gem will apply its improvements for nested saving automatically. Unfortunately, that is not the case now and we don't want to break this behavior (at least by default).

There are a few things we can do:

provide an option to validators to catch database errors (and turn them to validation errors) even if validate: false was provided.
provide an option to improve valid? method to check in-memory objects that are about to be saved if they're unique among themselves.

I guess (not sure yet) that the second option would resolve the issue you mentioned too as before saving nested associations, it actually checks them (AFAIR) for validness.

Please, let me know if you want to contribute to the gem with any of those. That's fine if you won't, then I'll do some changes myself and we will check if they help in your cases.

djezzzl commented 3 years ago

The issue was also discussed here: https://github.com/toptal/database_validations/issues/55.

djezzzl commented 3 years ago

Hi @ktweeden ,

I've just released version 1.1.0 with new option: https://github.com/toptal/database_validations#rescue-option. Could you please have a look and let me know if it helps with your issue?

Please feel free to reopen the issue. Have a great day!

toptal / database_validations

`update` causing unexpected RecordNotUnique error #60