From c7757522c2750675fab6c4324453e237b863d84c Mon Sep 17 00:00:00 2001 From: hukl Date: Sun, 1 Mar 2009 13:56:24 +0100 Subject: 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. --- app/controllers/nodes_controller.rb | 3 +- app/models/node.rb | 35 +++++++------- db/migrate/20090301104237_add_draft_id_to_nodes.rb | 9 ++++ test/fixtures/nodes.yml | 3 ++ test/fixtures/pages.yml | 15 ++++++ test/functional/nodes_controller_test.rb | 19 ++++++-- test/unit/node_test.rb | 55 ++++++++++++++++++---- 7 files changed, 106 insertions(+), 33 deletions(-) create mode 100644 db/migrate/20090301104237_add_draft_id_to_nodes.rb 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 -- cgit v1.3