refactored drafts which are now identified by a draft_id on the node rather than by guessing it. i was really that close to make that mistake again.

7 files changed, 106 insertions, 33 deletions

diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb
index 9ba91f6..a403b95 100644
--- a/app/controllers/nodes_controller.rb
+++ b/app/controllers/nodes_controller.rb
@@ -12,7 +12,8 @@ class NodesController < ApplicationController
 
   def index
     @nodes = Node.root.descendants.paginate( 
-      :include => :head, 
+      :include => :head,
+      :include => :draft,
       :page => params[:page], 
       :per_page => 25,
       :order => 'id DESC'
diff --git a/app/models/node.rb b/app/models/node.rb
index e2cb564..99a2d84 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -5,6 +5,7 @@ class Node < ActiveRecord::Base
   # Associations
   has_many    :pages, :order => "revision ASC"
   belongs_to  :head,  :class_name => "Page",  :foreign_key => :head_id
+  belongs_to  :draft, :class_name => "Page",  :foreign_key => :draft_id
   has_many    :permissions
   
   # Callbacks
@@ -41,22 +42,14 @@ class Node < ActiveRecord::Base
   
   # Instance Methods
   
-  # check if there is a page which has a nil :published_at column
-  # if there is one - it is considered a draft 
-  def draft
-    if draft = pages.find_by_published_at(nil)
-      draft
-    end
-  end
-  
+
   def find_or_create_draft user
     if draft && draft.user == user
       draft
     elsif draft && draft.user.nil?
-      tmp_draft = draft
-      tmp_draft.user = user
-      tmp_draft.save
-      tmp_draft
+      draft.user = user
+      save
+      draft
     elsif draft && draft.user != user
       raise "Page is locked"
     else
@@ -65,16 +58,18 @@ class Node < ActiveRecord::Base
   end
   
   def create_new_draft user
-    page = self.pages.create!
+    empty_page = self.pages.new
+    empty_page.user = user
+    empty_page.save!
+    
+    self.draft = empty_page
     
     if self.head
-      clone_attributes_to page
+      clone_attributes_to draft
     end
     
-    page.user = user
-    page.save
-    page.reload
-    page
+    self.save
+    draft
   end
   
   def clone_attributes_to page
@@ -98,6 +93,7 @@ class Node < ActiveRecord::Base
   def publish_draft!
     if self.draft
       self.head = self.draft
+      self.draft = nil
       self.save!
       
       self.head.published_at = Time.now
@@ -131,7 +127,8 @@ class Node < ActiveRecord::Base
     # that draft and publishes it.
     def initialize_empty_page
       if self.pages.empty?
-        self.pages.create!
+        self.draft = self.pages.create!
+        self.save
       end
     end
 end
diff --git a/db/migrate/20090301104237_add_draft_id_to_nodes.rb b/db/migrate/20090301104237_add_draft_id_to_nodes.rb
new file mode 100644
index 0000000..a0ea672
--- /dev/null
+++ b/db/migrate/20090301104237_add_draft_id_to_nodes.rb
@@ -0,0 +1,9 @@
+class AddDraftIdToNodes < ActiveRecord::Migration
+  def self.up
+    add_column :nodes, :draft_id, :integer
+  end
+
+  def self.down
+    remove_column :nodes, :draft_id
+  end
+end
diff --git a/test/fixtures/nodes.yml b/test/fixtures/nodes.yml
index 642d937..8574c1b 100644
--- a/test/fixtures/nodes.yml
+++ b/test/fixtures/nodes.yml
@@ -14,6 +14,7 @@ first_child:
   lft: 2
   rgt: 3
   parent_id: 1
+  draft_id: 100
   slug: first_child
   unique_name: first_child
 
@@ -22,6 +23,7 @@ second_child:
   lft: 4
   rgt: 5
   parent_id: 1
+  draft_id: 101
   slug: second_child
   unique_name: second_child
   
@@ -30,5 +32,6 @@ third_child:
   lft: 6
   rgt: 7
   parent_id: 1
+  draft_id: 102
   slug: third_child
   unique_name: third_child
\ No newline at end of file
diff --git a/test/fixtures/pages.yml b/test/fixtures/pages.yml
index 614b9a6..8171b38 100644
--- a/test/fixtures/pages.yml
+++ b/test/fixtures/pages.yml
@@ -8,4 +8,19 @@ one:
 two:
   id: 2
   revision: 1
+  node_id: 4
+  
+draft1:
+  id: 100
+  revision: 1
+  node_id: 2
+  
+draft2:
+  id: 101
+  revision: 1
+  node_id: 3
+  
+draft3:
+  id: 102
+  revision: 1
   node_id: 4
\ No newline at end of file
diff --git a/test/functional/nodes_controller_test.rb b/test/functional/nodes_controller_test.rb
index d6d579d..85f151a 100644
--- a/test/functional/nodes_controller_test.rb
+++ b/test/functional/nodes_controller_test.rb
@@ -1,8 +1,21 @@
 require 'test_helper'
 
 class NodesControllerTest < ActionController::TestCase
-  # Replace this with your real tests.
-  test "the truth" do
-    assert true
+
+  include AuthenticatedTestHelper
+
+  def test_get_index
+    login_as :quentin
+    get :index
+    assert_response :success
+  end
+  
+  def test_update_a_draft
+    test_node = Node.create! :slug => "test_node"
+    test_node.move_to_child_of Node.root
+    
+    login_as :quentin
+    
+    
   end
 end
diff --git a/test/unit/node_test.rb b/test/unit/node_test.rb
index 549cccd..2558ca3 100644
--- a/test/unit/node_test.rb
+++ b/test/unit/node_test.rb
@@ -11,6 +11,21 @@ class NodeTest < ActiveSupport::TestCase
     @user2 = User.create :login => 'show', :email => "f@b.com", :password => 'foobar', :password_confirmation => 'foobar'
   end
   
+  def test_returning_existing_drafts
+    test_node = Node.create! :slug => "test_node"
+    test_node.move_to_child_of Node.root
+    
+    assert_not_nil test_node.draft
+    assert_equal 1, test_node.pages.length
+    assert_nil test_node.draft.user
+    
+    3.times do 
+      test_node.find_or_create_draft @user1
+    end
+    
+    assert_equal 1, test_node.pages.length
+  end
+  
   def test_user_gets_assigned_to_unlocked_draft
     assert_not_nil @first_child.draft
     assert_nil @first_child.draft.user
@@ -34,6 +49,7 @@ class NodeTest < ActiveSupport::TestCase
   end
   
   def test_publish_draft_on_a_node_without_a_draft_returns_nil
+    
     assert @first_child.publish_draft!
     assert_nil @first_child.publish_draft!
   end
@@ -127,34 +143,34 @@ class NodeTest < ActiveSupport::TestCase
   
   def test_order_of_pages_by_revision
     # This test should make sure the order is the same on different db's
-
-    one   = @second_child.pages.create :title => "one"
+    # Remember, there is already an empty draft
     two   = @second_child.pages.create :title => "two"
     three = @second_child.pages.create :title => "three"
+    four  = @second_child.pages.create :title => "four"
 
     @second_child.pages.reload
 
-    assert_equal [1,2,3], @second_child.pages.map { |x| x.revision }
+    assert_equal [1,2,3,4], @second_child.pages.map { |x| x.revision }
   end
   
   def test_behavior_of_acts_as_list
-    one   = @second_child.pages.create :title => "one"
     two   = @second_child.pages.create :title => "two"
     three = @second_child.pages.create :title => "three"
+    four  = @second_child.pages.create :title => "four"
 
-    assert_equal 1, one.revision
     assert_equal 2, two.revision
     assert_equal 3, three.revision
+    assert_equal 4, four.revision
 
-    assert_equal three, @second_child.pages.last
+    assert_equal four, @second_child.pages.last
 
-    assert one.move_to_bottom
+    assert two.move_to_bottom
 
-    one.reload; two.reload; three.reload;
+    two.reload; three.reload; four.reload;
 
-    assert_equal 3, one.revision
-    assert_equal 1, two.revision
+    assert_equal 4, two.revision
     assert_equal 2, three.revision
+    assert_equal 3, four.revision
   end
   
   def test_retrieving_page_current
@@ -201,4 +217,23 @@ class NodeTest < ActiveSupport::TestCase
     page = Node.find_page("updates/2008/foo", 2)
     assert_equal "Version 2", page.title
   end
+  
+  # Thats a lengthy test to make sure everything works as it should, it was 
+  # created during a bug hunt
+  def test_creating_new_draft
+    test_node = Node.create! :slug => "test_node"
+    test_node.move_to_child_of Node.root
+    test_node.draft.user = @user1
+    test_node.save
+    assert test_node.publish_draft!
+    test_node.reload
+    assert_equal 1, test_node.pages.length
+    assert_not_nil test_node.head
+    assert_nil test_node.draft
+    test_node.find_or_create_draft @user1
+    test_node.reload
+    assert_equal 2, test_node.pages.length
+    assert_not_nil test_node.draft
+    assert test_node.head != test_node.draft
+  end
 end