From 8980b38f82ba92cba3542bb68dfc0c3c43a7864b Mon Sep 17 00:00:00 2001 From: Jaime Bellmyer Date: Fri, 11 Dec 2009 13:29:18 -0600 Subject: [PATCH] fixed PagesController tests tested that all actions except 'show' succeed as admin, return unauthorized as members (non-admins) and redirect to login for visitors (not logged in). This uncovered a glitch where visitors weren't required to be logged on for pages, but *were* required to have an admin role. Show action should succeed for admins, members, and visitors. Fixed Page.pages_menu to return [] instead of nil when it finds an empty set. This fixed a view that was relying on an array. Fixed index test to check for viewable, editable, and parents arrays, instead of the default pages array. --- app/controllers/pages_controller.rb | 14 +-- app/models/page.rb | 2 +- test/fixtures/pages.yml | 10 +- test/functional/pages_controller_test.rb | 134 ++++++++++++++++++++--- 4 files changed, 135 insertions(+), 25 deletions(-) diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 05bcd0c..6871f17 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -2,7 +2,7 @@ class PagesController < ApplicationController before_filter :login_required, :except => [:show] cache_sweeper :page_sweeper, :only => [:create, :update, :destroy] - require_role "admin" + require_role "admin", :except => [:show] def index @viewable = Role.pages_for_viewable_by @@ -16,12 +16,12 @@ def new end def show - if params[:id] != "login" - @page = Page.find(params[:id]) - redirect_to resources_path(@page) - else - redirect_to home_url - end + if params[:id] != "login" + @page = Page.find(params[:id]) + redirect_to resources_path(@page) + else + redirect_to home_url + end end def edit diff --git a/app/models/page.rb b/app/models/page.rb index 5f918a8..b3d52ff 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -19,7 +19,7 @@ def self.cached_menu_pages end def self.pages_menu(type="primary") - all_menu_pages.group_by { |p| p[:menu_type] }[type] + all_menu_pages.group_by { |p| p[:menu_type] }[type] || [] end def self.all_menu_pages diff --git a/test/fixtures/pages.yml b/test/fixtures/pages.yml index d8897fb..953c465 100644 --- a/test/fixtures/pages.yml +++ b/test/fixtures/pages.yml @@ -1,13 +1,17 @@ # Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html one: - title: MyString - name: MyString - description: MyString + id: 1 + title: public + name: public + description: public page + viewable_by: public parent_id: 1 two: + id: 2 title: MyString name: MyString description: MyString + viewable_by: public parent_id: 1 diff --git a/test/functional/pages_controller_test.rb b/test/functional/pages_controller_test.rb index 66415ab..cd19e4a 100644 --- a/test/functional/pages_controller_test.rb +++ b/test/functional/pages_controller_test.rb @@ -1,45 +1,151 @@ require 'test_helper' class PagesControllerTest < ActionController::TestCase - def test_should_get_index + include AuthenticatedTestHelper + + def setup + @valid = {:title => 'Page', :name => 'Page'} + end + + def test_as_admin_getting_index_should_succeed + login_as :admin get :index assert_response :success - assert_not_nil assigns(:pages) + + assert_not_nil assigns(:viewable) + assert assigns(:viewable).is_a?(Array) + + assert_not_nil assigns(:editable) + assert assigns(:editable).is_a?(Array) + + assert_not_nil assigns(:parents) + assert assigns(:parents).is_a?(Array) + assert_equal ['parent_0', 'parent_00'], assigns(:parents)[0,2] + end + + def test_as_member_getting_index_should_not_be_authorized + login_as :quentin + get :index + assert_response 401 + end + + def test_as_visitor_getting_index_should_be_redirected_to_login + get :index + assert_redirected_to new_session_path end - def test_should_get_new + def test_as_admin_getting_new_should_succeed + login_as :admin get :new assert_response :success end + + def test_as_member_getting_new_should_not_be_authorized + login_as :quentin + get :new + assert_response 401 + end + + def test_as_visitor_getting_new_should_be_redirected_to_login + get :new + assert_redirected_to new_session_path + end - def test_should_create_page + def test_as_admin_creating_page_should_succeed + login_as :admin assert_difference('Page.count') do - post :create, :page => { } + post :create, :page => @valid end - assert_redirected_to page_path(assigns(:page)) + assert_redirected_to resources_path(assigns(:page)) + end + + def test_as_member_creating_page_should_not_be_authorized + login_as :quentin + post :create, :page => @valid + assert_response 401 end - def test_should_show_page + def test_as_visitor_creating_page_should_be_redirected_to_login + post :create, :page => @valid + assert_redirected_to new_session_path + end + + def test_as_admin_showing_page_should_succeed + login_as :admin get :show, :id => pages(:one).id - assert_response :success + assert_redirected_to resources_path(pages(:one)) end - def test_should_get_edit + def test_as_user_showing_page_should_succeed + login_as :quentin + get :show, :id => pages(:one).id + assert_redirected_to resources_path(pages(:one)) + end + + def test_as_visitor_showing_page_should_succeed + get :show, :id => pages(:one).id + assert_redirected_to resources_path(pages(:one)) + end + + def test_as_admin_getting_edit_should_succeed + login_as :admin get :edit, :id => pages(:one).id assert_response :success end - def test_should_update_page + def test_as_member_getting_edit_should_not_be_authorized + login_as :quentin + get :edit, :id => 1 + assert_response 401 + end + + def test_as_visitor_getting_edit_should_be_redirected_to_login + get :edit, :id => 1 + assert_redirected_to new_session_path + end + + def test_as_admin_updating_page_should_succeed + login_as :admin + put :update, :id => pages(:one).id, :page => { } + assert_redirected_to resources_path(assigns(:page)) + end + + def test_as_user_updating_page_should_not_be_authorized + login_as :quentin + put :update, :id => pages(:one).id, :page => { } + assert_response 401 + end + + def test_as_visitor_updating_page_should_redirect_to_login put :update, :id => pages(:one).id, :page => { } - assert_redirected_to page_path(assigns(:page)) + assert_redirected_to new_session_path end - def test_should_destroy_page + def test_as_admin_destroying_page_should_succeed + login_as :admin assert_difference('Page.count', -1) do - delete :destroy, :id => pages(:one).id + delete :destroy, :id => pages(:two).id end - assert_redirected_to pages_path + assert_redirected_to resources_path(pages(:one)) + end + + def test_as_member_destroying_page_should_not_be_authorized + login_as :quentin + delete :destroy, :id => 1 + assert_response 401 + end + + def test_as_admin_destroying_page_should_succeed + delete :destroy, :id => 1 + assert_redirected_to new_session_path + end + + protected + + def resources_path(page) + eval ("#{page.kind}_path(#{page.id})") end + end