[deploy] Add missing foreign keys to models related to users (#10018)

* Add missing foreign keys to models related to users

* Fix specs

* Change FK for page_views and rating_votes to nullify

* Improve rating vote validation and fix Users::Delete specs

* Revert "Spec Fix:Nullify Rating Votes when Deleting Users (#10071)"

This reverts commit 12dfe4ab3b.

app/models/rating_vote.rb

@@ -1,11 +1,13 @@
 class RatingVote < ApplicationRecord
   belongs_to :article
-  belongs_to :user
+  belongs_to :user, optional: true
 
-  validates :user_id, uniqueness: { scope: %i[article_id context] }
-  validates :group, inclusion: { in: %w[experience_level] }
   validates :context, inclusion: { in: %w[explicit readinglist_reaction comment] }
+  validates :group, inclusion: { in: %w[experience_level] }
   validates :rating, numericality: { greater_than: 0.0, less_than_or_equal_to: 10.0 }
+  validates :user_id, presence: true, on: :create
+  validates :user_id, uniqueness: { scope: %i[article_id context] }
+
   validate :permissions
 
   after_create_commit :assign_article_rating
@@ -13,12 +15,12 @@ class RatingVote < ApplicationRecord
   counter_culture :article
   counter_culture :user
 
+  private
+
   def assign_article_rating
     RatingVotes::AssignRatingWorker.perform_async(article_id)
   end
 
-  private
-
   def permissions
     return unless context == "explicit" && !user&.trusted && user_id != article&.user_id
 

db/migrate/20200826132639_add_missing_foreign_keys_to_models_related_to_users.rb

@@ -0,0 +1,17 @@
+class AddMissingForeignKeysToModelsRelatedToUsers < ActiveRecord::Migration[6.0]
+  def change
+    add_foreign_key :articles, :users, on_delete: :cascade, validate: false
+    add_foreign_key :collections, :users, on_delete: :cascade, validate: false
+    add_foreign_key :comments, :users, on_delete: :cascade, validate: false
+    add_foreign_key :credits, :users, on_delete: :cascade, validate: false
+    add_foreign_key :github_repos, :users, on_delete: :cascade, validate: false
+    add_foreign_key :html_variants, :users, on_delete: :cascade, validate: false
+    add_foreign_key :mentions, :users, on_delete: :cascade, validate: false
+    add_foreign_key :notifications, :users, on_delete: :cascade, validate: false
+    add_foreign_key :organization_memberships, :users, on_delete: :cascade, validate: false
+    add_foreign_key :page_views, :users, on_delete: :nullify, validate: false
+    add_foreign_key :rating_votes, :users, on_delete: :nullify, validate: false
+    add_foreign_key :reactions, :users, on_delete: :cascade, validate: false
+    add_foreign_key :tweets, :users, on_delete: :nullify, validate: false
+  end
+end

db/schema.rb

@@ -1369,6 +1369,7 @@ ActiveRecord::Schema.define(version: 2020_08_28_032013) do
   add_foreign_key "ahoy_visits", "users", on_delete: :cascade
   add_foreign_key "api_secrets", "users", on_delete: :cascade
   add_foreign_key "articles", "organizations", on_delete: :nullify
+  add_foreign_key "articles", "users", on_delete: :cascade
   add_foreign_key "audit_logs", "users"
   add_foreign_key "badge_achievements", "badges"
   add_foreign_key "badge_achievements", "users"
@@ -1381,26 +1382,35 @@ ActiveRecord::Schema.define(version: 2020_08_28_032013) do
   add_foreign_key "classified_listings", "organizations", on_delete: :cascade
   add_foreign_key "classified_listings", "users", on_delete: :cascade
   add_foreign_key "collections", "organizations", on_delete: :nullify
+  add_foreign_key "collections", "users", on_delete: :cascade
+  add_foreign_key "comments", "users", on_delete: :cascade
   add_foreign_key "credits", "organizations", on_delete: :restrict
+  add_foreign_key "credits", "users", on_delete: :cascade
   add_foreign_key "display_ad_events", "display_ads", on_delete: :cascade
   add_foreign_key "display_ad_events", "users", on_delete: :cascade
   add_foreign_key "display_ads", "organizations", on_delete: :cascade
   add_foreign_key "email_authorizations", "users", on_delete: :cascade
+  add_foreign_key "github_repos", "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 "html_variants", "users", on_delete: :cascade
   add_foreign_key "identities", "users", on_delete: :cascade
+  add_foreign_key "mentions", "users", on_delete: :cascade
   add_foreign_key "messages", "chat_channels"
   add_foreign_key "messages", "users"
   add_foreign_key "notification_subscriptions", "users", on_delete: :cascade
   add_foreign_key "notifications", "organizations", on_delete: :cascade
+  add_foreign_key "notifications", "users", on_delete: :cascade
   add_foreign_key "oauth_access_grants", "oauth_applications", column: "application_id"
   add_foreign_key "oauth_access_grants", "users", column: "resource_owner_id"
   add_foreign_key "oauth_access_tokens", "oauth_applications", column: "application_id"
   add_foreign_key "oauth_access_tokens", "users", column: "resource_owner_id"
   add_foreign_key "organization_memberships", "organizations", on_delete: :cascade
+  add_foreign_key "organization_memberships", "users", on_delete: :cascade
   add_foreign_key "page_views", "articles", on_delete: :cascade
+  add_foreign_key "page_views", "users", on_delete: :nullify
   add_foreign_key "podcast_episodes", "podcasts", on_delete: :cascade
   add_foreign_key "podcasts", "users", column: "creator_id"
   add_foreign_key "poll_options", "polls", on_delete: :cascade
@@ -1413,12 +1423,15 @@ ActiveRecord::Schema.define(version: 2020_08_28_032013) do
   add_foreign_key "profile_fields", "profile_field_groups"
   add_foreign_key "profiles", "users", on_delete: :cascade
   add_foreign_key "rating_votes", "articles", on_delete: :cascade
+  add_foreign_key "rating_votes", "users", on_delete: :nullify
+  add_foreign_key "reactions", "users", on_delete: :cascade
   add_foreign_key "response_templates", "users"
   add_foreign_key "sponsorships", "organizations"
   add_foreign_key "sponsorships", "users"
   add_foreign_key "tag_adjustments", "articles", on_delete: :cascade
   add_foreign_key "tag_adjustments", "tags", on_delete: :cascade
   add_foreign_key "tag_adjustments", "users", on_delete: :cascade
+  add_foreign_key "tweets", "users", on_delete: :nullify
   add_foreign_key "user_blocks", "users", column: "blocked_id"
   add_foreign_key "user_blocks", "users", column: "blocker_id"
   add_foreign_key "user_subscriptions", "users", column: "author_id"

spec/factories/rating_votes.rb

@@ -4,7 +4,7 @@ FactoryBot.define do
     group { "experience_level" }
     rating { rand(1.0..8.0) }
     after(:build) do |vote|
-      vote.article ||= create(:article, user: vote.user)
+      vote.article ||= create(:article)
     end
   end
 end

spec/models/notification_spec.rb

@@ -13,11 +13,11 @@ RSpec.describe Notification, type: :model do
   let(:comment) { create(:comment, user: user2, commentable: article) }
   let(:badge_achievement) { create(:badge_achievement) }
 
-  it do
+  it "validates a unique user_id according to the correct scope" do
+    notification = create(:notification, user: user2, notifiable: article, organization: nil)
+
     scopes = %i[organization_id notifiable_id notifiable_type action]
-    # rubocop:disable RSpec/NamedSubject
-    expect(subject).to validate_uniqueness_of(:user_id).scoped_to(scopes)
-    # rubocop:enable RSpec/NamedSubject
+    expect(notification).to validate_uniqueness_of(:user_id).scoped_to(scopes)
   end
 
   describe "when trying to create duplicate notifications" do

spec/models/rating_vote_spec.rb

@@ -4,10 +4,21 @@ RSpec.describe RatingVote, type: :model do
   let(:user) { create(:user, :trusted) }
   let(:user2) { create(:user, :trusted) }
   let(:article) { create(:article, user: user) }
+  let(:rating_vote) { create(:rating_vote, user: user2, article: article) }
 
   describe "validations" do
-    it { is_expected.to validate_numericality_of(:rating).is_greater_than(0.0).is_less_than_or_equal_to(10.0) }
-    it { is_expected.to validate_inclusion_of(:group).in_array(%w[experience_level]) }
+    describe "builtin validations" do
+      subject { rating_vote }
+
+      it { is_expected.to belong_to(:article) }
+      it { is_expected.to belong_to(:user).optional }
+
+      it { is_expected.to validate_inclusion_of(:context).in_array(%w[explicit readinglist_reaction comment]) }
+      it { is_expected.to validate_inclusion_of(:group).in_array(%w[experience_level]) }
+      it { is_expected.to validate_numericality_of(:rating).is_greater_than(0.0).is_less_than_or_equal_to(10.0) }
+      it { is_expected.to validate_presence_of(:user_id).on(:create) }
+      it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(%i[article_id context]) }
+    end
   end
 
   describe "uniqueness" do

spec/policies/organization_policy_spec.rb

@@ -27,7 +27,7 @@ RSpec.describe OrganizationPolicy, type: :policy do
   context "when user is an org admin of an org" do
     subject(:organization_policy) { described_class.new(user, org) }
 
-    let(:user) { build_stubbed(:user) }
+    let(:user) { create(:user) }
     let(:org)  { create(:organization) }
 
     before do
@@ -42,7 +42,7 @@ RSpec.describe OrganizationPolicy, type: :policy do
   context "when user is an org admin of another org" do
     subject(:organization_policy) { described_class.new(user, new_org) }
 
-    let(:user) { build_stubbed(:user) }
+    let(:user) { create(:user) }
     let(:org)  { create(:organization) }
     let(:new_org) { build_stubbed(:organization) }
 

spec/services/users/delete_spec.rb

@@ -3,7 +3,7 @@ require "rails_helper"
 RSpec.describe Users::Delete, type: :service do
   before { omniauth_mock_github_payload }
 
-  let(:user) { create(:user, :with_identity, identities: ["github"]) }
+  let(:user) { create(:user, :trusted, :with_identity, identities: ["github"]) }
 
   it "deletes user" do
     described_class.call(user)
@@ -115,10 +115,6 @@ RSpec.describe Users::Delete, type: :service do
       create_associations(direct_associations.select { |a| kept_association_names.include?(a.name) })
     end
 
-    before do
-      allow(user).to receive(:trusted).and_return(true)
-    end
-
     def create_associations(names)
       associations = []
 
@@ -150,11 +146,6 @@ RSpec.describe Users::Delete, type: :service do
     end
 
     it "keeps the kept associations" do
-      # Rating Vote must be for NOT the user's own article
-      other_article = create(:article)
-      rating_vote = kept_associations.detect { |a| a.is_a?(RatingVote) }
-      rating_vote.update(article_id: other_article.id)
-
       expect(kept_associations).not_to be_empty
       user.reload
       described_class.call(user)