From 0b1be05ea6c1ff2ec5f99c7e0999df3e9c561938 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Fri, 21 Jun 2024 11:06:28 +0200
Subject: [PATCH 1/4] Allow exporting score items

---
 app/controllers/score_items_controller.rb  |  7 +++++++
 app/policies/evaluation_exercise_policy.rb | 12 ++++++++++++
 app/views/score_items/index.csv.erb        |  9 +++++++++
 config/routes.rb                           |  4 ++++
 4 files changed, 32 insertions(+)
 create mode 100644 app/views/score_items/index.csv.erb

diff --git a/app/controllers/score_items_controller.rb b/app/controllers/score_items_controller.rb
index 0b67381489..9d91b46481 100644
--- a/app/controllers/score_items_controller.rb
+++ b/app/controllers/score_items_controller.rb
@@ -22,6 +22,13 @@ def copy
     end
   end
 
+  def index
+    @evaluation_exercise = EvaluationExercise.find(params[:evaluation_exercise_id])
+    authorize @evaluation_exercise, :show?
+
+    @score_items = policy_scope(@evaluation_exercise.score_items)
+  end
+
   def create
     @score_item = ScoreItem.new(permitted_attributes(ScoreItem))
     @score_item.last_updated_by = current_user
diff --git a/app/policies/evaluation_exercise_policy.rb b/app/policies/evaluation_exercise_policy.rb
index 43d72dcac2..5ada94a2f5 100644
--- a/app/policies/evaluation_exercise_policy.rb
+++ b/app/policies/evaluation_exercise_policy.rb
@@ -6,6 +6,14 @@ def show_total?
     record.visible_score? && record.evaluation.released?
   end
 
+  def show?
+    return true if course_admin?
+
+    return false unless evaluation_member?
+
+    record&.visible_score? && record&.evaluation&.released?
+  end
+
   def permitted_attributes
     %i[visible_score]
   end
@@ -16,4 +24,8 @@ def course_admin?
     course = record&.evaluation&.series&.course
     user&.course_admin?(course)
   end
+
+  def evaluation_member?
+    record&.evaluation&.users&.include?(user)
+  end
 end
diff --git a/app/views/score_items/index.csv.erb b/app/views/score_items/index.csv.erb
new file mode 100644
index 0000000000..ab39c2ad21
--- /dev/null
+++ b/app/views/score_items/index.csv.erb
@@ -0,0 +1,9 @@
+<%= CSV.generate_line %w[name maximum description visible], row_sep: nil %>
+<% @score_items.each do |cm| %>
+<%= CSV.generate_line([
+  cm.name,
+  cm.maximum,
+  cm.description,
+  cm.visible
+], row_sep: nil).html_safe  %>
+<% end %>
diff --git a/config/routes.rb b/config/routes.rb
index 13a9d5269b..5707a0f632 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -228,6 +228,10 @@
         post 'add_all', on: :collection
       end
       resources :scores, only: %i[show create update destroy]
+
+      resources :evaluation_exercise do
+        resources :score_items, only: %i[index]
+      end
     end
     resources :feedbacks, only: %i[show edit update] do
       delete 'scores', action: :destroy_scores, controller: :feedbacks, on: :member

From 4bced1d208d97d73df9f52bfe3a5cb29221bc69f Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Fri, 21 Jun 2024 14:36:34 +0200
Subject: [PATCH 2/4] Fix file upload

---
 app/controllers/score_items_controller.rb  | 39 ++++++++++++++++++++++
 app/policies/evaluation_exercise_policy.rb |  4 +++
 app/views/score_items/_exercise.html.erb   | 31 +++++++++++++++--
 app/views/score_items/_upload.html.erb     | 25 ++++++++++++++
 config/locales/views/score_items/en.yml    |  6 ++++
 config/locales/views/score_items/nl.yml    |  6 ++++
 config/routes.rb                           |  4 ++-
 7 files changed, 112 insertions(+), 3 deletions(-)
 create mode 100644 app/views/score_items/_upload.html.erb

diff --git a/app/controllers/score_items_controller.rb b/app/controllers/score_items_controller.rb
index 9d91b46481..465686ee57 100644
--- a/app/controllers/score_items_controller.rb
+++ b/app/controllers/score_items_controller.rb
@@ -22,6 +22,45 @@ def copy
     end
   end
 
+  def upload
+    return render json: { message: I18n.t('course_members.upload_labels_csv.no_file') }, status: :unprocessable_entity if params[:upload].nil? || params[:upload][:file] == 'undefined' || params[:upload][:file].nil?
+
+    file = params[:upload][:file]
+
+    @evaluation_exercise = EvaluationExercise.find(params[:evaluation_exercise_id])
+    authorize @evaluation_exercise, :update?
+
+    begin
+      headers = CSV.foreach(file.path).first
+      %w[name maximum].each do |column|
+        return render json: { message: I18n.t('course_members.upload_labels_csv.missing_column', column: column) }, status: :unprocessable_entity unless headers&.include?(column)
+      end
+
+      # Remove existing score items.
+      @evaluation_exercise.score_items.destroy_all
+
+      CSV.foreach(file.path, headers: true) do |row|
+        row = row.to_hash
+        score_item = ScoreItem.new(
+          name: row['name'],
+          maximum: row['maximum'],
+          visible: row.key?('visible') ? row['visible'] : true,
+          description: row.key?('description') ? row['description'] : nil,
+          evaluation_exercise: @evaluation_exercise
+        )
+        score_item.save!
+      end
+    rescue CSV::MalformedCSVError
+      return render json: { message: I18n.t('course_members.upload_labels_csv.malformed') }, status: :unprocessable_entity
+    end
+
+
+    respond_to do |format|
+      format.js { render 'score_items/index', locals: { new: nil, evaluation_exercise: @evaluation_exercise.reload } }
+      format.json { head :no_content }
+    end
+  end
+
   def index
     @evaluation_exercise = EvaluationExercise.find(params[:evaluation_exercise_id])
     authorize @evaluation_exercise, :show?
diff --git a/app/policies/evaluation_exercise_policy.rb b/app/policies/evaluation_exercise_policy.rb
index 5ada94a2f5..1c1d58addb 100644
--- a/app/policies/evaluation_exercise_policy.rb
+++ b/app/policies/evaluation_exercise_policy.rb
@@ -14,6 +14,10 @@ def show?
     record&.visible_score? && record&.evaluation&.released?
   end
 
+  def update?
+    course_admin?
+  end
+
   def permitted_attributes
     %i[visible_score]
   end
diff --git a/app/views/score_items/_exercise.html.erb b/app/views/score_items/_exercise.html.erb
index 4d88cf0b35..6e7000d8b4 100644
--- a/app/views/score_items/_exercise.html.erb
+++ b/app/views/score_items/_exercise.html.erb
@@ -2,8 +2,30 @@
   <%= javascript_include_tag 'score_item' %>
 <% end %>
 <% maximum_score = evaluation_exercise.maximum_score %>
-<div id="score-items-<%= evaluation_exercise.id %>">
-  <h4><%= evaluation_exercise.exercise.name %></h4>
+<div id="score-items-<%= evaluation_exercise.id %>" class="mb-2">
+  <div class="d-flex flex-row align-items-center">
+    <h4><%= evaluation_exercise.exercise.name %></h4>
+    <div class="flex-spacer"></div>
+    <div class="dropdown">
+      <a class="btn btn-icon dropdown-toggle" data-bs-toggle="dropdown">
+        <i class="mdi mdi-dots-vertical"></i>
+      </a>
+      <ul class="dropdown-menu dropdown-menu-end">
+        <li>
+          <%= link_to evaluation_evaluation_exercise_score_items_path(evaluation_exercise.evaluation_id, evaluation_exercise.id, format: :csv), class: "dropdown-item" do %>
+            <i class="mdi mdi-download mdi-18"></i>
+            <%= I18n.t "score_items.exercise.download" %>
+          <% end %>
+        </li>
+        <li>
+          <a href="#upload-form-<%=evaluation_exercise.id%>" data-bs-toggle="modal" class="dropdown-item">
+            <i class="mdi mdi-upload mdi-18"></i>
+            <%= I18n.t "score_items.exercise.upload" %>
+          </a>
+        </li>
+      </ul>
+    </div>
+  </div>
 
   <table class="table table-resource score-items-table" id="table-for-<%= evaluation_exercise.id %>">
     <thead>
@@ -104,6 +126,11 @@
       <%= render 'score_items/copy', evaluation_exercise: evaluation_exercise, evaluation: @evaluation %>
     </div>
   </div>
+  <div class="modal fade modal-<%= evaluation_exercise.id %>" id="upload-form-<%= evaluation_exercise.id %>" tabindex="-1" role="dialog">
+    <div class="modal-dialog" role="document">
+      <%= render partial: 'score_items/upload', locals: {evaluation_exercise: evaluation_exercise} %>
+    </div>
+  </div>
 </div>
 <script>
     window.dodona.initVisibilityCheckboxes(document.getElementById("score-items-<%= evaluation_exercise.id %>"));
diff --git a/app/views/score_items/_upload.html.erb b/app/views/score_items/_upload.html.erb
new file mode 100644
index 0000000000..0c0433e299
--- /dev/null
+++ b/app/views/score_items/_upload.html.erb
@@ -0,0 +1,25 @@
+<div class="modal-content">
+  <div class="modal-header">
+    <h4 class="modal-title"><%= t "score_items.upload.title" %></h4>
+    <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="<%= t("score_items.form.close") %>"></button>
+  </div>
+
+  <%= form_for(:upload, url: upload_evaluation_evaluation_exercise_score_items_path(evaluation_exercise.evaluation_id, evaluation_exercise.id), html: { class: 'form-horizontal' }, namespace: evaluation_exercise.id, remote: true) do |f| %>
+    <div class="modal-body">
+      <div class="field form-group row">
+        <div class="col-12">
+          <%= f.file_field :file,
+                           class: "form-control",
+                           accept: '.csv',
+                           required: true
+          %>
+        </div>
+      </div>
+    </div>
+    <div class="modal-footer">
+      <%= f.submit t("score_items.upload.upload"),
+                   class: "btn btn-filled",
+                   data: { confirm: t('score_items.upload.confirm') } %>
+    </div>
+  <% end %>
+</div>
diff --git a/config/locales/views/score_items/en.yml b/config/locales/views/score_items/en.yml
index 11b39e0d54..4beb4cf259 100644
--- a/config/locales/views/score_items/en.yml
+++ b/config/locales/views/score_items/en.yml
@@ -55,3 +55,9 @@ en:
         Show or hide the calculated total score in Dodona for this exercise when the feedback is released.
         Individual score items are still visible if they are marked as such.
       visibility: Show or hide this score item.
+      upload: Upload score items
+      download: Download score items
+    upload:
+      title: Upload csv with score items
+      upload: Upload
+      confirm: Are you sure you want to upload these score items? Existing score items will be overwritten.
diff --git a/config/locales/views/score_items/nl.yml b/config/locales/views/score_items/nl.yml
index cc57f3916e..4e495eab09 100644
--- a/config/locales/views/score_items/nl.yml
+++ b/config/locales/views/score_items/nl.yml
@@ -55,3 +55,9 @@ nl:
         Toon of verberg de berekende totaalscore in Dodona voor deze oefening als de feedback vrijgegeven wordt.
         Individuele scoreonderdelen zijn nog wel zichtbaar als ze als zodanig gemarkeerd zijn.
       visibility: Toon of verberg dit scoreonderdeel.
+      upload: Scoreonderdelen uploaden
+      download: Scoreonderdelen downloaden
+    upload:
+      title: CSV met scoreonderdelen uploaden
+      upload: Uploaden
+      confirm: Ben je zeker dat je deze scoreonderdelen wil uploaden? Bestaande scoreonderdelen zullen overschreven worden.
diff --git a/config/routes.rb b/config/routes.rb
index 5707a0f632..0db1767871 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -230,7 +230,9 @@
       resources :scores, only: %i[show create update destroy]
 
       resources :evaluation_exercise do
-        resources :score_items, only: %i[index]
+        resources :score_items, only: %i[index] do
+          post 'upload', on: :collection
+        end
       end
     end
     resources :feedbacks, only: %i[show edit update] do

From 34c27506d472fe795658f178473c4a4f1a5e5b0f Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Fri, 21 Jun 2024 14:42:48 +0200
Subject: [PATCH 3/4] Fix linting

---
 app/controllers/score_items_controller.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/controllers/score_items_controller.rb b/app/controllers/score_items_controller.rb
index 465686ee57..9f1124e1fa 100644
--- a/app/controllers/score_items_controller.rb
+++ b/app/controllers/score_items_controller.rb
@@ -54,7 +54,6 @@ def upload
       return render json: { message: I18n.t('course_members.upload_labels_csv.malformed') }, status: :unprocessable_entity
     end
 
-
     respond_to do |format|
       format.js { render 'score_items/index', locals: { new: nil, evaluation_exercise: @evaluation_exercise.reload } }
       format.json { head :no_content }

From bbdf0dda98a53c45d10cf04a5e34dee76eca2ac7 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Fri, 21 Jun 2024 15:12:32 +0200
Subject: [PATCH 4/4] Add tests

---
 app/controllers/score_items_controller.rb     |  31 ++--
 app/models/score_item.rb                      |   1 +
 .../score_items_controller_test.rb            | 160 ++++++++++++++++++
 3 files changed, 178 insertions(+), 14 deletions(-)

diff --git a/app/controllers/score_items_controller.rb b/app/controllers/score_items_controller.rb
index 9f1124e1fa..7719a046f8 100644
--- a/app/controllers/score_items_controller.rb
+++ b/app/controllers/score_items_controller.rb
@@ -36,21 +36,24 @@ def upload
         return render json: { message: I18n.t('course_members.upload_labels_csv.missing_column', column: column) }, status: :unprocessable_entity unless headers&.include?(column)
       end
 
-      # Remove existing score items.
-      @evaluation_exercise.score_items.destroy_all
-
-      CSV.foreach(file.path, headers: true) do |row|
-        row = row.to_hash
-        score_item = ScoreItem.new(
-          name: row['name'],
-          maximum: row['maximum'],
-          visible: row.key?('visible') ? row['visible'] : true,
-          description: row.key?('description') ? row['description'] : nil,
-          evaluation_exercise: @evaluation_exercise
-        )
-        score_item.save!
+      ScoreItem.transaction do
+        # Remove existing score items.
+        @evaluation_exercise.score_items.destroy_all
+
+        CSV.foreach(file.path, headers: true) do |row|
+          row = row.to_hash
+          score_item = ScoreItem.new(
+            name: row['name'],
+            maximum: row['maximum'],
+            visible: row.key?('visible') ? row['visible'] : true,
+            description: row.key?('description') ? row['description'] : nil,
+            evaluation_exercise: @evaluation_exercise
+          )
+
+          score_item.save!
+        end
       end
-    rescue CSV::MalformedCSVError
+    rescue CSV::MalformedCSVError, ActiveRecord::RecordInvalid
       return render json: { message: I18n.t('course_members.upload_labels_csv.malformed') }, status: :unprocessable_entity
     end
 
diff --git a/app/models/score_item.rb b/app/models/score_item.rb
index b7031b95da..ad4fa6c6e5 100644
--- a/app/models/score_item.rb
+++ b/app/models/score_item.rb
@@ -22,6 +22,7 @@ class ScoreItem < ApplicationRecord
   after_update :uncomplete_feedbacks_if_maximum_changed
 
   validates :maximum, numericality: { greater_than: 0, less_than: 1000 }
+  validates :name, presence: true
 
   default_scope { order(id: :asc) }
 
diff --git a/test/controllers/score_items_controller_test.rb b/test/controllers/score_items_controller_test.rb
index 781fcc97e7..0fb24a387b 100644
--- a/test/controllers/score_items_controller_test.rb
+++ b/test/controllers/score_items_controller_test.rb
@@ -177,4 +177,164 @@ def setup
       exercise.update!(score_items: [])
     end
   end
+
+  test 'should be able to download score items as CSV' do
+    exercise = @evaluation.evaluation_exercises.first
+    create :score_item, evaluation_exercise: exercise, name: 'foo', maximum: 10.0, description: 'bar'
+    create :score_item, evaluation_exercise: exercise, name: 'baz', maximum: 20.0, description: 'qux', visible: false
+
+    get evaluation_evaluation_exercise_score_items_path(@evaluation, exercise, format: :csv)
+
+    assert_response :success
+
+    csv = CSV.parse(response.body, headers: true)
+
+    assert_equal 2, csv.length
+    assert_equal %w[name maximum description visible], csv.headers
+    assert_equal %w[foo 10.0 bar true], csv[0].fields
+    assert_equal %w[baz 20.0 qux false], csv[1].fields
+  end
+
+  test 'should be able to upload score items as CSV' do
+    exercise = @evaluation.evaluation_exercises.first
+
+    assert_equal 0, exercise.score_items.count
+
+    file = Tempfile.new
+    file.write("name,maximum,description,visible\n")
+    file.write("foo,10.0,bar,true\n")
+    file.write("baz,20.0,qux,false\n")
+    file.close
+
+    post upload_evaluation_evaluation_exercise_score_items_path(@evaluation, exercise, format: :json), params: {
+      upload: {
+        file: Rack::Test::UploadedFile.new(file.path, 'text/csv')
+      }
+    }
+
+    assert_response :no_content
+    exercise.reload
+
+    assert_equal 2, exercise.score_items.count
+    assert_equal 'foo', exercise.score_items[0].name
+    assert_in_delta(10.0, exercise.score_items[0].maximum)
+    assert_equal 'bar', exercise.score_items[0].description
+    assert exercise.score_items[0].visible
+    assert_equal 'baz', exercise.score_items[1].name
+    assert_in_delta(20.0, exercise.score_items[1].maximum)
+    assert_equal 'qux', exercise.score_items[1].description
+    assert_not exercise.score_items[1].visible
+  end
+
+  test 'should be able to upload with only name and maximum' do
+    exercise = @evaluation.evaluation_exercises.first
+
+    assert_equal 0, exercise.score_items.count
+
+    file = Tempfile.new
+    file.write("name,maximum\n")
+    file.write("foo,10.0\n")
+    file.write("baz,20.0\n")
+    file.close
+
+    post upload_evaluation_evaluation_exercise_score_items_path(@evaluation, exercise, format: :json), params: {
+      upload: {
+        file: Rack::Test::UploadedFile.new(file.path, 'text/csv')
+      }
+    }
+
+    assert_response :success
+
+    exercise.reload
+
+    assert_equal 2, exercise.score_items.count
+    assert_equal 'foo', exercise.score_items[0].name
+    assert_in_delta(10.0, exercise.score_items[0].maximum)
+    assert_nil exercise.score_items[0].description
+    assert exercise.score_items[0].visible
+    assert_equal 'baz', exercise.score_items[1].name
+    assert_in_delta(20.0, exercise.score_items[1].maximum)
+    assert_nil exercise.score_items[1].description
+    assert exercise.score_items[1].visible
+  end
+
+  test 'should not upload without name or maximum' do
+    exercise = @evaluation.evaluation_exercises.first
+
+    assert_equal 0, exercise.score_items.count
+
+    file = Tempfile.new
+    file.write("name,maximum\n")
+    file.write("foo,10.0\n")
+    file.write(",20.0\n")
+    file.close
+
+    post upload_evaluation_evaluation_exercise_score_items_path(@evaluation, exercise, format: :json), params: {
+      upload: {
+        file: Rack::Test::UploadedFile.new(file.path, 'text/csv')
+      }
+    }
+
+    assert_response :unprocessable_entity
+
+    exercise.reload
+
+    assert_equal 0, exercise.score_items.count
+  end
+
+  test 'should replace score items if uploading again' do
+    exercise = @evaluation.evaluation_exercises.first
+    create :score_item, evaluation_exercise: exercise, name: 'foo', maximum: 10.0, description: 'bar'
+
+    assert_equal 1, exercise.score_items.count
+
+    file = Tempfile.new
+    file.write("name,maximum,description,visible\n")
+    file.write("baz,20.0,qux,false\n")
+    file.close
+
+    post upload_evaluation_evaluation_exercise_score_items_path(@evaluation, exercise, format: :json), params: {
+      upload: {
+        file: Rack::Test::UploadedFile.new(file.path, 'text/csv')
+      }
+    }
+
+    assert_response :no_content
+
+    exercise.reload
+
+    assert_equal 1, exercise.score_items.count
+    assert_equal 'baz', exercise.score_items[0].name
+    assert_in_delta(20.0, exercise.score_items[0].maximum)
+    assert_equal 'qux', exercise.score_items[0].description
+    assert_not exercise.score_items[0].visible
+  end
+
+  test 'Should not replace score items if uploading invalid data' do
+    exercise = @evaluation.evaluation_exercises.first
+    create :score_item, evaluation_exercise: exercise, name: 'foo', maximum: 10.0, description: 'bar'
+
+    assert_equal 1, exercise.score_items.count
+
+    file = Tempfile.new
+    file.write("name,maximum,description,visible\n")
+    file.write("baz,-20.0,qux,false\n")
+    file.close
+
+    post upload_evaluation_evaluation_exercise_score_items_path(@evaluation, exercise, format: :json), params: {
+      upload: {
+        file: Rack::Test::UploadedFile.new(file.path, 'text/csv')
+      }
+    }
+
+    assert_response :unprocessable_entity
+
+    exercise.reload
+
+    assert_equal 1, exercise.score_items.count
+    assert_equal 'foo', exercise.score_items[0].name
+    assert_in_delta(10.0, exercise.score_items[0].maximum)
+    assert_equal 'bar', exercise.score_items[0].description
+    assert exercise.score_items[0].visible
+  end
 end