[deploy] Add relations and foreign keys between HtmlVariant related models (#9846)

* Add inverse has_many relation betwen User and Ahoy::[Event|Visit
]

* Add FKs and proper relations to HtmlVariant models

.rubocop_todo.yml

@@ -25,8 +25,6 @@ Rails/HasManyOrHasOneDependent:
   Exclude:
     - 'app/models/article.rb'
     - 'app/models/concerns/user_subscription_sourceable.rb'
-    - 'app/models/display_ad.rb'
-    - 'app/models/html_variant.rb'
     - 'app/models/listing_category.rb'
     - 'app/models/organization.rb'
     - 'app/models/podcast.rb'

app/models/article.rb

@@ -41,6 +41,8 @@ class Article < ApplicationRecord
            class_name: "Comment"
   has_many :profile_pins, as: :pinnable, inverse_of: :pinnable
   has_many :buffer_updates, dependent: :destroy
+  has_many :html_variant_successes, dependent: :nullify
+  has_many :html_variant_trials, dependent: :nullify
   has_many :notifications, as: :notifiable, inverse_of: :notifiable, dependent: :delete_all
   has_many :notification_subscriptions, as: :notifiable, inverse_of: :notifiable, dependent: :destroy
   has_many :rating_votes

app/models/html_variant.rb

@@ -1,15 +1,17 @@
 class HtmlVariant < ApplicationRecord
   GROUP_NAMES = %w[article_show_below_article_cta badge_landing_page campaign].freeze
 
+  belongs_to :user, optional: true
+
+  has_many :html_variant_successes, dependent: :destroy
+  has_many :html_variant_trials, dependent: :destroy
+
+  validates :group, inclusion: { in: GROUP_NAMES }
   validates :html, presence: true
   validates :name, uniqueness: true
-  validates :group, inclusion: { in: GROUP_NAMES }
   validates :success_rate, presence: true
-  validate  :no_edits
 
-  belongs_to :user, optional: true
-  has_many :html_variant_trials
-  has_many :html_variant_successes
+  validate  :no_edits
 
   before_save :prefix_all_images
 

app/models/html_variant_success.rb

@@ -1,5 +1,6 @@
 class HtmlVariantSuccess < ApplicationRecord
-  validates :html_variant_id, presence: true
   belongs_to :html_variant
   belongs_to :article, optional: true
+
+  validates :html_variant_id, presence: true
 end

app/models/html_variant_trial.rb

@@ -1,5 +1,6 @@
 class HtmlVariantTrial < ApplicationRecord
-  validates :html_variant_id, presence: true
   belongs_to :html_variant
   belongs_to :article, optional: true
+
+  validates :html_variant_id, presence: true
 end

app/models/user.rb

@@ -62,6 +62,8 @@ class User < ApplicationRecord
                            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
+  has_many :ahoy_events, class_name: "Ahoy::Event", dependent: :destroy
+  has_many :ahoy_visits, class_name: "Ahoy::Visit", dependent: :destroy
   has_many :api_secrets, dependent: :destroy
   has_many :articles, dependent: :destroy
   has_many :audit_logs, dependent: :nullify

db/migrate/20200818101637_add_missing_foreign_keys_to_html_variants_models.rb

@@ -0,0 +1,9 @@
+class AddMissingForeignKeysToHtmlVariantsModels < ActiveRecord::Migration[6.0]
+  def change
+    add_foreign_key :html_variant_successes, :html_variants, column: :html_variant_id, on_delete: :cascade, validate: false
+    add_foreign_key :html_variant_successes, :articles, column: :article_id, on_delete: :nullify, validate: false
+
+    add_foreign_key :html_variant_trials, :html_variants, column: :html_variant_id, on_delete: :cascade, validate: false
+    add_foreign_key :html_variant_trials, :articles, column: :article_id, on_delete: :nullify, validate: false
+  end
+end

db/migrate/20200818101700_validate_add_missing_foreign_keys_to_html_variants_models.rb

@@ -0,0 +1,8 @@
+class ValidateAddMissingForeignKeysToHtmlVariantsModels < ActiveRecord::Migration[6.0]
+  def change
+    validate_foreign_key :html_variant_successes, :html_variants
+    validate_foreign_key :html_variant_successes, :articles
+    validate_foreign_key :html_variant_trials, :html_variants
+    validate_foreign_key :html_variant_trials, :articles
+  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_08_14_142648) do
+ActiveRecord::Schema.define(version: 2020_08_18_101700) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
@@ -1352,6 +1352,10 @@ ActiveRecord::Schema.define(version: 2020_08_14_142648) do
   add_foreign_key "display_ad_events", "display_ads", on_delete: :cascade
   add_foreign_key "display_ad_events", "users", on_delete: :cascade
   add_foreign_key "email_authorizations", "users", on_delete: :cascade
+  add_foreign_key "html_variant_successes", "articles", on_delete: :nullify
+  add_foreign_key "html_variant_successes", "html_variants", on_delete: :cascade
+  add_foreign_key "html_variant_trials", "articles", on_delete: :nullify
+  add_foreign_key "html_variant_trials", "html_variants", on_delete: :cascade
   add_foreign_key "identities", "users", on_delete: :cascade
   add_foreign_key "messages", "chat_channels"
   add_foreign_key "messages", "users"

spec/models/article_spec.rb

@@ -24,6 +24,8 @@ RSpec.describe Article, type: :model do
     it { is_expected.to belong_to(:organization).optional }
     it { is_expected.to belong_to(:collection).optional }
     it { is_expected.to have_many(:comments) }
+    it { is_expected.to have_many(:html_variant_successes).dependent(:nullify) }
+    it { is_expected.to have_many(:html_variant_trials).dependent(:nullify) }
     it { is_expected.to have_many(:reactions).dependent(:destroy) }
     it { is_expected.to have_many(:notifications).dependent(:delete_all) }
     it { is_expected.to have_many(:notification_subscriptions).dependent(:destroy) }

spec/models/html_variant_spec.rb

@@ -3,9 +3,21 @@ require "rails_helper"
 RSpec.describe HtmlVariant, type: :model do
   let(:html_variant) { create(:html_variant, approved: true, published: true) }
 
-  it { is_expected.to validate_uniqueness_of(:name) }
-  it { is_expected.to validate_presence_of(:html) }
-  it { is_expected.to belong_to(:user).optional }
+  describe "validations" do
+    describe "builtin validations" do
+      subject { html_variant }
+
+      it { is_expected.to belong_to(:user).optional }
+
+      it { is_expected.to have_many(:html_variant_trials).dependent(:destroy) }
+      it { is_expected.to have_many(:html_variant_successes).dependent(:destroy) }
+
+      it { is_expected.to validate_inclusion_of(:group).in_array(described_class::GROUP_NAMES) }
+      it { is_expected.to validate_presence_of(:html) }
+      it { is_expected.to validate_presence_of(:success_rate) }
+      it { is_expected.to validate_uniqueness_of(:name) }
+    end
+  end
 
   it "calculates success rate" do
     4.times { HtmlVariantTrial.create!(html_variant_id: html_variant.id) }

spec/models/user_spec.rb

@@ -34,6 +34,8 @@ RSpec.describe User, type: :model do
     describe "builtin validations" do
       subject { user }
 
+      it { is_expected.to have_many(:ahoy_events).dependent(:destroy) }
+      it { is_expected.to have_many(:ahoy_visits).dependent(:destroy) }
       it { is_expected.to have_many(:api_secrets).dependent(:destroy) }
       it { is_expected.to have_many(:articles).dependent(:destroy) }
       it { is_expected.to have_many(:audit_logs).dependent(:nullify) }