Stop matching by phone number when upserting members in Identity

app/models/identity_spoke/campaign_contact.rb

@@ -36,5 +36,58 @@ def self.remove_exclusions_from_batch(campaign_id, batch)
                        .pluck(:cell)
       batch.reject { |row| existing_cells.include?(row[:cell]) }
     end
+
+    def upsert_member
+      # Upsert this contacted member in Identity.
+      #
+      # Use the campaign contact's external_id if known so that the
+      # correct member gets used, irrespective of the phone number.
+      # If no external is set that's fine - a new member will always
+      # be created for the contact instead. But if so update the
+      # contact with the new member's external id for it for future
+      # upserts.
+      #
+      # See also User.upsert_member.
+      #
+      # Notes about the arguments here:
+      #
+      # 1. Don't include this campaign contact as an Identity external
+      # id since they are per-Spoke-campaign and will be different for
+      # each campaign.
+      #
+      # 2. Don't subscribe campaign contacts. They should be
+      # subscribed already if they were pushed from Id, if not
+      # (e.g. from a manual CSV upload) then they should be subscribed
+      # manually in Id if appropriate.
+      #
+      # 3. Don't match based on phone numbers. As of 2024, our member
+      # phone number data is in super bad shape, with the same number
+      # appearing on multiple members. As a result, matching on phone
+      # number only is as likely to be wrong as it is right.
+      #
+      # 4. Don't change anyone's names, the names here will likely be
+      # sourced from Id anyway, and if not, should be treated as less
+      # reliable than that in Id anyway
+      member = UpsertMember.call(
+        {
+          phones: [
+            { phone: cell.sub(/^[+]*/, '') }
+          ],
+          firstname: first_name,
+          lastname: last_name,
+          member_id: external_id.present? ? external_id.to_i : nil,
+          ignore_phone_number_match: true,
+        },
+        entry_point: "#{SYSTEM_NAME}",
+        new_member_opt_in: false,
+        ignore_name_change: true,
+      )
+
+      if external_id.blank?
+        update!(external_id: member.id.to_s)
+      end
+
+      return member
+    end
   end
 end

app/models/identity_spoke/user.rb

@@ -1,7 +1,61 @@
 module IdentitySpoke
-  class User < ReadOnly
+  class User < ReadWrite
     self.table_name = "user"
     has_many :assignment, dependent: nil
     has_many :message, dependent: nil
+
+    def upsert_member
+      # Upsert this Spoke backend account in Identity.
+      #
+      # Hook up both the Spoke account's id to Identity external id,
+      # and vice-versa. We can do this here since unlike a
+      # CampaignContact, User entities are long lived.
+      #
+      # See also CampaignContact.upsert_member.
+      #
+      # Notes about the arguments here:
+      #
+      # 1. Don't subscribe accounts. They should be staff or vollies
+      # and hence subscribed already. If not, something has gone
+      # wrong, but also they probably haven't been asked for consent.
+      #
+      # 2. Don't match based on phone numbers. As of 2024, our member
+      # phone number data is in super bad shape, with the same number
+      # appearing on multiple members. As a result, matching on phone
+      # number only is as likely to be wrong as it is right.
+      #
+      # 3. Don't change anyone's names, the names here will likely be
+      # sourced from Id anyway, and if not, should be treated as less
+      # reliable than that in Id anyway.
+      if self.extra.nil?
+        self.extra = {}
+      end
+
+      member = UpsertMember.call(
+        {
+          emails: [
+            { email: email }
+          ],
+          phones: [
+            { phone: cell.sub(/^[+]*/, '') }
+          ],
+          firstname: first_name,
+          lastname: last_name,
+          member_id: self.extra.dig(:identity, :member_id),
+          external_ids: {
+            spoke: id.to_s
+          },
+          ignore_phone_number_match: true
+        },
+        entry_point: "#{SYSTEM_NAME}",
+        new_member_opt_in: false,
+        ignore_name_change: true
+      )
+
+      self.extra[:identity] = { member_id: member.id }
+      save!
+
+      return member
+    end
   end
 end

app/serializers/identity_spoke/spoke_member_sync_push_serializer.rb

@@ -3,7 +3,7 @@ class SpokeMemberSyncPushSerializer < ActiveModel::Serializer
     attributes :external_id, :first_name, :last_name, :cell, :campaign_id, :custom_fields
 
     def external_id
-      @object.id
+      @object.id.to_s
     end
 
     def first_name

lib/identity_spoke.rb

@@ -137,24 +137,43 @@ def self.fetch_new_opt_outs_impl(sync_id, force)
 
     iteration_method = force ? :find_each : :each
 
+    subscription = Subscription.find_sole_by(
+      id: Settings.spoke.subscription_id
+    )
+
     updated_opt_outs.send(iteration_method) do |opt_out|
       Rails.logger.info "#{SYSTEM_NAME.titleize} #{sync_id}: Handling opt-out: #{opt_out.id}/#{opt_out.created_at.utc.to_fs(:inspect)}"
 
-      campaign_contact = IdentitySpoke::CampaignContact.where(cell: opt_out.cell).last
-      if campaign_contact
-        contactee = UpsertMember.call(
-          {
-            phones: [{ phone: campaign_contact.cell.sub(/^[+]*/, '') }],
-            firstname: campaign_contact.first_name,
-            lastname: campaign_contact.last_name,
-            member_id: campaign_contact.external_id
-          },
-          entry_point: "#{SYSTEM_NAME}",
-          ignore_name_change: false
+      # Spoke only updates CampaignContact.is_opted_out in a batch,
+      # and doesn't update the updated_at column when it does. So just
+      # look for the most recent campaign contact with matching phone
+      # number instead.
+      #
+      # If that contact has an external_id use that to look up the
+      # specific member, otherwise we need to opt out everyone with
+      # that phone number - even though our data is low quality such
+      # that that we may have multiple members with the opted out
+      # number, the actual person with that number doesn't want to
+      # hear from us so we can't leave any subscribed.
+
+      cell = opt_out.cell
+      campaign_contact = IdentitySpoke::CampaignContact.where(cell: cell).last!
+      if campaign_contact.external_id.present?
+        members = [Member.find_sole_by(id: campaign_contact.external_id.to_i)]
+      else
+        members = Member.select(:id).distinct.joins(:phone_numbers).where(
+          phone_numbers: {
+            phone: PhoneNumber.standardise_phone_number(cell)
+          }
         )
-        subscription = Subscription.find(Settings.spoke.subscription_id)
-        contactee.unsubscribe_from(subscription, reason: 'spoke:opt_out', event_time: DateTime.now) if contactee
+        if members.size == 0
+          members = [campaign_contact.upsert_member()]
+        end
       end
+
+      members.each { |member|
+        member.unsubscribe_from(subscription, reason: 'spoke:opt_out')
+      }
     end
 
     unless updated_opt_outs.empty?
@@ -256,42 +275,22 @@ def self.handle_new_message(sync_id, message)
       return
     end
 
-    ## Create Member for campaign contact
-    campaign_contact_member = UpsertMember.call(
-      {
-        phones: [{ phone: campaign_contact.cell.sub(/^[+]*/, '') }],
-        firstname: campaign_contact.first_name,
-        lastname: campaign_contact.last_name,
-        member_id: campaign_contact.external_id
-      },
-      entry_point: "#{SYSTEM_NAME}",
-      ignore_name_change: false
-    )
-
-    user = message.user
-    if user
-      user_member = UpsertMember.call(
-        {
-          phones: [{ phone: user.cell.sub(/^[+]*/, '') }],
-          firstname: user.first_name,
-          lastname: user.last_name
-        },
-        entry_point: "#{SYSTEM_NAME}",
-        ignore_name_change: false
-      )
-    else
-      user_member = UpsertMember.call(
-        {
-          phones: [{ phone: message.user_number.sub(/^[+]*/, '') }],
-        },
-        entry_point: "#{SYSTEM_NAME}",
-        ignore_name_change: false
-      )
-    end
-
-    ## Assign the contactor and contactee according to if the message was from the campaign contact
-    contactor = message.is_from_contact ? campaign_contact_member : user_member
-    contactee = message.is_from_contact ? user_member : campaign_contact_member
+    # Upsert contacted member
+    contactee = campaign_contact.upsert_member
+
+    # Upsert contacting member. If a message is outgoing,
+    # 'message.user' will be set, otherwise it will be null since
+    # anyone could reply to the message.
+    #
+    # However for incoming messages, since we don't know in advance
+    # who will rely to it, and since we only have a phone number for
+    # them (and hence can't reliably upsert them), don't try to
+    # upsert.
+    #
+    # In particular, DON'T EVER USE message.user_number - for Twilio
+    # at least it will be a generated number, completely unrelated to
+    # the Spoke user's actual number.
+    contactor = message.user.present? ? message.user.upsert_member() : nil
 
     ## Find or create the contact campaign
     contact_campaign = handle_campaign(campaign_contact.campaign, false)