From ba3743388d1c2eb8cadd52a3aef81eabc0fd1eda Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Fri, 21 Jun 2024 15:47:15 +0100 Subject: [PATCH] Close government / update ministers transactionally Currently, it's possible for this method to close the government, and then run into trouble later on while closing ministerial appointments. If one of these `save!` calls failed, we'd be left in an inconsistent state. Wrapping the update and save! calls in a transaction will mean that they either all succeed, or are all rolled back, avoiding the chance of getting into this bad state. I also suspect (but am not 100% sure) that doing this transactionally will significantly speed up the code, because we won't have to open / close a transaction for every role appointment we save. --- app/controllers/admin/governments_controller.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/governments_controller.rb b/app/controllers/admin/governments_controller.rb index 351c55ae8ec..eccf10f87f4 100644 --- a/app/controllers/admin/governments_controller.rb +++ b/app/controllers/admin/governments_controller.rb @@ -42,11 +42,13 @@ def prepare_to_close def close government = Government.find(params[:id]) - government.update!(end_date: Time.zone.today) unless government.end_date + ActiveRecord::Base.transaction do + government.update!(end_date: Time.zone.today) unless government.end_date - current_active_ministerial_appointments.each do |appointment| - appointment.ended_at = government.end_date - appointment.save!(validate: false) + current_active_ministerial_appointments.each do |appointment| + appointment.ended_at = government.end_date + appointment.save!(validate: false) + end end redirect_to edit_admin_government_path(government), notice: "Government closed"