code-dot-org · ashercodeorg · Jul 6, 2016 · Jun 24, 2016 · Jun 24, 2016 · Jun 27, 2016
diff --git a/dashboard/app/controllers/levels_controller.rb b/dashboard/app/controllers/levels_controller.rb
@@ -113,7 +113,7 @@ def update
   # POST /levels
   # POST /levels.json
   def create
-    authorize! :create, :level
+    authorize! :create, Level
     type_class = level_params[:type].constantize
 
     # Set some defaults.
@@ -153,7 +153,7 @@ def destroy
   end
 
   def new
-    authorize! :create, :level
+    authorize! :create, Level
     if params.key? :type
       @type_class = params[:type].constantize
       if @type_class == Artist

diff --git a/dashboard/app/models/ability.rb b/dashboard/app/models/ability.rb
@@ -171,6 +171,7 @@ def initialize(user)
     # permissions.
     if user.persisted? && user.permission?(UserPermission::LEVELBUILDER)
       can :manage, [
+        Game,
         Level,
         Script,
         ScriptLevel
@@ -185,10 +186,15 @@ def initialize(user)
     if user.admin?
       can :manage, :all
 
-      # Only custom levels are editable
-      cannot [:update, :destroy], Level do |level|
-        !level.custom?
-      end
+      cannot :manage, [
+        Activity,
+        Game,
+        Level,
+        Script,
+        ScriptLevel,
+        UserLevel,
+        UserScript
+      ]
     end
   end
 end
diff --git a/dashboard/test/controllers/levels_controller_test.rb b/dashboard/test/controllers/levels_controller_test.rb
@@ -8,11 +8,11 @@ class LevelsControllerTest < ActionController::TestCase
     Level.any_instance.stubs(:write_to_file?).returns(false) # don't write to level files
 
     @level = create(:level)
-    @user = create(:admin)
-    sign_in(@user)
-    @program = '<hey/>'
-
+    @admin = create(:admin)
     @not_admin = create(:user)
+    @levelbuilder = create(:levelbuilder)
+    sign_in(@levelbuilder)
+    @program = '<hey/>'
   end
 
   test "should get index" do
@@ -48,22 +48,24 @@ class LevelsControllerTest < ActionController::TestCase
   end
 
   test "should alphanumeric order custom levels on new" do
-    Level.where(user_id: @user.id).map(&:destroy)
-    level_1 = create(:level, user: @user, name: "BBBB")
-    level_2 = create(:level, user: @user, name: "AAAA")
-    level_3 = create(:level, user: @user, name: "Z1")
-    level_4 = create(:level, user: @user, name: "Z10")
-    level_5 = create(:level, user: @user, name: "Z2")
+    Level.where(user_id: @levelbuilder.id).map(&:destroy)
+    level_1 = create(:level, user: @levelbuilder, name: "BBBB")
+    level_2 = create(:level, user: @levelbuilder, name: "AAAA")
+    level_3 = create(:level, user: @levelbuilder, name: "Z1")
+    level_4 = create(:level, user: @levelbuilder, name: "Z10")
+    level_5 = create(:level, user: @levelbuilder, name: "Z2")
 
     get :new, game_id: @level.game
 
     assert_equal [level_2, level_1, level_3, level_5, level_4], assigns(:levels)
   end
 
-  test "should not get builder if not admin" do
-    sign_in @not_admin
-    get :new, game_id: @level.game
-    assert_response :forbidden
+  test "should not get builder if not levelbuilder" do
+    [@not_admin, @admin].each do |user|
+      sign_in user
+      get :new, game_id: @level.game
+      assert_response :forbidden
+    end
   end
 
   test "should create maze level" do
@@ -136,7 +138,7 @@ class LevelsControllerTest < ActionController::TestCase
 
     assert assigns(:level)
     assert assigns(:level).game
-    assert_equal @user, assigns(:level).user
+    assert_equal @levelbuilder, assigns(:level).user
 
     assert_equal edit_level_path(assigns(:level)), JSON.parse(@response.body)["redirect"]
   end
@@ -203,28 +205,32 @@ class LevelsControllerTest < ActionController::TestCase
     assert_equal level.properties[:toolbox_blocks.to_s], @program
   end
 
-  test "should not update blocks if not admin" do
-    sign_in @not_admin
-    post :update_blocks, :level_id => @level.id, :game_id => @level.game.id, :type => 'toolbox_blocks', :program => @program
-    assert_response :forbidden
+  test "should not update blocks if not levelbuilder" do
+    [@not_admin, @admin].each do |user|
+      sign_in user
+      post :update_blocks, :level_id => @level.id, :game_id => @level.game.id, :type => 'toolbox_blocks', :program => @program
+      assert_response :forbidden
+    end
   end
 
   test "should not edit level if not custom level" do
     level = Script.twenty_hour_script.levels.first
-    can_edit =  Ability.new(@user).can? :edit, level
+    can_edit =  Ability.new(@levelbuilder).can? :edit, level
     assert_equal false, can_edit
 
     post :update_blocks, :level_id => level.id, :game_id => level.game.id, :type => 'toolbox_blocks', :program => @program
     assert_response :forbidden
   end
 
-  test "should not create level if not admin" do
-    sign_in @not_admin
-    assert_no_difference('Level.count') do
-      post :create, :name => "NewCustomLevel", :program => @program
-    end
+  test "should not create level if not levelbuilder" do
+    [@not_admin, @admin].each do |user|
+      sign_in user
+      assert_no_difference('Level.count') do
+        post :create, :name => "NewCustomLevel", :program => @program
+      end
 
-    assert_response :forbidden
+      assert_response :forbidden
+    end
   end
 
   # This should represent the behavior on production.
@@ -415,7 +421,7 @@ class LevelsControllerTest < ActionController::TestCase
   test 'should hide legacy unplugged pdf download button for students' do
     level = create :unplugged, name: 'OldUnplugged', type: 'Unplugged'
     teacher = create(:teacher)
-    sign_out(@user)
+    sign_out(@levelbuilder)
     sign_in(teacher)
     get :show, id: level, game_id: level.game
     assert_select '.pdf-button'
@@ -439,7 +445,7 @@ class LevelsControllerTest < ActionController::TestCase
   test 'should hide unplugged pdf download section for students' do
     level = create :unplugged, name: 'NewUnplugged', type: 'Unplugged'
     teacher = create(:teacher)
-    sign_out(@user)
+    sign_out(@levelbuilder)
     sign_in(teacher)
     get :show, id: level, game_id: level.game
     assert_select '.pdf-button'
@@ -520,7 +526,7 @@ class LevelsControllerTest < ActionController::TestCase
     set_env :test
 
     level = create :artist
-    sign_out @user
+    sign_out @levelbuilder
 
     get :edit, id: level
     assert_response :redirect
@@ -533,7 +539,7 @@ class LevelsControllerTest < ActionController::TestCase
     set_env :test
 
     level = create :artist
-    sign_out @user
+    sign_out @levelbuilder
 
     get :embed_level, level_id: level
     assert_response :success
@@ -543,7 +549,7 @@ class LevelsControllerTest < ActionController::TestCase
     set_env :test
 
     level = create :artist
-    sign_out @user
+    sign_out @levelbuilder
 
     get :embed_blocks, level_id: level, block_type: :solution_blocks
     assert_response :success

diff --git a/dashboard/test/controllers/script_levels_controller_test.rb b/dashboard/test/controllers/script_levels_controller_test.rb
@@ -753,23 +753,23 @@ def assert_caching_enabled(cache_control_header, max_age, proxy_max_age)
   end
 
   test 'includes makerlab javascript dependencies when makerlab level' do
-    sign_in @admin
+    sign_in @teacher
 
     level = create :makerlab
     script_level = create :script_level, levels: [level]
 
-    get :show, script_id: script_level.script, stage_id: script_level.stage, id: script_level.position, user_id: @admin.id
+    get :show, script_id: script_level.script, stage_id: script_level.stage, id: script_level.position, user_id: @teacher.id
 
     assert_select 'script[src=?]', ActionController::Base.helpers.javascript_path('js/makerlab')
   end
 
   test 'excludes makerlab javascript dependencies when applab level' do
-    sign_in @admin
+    sign_in @teacher
 
     level = create :applab
     script_level = create :script_level, levels: [level]
 
-    get :show, script_id: script_level.script, stage_id: script_level.stage, id: script_level.position, user_id: @admin.id
+    get :show, script_id: script_level.script, stage_id: script_level.stage, id: script_level.position, user_id: @teacher.id
 
     assert_select 'script[src=?]', ActionController::Base.helpers.javascript_path('js/makerlab'), false
   end
@@ -962,12 +962,15 @@ def create_admin_script
     assert_response :forbidden
   end
 
-  test "should get show of admin script if signed in as admin" do
+  # TODO(asher): Consolidate the tests when the user is an admin. In particular,
+  # these scripts should no longer exist, should be accessible to noone, and
+  # will go away.
+  test "should not get show of admin script if signed in as admin" do
     admin_script = create_admin_script
 
     sign_in create(:admin)
     get :show, script_id: admin_script.name, stage_id: 1, id: 1
-    assert_response :success
+    assert_response :forbidden
   end
 
   def create_student_of_admin_script
@@ -999,12 +1002,12 @@ def create_student_of_admin_script
     assert_response :success
   end
 
-  test "should get show of student_of_admin script if signed in as admin" do
+  test "should not get show of student_of_admin script if signed in as admin" do
     script = create_student_of_admin_script
 
     sign_in create(:admin)
     get :show, script_id: script.name, stage_id: 1, id: 1
-    assert_response :success
+    assert_response :forbidden
   end
 
   test "should have milestone posting disabled if Milestone is set" do

diff --git a/dashboard/test/controllers/scripts_controller_test.rb b/dashboard/test/controllers/scripts_controller_test.rb
@@ -7,14 +7,15 @@ class ScriptsControllerTest < ActionController::TestCase
   setup do
     @admin = create(:admin)
     @not_admin = create(:user)
+    @levelbuilder = create(:levelbuilder)
 
     Rails.application.config.stubs(:levelbuilder_mode).returns false
   end
 
   test "should get index" do
     Rails.application.config.stubs(:levelbuilder_mode).returns true
 
-    sign_in(@admin)
+    sign_in(@levelbuilder)
     get :index
     assert_response :success
     assert_not_nil assigns(:scripts)
@@ -27,12 +28,14 @@ class ScriptsControllerTest < ActionController::TestCase
     assert_redirected_to_sign_in
   end
 
-  test "should not get index if not admin" do
-    sign_in @not_admin
+  test "should not get index if not levelbuilder" do
+    [@admin, @not_admin].each do |user|
+      sign_in user
 
-    get :index
+      get :index
 
-    assert_response :forbidden
+      assert_response :forbidden
+    end
   end
 
   test "should get show of hoc" do
@@ -125,6 +128,12 @@ class ScriptsControllerTest < ActionController::TestCase
     assert_response :success
   end
 
+  test 'should not get show if admin' do
+    sign_in @admin
+    get :show, id: Script::FLAPPY_NAME
+    assert_response :forbidden
+  end
+
   test "should use script name as param where script name is words but looks like a number" do
     script = create(:script, name: '15-16')
     get :show, id: "15-16"
@@ -153,9 +162,9 @@ class ScriptsControllerTest < ActionController::TestCase
     end
   end
 
-  test "should not get edit if not levelbuilder" do
+  test "should not get edit if not levelbuilder mode" do
     Rails.application.config.stubs(:levelbuilder_mode).returns false
-    sign_in @admin
+    sign_in @levelbuilder
     get :edit, id: 'course1'
 
     assert_response :forbidden
@@ -168,17 +177,19 @@ class ScriptsControllerTest < ActionController::TestCase
     assert_redirected_to_sign_in
   end
 
-  test "should not get edit if not admin" do
+  test "should not get edit if not levelbuilder" do
     Rails.application.config.stubs(:levelbuilder_mode).returns true
-    sign_in @not_admin
-    get :edit, id: 'course1'
+    [@not_admin, @admin].each do |user|
+      sign_in user
+      get :edit, id: 'course1'
 
-    assert_response :forbidden
+      assert_response :forbidden
+    end
   end
 
   test "edit" do
     Rails.application.config.stubs(:levelbuilder_mode).returns true
-    sign_in @admin
+    sign_in @levelbuilder
     script = Script.find_by_name('course1')
     get :edit, id: script.name
 
@@ -200,7 +211,7 @@ class ScriptsControllerTest < ActionController::TestCase
   end
 
   test "edit forbidden if not on levelbuilder" do
-    sign_in @admin
+    sign_in @levelbuilder
     get :edit, id: 'course1'
     assert_response :forbidden
   end
@@ -224,11 +235,11 @@ def create_admin_script
     assert_response :forbidden
   end
 
-  test "should get show of admin script if signed in as admin" do
+  test "should not get show of admin script if signed in as admin" do
     admin_script = create_admin_script
 
     sign_in @admin
     get :show, id: admin_script.name
-    assert_response :success
+    assert_response :forbidden
   end
 end
diff --git a/dashboard/test/factories.rb b/dashboard/test/factories.rb
@@ -13,6 +13,13 @@
       admin true
     end
 
+    factory :levelbuilder do
+      after(:create) do |levelbuilder|
+        levelbuilder.permission = UserPermission::LEVELBUILDER
+        levelbuilder.save
+      end
+    end
+
     factory :teacher do
       user_type User::TYPE_TEACHER
       birthday Date.new(1980, 03, 14)

diff --git a/dashboard/test/helpers/levels_helper_test.rb b/dashboard/test/helpers/levels_helper_test.rb
@@ -311,7 +311,7 @@ def stub_country(code)
     assert_not can_view_solution?
 
     sign_out user
-    user = create :admin
+    user = create :levelbuilder
     sign_in user
     assert can_view_solution?
 
@@ -333,7 +333,7 @@ def stub_country(code)
     @script = create(:script)
     @script_level = create(:script_level, level: @level, script: @script)
 
-    user = create :admin
+    user = create :levelbuilder
     sign_in user
     assert can_view_solution?