Skip to content

Do not send emails for in-product marketing twice to a user

Nicolas Dular requested to merge nicolasdular/record-product-email-campaign-emails into master

What does this MR do?

This adds a table to record all sent emails of the in-product marketing emails to not send them twice to our users.

These in-product marketing emails are send based on the usage of the user & try to give tips how to get started (see original MR)

We already don't send multiple emails per day to a user, when they are part of multiple namespaces, however, we also don't want to send the same email multiple times to users. That's why we need to store which emails they already got.

bin/rails db:migrate 
 == 20210317104301 CreateInProductMarketingEmails: migrating ===================
-- create_table(:in_product_marketing_emails)
   -> 0.0042s
-- add_index(:in_product_marketing_emails, :user_id)
   -> 0.0050s
-- add_index(:in_product_marketing_emails, [:user_id, :track, :series], {:unique=>true, :name=>"index_in_product_marketing_emails_on_user_track_series"})
   -> 0.0053s
== 20210317104301 CreateInProductMarketingEmails: migrated (0.0192s) ==========

== 20210317105904 AddUserForeignKeyToInProductMarketingEmails: migrating ======
-- transaction_open?()
   -> 0.0000s
-- foreign_keys(:in_product_marketing_emails)
   -> 0.0032s
-- execute("ALTER TABLE in_product_marketing_emails\nADD CONSTRAINT fk_35c9101b63\nFOREIGN KEY (user_id)\nREFERENCES users (id)\nON DELETE CASCADE\nNOT VALID;\n")
   -> 0.0025s
-- execute("SET statement_timeout TO 0")
   -> 0.0006s
-- execute("ALTER TABLE in_product_marketing_emails VALIDATE CONSTRAINT fk_35c9101b63;")
   -> 0.0074s
-- execute("RESET ALL")
   -> 0.0007s
== 20210317105904 AddUserForeignKeyToInProductMarketingEmails: migrated (0.0199s)
bin/rails db:rollback STEP=2 
== 20210317105904 AddUserForeignKeyToInProductMarketingEmails: reverting ======
-- remove_foreign_key(:in_product_marketing_emails, {:column=>:user_id})
   -> 0.0035s
== 20210317105904 AddUserForeignKeyToInProductMarketingEmails: reverted (0.0144s)

== 20210317104301 CreateInProductMarketingEmails: reverting ===================
-- drop_table(:in_product_marketing_emails)
   -> 0.0009s
== 20210317104301 CreateInProductMarketingEmails: reverted (0.0044s) ==========
Query Plan

ActiveRecord

      group.users.where(email_opted_in: true)
      .left_outer_joins(:in_product_marketing_emails)
      .where.not(id: current_batch_user_ids)
      .merge(
        Namespaces::InProductMarketingEmail.where.not(track: track).or(Namespaces::InProductMarketingEmail.where.not(series: series))
          .or(Namespaces::InProductMarketingEmail.where(track: nil).or(Namespaces::InProductMarketingEmail.where(series: nil)))
      )

RAW SQL

SELECT users.*
FROM users
INNER JOIN members ON users.id = members.user_id
LEFT OUTER JOIN in_product_marketing_emails ON in_product_marketing_emails.user_id = users.id
WHERE members.type = 'GroupMember'
  AND members.source_type = 'Namespace'
  AND members.source_id = 1
  AND members.source_type = 'Namespace'
  AND members.requested_at IS NULL
  AND members.access_level != 5
  AND users.email_opted_in = TRUE
  AND 1=1
  AND ((in_product_marketing_emails.track != 0
        OR in_product_marketing_emails.series != 0)
       OR (in_product_marketing_emails.track IS NULL
           OR in_product_marketing_emails.series IS NULL))

Query Plan

Before

 Nested Loop  (cost=0.29..17.79 rows=1 width=2707)
   ->  Index Scan using index_members_on_source_id_and_source_type on members  (cost=0.15..8.17 rows=1 width=4)
         Index Cond: ((source_id = 1) AND ((source_type)::text = 'Namespace'::text))
         Filter: ((requested_at IS NULL) AND (access_level <> 5) AND ((type)::text = 'GroupMember'::text))
   ->  Index Scan using users_pkey on users  (cost=0.14..8.15 rows=1 width=2707)
         Index Cond: (id = members.user_id)
         Filter: email_opted_in

After (joining in_product_marketing_emails)

 Nested Loop Left Join  (cost=0.44..22.50 rows=51 width=2707)
   Filter: ((in_product_marketing_emails.track <> 0) OR (in_product_marketing_emails.series <> 0) OR (in_product_marketing_emails.track IS NULL) OR (in_product_marketing_emails.series IS NULL))
   ->  Nested Loop  (cost=0.29..17.79 rows=1 width=2707)
         ->  Index Scan using index_members_on_source_id_and_source_type on members  (cost=0.15..8.17 rows=1 width=4)
               Index Cond: ((source_id = 1) AND ((source_type)::text = 'Namespace'::text))
               Filter: ((requested_at IS NULL) AND (access_level <> 5) AND ((type)::text = 'GroupMember'::text))
         ->  Index Scan using users_pkey on users  (cost=0.14..8.15 rows=1 width=2707)
               Index Cond: (id = members.user_id)
               Filter: email_opted_in
   ->  Index Only Scan using index_in_product_marketing_emails_on_user_track_series on in_product_marketing_emails  (cost=0.15..4.64 rows=5 width=12)
         Index Cond: (user_id = users.id)

Related to:

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Nicolas Dular

Merge request reports