From 07a4bdcb21af33ab3b2cfa0c138ef42db965c8c7 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 25 Nov 2017 12:42:09 -0600 Subject: [PATCH 01/23] posts: display validation warnings in flash notice. --- app/controllers/posts_controller.rb | 4 ++++ app/controllers/uploads_controller.rb | 7 ++++++- app/models/application_record.rb | 4 ++++ app/models/upload.rb | 6 +++++- app/views/layouts/default.html.erb | 2 +- 5 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index eac3b30c2..b16f1fcd8 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -110,6 +110,10 @@ private def respond_with_post_after_update(post) respond_with(post) do |format| format.html do + if post.warnings.any? + flash[:notice] = post.warnings.full_messages.join(".\n \n") + end + if post.errors.any? @error_message = post.errors.full_messages.join("; ") render :template => "static/error", :status => 500 diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index fa2655d9f..6c4466f57 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -53,7 +53,12 @@ class UploadsController < ApplicationController def create @upload = Upload.create(params[:upload].merge(:server => Socket.gethostname)) - @upload.process! if @upload.errors.empty? + + if @upload.errors.empty? + post = @upload.process! + flash[:notice] = post.warnings.full_messages.join(".\n \n") if post.present? && post.warnings.any? + end + save_recent_tags respond_with(@upload) end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index d0bdfc17e..45af77b23 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -43,5 +43,9 @@ class ApplicationRecord < ActiveRecord::Base end end + def warnings + @warnings ||= ActiveModel::Errors.new(self) + end + include ApiMethods end diff --git a/app/models/upload.rb b/app/models/upload.rb index 4d0704d24..0115f4c46 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -148,6 +148,8 @@ class Upload < ApplicationRecord else update_attribute(:status, "error: " + post.errors.full_messages.join(", ")) end + + post end def process!(force = false) @@ -155,7 +157,7 @@ class Upload < ApplicationRecord return if !force && status =~ /processing|completed|error/ process_upload - create_post_from_upload + post = create_post_from_upload rescue Timeout::Error, Net::HTTP::Persistent::Error => x if @tries > 3 @@ -164,9 +166,11 @@ class Upload < ApplicationRecord @tries += 1 retry end + nil rescue Exception => x update_attributes(:status => "error: #{x.class} - #{x.message}", :backtrace => x.backtrace.join("\n")) + nil ensure delete_temp_file diff --git a/app/views/layouts/default.html.erb b/app/views/layouts/default.html.erb index 968be757e..3da3aed44 100644 --- a/app/views/layouts/default.html.erb +++ b/app/views/layouts/default.html.erb @@ -112,7 +112,7 @@ <% end %>
"> - <%= flash[:notice] %> + <%= format_text(flash[:notice], inline: true) %>. close
From 963eacd849692ff4d81196e4a12717dc1e0e8293 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 8 Nov 2017 19:33:02 -0600 Subject: [PATCH 02/23] posts: warn when adding newly created tags. --- app/models/post.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/app/models/post.rb b/app/models/post.rb index 1a25add2e..ebeee8274 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -18,6 +18,7 @@ class Post < ApplicationRecord validates_uniqueness_of :md5, :on => :create validates_inclusion_of :rating, in: %w(s q e), message: "rating must be s, q, or e" validate :tag_names_are_valid + validate :added_tags_are_valid validate :post_is_not_its_own_parent validate :updater_can_change_rating before_save :update_tag_post_counts @@ -592,6 +593,18 @@ class Post < ApplicationRecord @tag_array_was ||= Tag.scan_tags(tag_string_was) end + def tags + Tag.where(name: tag_array) + end + + def tags_was + Tag.where(name: tag_array_was) + end + + def added_tags + tags - tags_was + end + def decrement_tag_post_counts Tag.where(:name => tag_array).update_all("post_count = post_count - 1") if tag_array.any? end @@ -1707,6 +1720,17 @@ class Post < ApplicationRecord end end end + + def added_tags_are_valid + new_tags = added_tags.select { |t| t.post_count <= 1 } + new_general_tags = new_tags.select { |t| t.category == Tag.categories.general } + + if new_general_tags.present? + n = new_general_tags.size + tag_wiki_links = new_general_tags.map { |tag| "[[#{tag.name}]]" } + self.warnings[:base] << "Created #{n} new #{n == 1 ? "tag" : "tags"}: #{tag_wiki_links.join(", ")}" + end + end end include FileMethods From d571efd7030b2fdd638da871c96e4ff2e938d6dc Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 8 Nov 2017 19:37:09 -0600 Subject: [PATCH 03/23] posts: warn when adding artist tag with no artist entry. --- app/models/post.rb | 7 +++++++ app/models/tag.rb | 1 + 2 files changed, 8 insertions(+) diff --git a/app/models/post.rb b/app/models/post.rb index ebeee8274..8795beed2 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1724,12 +1724,19 @@ class Post < ApplicationRecord def added_tags_are_valid new_tags = added_tags.select { |t| t.post_count <= 1 } new_general_tags = new_tags.select { |t| t.category == Tag.categories.general } + new_artist_tags = new_tags.select { |t| t.category == Tag.categories.artist } if new_general_tags.present? n = new_general_tags.size tag_wiki_links = new_general_tags.map { |tag| "[[#{tag.name}]]" } self.warnings[:base] << "Created #{n} new #{n == 1 ? "tag" : "tags"}: #{tag_wiki_links.join(", ")}" end + + new_artist_tags.each do |tag| + if tag.artist.blank? + self.warnings[:base] << "Artist [[#{tag.name}]] requires an artist entry. \"Create new artist entry\":[/artists/new?name=#{CGI::escape(tag.name)}]" + end + end end end diff --git a/app/models/tag.rb b/app/models/tag.rb index 5f3a3fdc2..1cffddc6a 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -6,6 +6,7 @@ class Tag < ApplicationRecord attr_accessible :category, :as => [:moderator, :gold, :platinum, :member, :anonymous, :default, :builder, :admin] attr_accessible :is_locked, :as => [:moderator, :admin] has_one :wiki_page, :foreign_key => "title", :primary_key => "name" + has_one :artist, :foreign_key => "name", :primary_key => "name" has_one :antecedent_alias, lambda {active}, :class_name => "TagAlias", :foreign_key => "antecedent_name", :primary_key => "name" has_many :consequent_aliases, lambda {active}, :class_name => "TagAlias", :foreign_key => "consequent_name", :primary_key => "name" has_many :antecedent_implications, lambda {active}, :class_name => "TagImplication", :foreign_key => "antecedent_name", :primary_key => "name" From 4cd372296d7bc713f24dda867dda85f5af731c3a Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 25 Nov 2017 13:45:17 -0600 Subject: [PATCH 04/23] posts: warn when post is missing copyright tags. --- app/models/post.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/models/post.rb b/app/models/post.rb index 8795beed2..ec929b874 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -19,6 +19,7 @@ class Post < ApplicationRecord validates_inclusion_of :rating, in: %w(s q e), message: "rating must be s, q, or e" validate :tag_names_are_valid validate :added_tags_are_valid + validate :has_copyright_tag validate :post_is_not_its_own_parent validate :updater_can_change_rating before_save :update_tag_post_counts @@ -1738,6 +1739,13 @@ class Post < ApplicationRecord end end end + + def has_copyright_tag + return if !new_record? + return if has_tag?("copyright_request") || tags.any? { |t| t.category == Tag.categories.copyright } + + self.warnings[:base] << "Copyright tag is required. Consider adding [[copyright request]] or [[original]]" + end end include FileMethods From 94fa83573360b4c525b3e5331f3877d5215c58ea Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 25 Nov 2017 14:44:22 -0600 Subject: [PATCH 05/23] posts: warn when post from known source is missing an artist tag. --- app/models/post.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/app/models/post.rb b/app/models/post.rb index ec929b874..cf0ef3511 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -19,6 +19,7 @@ class Post < ApplicationRecord validates_inclusion_of :rating, in: %w(s q e), message: "rating must be s, q, or e" validate :tag_names_are_valid validate :added_tags_are_valid + validate :has_artist_tag validate :has_copyright_tag validate :post_is_not_its_own_parent validate :updater_can_change_rating @@ -1740,6 +1741,17 @@ class Post < ApplicationRecord end end + def has_artist_tag + return if !new_record? + return if source !~ %r!\Ahttps?://! + return if has_tag?("artist_request") || tags.any? { |t| t.category == Tag.categories.artist } + + site = Sources::Site.new(source) + self.warnings[:base] << "Artist tag is required. Create a new tag with [[artist:]]. Ask on the forum if you need naming help" + rescue Sources::Site::NoStrategyError => e + # unrecognized source; do nothing. + end + def has_copyright_tag return if !new_record? return if has_tag?("copyright_request") || tags.any? { |t| t.category == Tag.categories.copyright } From cc1f8ab9ed58d145dd9ac5f78e9f33fd7cf384ad Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 25 Nov 2017 17:02:16 -0600 Subject: [PATCH 06/23] posts: warn when a tag cannot be removed due to implications / automatic tags. --- app/models/post.rb | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index cf0ef3511..3d7e56f75 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -19,6 +19,7 @@ class Post < ApplicationRecord validates_inclusion_of :rating, in: %w(s q e), message: "rating must be s, q, or e" validate :tag_names_are_valid validate :added_tags_are_valid + validate :removed_tags_are_valid validate :has_artist_tag validate :has_copyright_tag validate :post_is_not_its_own_parent @@ -648,12 +649,18 @@ class Post < ApplicationRecord end def merge_old_changes + @removed_tags = [] + if old_tag_string # If someone else committed changes to this post before we did, # then try to merge the tag changes together. current_tags = tag_array_was() new_tags = tag_array() old_tags = Tag.scan_tags(old_tag_string) + + kept_tags = current_tags & new_tags + @removed_tags = old_tags - kept_tags + set_tag_string(((current_tags + new_tags) - old_tags + (current_tags & new_tags)).uniq.sort.join(" ")) end @@ -702,10 +709,10 @@ class Post < ApplicationRecord end def remove_negated_tags(tags) - negated_tags, tags = tags.partition {|x| x =~ /\A-/i} - negated_tags = negated_tags.map {|x| x[1..-1]} - negated_tags = TagAlias.to_aliased(negated_tags) - return tags - negated_tags + @negated_tags, tags = tags.partition {|x| x =~ /\A-/i} + @negated_tags = @negated_tags.map {|x| x[1..-1]} + @negated_tags = TagAlias.to_aliased(@negated_tags) + return tags - @negated_tags end def add_automatic_tags(tags) @@ -1741,6 +1748,16 @@ class Post < ApplicationRecord end end + def removed_tags_are_valid + attempted_removed_tags = @removed_tags + @negated_tags + unremoved_tags = tag_array & attempted_removed_tags + + if unremoved_tags.present? + unremoved_tags_list = unremoved_tags.map { |t| "[[#{t}]]" }.to_sentence + self.warnings[:base] << "#{unremoved_tags_list} could not be removed. Check for implications and try again" + end + end + def has_artist_tag return if !new_record? return if source !~ %r!\Ahttps?://! From bc3e2438d91940383c9a8c87bac46948b31d900f Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 25 Nov 2017 17:02:58 -0600 Subject: [PATCH 07/23] posts: add tests for warning validations. --- test/unit/post_test.rb | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 42deb0b7f..bd58894a0 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1546,6 +1546,42 @@ class PostTest < ActiveSupport::TestCase assert_equal("http://www.hentai-foundry.com/pictures/user/AnimeFlux/219123", @post.normalized_source) end end + + context "when validating tags" do + should "warn when creating a new general tag" do + @post.add_tag("tag") + @post.save + + assert_match(/Created 1 new tag: \[\[tag\]\]/, @post.warnings.full_messages.join) + end + + should "warn when adding an artist tag without an artist entry" do + @post.add_tag("artist:bkub") + @post.save + + assert_match(/Artist \[\[bkub\]\] requires an artist entry./, @post.warnings.full_messages.join) + end + + should "warn when a tag removal failed due to implications or automatic tags" do + ti = FactoryGirl.create(:tag_implication, antecedent_name: "cat", consequent_name: "animal") + @post.reload + @post.update(old_tag_string: @post.tag_string, tag_string: "chen_(cosplay) chen cosplay cat animal") + @post.reload + @post.update(old_tag_string: @post.tag_string, tag_string: "chen_(cosplay) chen cosplay cat -cosplay") + + assert_match(/\[\[animal\]\] and \[\[cosplay\]\] could not be removed./, @post.warnings.full_messages.join) + end + + should "warn when a post from a known source is missing an artist tag" do + post = FactoryGirl.build(:post, source: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=65985331") + post.save + assert_match(/Artist tag is required/, post.warnings.full_messages.join) + end + + should "warn when missing a copyright tag" do + assert_match(/Copyright tag is required/, @post.warnings.full_messages.join) + end + end end end From 127e4e385b8a596463af0a3440cbcf603f97ddd2 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 25 Nov 2017 21:26:59 -0600 Subject: [PATCH 08/23] Fix dmail failures when akismet isn't configured. --- app/models/dmail.rb | 13 +++++++------ config/application.rb | 5 ----- config/danbooru_default_config.rb | 7 +++++++ config/initializers/rakismet.rb | 2 ++ 4 files changed, 16 insertions(+), 11 deletions(-) create mode 100644 config/initializers/rakismet.rb diff --git a/app/models/dmail.rb b/app/models/dmail.rb index 999fead09..4e861d5d1 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -30,6 +30,12 @@ class Dmail < ApplicationRecord def creator_ip_addr_str creator_ip_addr.to_s end + + def spam?(sender = CurrentUser.user) + return false if Danbooru.config.rakismet_key.blank? + return false if sender.is_gold? + super() + end end module AddressMethods @@ -52,12 +58,7 @@ class Dmail < ApplicationRecord def initialize_attributes self.from_id ||= CurrentUser.id self.creator_ip_addr ||= CurrentUser.ip_addr - if CurrentUser.is_gold? - self.is_spam = false - else - self.is_spam = spam? - end - true + self.is_spam = spam?(CurrentUser.user) end end diff --git a/config/application.rb b/config/application.rb index 4fa9af05c..b5627814a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -30,11 +30,6 @@ module Danbooru config.x.git_hash = nil end - if ENV["DANBOORU_RAKISMET_KEY"] - config.rakismet.key = ENV["DANBOORU_RAKISMET_KEY"] - config.rakismet.url = ENV["DANBOORU_RAKISMET_URL"] - end - config.after_initialize do Rails.application.routes.default_url_options = { host: Danbooru.config.hostname, diff --git a/config/danbooru_default_config.rb b/config/danbooru_default_config.rb index dd2e88dec..8e297a5f0 100644 --- a/config/danbooru_default_config.rb +++ b/config/danbooru_default_config.rb @@ -619,6 +619,13 @@ module Danbooru def aws_sqs_cropper_url end + + # Akismet API key. Used for Dmail spam detection. http://akismet.com/signup/ + def rakismet_key + end + + def rakismet_url + end end class EnvironmentConfiguration diff --git a/config/initializers/rakismet.rb b/config/initializers/rakismet.rb new file mode 100644 index 000000000..c521fc0b6 --- /dev/null +++ b/config/initializers/rakismet.rb @@ -0,0 +1,2 @@ +Rails.application.config.rakismet.key = Danbooru.config.rakismet_key +Rails.application.config.rakismet.url = Danbooru.config.rakismet_url From 810b6b8b9950ac9cc53cc37009e8f15d3b686297 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 25 Nov 2017 21:52:32 -0600 Subject: [PATCH 09/23] Fix #3039: Test failures under ruby 2.4. --- Gemfile.lock | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 663085b7a..65fa9f436 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -64,8 +64,8 @@ GEM minitest (~> 5.1) thread_safe (~> 0.3, >= 0.3.4) tzinfo (~> 1.1) - addressable (2.5.1) - public_suffix (~> 2.0, >= 2.0.2) + addressable (2.5.2) + public_suffix (>= 2.0.2, < 4.0) arel (6.0.4) awesome_print (1.7.0) aws-sdk (2.7.4) @@ -108,7 +108,7 @@ GEM cityhash (0.8.1) coderay (1.1.1) colorize (0.7.7) - crack (0.4.2) + crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.2) daemons (1.2.3) @@ -156,6 +156,7 @@ GEM multi_json (~> 1.11) os (~> 0.9) signet (~> 0.7) + hashdiff (0.3.7) highline (1.7.8) hike (1.2.3) http (2.2.2) @@ -246,7 +247,7 @@ GEM pry-byebug (3.4.2) byebug (~> 9.0) pry (~> 0.10) - public_suffix (2.0.5) + public_suffix (3.0.1) rack (1.6.5) rack-test (0.6.3) rack (>= 1.0) @@ -385,9 +386,10 @@ GEM unicorn-worker-killer (0.4.4) get_process_mem (~> 0) unicorn (>= 4, < 6) - webmock (1.21.0) + webmock (3.1.1) addressable (>= 2.3.6) crack (>= 0.3.2) + hashdiff webrobots (0.1.2) whenever (0.9.7) chronic (>= 0.6.3) From 80e115b600a4d90a9a40e3b50a88ecc8bca02f11 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 26 Nov 2017 10:31:28 -0600 Subject: [PATCH 10/23] favgroups: fix race condition when adding posts to favgroups. Adding or removing a post id to a favgroup's post_ids string is non-atomic. Lock it to prevent simultaneous updates to the same favgroup from clobbering each other. Same bug as #3091. --- app/models/favorite_group.rb | 20 ++++++++++++-------- test/unit/post_test.rb | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/app/models/favorite_group.rb b/app/models/favorite_group.rb index f6047e9e2..2d87845f3 100644 --- a/app/models/favorite_group.rb +++ b/app/models/favorite_group.rb @@ -173,11 +173,13 @@ class FavoriteGroup < ApplicationRecord end def add!(post_id) - post_id = post_id.id if post_id.is_a?(Post) - return if contains?(post_id) + with_lock do + post_id = post_id.id if post_id.is_a?(Post) + return if contains?(post_id) - clear_post_id_array - update_attributes(:post_ids => add_number_to_string(post_id, post_ids)) + clear_post_id_array + update_attributes(:post_ids => add_number_to_string(post_id, post_ids)) + end end def self.purge_post(post_id) @@ -188,11 +190,13 @@ class FavoriteGroup < ApplicationRecord end def remove!(post_id) - post_id = post_id.id if post_id.is_a?(Post) - return unless contains?(post_id) + with_lock do + post_id = post_id.id if post_id.is_a?(Post) + return unless contains?(post_id) - clear_post_id_array - update_attributes(:post_ids => remove_number_from_string(post_id, post_ids)) + clear_post_id_array + update_attributes(:post_ids => remove_number_from_string(post_id, post_ids)) + end end def add_number_to_string(number, string) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 42deb0b7f..15ca6f194 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -813,6 +813,25 @@ class PostTest < ActiveSupport::TestCase end end + context "for a favgroup" do + setup do + @favgroup = FactoryGirl.create(:favorite_group, creator: @user) + @post = FactoryGirl.create(:post, :tag_string => "aaa favgroup:#{@favgroup.id}") + end + + should "add the post to the favgroup" do + assert_equal(1, @favgroup.reload.post_count) + assert_equal(true, !!@favgroup.contains?(@post.id)) + end + + should "remove the post from the favgroup" do + @post.update(:tag_string => "-favgroup:#{@favgroup.id}") + + assert_equal(0, @favgroup.reload.post_count) + assert_equal(false, !!@favgroup.contains?(@post.id)) + end + end + context "for a pool" do setup do mock_pool_archive_service! From 6d9708a22e310d57f35d8c44a71307addce73533 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 26 Nov 2017 11:42:00 -0600 Subject: [PATCH 11/23] posts: don't warn about missing artist tags for official_art. --- app/models/post.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/post.rb b/app/models/post.rb index 3d7e56f75..ff8469d90 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1761,7 +1761,8 @@ class Post < ApplicationRecord def has_artist_tag return if !new_record? return if source !~ %r!\Ahttps?://! - return if has_tag?("artist_request") || tags.any? { |t| t.category == Tag.categories.artist } + return if has_tag?("artist_request") || has_tag?("official_art") + return if tags.any? { |t| t.category == Tag.categories.artist } site = Sources::Site.new(source) self.warnings[:base] << "Artist tag is required. Create a new tag with [[artist:]]. Ask on the forum if you need naming help" From 255082d3b55ac726353b06f22a2d572929bfc491 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 26 Nov 2017 15:37:51 -0600 Subject: [PATCH 12/23] tumblr: fix test failure. --- test/unit/sources/tumblr_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/sources/tumblr_test.rb b/test/unit/sources/tumblr_test.rb index d47f7bcba..b1d811d5b 100644 --- a/test/unit/sources/tumblr_test.rb +++ b/test/unit/sources/tumblr_test.rb @@ -152,7 +152,7 @@ module Sources should "get the image urls" do urls = %w[ - https://vt.media.tumblr.com/tumblr_os31dkexhK1wsfqep.mp4 + https://vtt.tumblr.com/tumblr_os31dkexhK1wsfqep.mp4 http://data.tumblr.com/afed9f5b3c33c39dc8c967e262955de2/tumblr_inline_os31dclyCR1v11u29_raw.png ] From a7566ae85115cf3c11c67e6114a972480cb62917 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 26 Nov 2017 16:17:21 -0600 Subject: [PATCH 13/23] post_view_count_service_test.rb: fix test failure. --- test/unit/post_view_count_service_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/post_view_count_service_test.rb b/test/unit/post_view_count_service_test.rb index 3099bf005..a20e8d5e0 100644 --- a/test/unit/post_view_count_service_test.rb +++ b/test/unit/post_view_count_service_test.rb @@ -49,6 +49,7 @@ class PostViewCountServiceTest < ActiveSupport::TestCase context "failure" do setup do + @date = "2000-01-01" stub_request(:get, "localhost:1234/post_views/rank").with(query: {"date" => @date}).to_return(body: "", status: 400) end From 4939c0345a0a2633b98fdaeb67711f25ba40a8ef Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 26 Nov 2017 18:10:08 -0600 Subject: [PATCH 14/23] Fix test failures when removing posts from deleted pools. These tests failed because removing posts from deleted pools is now Builder-only. --- test/unit/pool_test.rb | 3 +++ test/unit/post_test.rb | 3 +++ 2 files changed, 6 insertions(+) diff --git a/test/unit/pool_test.rb b/test/unit/pool_test.rb index fc4f3ec3f..3362dadf3 100644 --- a/test/unit/pool_test.rb +++ b/test/unit/pool_test.rb @@ -149,6 +149,9 @@ class PoolTest < ActiveSupport::TestCase context "to a deleted pool" do setup do + # must be a builder to update deleted pools. + CurrentUser.user = FactoryGirl.create(:builder_user) + @pool.update_attribute(:is_deleted, true) @pool.post_ids = "#{@pool.post_ids} #{@p2.id}" @pool.synchronize! diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 42deb0b7f..026ad97b5 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -111,6 +111,9 @@ class PostTest < ActiveSupport::TestCase context "that belongs to a pool" do setup do + # must be a builder to update deleted pools. must be >1 week old to remove posts from pools. + CurrentUser.user = FactoryGirl.create(:builder_user, created_at: 1.month.ago) + SqsService.any_instance.stubs(:send_message) @pool = FactoryGirl.create(:pool) @pool.add!(@post) From 3c6a613964dcbe5e16281e2fc4d4ebbecd416def Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 29 Nov 2017 12:26:32 -0600 Subject: [PATCH 15/23] Fix #3410: Unable to create a new wiki page. Fix `Post.fast_count(nil)` failing when the user had the "safe mode" or "deleted post filter" options turned on. --- app/models/post.rb | 1 + test/unit/post_test.rb | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 1a25add2e..215516afb 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1136,6 +1136,7 @@ class Post < ApplicationRecord module CountMethods def fast_count(tags = "", options = {}) + tags = tags.to_s tags += " rating:s" if CurrentUser.safe_mode? tags += " -status:deleted" if CurrentUser.hide_deleted_posts? && tags !~ /(?:^|\s)(?:-)?status:.+/ tags = Tag.normalize_query(tags) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 026ad97b5..2ce394014 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -2461,13 +2461,17 @@ class PostTest < ActiveSupport::TestCase context "in safe mode" do setup do CurrentUser.stubs(:safe_mode?).returns(true) + FactoryGirl.create(:post, "rating" => "s") end - should "execute a search" do - Post.expects(:fast_count_search).once.with("rating:s", kind_of(Hash)).returns(1) + should "work for a blank search" do assert_equal(1, Post.fast_count("")) end + should "work for a nil search" do + assert_equal(1, Post.fast_count(nil)) + end + should "not fail for a two tag search by a member" do post1 = FactoryGirl.create(:post, tag_string: "aaa bbb rating:s") post2 = FactoryGirl.create(:post, tag_string: "aaa bbb rating:e") From 649969156ec9d6204b76f068c3fe8e4228fe0287 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 29 Nov 2017 12:56:58 -0600 Subject: [PATCH 16/23] /wiki_pages: don't show post count in navbar when creating new wikis. --- .../wiki_pages/_secondary_links.html.erb | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/app/views/wiki_pages/_secondary_links.html.erb b/app/views/wiki_pages/_secondary_links.html.erb index ee56fab2c..ccb04ecf0 100644 --- a/app/views/wiki_pages/_secondary_links.html.erb +++ b/app/views/wiki_pages/_secondary_links.html.erb @@ -10,20 +10,17 @@
  • <%= link_to "Help", wiki_pages_path(:search => {:title => "help:wiki"}) %>
    • - <% if @wiki_page %> + <% if @wiki_page && !@wiki_page.new_record? %>
  • |
  • <%= link_to "Posts (#{Post.fast_count(@wiki_page.title)})", posts_path(:tags => @wiki_page.title) %>
    • - <% unless @wiki_page.new_record? %> -
  • <%= link_to "History", wiki_page_versions_path(:search => {:wiki_page_id => @wiki_page.id}) %>
    • - <% if CurrentUser.is_member? %> -
  • <%= link_to "Edit", edit_wiki_page_path(@wiki_page) %>
    • - <% end %> - <% if CurrentUser.is_builder? && !@wiki_page.is_deleted? %> -
  • <%= link_to "Delete", wiki_page_path(@wiki_page), :remote => true, :method => :delete, :data => {:confirm => "Are you sure you want to delete this wiki page?"} %>
    • - <% end %> +
  • <%= link_to "History", wiki_page_versions_path(:search => {:wiki_page_id => @wiki_page.id}) %>
    • + <% if CurrentUser.is_member? %> +
  • <%= link_to "Edit", edit_wiki_page_path(@wiki_page) %>
    • <% end %> - <% end %> - <% if @wiki_page_version %> + <% if CurrentUser.is_builder? && !@wiki_page.is_deleted? %> +
  • <%= link_to "Delete", wiki_page_path(@wiki_page), :remote => true, :method => :delete, :data => {:confirm => "Are you sure you want to delete this wiki page?"} %>
    • + <% end %> + <% elsif @wiki_page_version %>
  • |
  • <%= link_to "Newest", wiki_page_path(@wiki_page_version.wiki_page_id) %>
    • <% if CurrentUser.is_member? %> From 200071922761b5c0fbe0604b3447a3857f10f014 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 29 Nov 2017 13:12:53 -0600 Subject: [PATCH 17/23] /wiki_pages: get navbar post count from tags table. Post.fast_count is dependent on the current user's settings. This meant that tag counts in wiki pages could be different from the tag counts displayed in tag lists. --- app/views/wiki_pages/_secondary_links.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/wiki_pages/_secondary_links.html.erb b/app/views/wiki_pages/_secondary_links.html.erb index ccb04ecf0..17f9a5758 100644 --- a/app/views/wiki_pages/_secondary_links.html.erb +++ b/app/views/wiki_pages/_secondary_links.html.erb @@ -12,7 +12,7 @@ <% if @wiki_page && !@wiki_page.new_record? %>
  • |
    • -
  • <%= link_to "Posts (#{Post.fast_count(@wiki_page.title)})", posts_path(:tags => @wiki_page.title) %>
    • +
  • <%= link_to "Posts (#{@wiki_page.tag.try(:post_count) || 0})", posts_path(:tags => @wiki_page.title) %>
  • <%= link_to "History", wiki_page_versions_path(:search => {:wiki_page_id => @wiki_page.id}) %>
    • <% if CurrentUser.is_member? %>
  • <%= link_to "Edit", edit_wiki_page_path(@wiki_page) %>
    • From feb3ec07500586e0e9d3dc5594414f3b59766349 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 2 Dec 2017 19:10:38 -0600 Subject: [PATCH 18/23] Fix #3417: Deleting a user's comment credits the change to them. --- app/models/comment.rb | 4 ++-- test/unit/comment_test.rb | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index be656fc88..3ca0a40bb 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -182,8 +182,8 @@ class Comment < ApplicationRecord end def initialize_updater - self.updater_id ||= CurrentUser.user.id - self.updater_ip_addr ||= CurrentUser.ip_addr + self.updater_id = CurrentUser.user.id + self.updater_ip_addr = CurrentUser.ip_addr end def creator_name diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb index fa8df48d7..e14da6afc 100644 --- a/test/unit/comment_test.rb +++ b/test/unit/comment_test.rb @@ -244,6 +244,11 @@ class CommentTest < ActiveSupport::TestCase @comment.update_attributes({:body => "nope"}, :as => :moderator) end end + + should "credit the moderator as the updater" do + @comment.update({ body: "test" }, as: :moderator) + assert_equal(@mod.id, @comment.updater_id) + end end context "that is below the score threshold" do From acd49be4ccac2e20b6ecc053ab35ee5bbcb7ae5b Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 5 Dec 2017 18:53:13 -0600 Subject: [PATCH 19/23] Fix #3419: Deleting a post doesn't clear parent's "parent" status. Bug: when deleting a child post and the "Move favorites to parent?" option is set, the parent's has_active_children flag is not cleared. `give_favorites_to_parent` moves the votes, and moving the votes has the side effect of reloading the post (to get the new score). But reloading the post wipes out the is_deleted_changed? flag, which is used by `update_parent_on_save`. Fix: update the `is_deleted` flag *before* moving favorites, so that the `update_parent_on_save` callback runs before `give_favorite_to_parent` runs. --- app/models/post.rb | 19 ++++++++----------- test/unit/post_test.rb | 11 +++++++++++ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 215516afb..2f6bc6653 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1369,18 +1369,15 @@ class Post < ApplicationRecord Post.transaction do flag!(reason, is_deletion: true) - self.is_deleted = true - self.is_pending = false - self.is_flagged = false - self.is_banned = true if options[:ban] || has_tag?("banned_artist") - update_columns( - :is_deleted => is_deleted, - :is_pending => is_pending, - :is_flagged => is_flagged, - :is_banned => is_banned - ) + update({ + is_deleted: true, + is_pending: false, + is_flagged: false, + is_banned: is_banned || options[:ban] || has_tag?("banned_artist") + }, without_protection: true) + + # XXX This must happen *after* the `is_deleted` flag is set to true (issue #3419). give_favorites_to_parent if options[:move_favorites] - update_parent_on_save unless options[:without_mod_action] ModAction.log("deleted post ##{id}, reason: #{reason}") diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 2ce394014..a076af4e0 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -353,6 +353,17 @@ class PostTest < ActiveSupport::TestCase p1.reload assert(p1.has_children?, "Parent should have children") end + + should "clear the has_active_children flag when the 'move favorites' option is set" do + user = FactoryGirl.create(:gold_user) + p1 = FactoryGirl.create(:post) + c1 = FactoryGirl.create(:post, :parent_id => p1.id) + c1.add_favorite!(user) + + assert_equal(true, p1.reload.has_active_children?) + c1.delete!("test", :move_favorites => true) + assert_equal(false, p1.reload.has_active_children?) + end end context "one child" do From 839f0f653f57a12b00c7ed73be55ace0b2ce83a0 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Thu, 23 Nov 2017 13:07:01 -0800 Subject: [PATCH 20/23] Changed safe mode error message for Gold+ users - Made explicit the error messages and their order - Banned takes priority, then Gold+, then Safe - Made the groups exclusive of each other --- app/logical/post_sets/post.rb | 8 ++++++-- app/models/post.rb | 19 +++++++++++++++---- app/presenters/post_presenter.rb | 15 ++++++++++++--- .../posts/partials/index/_posts.html.erb | 8 ++++++-- 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index c91f31c7d..80bd19450 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -87,11 +87,15 @@ module PostSets end def banned_posts - posts.select(&:is_banned?) + posts.select { |p| p.banblocked? } end def censored_posts - hidden_posts - banned_posts + posts.select { |p| p.levelblocked? && !p.banblocked? } + end + + def safe_posts + posts.select { |p| p.safeblocked? && !p.levelblocked? && !p.banblocked? } end def use_sequential_paginator? diff --git a/app/models/post.rb b/app/models/post.rb index bf0f2a406..1e1b3c1b0 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1761,11 +1761,22 @@ class Post < ApplicationRecord ) has_bit_flags BOOLEAN_ATTRIBUTES + def safeblocked? + CurrentUser.safe_mode? && (rating != "s" || has_tag?("toddlercon|toddler|diaper|tentacle|rape|bestiality|beastiality|lolita|loli|nude|shota|pussy|penis")) + end + + def levelblocked? + !Danbooru.config.can_user_see_post?(CurrentUser.user, self) + end + + def banblocked? + is_banned? && !CurrentUser.is_gold? + end + def visible? - return false if !Danbooru.config.can_user_see_post?(CurrentUser.user, self) - return false if CurrentUser.safe_mode? && rating != "s" - return false if CurrentUser.safe_mode? && has_tag?("toddlercon|toddler|diaper|tentacle|rape|bestiality|beastiality|lolita|loli|nude|shota|pussy|penis") - return false if is_banned? && !CurrentUser.is_gold? + return false if safeblocked? + return false if levelblocked? + return false if banblocked? return true end diff --git a/app/presenters/post_presenter.rb b/app/presenters/post_presenter.rb index d9ca80ac1..eb9849ecf 100644 --- a/app/presenters/post_presenter.rb +++ b/app/presenters/post_presenter.rb @@ -149,10 +149,19 @@ class PostPresenter < Presenter categorized_tag_groups.flatten.slice(0, 25).join(", ").tr("_", " ") end + def safe_mode_message(template) + html = ["This image is unavailable on safe mode (#{Danbooru.config.app_name}). Go to "] + html << template.link_to("Danbooru", "http://danbooru.donmai.us") + html << " or disable safe mode to view (" + html << template.link_to("learn more", template.wiki_pages_path(title: "help:user_settings")) + html << ")." + html.join.html_safe + end + def image_html(template) - return template.content_tag("p", "The artist requested removal of this image") if @post.is_banned? && !CurrentUser.user.is_gold? - return template.content_tag("p", template.link_to("You need a gold account to see this image.", template.new_user_upgrade_path)) if !Danbooru.config.can_user_see_post?(CurrentUser.user, @post) - return template.content_tag("p", "This image is unavailable") if !@post.visible? + return template.content_tag("p", "The artist requested removal of this image") if @post.banblocked? + return template.content_tag("p", template.link_to("You need a gold account to see this image.", template.new_user_upgrade_path)) if @post.levelblocked? + return template.content_tag("p", safe_mode_message(template)) if @post.safeblocked? if @post.is_flash? template.render("posts/partials/show/flash", :post => @post) diff --git a/app/views/posts/partials/index/_posts.html.erb b/app/views/posts/partials/index/_posts.html.erb index 9b831b80f..3322db011 100644 --- a/app/views/posts/partials/index/_posts.html.erb +++ b/app/views/posts/partials/index/_posts.html.erb @@ -6,11 +6,15 @@ <% if post_set.hidden_posts.present? %>
    <% if post_set.banned_posts.present? %> - <%= post_set.banned_posts.size %> post(s) were removed from this page at the artist's request (<%= link_to "learn more", wiki_pages_path(title: "banned_artist") %>). + <%= post_set.banned_posts.size %> post(s) were removed from this page at the artist's request (<%= link_to "learn more", wiki_pages_path(title: "banned_artist") %>).
    <% end %> <% if post_set.censored_posts.present? %> - <%= post_set.censored_posts.size %> post(s) on this page require a <%= link_to "Gold account", new_user_upgrade_path %> to view (<%= link_to "learn more", wiki_pages_path(title: "help:censored_tags") %>). + <%= post_set.censored_posts.size %> post(s) on this page require a <%= link_to "Gold account", new_user_upgrade_path %> to view (<%= link_to "learn more", wiki_pages_path(title: "help:censored_tags") %>).
    + <% end %> + + <% if post_set.safe_posts.present? %> + <%= post_set.safe_posts.size %> post(s) on this page were hidden by safe mode (<%= Danbooru.config.app_name %>). Go to <%= link_to "Danbooru", "http://danbooru.donmai.us" %> or disable safe mode to view (<%= link_to "learn more", wiki_pages_path(title: "help:user_settings") %>).
    <% end %>
    <% end %> From 131c0109d43940406e8e3ade375edb8629164309 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 6 Dec 2017 09:01:13 -0600 Subject: [PATCH 21/23] Address #3415: `og:image` meta tags can point to video files. --- app/models/post.rb | 8 ++++++++ app/views/posts/show.html.erb | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 2f6bc6653..d8f26e865 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -196,6 +196,14 @@ class Post < ApplicationRecord "http://#{Danbooru.config.hostname}#{preview_file_url}" end + def open_graph_image_url + if is_image? && has_large? + "http://#{Danbooru.config.hostname}#{large_file_url}" + else + complete_preview_file_url + end + end + def file_url_for(user) if CurrentUser.mobile_mode? large_file_url diff --git a/app/views/posts/show.html.erb b/app/views/posts/show.html.erb index 7fa0b6706..97bf19e60 100644 --- a/app/views/posts/show.html.erb +++ b/app/views/posts/show.html.erb @@ -174,7 +174,7 @@ <% if @post.visible? %> - + <% end %> <% if Danbooru.config.enable_post_search_counts %> @@ -189,7 +189,7 @@ <% if @post.visible? %> - + <% end %> <% end %> From d6d73404a9e4f2ed455d60f75900b7c00626f55b Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 6 Dec 2017 12:33:06 -0600 Subject: [PATCH 22/23] Apply aliases to characters in _(cosplay) tags (#3409). --- app/models/tag_implication.rb | 2 +- test/unit/post_test.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index 9e1b75cde..d50e7df18 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -21,7 +21,7 @@ class TagImplication < TagRelationship end def automatic_tags_for(names) - tags = names.grep(/\A(.+)_\(cosplay\)\Z/) { "char:#{$1}" } + tags = names.grep(/\A(.+)_\(cosplay\)\Z/) { "char:#{TagAlias.to_aliased([$1]).first}" } tags << "cosplay" if tags.present? tags.uniq end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index a076af4e0..ffc3849e1 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -790,6 +790,14 @@ class PostTest < ActiveSupport::TestCase assert(Tag.where(name: "someone_(cosplay)", category: 4).exists?, "expected 'someone_(cosplay)' tag to be created as character") assert(Tag.where(name: "someone", category: 4).exists?, "expected 'someone' tag to be created") end + + should "apply aliases when the character tag is added" do + FactoryGirl.create(:tag_alias, antecedent_name: "jim", consequent_name: "james") + @post.add_tag("jim_(cosplay)") + @post.save + + assert(@post.has_tag?("james"), "expected 'jim' to be aliased to 'james'") + end end context "for a parent" do From 5819afced76ab984a62f8a0aa07a16402cf698a0 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 6 Dec 2017 14:46:12 -0600 Subject: [PATCH 23/23] Fix #3412: Mass updates incorrectly move saved searches. --- app/logical/moderator/tag_batch_change.rb | 9 +++++---- test/unit/moderator/tag_batch_change_test.rb | 17 +++++++++++++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/logical/moderator/tag_batch_change.rb b/app/logical/moderator/tag_batch_change.rb index 230db6f01..395344373 100644 --- a/app/logical/moderator/tag_batch_change.rb +++ b/app/logical/moderator/tag_batch_change.rb @@ -18,11 +18,12 @@ module Moderator post.update_attributes(:tag_string => tags) end - tags = Tag.scan_tags(antecedent, :strip_metatags => true) - conds = tags.map {|x| "query like ?"}.join(" AND ") - conds = [conds, *tags.map {|x| "%#{x.to_escaped_for_sql_like}%"}] if SavedSearch.enabled? - SavedSearch.where(*conds).find_each do |ss| + tags = Tag.scan_tags(antecedent, :strip_metatags => true) + + # https://www.postgresql.org/docs/current/static/functions-array.html + saved_searches = SavedSearch.where("string_to_array(query, ' ') @> ARRAY[?]", tags) + saved_searches.find_each do |ss| ss.query = (ss.query.split - tags + [consequent]).uniq.join(" ") ss.save end diff --git a/test/unit/moderator/tag_batch_change_test.rb b/test/unit/moderator/tag_batch_change_test.rb index 9a4b88a0a..df03084b3 100644 --- a/test/unit/moderator/tag_batch_change_test.rb +++ b/test/unit/moderator/tag_batch_change_test.rb @@ -34,8 +34,21 @@ module Moderator ss = FactoryGirl.create(:saved_search, :user => @user, :query => "123 ... 456") tag_batch_change = TagBatchChange.new("...", "bbb", @user.id, "127.0.0.1") tag_batch_change.perform - ss.reload - assert_equal(%w(123 456 bbb), ss.query.scan(/\S+/).sort) + + assert_equal("123 456 bbb", ss.reload.normalized_query) + end + + should "move only saved searches that match the mass update exactly" do + ss = FactoryGirl.create(:saved_search, :user => @user, :query => "123 ... 456") + tag_batch_change = TagBatchChange.new("1", "bbb", @user.id, "127.0.0.1") + tag_batch_change.perform + + assert_equal("... 123 456", ss.reload.normalized_query, "expected '123' to remain unchanged") + + tag_batch_change = TagBatchChange.new("123 456", "789", @user.id, "127.0.0.1") + tag_batch_change.perform + + assert_equal("... 789", ss.reload.normalized_query, "expected '123 456' to be changed to '789'") end should "raise an error if there is no predicate" do