From d3117244c148fcbb99737b6889853c5d35d428d7 Mon Sep 17 00:00:00 2001 From: Nic Duke Date: Mon, 30 Sep 2024 16:21:55 +1300 Subject: [PATCH] Replace use of return keyword as logic control inside AR transactions, as this is not supported in Ruby 3 --- lib/draft_approve/models/draft_transaction.rb | 20 ++++++++------ lib/draft_approve/persistor.rb | 26 +++++++++++-------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/lib/draft_approve/models/draft_transaction.rb b/lib/draft_approve/models/draft_transaction.rb index 04d1c46..32bd8a6 100644 --- a/lib/draft_approve/models/draft_transaction.rb +++ b/lib/draft_approve/models/draft_transaction.rb @@ -56,24 +56,28 @@ class DraftTransaction < ActiveRecord::Base # # @return [Boolean] +true+ if the changes were successfully applied def approve_changes!(reviewed_by: nil, review_reason: nil) + result = false begin ActiveRecord::Base.transaction do - self.lock! # Avoid multiple threads applying changes concurrently - return false unless self.status == PENDING_APPROVAL - - drafts.order(:created_at, :id).each do |draft| - draft.apply_changes! + self.lock! # Avoid multiple threads applying changes concurrently + if self.status == PENDING_APPROVAL + drafts.order(:created_at, :id).each do |draft| + draft.apply_changes! + end + + self.update!(status: APPROVED, reviewed_by: reviewed_by, review_reason: review_reason) + result = true + next end - - self.update!(status: APPROVED, reviewed_by: reviewed_by, review_reason: review_reason) - return true end rescue StandardError => e # Log the error in the database table and re-raise self.update!(status: APPROVAL_ERROR, error: "#{e.inspect}\n#{e.backtrace.join("\n")}") raise end + result end + # Reject all changes in this +DraftTransaction+. # diff --git a/lib/draft_approve/persistor.rb b/lib/draft_approve/persistor.rb index 23ee47f..9b1670c 100644 --- a/lib/draft_approve/persistor.rb +++ b/lib/draft_approve/persistor.rb @@ -46,7 +46,7 @@ def self.write_draft_from_model(action_type, model, options = nil) if validate_model?(options) && model.invalid? raise(ActiveRecord::RecordInvalid, model) end - + result = false DraftApprove::Transaction.ensure_in_draft_transaction do # Now we're in a Transaction, ensure we don't get multiple drafts for the same object if model.persisted? && Draft.pending_approval.where(draftable: model).count > 0 @@ -76,17 +76,21 @@ def self.write_draft_from_model(action_type, model, options = nil) changes = serializer.changes_for_model(model) # Don't write no-op updates! - return false if changes.empty? && action_type == Draft::UPDATE - - return model.draft_pending_approval = Draft.create!( - draft_transaction: draft_transaction, - draftable_type: draftable_type, - draftable_id: draftable_id, - draft_action_type: action_type, - draft_changes: changes, - draft_options: draft_options - ) + if changes.empty? && action_type == Draft::UPDATE + next + else + model.draft_pending_approval = Draft.create!( + draft_transaction: draft_transaction, + draftable_type: draftable_type, + draftable_id: draftable_id, + draft_action_type: action_type, + draft_changes: changes, + draft_options: draft_options + ) + result = model.draft_pending_approval + end end + result end # Write the changes represented by the given +Draft+ object to the database.