From 24b65c0c27ad1d3b3d1792e215f1700e7290ebc2 Mon Sep 17 00:00:00 2001 From: rhymes Date: Thu, 20 Aug 2020 16:53:53 +0200 Subject: [PATCH] [deploy] Add relations and foreign keys between Podcast related models (#9888) * Add proper relations and dependent clauses * Add foreign key between podcast_episodes and podcasts --- .rubocop_todo.yml | 2 -- app/models/podcast.rb | 2 +- app/models/podcast_episode.rb | 2 +- ...issing_foreign_keys_to_podcast_episodes.rb | 5 +++++ ...issing_foreign_keys_to_podcast_episodes.rb | 5 +++++ db/schema.rb | 3 ++- spec/models/podcast_episode_spec.rb | 17 ++++++++++++----- spec/models/podcast_spec.rb | 19 +++++++++++++------ 8 files changed, 39 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20200820093731_add_missing_foreign_keys_to_podcast_episodes.rb create mode 100644 db/migrate/20200820093752_validate_add_missing_foreign_keys_to_podcast_episodes.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3950358e9..15fadbf50 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -26,8 +26,6 @@ Rails/HasManyOrHasOneDependent: - 'app/models/article.rb' - 'app/models/concerns/user_subscription_sourceable.rb' - 'app/models/organization.rb' - - 'app/models/podcast.rb' - - 'app/models/podcast_episode.rb' - 'app/models/user.rb' # Offense count: 4 diff --git a/app/models/podcast.rb b/app/models/podcast.rb index d6ba40457..70986c12d 100644 --- a/app/models/podcast.rb +++ b/app/models/podcast.rb @@ -1,8 +1,8 @@ class Podcast < ApplicationRecord resourcify - has_many :podcast_episodes belongs_to :creator, class_name: "User", inverse_of: :created_podcasts, optional: true + has_many :podcast_episodes, dependent: :destroy mount_uploader :image, ProfileImageUploader mount_uploader :pattern_image, ProfileImageUploader diff --git a/app/models/podcast_episode.rb b/app/models/podcast_episode.rb index 508b04643..897f85138 100644 --- a/app/models/podcast_episode.rb +++ b/app/models/podcast_episode.rb @@ -15,7 +15,7 @@ class PodcastEpisode < ApplicationRecord delegate :published, to: :podcast belongs_to :podcast - has_many :comments, as: :commentable, inverse_of: :commentable + has_many :comments, as: :commentable, inverse_of: :commentable, dependent: :nullify mount_uploader :image, ProfileImageUploader mount_uploader :social_image, ProfileImageUploader diff --git a/db/migrate/20200820093731_add_missing_foreign_keys_to_podcast_episodes.rb b/db/migrate/20200820093731_add_missing_foreign_keys_to_podcast_episodes.rb new file mode 100644 index 000000000..7d1aa0241 --- /dev/null +++ b/db/migrate/20200820093731_add_missing_foreign_keys_to_podcast_episodes.rb @@ -0,0 +1,5 @@ +class AddMissingForeignKeysToPodcastEpisodes < ActiveRecord::Migration[6.0] + def change + add_foreign_key :podcast_episodes, :podcasts, column: :podcast_id, on_delete: :cascade, validate: false + end +end diff --git a/db/migrate/20200820093752_validate_add_missing_foreign_keys_to_podcast_episodes.rb b/db/migrate/20200820093752_validate_add_missing_foreign_keys_to_podcast_episodes.rb new file mode 100644 index 000000000..417461e98 --- /dev/null +++ b/db/migrate/20200820093752_validate_add_missing_foreign_keys_to_podcast_episodes.rb @@ -0,0 +1,5 @@ +class ValidateAddMissingForeignKeysToPodcastEpisodes < ActiveRecord::Migration[6.0] + def change + validate_foreign_key :podcast_episodes, :podcasts + end +end diff --git a/db/schema.rb b/db/schema.rb index e5357e789..927fd7483 100644 --- a/db/schema.rb +++ b/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_18_163834) do +ActiveRecord::Schema.define(version: 2020_08_20_093752) do # These are extensions that must be enabled in order to support this database enable_extension "citext" @@ -1378,6 +1378,7 @@ ActiveRecord::Schema.define(version: 2020_08_18_163834) do 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 "page_views", "articles", on_delete: :cascade + 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 add_foreign_key "poll_skips", "polls", on_delete: :cascade diff --git a/spec/models/podcast_episode_spec.rb b/spec/models/podcast_episode_spec.rb index 3ba3d659a..954f310d0 100644 --- a/spec/models/podcast_episode_spec.rb +++ b/spec/models/podcast_episode_spec.rb @@ -3,12 +3,19 @@ require "rails_helper" RSpec.describe PodcastEpisode, type: :model do let(:podcast_episode) { create(:podcast_episode) } - it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_presence_of(:slug) } - it { is_expected.to validate_presence_of(:media_url) } - it { is_expected.to validate_presence_of(:guid) } - describe "validations" do + describe "builtin validations" do + subject { podcast_episode } + + it { is_expected.to belong_to(:podcast) } + it { is_expected.to have_many(:comments).inverse_of(:commentable).dependent(:nullify) } + + it { is_expected.to validate_presence_of(:guid) } + it { is_expected.to validate_presence_of(:media_url) } + it { is_expected.to validate_presence_of(:slug) } + it { is_expected.to validate_presence_of(:title) } + end + # Couldn't use shoulda matchers for these tests because: # Shoulda uses `save(validate: false)` which skips validations, but runs callbacks # So an invalid record is saved and the elasticsearch callback fails because there's no associated podcast diff --git a/spec/models/podcast_spec.rb b/spec/models/podcast_spec.rb index 8e9f59da5..8e68795a9 100644 --- a/spec/models/podcast_spec.rb +++ b/spec/models/podcast_spec.rb @@ -3,12 +3,6 @@ require "rails_helper" RSpec.describe Podcast, type: :model do let(:podcast) { create(:podcast) } - it { is_expected.to validate_presence_of(:image) } - it { is_expected.to validate_presence_of(:slug) } - it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_presence_of(:main_color_hex) } - it { is_expected.to validate_presence_of(:feed_url) } - it "has a creator" do user = build(:user) pod = create(:podcast, creator: user) @@ -24,6 +18,19 @@ RSpec.describe Podcast, type: :model do end describe "validations" do + describe "builtin validations" do + subject { podcast } + + it { is_expected.to belong_to(:creator).class_name("User").inverse_of(:created_podcasts).optional } + it { is_expected.to have_many(:podcast_episodes).dependent(:destroy) } + + it { is_expected.to validate_presence_of(:feed_url) } + it { is_expected.to validate_presence_of(:image) } + it { is_expected.to validate_presence_of(:main_color_hex) } + it { is_expected.to validate_presence_of(:slug) } + it { is_expected.to validate_presence_of(:title) } + end + # Couldn't use shoulda uniqueness matchers for these tests because: # Shoulda uses `save(validate: false)` which skips validations # So an invalid record is trying to be saved but fails because of the db constraints