Error when deleting models with transitions

[glenpike]

We implemented state machines with Statesman recently - which has helped us loads, thanks!

One thing we've just run into is an issue when deleting models that have transitions - we get an error like the following (tables and id's changed):

ActiveRecord::InvalidForeignKey: PG::ForeignKeyViolation: ERROR:  update or delete on table "x" violates foreign key constraint "fk_rails_1234abcd" on table "x_transitions"
DETAIL:  Key (id)=(7a1031e7-e0a7-4f1b-bf87-392d1b7161ba) is still referenced from table "x_transitions".
: DELETE FROM "x" WHERE "x"."id" = $1

I guess this is possibly because we're using ActiveRecord to persist?

I fixed the issue by adding dependent: :destroy to the association:

has_many :x_transitions, autosave: false, dependent: :destroy

Should this be in the doc's?

Also added a migration to fix existing records:

class UpdateForeignKeyXTransitionsX < ActiveRecord::Migration[5.2]
  def change
    # remove the old foreign_key
    remove_foreign_key :x_transitions, :x

    # add the new foreign_key
    add_foreign_key :x_transitions, :x, on_delete: :cascade
  end
end

[Sinjo]

Hi Glen. Thanks for your feedback!

Cascading deletes, while convenient, make it pretty easy to unintentionally delete more data than you realised you were going to. Because of that, they're not really something we use here at GC.

I can see the argument for using them here. State transitions are heavily coupled to their parent model. I'm not in favour of changing our generators or our standard examples though.

At most, I'd bring up the issue in its own 'Cascading deletes' section of the README, and lay out the options for users to choose between.

[timrogers]

Using dependent: :destroy is the right thing to do if you're sure you want this behaviour.

I tend towards the view that it's better to require any associations (in this case, the transitions) to be manually deleted. It makes the deletion of data explicit, rather than implicit and prevents any surprises.

The on_delete: :cascade in the migration is interesting too - I didn't know you could do that!

[glenpike]

Thanks for the reply @Sinjo - yes, just a README change is ideal :)

In this situation - we definitely want the transitions to disappear if the parent model is removed. My guess is that if we wanted to keep the transition data, but delete parent models, would we have to do something funky with the association to avoid foreign keys referencing the parent? (n00b-ish question I'm afraid :) )

[danwakefield]

Readme was updated in https://github.com/gocardless/statesman/pull/345/