[pull] main from forem:main

app/mailers/concerns/deliverable.rb

@@ -17,11 +17,20 @@ module Deliverable
   # stops every Forem-side broadcast anyway.
   CUSTOMERIO_BROADCAST_PASSTHROUGH_FLAG = :customerio_broadcast_passthrough
 
+  # Backoff is 2^n seconds capped at 5 minutes, so 20 holds span ~1 hour.
+  CUSTOMERIO_LINK_HOLD_ATTEMPTS = 20
+
   included do
     before_action :set_perform_deliveries
     after_action  :set_delivery_options
   end
 
+  # Kept so hold_until_linked can replay the exact mailer call.
+  def process(method_name, *args)
+    @customerio_action_args = args
+    super
+  end
+
   # Mailer actions call this before mail() to attach the Customer.io
   # transactional template id and its Liquid payload, e.g.
   #   customerio_delivery_options(transactional_message_id: "dev_new_reply_email",
@@ -49,6 +58,8 @@ def set_delivery_options
     return unless @_mail_was_called
 
     if deliver_via_customerio?
+      return hold_until_linked if customerio_recipient && customerio_mlh_uid.blank?
+
       # Deliverable on a Customer.io-only instance (no SMTP creds) must still
       # send, so the per-message flag overrides the SMTP-based default above.
       message.perform_deliveries = true
@@ -140,12 +151,37 @@ def customerio_recipient
   end
 
   # People in Customer.io are keyed by MLH Core user id (DEV profiles were
-  # stitched via the dev:<id> anonymous id); fall back to email for
-  # recipients without a linked Core account.
+  # stitched via the dev:<id> anonymous id). Users are never keyed by email --
+  # hold_until_linked waits for the id instead -- so email only identifies
+  # recipients who are not users at all.
   def customerio_identifiers
-    mlh_uid = customerio_recipient&.identities&.where(provider: "mlh")&.pick(:uid)
-    return { id: mlh_uid } if mlh_uid.present?
+    customerio_recipient ? { id: customerio_mlh_uid } : { email: mail.to.first }
+  end
+
+  def customerio_mlh_uid
+    return @customerio_mlh_uid if defined?(@customerio_mlh_uid)
+
+    @customerio_mlh_uid = customerio_recipient&.identities&.where(provider: "mlh")&.pick(:uid)
+  end
+
+  # A new user's mlh identity lands seconds after signup (76% within 5s, 97%
+  # within 2m), but signup-time mail (confirmation, magic link) goes out first.
+  # Keying that send by email would mint a Customer.io person Core's id-keyed
+  # person never merges with, so skip it and re-run the same mailer call once
+  # the link has had time to land. Users who never link are ones Core rejected
+  # (disposable domains, no email); after ~1h they are dropped, not mailed.
+  def hold_until_linked
+    message.perform_deliveries = false
+    attempt = params.to_h.fetch(:customerio_link_hold, 0) + 1
+    if attempt > CUSTOMERIO_LINK_HOLD_ATTEMPTS
+      Rails.logger.warn(
+        "[Deliverable] dropped #{self.class.name}##{action_name} for unlinked user #{customerio_recipient.id}",
+      )
+      return
+    end
 
-    { email: mail.to.first }
+    self.class.with(params.to_h.merge(customerio_link_hold: attempt))
+      .public_send(action_name, *@customerio_action_args)
+      .deliver_later(wait: [2**attempt, 300].min.seconds)
   end
 end

app/models/user.rb

@@ -1211,8 +1211,10 @@ def create_password_change_note
     )
   end
 
+  # super is Devise's !confirmed?: without it, users confirmed at creation
+  # (OAuth signups via skip_confirmation!) still get a confirmation email.
   def confirmation_required?
-    ForemInstance.smtp_enabled?
+    ForemInstance.smtp_enabled? && super
   end
 
   def update_user_roles_cache(role)

lib/data_update_scripts/20260923010000_merge_email_keyed_customerio_profiles.rb

@@ -0,0 +1,36 @@
+module DataUpdateScripts
+  # Before Deliverable held mail for users without an mlh identity, mail sent
+  # to a new user before that link landed was keyed by email. Customer.io
+  # stored that address as the person's id, leaving an orphan profile beside
+  # the one keyed by the MLH uid that nothing ever merges. Merge each orphan
+  # into its linked person.
+  #
+  # Scoped to users linked after signup since Customer.io delivery shipped
+  # (2026-07-16) -- the only accounts that could have been mailed unlinked.
+  # Users never mailed before linking have no orphan; their merges come back as
+  # per-op errors in a 207, which the client treats as success, so they never
+  # fail the batch.
+  class MergeEmailKeyedCustomerioProfiles
+    ROLLOUT_DATE = Time.zone.parse("2026-07-16")
+    # /v2/batch caps a request at 500KB; a merge op is ~150 bytes.
+    BATCH_SIZE = 1_000
+
+    def run
+      return unless ForemInstance.customerio_track_enabled?
+
+      # Batch from the first post-rollout id: in_batches walks the primary key,
+      # so a created_at filter alone scans every older user and times out.
+      start_id = User.where(created_at: ROLLOUT_DATE..).minimum(:id)
+      return unless start_id
+
+      User.where(id: start_id..).where.not(email: [nil, ""]).in_batches(of: BATCH_SIZE) do |users|
+        ops = users.joins(:identities)
+          .where(identities: { provider: "mlh" })
+          .where("identities.created_at > users.created_at")
+          .pluck("identities.uid", "users.email")
+          .map { |uid, email| { type: "person", action: "merge", primary: { id: uid }, secondary: { id: email } } }
+        CUSTOMERIO_TRACK_API.batch(ops) if ops.any?
+      end
+    end
+  end
+end

spec/lib/data_update_scripts/merge_email_keyed_customerio_profiles_spec.rb

@@ -0,0 +1,39 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20260923010000_merge_email_keyed_customerio_profiles.rb",
+)
+
+describe DataUpdateScripts::MergeEmailKeyedCustomerioProfiles do
+  let(:track_client) { instance_double(Customerio::Client, batch: nil) }
+
+  before do
+    stub_const("CUSTOMERIO_TRACK_API", track_client)
+    allow(ForemInstance).to receive(:customerio_track_enabled?).and_return(true)
+    omniauth_mock_mlh_payload
+  end
+
+  def link(user, uid, at:)
+    create(:identity, provider: "mlh", user: user, uid: uid, created_at: at)
+  end
+
+  it "merges the email-keyed profile of a user linked after signup into the linked person" do
+    user = create(:user, created_at: 1.day.ago)
+    link(user, "core-1", at: user.created_at + 30.seconds)
+
+    described_class.new.run
+
+    expect(track_client).to have_received(:batch).with(
+      [{ type: "person", action: "merge", primary: { id: "core-1" }, secondary: { id: user.email } }],
+    )
+  end
+
+  it "skips users linked before the rollout or not linked after signup" do
+    old_user = create(:user, created_at: Time.zone.parse("2026-07-01"))
+    link(old_user, "core-old", at: old_user.created_at + 30.seconds)
+    create(:user, created_at: 1.day.ago)
+
+    described_class.new.run
+
+    expect(track_client).not_to have_received(:batch)
+  end
+end

spec/mailers/concerns/deliverable_spec.rb

@@ -30,6 +30,11 @@ def built_message(to:, customerio_options: nil)
     before do
       allow(ApplicationConfig).to receive(:[]).and_call_original
       allow(ApplicationConfig).to receive(:[]).with("CUSTOMERIO_APP_KEY").and_return("app-key")
+      # The :identity factory's auth_data_dump reads OmniAuth.config.mock_auth,
+      # which has no built-in :mlh payload; register one (as other mlh-identity
+      # specs do) so the factory doesn't raise KeyError.
+      omniauth_mock_mlh_payload
+      create(:identity, provider: "mlh", user: user, uid: "core-99")
     end
 
     after { FeatureFlag.remove(Deliverable::CUSTOMERIO_FLAG) }
@@ -40,21 +45,35 @@ def built_message(to:, customerio_options: nil)
       message = built_message(to: user.email)
 
       expect(message.delivery_method).to be_a(DeliveryMethods::CustomerIo)
-      expect(message.delivery_method.settings[:identifiers]).to eq(email: user.email)
+      expect(message.delivery_method.settings[:identifiers]).to eq(id: "core-99")
       expect(message.perform_deliveries).to be(true)
     end
 
-    it "identifies the recipient by MLH Core uid when an mlh identity exists" do
-      # The :identity factory's auth_data_dump reads OmniAuth.config.mock_auth,
-      # which has no built-in :mlh payload; register one (as other mlh-identity
-      # specs do) so the factory doesn't raise KeyError.
-      omniauth_mock_mlh_payload
-      create(:identity, provider: "mlh", user: user, uid: "core-99")
-      FeatureFlag.enable(Deliverable::CUSTOMERIO_FLAG, FeatureFlag::Actor[user])
+    describe "a user recipient without an mlh identity yet" do
+      let(:unlinked) { create(:user) }
 
-      message = built_message(to: user.email)
+      before { FeatureFlag.enable(Deliverable::CUSTOMERIO_FLAG, FeatureFlag::Actor[unlinked]) }
 
-      expect(message.delivery_method.settings[:identifiers]).to eq(id: "core-99")
+      it "is not sent now, so no email-keyed Customer.io person is minted" do
+        message = built_message(to: unlinked.email)
+
+        expect(message.perform_deliveries).to be(false)
+        expect(message.delivery_method).not_to be_a(DeliveryMethods::CustomerIo)
+      end
+
+      it "re-enqueues the same mailer call to retry once the link lands" do
+        expect { built_message(to: unlinked.email) }
+          .to have_enqueued_mail(DeliverableTestMailer, :test_email)
+          .with(params: { to: unlinked.email, customerio_options: nil, customerio_link_hold: 1 }, args: [])
+      end
+
+      it "drops the message once the hold attempts are exhausted" do
+        expect do
+          DeliverableTestMailer
+            .with(to: unlinked.email, customerio_link_hold: Deliverable::CUSTOMERIO_LINK_HOLD_ATTEMPTS)
+            .test_email.message
+        end.not_to have_enqueued_mail(DeliverableTestMailer, :test_email)
+      end
     end
 
     it "passes customerio_delivery_options through to the delivery method" do
@@ -128,7 +147,7 @@ def built_message(to:, customerio_options: nil)
 
         expect(message.delivery_method).to be_a(DeliveryMethods::CustomerIoEvent)
         expect(message.delivery_method.settings[:customerio_event_name]).to eq("digest_ready")
-        expect(message.delivery_method.settings[:identifiers]).to eq(email: user.email)
+        expect(message.delivery_method.settings[:identifiers]).to eq(id: "core-99")
         expect(message.perform_deliveries).to be(true)
       end
 

spec/mailers/custom_mailer_spec.rb

@@ -172,6 +172,7 @@
         allow(ForemInstance).to receive_messages(smtp_enabled?: true, customerio_enabled?: true)
         allow(FeatureFlag).to receive(:enabled_for_user?)
           .with(Deliverable::CUSTOMERIO_FLAG, having_attributes(id: user.id)).and_return(true)
+        link_mlh_identity(user)
       end
 
       it "sends nothing -- Customer.io is already sending this broadcast" do
@@ -220,6 +221,7 @@
         )
         allow(FeatureFlag).to receive(:enabled_for_user?)
           .with(Deliverable::CUSTOMERIO_FLAG, having_attributes(id: user.id)).and_return(true)
+        link_mlh_identity(user)
         stub_const("CUSTOMERIO_API", api_client)
       end
 

spec/mailers/devise_mailer_spec.rb

@@ -145,6 +145,31 @@
         expect(email["from"].value).to eq(expected_from)
       end
     end
+
+    # Signup-time confirmations race the mlh identity link; Deliverable holds
+    # them rather than mint an email-keyed Customer.io person. DeviseMailer is
+    # not parameterized, so the replay must carry its positional args.
+    context "when routed through Customer.io before the mlh identity exists" do
+      before do
+        allow(ApplicationConfig).to receive(:[]).and_call_original
+        allow(ApplicationConfig).to receive(:[]).with("CUSTOMERIO_APP_KEY").and_return("app-key")
+        FeatureFlag.enable(Deliverable::CUSTOMERIO_FLAG, FeatureFlag::Actor[user])
+      end
+
+      after { FeatureFlag.remove(Deliverable::CUSTOMERIO_FLAG) }
+
+      it "holds the email and sends it by MLH uid once linked" do
+        held = described_class.confirmation_instructions(user, "faketoken").message
+        expect(held.perform_deliveries).to be(false)
+
+        omniauth_mock_mlh_payload
+        create(:identity, provider: "mlh", user: user, uid: "core-7")
+        replay = described_class.with(customerio_link_hold: 1).confirmation_instructions(user, "faketoken").message
+
+        expect(replay.perform_deliveries).to be(true)
+        expect(replay.delivery_method.settings[:identifiers]).to eq(id: "core-7")
+      end
+    end
   end
 
   describe "#invitation_instructions" do
@@ -255,6 +280,7 @@
         allow(ApplicationConfig).to receive(:[]).and_call_original
         allow(ApplicationConfig).to receive(:[]).with("CUSTOMERIO_APP_KEY").and_return("app-key")
         FeatureFlag.enable(Deliverable::CUSTOMERIO_FLAG, FeatureFlag::Actor[user])
+        link_mlh_identity(user)
       end
 
       after { FeatureFlag.remove(Deliverable::CUSTOMERIO_FLAG) }
@@ -272,6 +298,7 @@
         allow(ApplicationConfig).to receive(:[]).and_call_original
         allow(ApplicationConfig).to receive(:[]).with("CUSTOMERIO_APP_KEY").and_return("app-key")
         FeatureFlag.enable(Deliverable::CUSTOMERIO_FLAG, FeatureFlag::Actor[user])
+        link_mlh_identity(user)
       end
 
       after { FeatureFlag.remove(Deliverable::CUSTOMERIO_FLAG) }

spec/mailers/digest_mailer_spec.rb

@@ -117,6 +117,7 @@
         allow(ApplicationConfig).to receive(:[]).and_call_original
         allow(ApplicationConfig).to receive(:[]).with("CUSTOMERIO_APP_KEY").and_return("app-key")
         FeatureFlag.enable(Deliverable::CUSTOMERIO_FLAG, FeatureFlag::Actor[user])
+        link_mlh_identity(user)
       end
 
       after { FeatureFlag.remove(Deliverable::CUSTOMERIO_FLAG) }