From 0ca4e8b3719995450cec0542ef50268b9bbc3e5c Mon Sep 17 00:00:00 2001 From: rhymes Date: Tue, 29 Sep 2020 16:49:47 +0200 Subject: [PATCH] [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 | 3 + app/models/tag.rb | 5 +- ...eedback_messages_notes_tags_users_roles.rb | 14 ++++ ...eedback_messages_notes_tags_users_roles.rb | 14 ++++ ...56_add_missing_foreign_keys_to_taggings.rb | 5 ++ ...lidate_missing_foreign_keys_to_taggings.rb | 5 ++ db/schema.rb | 8 +++ spec/models/chat_channel_spec.rb | 13 +++- spec/models/feedback_message_spec.rb | 69 +++++++++++-------- spec/models/tag_spec.rb | 21 +++++- 10 files changed, 121 insertions(+), 36 deletions(-) create mode 100644 db/migrate/20200917154147_add_missing_foreign_keys_to_feedback_messages_notes_tags_users_roles.rb create mode 100644 db/migrate/20200917154234_validate_missing_foreign_keys_to_feedback_messages_notes_tags_users_roles.rb create mode 100644 db/migrate/20200917154256_add_missing_foreign_keys_to_taggings.rb create mode 100644 db/migrate/20200917154306_validate_missing_foreign_keys_to_taggings.rb diff --git a/app/models/chat_channel.rb b/app/models/chat_channel.rb index eab7ef3bb..1d3d18a01 100644 --- a/app/models/chat_channel.rb +++ b/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 diff --git a/app/models/tag.rb b/app/models/tag.rb index 6b624a389..c64a70db1 100644 --- a/app/models/tag.rb +++ b/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 diff --git a/db/migrate/20200917154147_add_missing_foreign_keys_to_feedback_messages_notes_tags_users_roles.rb b/db/migrate/20200917154147_add_missing_foreign_keys_to_feedback_messages_notes_tags_users_roles.rb new file mode 100644 index 000000000..af8daedf5 --- /dev/null +++ b/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 diff --git a/db/migrate/20200917154234_validate_missing_foreign_keys_to_feedback_messages_notes_tags_users_roles.rb b/db/migrate/20200917154234_validate_missing_foreign_keys_to_feedback_messages_notes_tags_users_roles.rb new file mode 100644 index 000000000..e5d3f9981 --- /dev/null +++ b/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 diff --git a/db/migrate/20200917154256_add_missing_foreign_keys_to_taggings.rb b/db/migrate/20200917154256_add_missing_foreign_keys_to_taggings.rb new file mode 100644 index 000000000..06eb69cf2 --- /dev/null +++ b/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 diff --git a/db/migrate/20200917154306_validate_missing_foreign_keys_to_taggings.rb b/db/migrate/20200917154306_validate_missing_foreign_keys_to_taggings.rb new file mode 100644 index 000000000..d2e8b5e4a --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 180a9ad4f..2f2481448 100644 --- a/db/schema.rb +++ b/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" diff --git a/spec/models/chat_channel_spec.rb b/spec/models/chat_channel_spec.rb index 56d1516b0..93d81b6bc 100644 --- a/spec/models/chat_channel_spec.rb +++ b/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 diff --git a/spec/models/feedback_message_spec.rb b/spec/models/feedback_message_spec.rb index f569d68e3..372d50f07 100644 --- a/spec/models/feedback_message_spec.rb +++ b/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 diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 07233f638..b503c02aa 100644 --- a/spec/models/tag_spec.rb +++ b/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"