* 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

@@ -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

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

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

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

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)
+    def after_id
-      super
+      params[:after_id]
-      @before_id = params[:before_id]
-      @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
     

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

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

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

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)
+    def add_favorite!(user)
-      if user_id.is_a?(ActiveRecord::Base)
+      return if favorited_by?(user.id)
-        user_id = user_id.id
+      append_user_to_fav_string(user.id)
-      end
+      user.add_favorite!(self)
-      
-      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)
     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)
+    def remove_favorite!(user)
-      if user_id.is_a?(ActiveRecord::Base)
+      return unless favorited_by?(user.id)
-        user_id = user_id.id
+      delete_user_from_fav_string(user.id)
-      end
+      user.remove_favorite!(self)
-      
-      delete_user_from_fav_string(user_id)
-
-      Favorite.model_for(user_id).destroy_all(:user_id => user_id, :post_id => id)
     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)
+        parent.add_favorite!(User.find(user_id))
-        remove_favorite(user_id)
+        remove_favorite!(User.find(user_id))
       end
     end
   end

app/models/user.rb

@@ -117,8 +117,20 @@ class User < ActiveRecord::Base
   end
   
   module FavoriteMethods
-    def favorites(options = {})
+    def favorites
-      Favorite.model_for(id).where("user_id = ?", id)
+      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
   

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)
+      user1.add_favorite!(p1)
-      p2.add_favorite(user1)
+      user1.add_favorite!(p2)
-      p1.add_favorite(user2)
+      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)
-      
+      user1.add_favorite!(p1)
-      p1.add_favorite(user1)
+      user1.add_favorite!(p1)
-      p1.add_favorite(user1)
       
       assert_equal(1, user1.favorites.count)
     end

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

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
+      context "a post pool set for page 2" do
-        should "by default sort the posts by id" do
+        setup do
-          assert_equal([@post_1.id, @post_2.id, @post_3.id], @set.posts.map(&:id))
+          @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
+        context "a numbered paginator" do
-          assert_equal([@post_2.id, @post_1.id, @post_3.id], @set.sorted_posts.map(&:id))
+          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

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)