diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index c09c81b76..3af0df2d9 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -140,16 +140,15 @@ class PostsController < ApplicationController if post.errors.any? @message = post.errors.full_messages.join("; ") - render :template => "static/error", :status => 500 - else - response_params = {:q => params[:tags_query], :pool_id => params[:pool_id], post_set_id: params[:post_set_id]} - response_params.reject!{|key, value| value.blank?} - redirect_to post_path(post, response_params) + if flash[:notice].present? + flash[:notice] += "\n\n" + @message + else + flash[:notice] = @message + end end - end - - format.json do - render :json => post + response_params = { q: params[:tags_query], pool_id: params[:pool_id], post_set_id: params[:post_set_id] } + response_params.compact_blank! + redirect_to post_path(post, response_params) end end end diff --git a/app/javascript/src/javascripts/posts.js b/app/javascript/src/javascripts/posts.js index 8de86766a..cd0d48592 100644 --- a/app/javascript/src/javascripts/posts.js +++ b/app/javascript/src/javascripts/posts.js @@ -757,9 +757,12 @@ Post.update = function (post_id, params) { Post.notice_update("dec"); Post.update_data(data); }, - error: function () { + error: function (data) { Post.notice_update("dec"); - $(window).trigger("danbooru:error", "There was an error updating post #" + post_id + ""); + const message = $ + .map(data.responseJSON.errors, function (msg) { return msg; }) + .join("; "); + $(window).trigger("danbooru:error", `There was an error updating post #${post_id}: ${message}`); }, }); }); diff --git a/app/models/pool.rb b/app/models/pool.rb index be510f40c..0aa60bb24 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -194,8 +194,9 @@ class Pool < ApplicationRecord post_ids_before = post_ids_before_last_save || post_ids_was added = post_ids - post_ids_before return unless added.size > 0 - if post_ids.size > 1_000 - errors.add(:base, "Pools can have up to 1,000 posts each") + max = Danbooru.config.pool_post_limit(CurrentUser.user) + if post_ids.size > max + errors.add(:base, "Pools can only have up to #{ActiveSupport::NumberHelper.number_to_delimited(max)} posts each") false else true @@ -211,6 +212,7 @@ class Pool < ApplicationRecord reload self.skip_sync = true update(post_ids: post_ids + [post.id]) + raise(ActiveRecord::Rollback) unless valid? self.skip_sync = false post.add_pool!(self) post.save @@ -232,6 +234,7 @@ class Pool < ApplicationRecord reload self.skip_sync = true update(post_ids: post_ids - [post.id]) + raise(ActiveRecord::Rollback) unless valid? self.skip_sync = false post.remove_pool!(self) post.save diff --git a/app/models/post.rb b/app/models/post.rb index 10af8a718..436d37433 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -730,40 +730,76 @@ class Post < ApplicationRecord @post_metatags.each do |tag| case tag when /^-pool:(\d+)$/i - pool = Pool.find_by_id($1.to_i) - pool.remove!(self) if pool + pool = Pool.find_by(id: $1.to_i) + if pool + pool.remove!(self) + if pool.errors.any? + errors.add(:base, pool.errors.full_messages.join("; ")) + end + end when /^-pool:(.+)$/i - pool = Pool.find_by_name($1) - pool.remove!(self) if pool + pool = Pool.find_by(name: $1) + if pool + pool.remove!(self) + if pool.errors.any? + errors.add(:base, pool.errors.full_messages.join("; ")) + end + end when /^pool:(\d+)$/i - pool = Pool.find_by_id($1.to_i) - pool.add!(self) if pool + pool = Pool.find_by(id: $1.to_i) + if pool + pool.add!(self) + if pool.errors.any? + errors.add(:base, pool.errors.full_messages.join("; ")) + end + end - when /^pool:(.+)$/i - pool = Pool.find_by_name($1) - pool.add!(self) if pool - - when /^newpool:(.+)$/i - pool = Pool.find_by_name($1) - pool.add!(self) if pool + when /^(?:new)?pool:(.+)$/i + pool = Pool.find_by(name: $1) + if pool + pool.add!(self) + if pool.errors.any? + errors.add(:base, pool.errors.full_messages.join("; ")) + end + end when /^set:(\d+)$/i - set = PostSet.find_by_id($1.to_i) - set.add!(self) if set&.can_edit_posts?(CurrentUser.user) + set = PostSet.find_by(id: $1.to_i) + if set&.can_edit_posts?(CurrentUser.user) + set.add!(self) + if set.errors.any? + errors.add(:base, set.errors.full_messages.join("; ")) + end + end when /^-set:(\d+)$/i - set = PostSet.find_by_id($1.to_i) - set.remove!(self) if set&.can_edit_posts?(CurrentUser.user) + set = PostSet.find_by(id: $1.to_i) + if set&.can_edit_posts?(CurrentUser.user) + set.remove!(self) + if set.errors.any? + errors.add(:base, set.errors.full_messages.join("; ")) + end + end when /^set:(.+)$/i - set = PostSet.find_by_shortname($1) - set.add!(self) if set&.can_edit_posts?(CurrentUser.user) + set = PostSet.find_by(shortname: $1) + if set&.can_edit_posts?(CurrentUser.user) + set.add!(self) + if set.errors.any? + errors.add(:base, set.errors.full_messages.join("; ")) + end + end when /^-set:(.+)$/i - set = PostSet.find_by_shortname($1) - set.remove!(self) if set&.can_edit_posts?(CurrentUser.user) + set = PostSet.find_by(shortname: $1) + if set&.can_edit_posts?(CurrentUser.user) + set.remove!(self) + if set.errors.any? + errors.add(:base, set.errors.full_messages.join("; ")) + end + end when /^child:none$/i children.each do |post| diff --git a/app/models/post_set.rb b/app/models/post_set.rb index abcd7e13e..1af91525e 100644 --- a/app/models/post_set.rb +++ b/app/models/post_set.rb @@ -136,8 +136,9 @@ class PostSet < ApplicationRecord post_ids_before = post_ids_before_last_save || post_ids_was added = post_ids - post_ids_before return unless added.size > 0 - if post_ids.size > 10_000 - errors.add(:base, "Sets can have up to 10,000 posts each") + max = Danbooru.config.set_post_limit(CurrentUser.user) + if post_ids.size > max + errors.add(:base, "Sets can only have up to #{ActiveSupport::NumberHelper.number_to_delimited(max)} posts each") false else true @@ -203,6 +204,7 @@ class PostSet < ApplicationRecord reload self.skip_sync = true update(post_ids: post_ids + [post.id]) + raise(ActiveRecord::Rollback) unless valid? post.add_set!(self, true) post.save end @@ -219,6 +221,7 @@ class PostSet < ApplicationRecord reload self.skip_sync = true update(post_ids: post_ids - [post.id]) + raise(ActiveRecord::Rollback) unless valid? post.remove_set!(self) post.save end diff --git a/config/danbooru_default_config.rb b/config/danbooru_default_config.rb index 1a345f44e..fd3cadda5 100644 --- a/config/danbooru_default_config.rb +++ b/config/danbooru_default_config.rb @@ -337,6 +337,14 @@ module Danbooru 20_000 end + def pool_post_limit(_user) + 1_000 + end + + def set_post_limit(_user) # rubocop:disable Naming/AccessorMethodName + 10_000 + end + def discord_site end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 9f8f8624c..70ddc8a75 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -2269,4 +2269,99 @@ class PostTest < ActiveSupport::TestCase end end end + + context "Metatags:" do + context "set:" do + setup do + @set = create(:post_set, creator: @user) + @post = create(:post) + end + + context "with an id" do + should "work" do + @post.update(tag_string_diff: "set:#{@set.id}") + assert_equal([@post.id], @set.reload.post_ids) + assert_equal("set:#{@set.id}", @post.pool_string) + end + + should "gracefully fail if the set is full" do + Danbooru.config.stubs(:set_post_limit).returns(0) + @post.update(tag_string_diff: "set:#{@set.id}") + assert_equal(["Sets can only have up to 0 posts each"], @post.errors.full_messages) + assert_equal([], @set.reload.post_ids) + assert_equal("", @post.pool_string) + end + end + + context "with a shortname" do + should "work" do + @post.update(tag_string_diff: "set:#{@set.shortname}") + assert_equal([@post.id], @set.reload.post_ids) + assert_equal("set:#{@set.id}", @post.pool_string) + end + + should "gracefully fail if the set is full" do + Danbooru.config.stubs(:set_post_limit).returns(0) + @post.update(tag_string_diff: "set:#{@set.shortname}") + assert_equal(["Sets can only have up to 0 posts each"], @post.errors.full_messages) + assert_equal([], @set.reload.post_ids) + assert_equal("", @post.pool_string) + end + end + end + + context "pool:" do + setup do + @pool = create(:pool) + @post = create(:post) + end + + context "with an id" do + should "work" do + @post.update(tag_string_diff: "pool:#{@pool.id}") + assert_equal([@post.id], @pool.reload.post_ids) + assert_equal("pool:#{@pool.id}", @post.pool_string) + end + + should "gracefully fail if the pool is full" do + Danbooru.config.stubs(:pool_post_limit).returns(0) + @post.update(tag_string_diff: "pool:#{@pool.id}") + assert_equal(["Pools can only have up to 0 posts each"], @post.errors.full_messages) + assert_equal([], @pool.reload.post_ids) + assert_equal("", @post.pool_string) + end + end + + context "with a name" do + should "work" do + @post.update(tag_string_diff: "pool:#{@pool.name}") + assert_equal([@post.id], @pool.reload.post_ids) + assert_equal("pool:#{@pool.id}", @post.pool_string) + end + + should "gracefully fail if the pool is full" do + Danbooru.config.stubs(:pool_post_limit).returns(0) + @post.update(tag_string_diff: "pool:#{@pool.name}") + assert_equal(["Pools can only have up to 0 posts each"], @post.errors.full_messages) + assert_equal([], @pool.reload.post_ids) + assert_equal("", @post.pool_string) + end + end + end + + context "newpool:" do + setup do + @post = create(:post) + end + + should "work" do + assert_difference("Pool.count", 1) do + @post.update(tag_string_diff: "newpool:test") + end + @pool = Pool.last + assert_equal([@post.id], @pool.reload.post_ids) + assert_equal("pool:#{@pool.id}", @post.pool_string) + end + end + end end