update readme

README.md

@@ -224,6 +224,23 @@ Once `analytics.yml` and friends are populated, the associated models will be
 automatically instrumented to send updates to BigQuery via ActiveRecord
 callbacks.
 
+#### Transactions
+
+In order to correctly record data changes in transactions, specifically in regard to rollbacks and updates, we need to include the TransactionChanges module in all ActiveRecord models, so add this to config/initializers/dfe-analytics.rb
+
+```ruby
+ActiveSupport.on_load(:active_record) do
+  include DfE::Analytics::TransactionChanges
+end
+```
+
+This can conflict with other gems who modify the `active_record#write_attribute` method, e.g. `globalize` gem. The solution is to require this gem first, e.g.
+
+```
+gem 'dfe-analytics'
+gem 'globalize' require: ['dfe-analytics', 'globalize']
+```
+
 ### 6. Send web request events
 
 #### Web requests

lib/dfe/analytics/entities.rb

@@ -20,19 +20,11 @@ module Entities
 
         after_update_commit do
           # in this after_update hook we don't have access to the new fields via
-          # #attributes — we need to dig them out of saved_changes which stores
-          # them in the format { attr: ['old', 'new'] }
-
-          #changed_attributes = {}
-          #transaction_changed_attributes.each_key do |name|
-          #  changed_attributes.merge!(name => send(name))
-          #end
+          # attributes or saved changes in transactions, so we use the
+          # TransactionChanges module
 
           updated_attributes = DfE::Analytics.extract_model_attributes(self, changed_attributes)
 
-          Rails.logger.info("log_updated_attributes: #{updated_attributes}")
-          Rails.logger.info("log_model: #{self}")
-
           allowed_attributes = DfE::Analytics.extract_model_attributes(self).deep_merge(updated_attributes)
 
           send_event('update_entity', allowed_attributes) if updated_attributes.any?

spec/dfe/analytics/entities_spec.rb

@@ -283,4 +283,55 @@
       expect(DfE::Analytics::SendEvents).not_to have_received(:perform_later)
     end
   end
+
+  describe 'transaction behaviour' do
+    let(:allowlist_fields) { %w[email_address first_name] }
+
+    it 'sends transaction create event with the correct data' do
+      ActiveRecord::Base.transaction do
+        Candidate.create(email_address: '[email protected]', first_name: 'Jason')
+      end
+      expect(DfE::Analytics::SendEvents).to have_received(:perform_later)
+      .with([a_hash_including({
+            'entity_table_name' => Candidate.table_name,
+            'event_type' => 'create_entity',
+            'data' => [
+              { 'key' => 'email_address', 'value' => ['[email protected]'] },
+              { 'key' => 'first_name', 'value' => ['Jason'] }
+            ]
+          })])
+    end
+
+    it 'sends transaction delete event with the correct data' do
+      entity = Candidate.create(email_address: '[email protected]', first_name: 'Jason')
+      ActiveRecord::Base.transaction do
+        entity.destroy
+      end
+      expect(DfE::Analytics::SendEvents).to have_received(:perform_later)
+        .with([a_hash_including({
+          'entity_table_name' => Candidate.table_name,
+          'event_type' => 'delete_entity',
+          'data' => [
+            { 'key' => 'email_address', 'value' => ['[email protected]'] },
+            { 'key' => 'first_name', 'value' => ['Jason'] }
+          ]
+        })])
+    end
+
+    it 'sends transaction update event with the correct data' do
+      entity = Candidate.create(email_address: '[email protected]', first_name: 'Jason')
+      ActiveRecord::Base.transaction do
+        entity.update(email_address: '[email protected]')
+      end
+      expect(DfE::Analytics::SendEvents).to have_received(:perform_later)
+      .with([a_hash_including({
+            'entity_table_name' => Candidate.table_name,
+            'event_type' => 'update_entity',
+            'data' => [
+              { 'key' => 'email_address', 'value' => ['[email protected]'] },
+              { 'key' => 'first_name', 'value' => ['Jason'] }
+            ]
+          })])
+    end
+  end
 end

spec/dfe/analytics/transaction_changes_spec.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+RSpec.describe DfE::Analytics::TransactionChanges do
+  with_model :Candidate do
+    table do |t|
+      t.string :email_address
+      t.string :last_name
+      t.string :first_name
+      t.string :dob
+    end
+
+    model do
+      include DfE::Analytics::TransactionChanges
+      attr_accessor :stored_transaction_changes
+
+      after_commit :store_transaction_changes_for_tests
+
+      def store_transaction_changes_for_tests
+        @stored_transaction_changes = transaction_changed_attributes.reduce({}) do |changes, (attr_name, value)|
+          changes[attr_name] = [value, send(attr_name)]
+          changes
+        end
+      end
+    end
+  end
+
+  describe 'after commit' do
+    context 'without transaction' do
+      it 'create tracks changes to attributes' do
+        entity = Candidate.create(email_address: 'foo@bar')
+
+        expect(entity.stored_transaction_changes).to eq({ 'email_address' => [nil, 'foo@bar'], 'id' => [nil, 1] })
+      end
+
+      it 'update tracks changes to attributes' do
+        entity = Candidate.create(email_address: 'foo@bar')
+        entity.update(email_address: 'bar@baz')
+
+        expect(entity.stored_transaction_changes).to eq({ 'email_address' => ['foo@bar', 'bar@baz'] })
+      end
+    end
+
+    context 'with transaction' do
+      it 'create tracks changes to attributes' do
+        ActiveRecord::Base.transaction do
+          @entity = Candidate.create(email_address: 'foo@bar')
+        end
+
+        expect(@entity.stored_transaction_changes).to eq({ 'email_address' => [nil, 'foo@bar'], 'id' => [nil, 1] })
+      end
+
+      it 'update tracks changes to attributes' do
+        entity = Candidate.create(email_address: 'foo@bar')
+        ActiveRecord::Base.transaction do
+          entity.update(email_address: 'bar@baz')
+        end
+
+        expect(entity.stored_transaction_changes).to eq({ 'email_address' => ['foo@bar', 'bar@baz'] })
+      end
+    end
+  end
+end