From e47d0e0d05a9f0a2fbfa2a7b248c05d122be1a5f Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 23 Feb 2020 02:42:12 -0600 Subject: [PATCH] models: set more creator names explicitly. Set creators explicitly for bans, BURs, comment votes, and posts. --- app/controllers/bans_controller.rb | 2 +- app/controllers/users_controller.rb | 2 +- app/models/ban.rb | 5 ----- app/models/bulk_update_request.rb | 6 ------ app/models/comment.rb | 4 ++-- app/models/comment_vote.rb | 5 ----- app/models/forum_post.rb | 5 ----- app/models/forum_topic.rb | 5 ----- app/models/mod_action.rb | 9 ++------- app/models/post.rb | 11 ----------- app/models/user.rb | 1 - test/factories/comment_vote.rb | 1 + test/functional/comment_votes_controller_test.rb | 4 ++-- test/unit/comment_test.rb | 6 ------ test/unit/mod_action_test.rb | 4 ++-- test/unit/post_test.rb | 12 +++++++----- 16 files changed, 18 insertions(+), 64 deletions(-) diff --git a/app/controllers/bans_controller.rb b/app/controllers/bans_controller.rb index 2e7f45411..640672ad0 100644 --- a/app/controllers/bans_controller.rb +++ b/app/controllers/bans_controller.rb @@ -24,7 +24,7 @@ class BansController < ApplicationController end def create - @ban = Ban.create(ban_params(:create)) + @ban = Ban.create(banner: CurrentUser.user, **ban_params(:create)) if @ban.errors.any? render :action => "new" diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 503f2dae3..f43350b3a 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -59,7 +59,7 @@ class UsersController < ApplicationController end def create - @user = User.new(user_params(:create)) + @user = User.new(last_ip_addr: CurrentUser.ip_addr, **user_params(:create)) if !Danbooru.config.enable_recaptcha? || verify_recaptcha(model: @user) @user.save if @user.errors.empty? diff --git a/app/models/ban.rb b/app/models/ban.rb index 7b4e9dc6e..8c56eb6f2 100644 --- a/app/models/ban.rb +++ b/app/models/ban.rb @@ -8,7 +8,6 @@ class Ban < ApplicationRecord belongs_to :banner, :class_name => "User" validate :user_is_inferior validates_presence_of :reason, :duration - before_validation :initialize_banner_id, :on => :create scope :unexpired, -> { where("bans.expires_at > ?", Time.now) } scope :expired, -> { where("bans.expires_at <= ?", Time.now) } @@ -50,10 +49,6 @@ class Ban < ApplicationRecord end end - def initialize_banner_id - self.banner_id = CurrentUser.id if self.banner_id.blank? - end - def user_is_inferior if user if user.is_admin? diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index d2bfec01d..24505ab34 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -13,7 +13,6 @@ class BulkUpdateRequest < ApplicationRecord validate :script_formatted_correctly validate :forum_topic_id_not_invalid validate :validate_script, :on => :create - before_validation :initialize_attributes, :on => :create before_validation :normalize_text after_create :create_forum_topic after_save :update_notice @@ -173,11 +172,6 @@ class BulkUpdateRequest < ApplicationRecord lines.join("\n") end - def initialize_attributes - self.user_id = CurrentUser.user.id unless self.user_id - self.status = "pending" - end - def normalize_text self.script = script.downcase end diff --git a/app/models/comment.rb b/app/models/comment.rb index c98e74372..ca85317d0 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -48,9 +48,9 @@ class Comment < ApplicationRecord end module VoteMethods - def vote!(val) + def vote!(val, voter = CurrentUser.user) numerical_score = (val == "up") ? 1 : -1 - vote = votes.create!(:score => numerical_score) + vote = votes.create!(user: voter, score: numerical_score) if vote.is_positive? update_column(:score, score + 1) diff --git a/app/models/comment_vote.rb b/app/models/comment_vote.rb index 28163cb9c..9d080aa54 100644 --- a/app/models/comment_vote.rb +++ b/app/models/comment_vote.rb @@ -3,7 +3,6 @@ class CommentVote < ApplicationRecord belongs_to :comment belongs_to :user - before_validation :initialize_user, :on => :create validates_presence_of :score validates_uniqueness_of :user_id, :scope => :comment_id, :message => "have already voted for this comment" validate :validate_user_can_vote @@ -52,10 +51,6 @@ class CommentVote < ApplicationRecord score == -1 end - def initialize_user - self.user_id = CurrentUser.user.id - end - def self.available_includes [:comment, :user] end diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index 75a51a768..cc2782240 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -10,7 +10,6 @@ class ForumPost < ApplicationRecord has_one :tag_implication has_one :bulk_update_request - before_validation :initialize_is_deleted, :on => :create before_save :update_dtext_links, if: :dtext_links_changed? before_create :autoreport_spam after_create :update_topic_updated_at_on_create @@ -183,10 +182,6 @@ class ForumPost < ApplicationRecord topic.response_count -= 1 end - def initialize_is_deleted - self.is_deleted = false if is_deleted.nil? - end - def quoted_response DText.quote(body, creator.name) end diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index 1779f3453..887c1ba48 100644 --- a/app/models/forum_topic.rb +++ b/app/models/forum_topic.rb @@ -19,7 +19,6 @@ class ForumTopic < ApplicationRecord has_many :moderation_reports, through: :posts has_one :original_post, -> {order("forum_posts.id asc")}, class_name: "ForumPost", foreign_key: "topic_id", inverse_of: :topic - before_validation :initialize_is_deleted, :on => :create validates_presence_of :title validates_associated :original_post validates_inclusion_of :category_id, :in => CATEGORIES.keys @@ -151,10 +150,6 @@ class ForumTopic < ApplicationRecord ModAction.log("undeleted forum topic ##{id} (title: #{title})", :forum_topic_undelete) end - def initialize_is_deleted - self.is_deleted = false if is_deleted.nil? - end - def page_for(post_id) (posts.where("id < ?", post_id).count / Danbooru.config.posts_per_page.to_f).ceil end diff --git a/app/models/mod_action.rb b/app/models/mod_action.rb index 30f67c945..60e5108f9 100644 --- a/app/models/mod_action.rb +++ b/app/models/mod_action.rb @@ -1,6 +1,5 @@ class ModAction < ApplicationRecord belongs_to :creator, :class_name => "User" - before_validation :initialize_creator, :on => :create api_attributes including: [:category_id] @@ -75,12 +74,8 @@ class ModAction < ApplicationRecord self.class.categories[category] end - def self.log(desc, cat = :other) - create(:description => desc, :category => categories[cat]) - end - - def initialize_creator - self.creator_id = CurrentUser.id + def self.log(desc, cat = :other, user = CurrentUser.user) + create(creator: user, description: desc, category: categories[cat]) end def self.available_includes diff --git a/app/models/post.rb b/app/models/post.rb index 1370802f2..8c5ef6fff 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -9,7 +9,6 @@ class Post < ApplicationRecord # Tags to copy when copying notes. NOTE_COPY_TAGS = %w[translated partially_translated check_translation translation_request reverse_translation] - before_validation :initialize_uploader, :on => :create before_validation :merge_old_changes before_validation :normalize_tags before_validation :strip_source @@ -984,15 +983,6 @@ class Post < ApplicationRecord end end - module UploaderMethods - def initialize_uploader - if uploader_id.blank? - self.uploader_id = CurrentUser.id - self.uploader_ip_addr = CurrentUser.ip_addr - end - end - end - module PoolMethods def pools Pool.where("pools.post_ids && array[?]", id) @@ -1691,7 +1681,6 @@ class Post < ApplicationRecord include PresenterMethods include TagMethods include FavoriteMethods - include UploaderMethods include PoolMethods include VoteMethods extend CountMethods diff --git a/app/models/user.rb b/app/models/user.rb index ce7b1e4c1..f2e7714ee 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -729,7 +729,6 @@ class User < ApplicationRecord end def initialize_attributes - self.last_ip_addr ||= CurrentUser.ip_addr self.enable_post_navigation = true self.new_post_navigation_layout = true self.enable_sequential_post_navigation = true diff --git a/test/factories/comment_vote.rb b/test/factories/comment_vote.rb index 1419077df..1b8c42af0 100644 --- a/test/factories/comment_vote.rb +++ b/test/factories/comment_vote.rb @@ -1,5 +1,6 @@ FactoryBot.define do factory(:comment_vote) do + user score {1} end end diff --git a/test/functional/comment_votes_controller_test.rb b/test/functional/comment_votes_controller_test.rb index d5502f6ac..6f7496779 100644 --- a/test/functional/comment_votes_controller_test.rb +++ b/test/functional/comment_votes_controller_test.rb @@ -26,7 +26,7 @@ class CommentVotesControllerTest < ActionDispatch::IntegrationTest end should "fail silently on errors" do - create(:comment_vote, comment: @comment, score: -1) + create(:comment_vote, user: @user, comment: @comment, score: -1) assert_difference("CommentVote.count", 0) do post_auth comment_comment_votes_path(comment_id: @comment.id, score: "down", format: "json"), @user assert_response 422 @@ -47,7 +47,7 @@ class CommentVotesControllerTest < ActionDispatch::IntegrationTest end should "fail on errors" do - create(:comment_vote, :comment => @comment, :score => -1) + create(:comment_vote, user: @user, comment: @comment, score: -1) assert_difference("CommentVote.count", 0) do post_auth comment_comment_votes_path(comment_id: @comment.id, :score => "down", format: "js"), @user assert_response 422 diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb index c8345d87b..ce01abb09 100644 --- a/test/unit/comment_test.rb +++ b/test/unit/comment_test.rb @@ -115,12 +115,6 @@ class CommentTest < ActiveSupport::TestCase end end - should "be created" do - comment = FactoryBot.build(:comment) - comment.save - assert(comment.errors.empty?, comment.errors.full_messages.join(", ")) - end - should "not validate if the post does not exist" do comment = FactoryBot.build(:comment, :post_id => -1) diff --git a/test/unit/mod_action_test.rb b/test/unit/mod_action_test.rb index 54c386dd2..52a8edf08 100644 --- a/test/unit/mod_action_test.rb +++ b/test/unit/mod_action_test.rb @@ -10,8 +10,8 @@ class ModActionTest < ActiveSupport::TestCase should "hide ip addresses from non-moderators in ip ban modactions" do as(@mod) { create(:ip_ban, ip_addr: "1.1.1.1", reason: "test") } - as(@user) { assert_equal(0, ModAction.search({}).count) } - as(@mod) { assert_equal(1, ModAction.search({}).count) } + assert_equal(0, ModAction.visible(@user).count) + assert_equal(1, ModAction.visible(@mod).count) end end end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index dc74510f9..c3bfc8c41 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -2066,7 +2066,7 @@ class PostTest < ActiveSupport::TestCase end should "return posts for the user: metatag" do - users = FactoryBot.create_list(:user, 2) + users = FactoryBot.create_list(:user, 2, created_at: 2.weeks.ago) posts = users.map { |u| FactoryBot.create(:post, uploader: u) } assert_tag_match([posts[0]], "user:#{users[0].name}") @@ -2105,7 +2105,9 @@ class PostTest < ActiveSupport::TestCase should "return posts for the noter: metatag" do users = FactoryBot.create_list(:user, 2) posts = FactoryBot.create_list(:post, 2) - notes = users.zip(posts).map { |u, p| FactoryBot.create(:note, creator: u, post: p) } + notes = users.zip(posts).map do |u, p| + as(u) { create(:note, post: p) } + end assert_tag_match([posts[0]], "noter:#{users[0].name}") assert_tag_match([posts[1]], "noter:#{users[1].name}") @@ -2201,7 +2203,7 @@ class PostTest < ActiveSupport::TestCase disapproved = FactoryBot.create(:post, is_pending: true) create(:post_flag, post: flagged, creator: create(:user, created_at: 2.weeks.ago)) - FactoryBot.create(:post_disapproval, post: disapproved, reason: "disinterest") + create(:post_disapproval, user: CurrentUser.user, post: disapproved, reason: "disinterest") assert_tag_match([pending, flagged], "status:unmoderated") end @@ -2380,7 +2382,7 @@ class PostTest < ActiveSupport::TestCase CurrentUser.scoped(FactoryBot.create(:mod_user)) do pending = FactoryBot.create(:post, is_pending: true) disapproved = FactoryBot.create(:post, is_pending: true) - disapproval = FactoryBot.create(:post_disapproval, post: disapproved, reason: "disinterest") + disapproval = FactoryBot.create(:post_disapproval, user: CurrentUser.user, post: disapproved, reason: "disinterest") assert_tag_match([pending], "disapproval:none") assert_tag_match([disapproved], "disapproval:any") @@ -2412,7 +2414,7 @@ class PostTest < ActiveSupport::TestCase u = create(:user, created_at: 2.weeks.ago) create(:artist_commentary, post: p) create(:comment, post: p, creator: u, do_not_bump_post: false) - create(:note, post: p, creator: u) + create(:note, post: p) p end