From d3117244c148fcbb99737b6889853c5d35d428d7 Mon Sep 17 00:00:00 2001 From: Nic Duke Date: Mon, 30 Sep 2024 16:21:55 +1300 Subject: [PATCH 1/4] 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. From 9edec7bfc13ee423e47f8e0a7b8b9ac476f34281 Mon Sep 17 00:00:00 2001 From: Nic Duke Date: Mon, 30 Sep 2024 17:02:21 +1300 Subject: [PATCH 2/4] Amend CircleCI config to remove remove testing of Ruby versions < 3 --- .circleci/config.yml | 51 +------------------------------------------- 1 file changed, 1 insertion(+), 50 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4d9f3f0..d0cd5a5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -2,22 +2,6 @@ version: 2.1 # executors are environments in which a job runs. Here we use docker contaners. executors: - ruby-2-6: - docker: - - image: circleci/ruby:2.6.6-node-browsers - - image: circleci/postgres:10.6 - environment: - - POSTGRES_USER=draft_approve_test - - POSTGRES_DB=draft_approve_test - - ruby-2-7: - docker: - - image: circleci/ruby:2.7.2-node-browsers - - image: circleci/postgres:10.6 - environment: - - POSTGRES_USER=draft_approve_test - - POSTGRES_DB=draft_approve_test - ruby-3-0: docker: - image: circleci/ruby:3.0.0-node-browsers @@ -33,18 +17,12 @@ jobs: # We can use YAML to define a part of a job we alias as 'build', and then include it in other jobs to keep things DRY build-default: &build parameters: - run-activerecord-5-2-x: - type: boolean - default: true - run-activerecord-6-0-x: - type: boolean - default: true run-activerecord-6-1-x: type: boolean default: true # Specify a default executor for the job - executor: ruby-2-6 + executor: ruby-3-0 working_directory: ~/repo steps: @@ -84,20 +62,6 @@ jobs: mkdir /tmp/test-results TEST_FILES="$(circleci tests glob "spec/**/*_spec.rb" | circleci tests split --split-by=timings)" - - when: - condition: << parameters.run-activerecord-5-2-x >> - steps: - - run: - name: run activerecord-5.2.x tests - command: bundle exec appraisal activerecord-5-2-x rspec - - - when: - condition: << parameters.run-activerecord-6-0-x >> - steps: - - run: - name: run activerecord-6.0.x tests - command: bundle exec appraisal activerecord-6-0-x rspec - - when: condition: << parameters.run-activerecord-6-1-x >> steps: @@ -112,15 +76,6 @@ jobs: path: /tmp/test-results destination: test-results - # Specific build jobs for each ruby version - build-ruby-2-6: - <<: *build - executor: ruby-2-6 - - build-ruby-2-7: - <<: *build - executor: ruby-2-7 - build-ruby-3-0: <<: *build executor: ruby-3-0 @@ -129,8 +84,4 @@ jobs: workflows: build_all_versions: jobs: - - build-ruby-2-6 - - build-ruby-2-7 - build-ruby-3-0: - # Rails/ActiveRecord earlier than 6.x is not compatible with ruby 3 - run-activerecord-5-2-x: false From e79f8cd8122319f06fc4e9ecfa7b4804505db8ec Mon Sep 17 00:00:00 2001 From: Nic Duke Date: Mon, 30 Sep 2024 17:07:59 +1300 Subject: [PATCH 3/4] be better at yml --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index d0cd5a5..6e6eb92 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -84,4 +84,4 @@ jobs: workflows: build_all_versions: jobs: - - build-ruby-3-0: + - build-ruby-3-0 From 18653dca254f06f671516fc0a862f27a3166faf7 Mon Sep 17 00:00:00 2001 From: Nic Duke Date: Tue, 1 Oct 2024 11:10:21 +1300 Subject: [PATCH 4/4] Revert circleci config change --- .circleci/config.yml | 53 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6e6eb92..4d9f3f0 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -2,6 +2,22 @@ version: 2.1 # executors are environments in which a job runs. Here we use docker contaners. executors: + ruby-2-6: + docker: + - image: circleci/ruby:2.6.6-node-browsers + - image: circleci/postgres:10.6 + environment: + - POSTGRES_USER=draft_approve_test + - POSTGRES_DB=draft_approve_test + + ruby-2-7: + docker: + - image: circleci/ruby:2.7.2-node-browsers + - image: circleci/postgres:10.6 + environment: + - POSTGRES_USER=draft_approve_test + - POSTGRES_DB=draft_approve_test + ruby-3-0: docker: - image: circleci/ruby:3.0.0-node-browsers @@ -17,12 +33,18 @@ jobs: # We can use YAML to define a part of a job we alias as 'build', and then include it in other jobs to keep things DRY build-default: &build parameters: + run-activerecord-5-2-x: + type: boolean + default: true + run-activerecord-6-0-x: + type: boolean + default: true run-activerecord-6-1-x: type: boolean default: true # Specify a default executor for the job - executor: ruby-3-0 + executor: ruby-2-6 working_directory: ~/repo steps: @@ -62,6 +84,20 @@ jobs: mkdir /tmp/test-results TEST_FILES="$(circleci tests glob "spec/**/*_spec.rb" | circleci tests split --split-by=timings)" + - when: + condition: << parameters.run-activerecord-5-2-x >> + steps: + - run: + name: run activerecord-5.2.x tests + command: bundle exec appraisal activerecord-5-2-x rspec + + - when: + condition: << parameters.run-activerecord-6-0-x >> + steps: + - run: + name: run activerecord-6.0.x tests + command: bundle exec appraisal activerecord-6-0-x rspec + - when: condition: << parameters.run-activerecord-6-1-x >> steps: @@ -76,6 +112,15 @@ jobs: path: /tmp/test-results destination: test-results + # Specific build jobs for each ruby version + build-ruby-2-6: + <<: *build + executor: ruby-2-6 + + build-ruby-2-7: + <<: *build + executor: ruby-2-7 + build-ruby-3-0: <<: *build executor: ruby-3-0 @@ -84,4 +129,8 @@ jobs: workflows: build_all_versions: jobs: - - build-ruby-3-0 + - build-ruby-2-6 + - build-ruby-2-7 + - build-ruby-3-0: + # Rails/ActiveRecord earlier than 6.x is not compatible with ruby 3 + run-activerecord-5-2-x: false