Skip to content

Commit

Permalink
Merge pull request #52 from 38degrees/fix-deprecated-ar-syntax
Browse files Browse the repository at this point in the history
Replace use of return keyword as logic control inside AR transactions, as this is not supported in Ruby 3
  • Loading branch information
duknic authored Oct 7, 2024
2 parents dca1707 + 2b08303 commit e0b6f23
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 19 deletions.
20 changes: 12 additions & 8 deletions lib/draft_approve/models/draft_transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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+.
#
Expand Down
26 changes: 15 additions & 11 deletions lib/draft_approve/persistor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit e0b6f23

Please sign in to comment.