montrealrb/Montreal.rb

Bug: Cannot delete events that have talks

kirahowe opened this issue · 6 comments

kirahowe commented

To reproduce:

  • create a talk, assign it to an event
  • delete that event from administrate dashboard

Gives you
ActiveRecord::InvalidForeignKey in Admin::EventsController#destroy
PG::ForeignKeyViolation: ERROR: update or delete on table "events" violates foreign key constraint

You have to manually remove the talk from the event first before destroying an event. Is this intended behaviour?

sophiedeziel commented

Thank you for reporting! It's a good thing that error pops out now. We would have talks linked to non-existent events.

There are multiple solutions:

  • catch the error and display a nice message so the admin have to manually remove the talks
  • unlink the talk automatically without asking
  • asking confirmation before deleting the event and unlink the talk.
  • delete would not delete the record but "archive" it.

There is also some things that have to be taken in consideration:

  • Maybe we should be able to delete only proposed events. A scheduled or passed event should not be deleted unless it's "unpublished" by changing it's state. We're making sure that it's not deleted accidentally.
  • the soft-delete solution seems really good to me.
mathieugagne commented

I'm all in on non-delete but simply un-publish instead. It leaves a nice paper trail of would-have-been events.

Does it make sense for a talk not to be featured at an event?

sophiedeziel commented

Non-delete for the win!

@mathieugagne what do you mean? Technically, it seem easy, but I can't think of a use case. Have something in mind?

mathieugagne commented

I meant that soft-delete is nice and all when you have a case for a record to be resurrected. But if you can't have a talk without an event, then perhaps deleting the event should simply delete the talk.

On the unpublish part, I meant instead of having acts_as_paranoid-type solution, we could simply have states. Delete is hard delete. The other posts are unpublished if you wish to keep it but not display it. It has the advantage of keeping the db clean. Worst case you go fish an old backup for the one post you didn't mean to delete but I have never seen this. Sort of like going back in your git history for the one line you deleted. If it's not in the reach of ctrl-z, then live with it.

I wanted to be the voice that raises concerns about feature-bloat where not needed.

sophiedeziel commented

I agree with you @mathieugagne, you have good points. I guess the simplest solution is the best.

Then, we can hard-delete, buuuuut only on "unpublished" and when there is no talk associated to it. Most mistakes will be avoided, harmless and data stays clean.

Having talks without events can be useful when planning. Those are "proposed", they might be incomplete, or maybe they will be submitted by members before being picked by organizers...