Appeasing Rubocop as it sneaks some changes in (#16085)

I was working on another branch and as part of my commit, Rubocop
removed a validation (but not the spec that asserted the validation).

Below is the "non-updating" rubocop offense on the other branch.

```shell
❯ rubocop ./app/models/notification_subscription.rb
Inspecting 1 file
C

Offenses:

app/models/notification_subscription.rb:13:29: C: [Correctable]
Rails/RedundantPresenceValidationOnBelongsTo: Remove explicit presence
validation for notifiable_id.
  validates :notifiable_id, presence: true
                            ^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense auto-correctable
```

To remediate, I ran:

```shell
> rubocop --only "Rails/RedundantPresenceValidationOnBelongsTo" \
  --auto-correct
```

This resolved the `app/models`.  Then did some regex magic and removed
the assertions from `spec/models`.

For Forem folks, I wrote a [forem.team post][1] discuss if this is how
we want to proceed.

[1]:https://forem.team/jeremy/rubocop-auto-updating-mayhem-33a6
This commit is contained in:
Jeremy Friesen 2022-01-13 07:48:01 -05:00 committed by GitHub
parent ca646f65a9
commit b115b2d17e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
35 changed files with 13 additions and 56 deletions

View file

@ -4,7 +4,6 @@ class Collection < ApplicationRecord
belongs_to :user
belongs_to :organization, optional: true
validates :user_id, presence: true
validates :slug, presence: true, uniqueness: { scope: :user_id }
after_touch :touch_articles

View file

@ -11,6 +11,5 @@ class DiscussionLock < ApplicationRecord
include StringAttributeCleaner.for(:notes, :reason)
validates :article_id, presence: true, uniqueness: true
validates :locking_user_id, presence: true
validates :article_id, uniqueness: true
end

View file

@ -33,9 +33,7 @@ class Follow < ApplicationRecord
after_save :touch_follower
validates :blocked, inclusion: { in: [true, false] }
validates :followable_id, presence: true
validates :followable_type, presence: true
validates :follower_id, presence: true
validates :follower_type, presence: true
validates :subscription_status, presence: true, inclusion: { in: %w[all_articles none] }

View file

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

View file

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

View file

@ -18,7 +18,6 @@ class Identity < ApplicationRecord
validates :uid, uniqueness: { scope: :provider }, if: proc { |identity|
identity.uid_changed? || identity.provider_changed?
}
validates :user_id, presence: true
validates :user_id, uniqueness: { scope: :provider }, if: proc { |identity|
identity.user_id_changed? || identity.provider_changed?
}

View file

@ -19,7 +19,6 @@ class Listing < ApplicationRecord
acts_as_taggable_on :tags
has_many :credits, as: :purchase, inverse_of: :purchase, dependent: :nullify
validates :user_id, presence: true
validates :organization_id, presence: true, unless: :user_id?
validates :title, presence: true, length: { maximum: 128 }

View file

@ -9,8 +9,7 @@ class Mention < ApplicationRecord
belongs_to :user
belongs_to :mentionable, polymorphic: true
validates :user_id, presence: true, uniqueness: { scope: %i[mentionable_id mentionable_type] }
validates :mentionable_id, presence: true
validates :user_id, uniqueness: { scope: %i[mentionable_id mentionable_type] }
validates :mentionable_type, presence: true
validate :permission

View file

@ -10,11 +10,13 @@ class NotificationSubscription < ApplicationRecord
belongs_to :user
validates :config, presence: true, inclusion: { in: %w[all_comments top_level_comments only_author_comments] }
validates :notifiable_id, presence: true
validates :notifiable_type, presence: true, inclusion: { in: %w[Comment Article] }
validates :user_id, uniqueness: { scope: %i[notifiable_type notifiable_id] }
class << self
# @param notifiable [Comment, Article]
#
# @see notifiable_type's validation
def update_notification_subscriptions(notifiable)
NotificationSubscriptions::UpdateWorker.perform_async(
notifiable.id,

View file

@ -7,7 +7,7 @@ class OrganizationMembership < ApplicationRecord
USER_TYPES = %w[admin member guest].freeze
validates :user_id, :organization_id, :type_of_user, presence: true
validates :type_of_user, presence: true
validates :user_id, uniqueness: { scope: :organization_id }
validates :type_of_user, inclusion: { in: USER_TYPES }

View file

@ -5,6 +5,6 @@ class PodcastEpisodeAppearance < ApplicationRecord
belongs_to :user, class_name: "User", inverse_of: :podcast_episode_appearances
belongs_to :podcast_episode
validates :podcast_episode_id, uniqueness: { scope: :user_id }
validates :podcast_episode_id, :user_id, :role, presence: true
validates :role, presence: true
validates :role, inclusion: { in: %w[host guest], message: "provided role is not valid" }
end

View file

@ -6,5 +6,4 @@ class PodcastOwnership < ApplicationRecord
belongs_to :podcast
validates :podcast_id, uniqueness: { scope: :user_id }
validates :podcast_id, :user_id, presence: true
end

View file

@ -14,9 +14,9 @@ class PollVote < ApplicationRecord
counter_culture :poll
# In the future we'll remove this constraint if/when we allow multi-answer polls
validates :poll_id, presence: true, uniqueness: { scope: :user_id }
validates :poll_id, uniqueness: { scope: :user_id }
validates :poll_option_id, presence: true, uniqueness: { scope: :user_id }
validates :poll_option_id, uniqueness: { scope: :user_id }
validate :one_vote_per_poll_per_user
after_destroy :touch_poll_votes_count

View file

@ -5,9 +5,8 @@ class ProfilePin < ApplicationRecord
belongs_to :pinnable, polymorphic: true
belongs_to :profile, polymorphic: true
validates :profile_id, presence: true
validates :profile_type, inclusion: { in: %w[User] } # Future could be organization, tag, etc.
validates :pinnable_id, presence: true, uniqueness: { scope: %i[profile_id profile_type pinnable_type] }
validates :pinnable_id, uniqueness: { scope: %i[profile_id profile_type pinnable_type] }
validates :pinnable_type, inclusion: { in: %w[Article] } # Future could be comments, etc.
validate :only_five_pins_per_profile, on: :create
validate :pinnable_belongs_to_profile

View file

@ -19,7 +19,7 @@ class Sponsorship < ApplicationRecord
validates :level, presence: true, inclusion: { in: LEVELS }
validates :status, presence: true, inclusion: { in: STATUSES }
validates :url, url: { allow_blank: true, no_local: true, schemes: %w[http https] }
validates :user, :organization, :featured_number, presence: true
validates :featured_number, presence: true
validates :sponsorable_type, inclusion: {
in: SPONSORABLE_TYPES,
allow_blank: true,

View file

@ -1,7 +1,4 @@
class TagAdjustment < ApplicationRecord
validates :user_id, presence: true
validates :article_id, presence: true
validates :tag_id, presence: true
validates :tag_name, presence: true, uniqueness: { scope: :article_id, message: "can't be an already adjusted tag" }
validates :reason_for_adjustment, presence: true
validates :adjustment_type, inclusion: { in: %w[removal addition] }, presence: true

View file

@ -2,7 +2,7 @@ class UserBlock < ApplicationRecord
belongs_to :blocker, class_name: "User", inverse_of: :blocker_blocks
belongs_to :blocked, class_name: "User", inverse_of: :blocked_blocks
validates :blocked_id, :blocker_id, :config, presence: true
validates :config, presence: true
validates :blocked_id, uniqueness: { scope: %i[blocker_id] }
validates :config, inclusion: { in: %w[default] }
validate :blocker_cannot_be_same_as_blocked

View file

@ -11,10 +11,8 @@ class UserSubscription < ApplicationRecord
belongs_to :subscriber, class_name: "User", inverse_of: :subscribed_to_user_subscriptions
belongs_to :user_subscription_sourceable, polymorphic: true, optional: true
validates :author_id, presence: true
validates :subscriber_email, presence: true
validates :subscriber_id, presence: true, uniqueness: {
validates :subscriber_id, uniqueness: {
scope: %i[subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id]
}

View file

@ -24,7 +24,6 @@ module Users
:brand_color2,
format: { with: HEX_COLOR_REGEXP, message: "is not a valid hex color" },
allow_nil: true
validates :user_id, presence: true
validates :experience_level, numericality: { less_than_or_equal_to: 10 }, allow_blank: true
validates :feed_referential_link, inclusion: { in: [true, false] }
validates :feed_url, length: { maximum: 500 }, allow_nil: true

View file

@ -9,7 +9,6 @@ RSpec.describe Collection, type: :model do
it { is_expected.to belong_to(:organization).optional }
it { is_expected.to have_many(:articles).dependent(:nullify) }
it { is_expected.to validate_presence_of(:user_id) }
it { is_expected.to validate_presence_of(:slug) }
it { is_expected.to validate_uniqueness_of(:slug).scoped_to(:user_id) }
end

View file

@ -11,8 +11,6 @@ RSpec.describe DiscussionLock, type: :model do
describe "validations" do
subject { discussion_lock }
it { is_expected.to validate_presence_of(:article_id) }
it { is_expected.to validate_presence_of(:locking_user_id) }
it { is_expected.to validate_uniqueness_of(:article_id) }
it "sanitizes attributes before validation", :aggregate_failures do

View file

@ -9,9 +9,7 @@ RSpec.describe Follow, type: :model do
subject { user.follow(user_2) }
it { is_expected.to validate_inclusion_of(:subscription_status).in_array(%w[all_articles none]) }
it { is_expected.to validate_presence_of(:followable_id) }
it { is_expected.to validate_presence_of(:followable_type) }
it { is_expected.to validate_presence_of(:follower_id) }
it { is_expected.to validate_presence_of(:follower_type) }
it { is_expected.to validate_presence_of(:subscription_status) }
end

View file

@ -15,7 +15,6 @@ RSpec.describe GithubRepo, type: :model do
describe "builtin validations" do
subject { repo }
it { is_expected.to validate_presence_of(:github_id_code) }
it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_presence_of(:url) }
it { is_expected.to validate_uniqueness_of(:github_id_code) }

View file

@ -1,7 +1,6 @@
require "rails_helper"
RSpec.describe HtmlVariantSuccess, type: :model do
it { is_expected.to validate_presence_of(:html_variant_id) }
it { is_expected.to belong_to(:html_variant) }
it { is_expected.to belong_to(:article).optional }
end

View file

@ -1,7 +1,6 @@
require "rails_helper"
RSpec.describe HtmlVariantTrial, type: :model do
it { is_expected.to validate_presence_of(:html_variant_id) }
it { is_expected.to belong_to(:html_variant) }
it { is_expected.to belong_to(:article).optional }
end

View file

@ -15,7 +15,6 @@ RSpec.describe Identity, type: :model do
it { is_expected.to validate_presence_of(:provider) }
it { is_expected.to validate_presence_of(:uid) }
it { is_expected.to validate_presence_of(:user_id) }
it { is_expected.to validate_uniqueness_of(:uid).scoped_to(:provider) }
it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(:provider) }

View file

@ -19,7 +19,6 @@ RSpec.describe NotificationSubscription, type: :model do
end
it { is_expected.to validate_presence_of(:config) }
it { is_expected.to validate_presence_of(:notifiable_id) }
it { is_expected.to validate_presence_of(:notifiable_type) }
it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(%i[notifiable_type notifiable_id]) }

View file

@ -6,8 +6,6 @@ RSpec.describe OrganizationMembership, type: :model do
let(:organization) { create(:organization) }
it { is_expected.to validate_presence_of(:user_id) }
it { is_expected.to validate_presence_of(:organization_id) }
it { is_expected.to validate_presence_of(:type_of_user) }
it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(:organization_id) }
it { is_expected.to validate_inclusion_of(:type_of_user).in_array(OrganizationMembership::USER_TYPES) }

View file

@ -9,8 +9,6 @@ RSpec.describe PodcastEpisodeAppearance, type: :model do
it { is_expected.to belong_to(:user).inverse_of(:podcast_episode_appearances) }
it { is_expected.to belong_to(:podcast_episode) }
it { is_expected.to validate_presence_of(:podcast_episode_id) }
it { is_expected.to validate_presence_of(:user_id) }
it { is_expected.to validate_presence_of(:role) }
it do

View file

@ -9,8 +9,6 @@ RSpec.describe PodcastOwnership, type: :model do
it { is_expected.to belong_to(:owner).class_name("User").with_foreign_key(:user_id).inverse_of(:podcasts_owned) }
it { is_expected.to belong_to(:podcast) }
it { is_expected.to validate_presence_of(:podcast_id) }
it { is_expected.to validate_presence_of(:user_id) }
it { is_expected.to validate_uniqueness_of(:podcast_id).scoped_to(:user_id) }
end
end

View file

@ -16,7 +16,6 @@ RSpec.describe RatingVote, type: :model do
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

View file

@ -52,9 +52,7 @@ RSpec.describe Sponsorship, type: :model do
it { is_expected.to validate_inclusion_of(:status).in_array(Sponsorship::STATUSES) }
it { is_expected.to validate_presence_of(:level) }
it { is_expected.to validate_presence_of(:organization) }
it { is_expected.to validate_presence_of(:status) }
it { is_expected.to validate_presence_of(:user) }
it { is_expected.not_to allow_values(nil).for(:featured_number) }
it { is_expected.to allow_values(nil).for(:expires_at) }

View file

@ -12,9 +12,6 @@ RSpec.describe TagAdjustment, type: :model do
let(:mod_user) { create(:user) }
let(:regular_user) { create(:user) }
it { is_expected.to validate_presence_of(:user_id) }
it { is_expected.to validate_presence_of(:article_id) }
it { is_expected.to validate_presence_of(:tag_id) }
it { is_expected.to validate_presence_of(:tag_name) }
it { is_expected.to validate_presence_of(:adjustment_type) }
it { is_expected.to validate_presence_of(:status) }

View file

@ -5,7 +5,6 @@ RSpec.describe Tweet, type: :model, vcr: true do
let(:tweet_reply_id) { "1242938461784608770" }
let(:retweet_id) { "1262395854469677058" }
it { is_expected.to validate_presence_of(:twitter_id_code) }
it { is_expected.to validate_presence_of(:full_fetched_object_serialized) }
describe ".find_or_fetch" do

View file

@ -7,11 +7,8 @@ RSpec.describe UserSubscription, type: :model do
let(:source) { create(:article, :with_user_subscription_tag_role_user, with_user_subscription_tag: true) }
describe "validations" do
it { is_expected.to validate_presence_of(:user_subscription_sourceable_id) }
it { is_expected.to validate_presence_of(:user_subscription_sourceable_type) }
it { is_expected.to validate_presence_of(:subscriber_id) }
it { is_expected.to validate_presence_of(:subscriber_email) }
it { is_expected.to validate_presence_of(:author_id) }
it { is_expected.to validate_inclusion_of(:user_subscription_sourceable_type).in_array(%w[Article]) }
# rubocop:disable RSpec/NamedSubject