From c5ff9673ed2191ba3569d03ba41b3c75d07524e0 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Thu, 24 Feb 2022 13:21:37 +1100 Subject: [PATCH 01/22] Update to handle how Id now does phone numbers --- .../nation_builder_member_sync_push_serializer.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb b/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb index befba4a..1d9b267 100644 --- a/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb +++ b/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb @@ -10,11 +10,13 @@ def nationbuilder_id end def phone - strip_country_code(@object.landline) + number = @object.phone_numbers.landline.first + strip_country_code(number.phone) if number end def mobile - strip_country_code(@object.mobile) + number = @object.phone_numbers.mobile.first + strip_country_code(number.phone) if number end private From df3f5d29f66a69876594cd8f8e82c32655e219ef Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Thu, 24 Feb 2022 13:21:55 +1100 Subject: [PATCH 02/22] Update to handle how Id now does external ids --- .../nation_builder_member_sync_push_serializer.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb b/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb index 1d9b267..24b6f06 100644 --- a/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb +++ b/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb @@ -3,10 +3,8 @@ class NationBuilderMemberSyncPushSerializer < ActiveModel::Serializer attributes :id, :nationbuilder_id, :email, :phone, :mobile, :first_name, :last_name def nationbuilder_id - member_external_ids = member_external_ids ? member_external_ids.with_system('nation_builder') : nil - member_external_id = member_external_ids ? member_external_ids.first : nil - nationbuilder_id = member_external_ids ? member_external_id.external_id : nil - nationbuilder_id + id = @object.member_external_ids.with_system(SYSTEM_NAME).first + id.external_id if id end def phone From 4c7b4ba13d0211c411b1648322c6c9ee5ce2dedd Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Thu, 24 Feb 2022 15:09:19 +1100 Subject: [PATCH 03/22] Bump nationbuilder-rb version --- Gemfile | 2 +- Gemfile.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 0f42868..4a27dce 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,7 @@ gem 'rails', '~> 6.1.0' gem 'pg' gem 'active_model_serializers' -gem 'nationbuilder-rb', '~> 1.6.1' +gem 'nationbuilder-rb', '~> 1.6.2' gem 'sidekiq' gem 'sidekiq-batch' gem 'sidekiq-limit_fetch' diff --git a/Gemfile.lock b/Gemfile.lock index 78e9463..cbd39c3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -258,7 +258,7 @@ DEPENDENCIES factory_bot_rails faker identity_nation_builder! - nationbuilder-rb (~> 1.6.1) + nationbuilder-rb (~> 1.6.2) pg phony pry From c88482273c240f9660752008778f3ee2e776c94a Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 27 Feb 2022 16:20:27 +1100 Subject: [PATCH 04/22] Stop using obsolete method --- app/models/identity_nation_builder/api.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/identity_nation_builder/api.rb b/app/models/identity_nation_builder/api.rb index 508cbf1..3736259 100644 --- a/app/models/identity_nation_builder/api.rb +++ b/app/models/identity_nation_builder/api.rb @@ -162,7 +162,7 @@ def self.add_people_list(list_id, member_ids) end def self.tag_list(list_id, tag) - api(:lists, :add_tag, { list_id: list_id, tag: URI.escape(tag) }) + api(:lists, :add_tag, { list_id: list_id, tag: URI.encode_www_form_component(tag) }) end def self.recruiters From e58fa90da8e8cf19c356aaf648ca4b5e1aa6e4a0 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 27 Feb 2022 16:23:03 +1100 Subject: [PATCH 05/22] Update API methods to yield results expected by pull/push impl --- app/models/identity_nation_builder/api.rb | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/app/models/identity_nation_builder/api.rb b/app/models/identity_nation_builder/api.rb index 3736259..b091894 100644 --- a/app/models/identity_nation_builder/api.rb +++ b/app/models/identity_nation_builder/api.rb @@ -4,16 +4,19 @@ module IdentityNationBuilder class API def self.rsvp(site_slug, members, event_id, mark_as_attended=false, recruiter_id=nil) member_ids = members.map do |member| + identity_id = member[:id] member = member.except(:id, :nationbuilder_id) person = find_or_create_person(member) response = rsvp_person(site_slug, event_id, person, mark_as_attended, recruiter_id) - if person && !response.try(:[], 'rsvp') && mark_as_attended + if !response.try(:[], 'rsvp') && mark_as_attended pager = NationBuilder::Paginator.new(get_api_client, event_rsvps(site_slug, event_id)) rsvp = pager.body['results'].select { |result| result['person_id'] == person['id'] }.first if rsvp && !rsvp['attended'] update_rsvp(site_slug, rsvp, mark_as_attended) end end + + { identity_id: identity_id, nationbuilder_id: person['id'] } end yield member_ids.length, member_ids end @@ -32,21 +35,25 @@ def self.tag(site_slug, members, tag) end def self.mark_as_attended_to_all_events_on_date(site_slug, members) - marked_records = 0 - member_ids = members.map { |member| member[:id] } - rsvps_on_date = EventRsvp.where(member_id: member_ids) + rsvps_on_date = EventRsvp.where(member_id: members.map { |member| member[:id] }) .joins(:event) .where('events.start_time::date = ?', Date.current) .where(attended: false) + member_ids = [] rsvps_on_date.each do |rsvp| begin update_rsvp(rsvp.event.data['site_slug'], rsvp.data, true) - marked_records += 1 + member_ids.append( + { + identity_id: rsvp.member.id, + nationbuilder_id: rsvp.data['person_id'] + } + ) rescue NationBuilder::ClientError => response raise unless response.message =~ /Record not found/i end end - yield marked_records, member_ids + yield member_ids.length, member_ids end def self.sites From ce9e7b882eeb6f8364808d0fae49c00abb9db3e7 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 27 Feb 2022 16:55:12 +1100 Subject: [PATCH 06/22] Cache recruiters in API, to match how events and sites get cached --- app/models/identity_nation_builder/api.rb | 1 + lib/identity_nation_builder.rb | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/identity_nation_builder/api.rb b/app/models/identity_nation_builder/api.rb index b091894..b2c4232 100644 --- a/app/models/identity_nation_builder/api.rb +++ b/app/models/identity_nation_builder/api.rb @@ -176,6 +176,7 @@ def self.recruiters recruiters = api(:people_tags, :people, { tag: 'recruiter' })['results'].map{|org| [ org['last_name'], org['id'] ] }.sort + Sidekiq.redis { |r| r.set 'nationbuilder:recruiters', recruiters.to_json} recruiters end diff --git a/lib/identity_nation_builder.rb b/lib/identity_nation_builder.rb index fd2be7a..97c8004 100644 --- a/lib/identity_nation_builder.rb +++ b/lib/identity_nation_builder.rb @@ -209,7 +209,6 @@ def self.fetch_recruiters(sync_id) end recruiters = IdentityNationBuilder::API.recruiters - Sidekiq.redis { |r| r.set 'nationbuilder:recruiters', recruiters.to_json} yield( recruiters.size, recruiters, From ef529b2b37484e2838fc2833a722d8e5b4571bc2 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 27 Feb 2022 17:04:53 +1100 Subject: [PATCH 07/22] Chase event campaign becoming a required field Assume a default id setting exists (this should be set via a corresponding `NATION_BUILDER_DEFAULT_EVENT_CAMPAIGN_ID` setting) and set that on new events if needed. --- lib/identity_nation_builder.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/identity_nation_builder.rb b/lib/identity_nation_builder.rb index 97c8004..262097a 100644 --- a/lib/identity_nation_builder.rb +++ b/lib/identity_nation_builder.rb @@ -126,6 +126,10 @@ def self.fetch_new_events(sync_id, over_period_of_time=1.week) external_id: nb_event["id"] ) + if event.campaign == nil + event.campaign = Campaign.find(Settings.nation_builder.default_event_campaign_id) + end + event.update!( name: nb_event['name'], start_time: nb_event['start_time'] && DateTime.parse(nb_event['start_time']), From 09ca785afed4746d91ffbba4ba50a2d245e212c2 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 27 Feb 2022 18:36:20 +1100 Subject: [PATCH 08/22] Chase Member.update_external_id API change --- lib/identity_nation_builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/identity_nation_builder.rb b/lib/identity_nation_builder.rb index 262097a..821b1da 100644 --- a/lib/identity_nation_builder.rb +++ b/lib/identity_nation_builder.rb @@ -34,7 +34,7 @@ def self.push_in_batches(sync_id, members, external_system_params) if sync_type === 'tag' member_ids.each do |member_id| member = Member.find(member_id[:identity_id]) - member.update_external_id(SYSTEM_NAME, member_id[:nationbuilder_id], {sync_id: sync_id}) if member + member.update_external_id(SYSTEM_NAME, member_id[:nationbuilder_id]) end end From d4bf3d10f65b7cc47de0c93545516083c3f49e4b Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 27 Feb 2022 17:10:38 +1100 Subject: [PATCH 09/22] Update unit tests --- Gemfile | 4 +- Gemfile.lock | 10 +- spec/lib/identity_nation_builder_pull_spec.rb | 18 + spec/lib/identity_nation_builder_push_spec.rb | 30 +- .../identity_nation_builder/api_spec.rb | 60 +- spec/rails_helper.rb | 10 +- ...uilder_member_sync_push_serializer_spec.rb | 8 +- spec/support/external_database.rb | 1 + spec/test_identity_app/.env.test.sample | 10 + spec/test_identity_app/app/lib/name_helper.rb | 28 + spec/test_identity_app/app/lib/settings.rb | 19 +- spec/test_identity_app/app/models/campaign.rb | 267 +++ spec/test_identity_app/app/models/event.rb | 55 +- .../app/models/event_rsvp.rb | 69 +- spec/test_identity_app/app/models/member.rb | 1506 +++++++++++++---- .../app/models/member_external_id.rb | 3 +- .../app/models/subscription.rb | 114 +- .../app/services/identity_base_service.rb | 7 + .../app/services/upsert_member.rb | 187 ++ .../config/initializers/sidekiq.rb | 14 + .../db/migrate/000_initial.rb | 33 +- spec/test_identity_app/db/schema.rb | 53 +- .../spec/factories/identity/campaign.rb | 5 + 23 files changed, 2114 insertions(+), 397 deletions(-) create mode 100644 spec/test_identity_app/app/lib/name_helper.rb create mode 100644 spec/test_identity_app/app/models/campaign.rb create mode 100644 spec/test_identity_app/app/services/identity_base_service.rb create mode 100644 spec/test_identity_app/app/services/upsert_member.rb create mode 100644 spec/test_identity_app/spec/factories/identity/campaign.rb diff --git a/Gemfile b/Gemfile index 4a27dce..587bce8 100644 --- a/Gemfile +++ b/Gemfile @@ -18,7 +18,8 @@ group :development, :test do gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] gem 'rspec-rails' gem 'rspec-mocks' - gem 'database_cleaner' + gem 'database_cleaner-active_record' + gem 'database_cleaner-redis' gem 'factory_bot_rails' gem 'rubocop', require: false gem 'pry' @@ -27,4 +28,5 @@ group :development, :test do gem 'spring-commands-rspec' gem 'webmock', require: true gem 'timecop' + gem 'zip' end diff --git a/Gemfile.lock b/Gemfile.lock index cbd39c3..eb48457 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -90,12 +90,13 @@ GEM crack (0.4.5) rexml crass (1.0.6) - database_cleaner (2.0.1) - database_cleaner-active_record (~> 2.0.0) database_cleaner-active_record (2.0.1) activerecord (>= 5.a) database_cleaner-core (~> 2.0.0) database_cleaner-core (2.0.1) + database_cleaner-redis (2.0.0) + database_cleaner-core (~> 2.0.0) + redis diff-lcs (1.5.0) dotenv (2.7.6) dotenv-rails (2.7.6) @@ -246,6 +247,7 @@ GEM websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) zeitwerk (2.5.4) + zip (2.0.2) PLATFORMS ruby @@ -253,7 +255,8 @@ PLATFORMS DEPENDENCIES active_model_serializers byebug - database_cleaner + database_cleaner-active_record + database_cleaner-redis dotenv-rails factory_bot_rails faker @@ -275,6 +278,7 @@ DEPENDENCIES spring-commands-rspec timecop webmock + zip BUNDLED WITH 2.2.32 diff --git a/spec/lib/identity_nation_builder_pull_spec.rb b/spec/lib/identity_nation_builder_pull_spec.rb index 9c74d71..75c8614 100644 --- a/spec/lib/identity_nation_builder_pull_spec.rb +++ b/spec/lib/identity_nation_builder_pull_spec.rb @@ -3,6 +3,7 @@ describe IdentityNationBuilder do context '#pull' do before(:each) do + allow(Settings).to receive_message_chain(:app, :inbound_url).and_return("https://example.com/") clean_external_database @sync_id = 1 @external_system_params = JSON.generate({'pull_job' => 'fetch_new_events'}) @@ -23,6 +24,7 @@ let!(:person_mobileonly_response){ JSON.parse(File.read("spec/fixtures/person_mobileonly_response.json")) } before(:each) do + allow(Settings).to receive_message_chain(:app, :inbound_url).and_return("https://example.com/") clean_external_database Settings.stub_chain(:options, :default_phone_country_code) { '61' } @@ -32,7 +34,12 @@ end context 'with SideKiq inline' do + let!(:campaign) { FactoryBot.create(:campaign) } + before(:each) do + allow(Settings).to receive_message_chain(:nation_builder, :default_event_campaign_id).and_return(campaign.id) + allow(Settings).to receive_message_chain(:options, :allow_subscribe_via_upsert_member).and_return(true) + allow(Settings).to receive_message_chain(:options, :default_member_opt_in_subscriptions).and_return(true) IdentityNationBuilder::API.stub_chain(:all_event_rsvps) { event_rsvp_response["results"] } IdentityNationBuilder::API.stub_chain(:person) { person_response["person"] } end @@ -87,7 +94,9 @@ context 'with an existing event that is in the time period but not returned by the api' do let!(:removed_event){ Event.create!( system: IdentityNationBuilder::SYSTEM_NAME, + name: "Existing event", subsystem: 'action', + campaign_id: campaign.id, start_time: Time.now, updated_at: 4.days.ago, id: 9999, @@ -127,7 +136,10 @@ end context 'with an event without an address' do + let!(:campaign) { FactoryBot.create(:campaign) } + it 'should use the event name as the location' do + allow(Settings).to receive_message_chain(:nation_builder, :default_event_campaign_id).and_return(campaign.id) allow(IdentityNationBuilder).to receive(:fetch_new_event_rsvps).and_return(event_rsvp_response) Sidekiq::Testing.fake! events_without_venue_address = events_response['results'] @@ -148,6 +160,7 @@ end before(:each) do + allow(Settings).to receive_message_chain(:app, :inbound_url).and_return("https://example.com/") clean_external_database IdentityNationBuilder::API.stub_chain(:all_event_rsvps) { event_rsvp_response["results"] } @@ -158,6 +171,11 @@ end it 'should record details of member who only has a mobile number' do + campaign = FactoryBot.create(:campaign) + allow(Settings).to receive_message_chain(:nation_builder, :default_event_campaign_id).and_return(campaign.id) + allow(Settings).to receive_message_chain(:options, :allow_subscribe_via_upsert_member).and_return(true) + allow(Settings).to receive_message_chain(:options, :default_member_opt_in_subscriptions).and_return(true) + mobile_last_three_digits = person_mobileonly_response['person']['mobile'].from(-3) IdentityNationBuilder.fetch_new_events(@sync_id) {} expect(Member.last).to have_attributes( diff --git a/spec/lib/identity_nation_builder_push_spec.rb b/spec/lib/identity_nation_builder_push_spec.rb index fc57afc..340670d 100644 --- a/spec/lib/identity_nation_builder_push_spec.rb +++ b/spec/lib/identity_nation_builder_push_spec.rb @@ -5,8 +5,9 @@ context 'with valid parameters' do context 'with rsvp' do it 'returns correct sync item type' do - external_system_params = {'sync_type' => 'rsvp', 'event_id' => 1, 'mark_as_attended' => true} - expect(IdentityNationBuilder.sync_type_item(external_system_params)).to eq([1, true]) + external_system_params = {'sync_type' => 'rsvp', 'event_id' => 1, 'mark_as_attended' => true, 'recruiter_id' => 2} + + expect(IdentityNationBuilder.sync_type_item(external_system_params)).to eq([1, true, 2]) end end context 'with tag' do @@ -72,9 +73,13 @@ context 'with valid parameters' do it 'yeilds write_result_count' do external_system_params = JSON.generate({'sync_type' => 'rsvp', 'event_id' => 1, 'mark_as_attended' => true}) - expect(IdentityNationBuilder::API).to receive(:rsvp).exactly(1).times.with(anything, anything, 1, true, nil) { 2 } + expect(IdentityNationBuilder::API).to receive(:rsvp).exactly(1).times.with(anything, anything, 1, true, nil).and_yield( + @members.length, [ + {identity_id: @members[0].id, nationbuilder_id: @members[0].id }, + {identity_id: @members[1].id, nationbuilder_id: @members[1].id }, + ]) IdentityNationBuilder.push_in_batches(1, @members, external_system_params) do |batch_index, write_result_count| - expect(write_result_count).to eq(2) + expect(write_result_count).to eq(@members.length) end end end @@ -83,9 +88,13 @@ context 'with valid parameters' do it 'yeilds write_result_count' do external_system_params = JSON.generate({'sync_type' => 'tag', 'tag' => 'test_tag'}) - expect(IdentityNationBuilder::API).to receive(:tag).exactly(1).times.with(anything, anything, anything) { 2 } + expect(IdentityNationBuilder::API).to receive(:tag).exactly(1).times.with(anything, anything, anything).and_yield( + @members.length, [ + {identity_id: @members[0].id, nationbuilder_id: @members[0].id }, + {identity_id: @members[1].id, nationbuilder_id: @members[1].id }, + ]) IdentityNationBuilder.push_in_batches(1, @members, external_system_params) do |batch_index, write_result_count| - expect(write_result_count).to eq(2) + expect(write_result_count).to eq(@members.length) end end end @@ -95,9 +104,14 @@ context 'with valid parameters' do it 'yeilds write_result_count' do external_system_params = JSON.generate({'sync_type' => 'mark_as_attended_to_all_events_on_date'}) - expect(IdentityNationBuilder::API).to receive(:mark_as_attended_to_all_events_on_date).exactly(1).times.with(nil, instance_of(@members.class)) { 2 } + expect(IdentityNationBuilder::API).to receive(:mark_as_attended_to_all_events_on_date).exactly(1).times.with(nil, instance_of(@members.class)) +.and_yield( + 3, [ + {identity_id: @members[0].id, nationbuilder_id: @members[0].id }, + {identity_id: @members[1].id, nationbuilder_id: @members[1].id }, + ]) IdentityNationBuilder.push_in_batches(1, @members, external_system_params) do |batch_index, write_result_count| - expect(write_result_count).to eq(2) + expect(write_result_count).to eq(3) end end end diff --git a/spec/models/identity_nation_builder/api_spec.rb b/spec/models/identity_nation_builder/api_spec.rb index f5c584d..a45dbda 100644 --- a/spec/models/identity_nation_builder/api_spec.rb +++ b/spec/models/identity_nation_builder/api_spec.rb @@ -174,10 +174,13 @@ end describe '.rsvp' do - let!(:member) { { id: 1, mobile: '04000000000'} } - let!(:event_id) { 1 } + let!(:identity_id) { 10 } + let!(:nationbuilder_id) { 20 } + let!(:event_id) { 30 } + let!(:recruiter_id) { 40 } + let!(:member) { { id: identity_id, mobile: '04000000000'} } + let!(:person) { { id: nationbuilder_id, mobile: '04000000000'} } let!(:mark_as_attended) { true } - let!(:recruiter_id) { 3 } context 'with no existing rsvp' do it 'should call the rsvp/create endpoint with attended set' do @@ -186,17 +189,20 @@ { status: 200, headers: { 'Content-Type' => 'application/json' }, - body: { person: member }.to_json + body: { person: person }.to_json } } - rsvp_request = stub_request(:post, %r{/sites/test/pages/events/1/rsvps}) - .with(body: hash_including(rsvp: { person_id: 1, attended: true, recruiter_id: recruiter_id})) + rsvp_request = stub_request(:post, %r{/sites/test/pages/events/30/rsvps}) + .with(body: hash_including(rsvp: { person_id: nationbuilder_id, attended: true, recruiter_id: recruiter_id})) .to_return({ status: 200, headers: { 'Content-Type' => 'application/json' }, - body: { rsvp: { person_id: 1, attended: true }}.to_json + body: { rsvp: { person_id: nationbuilder_id, attended: true }}.to_json }) - IdentityNationBuilder::API.rsvp('test', [member], event_id, mark_as_attended, recruiter_id) + IdentityNationBuilder::API.rsvp('test', [member], event_id, mark_as_attended, recruiter_id) { |length, members| + expect(length).to eq(1) + expect(members).to eq([{ identity_id: identity_id, nationbuilder_id: nationbuilder_id }]) + } expect(people_match_endpoint).to have_been_requested expect(rsvp_request).to have_been_requested end @@ -209,11 +215,11 @@ { status: 200, headers: { 'Content-Type' => 'application/json' }, - body: { person: member }.to_json + body: { person: person }.to_json } } - rsvp_create_request = stub_request(:post, %r{/sites/test/pages/events/1/rsvps}) - .with(body: hash_including(rsvp: { person_id: 1, attended: true})) + rsvp_create_request = stub_request(:post, %r{/sites/test/pages/events/30/rsvps}) + .with(body: hash_including(rsvp: { person_id: nationbuilder_id, attended: true})) .to_return({ status: 400, headers: { 'Content-Type' => 'application/json' }, @@ -223,23 +229,26 @@ "validation_errors": [ "signup_id has already been taken" ] }.to_json }) - rsvp_list_request = stub_request(:get, %r{pages/events/1/rsvps}) + rsvp_list_request = stub_request(:get, %r{pages/events/30/rsvps}) .to_return({ status: 200, headers: { 'Content-Type' => 'application/json' }, body: { - results: [{ id: 12222, event_id: event_id, person_id: 1, attended: false }] + results: [{ id: 12222, event_id: event_id, person_id: nationbuilder_id, attended: false }] }.to_json }) - rsvp_update_request = stub_request(:put, %r{pages/events/1/rsvps}) + rsvp_update_request = stub_request(:put, %r{pages/events/30/rsvps}) .to_return({ status: 200, headers: { 'Content-Type' => 'application/json' }, body: { - rsvp: { id: 12222, event_id: event_id, person_id: 1, attended: false } + rsvp: { id: 12222, event_id: event_id, person_id: nationbuilder_id, attended: false } }.to_json }) - IdentityNationBuilder::API.rsvp('test', [member], event_id, mark_as_attended) + IdentityNationBuilder::API.rsvp('test', [member], event_id, mark_as_attended) { |length, members| + expect(length).to eq(1) + expect(members).to eq([{ identity_id: identity_id, nationbuilder_id: nationbuilder_id }]) + } expect(people_match_endpoint).to have_been_requested expect(rsvp_create_request).to have_been_requested expect(rsvp_list_request).to have_been_requested @@ -252,12 +261,13 @@ let!(:nb_event_data) { { "id": 1, "event_id": 2, "person_id": 3 } } let!(:member) { FactoryBot.create(:member) } let!(:member_data) { { id: member.id } } - let!(:event) { Event.create!(external_id: 2, start_time: Time.now, data: { 'site_slug': 'test_slug' }) } + let!(:campaign) { Campaign.create!(name: "Test campaign") } + let!(:event) { Event.create!(name: "Event 1", campaign: campaign, external_id: 2, start_time: Time.now, data: { 'site_slug': 'test_slug' }) } let!(:event_rsvp) { EventRsvp.create!(event: event, member: member, attended: false, data: nb_event_data) } - let!(:old_event) { Event.create!(external_id: 2, start_time: 5.days.ago) } + let!(:old_event) { Event.create!(name: "Event 2", campaign: campaign, external_id: 2, start_time: 5.days.ago) } let!(:old_nb_event_data) { { "id": 8, "event_id": 9, "person_id": 3 } } let!(:old_rsvp) { EventRsvp.create!(event: old_event, member: member, attended: false, data: nb_event_data) } - let!(:future_event) { Event.create!(external_id: 2, start_time: 1.days.since) } + let!(:future_event) { Event.create!(name: "Event 3", campaign: campaign, external_id: 2, start_time: 1.days.since) } let!(:future_nb_event_data) { { "id": 12, "event_id": 19, "person_id": 3 } } let!(:future_rsvp) { EventRsvp.create!(event: future_event, member: member, attended: false, data: nb_event_data) } @@ -271,8 +281,10 @@ rsvp: { id: 1, event_id: 2, person_id: 3, attended: false } }.to_json }) - result = IdentityNationBuilder::API.mark_as_attended_to_all_events_on_date('test', [member_data]) - expect(result).to eq(1) + IdentityNationBuilder::API.mark_as_attended_to_all_events_on_date('test', [member_data]) { |length, members| + expect(length).to eq(1) + expect(members).to eq([{ identity_id: member.id, nationbuilder_id: 3 }]) + } expect(rsvp_update_request).to have_been_requested end @@ -286,8 +298,10 @@ "code":"not_found", "message":"Record not found" }.to_json }) - result = IdentityNationBuilder::API.mark_as_attended_to_all_events_on_date(nil, [member_data]) - expect(result).to eq(0) + IdentityNationBuilder::API.mark_as_attended_to_all_events_on_date(nil, [member_data]) { |length, members| + expect(length).to eq(0) + expect(members).to eq([]) + } expect(rsvp_update_request).to have_been_requested end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 9b492a5..a7afa86 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -53,13 +53,9 @@ config.fixture_path = "#{Rails.root}/spec/fixtures" config.before(:suite) do - # Speed up tests by using :transaction - DatabaseCleaner.strategy = :transaction - # And clean initially using :truncation - DatabaseCleaner.clean_with(:truncation) - # Enable redis cleaning too (:truncation is the only option) - DatabaseCleaner[:redis].strategy = :truncation - # And clean initially + DatabaseCleaner[:active_record].strategy = :transaction + DatabaseCleaner[:active_record].clean_with(:truncation) + DatabaseCleaner[:redis].strategy = :deletion DatabaseCleaner[:redis].clean end diff --git a/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb b/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb index 331450b..03ad5dc 100644 --- a/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb +++ b/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb @@ -22,8 +22,8 @@ ).as_json expect(rows.count).to eq(2) expect(rows[0][:email]).to eq(ListMember.first.member.email) - expect(rows[0][:phone]).to eq(ListMember.first.member.landline) - expect(rows[0][:mobile]).to eq(ListMember.first.member.mobile) + expect(rows[0][:phone]).to eq(ListMember.first.member.phone_numbers.landline.first.phone) + expect(rows[0][:mobile]).to eq(ListMember.first.member.phone_numbers.mobile.first.phone) expect(rows[0][:first_name]).to eq(ListMember.first.member.first_name) expect(rows[0][:last_name]).to eq(ListMember.first.member.last_name) end @@ -37,8 +37,8 @@ @batch_members, serializer: IdentityNationBuilder::NationBuilderMemberSyncPushSerializer ).as_json - expect(rows[0][:phone]).to eq(@member.landline.gsub(/^#{country_code}/, '0')) - expect(rows[0][:mobile]).to eq(@member.mobile.gsub(/^#{country_code}/, '0')) + expect(rows[0][:phone]).to eq(@member.phone_numbers.landline.first.phone.gsub(/^#{country_code}/, '0')) + expect(rows[0][:mobile]).to eq(@member.phone_numbers.mobile.first.phone.gsub(/^#{country_code}/, '0')) end end end diff --git a/spec/support/external_database.rb b/spec/support/external_database.rb index 9a18efa..845e3ff 100644 --- a/spec/support/external_database.rb +++ b/spec/support/external_database.rb @@ -15,6 +15,7 @@ def setup def clean MemberExternalId.all.destroy_all Event.all.destroy_all + Campaign.all.destroy_all PhoneNumber.all.destroy_all ListMember.all.destroy_all List.all.destroy_all diff --git a/spec/test_identity_app/.env.test.sample b/spec/test_identity_app/.env.test.sample index 1d17dc2..aede346 100644 --- a/spec/test_identity_app/.env.test.sample +++ b/spec/test_identity_app/.env.test.sample @@ -1,2 +1,12 @@ DATABASE_URL=postgres://localhost/identity_nation_builder_test_host +SIDEKIQ_REDIS_URL=redis://localhost:6379 + +NATION_BUILDER_SITE= +NATION_BUILDER_TOKEN= +NATION_BUILDER_DEBUG= +NATION_BUILDER_AUTHOR_ID= +NATION_BUILDER_PULL_ACTIVATED=true +NATION_BUILDER_PUSH_ACTIVATED=true +NATION_BUILDER_PULL_BATCH_AMOUNT=10 +NATION_BUILDER_PUSH_BATCH_AMOUNT=10 diff --git a/spec/test_identity_app/app/lib/name_helper.rb b/spec/test_identity_app/app/lib/name_helper.rb new file mode 100644 index 0000000..f1903e1 --- /dev/null +++ b/spec/test_identity_app/app/lib/name_helper.rb @@ -0,0 +1,28 @@ +module NameHelper + # to allow intelligent 'upserting' of names + def self.combine_names(old_name, new_name) + old_name = old_name.slice(:first_name, :middle_names, :last_name) + new_name = new_name.slice(:first_name, :middle_names, :last_name) + + is_new_name = false + combined_name = old_name + + new_name.each do |key, new_value| + new_value = new_value.to_s.strip + current_value = old_name[key].to_s.strip + if current_value.downcase.starts_with?(new_value.downcase) || new_value.downcase.starts_with?(current_value.downcase) + if new_value.length > current_value.length + combined_name[key.to_sym] = new_value + end + else + is_new_name = true + end + end + + if is_new_name + combined_name = new_name.select { |_k, v| v.present? } + end + + return { first_name: nil, middle_names: nil, last_name: nil }.merge(combined_name) + end +end diff --git a/spec/test_identity_app/app/lib/settings.rb b/spec/test_identity_app/app/lib/settings.rb index 50d4009..84bbe9a 100644 --- a/spec/test_identity_app/app/lib/settings.rb +++ b/spec/test_identity_app/app/lib/settings.rb @@ -8,6 +8,7 @@ def method_missing(method, *opts) end class Settings + def self.nation_builder return { "site_slug" => ENV['NATION_BUILDER_SITE_SLUG'], @@ -15,14 +16,28 @@ def self.nation_builder "token" => ENV['NATION_BUILDER_TOKEN'], "debug" => ENV['NATION_BUILDER_DEBUG'], "author_id" => ENV['NATION_BUILDER_AUTHOR_ID'], - "push_batch_amount" => nil, - "pull_batch_amount" => nil, + "default_event_campaign_id" => ENV['NATION_BUILDER_DEFAULT_EVENT_CAMPAIGN_ID'].to_i, + "push_batch_amount" => ENV['NATION_BUILDER_PULL_BATCH_AMOUNT'].to_i, + "pull_batch_amount" => ENV['NATION_BUILDER_PUSH_BATCH_AMOUNT'].to_i, } end + def self.app + return { } + end + def self.options return { "ignore_name_change_for_donation" => true } end + + def self.sidekiq_redis_url + return ENV['SIDEKIQ_REDIS_URL'] + end + + def self.sidekiq_redis_pool_size + return ENV['SIDEKIQ_REDIS_POOL_SIZE'] || 12 + end + end diff --git a/spec/test_identity_app/app/models/campaign.rb b/spec/test_identity_app/app/models/campaign.rb new file mode 100644 index 0000000..4e32b38 --- /dev/null +++ b/spec/test_identity_app/app/models/campaign.rb @@ -0,0 +1,267 @@ +# == Schema Information +# +# Table name: campaigns +# +# id :integer not null, primary key +# name :text +# created_at :datetime +# updated_at :datetime +# issue_id :integer +# description :text +# author_id :integer +# controlshift_campaign_id :integer +# campaign_type :text +# languages :string array: true +# latitude :float +# longitude :float +# location :text +# image :text +# url :text +# slug :text +# won :boolean +# + +class Campaign < ApplicationRecord + has_many :actions + belongs_to :issue, optional: true + belongs_to :author, class_name: 'Member', optional: true + + scope :unfinished, -> { + where(finished_at: nil) + } + + scope :finished, -> { + where.not(finished_at: nil) + } + + scope :recently_finished, -> { + where("finished_at > ?", 1.year.ago) + } + + scope :controlshift, -> { + where(campaign_type: 'controlshift') + } + scope :not_controlshift, -> { + where.not(id: controlshift) + } + + scope :unfinished_or_recently_finished, -> { + self.unfinished.or(self.recently_finished) + } + + def store_action_language(language) + unless languages.include?(language) + languages.push(language) + save! + end + end + + def self.load_from_csv(row) + campaign = Campaign.find_or_initialize_by(controlshift_campaign_id: row['id']) + + # upsert campaign + campaign.name = row['title'].to_s + campaign.description = row['what'].to_s + campaign.campaign_type = 'controlshift' + campaign.image = row['image_file_name'].to_s + campaign.latitude = row['location_latitude'].to_f + campaign.longitude = row['location_longitude'].to_f + campaign.location = row['location_locality'].to_s + campaign.slug = row['slug'].to_s + campaign.moderation_status = row['admin_status'].to_s + + if row['ended_type'].present? && campaign.outcome != row['ended_type'].to_s + campaign.finished_at = row['updated_at'].to_s + campaign.outcome = row['ended_type'].to_s + end + + if row['created_at_date'] + campaign.created_at = Date.strptime(row['created_at_date'], '%m/%d/%Y') + end + + if (issue_link = ControlshiftIssueLink.find_by(controlshift_tag: (row['categories'] || '').split(',').first)) + campaign.issue_id = issue_link.issue_id + end + + campaign.save! + + # If the petition isn't launched, the petition starter hasn't finished creating the petition yet, so no-one can see + # or sign the petition, and the petition starter may not have filled out their email address yet! + # Ideally we would ignore petitions until they're launched, but the Controlshift CSV data load only sends petition + # data when the row is 1st created, and during the nightly full data load. So this would mean not recording any + # signatures for a petition in ID until we get the CSV confirming it has been laucnhed, which will only happen + # overnight. Could this be improved by using Controlshift webhooks (there's a petition.launched hook)? + if row['launched'].in?(['true', 't']) + ControlshiftGetPetitionAuthorWorker.perform_async(campaign.id) + end + + # add group campaign if it's linked to a group + if row['group_id'].present? + GroupCampaign.find_or_create_by!( + group: Group.find_or_create_by!(controlshift_group_id: row['group_id']), + campaign_id: campaign.id, + ) + end + end + + class << self + def trending_petitions(count = 10, hours = 24) + hours = ApplicationRecord.connection.quote(hours) + count = ApplicationRecord.connection.quote(count) + sql = <<~SQL + SELECT a.id, a.public_name, a.campaign_id, a.external_id, a.technical_type, + c.slug, count(distinct ma.member_id) as participants + FROM member_actions ma + JOIN actions a + ON a.id = ma.action_id + JOIN campaigns c + ON c.id = a.campaign_id + WHERE ma.created_at >= current_timestamp - interval '#{hours} hours' + AND a.action_type='petition' + GROUP BY a.id, a.public_name, a.campaign_id, a.external_id, a.technical_type, c.slug + ORDER BY count(*) desc + LIMIT #{count} + SQL + RedshiftDB.connection.execute(sql) + end + + def latest_wins(count = 10, days = 365) + days = ApplicationRecord.connection.quote(days) + count = ApplicationRecord.connection.quote(count) + sql = <<~SQL + SELECT c.id, a.public_name, c.outcome, c.finished_at, + count(distinct ma.member_id) as participants + FROM campaigns c + JOIN actions a + ON a.campaign_id = c.id + JOIN member_actions ma + ON ma.action_id = a.id + WHERE c.outcome = 'won' + AND c.finished_at > current_timestamp - interval '#{days} days' + AND a.action_type != 'create-petition' + GROUP BY c.id, a.public_name, c.outcome, c.finished_at + ORDER BY c.finished_at DESC + LIMIT #{count} + SQL + RedshiftDB.connection.execute(sql) + end + + def biggest_recent_wins(count = 10, days = 365, minimum_size = 1000) + days = ApplicationRecord.connection.quote(days) + count = ApplicationRecord.connection.quote(count) + minimum_size = ApplicationRecord.connection.quote(minimum_size) + sql = <<~SQL + SELECT id, public_name, finished_at, participants FROM ( + SELECT c.id, a.public_name, c.outcome, c.finished_at, + count(distinct ma.member_id) as participants + FROM campaigns c + JOIN actions a + ON a.campaign_id = c.id + JOIN member_actions ma + ON ma.action_id = a.id + WHERE c.outcome = 'won' + AND c.finished_at > current_timestamp - interval '#{days} days' + GROUP BY c.id, a.public_name, c.outcome, c.finished_at + ) tmp + WHERE participants >= #{minimum_size} + ORDER BY participants DESC + LIMIT #{count} + SQL + RedshiftDB.connection.execute(sql) + end + + def campaign_url(action) + # Construct the URL for a campaign + campaign_url = Settings.app.home_url + if action['technical_type']&.match(/speakout/) + campaign_url = "#{Settings.speakout.url}/campaigns/#{action['slug']}" + elsif action['technical_type'] == 'cby_petition' || action['technical_type'] == 'csl_petition' + campaign_url = "#{Settings.controlshift_api.url}/petitions/#{action['slug']}" + end + campaign_url + end + + def readable_date(date) + # Transform date to readable format, e.g. "5th of April" + dt = DateTime.parse date.to_s + "#{dt.day.ordinalize} of #{dt.strftime '%B'}" + end + + def readable_name(name) + # Strip trailing punctuation that interferes with pacing of verbal output + name.sub!(/[\.\,\!\?]+$/, '') + # Change smart/weird quotes into normal quotes + name.gsub!(/[‘’`´]/, "'") + name.gsub(/[“”]/, '"') + end + + def readable_number(number) + # Round down numbers for easier listening comprehension + if number > 10_000 + number = number.to_s.sub(/\d\d\d$/, '000') + elsif number > 1_000 + number = number.to_s.sub(/\d\d$/, '00') + elsif number > 100 + number = number.to_s.sub(/\d$/, '0') + end + number.to_i + end + + def alexa_flash_briefing + alexa_items = Array.new + now = Time.current + + # Build a feed containing today's top 3 petitions + trending_petitions = trending_petitions(3, 24).to_a + trending_petitions.each_with_index do |petition, i| + participants = readable_number petition['participants'].to_i + petition_name = readable_name petition['public_name'] + redirection_url = campaign_url petition + trending_item = { + uid: "CAMPAIGN_#{petition['campaign_id']}", + titleText: petition_name, + mainText: "#{participants} new people have signed #{petition_name}", + updateDate: (now - i).utc.iso8601, # Alexa uses this timestamp as an 'order by' + redirectionUrl: redirection_url + } + alexa_items.push trending_item + end + + # Add details of latest campaign win to end of feed + latest_win = latest_wins(1, 90).first + if latest_win + win_id = latest_win['id'] + win_date = readable_date latest_win['finished_at'] + win_name = readable_name latest_win['public_name'] + winners = readable_number latest_win['participants'].to_i + win_item = { + uid: "WIN_#{win_id}", + titleText: win_name, + mainText: "Most recent campaign victory: #{win_name} was won on the #{win_date}, by over #{winners} people", + updateDate: (now - 4).utc.iso8601, + redirectionUrl: Settings.app.home_url # TODO: blog post? + } + alexa_items.push win_item + + # Add details of latest big campaign win to end of feed, if different from above + big_win = biggest_recent_wins(1, 90, 10000).first + if big_win && big_win.id.to_i != latest_win.id.to_i + big_win_id = big_win['id'] + big_win_date = readable_date big_win['finished_at'] + big_win_name = readable_name big_win['public_name'] + big_winners = readable_number big_win['participants'].to_i + big_win_item = { + uid: "BIG_WIN_#{big_win_id}", + titleText: big_win_name, + mainText: "Featured campaign victory: #{big_win_name} was won on the #{big_win_date}, by over #{big_winners} people", + updateDate: (now - 5).utc.iso8601, + redirectionUrl: Settings.app.home_url # TODO: blog post? + } + alexa_items.push big_win_item + end + end + + alexa_items + end + end +end diff --git a/spec/test_identity_app/app/models/event.rb b/spec/test_identity_app/app/models/event.rb index 61464f4..36a3154 100644 --- a/spec/test_identity_app/app/models/event.rb +++ b/spec/test_identity_app/app/models/event.rb @@ -1,11 +1,50 @@ +# == Schema Information +# +# Table name: events +# +# id :integer not null, primary key +# name :text +# start_time :datetime +# end_time :datetime +# description :text +# campaign_id :integer +# created_at :datetime +# updated_at :datetime +# host_id :integer +# controlshift_event_id :integer +# location :text +# latitude :float +# longitude :float +# attendees :integer +# group_id :integer +# area_id :integer +# image_url :text +# + class Event < ApplicationRecord - attr_accessor :audit_data + has_many :event_rsvps has_many :members, through: :event_rsvps + belongs_to :campaign + belongs_to :host, class_name: 'Member', optional: true + validates_presence_of :name + belongs_to :area, optional: true + + def rsvp_total + event_rsvps.length + end + + def rsvp_guests_total + event_rsvps.with_guests.sum { |rsvp| rsvp.data.guests_count } + end + + def rsvp_total_with_guests + rsvp_total + rsvp_guests_total + end def set_constituency if (nearest_zip = Postcode.nearest_postcode(latitude, longitude)) - constituency = Area.where(area_type: 'pcon_new').where(code: nearest_zip.pcon_new).first + constituency = Area.find_by!(area_type: 'pcon_new', code: nearest_zip.pcon_new) update!(area_id: constituency.id) end end @@ -36,7 +75,7 @@ def upsert(payload) } member_hash = member_hash.merge(event_payload[:host]) if event_payload[:host] - host = Member.upsert_member(member_hash) + host = UpsertMember.call(member_hash) event_payload[:host_id] = host.id end @@ -64,28 +103,28 @@ def remove_event(payload) def load_from_csv(row) if ( - member = Member.upsert_member( + member = UpsertMember.call( { external_ids: { controlshift: row['user_id'] } }, - "event_host:#{row['title']}" + entry_point: "event_host:#{row['title']}" ) ) # create event event = Event.find_or_initialize_by(controlshift_event_id: row['id']) event.name = row['title'] - event.start_time = row['start'] + event.start_time = row['start_at'] event.description = row['description'] event.host_id = member.id # is it linked to a local group? unless row['local_chapter_id'].nil? event.group_id = row['local_chapter_id'] - if (group = Group.where(controlshift_group_id: row['local_chapter_id']).first) + if (group = Group.find_by(controlshift_group_id: row['local_chapter_id'])) group.count_events end end # do we have a location for it? - if (location = Location.where(controlshift_location_id: row['location_id']).first) + if (location = Location.find_by(controlshift_location_id: row['location_id'])) event.location = location.description event.latitude = location.latitude event.longitude = location.longitude diff --git a/spec/test_identity_app/app/models/event_rsvp.rb b/spec/test_identity_app/app/models/event_rsvp.rb index 5382b57..8bd50a7 100644 --- a/spec/test_identity_app/app/models/event_rsvp.rb +++ b/spec/test_identity_app/app/models/event_rsvp.rb @@ -1,34 +1,71 @@ +# == Schema Information +# +# Table name: event_rsvps +# +# id :integer not null, primary key +# event_id :integer +# member_id :integer +# created_at :datetime +# updated_at :datetime +# deleted_at :datetime +# + class EventRsvp < ApplicationRecord - attr_accessor :audit_data + belongs_to :event belongs_to :member + scope :with_guests, -> { + where("(data ->> 'guests_count')::integer > 0") + } + class << self def create_rsvp(payload) - if ( - event = Event.find_by( - external_id: payload[:event][:external_id], - technical_type: payload[:event][:technical_type], + raise ArgumentError.new('No request payload') unless payload + + unless ( + event = ( + payload[:event][:id] ? + Event.find(payload[:event][:id]) : + Event.find_by(external_id: payload[:event][:external_id], technical_type: payload[:event][:technical_type]) ) ) + raise "RSVP failed to save because event #{payload[:event].inspect} doesn't exist" + end + + rsvp = payload[:rsvp] + + if rsvp.key?(:member_id) + unless (member = Member.find(payload[:rsvp][:member_id])) + raise "RSVP failed to save because the member #{payload[:rsvp][:member_id]} doesn't exist" + end + end + + if !member member_hash = { emails: [{ email: payload[:rsvp][:email] }], firstname: payload[:rsvp][:first_name], - lastname: payload[:rsvp][:last_name] + lastname: payload[:rsvp][:last_name], + addresses: payload[:rsvp][:addresses] ? payload[:rsvp][:addresses] : [], + custom_fields: payload[:rsvp][:custom_fields] ? payload[:rsvp][:custom_fields] : [] } - if (member = Member.upsert_member(member_hash)) - EventRsvp.create!(member: member, event: event) - else - logger.info "RSVP failed to save because the member for this RSVP doesn't exist and couldn't be created from the payload" - false + unless (member = UpsertMember.call(member_hash, entry_point: "event_rsvp_#{event.id}")) + raise "RSVP failed to save because the member #{member_hash[:email]} doesn't exist and couldn't be created from the payload" end - else - logger.info "RSVP failed to save because event #{payload[:event].inspect} doesn't exist" - false end + + unless (event_rsvp = EventRsvp.find_or_initialize_by(member: member, event: event)) + raise "RSVP could not be found or created #{payload}" + end + + if rsvp.key?(:attended) + event_rsvp.attended = payload[:rsvp][:attended] + end + + event_rsvp.save! end def remove_rsvp(payload) @@ -38,7 +75,7 @@ def remove_rsvp(payload) technical_type: payload[:event][:technical_type], ) ) - if (member = Member.find_by_email(payload[:rsvp][:email])) + if (member = Member.find_by(email: payload[:rsvp][:email])) EventRsvp.find_by(event_id: event.id, member_id: member.id).try(:destroy) end end @@ -46,7 +83,7 @@ def remove_rsvp(payload) def load_from_csv(row) if (event = Event.find_by(controlshift_event_id: row['event_id'])) - if (member = Member.upsert_member({ emails: [{ email: row['email'] }] }, "event_rsvp")) + if (member = UpsertMember.call({ emails: [{ email: row['email'] }] }, entry_point: "event_rsvp_#{event.id}")) event_rsvp = EventRsvp.find_or_initialize_by({ event_id: event.id, member_id: member.id diff --git a/spec/test_identity_app/app/models/member.rb b/spec/test_identity_app/app/models/member.rb index 5bb4709..2e3072b 100644 --- a/spec/test_identity_app/app/models/member.rb +++ b/spec/test_identity_app/app/models/member.rb @@ -1,22 +1,227 @@ +# == Schema Information +# +# Table name: members +# +# id :integer not null, primary key +# email :text +# contact :json +# created_at :datetime +# updated_at :datetime +# joined_at :datetime +# crypted_password :text +# guid :text +# action_history :json +# reset_token :text +# admin :boolean +# authy_id :integer +# volunteer :boolean +# role_id :integer +# last_donated :datetime +# donations_count :integer +# average_donation :float +# highest_donation :float +# mosaic_group :text +# mosaic_code :text +# entry_point :text +# + +require 'zip' + class Member < ApplicationRecord - include ReadWriteIdentity - attr_accessor :audit_data - has_and_belongs_to_many :areas, join_table: :area_memberships - has_many :addresses - has_many :custom_fields - has_many :phone_numbers + # relationships + belongs_to :role, optional: true has_many :list_members + has_many :lists, through: :list_members, dependent: :destroy + has_many :conditional_list_members + has_many :conditional_lists, through: :conditional_list_members, dependent: :destroy + has_many :event_rsvps + has_many :events, through: :event_rsvps + has_many :hosted_events, class_name: 'Event', foreign_key: 'host_id' + has_and_belongs_to_many :areas, join_table: :area_memberships + has_many :group_members + has_many :groups, through: :group_members + has_many :journey_coordinators + has_many :coordinated_journeys, through: :journey_coordinators, source: :journey + has_many :member_skills + has_many :skills, through: :member_skills + has_many :organisation_memberships + has_many :organisations, through: :organisation_memberships + has_many :member_resources + has_many :resources, through: :member_resources + has_many :member_actions + has_many :actions, through: :member_actions + has_many :create_petition_actions, -> { merge(Action.create_petition) }, source: :action, through: :member_actions + has_many :campaigns, through: :actions + has_many :issues, through: :campaigns + has_many :issue_categories, through: :issues + + has_many :member_action_consents, through: :member_actions + + has_many :member_mailings + has_many :mailings, through: :member_mailings + has_many :mailing_logs, class_name: 'Mailer::MailingLog' + has_one :members_on_ice + has_one :one_month_active + has_one :three_month_active + has_one :members_on_ice_exclusion + has_one :spam_exclude + has_one :member_open_click_rate + + has_many :smses, class_name: 'TextBlasts::SMS' + has_many :text_blasts, -> { order 'text_blasts.created_at DESC' }, through: :smses + + has_many :notification_endpoints, class_name: 'WebNotifications::NotificationEndpoint' + has_many :member_notifications, class_name: 'WebNotifications::MemberNotification' + has_many :notifications, through: :member_notifications + + has_many :donations, class_name: 'Donations::Donation' + has_many :regular_donations, class_name: 'Donations::RegularDonation' + has_many :member_journeys + has_many :journeys, through: :member_journeys + has_many :notes, -> { order 'notes.created_at DESC' } + has_many :notes_written, class_name: 'Note', foreign_key: 'user_id' + has_many :follow_ups + has_many :member_volunteer_tasks + has_many :volunteer_tasks, through: :member_volunteer_tasks + has_many :call_sessions + has_many :phone_numbers, -> { order 'phone_numbers.updated_at DESC' }, dependent: :destroy + has_many :addresses, -> { order 'addresses.updated_at DESC' }, dependent: :destroy has_many :member_subscriptions, dependent: :destroy + has_many :member_subscription_events, through: :member_subscriptions + has_many :subscribables, through: :member_subscription_events + has_many :subscriptions, through: :member_subscriptions + + has_many :custom_fields + has_many :custom_field_keys, through: :custom_fields + has_many :contacts_received, class_name: 'Contact', foreign_key: 'contactee_id' has_many :contacts_made, class_name: 'Contact', foreign_key: 'contactor_id' + has_many :contact_responses, through: :contacts_received + + has_many :member_demographic_groups, class_name: 'Demographics::MemberDemographicGroup' + has_many :demographic_groups, class_name: 'Demographics::DemographicGroup', through: :member_demographic_groups + has_many :member_external_ids + has_many :dedupe_blocks + + has_many :search_authorships, class_name: 'Search', foreign_key: 'author_id' + has_many :list_authorships, class_name: 'List', foreign_key: 'author_id' + has_many :sync_authorships, class_name: 'Sync', foreign_key: 'author_id' + has_many :flow_authorships, class_name: 'Flow', foreign_key: 'author_id' + + has_many :anonymization_logs + + scope :with_member_data, -> { + includes(:regular_donations, :donations, :addresses, :areas) + } + + scope :authors_matching, ->(authorship, query) { + joins(authorship) + .where("email ILIKE ? OR #{BY_NAME_WHERE_SQL}", "%#{query}%", "%#{query}%") + .uniq + } scope :with_email, -> { where.not(email: nil) } + scope :with_phone_numbers, -> { + joins(:phone_numbers) + } + + scope :with_phone_type, ->(phone_type) { + with_phone_numbers + .merge(PhoneNumber.send(phone_type)) + } + + scope :with_mobile, -> { + with_phone_numbers + .merge(PhoneNumber.mobile) + } + + scope :with_landline, -> { + with_phone_numbers + .merge(PhoneNumber.landline) + } + + scope :active_regular_donor, -> { + joins(:regular_donations).merge(Donations::RegularDonation.active) + } + + scope :not_active_regular_donor, -> { + where.not(id: active_regular_donor) + } + + scope :active_petition_starter, -> { + joins(member_actions: { action: :campaign }) + .merge(Action.create_petition) + .merge(Campaign.unfinished_or_recently_finished) + } + + scope :not_active_petition_starter, -> { + where.not(id: active_petition_starter) + } + + scope :can_be_ghosted, -> { + if Settings.ghoster.ghost_active_petition_starters + not_active_regular_donor + else + not_active_regular_donor.not_active_petition_starter + end + } + + scope :can_be_forcibly_ghosted, -> { + not_active_regular_donor + } + + scope :subscribed_to_sms, -> { + joins(:member_subscriptions).where(member_subscriptions: { + subscription_id: Subscription::SMS_SUBSCRIPTION.id, + unsubscribed_at: nil + }) + } + + scope :subscribed_to, ->(subscription_slug) { + joins(member_subscriptions: :subscription).where(member_subscriptions: { + subscriptions: { slug: subscription_slug }, + unsubscribed_at: nil + }) + } + + scope :admins, -> { + where.not(role: nil) + } + + attr_accessor :password + + validates_uniqueness_of :email, allow_blank: true, allow_nil: true, if: Proc.new { |m| m.new_record? || m.email_changed? }, message: 'is already taken. Click Sign In above to sign in.' + EMAIL_VALIDATION_REGEX = /\A[^@\s]+@[^@\s]+\.[^@\s]+\Z/i.freeze + + # This WHERE query part is indexed with a GIN index, + # so it should run pretty fast and its usage is encouraged. + sql = <<~SQL + TRIM(COALESCE(first_name, '') || ' ' || COALESCE(last_name, '')) ILIKE ? + SQL + BY_NAME_WHERE_SQL = sql.freeze + + validates_format_of :email, with: EMAIL_VALIDATION_REGEX, allow_nil: true + + before_save { |member| member.email = member.email.try(:downcase).try(:strip) } + validates_presence_of :password, if: :password_present + validates_length_of :password, within: 16..40, if: :password_present + + # Use `by_name` to do full text searches on the indexed full name. + scope :by_name, ->(name, search_type = nil) { + if search_type == :contains + name = "%#{name}%" + end + + where(BY_NAME_WHERE_SQL, name) + } + + # Getters def name [first_name, middle_names, last_name].select(&:present?).join(' ') end @@ -31,88 +236,337 @@ def name=(name) self.middle_names = array.join(' ') if array.present? end + def flattened_custom_fields + custom_fields.inject({}) do |memo, custom_field| + memo.merge({ :"#{custom_field.custom_field_key.name}" => custom_field.data }) + end + end + + def vocative_or_first_name + FirstName.find_by('lower(first_name) = lower(?)', self.first_name).try(:vocative) || self.first_name + end + + def address + return addresses.sort_by(&:updated_at).last unless addresses.empty? + end + + def postcode + address.try(:postcode) + end + def phone phone_numbers.sort_by(&:updated_at).last.phone unless phone_numbers.empty? end - def landline - phone_numbers - .landline - .sort_by(&:updated_at) - .last.try(:phone) + def mobile_if_can_be_detected_or_phone + mobile = phone_numbers.mobile.first + return mobile if mobile.present? + + phone_numbers.not_landline.first end - def mobile - phone_numbers - .mobile - .sort_by(&:updated_at) - .last.try(:phone) + def actions_count + actions.length end - def flattened_custom_fields - custom_fields.inject({}) do |memo, custom_field| - memo.merge({ :"#{custom_field.custom_field_key.name}" => custom_field.data }) - end + # Get the most recent changed consent per consent_text_id + def current_consents + # Memoize. The `1` is in the distinct query because + # eager_load will do a thing where it sets up all the columns, + # and rails won't produce a valid query unless there is a column + # after `DISTINCT ON (...)` + # Rails 6.1 "fixed" the eager_load method, no more strange `1` in the distinct query + @current_consents ||= MemberActionConsent.select('DISTINCT ON (member_action_consents.consent_text_id) member_action_consents.*') + .joins(:member_action) + .eager_load(:consent_text) + .where.not(consent_level: :no_change) + .where('member_actions.member_id': id) + .order(:consent_text_id) + .order('member_action_consents.created_at DESC') + .order('member_action_consents.id DESC') + .to_a # Do this to avoid errors on `count` end - # update phone number - def update_phone_number(new_phone_number, new_phone_type = nil, audit_data = nil) - new_phone_number = new_phone_number.to_s - unless phone_numbers.first.try(:phone) == new_phone_number - if (phone_record = phone_numbers.find_by(phone: new_phone_number)) - phone_record.audit_data = audit_data - phone_record.update!(updated_at: DateTime.now) + # update address + def update_address(new_address) + old_address_id = address.try(:id) + + address_attributes = { + line1: new_address[:line1] || new_address[:addr1], + line2: new_address[:line2] || new_address[:addr2], + town: new_address[:town] || new_address[:city], + postcode: new_address[:postcode] || new_address[:zip], + state: new_address[:state], + country: new_address[:country], + } + + # If this user already has the canonical address among their addresses, touch it to make it their most recent. Otherwise insert it. + if (canonical_address = CanonicalAddress.search(address_attributes)) + if (new_address = addresses.find_by(canonical_address: canonical_address)) + new_address.touch! else - phone_number_attributes = { member_id: id, phone: new_phone_number} - phone_number_attributes[:phone_type] = new_phone_type unless new_phone_type.nil? - phone_number = PhoneNumber.new(phone_number_attributes) - if phone_number.valid? - phone_number.audit_data = audit_data - phone_number.save! - else - Rails.logger.info "Phone number for #{id} not updated" - end + new_address = addresses.create!(address_attributes.merge(canonical_address: canonical_address)) + end + else + # If we can't match the address + if (new_address = addresses.find_by(address_attributes)) + new_address.touch! + else + new_address = addresses.create!(address_attributes) end - true end + + unless new_address.try(:id) == old_address_id + # update_address will be called for newly created members + # inside a transaction, so in order to reduce retries inside + # UpdateMemberAreasWorker we schedule it 5 seconds in the + # future. + # + # TODO: figure out if update_areas even needs to happen + # inside a worker. + UpdateMemberAreasWorker.perform_in(5.seconds, id) + return true + end + false end - def subscribe_to(subscription, reason = nil, subscribe_time = DateTime.now, audit_data = nil) - return update_subscription(subscription, true, subscribe_time, reason, nil, audit_data) + def update_phone_number(new_phone_number, new_phone_type = nil) + # The primary phone number is the one a member has most recently told us about. + # Do not update if they have already told us this phone number and it is already the most recent + # If they have told us about it in the past and it is not the primary one, then make it the primary one + # If it is a new phone number, create a new phone number and it will be the primary one + + new_phone_number = PhoneNumber.standardise_phone_number(new_phone_number.to_s) + return false if phone_numbers.first.try(:phone) == new_phone_number + + if (phone_record = phone_numbers.find_by(phone: new_phone_number)) + # Make it most recently updated it so it becomes the primary phone number + phone_record.update! updated_at: DateTime.now + return true + end + + phone_number_attributes = { member_id: id, phone: new_phone_number } + phone_number_attributes[:phone_type] = new_phone_type unless new_phone_type.nil? + phone_number = PhoneNumber.new(phone_number_attributes) + + if phone_number.valid? + phone_number.save + return true + else + Rails.logger.info "Phone number for member #{id} not updated" + return false + end end - def update_subscription(subscription, should_subscribe, event_time, reason = nil, unsub_mailing_id = nil, audit_data = nil) - retried = false + def update_external_id(system, external_id) + errors = 0 begin - # Don't subscribe / re-sub anyone who is permanently unsub'd - return false if self.unsubscribed_permanently? + member_external_ids.find_or_create_by!(system: system, external_id: external_id) + rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid + errors += 1 + retry if errors < 2 + + Rails.logger.error "Can't create member_external_id for member #{id} - would create a duplicate: #{system} #{external_id}" + raise + end + end + + def get_external_ids(system) + member_external_ids.where(system: system).map(&:external_id) + end + + def add_or_update_custom_field(custom_field_key, data) + custom_field = CustomField.find_or_create_by!( + member_id: id, custom_field_key: custom_field_key + ) + custom_field.update!(data: data) + end + + # Recalculate donation information + def recalculate_donation_statistics + total_donations = 0.0 + highest_donation = 0.0 + last_donation = nil + for donation in donations + total_donations += donation.amount + highest_donation = donation.amount if donation.amount > highest_donation + if last_donation.nil? || donation.created_at > last_donation + last_donation = donation.created_at + end + end + update!( + last_donated: last_donation, + donations_count: donations.length, + highest_donation: highest_donation, + average_donation: (!donations.empty? ? (total_donations / donations.length) : 0) + ) + end + + def activity_history + ( + audits + associated_audits + member_mailings + member_actions + member_action_consents + + smses + member_notifications + contacts_received + ).sort_by(&:created_at).reverse + end + + def has_regular_donation? + !regular_donations.where(ended_at: nil).empty? + end + + # Update the area memberships of member + def update_areas + if (canonical_address = address.try(:canonical_address)) + areas = canonical_address.areas + elsif (zip = Postcode.search(postcode)) + areas = AreaZip.where(zip: zip.zip).map(&:area) + if Settings.geography.area_lookup.use_open_north_api + api = OpenNorthAPI.new + area_codes = api.get_boundaries_for_lat_lng(zip.latitude, zip.longitude) + area_codes.each do |area_type, code| + areas << Area.find_by(area_type: area_type, code: code) + end + end + if Settings.geography.area_lookup.use_canadian_province_lookup + canadian_province_lookup = CanadianProvinceLookup.new + if (code = canadian_province_lookup.get_province_for_postcode(zip.zip)) + areas << Area.find_by(area_type: 'canada_province', code: code) + end + end + # TODO: Legacy mosaic code - delete once all orgs who use this have migrated to the newer, more flexible approach + mosaic = Mosaic::Mosaic.find_by(postcode: postcode.upcase.delete!(' ')) + end + + areas = (areas ||= []).uniq + self.areas.clear + self.areas << (areas || []) + + # TODO: Legacy mosaic code - delete once all orgs who use this have migrated to the newer, more flexible approach + if mosaic + self.mosaic_group = mosaic.mosaic_group + self.mosaic_code = mosaic.code + save! + end + + zip_source = zip || canonical_address + if zip_source + update_demographic_groups(zip_source.zip) + end + + lat_lng_source = canonical_address || zip + if lat_lng_source + self.latitude = lat_lng_source.latitude + self.longitude = lat_lng_source.longitude + save! + end + end + + def update_demographic_groups(zip_string) + if Demographics::DemographicTracking.permitted?(self, 'zip') + new_demographics = Demographics::DemographicGroup.in_zip(zip_string) + + # Delete old demographic this member is linked to via a zip (the new zip doesn't include these demographics) + self.member_demographic_groups.where( + linked_via: 'zip' + ).where.not( + demographic_group_id: new_demographics.map(&:id) + ).destroy_all + + # Ensure the member is linked to all the demographics for this zip + new_demographics.each do |dg| + self.member_demographic_groups.create_or_find_by!(demographic_group: dg, linked_via: 'zip') + end + else + # Demographic tracking via zip is not permitted for this member, delete any demographic data linked via zip + self.member_demographic_groups.where(linked_via: 'zip').destroy_all + end + end + + def subscribe + subscribe_to(Subscription::EMAIL_SUBSCRIPTION) unless subscribed? + end + + def subscribe_to(subscription, reason: nil, event_time: DateTime.now, subscribable: nil) + return update_subscription( + subscription, + should_subscribe: true, + event_time: event_time, + operation_reason: reason, + subscribable: subscribable, + ) + end + + def unsubscribe_from(subscription, reason: nil, event_time: DateTime.now, subscribable: nil, unsub_mailing_id: nil, permanent: false) + return update_subscription( + subscription, + should_subscribe: false, + event_time: event_time, + operation_reason: reason, + subscribable: subscribable, + unsub_mailing_id: unsub_mailing_id, + permanent: permanent + ) + end + + # update_subscription is intended to be a single method to update subscriptions, + # which correctly handles old updates by checking the sub/unsub event is newer + # than the last time the subscription was updated before processing. + # Returns true if the subscription was updated, false if not (ie. old event) + def update_subscription(subscription, should_subscribe:, event_time:, operation_reason: nil, subscribable: nil, unsub_mailing_id: nil, permanent: false) + retried = false + begin ms = self.member_subscriptions.find_or_initialize_by(subscription: subscription) do |member_sub| # Ensure new records have the time of this event member_sub.created_at = event_time member_sub.updated_at = event_time end + # Ensure record has attributes against subscribable and operation_reason + ms.subscribable = subscribable + ms.operation_reason = operation_reason + # Only process this event if it's newer than the previous sub/unsub event or it's a new subscription if event_time > ms.updated_at || ms.new_record? - ms.audit_data = audit_data - if should_subscribe && !ms.unsubscribed_permanently? + if unsubscribed_permanently? + return ms.update!( + subscribed_at: nil, + subscribe_reason: nil, + unsubscribed_at: event_time, + unsubscribe_reason: 'Deferred as permanently unsubscribed from email', + updated_at: event_time, + permanent: true + ) + elsif ms.unsubscribed_permanently? return ms.update!( + subscribed_at: nil, + subscribe_reason: nil, + unsubscribed_at: event_time, + unsubscribe_reason: 'Deferred as permanently unsubscribed', + updated_at: event_time, + permanent: true + ) + elsif should_subscribe + return ms.update!( + subscribed_at: event_time, + subscribe_reason: (operation_reason || 'not specified'), unsubscribed_at: nil, unsubscribe_reason: nil, - updated_at: event_time, - subscribe_reason: (reason || 'not specified'), + updated_at: event_time ) - elsif !should_subscribe && ms.unsubscribed_at.nil? + elsif !should_subscribe return ms.update!( + subscribed_at: nil, + subscribe_reason: nil, unsubscribed_at: event_time, - unsubscribe_reason: (reason || 'not specified'), + unsubscribe_reason: (operation_reason || 'not specified'), unsubscribe_mailing_id: unsub_mailing_id, updated_at: event_time, + permanent: permanent ) end end + return false rescue ActiveRecord::RecordNotUnique # Safe to always retry because there must be a DB-level unique constraint, @@ -131,354 +585,798 @@ def update_subscription(subscription, should_subscribe, event_time, reason = nil end end + def is_subscribed_to?(subscription) + member_subscriptions.where(subscription: subscription, unsubscribed_at: nil).exists? + end + + def permanently_unsubscribed_from?(subscription) + ms = member_subscriptions.find_by(subscription: subscription) + ms.present? && ms.unsubscribed_permanently? + end + + def unsubscribe + unsubscribe_from(Subscription::EMAIL_SUBSCRIPTION) if subscribed? + end + + def unsubscribe_permanently(reason: nil) + Subscription.defaults.find_each { |subscription| unsubscribe_from(subscription, permanent: true, reason: reason) } + end + + def subscribe_email + subscribe_to(Subscription::EMAIL_SUBSCRIPTION) + end + + def unsubscribe_email(permanent: false, reason: nil) + unsubscribe_from(Subscription::EMAIL_SUBSCRIPTION, permanent: permanent, reason: reason) + end + + def subscribe_notifications + subscribe_to(Subscription::NOTIFICATION_SUBSCRIPTION) + end + + def unsubscribe_notifications(permanent: false, reason: nil) + unsubscribe_from(Subscription::NOTIFICATION_SUBSCRIPTION, permanent: permanent, reason: reason) + end + + def subscribe_text_blasts + subscribe_to(Subscription::SMS_SUBSCRIPTION) + end + + def unsubscribe_text_blasts(permanent: false, reason: nil) + unsubscribe_from(Subscription::SMS_SUBSCRIPTION, permanent: permanent, reason: reason) + end + + def subscribe_calling + subscribe_to(Subscription::CALLING_SUBSCRIPTION) + end + + def unsubscribe_calling(permanent: false, reason: nil) + unsubscribe_from(Subscription::CALLING_SUBSCRIPTION, permanent: permanent, reason: reason) + end + + def subscribe_facebook + subscribe_to(Subscription::FACEBOOK_SUBSCRIPTION) + end + + def unsubscribe_facebook(permanent: false, reason: nil) + unsubscribe_from(Subscription::FACEBOOK_SUBSCRIPTION, permanent: permanent, reason: reason) + end + + def subscribed? + # For legacy purposes I'm mantaining that a member is subscribed if and only if it's subscribed to emails + # In the future we probably want to change this into: + # subscribed_to_emails? or subscribed_to_notifications? or subscribed_to_text_blasts + subscribed_to_emails? + end + + def subscribed_to_emails? + member_subscription = member_subscriptions.find_by(subscription: Subscription::EMAIL_SUBSCRIPTION) + member_subscription && member_subscription.unsubscribed_at.nil? + end + + def subscribed_to_notifications? + member_subscription = member_subscriptions.find_by(subscription: Subscription::NOTIFICATION_SUBSCRIPTION) + member_subscription && member_subscription.unsubscribed_at.nil? + end + + def subscribed_to_text_blasts? + member_subscription = member_subscriptions.find_by(subscription: Subscription::SMS_SUBSCRIPTION) + member_subscription && member_subscription.unsubscribed_at.nil? + end + def unsubscribed_permanently? - if member_subscription = member_subscriptions.find_by(subscription_id: Subscription::EMAIL_SUBSCRIPTION) + if (member_subscription = member_subscriptions.find_by(subscription: Subscription::EMAIL_SUBSCRIPTION)) return member_subscription.unsubscribed_permanently? else return false end end - def self.upsert_member(hash, entry_point = '', audit_data = {}, ignore_name_change = false, strict_member_id_match = false) - ApplicationRecord.transaction do - return upsert_member_raw(hash, entry_point, audit_data, ignore_name_change, strict_member_id_match) + # Merge another member record with this member record + def merge_other_records!(*other_members) + transaction do + # Write an audit + self.audits << Audited::Audit.new({ action: 'merge', comment: { current_member: self.audits.as_json, other_members: other_members.map { |m| m.audits.as_json } }.to_json }) + + ### Deal with special cases + # Subscriptions: If either member is subbed to a subscription, then make the merge subbed + subs = {} + member_subs = other_members.map { |m| m.member_subscriptions }.flatten + self.member_subscriptions + member_subs.each do |ms| + # Once any ms.unsubscribed_at is not present, the value will remain true + subs[ms.subscription_id] = subs[ms.subscription_id] || ms.unsubscribed_at.blank? + ms.destroy! + end + + reason = 'admin:merge_records' + subs.each do |subscription_id, subbed| + if subbed + self.subscribe_to(Subscription.find(subscription_id), reason: reason) + else + self.unsubscribe_from(Subscription.find(subscription_id), reason: reason) + end + end + + # Delete area_memberships + [self, *other_members].each { |m| m.areas.delete_all } + + # Call the catch-all merge method: + self.merge!(*other_members) + + # update areas + self.update_areas end end - def self.upsert_member_raw(hash, entry_point, audit_data, ignore_name_change, strict_member_id_match) - # fail if there's no data - if hash.nil? - Rails.logger.info hash - return nil - end + def self.irl + joins(:actions).where(actions: { action_type: Action.irl_types }) + end - # fail if there's no valid email address - member_id = hash[:member_id] - external_matched_members = if hash[:external_ids].present? - hash[:external_ids].map do |system, id| - Member.find_by_external_id(system, id) - end.compact.uniq - end - email = Cleanser.cleanse_email(hash.try(:[], :emails).try(:[], 0).try(:[], :email)) - phone = PhoneNumber.standardise_phone_number(hash.try(:[], :phones).try(:[], 0).try(:[], :phone)) - guid = hash[:guid] + def self.has_notes + joins(:notes) + end - # reject the email address if it's invalid - email = nil unless Cleanser.accept_email?(email) + # Administration and account + before_save :encrypt_password, if: :password_present + before_save :generate_guid - # then create with the passed entry point - # use rescue..retry to avoid errors where two Sidekiq processes try to insert different actions at the same time - member_created = false - begin - member = Member.find(member_id) if member_id.present? - if hash[:strict_member_id_match] && !member - raise Exception.new('Member upsert rejected: Strict member id match found no match') - end + def has_password?(password) + ::BCrypt::Password.new(crypted_password) == password + end - member = external_matched_members.first if !member && external_matched_members.present? && external_matched_members.length == 1 + def self.generate_password + SecureRandom.urlsafe_base64(12).tr('lIO0', 'sxyz') + end - unless member || email || phone || guid - Rails.logger.info('Rejected upsert for member because there was no email or phone or guid found') - return nil - end - member = Member.find_by(email: email) if !member && email.present? - if !hash[:ignore_phone_number_match] - member = Member.find_by_phone(phone) if !member && phone.present? - end - member = Member.find_by(guid: guid) if !member && guid.present? + def generate_reset_token + # Copied from Devise::friendly_token. + # Generate a cryptographically secure random string 16 chars in lenght without + # characters often confused. + # TODO: Additionally encrypt this token too, since otherwise we're effectively + # storing a password as plaintext + SecureRandom.urlsafe_base64(12).tr('lIO0', 'sxyz') + end - unless member - member = Member.create!(email: email, - entry_point: entry_point) - member_created = true - end - rescue ActiveRecord::RecordNotUnique - retry - end + def notify_made_admin + signup_link = 'click here' + register_body = "Hi there,

You've been made an administrator on the #{Settings.app.org_title} Identity platform. Please complete your registration here: #{signup_link}.

Thanks,
The #{Settings.app.org_title} team" + + Mailer::TransactionalMailWithDefaultReplyTo.send_email( + from: "#{Settings.app.org_title} <#{AppSetting.emails.member_from_email}>", + to: [email], + subject: "#{Settings.app.org_title} - You've been made an administrator on Identity", + body: register_body + ) + end + + def email_member_data_export(email, password) + data_archive_url = get_data_archive(password) + + @member = self + + default_path = Rails.root.join('gems/idlayout/app/views/idlayout/layouts/export.html.erb') + org_path = Rails.root.join("gems/idlayout/app/views/idlayout/layouts/#{Settings.app.org_name}/export.html.erb") + path = File.exist?(org_path) ? org_path : default_path + template = File.read(path) + + message_content = ERB.new(template).result_with_hash( + # These are the variables that are available in the exports template + data_archive_url: data_archive_url, + password: password, + ) + + Mailer::TransactionalMailWithDefaultReplyTo.send_email( + to: [email], + subject: 'Personal data request', + body: message_content, + from: "#{Settings.app.org_title} <#{AppSetting.emails.member_from_email}>", + source: 'identity:member-data-json' + ) + end - member.audit_data = audit_data + def can_be_ghosted? + Member.can_be_ghosted.where(id: self.id).exists? + end + + def can_be_forcibly_ghosted? + Member.can_be_forcibly_ghosted.where(id: self.id).exists? + end - if hash.key?(:external_ids) - hash[:external_ids].each do |system, external_id| - raise "External ID for #{system} cannot be blank" if external_id.blank? + def ghosting_started? + anonymization_logs.any? + end + + def ghosting_finished? + anonymization_logs.where.not(anonymization_finished_at: nil).any? + end - member.update_external_id(system, external_id, audit_data) + def ghost_member(admin_member_id:, force: false) + Ghoster.ghost_members_by_id( + [self.id], + reason: 'manual', + admin_member_id: admin_member_id, + force: force + ) + + true + end + + def upsert_default_subscriptions(upsert_hash, _entry_point) + if upsert_hash.key?(:subscriptions) + passed_subscriptions = upsert_hash[:subscriptions].map do |sh| + if (subscription = Subscription.find_by(id: sh[:id]) || Subscription.find_by(slug: sh[:slug])) + subscription + end end + else + passed_subscriptions = [] end - # Don't update further details if upsert data is older than member.updated_at - return member if !member_created && hash[:updated_at].present? && hash[:updated_at] < member.updated_at + subscriptions_to_process = Subscription.defaults - passed_subscriptions + subscriptions_to_process.each do |subscription| + subscribe_to(subscription, reason: "default:opt_in") + end + end - # Handle names - unless ignore_name_change - new_name = { - first_name: hash[:firstname], - middle_names: hash[:middlenames], - last_name: hash[:lastname] + # metaclass methods + class << self + # import_from_csv is for importing members using the page on the Identity UI + def import_from_csv(row, options = {}) + row = row.map { |key, value| [key.downcase, value] }.to_h + + member_hash = { + emails: [{ + email: row['email'] + }], + firstname: row['first_name'], + middlenames: row['middle_names'], + lastname: row['last_name'] } - old_name = { - first_name: member.first_name, - middle_names: member.middle_names, - last_name: member.last_name - } + phone_data = select_data(row, 'phone') - if hash.key?(:name) - firstname, lastname = hash[:name].split(' ') - new_name[:first_name] = firstname unless firstname.empty? - new_name[:last_name] = lastname unless lastname.empty? + unless phone_data.empty? + phones = [] + phone_data.each do |_key, value| + phones.push(phone: value) + end + member_hash[:phones] = phones end - member.update!(combine_names(old_name, new_name)) - end - if hash.key?(:custom_fields) - hash[:custom_fields].each do |custom_field_hash| - if custom_field_hash[:value].present? - custom_field_key = CustomFieldKey.find_or_initialize_by(name: custom_field_hash[:name]) - custom_field_key.audit_data = audit_data - custom_field_key.save! if custom_field_key.new_record? - member.add_or_update_custom_field(custom_field_key, custom_field_hash[:value], audit_data) + address_data = select_data(row, 'address_') + unless address_data.empty? + member_hash[:addresses] = [parse_data(address_data)] + end + + %w(skill resource organisation).each do |w| + named_attribute_data = select_data(row, w) + unless named_attribute_data.empty? + hash_key = "#{w}s".to_sym + member_hash[hash_key] = [] + named_attribute_data.each do |_key, value| + member_hash[hash_key] << { name: value } + end end end - end - # if there are phone numbers present, save them to the member - if hash.key?(:phones) && !hash[:phones].empty? - hash[:phones].each do |phone_number| - member.update_phone_number(phone_number[:phone], nil, audit_data) + custom_data = select_data(row, 'custom_') + unless custom_data.empty? + member_hash[:custom_fields] = parse_custom_data(custom_data) end - end - # if there are addresses present, save them to the member - if hash.key?(:addresses) && !hash[:addresses].empty? - address = hash[:addresses][0] - # Don't update with any address containing only empty strings - if address.except(:country).values.any?(&:present?) - member.update_address(address, audit_data) + member = UpsertMember.call(member_hash, entry_point: options.entry_point) + + member.subscribe if options['create_email_subscription'] + member.subscribe_text_blasts if options['create_text_subscription'] + + if options['add_to_list_id'] + ListMember.find_or_create_by!(member: member, list_id: options['list_id']) end + + member.id end - if hash.key?(:subscriptions) - hash[:subscriptions].each do |sh| - next unless ( - subscription = Subscription.find_by(id: sh[:id]) || Subscription.find_by(slug: sh[:slug]) - ) + # load_from_csv is for loading members from external services such as ControlShift + # ControlShift has a nightly full data load, including old data, so can't just upsert everything + def load_from_csv(row) + payload = { + emails: [{ email: row['email'] }], + firstname: row['first_name'], + lastname: row['last_name'], + external_ids: { controlshift: row['id'] }, + updated_at: row['updated_at'] + } + UpsertMember.call(payload) + end - case sh[:action] - when 'subscribe' - member.subscribe_to(subscription, sh[:reason], DateTime.now, audit_data) - when 'unsubscribe' - member.unsubscribe_from(subscription, sh[:reason], DateTime.now, nil, audit_data) - end + def select_data(rows, key_name) + rows.select do |key, value| + key.start_with?(key_name) && value.present? end end - if hash.key?(:skills) - hash[:skills].each do |s| - if (skill = Skill.where('name ILIKE ?', s[:name]).order(created_at: :desc).first) - begin - new_member_skill = MemberSkill.new(member: member, skill: skill, rating: s[:rating].try(:to_i), notes: s[:notes], audit_comment: audit_data) - new_member_skill.audit_data = audit_data - new_member_skill.save! - rescue ActiveRecord::RecordInvalid - # Skill already assigned, no action needed - end - end + def parse_data(rows) + hash = {} + rows.each do |key, value| + key = key.split('_').last.to_sym + hash[key] = value end + hash end - if hash.key?(:resources) - hash[:resources].each do |s| - if (resource = Resource.where('name ILIKE ?', s[:name]).order(created_at: :desc).first) - begin - new_member_resource = MemberResource.new(member: member, resource: resource, notes: s[:notes], audit_comment: audit_data) - new_member_resource.audit_data = audit_data - new_member_resource.save! - rescue ActiveRecord::RecordInvalid - # Resource already assigned, no action needed - end - end + def parse_custom_data(rows) + custom_fields = [] + rows.each do |key, value| + key = key.split('_').last + custom_fields.push({ name: key, value: value }) end + custom_fields end - if hash.key?(:organisations) - hash[:organisations].each do |s| - if (organisation = Organisation.where('name ILIKE ?', s[:name]).order(created_at: :desc).first) + def record_action(payload, _route) + # TODO: Post-May, we probably need a check BEFORE we store any personal info that + # this person has either already consented to any required terms, OR the action + # payload includes the relevant consent. If no consent is present, we COULD still + # store this action, but without any personal data (eg. empty name, email, etc...) + + begin + # find/create the member + cons_hash = payload[:cons_hash].merge(updated_at: payload[:create_dt]) + ignore_names = Settings.options.ignore_name_change_for_donation && ['donate', 'regular_donate'].include?(payload[:action_type]) + member = UpsertMember.call( + cons_hash, + entry_point: "action:#{payload[:action_name]}", + ignore_name_change: ignore_names + ) + if member.present? + # find/create the action begin - new_organisation_membership = OrganisationMembership.new(member: member, organisation: organisation, notes: s[:notes], audit_comment: audit_data) - new_organisation_membership.audit_data = audit_data - new_organisation_membership.save! - rescue ActiveRecord::RecordInvalid - # Organisation already assigned, no action needed + query = { technical_type: payload[:action_technical_type], + external_id: payload[:external_id] } + action = nil + + ### If no language specified in payload, find actions matching technical_type & external_id + if payload[:language].nil? + actions = Action.where(query) + if actions.length == 1 + action = actions.first # if action only exists in a single language (or no language: legacy data) + elsif actions.length > 1 + Rails.logger.error "The member action [member_id: #{member.id}, external_id: #{payload[:external_id]}, "\ + "technical_type: #{payload[:action_technical_type]}] contains no language code but"\ + "the action already exists in more than one language" + + # Still want to record an action, so first try to find a matching action with the default language + action = actions.select { |a| a.language == AppSetting.actions.default_language }[0] + # If 'action' is still nil here, then a new action with the default language will be created below + end + else + # prefer searching for an (old) action with (explicitly) no language over creating a new (duplicate) one with a language + action = Action.find_by(query.merge(language: payload[:language])) || Action.find_by(query.merge(language: nil)) + end + + # Create a new action if none found + action = Action.create!( + name: payload[:action_name], + public_name: payload[:action_public_name], + action_type: payload[:action_type], + technical_type: payload[:action_technical_type], + description: payload[:action_description] || '', + external_id: payload[:external_id], + language: payload[:language].presence || AppSetting.actions.default_language + ) unless action + rescue ActiveRecord::RecordNotUnique + retry end - end - end - end - member - end + # If the action's name has changed + if payload[:action_name].present? && payload[:action_name] != action.name + action.update!(name: payload[:action_name]) + end - def self.find_by_phone(phone) - PhoneNumber.find_by_phone(phone).try(:member) - end + # If the action's public name has changed + if payload[:action_public_name].present? && payload[:action_public_name] != action.public_name + action.update!(public_name: payload[:action_public_name]) + end + + # Assign the controlshift campaign if one isn't set + if !action.campaign && action.technical_type == 'cby_petition' + campaign = Campaign.find_by(controlshift_campaign_id: action.external_id, campaign_type: 'controlshift') + campaign.store_action_language(action.language) if campaign + action.update!(campaign_id: campaign.id) if campaign + end + + if payload[:campaign_id].present? && !action.campaign + # Allow external actions to pass a known identity campaign id and link the action + # to that campaign + campaign = Campaign.find_by(id: payload[:campaign_id]) + campaign.store_action_language(action.language) if campaign + action.update!(campaign_id: campaign.id) if campaign + end + + # create member action + if payload[:create_dt].presence.is_a? String + created_at = ActiveSupport::TimeZone.new('UTC').parse(payload[:create_dt]) + else + created_at = payload[:create_dt] + end + + member_action = MemberAction.find_or_initialize_by( + action_id: action.id, + member_id: member.id, + created_at: created_at + ) + new_record = member_action.new_record? + + # subscribe the member to mailings + # don't subscribe if disable_auto_subscribe is enabled (subscriptions must be handled through consents and post_consent_methods) + # only if action is newer than his unsubscribe; + # if opt_in is present, it must be set to true + if !Settings.gdpr.disable_auto_subscribe && (payload[:opt_in].nil? || payload[:opt_in]) && !member.subscribed? + email_subscription = member.member_subscriptions.find_by(subscription: Subscription::EMAIL_SUBSCRIPTION) + if email_subscription.nil? + member.subscribe + elsif member_action.created_at > email_subscription.unsubscribed_at + member.subscribe + end + end + + if member_action.valid? && payload[:source].present? + # store utm codes against the action + source_hash = payload[:source].slice(:source, :medium, :campaign).select { |_k, v| v.present? } - def self.combine_names(old_name, new_name) - old_name = old_name.slice(:first_name, :middle_names, :last_name) - new_name = new_name.slice(:first_name, :middle_names, :last_name) + if source_hash.present? + source = Source.find_or_create_with_defaults(source_hash) - is_new_name = false - combined_name = old_name + # This *must* be an update in order to allow requests which update an old member action's source + member_action.update!(source_id: source.id) + end + end - new_name.each do |key, new_value| - new_value = new_value.to_s.strip - current_value = old_name[key].to_s.strip - if current_value.downcase.starts_with?(new_value.downcase) || new_value.downcase.starts_with?(current_value.downcase) - if new_value.length > current_value.length - combined_name[key.to_sym] = new_value + if new_record && member_action.valid? + # add consents to the member action + if payload[:consents].present? + payload[:consents].each do |consent_hash| + next if consent_hash[:consent_level] == 'no_change' && !Settings.consent.record_no_change_consents + + consent_text = ConsentText.find_by!(public_id: consent_hash[:public_id]) + + member_action.member_action_consents.build( + member_action: member_action, + consent_text: consent_text, + consent_level: consent_hash[:consent_level], + consent_method: consent_hash[:consent_method], + consent_method_option: consent_hash[:consent_method_option], + parent_member_action_consent: nil, # TODO: Something like `member.current_consents.find_by(consent_public_id: consent_text.public_id).member_action_consent` but only if it's a 'no_change'... + created_at: payload[:create_dt], + updated_at: payload[:create_dt] + ) + end + end + + ApplicationRecord.transaction do + member_action.save! + + # split meta data into keys + if payload[:metadata] + payload[:metadata].each do |key, value| + # Get the key + action_key = ActionKey.find_or_create_by!(action: action, key: key.to_s) + # Allow nested data as metadata + if value.is_a?(Hash) || value.is_a?(Array) + value = value.to_json + end + MemberActionData.create!( + member_action_id: member_action.id, + action_key: action_key, + value: value + ) + end + end + + # parse survey responses + if payload[:survey_responses] + payload[:survey_responses].each do |sr| + action_key = ActionKey.find_or_create_by!(action: action, key: sr[:question][:text]) + + Question.find_or_create_by! action_key: action_key do |q| + q.question_type = sr[:question][:qtype] + end + + values = if sr[:answer].is_a? Array + sr[:answer] + else + [sr[:answer]] + end + + values.each do |value| + MemberActionData.create!( + member_action_id: member_action.id, + action_key: action_key, + value: value + ) + end + end + end + + # XXX old 38 data has nil created_at. Can't compare nil and date like this. + if member.created_at.present? && member.created_at > member_action.created_at + member.created_at = member_action.created_at + member.save! + end + end + else + Rails.logger.info "Duplicate member action: action #{member_action.action_id} for member #{member_action.member_id}" + return member_action + end + else + Rails.logger.info "Failed to upsert member. Hash: #{payload.inspect}" + return nil end - else - is_new_name = true + return member_action + rescue => e + raise e end end - if is_new_name - combined_name = new_name.select { |k, v| v.present? } + # Takes a phone number and returns member or nil + def find_by_phone(phone) + PhoneNumber.find_by_phone(phone).try(:member) end - return { first_name: nil, middle_names: nil, last_name: nil }.merge(combined_name) + def find_by_external_id(system, id) + MemberExternalId.find_by(system: system, external_id: id).try(:member) + end end - def self.find_by_external_id(system, id) - MemberExternalId.find_by(system: system, external_id: id).try(:member) + # Blanking GUID so it will be regenerated in `before_save` + def invalidate_guid! + self.guid = '' + self.save! end - def update_external_id(system, external_id, audit_data = nil) - errors = 0 - begin - new_member_external_id = member_external_ids.find_or_initialize_by(system: system, external_id: external_id) - new_member_external_id.audit_data = audit_data - new_member_external_id.save! - rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid - errors += 1 - retry if errors < 2 + private + + TABULATE_CONFIG = { + _root: { + heading: 'Basic Details', + fields: %w(title first_name middle_names last_name email mobile_phone gender created_at updated_at) + }, + phone_numbers: %w(type phone updated_at), + addresses: %w(line1 line2 town postcode country state updated_at), + areas: %w(name area_type), + demographic_groups: %w(demographic_class name), + regular_donations: %w(started_at ended_at frequency medium source initial_amount current_amount amount_last_changed_at ended updated_at), + donations: %w(amount medium external_source created_at), + subscriptions: %w(subscription_type created_at unsubscribed_at unsubscribe_reason updated_at), + subscription_logs: %w(operation unsubscribe_reason created_at), + subscription_events: %w(operation operation_reason created_at), + actions: %w(name action_type description source.source source.medium custom_data created_at), + mailings_received: { + heading: 'Emails Received', + fields: %w(created_at subject first_opened first_clicked), + sort_field: :created_at + }, + sms_received: { + heading: 'Text Messages', + fields: %w(created_at body inbound from_number to_number status clicked_at), + sort_field: :created_at + }, + notifications_received: { + heading: 'Push Notifications Received', + fields: %w(created_at title opened), + sort_field: :created_at + }, + hosted_events: %w(name description location latitude longitude created_at), + event_rsvps: %w(name created_at), + notes: %w(note_type text created_at), + custom_fields: %w(name data), + groups: %w(name group_type membership_type), + skills: %w(name updated_at), + resources: %w(name updated_at), + organisations: %w(name updated_at), + audit_logs: :all, + audits: %w(ip request_method path created_at), + consents: { + heading: 'GDPR Consents', + fields: %w(consent_short_text consent_level consent_method consent_method_option created_at) + }, + lists: { + heading: 'Internal Lists', + fields: %w(name created_at) + } + }.freeze - Rails.logger.error "Can't create member_external_id for member #{id} - would create a duplicate: #{system} #{external_id}" - raise - end + def encrypt_password + self.crypted_password = ::BCrypt::Password.create(password) end - def unsubscribe_from(subscription, reason = nil, unsubscribe_time = DateTime.now, unsub_mailing_id = nil, audit_data = nil) - return update_subscription(subscription, false, unsubscribe_time, reason, unsub_mailing_id, audit_data) + def password_present + password.present? end - def is_subscribed_to?(subscription) - !!self.member_subscriptions.find_by(subscription: subscription, unsubscribed_at: nil) + def generate_guid + self.guid = SecureRandom.hex(64) if guid.blank? end - # update address - def update_address(new_address) - old_address_id = address.try(:id) + def get_data_archive(password) + full_details, summary = generate_all_data - address_attributes = { - line1: new_address[:line1] || new_address[:addr1], - line2: new_address[:line2] || new_address[:addr2], - town: new_address[:town] || new_address[:city], - postcode: new_address[:postcode] || new_address[:zip], - state: new_address[:state], - country: new_address[:country], - } + zip_filename = "Data-Export-#{self.id}-#{Time.now.iso8601.to_s.delete(':')}.zip" - # If this user already has the canonical address among their addresses, touch it to make it their most recent. Otherwise insert it. - if canonical_address = CanonicalAddress.search(address_attributes) - if address = addresses.where(canonical_address: canonical_address).first - address.touch - else - addresses << Address.new(address_attributes.merge(canonical_address: canonical_address)) + zip_file = Zip::OutputStream.write_buffer(::StringIO.new(''), Zip::TraditionalEncrypter.new(password)) do |zip| + zip.put_next_entry("Details_#{self.name.tr(' ', '_')[0..29]}.json") + zip.write full_details.to_json + zip.put_next_entry("Summary_#{self.name.tr(' ', '_')[0..29]}.txt") + zip.write summary + end.string + + # Upload to S3 + obj = S3_BUCKET.object("member-export/#{SecureRandom.hex(16)}/#{zip_filename}") + obj.put(body: zip_file) + + obj.presigned_url(:get, expires_in: 3 * 24 * 60 * 60) + end + + def generate_all_data + member_data = self.as_json(except: [:id]).compact + + member_data[:phone_numbers] = self.phone_numbers.as_json(except: [:id, :member_id]) + + member_data[:addresses] = self.addresses.as_json(except: [:id, :member_id]).map(&:compact) + + member_data[:areas] = self.areas.as_json(except: [:id, :member_id]).map(&:compact) + + member_data[:demographic_groups] = self.demographic_groups.as_json(except: [:id, :member_id]) + + member_data[:regular_donations] = self.regular_donations.as_json(except: [:id, :member_id]).map(&:compact) + + member_data[:donations] = self.donations.as_json(except: [:id, :member_id]).map(&:compact) + + member_data[:subscriptions] = self.member_subscriptions.as_json(except: [:id, :member_id]).map do |sub| + sub[:subscription_type] = Subscription.find(sub["subscription_id"]).name + sub.as_json(except: ["subscription_id"]).compact + end + + # Old subscription tracking that has been removed from identity + subscription_logs = ActiveRecord::Base.connection.exec_query("SELECT operation, unsubscribe_reason, created_at FROM member_subscription_logs WHERE member_id = #{self.id}") + member_data[:subscription_logs] = subscription_logs.to_a + + # New subscription tracking + member_data[:subscription_events] = self.member_subscription_events + .as_json(only: [:operation, :operation_reason, :created_at]) + .map(&:compact) + + member_data[:actions] = self.member_actions.as_json(except: [:member_id]).map do |member_action| + action = Action.find(member_action["action_id"]) + member_action[:name] = action.name + member_action[:description] = action.description + member_action[:action_type] = action.action_type + + source = Source.find(member_action["source_id"]) if member_action["source_id"] + if source + member_action[:source] = { + campaign: source.campaign, + medium: source.medium, + source: source.source + }.compact end - else - # If we can't match the address - if address = addresses.where(address_attributes).first - address.touch - else - addresses << Address.new(address_attributes) + + action_data = MemberActionData.where(member_action_id: member_action["id"]) + if action_data + custom_data = action_data.map do |ad| + { + type: ad.action_key.key, + value: ad.value, + created_at: ad.created_at + } unless ad.value.nil? + end + member_action[:custom_data] = custom_data.compact end + + member_action.as_json(except: ["id", "action_id", "source_id"]).compact end - unless self.address.try(:id) == old_address_id - UpdateMemberAreasWorker.perform_async(id) - return true + member_data[:mailings_received] = self.member_mailings.map do |mm| + member_mailing = mm + + mm = mm.as_json(except: [:id, :member_id, :mailing_variation_id, :mailing_id]) + mm[:name] = member_mailing.mailing.name + mm[:subject] = member_mailing.mailing.subject + mm[:first_opened] = member_mailing.first_opened + mm[:first_clicked] = member_mailing.first_clicked + + mm[:opens] = member_mailing.opens.map do |open| + { + opened_at: open.created_at, + ip: open.ip_address, + user_agent: open.user_agent + }.compact + end + + mm[:clicks] = member_mailing.clicks.map do |click| + { + clicked_at: click.created_at, + ip: click.ip_address, + user_agent: click.user_agent, + link: click.mailing_link.try(:url) + }.compact + end + + mm.as_json.compact.reject { |_, v| v.is_a?(Array) && v.empty? } end - false - end + member_data[:sms_received] = self.smses.as_json(except: [:id, :member_id]).map(&:compact) - def address - return addresses.sort_by(&:updated_at).last unless addresses.empty? - end + member_data[:notifications_received] = self.member_notifications.as_json(except: [:id, :member_id]).map(&:compact) - # update address - def update_address(new_address) - old_address_id = address.try(:id) + member_data[:hosted_events] = self.hosted_events.as_json(except: [:id, :host_id, :external_id]).map(&:compact) - address_attributes = { - line1: new_address[:line1] || new_address[:addr1], - line2: new_address[:line2] || new_address[:addr2], - town: new_address[:town] || new_address[:city], - postcode: new_address[:postcode] || new_address[:zip], - state: new_address[:state], - country: new_address[:country], - } + member_data[:event_rsvps] = self.event_rsvps.as_json(except: [:id, :member_id]).map do |rsvp| + rsvp[:name] = Event.find(rsvp['event_id']).name + rsvp.as_json(except: ['event_id']).compact + end - # If this user already has the canonical address among their addresses, touch it to make it their most recent. Otherwise insert it. - if canonical_address = CanonicalAddress.search(address_attributes) - if address = addresses.where(canonical_address: canonical_address).first - address.touch - else - addresses << Address.new(address_attributes.merge(canonical_address: canonical_address)) - end - else - # If we can't match the address - if address = addresses.where(address_attributes).first - address.touch - else - addresses << Address.new(address_attributes) - end + member_data[:notes] = self.notes.as_json(except: [:id, :member_id, :user_id]).map do |note| + note[:note_type] = NoteType.find(note['note_type_id']).name + note.as_json(except: ['note_type_id']).compact end - unless self.address.try(:id) == old_address_id - UpdateMemberAreasWorker.perform_async(id) - return true + member_data[:custom_fields] = self.custom_fields.as_json(except: [:id, :member_id]).map do |cf| + cf[:name] = CustomFieldKey.find(cf['custom_field_key_id']).name + cf.as_json(except: ['custom_field_key_id']).compact end - false - end + member_data[:groups] = self.group_members.as_json(except: [:id, :member_id]).map do |group_member| + group = Group.find(group_member["group_id"]) + group_member[:name] = group.name + group_member[:group_type] = group.group_type + group_member.as_json(except: [:group_id]).compact + end - # Update the area memberships of member - def update_areas - if canonical_address = address.try(:canonical_address) - areas = canonical_address.areas - elsif zip = Postcode.search(postcode) - areas = AreaZip.where(zip: zip.zip).map(&:area) - mosaic = Mosaic.where(postcode: postcode.upcase.delete!(' ')).first + member_data[:skills] = self.member_skills.as_json(except: [:id, :member_id]).map do |member_skill| + member_skill[:name] = Skill.find(member_skill["skill_id"]).name + member_skill.as_json(except: ["skill_id"]).compact end - self.areas.clear - self.areas << (areas ||= []) + member_data[:resources] = self.member_resources.as_json(except: [:id, :member_id]).map do |member_resource| + member_resource[:name] = Resource.find(member_resource['resource_id']).name + member_resource.as_json(except: ['resource_id']).compact + end - if mosaic = (mosaic ||= nil) - self.mosaic_group = mosaic.mosaic_group - self.mosaic_code = mosaic.code - save + member_data[:organisations] = self.organisation_memberships.as_json(except: [:id, :member_id]).map do |membership| + membership[:name] = Organisation.find(membership['organisation_id']).name + membership.as_json(except: ['organisation_id']).compact end - if lat_lng_source = (canonical_address ||= nil) || (zip ||= nil) - self.latitude = lat_lng_source.latitude - self.longitude = lat_lng_source.longitude - save + audit_logs = (self.audits + self.associated_audits).map do |audit| + audit_log = { + type: audit.action, + changes: audit.audited_changes.compact, + remote_address: audit.remote_address, + created_at: audit.created_at + }.as_json.compact + + audit.audited_changes.empty? ? {} : audit_log end - end + member_data[:audit_logs] = audit_logs.reject { |audit_log| audit_log.empty? } - def postcode - address.try(:postcode) + member_data[:consents] = self.member_action_consents.as_json(except: [:id, :member_action_id]).map do |mac| + mac[:text] = ConsentText.find(mac["consent_text_id"]).consent_short_text + + mac.as_json.compact + end + + if Settings.member_data_export.export_lists + member_data[:lists] = self.lists.as_json(except: [:id, :synced_to_redshift, :member_count]).map(&:compact) + end + + member_data = member_data.compact.reject { |_, v| v.is_a?(Array) && v.empty? } + + member_summary = tabulate_all( + member_data, + TABULATE_CONFIG + ) + + [member_data, member_summary] end end diff --git a/spec/test_identity_app/app/models/member_external_id.rb b/spec/test_identity_app/app/models/member_external_id.rb index a7f5a85..8f22fb4 100644 --- a/spec/test_identity_app/app/models/member_external_id.rb +++ b/spec/test_identity_app/app/models/member_external_id.rb @@ -1,6 +1,4 @@ class MemberExternalId < ApplicationRecord - include ReadWriteIdentity - attr_accessor :audit_data belongs_to :member validates_presence_of :member @@ -9,4 +7,5 @@ class MemberExternalId < ApplicationRecord scope :with_system, ->(system) { where(system: system).order('updated_at DESC') } + end diff --git a/spec/test_identity_app/app/models/subscription.rb b/spec/test_identity_app/app/models/subscription.rb index 82e7737..3043ecb 100644 --- a/spec/test_identity_app/app/models/subscription.rb +++ b/spec/test_identity_app/app/models/subscription.rb @@ -1,7 +1,111 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: subscriptions +# +# id :integer not null, primary key +# name :text +# description :text +# deleted_at :datetime +# created_at :datetime +# updated_at :datetime +# + class Subscription < ApplicationRecord - include ReadWriteIdentity - EMAIL_SUBSCRIPTION = 1 - SMS_SUBSCRIPTION = 2 - NOTIFICATION_SUBSCRIPTION = 3 - CALLING_SUBSCRIPTION = 4 + # subscription ids for various channels + EMAIL_SLUG = 'default-email' + SMS_SLUG = 'default-sms' + NOTIFICATION_SLUG = 'default-notification' + FACEBOOK_SLUG = 'default-facebook' + CALLING_SLUG = 'default-calling' + POST_SLUG = 'default-post' + PROFILING_SLUG = 'default-profiling' + + EMAIL_SUBSCRIPTION = Subscription.find_or_create_by! slug: Subscription::EMAIL_SLUG do |sub| + sub.name = 'Email' + end + + UNSUBSCRIBE_EMAIL_URL = "#{Settings.app.inbound_url}/subscriptions/unsubscribe?subscription=#{Subscription::EMAIL_SUBSCRIPTION.id}".freeze + + SMS_SUBSCRIPTION = Subscription.find_or_create_by! slug: Subscription::SMS_SLUG do |sub| + sub.name = 'SMS' + end + + NOTIFICATION_SUBSCRIPTION = Subscription.find_or_create_by! slug: Subscription::NOTIFICATION_SLUG do |sub| + sub.name = 'Push Notification' + end + + FACEBOOK_SUBSCRIPTION = Subscription.find_or_create_by! slug: Subscription::FACEBOOK_SLUG do |sub| + sub.name = 'Facebook Audience' + end + + CALLING_SUBSCRIPTION = Subscription.find_or_create_by! slug: Subscription::CALLING_SLUG do |sub| + sub.name = 'Calling' + end + + POST_SUBSCRIPTION = Subscription.find_or_create_by! slug: Subscription::POST_SLUG do |sub| + sub.name = 'Post' + end + + # 'Profiling' is a broad term and can mean, eg. linking members to demographics, using predictive modelling + # or machine learning techniques to model future member behaviour based on past behaviour of your members, etc... + PROFILING_SUBSCRIPTION = Subscription.find_or_create_by! slug: Subscription::PROFILING_SLUG do |sub| + sub.name = 'Profiling' + end + + has_many :member_subscriptions + has_many :member_subscription_events, through: :member_subscriptions + has_many :subscribables, through: :member_subscription_events + has_many :members, through: :member_subscriptions + + scope :defaults, -> { + where( + id: [ + EMAIL_SUBSCRIPTION, SMS_SUBSCRIPTION, NOTIFICATION_SUBSCRIPTION, FACEBOOK_SUBSCRIPTION, + CALLING_SUBSCRIPTION, POST_SUBSCRIPTION, PROFILING_SUBSCRIPTION + ] + ) + } + + # Currently loads unsubs from Ctrlshift + def self.load_from_csv(row) + return if row['unsubscribe_organisation'].eql?('f') + + member = + begin + Member.find_or_create_by!(email: row['email'].downcase) do |m| + m.entry_point = 'ctrlshift_unsubscribe' + end + rescue ActiveRecord::RecordNotUnique + retry + end + + # see if this is newer than the subscription record we have + if ( + existing_subscription = MemberSubscription.find_by( + member_id: member.id, + subscription: Subscription::EMAIL_SUBSCRIPTION + ) + ) + return if existing_subscription.updated_at >= row['updated_at'] + end + + # unsubscribe if all is well + member.unsubscribe + end + + def icon + { + 'Main list' => 'envelope', + 'email' => 'envelope', + 'Email' => 'envelope', + 'notification' => 'bell', + 'Push Notification' => 'bell', + 'text' => 'phone', + 'SMS' => 'phone', + 'Calling' => 'earphone', + 'Facebook Audience' => 'user' + }[name] + end end diff --git a/spec/test_identity_app/app/services/identity_base_service.rb b/spec/test_identity_app/app/services/identity_base_service.rb new file mode 100644 index 0000000..18286f3 --- /dev/null +++ b/spec/test_identity_app/app/services/identity_base_service.rb @@ -0,0 +1,7 @@ +class IdentityBaseService + # Factory stuff to make calling services easier. + + def self.call(...) + new(...).call + end +end diff --git a/spec/test_identity_app/app/services/upsert_member.rb b/spec/test_identity_app/app/services/upsert_member.rb new file mode 100644 index 0000000..f78e9a7 --- /dev/null +++ b/spec/test_identity_app/app/services/upsert_member.rb @@ -0,0 +1,187 @@ +class UpsertMember < IdentityBaseService + def initialize(payload, entry_point: '', ignore_name_change: false) + @payload = payload + @entry_point = entry_point + @ignore_name_change = ignore_name_change + @retries = 0 + end + + def call + ApplicationRecord.transaction do + # fail if there's no data + next if payload.blank? + + member, member_created = find_or_create_member(payload) + + next if member.blank? + + upsert_external_ids(payload[:external_ids], member) if payload.key?(:external_ids) + + # Don't update further details if upsert data is older than member.updated_at + next member if !member_created && payload[:updated_at].present? && payload[:updated_at] < member.updated_at + + upsert_names(payload, member) if !ignore_name_change || member_created + upsert_custom_fields(payload[:custom_fields], member) if payload.key?(:custom_fields) + upsert_phone_numbers(payload[:phones], member) if payload[:phones].present? + upsert_addresses(payload[:addresses], member) if payload[:addresses].present? + + if Settings.options.allow_subscribe_via_upsert_member + upsert_subscriptions(payload[:subscriptions], member) if payload.key?(:subscriptions) + + if member_created && Settings.options.default_member_opt_in_subscriptions + member.upsert_default_subscriptions(payload, entry_point) + end + end + + upsert_skills(payload[:skills], member) if payload.key?(:skills) + upsert_resources(payload[:resources], member) if payload.key?(:resources) + upsert_organisation(payload[:organisations], member) if payload.key?(:organisations) + + member + end + rescue StandardError + # Possibility of race conditions in the transaction. + # Most likely from different CSL imports trying to upsert the same member. + # Transaction will clean up after itself, and then retry again. + # Have a limit of 3 retries so that we don't retry forever. + @retries += 1 + retry if @retries < 3 + raise + end + + private + + attr_reader :payload, :entry_point, :ignore_name_change + + def find_or_create_member(payload) + external_matched_members = payload[:external_ids]&.map do |system, id| + Member.find_by_external_id(system, id) + end&.compact&.uniq + + email = Cleanser.cleanse_email(payload.try(:[], :emails).try(:[], 0).try(:[], :email)) + phone = PhoneNumber.standardise_phone_number(payload.try(:[], :phones).try(:[], 0).try(:[], :phone)) + guid = payload[:guid] + + email = nil unless Cleanser.accept_email?(email) + + member_created = false + member = external_matched_members.first if external_matched_members.present? && external_matched_members.length == 1 + + unless member || email || phone || guid + Rails.logger.info('Rejected upsert for member because there was no email or phone or guid found') + return nil, false + end + + member = Member.find_by(email: email) if !member && email.present? + if !payload[:ignore_phone_number_match] + member = Member.find_by_phone(phone) if !member && phone.present? + end + member = Member.find_by(guid: guid) if !member && guid.present? + + unless member + member = Member.create!(email: email, entry_point: entry_point) + member_created = true + end + + [member, member_created] + end + + def upsert_external_ids(external_ids, member) + external_ids.each do |system, external_id| + raise "External ID for #{system} cannot be blank" if external_id.blank? + + member.update_external_id(system, external_id) + end + end + + def upsert_custom_fields(custom_fields, member) + custom_fields.each do |custom_field_hash| + if custom_field_hash[:value].present? + custom_field_key = CustomFieldKey.find_or_create_by!(name: custom_field_hash[:name]) + member.add_or_update_custom_field(custom_field_key, custom_field_hash[:value]) + end + end + end + + def upsert_names(payload, member) + new_name = { + first_name: payload[:firstname], + middle_names: payload[:middlenames], + last_name: payload[:lastname] + } + + old_name = { + first_name: member.first_name, + middle_names: member.middle_names, + last_name: member.last_name + } + + if payload.key?(:name) + firstname, lastname = payload[:name].split(' ') + new_name[:first_name] = firstname if firstname.present? + new_name[:last_name] = lastname if lastname.present? + end + + member.update!(NameHelper.combine_names(old_name, new_name)) + end + + def upsert_phone_numbers(phone_numbers, member) + phone_numbers.each do |phone_number| + member.update_phone_number(phone_number[:phone]) + end + end + + def upsert_addresses(addresses, member) + address = addresses.first + # Don't update with any address containing only empty strings + if address.except(:country).values.any?(&:present?) + member.update_address(address) + end + end + + def upsert_subscriptions(subscriptions, member) + subscriptions.each do |s| + subscription = Subscription.find_by(id: s[:id]) || Subscription.find_by(slug: s[:slug]) + + if subscription.blank? + if Settings.options.allow_upsert_create_subscriptions && s[:create].eql?(true) + subscription = Subscription.create!(name: s[:name], slug: s[:slug]) + else + Rails.logger.error "Subscription not found #{s[:slug]}" + next + end + end + + case s[:action] + when 'subscribe' + member.subscribe_to(subscription, reason: s[:reason]) + when 'unsubscribe' + member.unsubscribe_from(subscription, reason: s[:reason]) + end + end + end + + def upsert_skills(skills, member) + skills.each do |s| + if (skill = Skill.where('name ILIKE ?', s[:name]).order(created_at: :desc).first) + MemberSkill.create_with(rating: s[:rating].try(:to_i)).find_or_create_by!(member: member, skill: skill, notes: s[:notes]) + end + end + end + + def upsert_resources(resources, member) + resources.each do |r| + if (resource = Resource.where('name ILIKE ?', r[:name]).order(created_at: :desc).first) + MemberResource.find_or_create_by!(member: member, resource: resource, notes: r[:notes]) + end + end + end + + def upsert_organisation(organisations, member) + organisations.each do |o| + if (organisation = Organisation.where('name ILIKE ?', o[:name]).order(created_at: :desc).first) + OrganisationMembership.find_or_create_by!(member: member, organisation: organisation, notes: o[:notes]) + end + end + end +end diff --git a/spec/test_identity_app/config/initializers/sidekiq.rb b/spec/test_identity_app/config/initializers/sidekiq.rb index 6507b04..fb19ac4 100644 --- a/spec/test_identity_app/config/initializers/sidekiq.rb +++ b/spec/test_identity_app/config/initializers/sidekiq.rb @@ -2,3 +2,17 @@ require 'sidekiq/middleware/i18n' Sidekiq::Extensions.enable_delay! + +Sidekiq.configure_server do |config| + config.redis = { + url: Settings.sidekiq_redis_url, + size: Settings.sidekiq_redis_pool_size + } +end + +Sidekiq.configure_client do |config| + config.redis = { + url: Settings.sidekiq_redis_url, + size: Settings.sidekiq_redis_pool_size + } +end diff --git a/spec/test_identity_app/db/migrate/000_initial.rb b/spec/test_identity_app/db/migrate/000_initial.rb index a5cf398..cdd4d26 100644 --- a/spec/test_identity_app/db/migrate/000_initial.rb +++ b/spec/test_identity_app/db/migrate/000_initial.rb @@ -13,6 +13,7 @@ class Initial < ActiveRecord::Migration[4.2] t.datetime "updated_at" t.datetime "deleted_at" t.boolean "attended" + t.json "data", default: "{}" t.index ["event_id"], name: "index_event_rsvps_on_event_id" t.index ["member_id"], name: "index_event_rsvps_on_member_id" end @@ -41,6 +42,7 @@ class Initial < ActiveRecord::Migration[4.2] t.text "technical_type" t.string "system" t.string "subsystem" + t.json "data", default: "{}" t.index ["area_id"], name: "index_events_on_area_id" t.index ["campaign_id"], name: "index_events_on_campaign_id" t.index ["external_id", "system", "subsystem"], name: "index_events_on_system", unique: true @@ -112,6 +114,30 @@ class Initial < ActiveRecord::Migration[4.2] t.datetime "updated_at" end + create_table "campaigns", force: :cascade do |t| + t.text "name" + t.datetime "created_at" + t.datetime "updated_at" + t.integer "issue_id" + t.text "description" + t.integer "author_id" + t.integer "controlshift_campaign_id" + t.text "campaign_type" + t.float "latitude" + t.float "longitude" + t.text "location" + t.text "image" + t.text "url" + t.text "slug" + t.text "moderation_status" + t.datetime "finished_at" + t.string "target_type" + t.string "outcome" + t.string "languages", default: [], array: true + t.index ["author_id"], name: "index_campaigns_on_author_id" + t.index ["issue_id"], name: "index_campaigns_on_issue_id" + end + create_table "list_members", id: :serial, force: :cascade do |t| t.integer "list_id", null: false t.integer "member_id", null: false @@ -133,7 +159,6 @@ class Initial < ActiveRecord::Migration[4.2] t.index ["synced_to_redshift"], name: "index_lists_on_synced_to_redshift" end - #?? create_table "member_external_ids", id: :serial, force: :cascade do |t| t.integer "member_id", null: false t.string "system", null: false @@ -151,13 +176,15 @@ class Initial < ActiveRecord::Migration[4.2] t.datetime "created_at" t.datetime "updated_at" t.text "unsubscribe_reason" - t.text "subscribe_reason" t.boolean "permanent" t.integer "unsubscribe_mailing_id" + t.string "subscribe_reason" + t.datetime "subscribed_at" t.index ["member_id", "subscription_id"], name: "index_member_subscriptions_on_member_id_and_subscription_id", unique: true t.index ["member_id"], name: "index_member_subscriptions_on_member_id" t.index ["subscription_id"], name: "index_member_subscriptions_on_subscription_id" t.index ["unsubscribe_mailing_id"], name: "index_member_subscriptions_on_unsubscribe_mailing_id" + t.index ["unsubscribed_at"], name: "index_member_subscriptions_on_unsubscribed_at" end create_table "members", force: :cascade do |t| @@ -272,6 +299,8 @@ class Initial < ActiveRecord::Migration[4.2] t.datetime "created_at" t.datetime "updated_at" t.integer "member_count", default: 0 + t.string "slug", null: false + t.index ["slug"], name: "index_subscriptions_on_slug", unique: true end create_table "custom_field_keys", id: :serial, force: :cascade do |t| diff --git a/spec/test_identity_app/db/schema.rb b/spec/test_identity_app/db/schema.rb index 8b67b33..e2dbae6 100644 --- a/spec/test_identity_app/db/schema.rb +++ b/spec/test_identity_app/db/schema.rb @@ -2,22 +2,22 @@ # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# Note that this schema.rb definition is the authoritative source for your -# database schema. If you need to create the application database on another -# system, you should be using db:schema:load, not running all the migrations -# from scratch. The latter is a flawed and unsustainable approach (the more migrations -# you'll amass, the slower it'll run and the greater likelihood for issues). +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. # # It's strongly recommended that you check this file into your version control system. ActiveRecord::Schema.define(version: 0) do # These are extensions that must be enabled in order to support this database - enable_extension "plpgsql" - enable_extension "pg_trgm" enable_extension "btree_gin" enable_extension "btree_gist" enable_extension "intarray" + enable_extension "pg_trgm" + enable_extension "plpgsql" create_table "addresses", force: :cascade do |t| t.integer "member_id", null: false @@ -67,6 +67,30 @@ t.text "representative_gender" end + create_table "campaigns", force: :cascade do |t| + t.text "name" + t.datetime "created_at" + t.datetime "updated_at" + t.integer "issue_id" + t.text "description" + t.integer "author_id" + t.integer "controlshift_campaign_id" + t.text "campaign_type" + t.float "latitude" + t.float "longitude" + t.text "location" + t.text "image" + t.text "url" + t.text "slug" + t.text "moderation_status" + t.datetime "finished_at" + t.string "target_type" + t.string "outcome" + t.string "languages", default: [], array: true + t.index ["author_id"], name: "index_campaigns_on_author_id" + t.index ["issue_id"], name: "index_campaigns_on_issue_id" + end + create_table "canonical_addresses", id: :serial, force: :cascade do |t| t.string "official_id" t.text "line1" @@ -80,9 +104,9 @@ t.text "search_text" t.datetime "created_at" t.datetime "updated_at" - t.index "search_text gist_trgm_ops", name: "canonical_addresses_search_text", using: :gist t.index ["official_id"], name: "index_canonical_addresses_on_official_id" t.index ["postcode"], name: "index_canonical_addresses_on_postcode" + t.index ["search_text"], name: "canonical_addresses_search_text", opclass: :gist_trgm_ops, using: :gist end create_table "contact_campaigns", id: :serial, force: :cascade do |t| @@ -125,6 +149,7 @@ t.datetime "happened_at" t.datetime "created_at" t.datetime "updated_at" + t.json "data", default: "{}" t.index ["contact_campaign_id"], name: "index_contacts_on_contact_campaign_id" t.index ["contact_type"], name: "index_contacts_on_contact_type" t.index ["contactee_id"], name: "index_contacts_on_contactee_id" @@ -173,7 +198,7 @@ t.datetime "updated_at" t.datetime "deleted_at" t.boolean "attended" - t.json 'data', default: '{}' + t.json "data", default: "{}" t.index ["event_id"], name: "index_event_rsvps_on_event_id" t.index ["member_id"], name: "index_event_rsvps_on_member_id" end @@ -202,7 +227,7 @@ t.text "technical_type" t.string "system" t.string "subsystem" - t.json 'data', default: '{}' + t.json "data", default: "{}" t.index ["area_id"], name: "index_events_on_area_id" t.index ["campaign_id"], name: "index_events_on_campaign_id" t.index ["external_id", "system", "subsystem"], name: "index_events_on_system", unique: true @@ -249,13 +274,15 @@ t.datetime "created_at" t.datetime "updated_at" t.text "unsubscribe_reason" - t.text "subscribe_reason" t.boolean "permanent" t.integer "unsubscribe_mailing_id" + t.string "subscribe_reason" + t.datetime "subscribed_at" t.index ["member_id", "subscription_id"], name: "index_member_subscriptions_on_member_id_and_subscription_id", unique: true t.index ["member_id"], name: "index_member_subscriptions_on_member_id" t.index ["subscription_id"], name: "index_member_subscriptions_on_subscription_id" t.index ["unsubscribe_mailing_id"], name: "index_member_subscriptions_on_unsubscribe_mailing_id" + t.index ["unsubscribed_at"], name: "index_member_subscriptions_on_unsubscribed_at" end create_table "members", force: :cascade do |t| @@ -374,6 +401,8 @@ t.datetime "created_at" t.datetime "updated_at" t.integer "member_count", default: 0 + t.string "slug", null: false + t.index ["slug"], name: "index_subscriptions_on_slug", unique: true end create_table "syncs", force: :cascade do |t| @@ -387,7 +416,7 @@ t.bigint "list_id" t.bigint "contact_campaign_id" t.bigint "author_id" - t.json "reference_data", default: '{}' + t.json "reference_data", default: "{}" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["author_id"], name: "index_syncs_on_author_id" diff --git a/spec/test_identity_app/spec/factories/identity/campaign.rb b/spec/test_identity_app/spec/factories/identity/campaign.rb new file mode 100644 index 0000000..92e764e --- /dev/null +++ b/spec/test_identity_app/spec/factories/identity/campaign.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :campaign do + name { Faker::Book.title } + end +end From 7c1f4e109c4976ef05353202e21d628c360f5ae4 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 27 Feb 2022 21:08:56 +1100 Subject: [PATCH 10/22] Update API api call and logging to make it easier to read and understand --- app/models/identity_nation_builder/api.rb | 59 +++++++++++++---------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/app/models/identity_nation_builder/api.rb b/app/models/identity_nation_builder/api.rb index b2c4232..a3b648a 100644 --- a/app/models/identity_nation_builder/api.rb +++ b/app/models/identity_nation_builder/api.rb @@ -224,49 +224,56 @@ def self.upsert_person_without_retry(member) api(:people, :add, { person: member })['person'] end - def self.api(*args) - args[2] = {} unless args.third - args.third[:fire_webhooks] = false + def self.api(endpoint, method, params) started_at = DateTime.now begin - payload = get_api_client.call(*args) - raise_if_empty_payload payload - rescue NationBuilder::RateLimitedError - raise + response = get_api_client.call(endpoint, method, params) + + if not response + raise NationBuilder::RateLimitedError, + 'Empty response returned from NB API - likely due to Rate Limiting' + end + rescue NationBuilder::RateLimitedError => e + Rails.logger.error('nation_builder.api') { e } + raise e rescue NationBuilder::ClientError => e - payload = JSON.parse(e.message) - unless payload_has_a_no_match_code?(payload) || attempt_to_rsvp_person_twice(args[1], e.message) - log_api_call(started_at, payload, *args) - raise + response = JSON.parse(e.message) + unless response_has_a_no_match_code?(response) || + attempt_to_rsvp_person_twice(method, e.message) + Rails.logger.error('nation_builder.api') { e } + raise e end + ensure + log_api_call(started_at, endpoint, method, params, response) end - log_api_call(started_at, payload, *args) - payload + + response end def self.get_api_client NationBuilder::Client.new Settings.nation_builder.site, Settings.nation_builder.token, retries: 8 end - def self.payload_has_a_no_match_code?(payload) - payload && payload['code'] == 'no_matches' + def self.response_has_a_no_match_code?(response) + response && response['code'] == 'no_matches' end def self.attempt_to_rsvp_person_twice(api_call, error) api_call == :rsvp_create && error.include?("signup_id has already been taken") end - def self.raise_if_empty_payload(payload) - raise RuntimeError, 'Empty payload returned from NB API - likely due to Rate Limiting' if payload.nil? - end - - def self.log_api_call(started_at, payload, *call_args) - return unless Settings.nation_builder.debug - data = { - started_at: started_at, payload: payload, completed_at: DateTime.now, - endpoint: call_args[0..1].join('/'), data: call_args.third, - } - puts "NationBuilder API: #{data.inspect}" + def self.log_api_call(started_at, endpoint, method, params, response) + if Settings.nation_builder.debug + data = { + started_at: started_at, + endpoint: endpoint, + method: method, + params: params, + response: response, + completed_at: DateTime.now, + } + puts "NationBuilder API: #{data.inspect}" + end end def self.list_from_cache(cache_key) From f1ec945a3d48b954f576e362eb7c4508f39fa551 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Wed, 2 Mar 2022 08:37:15 +1100 Subject: [PATCH 11/22] Use "recruiter" rather than "recruitment method" in RSVP push ui --- app/views/identity_nation_builder/_service_form_fields.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/identity_nation_builder/_service_form_fields.html.erb b/app/views/identity_nation_builder/_service_form_fields.html.erb index 757436d..d54412b 100644 --- a/app/views/identity_nation_builder/_service_form_fields.html.erb +++ b/app/views/identity_nation_builder/_service_form_fields.html.erb @@ -19,7 +19,7 @@ <%= check_box_tag "nation_builder[mark_as_attended]", true, false %> <% if recruiters = IdentityNationBuilder::ApplicationHelper.recruiters %>
- <%= label_tag "nation_builder[recruiter_id]", 'Recruitment method: ' %> + <%= label_tag "nation_builder[recruiter_id]", 'Recruiter: ' %> <%= select_tag "nation_builder[recruiter_id]", options_for_select(recruiters), { style: 'width:100%;' } %> <% end %> From 6109886302a060cf0d10cf47c58d5adb8389629c Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Wed, 2 Mar 2022 08:45:20 +1100 Subject: [PATCH 12/22] Fix mark attended checkbox un push UI never being active --- .../identity_nation_builder/_service_form_fields.html.erb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/views/identity_nation_builder/_service_form_fields.html.erb b/app/views/identity_nation_builder/_service_form_fields.html.erb index d54412b..7568904 100644 --- a/app/views/identity_nation_builder/_service_form_fields.html.erb +++ b/app/views/identity_nation_builder/_service_form_fields.html.erb @@ -42,6 +42,7 @@ $(".push_to_list_type").html("and RSVP") $(".event_rsvp_container").show() $(".event_rsvp_container select").prop("disabled", false) + $(".event_rsvp_container input").prop("disabled", false) $(".mark_as_attended_to_all_events_on_date_container").hide() $(".tag_container").hide() $(".tag_container input").prop("disabled", true) @@ -52,10 +53,12 @@ $(".tag_container input").prop("disabled", true) $(".event_rsvp_container").hide() $(".event_rsvp_container select").prop("disabled", true) + $(".event_rsvp_container input").prop("disabled", true) } else if ($("#nation_builder_sync_type").val() === "tag"){ $(".push_to_list_type").html("and tag") $(".event_rsvp_container").hide() $(".event_rsvp_container select").prop("disabled", true) + $(".event_rsvp_container input").prop("disabled", true) $(".tag_container").show() $(".tag_container input").prop("disabled", false) $(".mark_as_attended_to_all_events_on_date_container").hide() From abc6f8cc1ca8634d0bfbf9ad5d36ebb48aed241a Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Wed, 2 Mar 2022 08:56:07 +1100 Subject: [PATCH 13/22] Fix a test using local time rather than UTC --- spec/lib/identity_nation_builder_pull_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/identity_nation_builder_pull_spec.rb b/spec/lib/identity_nation_builder_pull_spec.rb index 75c8614..14330e3 100644 --- a/spec/lib/identity_nation_builder_pull_spec.rb +++ b/spec/lib/identity_nation_builder_pull_spec.rb @@ -87,7 +87,7 @@ IdentityNationBuilder.fetch_new_events(@sync_id) {} Event.update_all(updated_at: 3.days.ago) IdentityNationBuilder.fetch_new_events(@sync_id) {} - expect(Event.first.updated_at.to_date).to eq(Date.today) + expect(Event.first.updated_at.to_date).to eq(Time.now.utc.to_date) end end From 932db3bdf5fec574c4ddafc213ea5719d12b124a Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Wed, 2 Mar 2022 09:03:24 +1100 Subject: [PATCH 14/22] Remove mark attended to all events on date option --- .../application_helper.rb | 5 +- app/models/identity_nation_builder/api.rb | 39 +++------------ .../_service_form_fields.html.erb | 12 ----- lib/identity_nation_builder.rb | 16 ++---- spec/lib/identity_nation_builder_push_spec.rb | 25 +--------- .../identity_nation_builder/api_spec.rb | 49 ------------------- 6 files changed, 17 insertions(+), 129 deletions(-) diff --git a/app/helpers/identity_nation_builder/application_helper.rb b/app/helpers/identity_nation_builder/application_helper.rb index 5fb5d86..e4ceb51 100644 --- a/app/helpers/identity_nation_builder/application_helper.rb +++ b/app/helpers/identity_nation_builder/application_helper.rb @@ -1,7 +1,10 @@ module IdentityNationBuilder module ApplicationHelper def self.push_types_for_select - [["Event RSVP", :rsvp], ["Tag", :tag], ["Mark as attended to today's events", :mark_as_attended_to_all_events_on_date]] + [ + ["Event RSVP", :rsvp], + ["Tag", :tag], + ] end def self.events_for_select diff --git a/app/models/identity_nation_builder/api.rb b/app/models/identity_nation_builder/api.rb index a3b648a..b756f5d 100644 --- a/app/models/identity_nation_builder/api.rb +++ b/app/models/identity_nation_builder/api.rb @@ -12,7 +12,13 @@ def self.rsvp(site_slug, members, event_id, mark_as_attended=false, recruiter_id pager = NationBuilder::Paginator.new(get_api_client, event_rsvps(site_slug, event_id)) rsvp = pager.body['results'].select { |result| result['person_id'] == person['id'] }.first if rsvp && !rsvp['attended'] - update_rsvp(site_slug, rsvp, mark_as_attended) + api( + :events, :rsvp_update, { + rsvp_id: rsvp['id'], + event_id: rsvp['event_id'], + site_slug: site_slug, + rsvp: { attended: mark_as_attended, person_id: rsvp['person_id'] } + }) end end @@ -34,28 +40,6 @@ def self.tag(site_slug, members, tag) yield member_ids.length, member_ids end - def self.mark_as_attended_to_all_events_on_date(site_slug, members) - rsvps_on_date = EventRsvp.where(member_id: members.map { |member| member[:id] }) - .joins(:event) - .where('events.start_time::date = ?', Date.current) - .where(attended: false) - member_ids = [] - rsvps_on_date.each do |rsvp| - begin - update_rsvp(rsvp.event.data['site_slug'], rsvp.data, true) - member_ids.append( - { - identity_id: rsvp.member.id, - nationbuilder_id: rsvp.data['person_id'] - } - ) - rescue NationBuilder::ClientError => response - raise unless response.message =~ /Record not found/i - end - end - yield member_ids.length, member_ids - end - def self.sites api(:sites, :index, { per_page: 100 })['results'] end @@ -127,15 +111,6 @@ def self.rsvp_person(site_slug, event_id, person, attended=false, recruiter_id=n api(:events, :rsvp_create, { id: event_id, site_slug: site_slug, rsvp: rsvp_data }) end - def self.update_rsvp(site_slug, rsvp, mark_as_attended) - api(:events, :rsvp_update, { - rsvp_id: rsvp['id'], - event_id: rsvp['event_id'], - site_slug: site_slug, - rsvp: { attended: mark_as_attended, person_id: rsvp['person_id'] } - }) - end - def self.person(people_id) api(:people, :show, { id: people_id })["person"] end diff --git a/app/views/identity_nation_builder/_service_form_fields.html.erb b/app/views/identity_nation_builder/_service_form_fields.html.erb index 7568904..4b3ffd7 100644 --- a/app/views/identity_nation_builder/_service_form_fields.html.erb +++ b/app/views/identity_nation_builder/_service_form_fields.html.erb @@ -27,8 +27,6 @@ <%= label_tag "nation_builder[tag]", 'Tag Name: ' %> <%= text_field_tag "nation_builder[tag]", nil, {:placeholder => 'Tag Name'} %> -
-
@@ -43,17 +41,8 @@ $(".event_rsvp_container").show() $(".event_rsvp_container select").prop("disabled", false) $(".event_rsvp_container input").prop("disabled", false) - $(".mark_as_attended_to_all_events_on_date_container").hide() - $(".tag_container").hide() - $(".tag_container input").prop("disabled", true) - } else if ($("#nation_builder_sync_type").val() === "mark_as_attended_to_all_events_on_date") { - $(".push_to_list_type").html("and mark all as attended to events today") - $(".mark_as_attended_to_all_events_on_date_container").show() $(".tag_container").hide() $(".tag_container input").prop("disabled", true) - $(".event_rsvp_container").hide() - $(".event_rsvp_container select").prop("disabled", true) - $(".event_rsvp_container input").prop("disabled", true) } else if ($("#nation_builder_sync_type").val() === "tag"){ $(".push_to_list_type").html("and tag") $(".event_rsvp_container").hide() @@ -61,7 +50,6 @@ $(".event_rsvp_container input").prop("disabled", true) $(".tag_container").show() $(".tag_container input").prop("disabled", false) - $(".mark_as_attended_to_all_events_on_date_container").hide() } else { $(".push_to_list_type").html('') $(".nationbuilder_container").hide() diff --git a/lib/identity_nation_builder.rb b/lib/identity_nation_builder.rb index 821b1da..46e9c0d 100644 --- a/lib/identity_nation_builder.rb +++ b/lib/identity_nation_builder.rb @@ -3,7 +3,7 @@ module IdentityNationBuilder SYSTEM_NAME = 'nation_builder' SYNCING = 'members' - CONTACT_TYPE = {'rsvp' => 'event', 'tag' => 'list', 'mark_as_attended_to_all_events_on_date' => ' mark as attended'} + CONTACT_TYPE = {'rsvp' => 'event', 'tag' => 'tag'} PULL_JOBS = [[:fetch_new_events, 1.hours], [:fetch_recruiters, 1.hours]] MEMBER_RECORD_DATA_TYPE='object' @@ -22,14 +22,10 @@ def self.push_in_batches(sync_id, members, external_system_params) external_system_params_hash = JSON.parse(external_system_params) sync_type = external_system_params_hash['sync_type'] site_slug = external_system_params_hash['site_slug'] - if sync_type == 'mark_as_attended_to_all_events_on_date' - rows = batch_members - else - rows = ActiveModel::Serializer::CollectionSerializer.new( - batch_members, - serializer: NationBuilderMemberSyncPushSerializer - ).as_json - end + rows = ActiveModel::Serializer::CollectionSerializer.new( + batch_members, + serializer: NationBuilderMemberSyncPushSerializer + ).as_json IdentityNationBuilder::API.send(sync_type, site_slug, rows, *sync_type_item(external_system_params_hash)) do |write_result_count, member_ids| if sync_type === 'tag' member_ids.each do |member_id| @@ -52,8 +48,6 @@ def self.sync_type_item(external_system_params_hash) [external_system_params_hash['event_id'], external_system_params_hash['mark_as_attended'], external_system_params_hash['recruiter_id']] when 'tag' [external_system_params_hash['tag']] - when 'mark_as_attended_to_all_events_on_date' - [] end end diff --git a/spec/lib/identity_nation_builder_push_spec.rb b/spec/lib/identity_nation_builder_push_spec.rb index 340670d..bc03e05 100644 --- a/spec/lib/identity_nation_builder_push_spec.rb +++ b/spec/lib/identity_nation_builder_push_spec.rb @@ -16,14 +16,8 @@ expect(IdentityNationBuilder.sync_type_item(external_system_params)).to eq(['test_tag']) end end - - context 'with mark_as_attended_to_all_events_on_date' do - it 'returns correct sync item type' do - external_system_params = {'sync_type' => 'mark_as_attended_to_all_events_on_date', 'site_slug' => 'action'} - expect(IdentityNationBuilder.sync_type_item(external_system_params)).to eq([]) - end - end end + context 'with invalid parameters' do it 'returns no sync item type' do external_system_params = {'sync_type' => 'yada'} @@ -99,23 +93,6 @@ end end end - - context 'mark_as_attended_to_all_events_on_date' do - context 'with valid parameters' do - it 'yeilds write_result_count' do - external_system_params = JSON.generate({'sync_type' => 'mark_as_attended_to_all_events_on_date'}) - expect(IdentityNationBuilder::API).to receive(:mark_as_attended_to_all_events_on_date).exactly(1).times.with(nil, instance_of(@members.class)) -.and_yield( - 3, [ - {identity_id: @members[0].id, nationbuilder_id: @members[0].id }, - {identity_id: @members[1].id, nationbuilder_id: @members[1].id }, - ]) - IdentityNationBuilder.push_in_batches(1, @members, external_system_params) do |batch_index, write_result_count| - expect(write_result_count).to eq(3) - end - end - end - end end describe '#attempt_to_rsvp_person_twice' do diff --git a/spec/models/identity_nation_builder/api_spec.rb b/spec/models/identity_nation_builder/api_spec.rb index a45dbda..2dcc2b8 100644 --- a/spec/models/identity_nation_builder/api_spec.rb +++ b/spec/models/identity_nation_builder/api_spec.rb @@ -256,55 +256,6 @@ end end end - - describe '.mark_as_attended_to_all_events_on_date' do - let!(:nb_event_data) { { "id": 1, "event_id": 2, "person_id": 3 } } - let!(:member) { FactoryBot.create(:member) } - let!(:member_data) { { id: member.id } } - let!(:campaign) { Campaign.create!(name: "Test campaign") } - let!(:event) { Event.create!(name: "Event 1", campaign: campaign, external_id: 2, start_time: Time.now, data: { 'site_slug': 'test_slug' }) } - let!(:event_rsvp) { EventRsvp.create!(event: event, member: member, attended: false, data: nb_event_data) } - let!(:old_event) { Event.create!(name: "Event 2", campaign: campaign, external_id: 2, start_time: 5.days.ago) } - let!(:old_nb_event_data) { { "id": 8, "event_id": 9, "person_id": 3 } } - let!(:old_rsvp) { EventRsvp.create!(event: old_event, member: member, attended: false, data: nb_event_data) } - let!(:future_event) { Event.create!(name: "Event 3", campaign: campaign, external_id: 2, start_time: 1.days.since) } - let!(:future_nb_event_data) { { "id": 12, "event_id": 19, "person_id": 3 } } - let!(:future_rsvp) { EventRsvp.create!(event: future_event, member: member, attended: false, data: nb_event_data) } - - it 'should mark the member as attened to any events on the specified date' do - rsvp_update_request = stub_request(:put, %r{sites/test_slug/pages/events/2/rsvps/1}) - .with(body: /"attended":true.*"person_id":3/) - .to_return({ - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { - rsvp: { id: 1, event_id: 2, person_id: 3, attended: false } - }.to_json - }) - IdentityNationBuilder::API.mark_as_attended_to_all_events_on_date('test', [member_data]) { |length, members| - expect(length).to eq(1) - expect(members).to eq([{ identity_id: member.id, nationbuilder_id: 3 }]) - } - expect(rsvp_update_request).to have_been_requested - end - - it 'should skip rsvps that 404' do - rsvp_update_request = stub_request(:put, %r{pages/events/2/rsvps/1}) - .with(body: /"attended":true.*"person_id":3/) - .to_return({ - status: 404, - headers: { 'Content-Type' => 'application/json' }, - body: { - "code":"not_found", "message":"Record not found" - }.to_json - }) - IdentityNationBuilder::API.mark_as_attended_to_all_events_on_date(nil, [member_data]) { |length, members| - expect(length).to eq(0) - expect(members).to eq([]) - } - expect(rsvp_update_request).to have_been_requested - end - end end describe '.recruiters' do From 9c2411f25a8467abec34c5dadfd435a84be75379 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 17 Apr 2022 22:14:36 +1000 Subject: [PATCH 15/22] Update id test instance classes --- spec/test_identity_app/app/lib/settings.rb | 8 ++- .../app/models/application_record.rb | 9 +++ .../app/models/canonical_address.rb | 27 +++++--- spec/test_identity_app/app/models/member.rb | 7 +- .../app/models/member_subscription.rb | 65 ++++++++++++++++++- .../app/models/member_subscription_event.rb | 40 ++++++++++++ .../app/models/subscription.rb | 3 +- .../db/migrate/000_initial.rb | 38 +++++++++-- spec/test_identity_app/db/schema.rb | 38 +++++++++-- 9 files changed, 205 insertions(+), 30 deletions(-) create mode 100644 spec/test_identity_app/app/models/member_subscription_event.rb diff --git a/spec/test_identity_app/app/lib/settings.rb b/spec/test_identity_app/app/lib/settings.rb index 84bbe9a..893b47e 100644 --- a/spec/test_identity_app/app/lib/settings.rb +++ b/spec/test_identity_app/app/lib/settings.rb @@ -23,7 +23,9 @@ def self.nation_builder end def self.app - return { } + return { + "inbound_url" => 'https://example.com/inbound' + } end def self.options @@ -32,6 +34,10 @@ def self.options } end + def self.databases + return { } + end + def self.sidekiq_redis_url return ENV['SIDEKIQ_REDIS_URL'] end diff --git a/spec/test_identity_app/app/models/application_record.rb b/spec/test_identity_app/app/models/application_record.rb index 10a4cba..f032c84 100644 --- a/spec/test_identity_app/app/models/application_record.rb +++ b/spec/test_identity_app/app/models/application_record.rb @@ -1,3 +1,12 @@ class ApplicationRecord < ActiveRecord::Base self.abstract_class = true + + # This retains legacy AR behaviour + # TODO: Remove this after tightening up foreign keys and updating factories + self.belongs_to_required_by_default = false + + # Touch method that validates + def touch! + update! updated_at: DateTime.now + end end diff --git a/spec/test_identity_app/app/models/canonical_address.rb b/spec/test_identity_app/app/models/canonical_address.rb index 05380a6..6e06c12 100644 --- a/spec/test_identity_app/app/models/canonical_address.rb +++ b/spec/test_identity_app/app/models/canonical_address.rb @@ -1,22 +1,31 @@ class CanonicalAddress < ApplicationRecord - include ReadWriteIdentity + has_many :addresses + has_and_belongs_to_many :areas + + before_save do + self.search_text = [line1, line2, suburb, town, state, postcode, country].join(', ') + end + + alias_attribute :zip, :postcode + class << self def search(address = {}) - return nil unless address.present? + return nil if address.blank? - address_string = [address[:line1], address[:line2], address[:town], address[:state], address[:postcode], address[:country]].join(', ').upcase + address_string = [address[:line1], address[:line2], address[:suburb], address[:town], address[:state], address[:postcode], address[:country]].join(', ').upcase - query = CanonicalAddress - .where('search_text % ?', address_string) - .order("similarity(search_text, #{ApplicationRecord.connection.quote(address_string)}) DESC") - .select("*, similarity(search_text, #{ApplicationRecord.connection.quote(address_string)}) as similarity") + core_schema = ApplicationRecord.connection.quote_table_name(Settings.databases.extensions_schemas.core) + query = CanonicalAddress.where('search_text % ?', address_string) + .order(Arel.sql("#{core_schema}.similarity(search_text, #{ApplicationRecord.connection.quote(address_string)}) DESC")) + .select(Arel.sql("*, #{core_schema}.similarity(search_text, #{ApplicationRecord.connection.quote(address_string)}) as similarity")) if address[:postcode] query = query.where(postcode: address[:postcode].to_s.upcase.delete(' ')) end - return nil unless ca = query.first - return ca if ca.similarity > 0.7 + return nil unless (ca = query.first) + + ca if ca.similarity > 0.7 end end end diff --git a/spec/test_identity_app/app/models/member.rb b/spec/test_identity_app/app/models/member.rb index 2e3072b..4638976 100644 --- a/spec/test_identity_app/app/models/member.rb +++ b/spec/test_identity_app/app/models/member.rb @@ -300,8 +300,11 @@ def update_address(new_address) country: new_address[:country], } - # If this user already has the canonical address among their addresses, touch it to make it their most recent. Otherwise insert it. - if (canonical_address = CanonicalAddress.search(address_attributes)) + # If the new address has line1/line2, and if the member already has the + # canonical address among their addresses, touch it to make it + # their most recent. Otherwise insert it. + if address_attributes.slice(:line1, :line2).values.any?(&:present?) && + (canonical_address = CanonicalAddress.search(address_attributes)) if (new_address = addresses.find_by(canonical_address: canonical_address)) new_address.touch! else diff --git a/spec/test_identity_app/app/models/member_subscription.rb b/spec/test_identity_app/app/models/member_subscription.rb index 71ae7a8..7f0536e 100644 --- a/spec/test_identity_app/app/models/member_subscription.rb +++ b/spec/test_identity_app/app/models/member_subscription.rb @@ -1,8 +1,67 @@ +# == Schema Information +# +# Table name: member_subscriptions +# +# id :integer not null, primary key +# subscription_id :integer +# member_id :integer +# unsubscribed_at :datetime +# unsubscribe_reason :text +# subscribed_at :datetime +# subscribe_reason :text +# created_at :datetime +# updated_at :datetime +# + class MemberSubscription < ApplicationRecord - include ReadWriteIdentity - attr_accessor :audit_data - belongs_to :member + attr_accessor :subscribable + attr_accessor :operation_reason + + self.table_name = 'member_subscriptions' belongs_to :subscription + belongs_to :member + belongs_to :unsubscribe_mailing, class_name: 'Mailing', optional: true + has_many :member_subscription_events + + after_create :record_member_subscription_create_event + around_update :record_member_subscription_update_event + + validates_uniqueness_of :member, scope: :subscription + + def record_member_subscription_create_event + record_member_subscription_event(action: 'create', subscription_status_changed: true) + end + + def record_member_subscription_update_event + # unsubscribed_at can change without actually changing if someone is subscribed or not + # so we have to also check the before and after values + subscribe = unsubscribed_at_was.nil? + unsubscribe = unsubscribed_at_was.present? && unsubscribed_at.nil? + subscription_status_changed = unsubscribed_at_changed? && (subscribe || unsubscribe) + + yield + + record_member_subscription_event(action: 'update', subscription_status_changed: subscription_status_changed) + end + + def record_member_subscription_event(action:, subscription_status_changed:) + operation = unsubscribed_at.nil? ? 'subscribe' : 'unsubscribe' + member_subscription_events.create!( + action: action, + operation: operation, + operation_reason: operation_reason, + subscription_status_changed: subscription_status_changed, + subscribable: subscribable + ) + end + + def latest_member_subscription_event + member_subscription_events.last + end + + def latest_subscribable + latest_member_subscription_event&.subscribable + end def unsubscribed_permanently? return !unsubscribed_at.nil? && permanent diff --git a/spec/test_identity_app/app/models/member_subscription_event.rb b/spec/test_identity_app/app/models/member_subscription_event.rb new file mode 100644 index 0000000..4063bf4 --- /dev/null +++ b/spec/test_identity_app/app/models/member_subscription_event.rb @@ -0,0 +1,40 @@ +# == Schema Information +# +# Table name: member_subscription_events +# +# id :integer not null, primary key +# action :string +# operation :string +# operation_reason :text +# operation_permanently_deferred :boolean +# subscription_status_changed :boolean +# member_subscription_id :integer +# subscribable_id :integer +# subscribable_type :string +# created_at :datetime +# updated_at :datetime +# + +class MemberSubscriptionEvent < ApplicationRecord + belongs_to :member_subscription + belongs_to :subscribable, polymorphic: true + + validates :action, inclusion: { in: %w(create update) } + validates :operation, inclusion: { in: %w(subscribe unsubscribe) } + + scope :subscriptions, -> { + where(operation: 'subscribe', subscription_status_changed: true) + } + + scope :new_subscriptions, -> { + where(operation: 'subscribe', action: 'create', subscription_status_changed: true) + } + + scope :resubscriptions, -> { + where(operation: 'subscribe', action: 'update', subscription_status_changed: true) + } + + scope :unsubscriptions, -> { + where(operation: 'unsubscribe', subscription_status_changed: true) + } +end diff --git a/spec/test_identity_app/app/models/subscription.rb b/spec/test_identity_app/app/models/subscription.rb index 3043ecb..7d35ff9 100644 --- a/spec/test_identity_app/app/models/subscription.rb +++ b/spec/test_identity_app/app/models/subscription.rb @@ -64,7 +64,8 @@ class Subscription < ApplicationRecord id: [ EMAIL_SUBSCRIPTION, SMS_SUBSCRIPTION, NOTIFICATION_SUBSCRIPTION, FACEBOOK_SUBSCRIPTION, CALLING_SUBSCRIPTION, POST_SUBSCRIPTION, PROFILING_SUBSCRIPTION - ] + ], + deleted_at: nil ) } diff --git a/spec/test_identity_app/db/migrate/000_initial.rb b/spec/test_identity_app/db/migrate/000_initial.rb index cdd4d26..5c25b37 100644 --- a/spec/test_identity_app/db/migrate/000_initial.rb +++ b/spec/test_identity_app/db/migrate/000_initial.rb @@ -169,6 +169,34 @@ class Initial < ActiveRecord::Migration[4.2] t.index ["system", "external_id"], name: "index_member_external_ids_on_system_and_external_id", unique: true end + create_table "member_subscription_events", force: :cascade do |t| + t.string "action", null: false + t.string "operation", null: false + t.string "operation_reason", default: "not_specified" + t.boolean "operation_permanently_deferred", default: false + t.bigint "member_subscription_id" + t.integer "subscribable_id" + t.string "subscribable_type" + t.boolean "subscription_status_changed" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["member_subscription_id", "operation"], name: "index_member_subscription_events_member_subscription_operation" + t.index ["member_subscription_id"], name: "index_member_subscription_events_on_member_subscription_id" + t.index ["subscribable_type", "subscribable_id", "operation"], name: "index_member_subscription_events_subscribable_operation" + t.index ["subscribable_type", "subscribable_id"], name: "index_member_subscription_events_subscribable" + end + + create_table "member_subscription_logs", id: :serial, force: :cascade do |t| + t.integer "member_id" + t.integer "subscription_id" + t.text "operation" + t.datetime "created_at" + t.datetime "updated_at" + t.string "dyno" + t.text "unsubscribe_reason" + t.text "backtrace" + end + create_table "member_subscriptions", id: :serial, force: :cascade do |t| t.integer "subscription_id", null: false t.integer "member_id", null: false @@ -188,10 +216,7 @@ class Initial < ActiveRecord::Migration[4.2] end create_table "members", force: :cascade do |t| - t.integer "cons_id" t.text "email" - t.json "contact" - t.json "meta" t.datetime "created_at" t.datetime "updated_at" t.datetime "joined_at" @@ -200,7 +225,6 @@ class Initial < ActiveRecord::Migration[4.2] t.json "action_history" t.text "reset_token" t.integer "authy_id" - t.integer "point_person_id" t.integer "role_id" t.datetime "last_donated" t.integer "donations_count" @@ -218,10 +242,10 @@ class Initial < ActiveRecord::Migration[4.2] t.string "title" t.string "gender" t.string "donation_preference", limit: 20 + t.index "btrim(((COALESCE(first_name, ''::text) || ' '::text) || COALESCE(last_name, ''::text))) gin_trgm_ops", name: "index_members_on_full_name_gin", using: :gin t.index ["email"], name: "index_members_on_email" - t.index ["first_name"], name: "index_members_on_first_name" - t.index ["last_name"], name: "index_members_on_last_name" - t.index ["point_person_id"], name: "index_members_on_point_person_id" + t.index ["email"], name: "index_members_on_email_gin", opclass: :gin_trgm_ops, using: :gin + t.index ["guid"], name: "index_members_on_guid", unique: true t.index ["role_id"], name: "index_members_on_role_id" end diff --git a/spec/test_identity_app/db/schema.rb b/spec/test_identity_app/db/schema.rb index e2dbae6..bf785e5 100644 --- a/spec/test_identity_app/db/schema.rb +++ b/spec/test_identity_app/db/schema.rb @@ -267,6 +267,34 @@ t.index ["system", "external_id"], name: "index_member_external_ids_on_system_and_external_id", unique: true end + create_table "member_subscription_events", force: :cascade do |t| + t.string "action", null: false + t.string "operation", null: false + t.string "operation_reason", default: "not_specified" + t.boolean "operation_permanently_deferred", default: false + t.bigint "member_subscription_id" + t.integer "subscribable_id" + t.string "subscribable_type" + t.boolean "subscription_status_changed" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["member_subscription_id", "operation"], name: "index_member_subscription_events_member_subscription_operation" + t.index ["member_subscription_id"], name: "index_member_subscription_events_on_member_subscription_id" + t.index ["subscribable_type", "subscribable_id", "operation"], name: "index_member_subscription_events_subscribable_operation" + t.index ["subscribable_type", "subscribable_id"], name: "index_member_subscription_events_subscribable" + end + + create_table "member_subscription_logs", id: :serial, force: :cascade do |t| + t.integer "member_id" + t.integer "subscription_id" + t.text "operation" + t.datetime "created_at" + t.datetime "updated_at" + t.string "dyno" + t.text "unsubscribe_reason" + t.text "backtrace" + end + create_table "member_subscriptions", id: :serial, force: :cascade do |t| t.integer "subscription_id", null: false t.integer "member_id", null: false @@ -286,10 +314,7 @@ end create_table "members", force: :cascade do |t| - t.integer "cons_id" t.text "email" - t.json "contact" - t.json "meta" t.datetime "created_at" t.datetime "updated_at" t.datetime "joined_at" @@ -298,7 +323,6 @@ t.json "action_history" t.text "reset_token" t.integer "authy_id" - t.integer "point_person_id" t.integer "role_id" t.datetime "last_donated" t.integer "donations_count" @@ -316,10 +340,10 @@ t.string "title" t.string "gender" t.string "donation_preference", limit: 20 + t.index "btrim(((COALESCE(first_name, ''::text) || ' '::text) || COALESCE(last_name, ''::text))) gin_trgm_ops", name: "index_members_on_full_name_gin", using: :gin t.index ["email"], name: "index_members_on_email" - t.index ["first_name"], name: "index_members_on_first_name" - t.index ["last_name"], name: "index_members_on_last_name" - t.index ["point_person_id"], name: "index_members_on_point_person_id" + t.index ["email"], name: "index_members_on_email_gin", opclass: :gin_trgm_ops, using: :gin + t.index ["guid"], name: "index_members_on_guid", unique: true t.index ["role_id"], name: "index_members_on_role_id" end From b2d0693d873556bf8284d964c7864340d6bf808a Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 17 Apr 2022 22:15:14 +1000 Subject: [PATCH 16/22] Enable logging for tests --- spec/test_identity_app/config/environments/test.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/test_identity_app/config/environments/test.rb b/spec/test_identity_app/config/environments/test.rb index 8e5cbde..23ed8b1 100644 --- a/spec/test_identity_app/config/environments/test.rb +++ b/spec/test_identity_app/config/environments/test.rb @@ -39,4 +39,13 @@ # Raises error for missing translations # config.action_view.raise_on_missing_translations = true + + config.log_level = :debug + + if ENV["RAILS_LOG_TO_STDOUT"].present? + logger = ActiveSupport::Logger.new(STDOUT) + logger.formatter = config.log_formatter + logger.level = Logger.const_get('DEBUG') + config.logger = ActiveSupport::TaggedLogging.new(logger) + end end From 31cf7250897bfc9b2b42b7c99a23f4ca5ea38bc7 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 24 Jun 2022 10:17:23 +1000 Subject: [PATCH 17/22] Remove dev cruft --- .env.development | 5 ----- .ruby-version | 1 - 2 files changed, 6 deletions(-) delete mode 100644 .env.development delete mode 100644 .ruby-version diff --git a/.env.development b/.env.development deleted file mode 100644 index dc6aa57..0000000 --- a/.env.development +++ /dev/null @@ -1,5 +0,0 @@ -### NationBuilder -NATION_BUILDER_SITE=getupstaging -NATION_BUILDER_TOKEN=63d6c4c55514f72a30e5d3a77d435e502092a35dae9ad4124dfae16bbc4dc4a5 -NATION_BUILDER_DEBUG=1 -NATION_BUILDER_AUTHOR_ID=9802 \ No newline at end of file diff --git a/.ruby-version b/.ruby-version deleted file mode 100644 index 37c2961..0000000 --- a/.ruby-version +++ /dev/null @@ -1 +0,0 @@ -2.7.2 From 0f435bfecf6272be3f249b92400f3a277ae429e2 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 17 Apr 2022 22:15:32 +1000 Subject: [PATCH 18/22] Update Id fixture factories --- .../spec/factories/identity/member.rb | 8 ++++++++ .../spec/factories/identity/subscription.rb | 14 +++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/spec/test_identity_app/spec/factories/identity/member.rb b/spec/test_identity_app/spec/factories/identity/member.rb index 81c3d88..0115bd9 100644 --- a/spec/test_identity_app/spec/factories/identity/member.rb +++ b/spec/test_identity_app/spec/factories/identity/member.rb @@ -13,6 +13,10 @@ create(:custom_field, member: member, custom_field_key: FactoryBot.create(:custom_field_key)) end end + + factory :member_with_mobile_without_email do + email { nil } + end end factory :member_without_email do @@ -23,6 +27,10 @@ after(:create) do |member, evaluator| create(:landline_number, member: member) end + + factory :member_with_landline_without_email do + email { nil } + end end factory :member_with_both_phones do diff --git a/spec/test_identity_app/spec/factories/identity/subscription.rb b/spec/test_identity_app/spec/factories/identity/subscription.rb index 3ca8bb2..fd11933 100644 --- a/spec/test_identity_app/spec/factories/identity/subscription.rb +++ b/spec/test_identity_app/spec/factories/identity/subscription.rb @@ -1,16 +1,16 @@ FactoryBot.define do factory :subscription do - factory :calling_subscription do - id { Subscription::CALLING_SUBSCRIPTION } - name { 'Calling' } - end factory :email_subscription do id { Subscription::EMAIL_SUBSCRIPTION } name { 'Email' } end - factory :nation_builder_subscription do - id { Settings.nation_builder.opt_out_subscription_id } - name { 'NationBuilder Calling' } + factory :calling_subscription do + id { Subscription::CALLING_SUBSCRIPTION } + name { 'Calling' } + end + factory :sms_subscription do + id { Subscription::SMS_SUBSCRIPTION } + name { 'Texting' } end end end From 07fe7ce9ad0e36b0619fa3172f0496621bf80eee Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 17 Apr 2022 22:19:01 +1000 Subject: [PATCH 19/22] Create default subscriptions before tests are run to ensure they exist --- spec/rails_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index a7afa86..9e98db4 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -60,6 +60,8 @@ end config.around(:each) do |example| + # Default subscriptions need to exist outside of test transations + Subscription.defaults.all() DatabaseCleaner.cleaning do # Allow individual specs to do this when they need to via a method in # auth_helpers, which could also run `FactoryBot.create(:member_admin)` From fd68f149631705bcb21fa6d0ee0bd33c03170f91 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 17 Apr 2022 22:18:40 +1000 Subject: [PATCH 20/22] Implement low-level Identity member object upserting Handle merging base on NB persion data using both merging and overwriting strategies. Merging allows two records that were not previously associated to be updated from both. Overwriting allows subsequent updates to be applied, even if destructive. --- ...ion_builder_member_sync_push_serializer.rb | 260 +++++++++++++- ...uilder_member_sync_push_serializer_spec.rb | 319 +++++++++++++++++- 2 files changed, 560 insertions(+), 19 deletions(-) diff --git a/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb b/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb index 24b6f06..d11dd82 100644 --- a/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb +++ b/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb @@ -1,15 +1,26 @@ module IdentityNationBuilder class NationBuilderMemberSyncPushSerializer < ActiveModel::Serializer - attributes :id, :nationbuilder_id, :email, :phone, :mobile, :first_name, :last_name - def nationbuilder_id - id = @object.member_external_ids.with_system(SYSTEM_NAME).first - id.external_id if id + attributes :prefix, :first_name, :middle_name, :last_name, :sex, + :email, :email_opt_in, + :mobile, :mobile_opt_in, + :phone, :do_not_call, + :home_address + + def prefix + @object.title end - def phone - number = @object.phone_numbers.landline.first - strip_country_code(number.phone) if number + def middle_name + @object.middle_names + end + + def sex + @object.gender + end + + def email_opt_in + @object.is_subscribed_to?(Subscription::EMAIL_SUBSCRIPTION) end def mobile @@ -17,11 +28,246 @@ def mobile strip_country_code(number.phone) if number end + def mobile_opt_in + @object.is_subscribed_to?(Subscription::SMS_SUBSCRIPTION) + end + + def phone + number = @object.phone_numbers.landline.first + strip_country_code(number.phone) if number + end + + def do_not_call + !@object.is_subscribed_to?(Subscription::CALLING_SUBSCRIPTION) + end + + def home_address + address = @object.addresses.first + { + 'address1' => address.line1, + 'address2' => address.line2, + #'address3' => address.line3, + 'city' => address.town, + 'state' => address.state, + 'zip' => address.postcode, + 'country_code' => address.country, + } if address + end + + def upsert_from_person(person_data, type) + was_upserted = false + #ApplicationRecord.transaction do + id_data = serializable_hash + person_updated = Time.parse(person_data['updated_at']) + upsert_data = {} + + id_updated_later = -> (attribute_name) { + if attribute_name == 'email_opt_in' + subscription_updated_after?(Subscription::EMAIL_SUBSCRIPTION, person_updated) + elsif attribute_name == 'mobile' + object_updated_after?(@object.phone_numbers.mobile.first, person_updated) + elsif attribute_name == 'mobile_opt_in' + subscription_updated_after?(Subscription::SMS_SUBSCRIPTION, person_updated) + elsif attribute_name == 'phone' + object_updated_after?(@object.phone_numbers.landline.first, person_updated) + elsif attribute_name == 'do_not_call' + subscription_updated_after?(Subscription::CALLING_SUBSCRIPTION, person_updated) + elsif attribute_name == 'home_address' + object_updated_after?(@object.addresses.first, person_updated) + else + object_updated_after?(@object, person_updated) + end + } + + # Handle merging or not + + # Takes the most recently modified object value unless it is + # empty/nill, otherwise takes the other object's value. + update_upsert_merge = -> (key, attribute_name, &block) { + id_value = id_data[attribute_name.to_sym] + nb_value = person_data[attribute_name] + + if valid?(id_value) && valid?(nb_value) + value = id_updated_later::(attribute_name) ? id_value : nb_value + elsif valid?(id_value) + value = id_value + else + value = nb_value + end + + # only actually upsert if the value has changed + if value != id_value + value = block::(value, upsert_data[key]) if block + upsert_data[key] = value + end + } + + # Takes the most recently modified object value, even if + # empty. This allows values to be removed and this to be + # reflected at the other end. + update_upsert_overwrite = ->(key, attribute_name, &block) { + id_value = id_data[attribute_name.to_sym] + nb_value = person_data[attribute_name] + + value = id_updated_later::(attribute_name) ? id_value : nb_value + + # only actually upsert if the value has changed + if value != id_value + value = block::(value, upsert_data[key]) if block + upsert_data[key] = value + end + } + + case type + when :merge + update_upsert = update_upsert_merge + when :overwrite + update_upsert = update_upsert_overwrite + else + raise RuntimeError('unknown upsert type') + end + + # Work out what all supported upsert values should actually be + + update_upsert::(:firstname, 'first_name') + update_upsert::(:middlenames, 'middle_name') + update_upsert::(:lastname, 'last_name') + + # id will erase the other name parts if present, so need to + # set all of them if any + + update_upsert::(:title, 'prefix') + update_upsert::(:gender, 'sex') + + update_upsert::(:email, 'email') + update_upsert::(:phones, 'mobile') { |number, phones| + phones = [] if !phones + phones.append({ phone: number }) + phones + } + update_upsert::(:phones, 'phone') { |number, phones| + phones = [] if !phones + phones.append({ phone: number }) + phones + } + + update_upsert::(:subscriptions, 'email_opt_in') { |opt_in, subscriptions| + subscriptions = [] if !subscriptions + subscriptions.append( + { + slug: Subscription::EMAIL_SLUG, + action: opt_in ? 'subscribe' : 'unsubscribe', + reason: SYSTEM_NAME + } + ) + } + update_upsert::(:subscriptions, 'mobile_opt_in') { |opt_in, subscriptions| + subscriptions = [] if !subscriptions + subscriptions.append( + { + slug: Subscription::SMS_SLUG, + action: opt_in ? 'subscribe' : 'unsubscribe', + reason: SYSTEM_NAME + } + ) + } + update_upsert::(:subscriptions, 'do_not_call') { |opt_out, subscriptions| + subscriptions = [] if !subscriptions + subscriptions.append( + { + slug: Subscription::CALLING_SLUG, + action: opt_out ? 'unsubscribe' : 'subscribe', + reason: SYSTEM_NAME + } + ) + } + + update_upsert::(:addresses, 'home_address') { |address, addresses| + addresses = [] if !addresses + addresses.append( + { + line1: address['address1'], + line2: address['address2'], + #line3: address['address3'], + town: address['city'], + state: address['state'], + postcode: address['zip'], + country: address['country_code'], + } + ) + } + + if !upsert_data.empty? + member = UpsertMember::( + upsert_data.merge(external_ids: { SYSTEM_NAME => person_data['id'] }), + ignore_name_change: false, + entry_point: SYSTEM_NAME, + ) + was_upserted = true + + # XXX extend UpsertMember to support these + if upsert_data.has_key?(:email) + member.update!(email: upsert_data[:email]) + end + if upsert_data.has_key?(:title) + member.update!(title: upsert_data[:title]) + end + if upsert_data.has_key?(:gender) + member.update!(gender: upsert_data[:gender]) + end + + @object = member + end + #end + + was_upserted + end + + def attribute_equal(attribute_name, person_data) + return send(attribute_name) == person_data[attribute_name] + end + + def ==(person_data) + id_data = serializable_hash + + equal = true + attributes.each { |attr,value| + equal = id_data[attr] == person_data[attr.to_s] + break unless equal + } + equal + end + + def newer_than(person_data) + person_updated = person_data['updated_at'] + @object.updated_at > Time.parse(person_updated) if person_updated + end + + def older_than(person_data) + person_updated = person_data['updated_at'] + @object.updated_at < Time.parse(person_updated) if person_updated + end + private def strip_country_code(phone) code = Settings.options.try(:default_phone_country_code) code ? phone.try(:gsub, /^#{code}/, '0') : phone end + + def object_updated_after?(object, time) + valid?(object) && object.updated_at > time + end + + def subscription_updated_after?(subscription_type, time) + sub = @object.member_subscriptions.where( + subscription: subscription_type + ).first + !sub.nil? && sub.updated_at > time + end + + def valid?(value) + return !value.nil? && value != '' + end end end diff --git a/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb b/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb index 03ad5dc..9adadac 100644 --- a/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb +++ b/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb @@ -1,16 +1,25 @@ + +$nb_id = 0 + describe IdentityNationBuilder::NationBuilderMemberSyncPushSerializer do - context 'serialize' do + let!(:country_code) { '61' } + before(:each) do + allow(Settings).to( + receive_message_chain(:options, :default_phone_country_code) { country_code } + ) + allow(Settings).to( + receive_message_chain(:options, :default_mobile_phone_national_destination_code) { '4' } + ) + end + + context 'serialize' do before(:each) do - Settings.stub_chain(:options, :default_phone_country_code) { '61' } - Settings.stub_chain(:options, :default_mobile_phone_national_destination_code) { 4 } Member.all.destroy_all - Settings.stub_chain(:nation_builder) { {} } @member = FactoryBot.create(:member_with_both_phones) list = FactoryBot.create(:list) FactoryBot.create(:list_member, list: list, member: @member) FactoryBot.create(:member_with_both_phones) - Settings.stub_chain(:options, :default_phone_country_code).and_return(nil) @batch_members = Member.all.in_batches.first end @@ -22,23 +31,309 @@ ).as_json expect(rows.count).to eq(2) expect(rows[0][:email]).to eq(ListMember.first.member.email) - expect(rows[0][:phone]).to eq(ListMember.first.member.phone_numbers.landline.first.phone) - expect(rows[0][:mobile]).to eq(ListMember.first.member.phone_numbers.mobile.first.phone) + expect(rows[0][:phone]).to eq(ListMember.first.member.phone_numbers.landline.first.phone.gsub(/^#{country_code}/, '0')) + expect(rows[0][:mobile]).to eq(ListMember.first.member.phone_numbers.mobile.first.phone.gsub(/^#{country_code}/, '0')) expect(rows[0][:first_name]).to eq(ListMember.first.member.first_name) expect(rows[0][:last_name]).to eq(ListMember.first.member.last_name) end - context 'with Settings.options.default_phone_country_code set' do - let!(:country_code) { '61'} - before { Settings.stub_chain(:options, :default_phone_country_code).and_return(country_code) } + context 'with Settings.options.default_phone_country_code set to nil' do + before(:each) do + allow(Settings).to receive_message_chain(:options, :default_phone_country_code) { nil } + end it 'returns valid object' do rows = ActiveModel::Serializer::CollectionSerializer.new( @batch_members, serializer: IdentityNationBuilder::NationBuilderMemberSyncPushSerializer ).as_json - expect(rows[0][:phone]).to eq(@member.phone_numbers.landline.first.phone.gsub(/^#{country_code}/, '0')) - expect(rows[0][:mobile]).to eq(@member.phone_numbers.mobile.first.phone.gsub(/^#{country_code}/, '0')) + expect(rows[0][:phone]).to eq(@member.phone_numbers.landline.first.phone) + expect(rows[0][:mobile]).to eq(@member.phone_numbers.mobile.first.phone) + end + end + + context 'serialiser data equality' do + let!(:equality_member) { FactoryBot.create(:member_with_mobile) } + let!(:serialiser) { + IdentityNationBuilder::NationBuilderMemberSyncPushSerializer.new(equality_member) + } + let!(:person_data) { + # convert keys to strings since NB lib returns data with string keys + Hash[serialiser.serializable_hash.map { |k,v| [k.to_s, v] }] + } + + it 'returns true when compared for equality with own data' do + expect(serialiser == person_data).to eq(true) + end + + it 'returns false when compared for equality with different data' do + different = person_data + different['mobile'] = '555 1234' + expect(serialiser == different).to eq(false) + end + + it 'returns false when compared for equality with missing NB data' do + missing = person_data.except('mobile') + expect(serialiser == missing).to eq(false) + end + + it 'returns false when compared for equality with missing Id data' do + missing = person_data + missing['phone'] = '555 1234' + expect(serialiser == missing).to eq(false) + end + end + end + + context 'temporal comparisons' do + let!(:temporal_member) { FactoryBot.create(:member_with_mobile) } + let!(:serialiser) { + IdentityNationBuilder::NationBuilderMemberSyncPushSerializer.new(temporal_member) + } + let!(:newer_data) { + data = serialiser.serializable_hash + data['updated_at'] = (temporal_member.updated_at + 3660).to_s + data + } + let!(:older_data) { + data = serialiser.serializable_hash + data['updated_at'] = (temporal_member.updated_at - 3660).to_s + data + } + + it 'returns true when newer_than compared with older data' do + expect(serialiser.newer_than(older_data)).to eq(true) + end + + it 'returns false when newer_than compared with newer data' do + expect(serialiser.newer_than(newer_data)).to eq(false) + end + + it 'returns true when older_than compared with newer data' do + expect(serialiser.older_than(newer_data)).to eq(true) + end + + it 'returns false when older_than compared with older data' do + expect(serialiser.older_than(older_data)).to eq(false) + end + end + + context 'upsert using :merge' do + before(:each) do + allow(Settings).to( + receive_message_chain(:options, :allow_subscribe_via_upsert_member) { true } + ) + allow(Settings).to( + receive_message_chain(:databases, :extensions_schemas, :core) { 'public' } + ) + end + + let(:nb_id) { ($nb_id += 1).to_s } + let!(:member) { + member = FactoryBot.create(:member_with_both_phones) + member.update_external_id('nation_builder', nb_id) + member + } + let!(:serialiser) { + IdentityNationBuilder::NationBuilderMemberSyncPushSerializer.new(member) + } + let!(:person_data) { + # convert keys to strings since NB lib returns data with string keys + data = Hash[serialiser.serializable_hash.map { |k,v| [k.to_s, v] }] + data['id'] = nb_id + data['updated_at'] = member.updated_at.to_s + data + } + + context 'identity is more recently updated' do + before(:each) do + member.updated_at = (member.updated_at + 3600) + end + + it 'makes no change when no changes are present' do + expect(UpsertMember).to receive(:new).at_most(0).times + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(false) + expect(serialiser).to eq(person_data) + end + + it 'makes no change in id when id data is more recent' do + member.update!( + first_name: 'updated', + last_name: 'updated', + ) + + expect(UpsertMember).to receive(:new).at_most(0).times + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(false) + + # Will now be different than the person data + expect(serialiser).not_to eq(person_data) + end + end + + context 'nationbuilder is more recently updated' do + before(:each) do + person_data['updated_at'] = (member.updated_at + 3600).to_s + end + + it 'makes no change when no changes are present' do + expect(UpsertMember).to receive(:new).at_most(0).times + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(false) + expect(serialiser).to eq(person_data) + end + + it 'updates a member\'s given name' do + person_data['first_name'] = 'updated first' + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'updates a member\'s family name' do + person_data['last_name'] = 'updated last' + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'updates a member\'s full name' do + person_data['first_name'] = 'updated first' + person_data['middle_name'] = 'updated middle' + person_data['last_name'] = 'updated last' + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'updates a member email' do + person_data['email'] = 'updated@example.com' + + expect(UpsertMember).to( + receive(:new).with( + { + email: 'updated@example.com', + external_ids: { 'nation_builder' => person_data['id'] } + }, + ignore_name_change: false, + entry_point: 'nation_builder' + ).and_call_original + ) + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'subscribes a member to email' do + person_data['email_opt_in'] = true + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'unsubscribes a member from email' do + member.subscribe_to(Subscription::EMAIL_SUBSCRIPTION) + person_data['email_opt_in'] = false + person_data['updated_at'] = ( + member.member_subscriptions.where( + subscription: Subscription::EMAIL_SUBSCRIPTION + ).first.updated_at + 3600 + ).to_s + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'updates a member mobile' do + person_data['mobile'] = '0455123456' + + expect(UpsertMember).to( + receive(:new).with( + { + phones: [{ phone: '0455123456' }], + external_ids: { 'nation_builder' => person_data['id'] } + }, + ignore_name_change: false, + entry_point: 'nation_builder' + ).and_call_original + ) + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'subscribes a member to sms' do + person_data['mobile_opt_in'] = true + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'unsubscribes a member from sms' do + member.subscribe_to(Subscription::SMS_SUBSCRIPTION) + person_data['mobile_opt_in'] = false + person_data['updated_at'] = ( + member.member_subscriptions.where( + subscription: Subscription::SMS_SUBSCRIPTION + ).first.updated_at + 3600 + ).to_s + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'updates a member landline' do + person_data['phone'] = '0255551234' + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'subscribes a member to calls' do + person_data['do_not_call'] = false + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'unsubscribes a member from calls' do + member.subscribe_to(Subscription::CALLING_SUBSCRIPTION) + person_data['do_not_call'] = true + person_data['updated_at'] = ( + member.member_subscriptions.where( + subscription: Subscription::CALLING_SUBSCRIPTION + ).first.updated_at + 3600 + ).to_s + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'updates a member title' do + person_data['prefix'] = 'test title' + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'updates a member gender' do + person_data['sex'] = 'test gender' + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) + end + + it 'updates a member address' do + person_data['home_address'] = { + 'address1' => 'Test 1', + 'address2' => 'Test 2', + #'address3' => 'Test 3', + 'city' => 'Testing', + 'state' => 'TST', + 'zip' => '9999', + 'country_code' => 'TT', + } + + expect(serialiser.upsert_from_person(person_data, :merge)).to eq(true) + expect(serialiser).to eq(person_data) end end end From a4abb0d560edfd0a97536ab0199f87313a1a95ca Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 24 Jun 2022 10:30:49 +1000 Subject: [PATCH 21/22] Rename NB Person serialiser The old `NationBuilderMemberSyncPushSerializer` name is redundant with the `IdentityNationBuilder` namespace and is not specific to syncs or pushes. It's also rather long. So call it `PersonSerialzer` instead. --- ..._sync_push_serializer.rb => person_serializer.rb} | 8 +++++++- ..._serializer_spec.rb => person_serializer_spec.rb} | 12 ++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) rename app/serializers/identity_nation_builder/{nation_builder_member_sync_push_serializer.rb => person_serializer.rb} (97%) rename spec/serializers/{nation_builder_member_sync_push_serializer_spec.rb => person_serializer_spec.rb} (95%) diff --git a/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb b/app/serializers/identity_nation_builder/person_serializer.rb similarity index 97% rename from app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb rename to app/serializers/identity_nation_builder/person_serializer.rb index d11dd82..c36be90 100644 --- a/app/serializers/identity_nation_builder/nation_builder_member_sync_push_serializer.rb +++ b/app/serializers/identity_nation_builder/person_serializer.rb @@ -1,5 +1,10 @@ module IdentityNationBuilder - class NationBuilderMemberSyncPushSerializer < ActiveModel::Serializer + + # + # Handles serialisation, sdeserialisation, and merging of a NB + # Person with an Id Member + # + class PersonSerializer < ActiveModel::Serializer attributes :prefix, :first_name, :middle_name, :last_name, :sex, :email, :email_opt_in, @@ -232,6 +237,7 @@ def ==(person_data) equal = true attributes.each { |attr,value| + puts " compare: #{attr}: #{id_data[attr]}/#{person_data[attr.to_s]}" equal = id_data[attr] == person_data[attr.to_s] break unless equal } diff --git a/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb b/spec/serializers/person_serializer_spec.rb similarity index 95% rename from spec/serializers/nation_builder_member_sync_push_serializer_spec.rb rename to spec/serializers/person_serializer_spec.rb index 9adadac..f7c2ff2 100644 --- a/spec/serializers/nation_builder_member_sync_push_serializer_spec.rb +++ b/spec/serializers/person_serializer_spec.rb @@ -1,7 +1,7 @@ $nb_id = 0 -describe IdentityNationBuilder::NationBuilderMemberSyncPushSerializer do +describe IdentityNationBuilder::PersonSerializer do let!(:country_code) { '61' } before(:each) do @@ -27,7 +27,7 @@ it 'returns valid object' do rows = ActiveModel::Serializer::CollectionSerializer.new( @batch_members, - serializer: IdentityNationBuilder::NationBuilderMemberSyncPushSerializer + serializer: IdentityNationBuilder::PersonSerializer ).as_json expect(rows.count).to eq(2) expect(rows[0][:email]).to eq(ListMember.first.member.email) @@ -45,7 +45,7 @@ it 'returns valid object' do rows = ActiveModel::Serializer::CollectionSerializer.new( @batch_members, - serializer: IdentityNationBuilder::NationBuilderMemberSyncPushSerializer + serializer: IdentityNationBuilder::PersonSerializer ).as_json expect(rows[0][:phone]).to eq(@member.phone_numbers.landline.first.phone) expect(rows[0][:mobile]).to eq(@member.phone_numbers.mobile.first.phone) @@ -55,7 +55,7 @@ context 'serialiser data equality' do let!(:equality_member) { FactoryBot.create(:member_with_mobile) } let!(:serialiser) { - IdentityNationBuilder::NationBuilderMemberSyncPushSerializer.new(equality_member) + IdentityNationBuilder::PersonSerializer.new(equality_member) } let!(:person_data) { # convert keys to strings since NB lib returns data with string keys @@ -88,7 +88,7 @@ context 'temporal comparisons' do let!(:temporal_member) { FactoryBot.create(:member_with_mobile) } let!(:serialiser) { - IdentityNationBuilder::NationBuilderMemberSyncPushSerializer.new(temporal_member) + IdentityNationBuilder::PersonSerializer.new(temporal_member) } let!(:newer_data) { data = serialiser.serializable_hash @@ -135,7 +135,7 @@ member } let!(:serialiser) { - IdentityNationBuilder::NationBuilderMemberSyncPushSerializer.new(member) + IdentityNationBuilder::PersonSerializer.new(member) } let!(:person_data) { # convert keys to strings since NB lib returns data with string keys From 0ad7927c92ffee6b7b6ac1f0cc5814b2601a5ca7 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 24 Jun 2022 10:32:38 +1000 Subject: [PATCH 22/22] WIP: Rework NB API for upserting people to do a full bidi merge/sync XXX This is not yet complete or functional. --- app/models/identity_nation_builder/api.rb | 194 ++++--- .../identity_nation_builder/api_spec.rb | 507 ++++++++++-------- 2 files changed, 396 insertions(+), 305 deletions(-) diff --git a/app/models/identity_nation_builder/api.rb b/app/models/identity_nation_builder/api.rb index b756f5d..cebef5c 100644 --- a/app/models/identity_nation_builder/api.rb +++ b/app/models/identity_nation_builder/api.rb @@ -2,42 +2,8 @@ module IdentityNationBuilder class API - def self.rsvp(site_slug, members, event_id, mark_as_attended=false, recruiter_id=nil) - member_ids = members.map do |member| - identity_id = member[:id] - member = member.except(:id, :nationbuilder_id) - person = find_or_create_person(member) - response = rsvp_person(site_slug, event_id, person, mark_as_attended, recruiter_id) - if !response.try(:[], 'rsvp') && mark_as_attended - pager = NationBuilder::Paginator.new(get_api_client, event_rsvps(site_slug, event_id)) - rsvp = pager.body['results'].select { |result| result['person_id'] == person['id'] }.first - if rsvp && !rsvp['attended'] - api( - :events, :rsvp_update, { - rsvp_id: rsvp['id'], - event_id: rsvp['event_id'], - site_slug: site_slug, - rsvp: { attended: mark_as_attended, person_id: rsvp['person_id'] } - }) - end - end - - { identity_id: identity_id, nationbuilder_id: person['id'] } - end - yield member_ids.length, member_ids - end - - def self.tag(site_slug, members, tag) - list_id = create_list(tag)['id'] - member_ids = members.map do |member| - identity_id = member[:id] - member = member.except(:id, :nationbuilder_id) - { identity_id: identity_id, nationbuilder_id: find_or_create_person(member)['id'] } - end - nationbuilder_ids = member_ids.map { |member| member[:nationbuilder_id] } - add_people_list(list_id, nationbuilder_ids) - tag_list(list_id, tag) - yield member_ids.length, member_ids + def self.to_slug(name) + slug = name.parameterize(separator: "_") end def self.sites @@ -99,13 +65,92 @@ def self.event_rsvps(site_slug, event_id) api(:events, :rsvps, { site_slug: site_slug, id: event_id, per_page: 100 }) end - def self.find_or_create_person(member) - person = find_person_by_mobile_or_phone(member) - person ? person : upsert_person(member) + def self.person_upsert(id_member) + id_data = PersonSerializer.new(id_member) + nb_id = id_member.member_external_ids.with_system(SYSTEM_NAME).first&.external_id + person_data = nil + + if nb_id != nil + # Member has an NB id already, so just fetch that. + # + # XXX what about multiple NB ids? + # + # Id external ids are strings, the NB API returns ints though, so + # keep the id consistent with NB. + nb_id = Integer(nb_id) + person_data = api(:people, :show, { id: nb_id })['person'] + else + # Don't know the member's NB id, so need to try to find + # them. Search by email first since that's the best indicator + # to match data on, then try the phones + + response = {} + + if id_member.email.present? + response = api(:people, :match, { email: id_member.email }) + end + + if !response.has_key?('person') && id_data.mobile.present? + response = api(:people, :match, { mobile: id_data[:mobile] }) + end + + if !response.has_key?('person') && id_data.phone.present? + response = api(:people, :match, { phone: id_data[:phone] }) + end + + if response.has_key?('person') + person_data = response['person'] + nb_id = person_data['id'] + end + end + + if person_data + # Have or found a NB person that is associated with the Id + # member. + # + # If there is no NB external system id already present on the + # Id member, assume the two records have not been synchronised + # before, and perform a full merge. That is, if both records + # have a different value, use the most recently updated + # record's value, but if one record does not have a value and + # the other does, always use that value (disregarding which + # was most recently updated), so no data is lost. + # + # If there is a NB external system id present, assume the two + # have been merged merged already and always use the more + # recent values for each attribute. + # + # Finally, only do this work if the data does not match, to + # avoid uncessary work and help acheive a steady state + if id_data != person_data + upsert_type = :overwrite + + if !id_member.member_external_ids.with_system(SYSTEM_NAME).first.present? + upsert_type = :merge + # set the external id here so UpsertService can find the + # member + id_member.update_external_id(SYSTEM_NAME, nb_id) + end + + id_data.upsert_from_person(person_data, upsert_type) + + if id_data != person_data + # Id and NB person data still doesn't match, so + # some more recent updates. Push these to NB. + api(:people, :update, { id: nb_id, person: id_data.as_json } )['person']['id'] + end + end + else + # Don't have an NB id and couldn't find one, so create them in NB + nb_id = api(:people, :create, { person: id_data })['person']['id'] + id_member.update_external_id(SYSTEM_NAME, nb_id) + end + + nb_id end - def self.rsvp_person(site_slug, event_id, person, attended=false, recruiter_id=nil) - rsvp_data = { person_id: person['id'] } + def self.rsvp_person(site_slug, event_id, person_id, attended=false, recruiter_id=nil) + rsvp_data = { person_id: person_id } rsvp_data[:attended] = true if attended rsvp_data[:recruiter_id] = recruiter_id if recruiter_id api(:events, :rsvp_create, { id: event_id, site_slug: site_slug, rsvp: rsvp_data }) @@ -129,22 +174,32 @@ def self.all_lists list_results end - def self.lists - api(:lists, :index, { per_page: 100 }) - end - - def self.create_list(tag) - slug = "tempid_#{SecureRandom.hex(11)}" - name = "Temp list for tag - #{tag}" - api(:lists, :create, { list: { name: name, slug: slug, author_id: Settings.nation_builder.author_id } })['list_resource'] + def self.list_find(name) + id = nil + api(:lists, :index, { per_page: 100 })['results'].each do |result| + if result['name'] == name + id = result['id'] + break + end + end + raise RuntimeError, 'no such list with slug: #{slug}' unless id + id end - def self.add_people_list(list_id, member_ids) - api(:lists, :add_people, { list_id: list_id, people_ids: member_ids }) + def self.list_create(name) + slug = to_slug(name) + api(:lists, :create, + { list: { + name: name, + slug: slug, + author_id: Settings.nation_builder.author_id + } + } + )['list_resource']['id'] end - def self.tag_list(list_id, tag) - api(:lists, :add_tag, { list_id: list_id, tag: URI.encode_www_form_component(tag) }) + def self.list_add_people(list_id, member_ids) + api(:lists, :add_people, { list_id: list_id, people_ids: member_ids })['id'] end def self.recruiters @@ -161,44 +216,11 @@ def self.cached_recruiters private - def self.find_person_by_mobile_or_phone(member) - phone_to_lookup = member[:mobile].present? ? member[:mobile] : member[:phone] - if phone_to_lookup.present? - phone_to_lookup = strip_leading_zero(phone_to_lookup) - phone_type = is_mobile?(phone_to_lookup) ? "mobile" : "phone" - response = api(:people, :match, { phone_type => phone_to_lookup }) - matched_person = response['person'] - return matched_person if matched_person - end - end - - def self.strip_leading_zero(phone) - phone.gsub(/^0/, '') - end - def self.is_mobile?(phone) mobile_prefix = Settings.options.default_mobile_phone_national_destination_code.to_s mobile_prefix && phone =~ /^#{mobile_prefix}/ end - def self.upsert_person(member) - begin - upsert_person_without_retry(member) - rescue NationBuilder::ClientError => response - validation_errors = JSON.parse(response.message)['validation_errors'] - if validation_errors.try(:first).try(:match, /should look like an email address/) - upsert_person_without_retry(member.except(:email)) - else - raise response - end - end - end - - - def self.upsert_person_without_retry(member) - api(:people, :add, { person: member })['person'] - end - def self.api(endpoint, method, params) started_at = DateTime.now begin diff --git a/spec/models/identity_nation_builder/api_spec.rb b/spec/models/identity_nation_builder/api_spec.rb index 2dcc2b8..601cc24 100644 --- a/spec/models/identity_nation_builder/api_spec.rb +++ b/spec/models/identity_nation_builder/api_spec.rb @@ -1,11 +1,295 @@ require 'spec_helper' +$nb_id = 0 + describe IdentityNationBuilder::API do before do - allow(Settings).to receive_message_chain(:nation_builder, :site).and_return('test') - allow(Settings).to receive_message_chain(:nation_builder, :token).and_return('test') - allow(Settings).to receive_message_chain(:nation_builder, :debug).and_return(false) - allow(Settings).to receive_message_chain(:options, :default_mobile_phone_national_destination_code).and_return(4) + allow(Settings).to receive_message_chain(:nation_builder, :site).and_return('test_site') + allow(Settings).to receive_message_chain(:nation_builder, :token).and_return('test_token') + allow(Settings).to receive_message_chain(:nation_builder, :author_id).and_return(1234) + allow(Settings).to receive_message_chain(:nation_builder, :debug).and_return(true) + allow(Settings).to( + receive_message_chain(:options, :default_phone_country_code) { country_code } + ) + allow(Settings).to( + receive_message_chain(:options, :default_mobile_phone_national_destination_code) { '4' } + ) + allow(Settings).to( + receive_message_chain(:options, :allow_subscribe_via_upsert_member) { true } + ) + end + + describe 'person_upsert' do + let(:nb_id) { ($nb_id += 1) } + + context 'a member with a nationbuilder id' do + let!(:member_with_email) { + member = FactoryBot.create(:member) + member.update_external_id('nation_builder', nb_id) + member + } + let!(:person_with_email) { + serialiser = IdentityNationBuilder::PersonSerializer.new(member_with_email) + # convert keys to strings since NB lib returns data with string keys + data = Hash[serialiser.serializable_hash.map { |k,v| [k.to_s, v] }] + data['id'] = nb_id + data['updated_at'] = member_with_email.updated_at.to_s + data + } + let(:updated_at_earlier) { (member_with_email.updated_at - 60).to_s } + let(:updated_at_later) { (member_with_email.updated_at + 60).to_s } + + it 'update member in Identity when NB more recently updated' do + match = stub_request( + :get, 'https://test_site.nationbuilder.com/api/v1/people/' + nb_id.to_s + ).to_return( + status: 200, + headers: { 'Content-Type': 'application/json' }, + body: { + 'person': person_with_email.update({ + 'first_name': 'updated_test_name', + 'updated_at': updated_at_later + }) + }.to_json + ) + + person_id = IdentityNationBuilder::API.person_upsert(member_with_email) + + expect(match).to have_been_made.times(1) + expect(person_id).to eq(nb_id) + expect(member_with_email.first_name).to eq('updated_test_name') + expect(member_with_email.member_external_ids.with_system('nation_builder').first).to eq(1000) + end + end + + context 'a member without a nationbuilder id' do + let(:updated_at_earlier) { (member_with_email.updated_at - 60).to_s } + let(:updated_at_later) { (member_with_email.updated_at + 60).to_s } + + it 'should find a more recently modified person in NB by email and update Id' do + match = stub_request(:get, 'https://test_site.nationbuilder.com/api/v1/people/match') + .with(query: hash_including({ email: @member_with_email.email})) + .to_return( + status: 200, + headers: { 'Content-Type': 'application/json' }, + body: { + 'person': { + 'id': 1000, + 'email': 'updated@test.com', + 'updated_at': updated_at_later, + } + }.to_json + ) + + person_id = IdentityNationBuilder::API.person_upsert(member_with_email) + + expect(match).to have_been_made.times(1) + expect(person_id).to eq(1000) + expect(member_with_email.email).to eq('updated@test.com') + expect(member_with_email.member_external_ids.with_system('nation_builder').first).to eq(1000) + end + + it 'should find a less recently modified person in NB by email and update both' do + match = stub_request(:get, 'https://test_site.nationbuilder.com/api/v1/people/match') + .with(query: hash_including({ email: @member_with_email.email})) + .to_return( + status: 200, + headers: { 'Content-Type': 'application/json' }, + body: { + 'person': { + 'id': 1000, + 'email': 'previously@test.com', + 'updated_at': updated_at_earlier, + } + }.to_json + ) + create = stub_request(:put, 'https://test_site.nationbuilder.com/api/v1/people/1000') + .to_return( + status: 201, + headers: { 'Content-Type': 'application/json' }, + body: { + 'person': { + 'id': 1000, + 'email': @member_with_email.email, + } + }.to_json + ) + + person_id = IdentityNationBuilder::API.person_upsert(@member_with_email) + + expect(match).to have_been_made.times(1) + expect(create).to have_been_made.times(1) + expect(person_id).to eq(1000) + expect(id_member.member_external_ids.with_system('nation_builder').first).to eq(1000) + end + + it 'should upsert an existing member with a mobile' do + mobile = @member_with_mobile.phone_numbers.mobile.first.phone.gsub(/^61/, '0') + match = stub_request(:get, 'https://test_site.nationbuilder.com/api/v1/people/match') + .with(query: hash_including({ mobile: mobile })) + .to_return( + status: 200, + headers: { 'Content-Type': 'application/json' }, + body: { + 'person': { 'id': 1000 } + }.to_json + ) + create = stub_request(:put, 'https://test_site.nationbuilder.com/api/v1/people/1000') + .to_return( + status: 201, + headers: { 'Content-Type': 'application/json' }, + body: { + 'person': { 'id': 1000 } + }.to_json + ) + + person_id = IdentityNationBuilder::API.person_upsert(@member_with_mobile) + + expect(match).to have_been_made.times(1) + expect(create).to have_been_made.times(1) + expect(person_id).to eq(1000) + end + + it 'should upsert an existing member with a landline' do + landline = @member_with_landline.phone_numbers.landline.first.phone.gsub(/^61/, '0') + match = stub_request(:get, %r{/api/v1/people/match}) + .with(query: hash_including({ phone: landline })) + .to_return( + status: 200, + headers: { 'Content-Type': 'application/json' }, + body: { + 'person': { 'id': 1000 } + }.to_json + ) + create = stub_request(:put, 'https://test_site.nationbuilder.com/api/v1/people/1000') + .to_return( + status: 201, + headers: { 'Content-Type': 'application/json' }, + body: { + 'person': { 'id': 1000 } + }.to_json + ) + + person_id = IdentityNationBuilder::API.person_upsert(@member_with_landline) + + expect(match).to have_been_made.times(1) + expect(create).to have_been_made.times(1) + expect(person_id).to eq(1000) + end + end + end + + describe 'lists' do + it 'list_create should a create new list' do + create = stub_request(:post, 'https://test_site.nationbuilder.com/api/v1/lists') + .with(body: hash_including({ list: { name: 'Test List', slug: 'test_list', author_id: 1234} })) + .to_return( + status: 201, + headers: { 'Content-Type': 'application/json' }, + body: { + "list_resource": { + "id": 12, + "name": "Test List", + "slug": "test_list", + } + }.to_json + ) + + list_id = IdentityNationBuilder::API.list_create('Test List') + + expect(create).to have_been_made.times(1) + expect(list_id).to eq(12) + end + + it 'list_create should raise an error if a list exists' do + create = stub_request(:post, 'https://test_site.nationbuilder.com/api/v1/lists') + .with(body: hash_including({ list: { name: 'Test List', slug: 'test_list', author_id: 1234} })) + .to_return( + status: 400, + headers: { 'Content-Type': 'application/json' }, + body: { + "code": "validation_failed", + "message": "Validation Failed.", + "validation_errors": [ "slug has already been taken" ] + }.to_json + ) + + expect { IdentityNationBuilder::API.list_create('Test List') } + .to raise_error(NationBuilder::ClientError) + expect(create).to have_been_made.times(1) + end + + it 'list_find should return the list if found' do + get = stub_request(:get, 'https://test_site.nationbuilder.com/api/v1/lists') + .with(query: hash_including({ per_page: '100' })) + .to_return( + status: 200, + headers: { 'Content-Type': 'application/json' }, + body: { + "next": nil, + "prev": nil, + "results": [ + { + "id": 10, + "name": "Test List", + "slug": "test_list", + "author_id": 1, + "count": 5 + } + ] + }.to_json + ) + + list_id = IdentityNationBuilder::API.list_find('Test List') + + expect(get).to have_been_made.times(1) + expect(list_id).to eq(10) + end + + it 'list_find should raise an error if the list is not found' do + get = stub_request(:get, 'https://test_site.nationbuilder.com/api/v1/lists') + .with(query: hash_including({ per_page: '100' })) + .to_return( + status: 200, + headers: { 'Content-Type': 'application/json' }, + body: { + "next": nil, + "prev": nil, + "results": [ + { + "id": 10, + "name": "Test List", + "slug": "test_list", + "author_id": 1, + "count": 5 + } + ] + }.to_json + ) + + expect { IdentityNationBuilder::API.list_find('Other List') } + .to raise_error(RuntimeError) + expect(get).to have_been_made.times(1) + end + + it 'list_add_people create should add ids' do + add = stub_request(:post, 'https://test_site.nationbuilder.com/api/v1/lists/100/people') + .with(body: hash_including({ people_ids: [1, 2, 3] })) + .to_return( + status: 201, + headers: { 'Content-Type': 'application/json' }, + body: { + "id": 100, + "name": "Test List", + "slug": "test_list", + }.to_json + ) + + list_id = IdentityNationBuilder::API.list_add_people(100, [1, 2, 3]) + + expect(add).to have_been_made.times(1) + expect(list_id).to eq(100) + end end describe '.sites_events' do @@ -43,221 +327,6 @@ end end - describe '.find_or_create_person' do - context 'with an invalid email' do - let!(:invalid_email) { 'invalid@email' } - let(:validation_failed_response) { - { - status: 400, - headers: { 'Content-Type' => 'application/json' }, - body: { - "code": "validation_failed", - "message": "Validation Failed.", - "validation_errors": [ "email 'test@invalid' should look like an email address" ] - }.to_json - } - } - let(:successful_response) { - { - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { person: [ { "mobile": "0401000000" } ] }.to_json - } - } - let!(:people_add_endpoint) { - stub_request(:put, %r{people/add}) - .to_return { |request| - if request.body.include?(invalid_email) - validation_failed_response - else - successful_response - end - } - } - - it 'should strip the email and retry' do - IdentityNationBuilder::API.find_or_create_person({ "email": invalid_email }) - expect(people_add_endpoint).to have_been_requested.twice - end - end - - context "with a user whose mobile that matches a signup in NationBuilder" do - let!(:mobile) { '0468519266' } - let!(:people_add_endpoint) { - stub_request(:put, %r{people/add}).and_return({ status: 400, body: {}.to_json}) - } - let!(:member_data) { - { mobile: mobile, phone: '1111111111', email: 'test@test.com' } - } - - it 'should should match the record on mobile (without leading zero), return the id but not update the record' do - people_match_endpoint = stub_request(:get, %r{people/match}) - .to_return { |request| - expect(request.uri.query_values).to include('mobile') - expect(request.uri.query_values['mobile']).to eq('468519266') - { - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { person: [ { "mobile": mobile } ] }.to_json - } - } - IdentityNationBuilder::API.find_or_create_person(member_data) - expect(people_match_endpoint).to have_been_requested - expect(people_add_endpoint).to_not have_been_requested - end - end - - context "with a user without a mobile but whose phone that matches a signup in NationBuilder" do - let!(:phone) { '0295700000' } - let!(:people_add_endpoint) { - stub_request(:put, %r{people/add}).and_return({ status: 400, body: {}.to_json}) - } - let!(:member_data) { - { mobile: '', phone: phone, email: 'test@test.com' } - } - - it 'should should match the record on phone (with leading zero removed), return the id but not update the record' do - people_match_endpoint = stub_request(:get, %r{people/match}) - .to_return { |request| - expect(request.uri.query_values).to include('phone') - expect(request.uri.query_values['phone']).to eq('295700000') - { - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { person: [ { "phone": phone } ] }.to_json - } - } - IdentityNationBuilder::API.find_or_create_person(member_data) - expect(people_match_endpoint).to have_been_requested - expect(people_add_endpoint).to_not have_been_requested - end - end - - context "with hose mobile DOES NOT matches a signup in NationBuilder" do - let!(:mobile) { '61468519266' } - let!(:people_match_endpoint) { - stub_request(:get, %r{people/match}) - .to_return { |request| - { - status: 400, - headers: { 'Content-Type' => 'application/json' }, - body: { "code": "no_matches", "message": "No people matched the given criteria." }.to_json - } - } - } - let!(:people_add_endpoint) { - stub_request(:put, %r{people/add}).and_return({ - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { person: {mobile: mobile, first_name: "new user"}}.to_json - }) - } - let!(:member_data) { - { mobile: mobile, phone: '1111111111', email: 'test@test.com' } - } - - it 'should upsert the user' do - IdentityNationBuilder::API.find_or_create_person(member_data) - expect(people_match_endpoint).to have_been_requested - expect(people_add_endpoint).to have_been_requested - end - end - - describe '.tag_list' do - let!(:tag) { 'will: barnstorm' } - - it 'should url encode the tag' do - tag_request = stub_request(:post, %r{lists/1/tag/will:%20barnstorm}) - IdentityNationBuilder::API.api(:lists, :add_tag, { list_id: 1, tag: tag }) - expect(tag_request).to have_been_requested - end - end - - describe '.rsvp' do - let!(:identity_id) { 10 } - let!(:nationbuilder_id) { 20 } - let!(:event_id) { 30 } - let!(:recruiter_id) { 40 } - let!(:member) { { id: identity_id, mobile: '04000000000'} } - let!(:person) { { id: nationbuilder_id, mobile: '04000000000'} } - let!(:mark_as_attended) { true } - - context 'with no existing rsvp' do - it 'should call the rsvp/create endpoint with attended set' do - people_match_endpoint = stub_request(:get, %r{people/match}) - .to_return { - { - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { person: person }.to_json - } - } - rsvp_request = stub_request(:post, %r{/sites/test/pages/events/30/rsvps}) - .with(body: hash_including(rsvp: { person_id: nationbuilder_id, attended: true, recruiter_id: recruiter_id})) - .to_return({ - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { rsvp: { person_id: nationbuilder_id, attended: true }}.to_json - }) - IdentityNationBuilder::API.rsvp('test', [member], event_id, mark_as_attended, recruiter_id) { |length, members| - expect(length).to eq(1) - expect(members).to eq([{ identity_id: identity_id, nationbuilder_id: nationbuilder_id }]) - } - expect(people_match_endpoint).to have_been_requested - expect(rsvp_request).to have_been_requested - end - end - - context 'with an existing rsvp' do - it 'should update the rsvp if attended is set' do - people_match_endpoint = stub_request(:get, %r{people/match}) - .to_return { - { - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { person: person }.to_json - } - } - rsvp_create_request = stub_request(:post, %r{/sites/test/pages/events/30/rsvps}) - .with(body: hash_including(rsvp: { person_id: nationbuilder_id, attended: true})) - .to_return({ - status: 400, - headers: { 'Content-Type' => 'application/json' }, - body: { - "code": "validation_failed", - "message": "Validation Failed.", - "validation_errors": [ "signup_id has already been taken" ] - }.to_json - }) - rsvp_list_request = stub_request(:get, %r{pages/events/30/rsvps}) - .to_return({ - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { - results: [{ id: 12222, event_id: event_id, person_id: nationbuilder_id, attended: false }] - }.to_json - }) - rsvp_update_request = stub_request(:put, %r{pages/events/30/rsvps}) - .to_return({ - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { - rsvp: { id: 12222, event_id: event_id, person_id: nationbuilder_id, attended: false } - }.to_json - }) - IdentityNationBuilder::API.rsvp('test', [member], event_id, mark_as_attended) { |length, members| - expect(length).to eq(1) - expect(members).to eq([{ identity_id: identity_id, nationbuilder_id: nationbuilder_id }]) - } - expect(people_match_endpoint).to have_been_requested - expect(rsvp_create_request).to have_been_requested - expect(rsvp_list_request).to have_been_requested - expect(rsvp_update_request).to have_been_requested - end - end - end - end - describe '.recruiters' do let!(:organisations) { [ { id: 1, last_name: 'Recruiter 1' }, { id: 2, last_name: 'Recruiter 2' } ] } let!(:tags_endpoint) {