Firstly, I want to say thanks for Active Admin, it's a great gift to the rails community ❤️

I dealt with a bug recently that caused creates to fail and could result in records being deleted that were unrelated to the action being performed in Active Admin.

Using the Pundit authorization adapter with a scope class similar to the example one that is referenced in the documentation.

The scope looks like this:

class ThingPolicy
  class Scope
    def resolve
      Thing.where(id: user.accessible_thing_ids)
    end
  end
end

The user can only see Thing based on an array of ids tied to their account. Which seems reasonable and works well.

The issue arises when this user wants to create a Thing, using the default create action.

This starts in InheritedResources::Actions#create calling build_resource then create_resource which are defined in ActiveAdmin::ResourceController::DataAccess

build_resource calls build_new_resource which applies the scope to the collection and boils down to Thing.where(id: user.accessible_thing_ids).new(params). Then create_resource saves that.

The problem with this is that the new resource picks up the where scoped attributes, which is a (rather unexpected) Active Record feature.

E.g.

pry(main)> Thing.where(id: [1]).new(title: "Cat")
=> #<Thing:0x0000ffff79e721c8 id: 1, title: "Cat">

Then the create_resource happens and tries to save it, resulting in an ActiveRecord::RecordNotUnique because id: 1 already exists.

This is the simple case but when you're using a nested form and the model looks like:

class Thing
  has_one :important, dependent: :destroy
  accepts_nested_attributes_for :important
end

Rails deletes the Important.where(thing_id: 1) in preparation for creating another one and there is no transactional rollback of this, so the Important is deleted even after Thing failed to create.

The main issue of using the scope on a create seems incorrect to me. There are a couple of previous issues 1 2 which didn't really get any traction as the issues were resolved with quick fixes and closed.

There is also a comment in the ActiveAdmin::PunditAdapter#scope_collection specifically saying it should only be used for read/index actions, which doesn't appear to be the case.

This isn't triggered in all cases, as Active Record's type casting negates the issue when there are multiple scoped ids and fails to typecast to a single number, resulting in nil and leading to the expected behaviour, in this specific case, though as the scoped attributes are user defined, this whole thing could cause all kinds of weirdness, like it succeeding but attributes getting set to values you haven't set specifically, etc:

pry(main)> Thing.where(id: [1, 2,3], published: true).new(title: "Cat")
=> #<Thing:0x0000ffff77723220 id: nil, title: "Cat", published: true>

If we scope to only show users published Things and we aren't in a state where the id is overwritten, we've automatically published this Thing (possibly contrived example, as otherwise, the user wouldn't see the unpublished thing they created, but it highlights the issue!)

Expected behavior

• New records are not scoped (or more specifically, shouldn't pick up the scope's where clauses as attributes) and save successfully
• If there is a failure to save, the DELETE for nested attributes should be rolled back.

Actual behavior

• New records pick up the scope's where clause attributes, causing an error
• When there is a failure to save, the DELETE for nested attributes has already been committed and the record lost.

How to reproduce

I've created a test based on the template here:

https://gist.github.com/michaelbaldry-il/41cf95c59656d7f317aa1fc935502b4e

It would be great to get your thoughts on the right path to a resolution for this. To me, it seems like a reasonable thing to not scope on create but there may be scenarios I'm not aware of where that is expected.