Remove score items controller

app/controllers/evaluation_exercise_controller.rb

@@ -7,7 +7,7 @@ def update
     if params[:evaluation_exercise].key?(:score_items)
       score_items = params[:evaluation_exercise][:score_items]
       ScoreItem.transaction do
-        new_items = score_items.filter { |item| item[:id].blank? }
+        new_items = score_items.filter { |item| !item.key?(:id) || item[:id].blank? }
         updated_items = score_items.filter { |item| item[:id].present? }
         @evaluation_exercise.score_items.each do |item|
           if (updated_item = updated_items.find { |i| i[:id].to_i == item.id })
@@ -19,6 +19,8 @@ def update
         new_items.each do |item|
           @evaluation_exercise.score_items.create!(item.permit(:name, :description, :maximum, :visible, :order))
         end
+      rescue ActiveRecord::RecordInvalid => e
+        return render json: { errors: e.record.errors }, status: :unprocessable_entity
       end
     end
 

app/policies/evaluation_policy.rb

@@ -15,10 +15,6 @@ def add_users?
     course_admin?
   end
 
-  def score_items?
-    course_admin?
-  end
-
   def manage_scores?
     course_admin?
   end

app/policies/score_item_policy.rb

@@ -17,12 +17,4 @@ def resolve
       end
     end
   end
-
-  def permitted_attributes_for_create
-    %i[evaluation_exercise_id maximum name visible description]
-  end
-
-  def permitted_attributes_for_update
-    %i[maximum name visible description]
-  end
 end

config/routes.rb

@@ -223,14 +223,7 @@
         post 'modify_grading_visibility'
       end
       resources :feedbacks, only: %i[show edit update]
-      resources :score_items, only: %i[create destroy update]
       resources :scores, only: %i[show create update destroy]
-
-      resources :evaluation_exercise do
-        resources :score_items do
-          patch '/', on: :collection, to: 'score_items#update_all'
-        end
-      end
     end
     resources :feedbacks, only: %i[show edit update] do
       delete 'scores', action: :destroy_scores, controller: :feedbacks, on: :member

test/controllers/evaluation_exercise_controller_test.rb

@@ -5,12 +5,11 @@ def setup
     @evaluation = create :evaluation, :with_submissions
     @staff_member = users(:staff)
     @evaluation.series.course.administrating_members << @staff_member
+    @exercise = @evaluation.evaluation_exercises.first
     sign_in @staff_member
   end
 
   test 'can update visibility as course admin' do
-    evaluation_exercise = @evaluation.evaluation_exercises.first
-
     [
       [@staff_member, :success],
       [users(:student), :forbidden],
@@ -20,25 +19,141 @@ def setup
     ].each do |user, expected|
       sign_in user if user.present?
 
-      evaluation_exercise.update!(visible_score: true)
+      @exercise.update!(visible_score: true)
 
-      assert_predicate evaluation_exercise, :visible_score?
+      assert_predicate @exercise, :visible_score?
 
-      patch evaluation_exercise_path(evaluation_exercise, format: :js), params: {
+      patch evaluation_exercise_path(@exercise, format: :js), params: {
         evaluation_exercise: {
           visible_score: false
         }
       }
 
       assert_response expected
-      evaluation_exercise.reload
+      @exercise.reload
+      if expected == :success
+        assert_not @exercise.visible_score?
+      else
+        assert_predicate @exercise, :visible_score?
+      end
+
+      sign_out user if user.present?
+    end
+  end
+
+  test 'should update all score items if course administrator' do
+    score_items = create_list :score_item, 3, evaluation_exercise: @exercise,
+                              description: 'Before test',
+                              maximum: '10.0'
+
+    [
+      [@staff_member, :success],
+      [users(:student), :forbidden],
+      [create(:staff), :forbidden],
+      [users(:zeus), :success],
+      [nil, :unauthorized]
+    ].each do |user, expected|
+      sign_in user if user.present?
+      patch evaluation_exercise_path(@exercise, format: :json), params: {
+        evaluation_exercise: {
+          score_items: [
+            { id: score_items[0].id, name: 'edited', description: 'After test', maximum: '20.0' },
+            { name: 'new', description: 'new value', maximum: '25.0' }
+          ]
+        }
+      }
+
+      assert_response expected
+
+      @exercise.score_items.reload
       if expected == :success
-        assert_not evaluation_exercise.visible_score?
+        assert_equal 2, @exercise.score_items.count
+        assert_equal 'After test', @exercise.score_items.first.description
+        assert_in_delta(20.0, @exercise.score_items.first.maximum)
+        assert_equal 'new', @exercise.score_items.last.name
+        assert_equal 'new value', @exercise.score_items.last.description
+        assert_in_delta(25.0, @exercise.score_items.last.maximum)
+
+        # reset
+        @exercise.score_items.each(&:destroy)
+        score_items = create_list :score_item, 3, evaluation_exercise: @exercise,
+                                  description: 'Before test',
+                                  maximum: '10.0'
       else
-        assert_predicate evaluation_exercise, :visible_score?
+        assert_equal 3, @exercise.score_items.count
       end
 
       sign_out user if user.present?
     end
   end
+
+  test 'should not create score item for invalid data' do
+    # Missing data
+    patch evaluation_exercise_path(@exercise, format: :json), params: {
+      evaluation_exercise: {
+        score_items: [
+          { name: 'new' }
+        ]
+      }
+    }
+
+    assert_response :unprocessable_entity
+
+    # Negative maximum
+    patch evaluation_exercise_path(@exercise, format: :json), params: {
+      evaluation_exercise: {
+        score_items: [
+          { name: 'new', description: 'new value', maximum: '-20.0' }
+        ]
+      }
+    }
+
+    assert_response :unprocessable_entity
+  end
+
+  test 'should not update score item for invalid data' do
+    score_item = create :score_item, evaluation_exercise: @exercise
+
+    # Negative maximum
+    patch evaluation_exercise_path(@exercise, format: :json), params: {
+      evaluation_exercise: {
+        score_items: [
+          { id: score_item.id, maximum: '-20.0' }
+        ]
+      }
+    }
+
+    assert_response :unprocessable_entity
+  end
+
+  test 'should delete score item if course administrator' do
+    assert_equal 0, @exercise.score_items.count
+
+    [
+      [@staff_member, :success],
+      [users(:student), :forbidden],
+      [create(:staff), :forbidden],
+      [users(:zeus), :success],
+      [nil, :unauthorized]
+    ].each do |user, expected|
+      score_item = create :score_item, evaluation_exercise: @exercise,
+                          description: 'Code re-use',
+                          maximum: '10.0'
+
+      assert_equal 1, @exercise.score_items.count
+      sign_in user if user.present?
+      patch evaluation_exercise_path(@exercise, format: :json), params: {
+        evaluation_exercise: {
+          score_items: []
+        }
+      }, as: :json
+
+      assert_response expected
+      @exercise.score_items.reload
+      assert_equal 0, @exercise.score_items.count if response == :success
+
+      sign_out user if user.present?
+      @exercise.update!(score_items: [])
+    end
+  end
 end