[deploy] Add missing FKs to FeedbackMessage, Note, Taggings, Tag, and UserRole (#10376)

* Add missing FKs to FeedbackMessage, Note, Taggings, Tag, and UserRole

* Add missing relations and specs

* Fix spec

app/models/chat_channel.rb

@@ -28,6 +28,9 @@ class ChatChannel < ApplicationRecord
   has_many :rejected_users, through: :rejected_memberships, class_name: "User", source: :user
   has_many :mod_users, through: :mod_memberships, class_name: "User", source: :user
 
+  has_one :mod_tag, class_name: "Tag", foreign_key: "mod_chat_channel_id",
+                    inverse_of: :mod_chat_channel, dependent: :nullify
+
   validates :channel_type, presence: true, inclusion: { in: CHANNEL_TYPES }
   validates :status, presence: true, inclusion: { in: STATUSES }
   validates :slug, uniqueness: true, presence: true

app/models/tag.rb

@@ -12,6 +12,7 @@ class Tag < ActsAsTaggableOn::Tag
   ALLOWED_CATEGORIES = %w[uncategorized language library tool site_mechanic location subcommunity].freeze
 
   belongs_to :badge, optional: true
+  belongs_to :mod_chat_channel, class_name: "ChatChannel", optional: true
   has_one :sponsorship, as: :sponsorable, inverse_of: :sponsorable, dependent: :destroy
 
   mount_uploader :profile_image, ProfileImageUploader
@@ -85,10 +86,6 @@ class Tag < ActsAsTaggableOn::Tag
     errors.add(:name, "contains non-ASCII characters") unless name.match?(/\A[[a-z0-9]]+\z/i)
   end
 
-  def mod_chat_channel
-    ChatChannel.find(mod_chat_channel_id) if mod_chat_channel_id
-  end
-
   private
 
   def evaluate_markdown

db/migrate/20200917154147_add_missing_foreign_keys_to_feedback_messages_notes_tags_users_roles.rb

@@ -0,0 +1,14 @@
+class AddMissingForeignKeysToFeedbackMessagesNotesTagsUsersRoles < ActiveRecord::Migration[6.0]
+  def change
+    add_foreign_key :feedback_messages, :users, column: :affected_id, on_delete: :nullify, validate: false
+    add_foreign_key :feedback_messages, :users, column: :offender_id, on_delete: :nullify, validate: false
+    add_foreign_key :feedback_messages, :users, column: :reporter_id, on_delete: :nullify, validate: false
+
+    add_foreign_key :notes, :users, column: :author_id, on_delete: :nullify, validate: false
+
+    add_foreign_key :tags, :badges, on_delete: :nullify, validate: false
+    add_foreign_key :tags, :chat_channels, column: :mod_chat_channel_id, on_delete: :nullify, validate: false
+
+    add_foreign_key :users_roles, :roles, on_delete: :cascade, validate: false
+  end
+end

db/migrate/20200917154234_validate_missing_foreign_keys_to_feedback_messages_notes_tags_users_roles.rb

@@ -0,0 +1,14 @@
+class ValidateMissingForeignKeysToFeedbackMessagesNotesTagsUsersRoles < ActiveRecord::Migration[6.0]
+  def change
+    validate_foreign_key :feedback_messages, :users, column: :affected_id
+    validate_foreign_key :feedback_messages, :users, column: :offender_id
+    validate_foreign_key :feedback_messages, :users, column: :reporter_id
+
+    validate_foreign_key :notes, :users, column: :author_id
+
+    validate_foreign_key :tags, :badges
+    validate_foreign_key :tags, :chat_channels, column: :mod_chat_channel_id
+
+    validate_foreign_key :users_roles, :roles
+  end
+end

db/migrate/20200917154256_add_missing_foreign_keys_to_taggings.rb

@@ -0,0 +1,5 @@
+class AddMissingForeignKeysToTaggings < ActiveRecord::Migration[6.0]
+  def change
+    add_foreign_key :taggings, :tags, on_delete: :cascade, validate: false
+  end
+end

db/migrate/20200917154306_validate_missing_foreign_keys_to_taggings.rb

@@ -0,0 +1,5 @@
+class ValidateMissingForeignKeysToTaggings < ActiveRecord::Migration[6.0]
+  def change
+    validate_foreign_key :taggings, :tags
+  end
+end

db/schema.rb

@@ -1399,6 +1399,9 @@ ActiveRecord::Schema.define(version: 2020_09_21_160153) do
   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 "feedback_messages", "users", column: "affected_id", on_delete: :nullify
+  add_foreign_key "feedback_messages", "users", column: "offender_id", on_delete: :nullify
+  add_foreign_key "feedback_messages", "users", column: "reporter_id", on_delete: :nullify
   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
@@ -1409,6 +1412,7 @@ ActiveRecord::Schema.define(version: 2020_09_21_160153) do
   add_foreign_key "mentions", "users", on_delete: :cascade
   add_foreign_key "messages", "chat_channels"
   add_foreign_key "messages", "users"
+  add_foreign_key "notes", "users", column: "author_id", on_delete: :nullify
   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
@@ -1440,11 +1444,15 @@ ActiveRecord::Schema.define(version: 2020_09_21_160153) do
   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 "taggings", "tags", on_delete: :cascade
+  add_foreign_key "tags", "badges", on_delete: :nullify
+  add_foreign_key "tags", "chat_channels", column: "mod_chat_channel_id", on_delete: :nullify
   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"
   add_foreign_key "user_subscriptions", "users", column: "subscriber_id"
+  add_foreign_key "users_roles", "roles", on_delete: :cascade
   add_foreign_key "users_roles", "users", on_delete: :cascade
   add_foreign_key "webhook_endpoints", "oauth_applications"
   add_foreign_key "webhook_endpoints", "users"

spec/models/chat_channel_spec.rb

@@ -9,8 +9,8 @@ RSpec.describe ChatChannel, type: :model do
     describe "builtin validations" do
       subject { chat_channel }
 
-      it { is_expected.to have_many(:messages).dependent(:destroy) }
       it { is_expected.to have_many(:chat_channel_memberships).dependent(:destroy) }
+      it { is_expected.to have_many(:messages).dependent(:destroy) }
       it { is_expected.to have_many(:users).through(:chat_channel_memberships) }
 
       it { is_expected.to validate_inclusion_of(:channel_type).in_array(%w[open invite_only direct]) }
@@ -19,6 +19,17 @@ RSpec.describe ChatChannel, type: :model do
       it { is_expected.to validate_presence_of(:channel_type) }
       it { is_expected.to validate_presence_of(:status) }
       it { is_expected.to validate_uniqueness_of(:slug) }
+
+      # rubocop:disable RSpec/NamedSubject
+      it do
+        expect(subject).to have_one(:mod_tag)
+          .class_name("Tag")
+          .inverse_of(:mod_chat_channel)
+          .with_foreign_key(:mod_chat_channel_id)
+          .dependent(:nullify)
+          .optional
+      end
+      # rubocop:enable RSpec/NamedSubject
     end
   end
 

spec/models/feedback_message_spec.rb

@@ -1,43 +1,56 @@
 require "rails_helper"
 
 RSpec.describe FeedbackMessage, type: :model do
-  subject(:feedback_message) { create(:feedback_message) }
+  let(:reporter) { create(:user) }
+  let(:abuse_report) { create(:feedback_message, :abuse_report, reporter: reporter) }
 
-  it { is_expected.to have_one(:email_message).dependent(:nullify).optional }
+  describe "validations" do
+    describe "builtin validations" do
+      subject(:feedback_message) { create(:feedback_message) }
 
-  it { is_expected.to validate_presence_of(:feedback_type) }
-  it { is_expected.to validate_presence_of(:message) }
-  it { is_expected.to validate_length_of(:reported_url).is_at_most(250) }
-  it { is_expected.to validate_length_of(:message).is_at_most(2500) }
+      it { is_expected.to belong_to(:offender).class_name("User").inverse_of(:offender_feedback_messages).optional }
+      it { is_expected.to belong_to(:reporter).class_name("User").inverse_of(:reporter_feedback_messages).optional }
+      it { is_expected.to belong_to(:affected).class_name("User").inverse_of(:affected_feedback_messages).optional }
 
-  it do
-    expect(feedback_message).to validate_inclusion_of(:category)
-      .in_array(["spam", "other", "rude or vulgar", "harassment", "bug"])
-  end
+      it { is_expected.to have_one(:email_message).dependent(:nullify).optional }
+      it { is_expected.to have_many(:notes).inverse_of(:noteable).dependent(:destroy) }
 
-  it do
-    expect(feedback_message).to validate_inclusion_of(:status)
-      .in_array(%w[Open Invalid Resolved])
-  end
+      it { is_expected.to validate_presence_of(:feedback_type) }
+      it { is_expected.to validate_presence_of(:message) }
+      it { is_expected.to validate_length_of(:reported_url).is_at_most(250) }
+      it { is_expected.to validate_length_of(:message).is_at_most(2500) }
 
-  describe "validations for an abuse report" do
-    subject(:feedback_message) { create(:feedback_message, :abuse_report) }
+      it do
+        expect(feedback_message).to validate_inclusion_of(:category)
+          .in_array(["spam", "other", "rude or vulgar", "harassment", "bug"])
+      end
 
-    it { is_expected.to validate_presence_of(:reported_url) }
-    it { is_expected.to validate_uniqueness_of(:reporter_id).scoped_to(%i[reported_url feedback_type]) }
-    it { is_expected.to validate_length_of(:reported_url).is_at_most(250) }
-    it { is_expected.to validate_presence_of(:category) }
-  end
+      it do
+        expect(feedback_message).to validate_inclusion_of(:status)
+          .in_array(%w[Open Invalid Resolved])
+      end
+    end
 
-  describe "validations for a bug report" do
-    subject(:feedback_message) { create(:feedback_message, :bug_report) }
+    describe "validations for an abuse report" do
+      subject(:feedback_message) { abuse_report }
 
-    it { is_expected.not_to validate_presence_of(:reported_url) }
-  end
+      it { is_expected.to validate_presence_of(:reported_url) }
+      it { is_expected.to validate_uniqueness_of(:reporter_id).scoped_to(%i[reported_url feedback_type]) }
+      it { is_expected.to validate_length_of(:reported_url).is_at_most(250) }
+      it { is_expected.to validate_presence_of(:category) }
 
-  describe "validations without a reporter_id" do
-    subject(:feedback_message) { build(:feedback_message, :abuse_report, reporter_id: nil) }
+      it "does not check for uniqueness if the new abuse report does not have a reporter id" do
+        new_abuse_report = build(:feedback_message, :abuse_report)
+        new_abuse_report.reported_url = abuse_report.reported_url
 
-    it { is_expected.not_to validate_uniqueness_of(:reporter_id).scoped_to(%i[reported_url feedback_type]) }
+        expect(new_abuse_report).to be_valid
+      end
+    end
+
+    describe "validations for a bug report" do
+      subject(:feedback_message) { create(:feedback_message, :bug_report, reporter: reporter) }
+
+      it { is_expected.not_to validate_presence_of(:reported_url) }
+    end
   end
 end

spec/models/tag_spec.rb

@@ -3,10 +3,25 @@ require "rails_helper"
 RSpec.describe Tag, type: :model do
   let(:tag) { build(:tag) }
 
-  it { is_expected.to validate_length_of(:name).is_at_most(30) }
-  it { is_expected.not_to allow_value("#Hello", "c++", "AWS-Lambda").for(:name) }
-
   describe "validations" do
+    describe "builtin validations" do
+      subject { tag }
+
+      it { is_expected.to belong_to(:badge).optional }
+      it { is_expected.to have_one(:sponsorship).inverse_of(:sponsorable).dependent(:destroy).optional }
+
+      it { is_expected.to validate_length_of(:name).is_at_most(30) }
+      it { is_expected.not_to allow_value("#Hello", "c++", "AWS-Lambda").for(:name) }
+
+      # rubocop:disable RSpec/NamedSubject
+      it do
+        expect(subject).to belong_to(:mod_chat_channel)
+          .class_name("ChatChannel")
+          .optional
+      end
+      # rubocop:enable RSpec/NamedSubject
+    end
+
     describe "bg_color_hex" do
       it "passes validations if bg_color_hex is valid" do
         tag.bg_color_hex = "#000000"