From c7c4b007621fd28c88b65db5fd3296ef730097d9 Mon Sep 17 00:00:00 2001 From: hukl Date: Sat, 17 Oct 2009 23:31:28 +0200 Subject: added some more validations to ensure data integrity and applied necessary changes on tests --- app/models/node.rb | 11 +++- test/functional/content_controller_test.rb | 3 +- test/functional/nodes_controller_test.rb | 102 ++++++++++++++++++++++------- test/test_helper.rb | 14 ++++ test/unit/event_test.rb | 3 +- test/unit/node_test.rb | 59 +++++------------ test/unit/page_test.rb | 15 ++--- 7 files changed, 125 insertions(+), 82 deletions(-) diff --git a/app/models/node.rb b/app/models/node.rb index 6587585..88a4d68 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -16,9 +16,12 @@ class Node < ActiveRecord::Base after_save :update_unique_names_of_children # Validations - validates_length_of :slug, :within => 1..255, :unless => "parent_id.nil?" + validates_length_of :slug, :within => 1..255, :unless => "parent_id.nil?" validates_presence_of :slug, :unless => "parent_id.nil?" validates_uniqueness_of :slug, :scope => :parent_id, :unless => "parent_id.nil?" + validates_presence_of :parent_id, :unless => "Node.root.nil?" + + validate :borders # This should never ever happen. # Index for Fulltext Search define_index do @@ -183,6 +186,12 @@ class Node < ActiveRecord::Base descendant.update_unique_name end end + + def borders + if lft && rgt && (lft > rgt) + errors.add("Fuck!. lft should never be smaller than rgt") + end + end end class LockedByAnotherUser < StandardError; end diff --git a/test/functional/content_controller_test.rb b/test/functional/content_controller_test.rb index b76e1d5..4fb3035 100644 --- a/test/functional/content_controller_test.rb +++ b/test/functional/content_controller_test.rb @@ -94,8 +94,7 @@ class ContentControllerTest < ActionController::TestCase protected def create_node_under_root slug - node = Node.create! :slug => slug - node.move_to_child_of Node.root + node = Node.root.children.create! :slug => slug node end diff --git a/test/functional/nodes_controller_test.rb b/test/functional/nodes_controller_test.rb index eca7d93..f8d42da 100644 --- a/test/functional/nodes_controller_test.rb +++ b/test/functional/nodes_controller_test.rb @@ -4,8 +4,7 @@ class NodesControllerTest < ActionController::TestCase def test_get_index Node.root.descendants.delete_all - test_node = Node.create :slug => "foo" - test_node.move_to_child_of Node.root + test_node = Node.root.children.create :slug => "foo" login_as :quentin get :index assert_response :success @@ -111,9 +110,22 @@ class NodesControllerTest < ActionController::TestCase assert_equal "World", node.find_or_create_draft( User.first ).body end + test "editing a locked node raises LockedByAnotherUser Exception" do + login_as :quentin + + node = create_node_with_draft + node.lock_owner = User.last + node.save + + assert node.locked? + + get :edit, :id => node.id + assert_response :redirect + assert @response.flash[:error] =~ /Page is locked by another user/ + end + def test_update_a_draft - test_node = Node.create! :slug => "test_node" - test_node.move_to_child_of Node.root + test_node = Node.root.children.create! :slug => "test_node" login_as :quentin put :update, :id => test_node.id, :page => {:title => "Hello", :body => "There"} @@ -123,8 +135,7 @@ class NodesControllerTest < ActionController::TestCase end def test_update_a_draft_with_changing_the_template - test_node = Node.create! :slug => "test_node" - test_node.move_to_child_of Node.root + test_node = Node.root.children.create! :slug => "test_node" login_as :quentin put :update, { @@ -146,8 +157,7 @@ class NodesControllerTest < ActionController::TestCase test "publish draft with staged_slug unqueal slug" do login_as :quentin - test_node = Node.create! :slug => "test_node", :staged_slug => "peter_pan" - test_node.move_to_child_of Node.root + test_node = Node.root.children.create! :slug => "test_node", :staged_slug => "peter_pan" put :publish, :id => test_node.id @@ -159,10 +169,8 @@ class NodesControllerTest < ActionController::TestCase test "publish draft with staged_slug with more levels of nodes" do login_as :quentin - test_node = Node.create! :slug => "test_node", :staged_slug => "peter_pan" - test_node.move_to_child_of Node.root - test_node2 = Node.create! :slug => "test_node2" - test_node2.move_to_child_of test_node + test_node = Node.root.children.create! :slug => "test_node", :staged_slug => "peter_pan" + test_node2 = test_node.children.create! :slug => "test_node2" put :publish, :id => test_node.id @@ -174,13 +182,10 @@ class NodesControllerTest < ActionController::TestCase test "publish draft with staged_parent_id" do login_as :quentin - parent = Node.create! :slug => "parent" - parent.move_to_child_of Node.root - test_node = Node.create! :slug => "test_node", :staged_parent_id => parent.id - test_node.move_to_child_of Node.root - test_node2 = Node.create! :slug => "test_node2" - test_node2.move_to_child_of test_node - + parent = Node.root.children.create! :slug => "parent" + test_node = Node.root.children.create! :slug => "test_node", :staged_parent_id => parent.id + test_node2 = test_node.children.create! :slug => "test_node2" + put :publish, :id => test_node.id test_node.reload; test_node2.reload @@ -191,18 +196,15 @@ class NodesControllerTest < ActionController::TestCase test "publish draft with staged_parent_id and staged_slug" do login_as :quentin - parent = Node.create! :slug => "parent" - parent.move_to_child_of Node.root + parent = Node.root.children.create! :slug => "parent" - test_node = Node.create!( + test_node = Node.root.children.create!( :slug => "test_node", :staged_parent_id => parent.id, :staged_slug => "peter_pan" ) - test_node.move_to_child_of Node.root - test_node2 = Node.create! :slug => "test_node2" - test_node2.move_to_child_of test_node + test_node2 = test_node.children.create! :slug => "test_node2" put :publish, :id => test_node.id @@ -211,4 +213,54 @@ class NodesControllerTest < ActionController::TestCase assert_equal "parent/peter_pan/test_node2", test_node2.unique_name end + test "show node with empty draft" do + login_as :quentin + assert_not_nil node = create_node_with_draft + get :show, :id => node.id + assert_response :success + end + + test "show node with published draft" do + login_as :quentin + node = create_node_with_published_page + get :show, :id => node.id + assert_response :success + assert_select "td", :text => "Test", :count => 3 + end + + test "unlocking a locked node" do + login_as :quentin + node = create_node_with_published_page + node.find_or_create_draft User.first + + assert node.locked? + + get :unlock, :id => node.id + assert_response :redirect + assert !node.reload.locked? + end + + test "unlocking an already unlocked node" do + login_as :quentin + node = create_node_with_published_page + + get :unlock, :id => node.id + assert_response :redirect + assert_equal "Already unlocked", @response.flash[:notice] + end + + test "updating a node by changing its parent" do + Node.root.descendants.destroy_all + login_as :quentin + node = create_node_with_published_page + node.find_or_create_draft User.first + + other_node = Node.root.children.create( :slug => "other" ) + + node.staged_parent_id = other_node.id + node.publish_draft! + + assert Node.valid? + + end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 21d4604..023ed96 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -38,4 +38,18 @@ class ActiveSupport::TestCase fixtures :all # Add more helper methods to be used by all tests here... + + def create_node_with_published_page + node = create_node_with_draft + draft = node.draft + draft.title = "Test" + draft.abstract = "Test" + draft.body = "Test" + node.publish_draft! + node + end + + def create_node_with_draft + Node.root.children.create :slug => "test_node" + end end diff --git a/test/unit/event_test.rb b/test/unit/event_test.rb index 78e082c..f310af8 100644 --- a/test/unit/event_test.rb +++ b/test/unit/event_test.rb @@ -4,8 +4,7 @@ class EventTest < ActiveSupport::TestCase def setup Page.delete_all - @cal_node = Node.create :slug => "calendar" - @cal_node.move_to_child_of Node.root + @cal_node = Node.root.children.create! :slug => "calendar" @draft = @cal_node.find_or_create_draft User.first @draft.title = "99C3" @draft.abstract = "The 99th Chaos Comunication Congress" diff --git a/test/unit/node_test.rb b/test/unit/node_test.rb index ee4d71f..92870aa 100644 --- a/test/unit/node_test.rb +++ b/test/unit/node_test.rb @@ -16,8 +16,7 @@ class NodeTest < ActiveSupport::TestCase end def test_returning_existing_drafts - test_node = Node.create! :slug => "test_node" - test_node.move_to_child_of Node.root + test_node = Node.root.children.create! :slug => "test_node" assert_not_nil test_node.draft assert_equal 1, test_node.pages.length @@ -39,10 +38,7 @@ class NodeTest < ActiveSupport::TestCase def test_unique_path_returns_an_array assert_equal ["first_child"], @first_child.unique_path - - new_node = Node.create! :slug => "third_child" - new_node.move_to_child_of @first_child - + new_node = @first_child.children.create! :slug => "third_child" assert_equal ["first_child", "third_child"], new_node.unique_path end @@ -87,8 +83,7 @@ class NodeTest < ActiveSupport::TestCase end def test_created_nodes_have_an_empty_draft_and_no_head - node = Node.create :slug => "third_child" - node.move_to_child_of @root + node = Node.root.children.create! :slug => "third_child_beta" assert !node.pages.empty? assert_equal 1, node.pages.length @@ -98,23 +93,18 @@ class NodeTest < ActiveSupport::TestCase end def test_create_new_draft_of_published_page - node = Node.create :slug => "xyz" - node.move_to_child_of @root - + node = Node.root.children.create :slug => "xyz" assert node.publish_draft! end def test_find_or_create_draft_if_no_draft_exists - node = Node.create :slug => "xyz" - node.move_to_child_of @root + node = Node.root.children.create :slug => "xyz" node.publish_draft! - assert_not_nil node.find_or_create_draft( @user1 ) end def test_find_or_create_draft_if_draft_exists_and_is_owned_by_user - node = Node.create :slug => "xyz" - node.move_to_child_of @root + node = Node.root.children.create :slug => "xyz" node.publish_draft! node.find_or_create_draft @user1 @@ -122,8 +112,7 @@ class NodeTest < ActiveSupport::TestCase end def test_exception_if_draft_exists_but_locked_by_another_user - node = Node.create :slug => "xyz" - node.move_to_child_of @root + node = Node.root.children.create :slug => "xyz" node.publish_draft! node.find_or_create_draft @user1 assert_equal @user1, node.lock_owner @@ -133,13 +122,11 @@ class NodeTest < ActiveSupport::TestCase end def test_creation_of_unique_name - node = Node.create :slug => 'child' - node.move_to_child_of @root + node = Node.root.children.create :slug => 'child' node.reload assert_equal 'child', node.unique_name - node = Node.create :slug => 'deep_child' - node.move_to_child_of @first_child + node = @first_child.children.create :slug => 'deep_child' node.reload assert_equal 'first_child/deep_child', node.unique_name end @@ -177,14 +164,9 @@ class NodeTest < ActiveSupport::TestCase end def test_retrieving_page_current - updates = Node.create(:slug => 'updates') - updates.move_to_child_of @root - - year = Node.create(:slug => '2008') - year.move_to_child_of updates - - foo = Node.create(:slug => 'foo') - foo.move_to_child_of year + updates = Node.root.children.create(:slug => 'updates') + year = updates.children.create(:slug => '2008') + foo = year.children.create(:slug => 'foo') assert_not_nil Node.find_by_unique_name('updates/2008/foo') @@ -201,14 +183,9 @@ class NodeTest < ActiveSupport::TestCase end def test_retrieving_page_by_revision - updates = Node.create(:slug => 'updates') - updates.move_to_child_of @root - - year = Node.create(:slug => '2008') - year.move_to_child_of updates - - foo = Node.create(:slug => 'foo') - foo.move_to_child_of year + updates = Node.root.children.create(:slug => 'updates') + year = updates.children.create(:slug => '2008') + foo = year.children.create(:slug => 'foo') assert_not_nil Node.find_by_unique_name('updates/2008/foo') @@ -224,8 +201,7 @@ class NodeTest < ActiveSupport::TestCase # 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 = Node.root.children.create! :slug => "test_node" test_node.draft.user = @user1 test_node.save assert test_node.publish_draft! @@ -241,8 +217,7 @@ class NodeTest < ActiveSupport::TestCase end test "restoring a revision" do - test_node = Node.create! :slug => "test_node" - test_node.move_to_child_of Node.root + test_node = Node.root.children.create! :slug => "test_node" create_revisions( test_node, 3 ) test_node.find_or_create_draft @user1 test_node.reload diff --git a/test/unit/page_test.rb b/test/unit/page_test.rb index a2083c0..c2599e3 100644 --- a/test/unit/page_test.rb +++ b/test/unit/page_test.rb @@ -9,10 +9,8 @@ class PageTest < ActiveSupport::TestCase def test_aggregation # Create two nodes and move them beneath the root node - n1 = Node.create! :slug => "one" - n2 = Node.create! :slug => "two" - n1.move_to_child_of Node.root - n2.move_to_child_of Node.root + n1 = Node.root.children.create! :slug => "one" + n2 = Node.root.children.create! :slug => "two" # get the drafts and assign a user to it assert_not_nil d1 = n1.find_or_create_draft( @user1 ) @@ -50,8 +48,7 @@ class PageTest < ActiveSupport::TestCase end def test_before_save_rewrite_links_in_body - n = Node.create :slug => "link_test" - n.move_to_child_of Node.root + n = Node.root.children.create :slug => "link_test" d = n.find_or_create_draft @user1 before = "

Hello World

\n" \ @@ -69,8 +66,7 @@ class PageTest < ActiveSupport::TestCase end def test_before_save_rewrite_links_in_body_if_no_locale_prefix_present - n = Node.create :slug => "link_test" - n.move_to_child_of Node.root + n = Node.root.children.create :slug => "link_test" d = n.find_or_create_draft @user1 before = "

Hello World

\n" \ @@ -88,8 +84,7 @@ class PageTest < ActiveSupport::TestCase end def test_before_save_rewrite_links_skips_on_external_links - n = Node.create :slug => "link_test" - n.move_to_child_of Node.root + n = Node.root.children.create :slug => "link_test" d = n.find_or_create_draft @user1 before = "

Hello World

\n" \ -- cgit v1.3