Revise mailchimp export code to apply new defaults (#4276)

app/services/mailchimp/csv_exporter.rb

@@ -32,10 +32,11 @@ class CsvExporter < BaseAudienceExportProcessorService
     attr_reader :new_nonsubscribed
 
     # Initialise with lists of hashed fields parsed from Mailchimp export
-    def initialize(subscribed:, nonsubscribed:, cleaned:, unsubscribed:)
+    def initialize(subscribed:, nonsubscribed:, cleaned:, unsubscribed:, add_default_interests: false)
       super(subscribed:, nonsubscribed:, cleaned:, unsubscribed:)
       @updated_audience = { subscribed: [], nonsubscribed: [], cleaned: [], unsubscribed: [] }
       @new_nonsubscribed = []
+      @add_default_interests = add_default_interests
     end
 
     # Match contacts against database, updating with latest data and mapping to
@@ -58,29 +59,33 @@ def match_and_update_contacts
           mailchimp_contact_type = mailchimp_contact_type(user.email)
           # remove matches from list
           contact = @audience[mailchimp_contact_type].delete(user.email)
-          # update user
-          @updated_audience[mailchimp_contact_type] << to_mailchimp_contact(user, contact)
+          # update user, only adding default interests if we're overriding current prefs
+          @updated_audience[mailchimp_contact_type] << to_mailchimp_contact(user, contact, add_default_interests: @add_default_interests)
         else
-          @new_nonsubscribed << to_mailchimp_contact(user, newsletter_subscriber: false)
+          # always add default interests
+          @new_nonsubscribed << to_mailchimp_contact(user, add_default_interests: true)
         end
       end
     end
 
     # Process any Mailchimp contacts that are not registered users
+    # Only add default interests if overriding current prefs
     def process_unmatched_contacts
       @audience.each do |category, contacts|
-        updated_audience[category] = updated_audience[category] + contacts.values.map {|c| copy_contact(c) }
+        updated_audience[category] = updated_audience[category] + contacts.values.map {|c| copy_contact(c, add_default_interests: @add_default_interests) }
       end
     end
 
-    def to_mailchimp_contact(user, existing_contact = nil, newsletter_subscriber: true)
+    def to_mailchimp_contact(user, existing_contact = nil, add_default_interests: false)
       # Convert from comma-separated names to hash
       interests = if existing_contact.present? && existing_contact[:interests].present?
                     existing_contact[:interests].split(',').index_with { |_i| true }
                   else
                     {}
                   end
-      interests['Newsletter'] = true if newsletter_subscriber
+
+      interests = default_interests(interests, user) if add_default_interests
+
       tags = existing_contact.present? && existing_contact[:tags].present? ? existing_contact[:tags].split(',') : []
       contact = Mailchimp::Contact.from_user(user, tags: tags, interests: interests)
 
@@ -121,52 +126,53 @@ def non_fsm_tags(existing_contact)
     #
     # This is to allow for migration to be re-run before we tidy up and remove some of
     # the old fields.
-    def copy_contact(existing_contact)
-      # TODO use new model
+    def copy_contact(existing_contact, add_default_interests: false)
       contact = ActiveSupport::OrderedOptions.new
       contact.email_address = existing_contact[:email_address]
       contact.contact_source = 'Organic'
       contact.locale = 'en'
       contact.tags = non_fsm_tags(existing_contact).join(',')
 
-      # If this is present then we're updating an existing contact that should
-      # have Newsletter set already
-      # TODO naming
-      contact.interests = existing_contact[:interests] || 'Newsletter'
-
-      first_and_last_name_fields = existing_contact[:first_name] && existing_contact[:last_name]
-      first_and_name_fields = existing_contact[:first_name] && existing_contact[:name] && !existing_contact[:name].include?(existing_contact[:first_name])
-
-      if first_and_last_name_fields # older Mailchimp only fields
-        contact.name = [existing_contact[:first_name], existing_contact[:last_name]].join(' ')
-      elsif first_and_name_fields # some users have entered first/last names into the first_name and name fields
-        contact.name = [existing_contact[:first_name], existing_contact[:name]].join(' ')
-      elsif existing_contact[:name] # if set, this is usually full name
-        contact.name = existing_contact[:name]
-      else # combine whatever name fields we have
-        contact.name = [existing_contact[:first_name], existing_contact[:last_name]].join('')
-      end
+      interests = if existing_contact[:interests].present?
+                    existing_contact[:interests].split(',').index_with { |_i| true }
+                  else
+                    {}
+                  end
 
-      contact.staff_role = existing_contact[:staff_role] || existing_contact[:user_type]
-      contact.school = existing_contact[:school] || existing_contact[:school_or_organisation]
-
-      if existing_contact[:school_group].present?
-        contact.school_group = existing_contact[:school_group]
-      else
-        # :local_authority_and_mats is the current field, but not all groups are present
-        # due to limitations in number of groups in Mailchimp.
-        #
-        # So use the "other" fields presented to users on the mailchimp form in preference as
-        # these are hopefully more accurate, otherwise fall back to the current field.
-        contact.school_group = if existing_contact[:other_mat].present?
-                                 existing_contact[:other_mat]
-                               elsif existing_contact[:other_la].present?
-                                 existing_contact[:other_la]
-                               else
-                                 existing_contact[:local_authority_and_mats]
-                               end
-      end
+      contact.interests = add_default_interests ? default_interests(interests) : interests
+      contact.name = existing_contact[:name]
+      contact.staff_role = existing_contact[:staff_role]
+      contact.school = existing_contact[:school_or_organisation]
+      contact.school_group = existing_contact[:school_group]
       contact
     end
+
+    def default_interests(interests, user = nil)
+      # hash of id to value
+      defaults = Mailchimp::Contact.default_interests(email_types, user)
+
+      id_to_name = email_types.to_h { |i| [i.id, i.name] }
+      named_defaults = defaults.transform_keys {|k| id_to_name[k] }
+      named_defaults.reject! { |_k, v| !v }
+      interests ? interests.merge(named_defaults) : named_defaults
+    end
+
+    def audience_manager
+      @audience_manager ||= Mailchimp::AudienceManager.new
+    end
+
+    def email_types
+      @email_types ||= list_of_email_types
+    end
+
+    def list_of_email_types
+      category = audience_manager.categories.detect {|c| c.title == 'Interests' }
+      return [] unless category
+      return audience_manager.interests(category.id)
+    rescue => e
+      Rails.logger.error(e)
+      Rollbar.error(e)
+      []
+    end
   end
 end

lib/tasks/mailchimp/csv_export.rake

@@ -1,16 +1,19 @@
 namespace :mailchimp do
   desc "Export new versions of Mailchimp export CSV files"
-  task :csv_export, [:dir] => :environment do |t,args|
+  task :csv_export, [:dir, :add_defaults] => :environment do |t,args|
+    args.with_defaults(add_defaults: false)
+    add_defaults = ActiveModel::Type::Boolean.new.cast( args.add_defaults )
+
     puts "#{DateTime.now.utc} Mailchimp CSV Export Started"
-    puts "Loading from #{args.dir}"
+    puts "Loading from #{args.dir}, add_defaults #{add_defaults}"
 
     audience = {}
     [:subscribed, :unsubscribed, :nonsubscribed, :cleaned].each do |category|
       file = Dir.glob("#{category}*", base: args.dir).first
       audience[category] = CSV.read("#{args.dir}/#{file}", headers: true, header_converters: :symbol)
     end
 
-    service = Mailchimp::CsvExporter.new(**audience)
+    service = Mailchimp::CsvExporter.new(add_default_interests: add_defaults, **audience)
     puts "#{DateTime.now.utc} Fetching data"
 
     service.perform

spec/services/mailchimp/csv_exporter_spec.rb

@@ -1,10 +1,13 @@
 require 'rails_helper'
 
 describe Mailchimp::CsvExporter do
+  include_context 'with a stubbed audience manager'
+
   subject(:service) do
-    described_class.new(subscribed: subscribed, nonsubscribed: nonsubscribed, unsubscribed: unsubscribed, cleaned: cleaned)
+    described_class.new(add_default_interests: add_default_interests, subscribed: subscribed, nonsubscribed: nonsubscribed, unsubscribed: unsubscribed, cleaned: cleaned)
   end
 
+  let(:add_default_interests) { true }
   let(:subscribed) { [] }
   let(:nonsubscribed) { [] }
   let(:unsubscribed) { [] }
@@ -19,13 +22,9 @@ def create_contact(email_address, **fields)
     contact
   end
 
-  shared_examples 'it adds interests correctly' do |newsletter: true|
-    it 'adds Newsletter', if: newsletter do
-      expect(contact.interests).to eq 'Newsletter'
-    end
-
-    it 'does not add Newsletter', unless: newsletter do
-      expect(contact.interests).to be_empty
+  shared_examples 'it adds interests correctly' do
+    it 'adds emails types' do
+      expect(contact.interests).to include 'Getting the most out of Energy Sparks'
     end
   end
 
@@ -106,13 +105,31 @@ def create_contact(email_address, **fields)
         expect(contact.tags.split(',')).to contain_exactly('FSM30', user.school.slug)
       end
 
+      context 'when not adding in default interests' do
+        let(:add_default_interests) { false }
+
+        it 'does not add any interests' do
+          expect(contact.interests).to eq('')
+        end
+      end
+
       context 'with existing interests' do
         let(:subscribed) do
-          [create_contact(user.email, interests: 'Newsletter,Others')]
+          [create_contact(user.email, interests: 'Getting the most out of Energy Sparks,Others')]
+        end
+
+        it 'preserves the existing interests' do
+          expect(contact.interests).to include('Getting the most out of Energy Sparks')
+          expect(contact.interests).to include('Others')
         end
 
-        it 'preserves the interests' do
-          expect(contact.interests).to eq('Newsletter,Others')
+        context 'when not adding in the default interests' do
+          let(:add_default_interests) { false }
+
+          it 'preserves the existing interests' do
+            expect(contact.interests).to include('Getting the most out of Energy Sparks')
+            expect(contact.interests).to include('Others')
+          end
         end
       end
 
@@ -238,87 +255,9 @@ def create_contact(email_address, **fields)
       service.perform
     end
 
-    context 'with a pre-migration contact' do
-      let(:subscribed) do
-        [create_contact('user@example.org', first_name: 'John', last_name: 'Smith', school_or_organisation: 'DfE', user_type: 'School management', other_la: 'bhcc', other_mat: 'Unity Schools Partnership', local_authority_and_mats: 'Other', tags: 'trustee,external support,FSM30')]
-      end
-
-      it 'retains the contact' do
-        expect(service.updated_audience[:subscribed].length).to eq(1)
-      end
-
-      it_behaves_like 'it adds interests correctly'
-
-      it 'populates the fields correctly' do
-        expect(contact.email_address).to eq('user@example.org')
-        expect(contact.name).to eq 'John Smith'
-        expect(contact.contact_source).to eq 'Organic'
-        expect(contact.confirmed_date).to be_nil
-        expect(contact.user_role).to be_nil
-        expect(contact.locale).to eq 'en'
-        expect(contact.staff_role).to eq 'School management'
-        expect(contact.school).to eq 'DfE'
-        expect(contact.school_group).to eq 'Unity Schools Partnership'
-      end
-
-      it 'copies tags, stripping free school meal tags' do
-        expect(contact.tags).to eq 'trustee,external support'
-      end
-
-      context 'with name and first name only' do
-        let(:subscribed) do
-          [create_contact('user@example.org', first_name: 'John', name: 'Smith')]
-        end
-
-        it 'builds the name correctly' do
-          expect(contact.name).to eq 'John Smith'
-        end
-      end
-
-      context 'with name only' do
-        let(:subscribed) do
-          [create_contact('user@example.org', name: 'John Smith')]
-        end
-
-        it 'builds the name correctly' do
-          expect(contact.name).to eq 'John Smith'
-        end
-      end
-
-      context 'with first name and name overlapping' do
-        let(:subscribed) do
-          [create_contact('user@example.org', name: 'John Smith', first_name: 'John')]
-        end
-
-        it 'builds the name correctly' do
-          expect(contact.name).to eq 'John Smith'
-        end
-      end
-
-      context 'with first name only' do
-        let(:subscribed) do
-          [create_contact('user@example.org', first_name: 'John')]
-        end
-
-        it 'builds the name correctly' do
-          expect(contact.name).to eq 'John'
-        end
-      end
-
-      context 'with last name only' do
-        let(:subscribed) do
-          [create_contact('user@example.org', last_name: 'Smith')]
-        end
-
-        it 'builds the name correctly' do
-          expect(contact.name).to eq 'Smith'
-        end
-      end
-    end
-
     context 'with a post-migration contact' do
       let(:subscribed) do
-        [create_contact('user@example.org', name: 'John Smith', first_name: 'John', last_name: 'Smith', school: 'DfE', user_type: 'School management', school_group: 'Unity Schools Partnership', school_or_organisation: 'XDfE', other_la: 'Xbhcc', other_mat: 'XUnity Schools Partnership', local_authority_and_mats: 'Other', tags: 'trustee,external support')]
+        [create_contact('user@example.org', name: 'John Smith', staff_role: 'School management', school_group: 'Unity Schools Partnership', school_or_organisation: 'DfE', tags: 'trustee,external support')]
       end
 
       it_behaves_like 'it adds interests correctly'
@@ -351,7 +290,14 @@ def create_contact(email_address, **fields)
       end
 
       it_behaves_like 'it correctly creates a contact', school_user: true
-      it_behaves_like 'it adds interests correctly', newsletter: false
+      it_behaves_like 'it adds interests correctly'
+
+      context 'when not adding defaults' do
+        let(:add_default_interests) { false }
+
+        # still add them here as user has not expressed a preference yet
+        it_behaves_like 'it adds interests correctly'
+      end
     end
 
     context 'with a group admin' do
@@ -362,7 +308,7 @@ def create_contact(email_address, **fields)
       end
 
       it_behaves_like 'it correctly creates a contact', group_admin: true
-      it_behaves_like 'it adds interests correctly', newsletter: false
+      it_behaves_like 'it adds interests correctly'
     end
 
     context 'with an admin' do
@@ -373,7 +319,7 @@ def create_contact(email_address, **fields)
       end
 
       it_behaves_like 'it correctly creates a contact'
-      it_behaves_like 'it adds interests correctly', newsletter: false
+      it_behaves_like 'it adds interests correctly'
     end
 
     context 'with an unconfirmed user' do