From 4f9882186c701e94c0e31af0cc848c7615a80702 Mon Sep 17 00:00:00 2001 From: Adam Stegman Date: Mon, 8 Jul 2024 21:14:32 +0000 Subject: [PATCH] Run callbacks at the right level Callbacks should be around the update, not just retrieving scrubbed values. I think this should have been moved in https://github.com/onemedical/acts_as_scrubbable/pull/18, but wasn't caught. --- lib/acts_as_scrubbable/scrub.rb | 26 +++++++++---------- lib/acts_as_scrubbable/update_processor.rb | 6 +++-- spec/lib/acts_as_scrubbable/scrub_spec.rb | 16 ++++-------- .../update_processor_spec.rb | 8 +++++- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/acts_as_scrubbable/scrub.rb b/lib/acts_as_scrubbable/scrub.rb index ffa9acc..28cacf2 100644 --- a/lib/acts_as_scrubbable/scrub.rb +++ b/lib/acts_as_scrubbable/scrub.rb @@ -4,24 +4,22 @@ module Scrub def scrubbed_values return unless self.class.scrubbable? - run_callbacks(:scrub) do - _updates = {} + _updates = {} - scrubbable_fields.each do |_field, value| - unless self.respond_to?(_field) - raise ArgumentError, "#{self.class} do not respond to #{_field}" - end - next if self.send(_field).blank? - - if ActsAsScrubbable.scrub_map.keys.include?(value) - _updates[_field] = ActsAsScrubbable.scrub_map[value].call - else - puts "Undefined scrub: #{value} for #{self.class}.#{_field}" - end + scrubbable_fields.each do |_field, value| + unless self.respond_to?(_field) + raise ArgumentError, "#{self.class} do not respond to #{_field}" end + next if self.send(_field).blank? - _updates + if ActsAsScrubbable.scrub_map.keys.include?(value) + _updates[_field] = ActsAsScrubbable.scrub_map[value].call + else + puts "Undefined scrub: #{value} for #{self.class}.#{_field}" + end end + + _updates end end end diff --git a/lib/acts_as_scrubbable/update_processor.rb b/lib/acts_as_scrubbable/update_processor.rb index 14bbcbd..b946517 100644 --- a/lib/acts_as_scrubbable/update_processor.rb +++ b/lib/acts_as_scrubbable/update_processor.rb @@ -8,8 +8,10 @@ class UpdateProcessor def handle_batch(batch) scrubbed_count = 0 batch.each do |obj| - _updates = obj.scrubbed_values - obj.update_columns(_updates) unless _updates.empty? + obj.run_callbacks(:scrub) do + _updates = obj.scrubbed_values + obj.update_columns(_updates) unless _updates.empty? + end scrubbed_count += 1 end scrubbed_count diff --git a/spec/lib/acts_as_scrubbable/scrub_spec.rb b/spec/lib/acts_as_scrubbable/scrub_spec.rb index 57d07be..39a35b9 100644 --- a/spec/lib/acts_as_scrubbable/scrub_spec.rb +++ b/spec/lib/acts_as_scrubbable/scrub_spec.rb @@ -57,31 +57,25 @@ expect(_updates[:address1]).to be_nil end - it 'runs scrub callbacks' do - subject.scrubbed_values - expect(subject.scrubbing_begun).to be(true) - expect(subject.scrubbing_finished).to be(true) - end - it 'output no information when all scrubbers found' do expect(STDOUT).to_not receive(:puts) - + _updates = subject.scrubbed_values end context "scrubbable" do subject { MissingScrubbableModel.new } - + it 'outputs warning message' do subject.first_name = "Johnny" subject.last_name = "Frank" - + allow(Faker::Name).to receive(:first_name).and_return("Larry") allow(Faker::Name).to receive(:last_name).and_return("Baker") - + expect(STDOUT).to receive(:puts).with('Undefined scrub: fake_first_name for MissingScrubbableModel.first_name') expect(Faker::Name).to_not receive(:first_name) - + _updates = subject.scrubbed_values expect(_updates[:last_name]).to eq('Baker') expect(_updates[:first_name]).to be_nil diff --git a/spec/lib/acts_as_scrubbable/update_processor_spec.rb b/spec/lib/acts_as_scrubbable/update_processor_spec.rb index 3a7c353..bb6ada1 100644 --- a/spec/lib/acts_as_scrubbable/update_processor_spec.rb +++ b/spec/lib/acts_as_scrubbable/update_processor_spec.rb @@ -15,5 +15,11 @@ expect(subject.send(:handle_batch, [model])).to eq 1 end + + it "runs scrub callbacks" do + subject.send(:handle_batch, [model]) + expect(model.scrubbing_begun).to be(true) + expect(model.scrubbing_finished).to be(true) + end end -end \ No newline at end of file +end