[deploy] Add subscriber_email to user_subscriptions & more backend updates (#8723)

* Add migrations for subscriber_email column

* Add new schema

* Update relationships, specs, and factory

* Use unless index_exists? over if !index_exists?

* Fix typo in last fix for migration

* Rename authored_user_subscriptions

- to source_authored_user_subcriptions

* Add factory methods and specs

- UserSubscription.make
- UserSubscription.make_new
- Article.new_user_subscription
- Article.create_user_subscription

* Remove self and &

* Change make_new to build

* new_user_subscription --> build_user_subscription

* Come on, Travis

app/models/article.rb

@@ -4,6 +4,7 @@ class Article < ApplicationRecord
   include Storext.model
   include Reactable
   include Searchable
+  include UserSubscriptionSourceable
 
   SEARCH_SERIALIZER = Search::ArticleSerializer
   SEARCH_CLASS = Search::FeedContent
@@ -28,8 +29,6 @@ class Article < ApplicationRecord
   counter_culture :organization
 
   has_many :comments, as: :commentable, inverse_of: :commentable
-  has_many :user_subscriptions, as: :user_subscription_sourceable
-  has_many :sourced_subscribers, class_name: "User", through: :user_subscriptions, source: :subscriber, foreign_key: :user_id
   has_many :top_comments,
            -> { where("comments.score > ? AND ancestry IS NULL and hidden_by_commentable_user is FALSE and deleted is FALSE", 10).order("comments.score DESC") },
            as: :commentable,

app/models/concerns/user_subscription_sourceable.rb

@@ -0,0 +1,33 @@
+module UserSubscriptionSourceable
+  extend ActiveSupport::Concern
+
+  # This all assumes there's an association with User under the column user_id.
+
+  included do
+    has_many :user_subscriptions, as: :user_subscription_sourceable
+    has_many :sourced_subscribers,
+             class_name: "User",
+             through: :user_subscriptions,
+             source: :subscriber,
+             foreign_key: :user_id
+  end
+
+  def build_user_subscription(subscriber)
+    UserSubscription.new(user_subscription_attributes(subscriber))
+  end
+
+  def create_user_subscription(subscriber)
+    UserSubscription.create(user_subscription_attributes(subscriber))
+  end
+
+  private
+
+  def user_subscription_attributes(subscriber)
+    {
+      user_subscription_sourceable: self,
+      author_id: user_id,
+      subscriber_id: subscriber&.id,
+      subscriber_email: subscriber&.email
+    }
+  end
+end

app/models/user.rb

@@ -45,6 +45,10 @@ class User < ApplicationRecord
   acts_as_followable
   acts_as_follower
 
+  has_many :source_authored_user_subscriptions, class_name: "UserSubscription", foreign_key: :author_id, inverse_of: :author, dependent: :destroy
+  has_many :subscribers, through: :source_authored_user_subscriptions, dependent: :destroy
+  has_many :subscribed_to_user_subscriptions, class_name: "UserSubscription", foreign_key: :subscriber_id, inverse_of: :subscriber, dependent: :destroy
+
   has_many :access_grants, class_name: "Doorkeeper::AccessGrant", foreign_key: :resource_owner_id, inverse_of: :resource_owner, dependent: :delete_all
   has_many :access_tokens, class_name: "Doorkeeper::AccessToken", foreign_key: :resource_owner_id, inverse_of: :resource_owner, dependent: :delete_all
   has_many :affected_feedback_messages, class_name: "FeedbackMessage", inverse_of: :affected, foreign_key: :affected_id, dependent: :nullify
@@ -67,8 +71,6 @@ class User < ApplicationRecord
   has_many :display_ad_events, dependent: :destroy
   has_many :email_authorizations, dependent: :delete_all
   has_many :email_messages, class_name: "Ahoy::Message", dependent: :destroy
-  has_many :user_subscriptions, foreign_key: :author_id, inverse_of: :author, dependent: :destroy
-  has_many :subscribers, through: :user_subscriptions, dependent: :destroy
   has_many :field_test_memberships, class_name: "FieldTest::Membership", as: :participant, dependent: :destroy
   has_many :github_repos, dependent: :destroy
   has_many :html_variants, dependent: :destroy

app/models/user_subscription.rb

@@ -4,12 +4,30 @@
 class UserSubscription < ApplicationRecord
   ALLOWED_TYPES = %w[Article].freeze
 
-  belongs_to :author, class_name: "User", inverse_of: :user_subscriptions
-  belongs_to :subscriber, class_name: "User", inverse_of: :user_subscriptions
+  belongs_to :author, class_name: "User", inverse_of: :source_authored_user_subscriptions
+  belongs_to :subscriber, class_name: "User", inverse_of: :subscribed_to_user_subscriptions
   belongs_to :user_subscription_sourceable, polymorphic: true
 
   validates :author_id, presence: true
-  validates :subscriber_id, presence: true, uniqueness: { scope: %i[user_subscription_sourceable_type user_subscription_sourceable_id] }
+  validates :subscriber_email, presence: true
+  validates :subscriber_id, presence: true, uniqueness: { scope: %i[subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id] }
   validates :user_subscription_sourceable_id, presence: true
   validates :user_subscription_sourceable_type, presence: true, inclusion: { in: ALLOWED_TYPES }
+
+  def self.build(source:, subscriber:)
+    new(build_attributes(source, subscriber))
+  end
+
+  def self.make(source:, subscriber:)
+    create(build_attributes(source, subscriber))
+  end
+
+  def self.build_attributes(source, subscriber)
+    {
+      user_subscription_sourceable: source,
+      author_id: source&.user_id,
+      subscriber_id: subscriber&.id,
+      subscriber_email: subscriber&.email
+    }
+  end
 end

db/migrate/20200617014320_add_subscriber_email_to_user_subscriptions.rb

@@ -0,0 +1,5 @@
+class AddSubscriberEmailToUserSubscriptions < ActiveRecord::Migration[6.0]
+  def change
+    add_column :user_subscriptions, :subscriber_email, :string, null: false
+  end
+end

db/migrate/20200617014509_add_subscriber_email_index_to_user_subscriptions.rb

@@ -0,0 +1,45 @@
+class AddSubscriberEmailIndexToUserSubscriptions < ActiveRecord::Migration[6.0]
+  disable_ddl_transaction!
+
+  def up
+    if index_exists?(:user_subscriptions, %i[subscriber_id user_subscription_sourceable_id user_subscription_sourceable_type])
+      remove_index :user_subscriptions,
+                   column: %i[subscriber_id user_subscription_sourceable_id user_subscription_sourceable_type],
+                   algorithm: :concurrently
+    end
+
+    unless index_exists?(:user_subscriptions, :subscriber_email)
+      add_index :user_subscriptions,
+                :subscriber_email,
+                algorithm: :concurrently
+    end
+
+    unless index_exists?(:user_subscriptions, %i[subscriber_id subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id])
+      add_index :user_subscriptions,
+                %i[subscriber_id subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id],
+                name: "index_subscriber_id_and_email_with_user_subscription_source",
+                unique: true,
+                algorithm: :concurrently
+    end
+  end
+
+  def down
+    unless index_exists?(:user_subscriptions, %i[subscriber_id user_subscription_sourceable_id user_subscription_sourceable_type])
+      add_index :user_subscriptions,
+                %i[subscriber_id user_subscription_sourceable_id user_subscription_sourceable_type],
+                unique: true,
+                name: :index_on_subscriber_id_user_subscription_sourceable_type_and_id,
+                algorithm: :concurrently
+    end
+
+    if index_exists?(:user_subscriptions, :subscriber_email)
+      remove_index :user_subscriptions, column: :subscriber_email, algorithm: :concurrently
+    end
+
+    if index_exists?(:user_subscriptions, %i[subscriber_id subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id])
+      remove_index :user_subscriptions,
+                   column: %i[subscriber_id subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id],
+                   algorithm: :concurrently
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_06_16_200005) do
+ActiveRecord::Schema.define(version: 2020_06_17_014509) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1134,12 +1134,14 @@ ActiveRecord::Schema.define(version: 2020_06_16_200005) do
   create_table "user_subscriptions", force: :cascade do |t|
     t.bigint "author_id", null: false
     t.datetime "created_at", precision: 6, null: false
+    t.string "subscriber_email", null: false
     t.bigint "subscriber_id", null: false
     t.datetime "updated_at", precision: 6, null: false
     t.bigint "user_subscription_sourceable_id", null: false
     t.string "user_subscription_sourceable_type", null: false
     t.index ["author_id"], name: "index_user_subscriptions_on_author_id"
-    t.index ["subscriber_id", "user_subscription_sourceable_id", "user_subscription_sourceable_type"], name: "index_on_subscriber_id_user_subscription_sourceable_type_and_id", unique: true
+    t.index ["subscriber_email"], name: "index_user_subscriptions_on_subscriber_email"
+    t.index ["subscriber_id", "subscriber_email", "user_subscription_sourceable_type", "user_subscription_sourceable_id"], name: "index_subscriber_id_and_email_with_user_subscription_source", unique: true
     t.index ["subscriber_id"], name: "index_user_subscriptions_on_subscriber_id"
     t.index ["user_subscription_sourceable_type", "user_subscription_sourceable_id"], name: "index_on_user_subscription_sourcebable_type_and_id"
   end

spec/factories/user_subscription.rb

@@ -3,6 +3,7 @@ FactoryBot.define do
     association :subscriber, factory: :user, strategy: :create
     association :user_subscription_sourceable, factory: :article
 
-    author { user_subscription_sourceable.user }
+    author           { user_subscription_sourceable.user }
+    subscriber_email { subscriber.email }
   end
 end

spec/models/article_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe Article, type: :model do
   let!(:article) { create(:article, user: user) }
 
   include_examples "#sync_reactions_count", :article
+  it_behaves_like "UserSubscriptionSourceable"
 
   describe "validations" do
     it { is_expected.to validate_uniqueness_of(:canonical_url).allow_blank }

spec/models/shared_examples/user_subscription_sourceable_spec.rb

@@ -0,0 +1,42 @@
+RSpec.shared_examples "UserSubscriptionSourceable" do
+  let(:model) { described_class }
+  let(:source) { create(model.to_s.underscore.to_sym) }
+  let(:subscriber) { create(:user) }
+
+  describe "#build_user_subscription" do
+    it "returns a new UserSubcription with the correct attributes" do
+      new_user_subscription = UserSubscription.new(
+        user_subscription_sourceable: source,
+        author_id: source.user_id,
+        subscriber_id: subscriber.id,
+        subscriber_email: subscriber.email,
+      )
+
+      factory_user_subscription = source.build_user_subscription(subscriber)
+
+      factory_user_subscription.attributes.each do |name, val|
+        expect(new_user_subscription[name]).to eq val
+      end
+    end
+  end
+
+  describe "#create_user_subscription" do
+    it "returns a created UserSubcription with the correct attributes" do
+      user_subscription_fields = %w[author_id subsciber_id subscriber_email user_subscription_sourceable_id user_susbcription_sourceable_type]
+
+      user_subscription = create(
+        :user_subscription,
+        user_subscription_sourceable: source,
+        author_id: source.user_id,
+        subscriber_id: subscriber.id,
+        subscriber_email: subscriber.email,
+      )
+
+      factory_user_subscription = source.create_user_subscription(subscriber)
+
+      user_subscription_fields.each do |field|
+        expect(factory_user_subscription[field]).to eq user_subscription[field]
+      end
+    end
+  end
+end

spec/models/user_subscription_spec.rb

@@ -3,12 +3,51 @@ require "rails_helper"
 RSpec.describe UserSubscription, type: :model do
   subject { build(:user_subscription) }
 
+  let(:source) { create(:article) }
+  let(:subscriber) { create(:user) }
+
   describe "validations" do
     it { is_expected.to validate_presence_of(:user_subscription_sourceable_id) }
     it { is_expected.to validate_presence_of(:user_subscription_sourceable_type) }
     it { is_expected.to validate_presence_of(:subscriber_id) }
+    it { is_expected.to validate_presence_of(:subscriber_email) }
     it { is_expected.to validate_presence_of(:author_id) }
     it { is_expected.to validate_inclusion_of(:user_subscription_sourceable_type).in_array(%w[Article]) }
-    it { is_expected.to validate_uniqueness_of(:subscriber_id).scoped_to(:user_subscription_sourceable_type, :user_subscription_sourceable_id) }
+    it { is_expected.to validate_uniqueness_of(:subscriber_id).scoped_to(%i[subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id]) }
+  end
+
+  describe "#build" do
+    it "returns a new UserSubcription with the correct attributes" do
+      new_user_subscription = described_class.new(
+        user_subscription_sourceable: source,
+        author_id: source.user_id,
+        subscriber_id: subscriber.id,
+        subscriber_email: subscriber.email,
+      )
+
+      factory_user_subscription = described_class.build(source: source, subscriber: subscriber)
+
+      factory_user_subscription.attributes.each do |name, val|
+        expect(new_user_subscription[name]).to eq val
+      end
+    end
+  end
+
+  describe "#make" do
+    it "returns a created UserSubcription with the correct attributes" do
+      user_subscription_fields = %w[author_id subsciber_id subscriber_email user_subscription_sourceable_id user_susbcription_sourceable_type]
+
+      user_subscription = create(:user_subscription,
+                                 user_subscription_sourceable: source,
+                                 author_id: source.user_id,
+                                 subscriber_id: subscriber.id,
+                                 subscriber_email: subscriber.email)
+
+      factory_user_subscription = described_class.make(source: source, subscriber: subscriber)
+
+      user_subscription_fields.each do |field|
+        expect(factory_user_subscription[field]).to eq user_subscription[field]
+      end
+    end
   end
 end