Fix #5146: Adding an existing favorite to favorite groups leads to an error.

Show "Favgroup already contains post XXX" error when trying to add a
post to a favgroup that already contains that post.

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

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
 

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

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");

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 = create(:favorite_group, post_ids: [@post.id])
-            @favgroup.add!(@post)
 
             @comment = create(:comment, post: @post, creator: @admin)
             create(:comment_vote, comment: @comment, user: @user)

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])