From a926b162be4d81f876eb0605e0bd7744a7fb6d65 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 4 Aug 2019 14:40:37 -0500 Subject: [PATCH] models: drop unnecessary presence validations. In rails 5, belongs_to associations automatically validate that the associated item is present, meaning that we don't need to validate these things manually any more. --- app/models/ban.rb | 2 +- app/models/bulk_update_request.rb | 1 - app/models/comment.rb | 4 ---- app/models/comment_vote.rb | 2 +- app/models/dmail_filter.rb | 1 - app/models/forum_post.rb | 10 +--------- app/models/forum_topic.rb | 2 +- app/models/ip_ban.rb | 2 +- app/models/janitor_trial.rb | 1 - app/models/mod_action.rb | 1 - app/models/note.rb | 10 +--------- app/models/post_appeal.rb | 2 +- app/models/post_vote.rb | 2 +- app/models/tag_relationship.rb | 3 +-- app/models/user_feedback.rb | 2 +- app/models/user_name_change_request.rb | 2 +- 16 files changed, 11 insertions(+), 36 deletions(-) diff --git a/app/models/ban.rb b/app/models/ban.rb index fe118fdba..4d54228ee 100644 --- a/app/models/ban.rb +++ b/app/models/ban.rb @@ -7,7 +7,7 @@ class Ban < ApplicationRecord belongs_to :user belongs_to :banner, :class_name => "User" validate :user_is_inferior - validates_presence_of :user_id, :reason, :duration + validates_presence_of :reason, :duration before_validation :initialize_banner_id, :on => :create scope :unexpired, -> { where("bans.expires_at > ?", Time.now) } diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index fd6abda69..1f6c94c1a 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -6,7 +6,6 @@ class BulkUpdateRequest < ApplicationRecord belongs_to :forum_post, optional: true belongs_to :approver, optional: true, class_name: "User" - validates_presence_of :user validates_presence_of :script validates_presence_of :title, if: ->(rec) {rec.forum_topic_id.blank?} validates_inclusion_of :status, :in => %w(pending approved rejected) diff --git a/app/models/comment.rb b/app/models/comment.rb index 5adf2385c..ee538b90d 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -136,10 +136,6 @@ class Comment < ApplicationRecord extend SearchMethods include VoteMethods - def validate_post_exists - errors.add(:post, "must exist") unless Post.exists?(post_id) - end - def validate_creator_is_not_limited if creator.is_comment_limited? && !do_not_bump_post? errors.add(:base, "You can only post #{Danbooru.config.member_comment_limit} comments per hour") diff --git a/app/models/comment_vote.rb b/app/models/comment_vote.rb index 4ed666095..fa08641af 100644 --- a/app/models/comment_vote.rb +++ b/app/models/comment_vote.rb @@ -4,7 +4,7 @@ class CommentVote < ApplicationRecord belongs_to :comment belongs_to :user before_validation :initialize_user, :on => :create - validates_presence_of :user_id, :comment_id, :score + validates_presence_of :score validates_uniqueness_of :user_id, :scope => :comment_id, :message => "have already voted for this comment" validate :validate_user_can_vote validate :validate_comment_can_be_down_voted diff --git a/app/models/dmail_filter.rb b/app/models/dmail_filter.rb index 6cd8f58f1..3842b8d3b 100644 --- a/app/models/dmail_filter.rb +++ b/app/models/dmail_filter.rb @@ -2,7 +2,6 @@ class DmailFilter < ApplicationRecord extend Memoist belongs_to :user - validates_presence_of :user before_validation :initialize_user def initialize_user diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index 602fe2240..6b101d1d4 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -13,9 +13,8 @@ class ForumPost < ApplicationRecord after_create :update_topic_updated_at_on_create after_update :update_topic_updated_at_on_update_for_original_posts after_destroy :update_topic_updated_at_on_destroy - validates_presence_of :body, :creator_id + validates_presence_of :body validate :validate_topic_is_unlocked - validate :topic_id_not_invalid validate :topic_is_not_restricted, :on => :create before_destroy :validate_topic_is_unlocked after_save :delete_topic_if_original_post @@ -144,13 +143,6 @@ class ForumPost < ApplicationRecord end end - def topic_id_not_invalid - if topic_id && !topic - errors[:base] << "Topic ID is invalid" - return false - end - end - def topic_is_not_restricted if topic && !topic.visible?(creator) errors[:topic] << "is restricted" diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index af4c7a29a..432bfabfd 100644 --- a/app/models/forum_topic.rb +++ b/app/models/forum_topic.rb @@ -17,7 +17,7 @@ class ForumTopic < ApplicationRecord has_one :original_post, -> {order("forum_posts.id asc")}, class_name: "ForumPost", foreign_key: "topic_id", inverse_of: :topic has_many :subscriptions, :class_name => "ForumSubscription" before_validation :initialize_is_deleted, :on => :create - validates_presence_of :title, :creator_id + validates_presence_of :title validates_associated :original_post validates_inclusion_of :category_id, :in => CATEGORIES.keys validates_inclusion_of :min_level, :in => MIN_LEVELS.values diff --git a/app/models/ip_ban.rb b/app/models/ip_ban.rb index f345b06c2..4671ad70e 100644 --- a/app/models/ip_ban.rb +++ b/app/models/ip_ban.rb @@ -1,7 +1,7 @@ class IpBan < ApplicationRecord IP_ADDR_REGEX = /\A(?:[0-9]{1,3}\.){3}[0-9]{1,3}\Z/ belongs_to_creator - validates_presence_of :reason, :creator, :ip_addr + validates_presence_of :reason, :ip_addr validates_format_of :ip_addr, :with => IP_ADDR_REGEX validates_uniqueness_of :ip_addr, :if => ->(rec) {rec.ip_addr =~ IP_ADDR_REGEX} after_create do |rec| diff --git a/app/models/janitor_trial.rb b/app/models/janitor_trial.rb index c49a63b17..b8996bd5a 100644 --- a/app/models/janitor_trial.rb +++ b/app/models/janitor_trial.rb @@ -2,7 +2,6 @@ class JanitorTrial < ApplicationRecord belongs_to :user after_create :send_dmail after_create :promote_user - validates_presence_of :user belongs_to_creator validates_inclusion_of :status, :in => %w(active inactive) before_validation :initialize_status diff --git a/app/models/mod_action.rb b/app/models/mod_action.rb index a1b6f04b9..fc28d0ee7 100644 --- a/app/models/mod_action.rb +++ b/app/models/mod_action.rb @@ -1,7 +1,6 @@ class ModAction < ApplicationRecord belongs_to :creator, :class_name => "User" before_validation :initialize_creator, :on => :create - validates_presence_of :creator_id #####DIVISIONS##### #Groups: 0-999 diff --git a/app/models/note.rb b/app/models/note.rb index 9bd1f2d15..7789695b3 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -5,8 +5,7 @@ class Note < ApplicationRecord belongs_to :post belongs_to_creator has_many :versions, -> {order("note_versions.id ASC")}, :class_name => "NoteVersion", :dependent => :destroy - validates_presence_of :creator_id, :x, :y, :width, :height, :body - validate :post_must_exist + validates_presence_of :x, :y, :width, :height, :body validate :note_within_image after_save :update_post after_save :create_version @@ -68,13 +67,6 @@ class Note < ApplicationRecord extend SearchMethods include ApiMethods - def post_must_exist - if !Post.exists?(post_id) - errors.add :post, "must exist" - return false - end - end - def post_must_not_be_note_locked if is_locked? errors.add :post, "is note locked" diff --git a/app/models/post_appeal.rb b/app/models/post_appeal.rb index f32a8b348..84027cca4 100644 --- a/app/models/post_appeal.rb +++ b/app/models/post_appeal.rb @@ -3,7 +3,7 @@ class PostAppeal < ApplicationRecord belongs_to :creator, :class_name => "User" belongs_to :post - validates_presence_of :post, :reason, :creator_id, :creator_ip_addr + validates_presence_of :reason, :creator_ip_addr validate :validate_post_is_inactive validate :validate_creator_is_not_limited before_validation :initialize_creator, :on => :create diff --git a/app/models/post_vote.rb b/app/models/post_vote.rb index 80c00aa0e..f6c7ce361 100644 --- a/app/models/post_vote.rb +++ b/app/models/post_vote.rb @@ -6,7 +6,7 @@ class PostVote < ApplicationRecord attr_accessor :vote after_initialize :initialize_attributes, if: :new_record? - validates_presence_of :post_id, :user_id, :score + validates_presence_of :score validates_inclusion_of :score, :in => [SuperVoter::MAGNITUDE, 1, -1, -SuperVoter::MAGNITUDE] after_create :update_post_on_create after_destroy :update_post_on_destroy diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index 537df4504..26a0f693e 100644 --- a/app/models/tag_relationship.rb +++ b/app/models/tag_relationship.rb @@ -27,8 +27,7 @@ class TagRelationship < ApplicationRecord before_validation :initialize_creator, :on => :create before_validation :normalize_names validates_format_of :status, :with => /\A(active|deleted|pending|processing|queued|retired|error: .*)\Z/ - validates_presence_of :creator_id, :antecedent_name, :consequent_name - validates :creator, presence: { message: "must exist" }, if: -> { creator_id.present? } + validates_presence_of :antecedent_name, :consequent_name validates :approver, presence: { message: "must exist" }, if: -> { approver_id.present? } validates :forum_topic, presence: { message: "must exist" }, if: -> { forum_topic_id.present? } validate :antecedent_and_consequent_are_different diff --git a/app/models/user_feedback.rb b/app/models/user_feedback.rb index b3930370b..67d1843f0 100644 --- a/app/models/user_feedback.rb +++ b/app/models/user_feedback.rb @@ -3,7 +3,7 @@ class UserFeedback < ApplicationRecord belongs_to :user belongs_to_creator attr_accessor :disable_dmail_notification - validates_presence_of :user, :creator, :body, :category + validates_presence_of :body, :category validates_inclusion_of :category, :in => %w(positive negative neutral) validate :creator_is_gold validate :user_is_not_creator diff --git a/app/models/user_name_change_request.rb b/app/models/user_name_change_request.rb index ba862e820..798ca73fc 100644 --- a/app/models/user_name_change_request.rb +++ b/app/models/user_name_change_request.rb @@ -1,6 +1,6 @@ class UserNameChangeRequest < ApplicationRecord after_initialize :initialize_attributes, if: :new_record? - validates_presence_of :user_id, :original_name, :desired_name + validates_presence_of :original_name, :desired_name validates_inclusion_of :status, :in => %w(pending approved rejected) belongs_to :user belongs_to :approver, :class_name => "User", optional: true