diff --git a/app/controllers/favorite_groups_controller.rb b/app/controllers/favorite_groups_controller.rb index ea112f0c4..a5ba8ee6d 100644 --- a/app/controllers/favorite_groups_controller.rb +++ b/app/controllers/favorite_groups_controller.rb @@ -55,6 +55,8 @@ class FavoriteGroupsController < ApplicationController def add_post @favorite_group = authorize FavoriteGroup.find(params[:id]) @post = Post.find(params[:post_id]) - @favorite_group.add!(@post) + @favorite_group.add(@post) + + respond_with(@favorite_group) end end diff --git a/app/models/favorite_group.rb b/app/models/favorite_group.rb index fad59840e..2e3430ec4 100644 --- a/app/models/favorite_group.rb +++ b/app/models/favorite_group.rb @@ -132,15 +132,15 @@ class FavoriteGroup < ApplicationRecord Post.joins("JOIN (#{favgroup_posts.to_sql}) favgroup_posts ON favgroup_posts.post_id = posts.id").order("favgroup_posts.favgroup_index ASC") end - def add!(post) + def add(post) with_lock do - update!(post_ids: post_ids + [post.id]) + update(post_ids: post_ids + [post.id]) end end - def remove!(post) + def remove(post) with_lock do - update!(post_ids: post_ids - [post.id]) + update(post_ids: post_ids - [post.id]) end end diff --git a/app/models/post.rb b/app/models/post.rb index 718a59f15..fd48a13bd 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -537,12 +537,12 @@ class Post < ApplicationRecord in "-favgroup", name favgroup = FavoriteGroup.find_by_name_or_id!(name, CurrentUser.user) raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, favgroup).update? - favgroup&.remove!(self) + favgroup&.remove(self) in "favgroup", name favgroup = FavoriteGroup.find_by_name_or_id!(name, CurrentUser.user) raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, favgroup).update? - favgroup&.add!(self) + favgroup&.add(self) end end @@ -632,7 +632,7 @@ class Post < ApplicationRecord def remove_from_fav_groups FavoriteGroup.for_post(id).find_each do |favgroup| - favgroup.remove!(self) + favgroup.remove(self) end end end diff --git a/app/views/favorite_groups/add_post.js.erb b/app/views/favorite_groups/add_post.js.erb index b888b83f9..fd712434f 100644 --- a/app/views/favorite_groups/add_post.js.erb +++ b/app/views/favorite_groups/add_post.js.erb @@ -1,2 +1,7 @@ -Danbooru.notice("Added post to favorite group <%= escape_javascript(@favorite_group.pretty_name) %>"); +<% if @favorite_group.errors.any? %> + Danbooru.notice("<%= j @favorite_group.errors.full_messages.join("; ") %>"); +<% else %> + Danbooru.notice("Added post to favorite group <%= j @favorite_group.pretty_name %>"); +<% end %> + $("#add-to-favgroup-dialog").dialog("close"); diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index 0efe421cd..e0c088ab6 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -534,8 +534,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest @pool = create(:pool) @pool.add!(@post) - @favgroup = create(:favorite_group) - @favgroup.add!(@post) + @favgroup = create(:favorite_group, post_ids: [@post.id]) @comment = create(:comment, post: @post, creator: @admin) create(:comment_vote, comment: @comment, user: @user) diff --git a/test/unit/favorite_group_test.rb b/test/unit/favorite_group_test.rb index 21285d57c..09743eb65 100644 --- a/test/unit/favorite_group_test.rb +++ b/test/unit/favorite_group_test.rb @@ -9,13 +9,13 @@ class FavoriteTest < ActiveSupport::TestCase should "return the fav group" do posts = create_list(:post, 3) - @fav_group.add!(posts[0]) + @fav_group.add(posts[0]) assert_equal(@fav_group.id, FavoriteGroup.for_post(posts[0].id).first.id) - @fav_group.add!(posts[1]) + @fav_group.add(posts[1]) assert_equal(@fav_group.id, FavoriteGroup.for_post(posts[1].id).first.id) - @fav_group.add!(posts[2]) + @fav_group.add(posts[2]) assert_equal(@fav_group.id, FavoriteGroup.for_post(posts[2].id).first.id) end end @@ -24,7 +24,7 @@ class FavoriteTest < ActiveSupport::TestCase should "remove it from all favorite groups" do @post = FactoryBot.create(:post) - @fav_group.add!(@post) + @fav_group.add(@post) assert_equal([@post.id], @fav_group.post_ids) @post.expunge! @@ -36,11 +36,14 @@ class FavoriteTest < ActiveSupport::TestCase should "not allow adding duplicate posts" do post = create(:post) - @fav_group.add!(post) + @fav_group.add(post) assert(@fav_group.valid?) assert_equal([post.id], @fav_group.reload.post_ids) - assert_raise(ActiveRecord::RecordInvalid) { @fav_group.add!(post) } + @fav_group.add(post) + assert_equal(false, @fav_group.valid?) + assert_match(/Favgroup already contains post #{post.id}/, @fav_group.errors.full_messages.join) + assert_equal([post.id], @fav_group.reload.post_ids) @fav_group.reload.update(post_ids: [post.id, post.id])