From f67374da83c29349e93368d8fff229b5c96e9fe5 Mon Sep 17 00:00:00 2001 From: albert Date: Tue, 7 Jun 2011 19:06:39 -0400 Subject: [PATCH] * Some major bug fixes related to post sets. Tests for pools and favorites were added. * Refactored the favorites code a bit. Adding a favorite from either an user or a post now works and will update all necessary records. --- app/logical/post_sets/favorite.rb | 10 ++- app/logical/post_sets/numbered.rb | 13 ++- app/logical/post_sets/pool.rb | 4 +- app/logical/post_sets/post.rb | 6 +- app/logical/post_sets/sequential.rb | 16 ++-- app/logical/post_sets/wiki_page.rb | 2 +- app/models/favorite.rb | 2 +- app/models/pool.rb | 3 +- app/models/post.rb | 32 +++----- app/models/user.rb | 16 +++- test/unit/favorite_test.rb | 11 ++- test/unit/post_sets/favorite_test.rb | 116 +++++++++++++++++++++++++++ test/unit/post_sets/pool_test.rb | 42 ++++++++-- test/unit/post_test.rb | 12 +-- 14 files changed, 215 insertions(+), 70 deletions(-) create mode 100644 test/unit/post_sets/favorite_test.rb diff --git a/app/logical/post_sets/favorite.rb b/app/logical/post_sets/favorite.rb index e9c25ba5c..b6e2c530b 100644 --- a/app/logical/post_sets/favorite.rb +++ b/app/logical/post_sets/favorite.rb @@ -1,7 +1,7 @@ module PostSets module Favorite def user - @user ||= User.find(params[:id]) + @user ||= ::User.find(params[:id]) end def tags @@ -19,11 +19,15 @@ module PostSets end def count - @count ||= Favorite.count(user.id) + @count ||= relation.count end def posts - @posts ||= user.favorites(pagination_options) + @posts ||= slice(relation).map(&:post) + end + + def relation + ::Favorite.model_for(user.id).where("user_id = ?", user.id).order("id desc") end end end diff --git a/app/logical/post_sets/numbered.rb b/app/logical/post_sets/numbered.rb index b9457decb..bc33fbcd3 100644 --- a/app/logical/post_sets/numbered.rb +++ b/app/logical/post_sets/numbered.rb @@ -1,12 +1,5 @@ module PostSets module Numbered - attr_reader :page - - def initialize(params) - super - @page = options[:page] ? options[:page].to_i : 1 - end - def total_pages @total_pages ||= (count / limit.to_f).ceil.to_i end @@ -17,7 +10,7 @@ module PostSets end def slice(relation) - relation.offset(offset).all + relation.offset(offset).limit(limit).all end def pagination_options @@ -28,6 +21,10 @@ module PostSets offset == 0 end + def page + @page ||= params[:page] ? params[:page].to_i : 1 + end + def offset ((page < 1) ? 0 : (page - 1)) * limit end diff --git a/app/logical/post_sets/pool.rb b/app/logical/post_sets/pool.rb index 44d9517e1..e2a45a152 100644 --- a/app/logical/post_sets/pool.rb +++ b/app/logical/post_sets/pool.rb @@ -3,7 +3,7 @@ module PostSets module Pool def pool - @pool ||= Pool.find(params[:id]) + @pool ||= ::Pool.find(params[:id]) end def tags @@ -19,7 +19,7 @@ module PostSets end def posts - @posts ||= pool.posts(pagination_options) + @posts ||= pool.posts(pagination_options).limit(limit).all end def reload diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index af53eafd7..8d5b90917 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -5,7 +5,7 @@ module PostSets attr_accessor :tags, :count, :wiki_page, :artist, :suggestions def tags - @tags ||= Tag.normalize(params[:tags]) + @tags ||= ::Tag.normalize(params[:tags]) end def count @@ -13,7 +13,7 @@ module PostSets end def posts - @posts ||= slice(::Post.tag_match(tags).limit(limit)) + @posts ||= slice(::Post.tag_match(tags)) end def reload @@ -42,7 +42,7 @@ module PostSets end def tag_array - @tag_array ||= Tag.scan_query(tags) + @tag_array ||= ::Tag.scan_query(tags) end def validate diff --git a/app/logical/post_sets/sequential.rb b/app/logical/post_sets/sequential.rb index 6201361a9..339657804 100644 --- a/app/logical/post_sets/sequential.rb +++ b/app/logical/post_sets/sequential.rb @@ -1,20 +1,20 @@ module PostSets module Sequential - attr_reader :before_id, :after_id + def before_id + params[:before_id] + end - def initialize(params) - super - @before_id = params[:before_id] - @after_id = params[:after_id] + def after_id + params[:after_id] end def slice(relation) if before_id - relation.where("id < ?", before_id).all + relation.where("id < ?", before_id).limit(limit).all elsif after_id - relation.where("id > ?", after_id).order("id asc").all.reverse + relation.where("id > ?", after_id).order("id asc").limit(limit).all.reverse else - relation.all + relation.limit(limit).all end end diff --git a/app/logical/post_sets/wiki_page.rb b/app/logical/post_sets/wiki_page.rb index c79385a4e..c0e2c6241 100644 --- a/app/logical/post_sets/wiki_page.rb +++ b/app/logical/post_sets/wiki_page.rb @@ -15,7 +15,7 @@ module PostSets end def tags - @tags ||= Tag.normalize(wiki_page.title) + @tags ||= ::Tag.normalize(wiki_page.title) end def posts diff --git a/app/models/favorite.rb b/app/models/favorite.rb index bb4a1c6f8..900cccad2 100644 --- a/app/models/favorite.rb +++ b/app/models/favorite.rb @@ -1,6 +1,6 @@ class Favorite < ActiveRecord::Base TABLE_COUNT = 100 - validates_uniqueness_of :post_id, :scope => :user_id + belongs_to :post def self.model_for(user_id) mod = user_id.to_i % TABLE_COUNT diff --git a/app/models/pool.rb b/app/models/pool.rb index b85d233bf..b19ceb5b6 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -78,8 +78,9 @@ class Pool < ActiveRecord::Base end def posts(options = {}) + limit = options[:limit] || Danbooru.config.posts_per_page + if options[:offset] - limit = options[:limit] || Danbooru.config.posts_per_page slice = post_id_array.slice(options[:offset], limit) if slice && slice.any? Post.where("id in (?)", slice).order(arbitrary_sql_order_clause(slice, "posts")) diff --git a/app/models/post.rb b/app/models/post.rb index 01aba4ada..2e8ab02ac 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -384,32 +384,20 @@ class Post < ActiveRecord::Base update_attribute(:fav_string, (fav_string + " fav:#{user_id}").strip) end - def add_favorite(user_id) - if user_id.is_a?(ActiveRecord::Base) - user_id = user_id.id - end - - if favorited_by?(user_id) - return false - end - - append_user_to_fav_string(user_id) - - Favorite.model_for(user_id).create(:user_id => user_id, :post_id => id) + def add_favorite!(user) + return if favorited_by?(user.id) + append_user_to_fav_string(user.id) + user.add_favorite!(self) end def delete_user_from_fav_string(user_id) update_attribute(:fav_string, fav_string.gsub(/(?:\A| )fav:#{user_id}(?:\Z| )/, " ").strip) end - def remove_favorite(user_id) - if user_id.is_a?(ActiveRecord::Base) - user_id = user_id.id - end - - delete_user_from_fav_string(user_id) - - Favorite.model_for(user_id).destroy_all(:user_id => user_id, :post_id => id) + def remove_favorite!(user) + return unless favorited_by?(user.id) + delete_user_from_fav_string(user.id) + user.remove_favorite!(self) end def favorited_user_ids @@ -785,8 +773,8 @@ class Post < ActiveRecord::Base return if parent.nil? favorited_user_ids.each do |user_id| - parent.add_favorite(user_id) - remove_favorite(user_id) + parent.add_favorite!(User.find(user_id)) + remove_favorite!(User.find(user_id)) end end end diff --git a/app/models/user.rb b/app/models/user.rb index 1890b8f95..948edc408 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -117,8 +117,20 @@ class User < ActiveRecord::Base end module FavoriteMethods - def favorites(options = {}) - Favorite.model_for(id).where("user_id = ?", id) + def favorites + Favorite.model_for(id).where("user_id = ?", id).order("id desc") + end + + def add_favorite!(post) + return if Favorite.model_for(id).exists?(:user_id => id, :post_id => post.id) + Favorite.model_for(id).create(:user_id => id, :post_id => post.id) + post.add_favorite!(self) + end + + def remove_favorite!(post) + return unless Favorite.model_for(id).exists?(:user_id => id, :post_id => post.id) + Favorite.model_for(id).destroy_all(:user_id => id, :post_id => post.id) + post.remove_favorite!(self) end end diff --git a/test/unit/favorite_test.rb b/test/unit/favorite_test.rb index 8299753c3..c590fee7b 100644 --- a/test/unit/favorite_test.rb +++ b/test/unit/favorite_test.rb @@ -21,9 +21,9 @@ class FavoriteTest < ActiveSupport::TestCase p1 = Factory.create(:post) p2 = Factory.create(:post) - p1.add_favorite(user1) - p2.add_favorite(user1) - p1.add_favorite(user2) + user1.add_favorite!(p1) + user1.add_favorite!(p2) + user2.add_favorite!(p1) favorites = user1.favorites.order("id desc") assert_equal(2, favorites.count) @@ -39,9 +39,8 @@ class FavoriteTest < ActiveSupport::TestCase user1 = Factory.create(:user) p1 = Factory.create(:post) p2 = Factory.create(:post) - - p1.add_favorite(user1) - p1.add_favorite(user1) + user1.add_favorite!(p1) + user1.add_favorite!(p1) assert_equal(1, user1.favorites.count) end diff --git a/test/unit/post_sets/favorite_test.rb b/test/unit/post_sets/favorite_test.rb new file mode 100644 index 000000000..2e8776eda --- /dev/null +++ b/test/unit/post_sets/favorite_test.rb @@ -0,0 +1,116 @@ +require_relative '../../test_helper' + +module PostSets + class FavoriteTest < ActiveSupport::TestCase + context "In all cases" do + setup do + @user = Factory.create(:user) + CurrentUser.user = @user + CurrentUser.ip_addr = "127.0.0.1" + MEMCACHE.flush_all + + @post_1 = Factory.create(:post) + @post_2 = Factory.create(:post) + @post_3 = Factory.create(:post) + @post_2.add_favorite!(@user) + @post_1.add_favorite!(@user) + @post_3.add_favorite!(@user) + end + + teardown do + CurrentUser.user = nil + CurrentUser.ip_addr = nil + end + + context "a favorite set for before the most recent post" do + setup do + id = ::Favorite.model_for(@user.id).where(:user_id => @user.id, :post_id => @post_3.id).first.id + @set = PostSets::Base.new(:id => @user.id, :before_id => id) + @set.stubs(:limit).returns(1) + @set.extend(PostSets::Favorite) + end + + context "a sequential paginator" do + setup do + @set.extend(PostSets::Sequential) + end + + should "return the second most recent element" do + assert_equal(1, @set.posts.size) + assert_equal(@post_1.id, @set.posts.first.id) + end + end + end + + context "a favorite set for after the second most recent post" do + setup do + id = ::Favorite.model_for(@user.id).where(:user_id => @user.id, :post_id => @post_2.id).first.id + @set = PostSets::Base.new(:id => @user.id, :after_id => id) + @set.stubs(:limit).returns(1) + @set.extend(PostSets::Favorite) + end + + context "a sequential paginator" do + setup do + @set.extend(PostSets::Sequential) + end + + should "return the most recent element" do + assert_equal(1, @set.posts.size) + assert_equal(@post_3.id, @set.posts.first.id) + end + end + end + + context "a favorite set for page 2" do + setup do + @set = PostSets::Base.new(:id => @user.id, :page => 2) + @set.stubs(:limit).returns(1) + @set.extend(PostSets::Favorite) + end + + context "a numbered paginator" do + setup do + @set.extend(PostSets::Numbered) + end + + should "return the second most recent element" do + assert_equal(1, @set.posts.size) + assert_equal(@post_1.id, @set.posts.first.id) + end + end + end + + context "a favorite set with no page specified" do + setup do + @set = PostSets::Base.new(:id => @user.id) + @set.stubs(:limit).returns(1) + @set.extend(PostSets::Favorite) + end + + context "a numbered paginator" do + setup do + @set.extend(PostSets::Numbered) + end + + should "return the most recent element" do + assert_equal(3, @set.count) + assert_equal(1, @set.posts.size) + assert_equal(@post_3.id, @set.posts.first.id) + end + end + + context "a sequential paginator" do + setup do + @set.extend(PostSets::Sequential) + end + + should "return the most recent element" do + assert_equal(1, @set.posts.size) + assert_equal(@post_3.id, @set.posts.first.id) + end + end + end + end + end +end diff --git a/test/unit/post_sets/pool_test.rb b/test/unit/post_sets/pool_test.rb index 95f4b047b..be25e8e7e 100644 --- a/test/unit/post_sets/pool_test.rb +++ b/test/unit/post_sets/pool_test.rb @@ -16,7 +16,6 @@ module PostSets @pool.add_post!(@post_2) @pool.add_post!(@post_1) @pool.add_post!(@post_3) - @set = PostSets::Pool.new(@pool, :page => 1) end teardown do @@ -24,13 +23,42 @@ module PostSets CurrentUser.ip_addr = nil end - context "a pool with three posts" do - should "by default sort the posts by id" do - assert_equal([@post_1.id, @post_2.id, @post_3.id], @set.posts.map(&:id)) + context "a post pool set for page 2" do + setup do + @set = PostSets::Base.new(:id => @pool.id, :page => 2) + @set.stubs(:limit).returns(1) + @set.extend(PostSets::Pool) end - - should "be capable of sorting by pool sequence" do - assert_equal([@post_2.id, @post_1.id, @post_3.id], @set.sorted_posts.map(&:id)) + + context "a numbered paginator" do + setup do + @set.extend(PostSets::Numbered) + end + + should "return the second element" do + assert_equal(1, @set.posts.size) + assert_equal(@post_1.id, @set.posts.first.id) + end + end + end + + context "a post pool set with no page specified" do + setup do + @set = PostSets::Base.new(:id => @pool.id) + @set.stubs(:limit).returns(1) + @set.extend(PostSets::Pool) + end + + context "a numbered paginator" do + setup do + @set.extend(PostSets::Numbered) + end + + should "return the first element" do + assert_equal(3, @set.count) + assert_equal(1, @set.posts.size) + assert_equal(@post_2.id, @set.posts.first.id) + end end end end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 9987df120..f5368db64 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -82,7 +82,7 @@ class PostTest < ActiveSupport::TestCase p1 = Factory.create(:post) c1 = Factory.create(:post, :parent_id => p1.id) user = Factory.create(:user) - c1.add_favorite(user) + c1.add_favorite!(user) c1.delete! p1.reload assert(!Favorite.model_for(user.id).exists?(:post_id => c1.id, :user_id => user.id)) @@ -332,22 +332,22 @@ class PostTest < ActiveSupport::TestCase should "update the fav strings ont he post" do user = Factory.create(:user) post = Factory.create(:post) - post.add_favorite(user) + post.add_favorite!(user) post.reload assert_equal("fav:#{user.id}", post.fav_string) assert(Favorite.model_for(user.id).exists?(:user_id => user.id, :post_id => post.id)) - post.add_favorite(user) + post.add_favorite!(user) post.reload assert_equal("fav:#{user.id}", post.fav_string) assert(Favorite.model_for(user.id).exists?(:user_id => user.id, :post_id => post.id)) - post.remove_favorite(user) + post.remove_favorite!(user) post.reload assert_equal("", post.fav_string) assert(!Favorite.model_for(user.id).exists?(:user_id => user.id, :post_id => post.id)) - post.remove_favorite(user) + post.remove_favorite!(user) post.reload assert_equal("", post.fav_string) assert(!Favorite.model_for(user.id).exists?(:user_id => user.id, :post_id => post.id)) @@ -474,7 +474,7 @@ class PostTest < ActiveSupport::TestCase post2 = Factory.create(:post) post3 = Factory.create(:post) user = Factory.create(:user) - post1.add_favorite(user) + post1.add_favorite!(user) relation = Post.tag_match("fav:#{user.name}") assert_equal(1, relation.count) assert_equal(post1.id, relation.first.id)