diff --git a/app/controllers/favorites_controller.rb b/app/controllers/favorites_controller.rb index 75e481e14..82c99ac86 100644 --- a/app/controllers/favorites_controller.rb +++ b/app/controllers/favorites_controller.rb @@ -1,6 +1,6 @@ class FavoritesController < ApplicationController before_action :member_only, except: [:index] - respond_to :html, :xml, :json + respond_to :html, :xml, :json, :js skip_before_action :api_check def index @@ -24,23 +24,10 @@ class FavoritesController < ApplicationController end def create - if CurrentUser.favorite_limit.nil? || CurrentUser.favorite_count < CurrentUser.favorite_limit - @post = Post.find(params[:post_id]) - @post.add_favorite!(CurrentUser.user) - else - @error_msg = "You can only keep up to #{CurrentUser.favorite_limit} favorites. Upgrade your account to save more." - end + @post = Post.find(params[:post_id]) + @post.add_favorite!(CurrentUser.user) - respond_with(@post) do |format| - format.js - format.json do - if @post - render :json => {:success => true}.to_json - else - render :json => {:success => false, :reason => @error_msg}.to_json, :status => 422 - end - end - end + respond_with(@post) end def destroy @@ -52,11 +39,6 @@ class FavoritesController < ApplicationController Favorite.remove(post_id: params[:id], user: CurrentUser.user) end - respond_with(@post) do |format| - format.js - format.json do - render :json => {:success => true}.to_json - end - end + respond_with(@post) end end diff --git a/app/models/favorite.rb b/app/models/favorite.rb index ed1ac6828..d751cee07 100644 --- a/app/models/favorite.rb +++ b/app/models/favorite.rb @@ -1,4 +1,6 @@ class Favorite < ApplicationRecord + class Error < Exception ; end + belongs_to :post belongs_to :user scope :for_user, ->(user_id) {where("user_id % 100 = #{user_id.to_i % 100} and user_id = #{user_id.to_i}")} @@ -7,7 +9,12 @@ class Favorite < ApplicationRecord Favorite.transaction do User.where(:id => user.id).select("id").lock("FOR UPDATE NOWAIT").first - return if Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post.id).exists? + if user.favorite_count >= user.favorite_limit + raise Error, "You can only keep up to #{user.favorite_limit} favorites. Upgrade your account to save more." + elsif Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post.id).exists? + raise Error, "You have already favorited this post" + end + Favorite.create!(:user_id => user.id, :post_id => post.id) Post.where(:id => post.id).update_all("fav_count = fav_count + 1") post.append_user_to_fav_string(user.id) diff --git a/app/models/user.rb b/app/models/user.rb index 85aef23bd..1a7117d16 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -605,7 +605,7 @@ class User < ApplicationRecord def favorite_limit if is_platinum? - nil + Float::INFINITY elsif is_gold? 20_000 else diff --git a/app/views/favorites/create.js.erb b/app/views/favorites/create.js.erb index 4439d2c53..2b66837a8 100644 --- a/app/views/favorites/create.js.erb +++ b/app/views/favorites/create.js.erb @@ -1,17 +1,13 @@ -<% if @error_msg %> - $(window).trigger("danbooru:error", "<%= @error_msg %>"); -<% else %> - $("#add-to-favorites").hide(); - $("#add-fav-button").hide(); - $("#remove-from-favorites").show(); - $("#remove-fav-button").show(); - $("#score-for-post-<%= @post.id %>").html(<%= @post.score %>); - $("#favcount-for-post-<%= @post.id %>").html(<%= @post.fav_count %>); - <% if CurrentUser.is_gold? %> - $("#favlist").html("<%= escape_javascript(post_favlist(@post)) %>"); - if (!$("#favlist").is(":visible")) { - $("#show-favlist-link").show(); - } - <% end %> - $(window).trigger("danbooru:notice", "You have favorited this post"); +$("#add-to-favorites").hide(); +$("#add-fav-button").hide(); +$("#remove-from-favorites").show(); +$("#remove-fav-button").show(); +$("#score-for-post-<%= @post.id %>").html(<%= @post.score %>); +$("#favcount-for-post-<%= @post.id %>").html(<%= @post.fav_count %>); +<% if CurrentUser.is_gold? %> + $("#favlist").html("<%= escape_javascript(post_favlist(@post)) %>"); + if (!$("#favlist").is(":visible")) { + $("#show-favlist-link").show(); + } <% end %> +$(window).trigger("danbooru:notice", "You have favorited this post"); diff --git a/app/views/static/error.js.erb b/app/views/static/error.js.erb new file mode 100644 index 000000000..dff1e527a --- /dev/null +++ b/app/views/static/error.js.erb @@ -0,0 +1,2 @@ +var message = <%= raw @error_message.try(:to_json) || @exception.message.to_json %>; +Danbooru.Utility.error(message); diff --git a/test/unit/favorite_test.rb b/test/unit/favorite_test.rb index 6bbdb4d10..c4536442f 100644 --- a/test/unit/favorite_test.rb +++ b/test/unit/favorite_test.rb @@ -2,10 +2,13 @@ require 'test_helper' class FavoriteTest < ActiveSupport::TestCase setup do - user = FactoryBot.create(:user) - CurrentUser.user = user + @user1 = FactoryBot.create(:user) + @user2 = FactoryBot.create(:user) + @p1 = FactoryBot.create(:post) + @p2 = FactoryBot.create(:post) + + CurrentUser.user = @user1 CurrentUser.ip_addr = "127.0.0.1" - Favorite # need to force loading the favorite model end teardown do @@ -15,44 +18,41 @@ class FavoriteTest < ActiveSupport::TestCase context "A favorite" do should "delete from all tables" do - user1 = FactoryBot.create(:user) - p1 = FactoryBot.create(:post) + @p1.add_favorite!(@user1) + assert_equal(1, @user1.favorite_count) - p1.add_favorite!(user1) - assert_equal(1, Favorite.count) - - Favorite.where(:user_id => user1.id, :post_id => p1.id).delete_all + Favorite.where(:user_id => @user1.id, :post_id => @p1.id).delete_all assert_equal(0, Favorite.count) end should "know which table it belongs to" do - user1 = FactoryBot.create(:user) - user2 = FactoryBot.create(:user) - p1 = FactoryBot.create(:post) - p2 = FactoryBot.create(:post) + @p1.add_favorite!(@user1) + @p2.add_favorite!(@user1) + @p1.add_favorite!(@user2) - p1.add_favorite!(user1) - p2.add_favorite!(user1) - p1.add_favorite!(user2) - - favorites = user1.favorites.order("id desc") + favorites = @user1.favorites.order("id desc") assert_equal(2, favorites.count) - assert_equal(p2.id, favorites[0].post_id) - assert_equal(p1.id, favorites[1].post_id) + assert_equal(@p2.id, favorites[0].post_id) + assert_equal(@p1.id, favorites[1].post_id) - favorites = user2.favorites.order("id desc") + favorites = @user2.favorites.order("id desc") assert_equal(1, favorites.count) - assert_equal(p1.id, favorites[0].post_id) + assert_equal(@p1.id, favorites[0].post_id) end should "not allow duplicates" do - user1 = FactoryBot.create(:user) - p1 = FactoryBot.create(:post) - p2 = FactoryBot.create(:post) - p1.add_favorite!(user1) - p1.add_favorite!(user1) + @p1.add_favorite!(@user1) + error = assert_raises(Favorite::Error) { @p1.add_favorite!(@user1) } - assert_equal(1, user1.favorites.count) + assert_equal("You have already favorited this post", error.message) + assert_equal(1, @user1.favorite_count) + end + + should "not allow exceeding the user's favorite limit" do + @user1.stubs(:favorite_limit).returns(0) + error = assert_raises(Favorite::Error) { @p1.add_favorite!(@user1) } + + assert_equal("You can only keep up to 0 favorites. Upgrade your account to save more.", error.message) end end end