* Refactored routes some

* Fixed some major bugs with pools and pool versioning

app/controllers/advertisement_hits_controller.rb

@@ -1,6 +1,6 @@
 class AdvertisementHitsController < ApplicationController
   def create
-    advertisement = Advertisement.find(params[:id])
+    advertisement = Advertisement.find(params[:advertisement_id])
     advertisement.hits.create(:ip_addr => request.remote_ip)
 		redirect_to advertisement.referral_url
   end

app/controllers/pools_posts_controller.rb

@@ -4,15 +4,15 @@ class PoolsPostsController < ApplicationController
   
   def create
     @pool = Pool.find(params[:pool_id])
-    @post = Post.find(params[:post_id])
+    @post = Post.find(params[:id])
     @pool.add_post!(@post)
-    respond_with(@pool)
+    respond_with(@pool, :location => pool_path(@pool))
   end
   
   def destroy
     @pool = Pool.find(params[:pool_id])
-    @post = Post.find(params[:post_id])
+    @post = Post.find(params[:id])
     @pool.remove_post!(@post)
-    respond_with(@pool)
+    respond_with(@pool, :location => pool_path(@pool))
   end
 end

app/controllers/post_moderation_controller.rb

@@ -1,5 +1,5 @@
-class PostModerationDetailsController < ApplicationController
-  def index
+class PostModerationController < ApplicationController
+  def show
   end
 
   def create

app/models/pool.rb

@@ -1,12 +1,11 @@
 class Pool < ActiveRecord::Base
-  attr_accessor :updater_id, :updater_ip_addr
   validates_uniqueness_of :name
-  validates_presence_of :name, :updater_id, :updater_ip_addr
   validates_format_of :name, :with => /\A[^\s;,]+\Z/, :on => :create, :message => "cannot have whitespace, commas, or semicolons"
   belongs_to :creator, :class_name => "User"
   belongs_to :updater, :class_name => "User"
   has_many :versions, :class_name => "PoolVersion", :dependent => :destroy
-  before_save :normalize_name
+  before_validation :normalize_name
+  before_validation :initialize_creator, :on => :create
   after_save :create_version
   attr_accessible :name, :description, :post_ids, :is_active
   
@@ -18,14 +17,16 @@ class Pool < ActiveRecord::Base
     Pool.new do |pool|
       pool.name = "TEMP:#{Time.now.to_f}.#{rand(1_000_000)}"
       pool.creator = creator
-      pool.updater_id = creator.id
-      pool.updater_ip_addr = creator_ip_addr
       pool.save
       pool.name = "anonymous:#{pool.id}"
       pool.save
     end
   end
   
+  def initialize_creator
+    self.creator_id = CurrentUser.id
+  end
+  
   def normalize_name
     self.name = name.downcase
   end
@@ -39,13 +40,13 @@ class Pool < ActiveRecord::Base
     return if post_ids =~ /(?:\A| )#{post.id}(?:\Z| )/
     
     self.post_ids += " #{post.id}"
-    self.post_ids.strip!
+    self.post_ids = post_ids.strip
     save
   end
   
   def remove_post!(post)
-    post_ids.gsub!(/(?:\A| )#{post.id}(?:\Z| )/, " ")
-    post_ids.strip!
+    self.post_ids = post_ids.gsub(/(?:\A| )#{post.id}(?:\Z| )/, " ")
+    self.post_ids = post_ids.strip
     save
   end
   
@@ -81,14 +82,10 @@ class Pool < ActiveRecord::Base
   def create_version
     last_version = versions.last
 
-    if last_version && updater_ip_addr == last_version.updater_ip_addr && updater_id == last_version.updater_id
+    if last_version && CurrentUser.ip_addr == last_version.updater_ip_addr && CurrentUser.id == last_version.updater_id
       last_version.update_attribute(:post_ids, post_ids)
     else
-      versions.create(
-        :post_ids => post_ids,
-        :updater_id => updater_id,
-        :updater_ip_addr => updater_ip_addr
-      )
+      versions.create(:post_ids => post_ids)
     end
   end
   

app/models/pool_version.rb

@@ -3,4 +3,10 @@ class PoolVersion < ActiveRecord::Base
   
   validates_presence_of :updater_id, :updater_ip_addr
   belongs_to :pool
+  before_validation :initialize_updater
+  
+  def initialize_updater
+    self.updater_id = CurrentUser.id
+    self.updater_ip_addr = CurrentUser.ip_addr
+  end
 end

config/routes.rb

@@ -3,21 +3,23 @@ Danbooru::Application.routes.draw do
     match 'users/edit' => 'users#edit', :via => :get
     match 'users' => 'users#update', :via => :put
   end
-  resources :advertisements
-  resources :advertisement_hits
+  resources :advertisements do
+    resources :hits, :controller => "advertisement_hits", :only => [:create]
+  end
   resources :artists do
     member do
       put :revert
     end
   end
-  resources :artist_versions
+  resources :artist_versions, :only => [:index]
   resources :bans
-  resources :comments
-  resources :comment_votes
+  resources :comments do
+    resources :votes, :controller => "comment_votes", :only => [:create, :destroy]
+  end
   resources :dmails
   resources :favorites
-  resources :forum_posts
   resources :forum_topics
+  resources :forum_posts
   resources :janitor_trials do
     member do
       put :promote
@@ -31,22 +33,22 @@ Danbooru::Application.routes.draw do
       put :revert
     end
   end
-  resources :note_versions
+  resources :note_versions, :only => [:index]
   resources :pools do
     resources :posts, :controller => "pools_posts", :only => [:create, :destroy]
     member do
       put :revert
     end
   end
-  resources :pool_versions
+  resources :pool_versions, :only => [:index]
   resources :posts do
+    resources :votes, :controller => "post_votes", :only => [:create, :destroy]
     member do
       put :revert
     end
   end
-  resources :post_moderation_details
-  resources :post_histories
-  resources :post_votes
+  resources :post_histories, :only => [:index]
+  resource :post_moderation, :controller => "post_moderation"
   resource :session
   resources :tags
   resources :tag_aliases do
@@ -65,7 +67,7 @@ Danbooru::Application.routes.draw do
       put :revert
     end
   end
-  resources :wiki_page_versions
+  resources :wiki_page_versions, :only => [:index]
 
   match '/dtext/preview' => 'dtext#preview', :via => :post
   match "/site_map" => "static#site_map", :as => "site_map"

test/factories/pool.rb

@@ -2,6 +2,4 @@ Factory.define(:pool) do |f|
   f.name {Faker::Name.first_name}
   f.creator {|x| x.association(:user)}
   f.description {Faker::Lorem.sentences}
-  f.updater_id {|x| x.creator_id}
-  f.updater_ip_addr "127.0.0.1"
 end

test/functional/advertisement_hits_controller_test.rb

@@ -9,7 +9,7 @@ class AdvertisementHitsControllerTest < ActionController::TestCase
 
     should "create a new hit" do
       assert_difference("AdvertisementHit.count", 1) do
-        post :create, {:id => @ad.id}
+        post :create, {:advertisement_id => @ad.id}
       end
       assert_redirected_to(@ad.referral_url)
     end

test/functional/pools_controller_test.rb

@@ -75,15 +75,18 @@ class PoolsControllerTest < ActionController::TestCase
     
     context "revert action" do
       setup do
-        @pool = Factory.create(:pool, :name => "000")
-        @pool.update_attributes(:name => "111")
-        @pool.update_attributes(:name => "222")
+        @pool = Factory.create(:pool, :post_ids => "1")
+        CurrentUser.ip_addr = "1.2.3.4" # this is to get around the version collation
+        @pool.update_attributes(:post_ids => "1 2")
+        CurrentUser.ip_addr = "127.0.0.1"
       end
       
       should "revert to a previous version" do
-        post :revert, {:id => @pool.id, :version_id => @pool.versions(true).first.id}
+        version = @pool.versions(true).first
+        assert_equal("1", version.post_ids)
+        post :revert, {:id => @pool.id, :version_id => version.id}
         @pool.reload
-        assert_equal("000", @pool.name)
+        assert_equal("1", @pool.post_ids)
       end
     end
   end

test/functional/pools_posts_controller_test.rb

@@ -0,0 +1,52 @@
+require 'test_helper'
+
+class PoolsPostsControllerTest < ActionController::TestCase
+  context "The pools posts controller" do
+    setup do
+      @user = Factory.create(:user)
+      @mod = Factory.create(:moderator_user)
+      CurrentUser.user = @user
+      CurrentUser.ip_addr = "127.0.0.1"
+      @post = Factory.create(:post)
+      @pool = Factory.create(:pool, :name => "abc")
+    end
+    
+    teardown do
+      CurrentUser.user = nil
+    end
+    
+    context "create action" do
+      should "add a post to a pool" do
+        post :create, {:pool_id => @pool.id, :id => @post.id, :format => "json"}, {:user_id => @user.id}
+        @pool.reload
+        assert_equal([@post.id], @pool.post_id_array)
+      end
+      
+      should "add a post to a pool once and only once" do
+        @pool.add_post!(@post)
+        post :create, {:pool_id => @pool.id, :id => @post.id, :format => "json"}, {:user_id => @user.id}
+        @pool.reload
+        assert_equal([@post.id], @pool.post_id_array)
+      end
+    end
+    
+    context "destroy action" do
+      setup do
+        @pool.add_post!(@post)
+      end
+      
+      should "remove a post from a pool" do
+        post :destroy, {:pool_id => @pool.id, :id => @post.id, :format => "json"}, {:user_id => @user.id}
+        @pool.reload
+        assert_equal([], @pool.post_id_array)
+      end
+      
+      should "do nothing if the post is not a member of the pool" do
+        @pool.remove_post!(@post)
+        post :destroy, {:pool_id => @pool.id, :id => @post.id, :format => "json"}, {:user_id => @user.id}
+        @pool.reload
+        assert_equal([], @pool.post_id_array)
+      end
+    end
+  end
+end

test/functional/post_moderation_controller_test.rb

@@ -1,7 +1,7 @@
 require File.join(File.dirname(__FILE__), %w(.. test_helper))
 
-class PostModerationDetailControllerTest < ActionController::TestCase
-  context "A post moderation detail controller" do
+class PostModerationControllerTest < ActionController::TestCase
+  context "A post moderation controller" do
     should "" do
       ModQueuePost.destroy_all
 

test/functional/post_moderation_details_controller_test.rb

@@ -1,8 +0,0 @@
-require 'test_helper'
-
-class PostModerationDetailsControllerTest < ActionController::TestCase
-  # Replace this with your real tests.
-  test "the truth" do
-    assert true
-  end
-end