Add admin UI to assign tips to exercises

2020-10-14 12:25:52 +02:00
parent 44ab28d6fe
commit 2678d9ecdf
14 changed files with 232 additions and 33 deletions
--- a/app/assets/javascripts/exercises.js.erb
+++ b/app/assets/javascripts/exercises.js.erb
@ -171,6 +171,87 @@ $(document).on('turbolinks:load', function () {
        }) || {};
    };

+    var initializeSortable = function() {
+        const nestedQuery = '.nested-sortable-list';
+        const root = document.getElementById('tip-list');
+        const containers = document.querySelectorAll(nestedQuery);
+
+        function serialize(sortable) {
+            let serialized = [];
+            const children = [].slice.call(sortable.children);
+            for (let i in children) {
+                const nested = children[i].querySelector(nestedQuery);
+                serialized.push({
+                    tip_id: children[i].dataset['tipId'],
+                    id: children[i].dataset['id'],
+                    children: nested ? serialize(nested) : []
+                });
+            }
+            return serialized
+        }
+
+        function updateTipsJSON(event) {
+            const input = $('#tips-json');
+            input.val(JSON.stringify(serialize(root)));
+            if (event) {
+                event.preventDefault();
+            }
+        }
+
+        function initializeSortable(element) {
+            new Sortable(element, {
+                group: 'nested',
+                animation: 150,
+                fallbackOnBody: true,
+                swapThreshold: 0.45,
+                handle: '.fa-bars',
+                onSort: updateTipsJSON
+            });
+        }
+
+        function removeTip(e) {
+            e.preventDefault();
+            const row = $(this).parent();
+            row.remove();
+            updateTipsJSON();
+        }
+
+        $('.remove-tip').on('click', removeTip);
+
+        function addTip(id, title) {
+            const tip = {id: id, title: title}
+            const template =
+                '<div class="list-group-item d-block" data-tip-id=' + tip.id + ' data-id="">' +
+                '<span class="fa fa-bars mr-3"></span>' + tip.title +
+                '<a class="fa fa-eye ml-2" href="/tips/' + tip.id + '" target="_blank"></a>' +
+                '<a class="fa fa-times ml-2 remove-tip" href="#""></a>' +
+                '<div class="list-group nested-sortable-list"></div>' +
+                '</div>';
+            const tipList = $('#tip-list').append(template);
+            tipList.find('.remove-tip').last().on('click', removeTip);
+            const nestedList = tipList.find('.nested-sortable-list').last().get()[0];
+            initializeSortable(nestedList);
+        }
+
+        $('#add-tips').on('click', function (e) {
+            e.preventDefault();
+            const chosenInputTips = $('#tip-selection').find('select');
+            const selectedTips = chosenInputTips[0].selectedOptions;
+            for (let i = 0; i < selectedTips.length; i++) {
+                addTip(selectedTips[i].value, selectedTips[i].label);
+            }
+            $('#add-tips-modal').modal('hide')
+            updateTipsJSON();
+            chosenInputTips.val('').trigger("chosen:updated");
+        });
+
+        for (let i = 0; i < containers.length; i++) {
+            initializeSortable(containers[i]);
+        }
+
+        updateTipsJSON();
+    };
+
    var highlightCode = function () {
        $('pre code').each(function (index, element) {
            hljs.highlightBlock(element);
@ -371,6 +452,7 @@ $(document).on('turbolinks:load', function () {
            execution_environments = $('form').data('execution-environments');
            file_types = $('form').data('file-types');

+            initializeSortable();
            enableInlineFileCreation();
            inferFileAttributes();
            observeFileRoleChanges();
--- a/app/controllers/exercises_controller.rb
+++ b/app/controllers/exercises_controller.rb
@ -13,6 +13,7 @@ class ExercisesController < ApplicationController
  before_action :set_external_user_and_authorize, only: [:statistics]
  before_action :set_file_types, only: %i[create edit new update]
  before_action :set_course_token, only: [:implement]
+  before_action :set_available_tips, only: %i[implement show new edit]

  skip_before_action :verify_authenticity_token, only: %i[import_exercise import_uuid_check export_external_confirm export_external_check]
  skip_after_action :verify_authorized, only: %i[import_exercise import_uuid_check export_external_confirm]
@ -78,6 +79,9 @@ class ExercisesController < ApplicationController
  def create
    @exercise = Exercise.new(exercise_params)
    collect_set_and_unset_exercise_tags
+    handle_exercise_tips
+    return if performed?
+
    myparam = exercise_params.present? ? exercise_params : {}
    checked_exercise_tags = @exercise_tags.select { |et| myparam[:tag_ids].include? et.tag.id.to_s }
    removed_exercise_tags = @exercise_tags.reject { |et| myparam[:tag_ids].include? et.tag.id.to_s }
@ -89,6 +93,7 @@ class ExercisesController < ApplicationController

    myparam[:exercise_tags] = checked_exercise_tags
    myparam.delete :tag_ids
+    myparam.delete :tips
    removed_exercise_tags.map(&:destroy)

    authorize!
@ -212,7 +217,7 @@ class ExercisesController < ApplicationController
  private :user_by_codeharbor_token

  def exercise_params
-    params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :submission_deadline, :late_submission_deadline, :public, :unpublished, :hide_file_tree, :allow_file_creation, :allow_auto_completion, :title, :expected_difficulty, files_attributes: file_attributes, tag_ids: []).merge(user_id: current_user.id, user_type: current_user.class.name) if params[:exercise].present?
+    params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :submission_deadline, :late_submission_deadline, :public, :unpublished, :hide_file_tree, :allow_file_creation, :allow_auto_completion, :title, :expected_difficulty, :tips, files_attributes: file_attributes, tag_ids: []).merge(user_id: current_user.id, user_type: current_user.class.name) if params[:exercise].present?
  end
  private :exercise_params

@ -233,6 +238,47 @@ class ExercisesController < ApplicationController
  end
  private :handle_file_uploads

+  def handle_exercise_tips
+    if exercise_params
+      begin
+        exercise_tips = JSON.parse(exercise_params[:tips])
+        # Order is important to ensure no foreign key restraints are violated during delete
+        previous_exercise_tips = ExerciseTip.where(exercise: @exercise).select(:id).order(rank: :desc).ids
+        remaining_exercise_tips = update_exercise_tips exercise_tips, nil, 1
+        # Destroy initializes each object and then calls a *single* SQL DELETE
+        ExerciseTip.destroy(previous_exercise_tips - remaining_exercise_tips)
+      rescue JSON::ParserError => e
+        flash[:danger] = "JSON error: #{e.message}"
+        redirect_to(edit_exercise_path(@exercise))
+      end
+    end
+  end
+  private :handle_exercise_tips
+
+  def update_exercise_tips(exercise_tips, parent_exercise_tip_id, rank)
+    result = []
+    exercise_tips.each do |exercise_tip|
+      exercise_tip.symbolize_keys!
+      current_exercise_tip = ExerciseTip.find_or_initialize_by(id: exercise_tip[:id],
+                                     exercise: @exercise,
+                                     tip_id: exercise_tip[:tip_id])
+      current_exercise_tip.parent_exercise_tip_id = parent_exercise_tip_id
+      current_exercise_tip.rank = rank
+      rank += 1
+      unless current_exercise_tip.save
+        flash[:danger] = current_exercise_tip.errors.full_messages.join('. ')
+        redirect_to(edit_exercise_path(@exercise)) and return
+      end
+
+      children = update_exercise_tips exercise_tip[:children], current_exercise_tip.id, rank
+
+      result << current_exercise_tip.id
+      result += children
+    end
+    result
+  end
+  private :update_exercise_tips
+
  def implement
    redirect_to(@exercise, alert: t('exercises.implement.unpublished')) if @exercise.unpublished? && current_user.role != 'admin' && current_user.role != 'teacher' # TODO: TESTESTEST
    redirect_to(@exercise, alert: t('exercises.implement.no_files')) unless @exercise.files.visible.exists?
@ -262,30 +308,6 @@ class ExercisesController < ApplicationController
               else
                 current_user.id
               end
-
-    # Order of elements is important and will be kept
-    available_tips = ExerciseTip.where(exercise: @exercise)
-                                .order(rank: :asc, parent_exercise_tip_id: :asc)
-
-    # Transform result set in a hash and prepare (temporary) children array.
-    # The children array will contain the sorted list of nested tips,
-    # shown for learners in the output sidebar with cards.
-    # Hash - Key: exercise_tip.id, value: exercise_tip Object loaded from database
-    nested_tips = available_tips.each_with_object({}) do |exercise_tip, hash|
-      exercise_tip.children = []
-      hash[exercise_tip.id] = exercise_tip
-    end
-
-    available_tips.each do |tip|
-      # A tip without a parent cannot be a children
-      next if tip.parent_exercise_tip_id.blank?
-
-      # Link tips if they are related
-      nested_tips[tip.parent_exercise_tip_id].children << tip
-    end
-
-    # Return an array with top-level tips
-    @tips = nested_tips.values.select { |tip| tip.parent_exercise_tip_id.nil? }
  end

  def set_course_token
@ -311,6 +333,32 @@ class ExercisesController < ApplicationController
  end
  private :set_course_token

+  def set_available_tips
+    # Order of elements is important and will be kept
+    available_tips = ExerciseTip.where(exercise: @exercise).order(rank: :asc)
+
+    # Transform result set in a hash and prepare (temporary) children array.
+    # The children array will contain the sorted list of nested tips,
+    # shown for learners in the output sidebar with cards.
+    # Hash - Key: exercise_tip.id, value: exercise_tip Object loaded from database
+    nested_tips = available_tips.each_with_object({}) do |exercise_tip, hash|
+      exercise_tip.children = []
+      hash[exercise_tip.id] = exercise_tip
+    end
+
+    available_tips.each do |tip|
+      # A tip without a parent cannot be a children
+      next if tip.parent_exercise_tip_id.blank?
+
+      # Link tips if they are related
+      nested_tips[tip.parent_exercise_tip_id].children << tip
+    end
+
+    # Return an array with top-level tips
+    @tips = nested_tips.values.select { |tip| tip.parent_exercise_tip_id.nil? }
+  end
+  private :set_available_tips
+
  def working_times
    working_time_accumulated = @exercise.accumulated_working_time_for_only(current_user)
    working_time_75_percentile = @exercise.get_quantiles([0.75]).first
@ -502,6 +550,9 @@ class ExercisesController < ApplicationController

  def update
    collect_set_and_unset_exercise_tags
+    handle_exercise_tips
+    return if performed?
+
    myparam = exercise_params
    checked_exercise_tags = @exercise_tags.select { |et| myparam[:tag_ids].include? et.tag.id.to_s }
    removed_exercise_tags = @exercise_tags.reject { |et| myparam[:tag_ids].include? et.tag.id.to_s }
@ -513,6 +564,7 @@ class ExercisesController < ApplicationController

    myparam[:exercise_tags] = checked_exercise_tags
    myparam.delete :tag_ids
+    myparam.delete :tips
    removed_exercise_tags.map(&:destroy)
    update_and_respond(object: @exercise, params: myparam)
  end
--- a/app/javascript/packs/sortable.js
+++ b/app/javascript/packs/sortable.js
@ -0,0 +1,12 @@
+/* eslint no-console:0 */
+// This file is automatically compiled by Webpack, along with any other files
+// present in this directory. You're encouraged to place your actual application logic in
+// a relevant structure within app/javascript and only use these pack files to reference
+// that code so it'll be compiled.
+//
+// To reference this file, add <%= javascript_pack_tag 'application' %> to the appropriate
+// layout file, like app/views/layouts/application.html.slim
+
+// JS
+import Sortable from 'sortablejs'
+window.Sortable = Sortable;
--- a/app/models/exercise_tip.rb
+++ b/app/models/exercise_tip.rb
@ -13,6 +13,6 @@ class ExerciseTip < ApplicationRecord

  def tip_chain?
    # Ensure each referenced parent exercise tip is set for this exercise
-    errors.add :parent_exercise_tip, I18n.t('activerecord.errors.messages.together', attribute: I18n.t('activerecord.attributes.exercise_tip.tip')) unless ExerciseTip.exists?(exercise: exercise, tip: parent_exercise_tip)
+    errors.add :parent_exercise_tip, I18n.t('activerecord.errors.messages.together', attribute: I18n.t('activerecord.attributes.exercise_tip.tip')) unless ExerciseTip.exists?(exercise: exercise, id: parent_exercise_tip)
  end
 end
--- a/app/views/exercises/_add_tip_modal.slim
+++ b/app/views/exercises/_add_tip_modal.slim
@ -0,0 +1,9 @@
+- tips = Tip.order(:title)
+
+form#tip-selection
+  .form-group
+    span.badge = t('activerecord.attributes.exercise_tip.tip')
+    .mb-2
+    = collection_select({}, :tip_ids, tips, :id, :to_s, {}, {id: 'add-tip-list', class: 'form-control', multiple: true})
+
+button.btn.btn-primary#add-tips = t('exercises.form.add_tips')
--- a/app/views/exercises/_form.html.slim
+++ b/app/views/exercises/_form.html.slim
@ -1,3 +1,9 @@
+- content_for :head do
+  // Force a full page reload, see https://github.com/turbolinks/turbolinks/issues/326.
+     Otherwise, code might not be highlighted correctly (race condition)
+  meta name='turbolinks-visit-control' content='reload'
+  = javascript_pack_tag('sortable', 'data-turbolinks-track': true)
+
 - execution_environments = ExecutionEnvironment.where('file_type_id IS NOT NULL').select(:file_type_id, :id)
 - file_types = FileType.where('file_extension IS NOT NULL').select(:file_extension, :id)

@ -71,6 +77,19 @@
                  td = b.object.tag.name
                  td = number_field "tag_factors[#{b.object.tag.id}]", :factor, :value => b.object.factor, in: 1..10, step: 1, class: 'form-control-sm'

+  h2 = t('.tips')
+  ul.list-unstyled.card-group
+    li.card
+      .card-header role="tab" id="tip-heading"
+        a.file-heading data-toggle="collapse" href="#tip-collapse"
+          div.clearfix role="button"
+            span = t('exercises.form.click_to_collapse')
+      .card-collapse.collapse.mx-2 id="tip-collapse" role="tabpanel"
+        = f.hidden_field(:tips, id: "tips-json", value: "")
+        .list-group.nested-sortable-list.mt-2#tip-list
+          = render(partial: 'tips/sortable_tip', collection: @tips, as: :exercise_tip)
+        button.btn.btn-outline-primary.my-2.w-100 type='button' data-toggle='modal' data-target='#add-tips-modal' = t('.add_tips')
+
  h2 = t('activerecord.attributes.exercise.files')
  ul#files.list-unstyled
    = f.fields_for :files do |files_form|
@ -81,3 +100,5 @@
    = render('file_form', f: files_form)

  .actions = render('shared/submit_button', f: f, object: @exercise)
+
+= render('shared/modal', id: 'add-tips-modal', title: t('.add_tips'), template: 'exercises/_add_tip_modal')
--- a/app/views/exercises/show.html.slim
+++ b/app/views/exercises/show.html.slim
@ -38,6 +38,9 @@ h1
 = row(label: 'exercise.embedding_parameters', class: 'mb-4') do
  = content_tag(:input, nil, class: 'form-control mb-4', readonly: true, value: @exercise.unpublished? ? t('exercises.show.is_unpublished') : embedding_parameters(@exercise))

+- unless @tips.blank?
+  = render(partial: 'tips_content')
+
 h2.mt-4 = t('activerecord.attributes.exercise.files')

 ul.list-unstyled#files
--- a/app/views/tips/_collapsed_card.html.slim
+++ b/app/views/tips/_collapsed_card.html.slim
@ -1,5 +1,5 @@
 - tip = exercise_tip.tip
-.card class="#{exercise_tip.parent_exercise_tip_id? ? 'mt-2' : ''}"
+.card class="#{exercise_tip.parent_exercise_tip_id? || exercise_tip.rank != 1 ? 'mt-2' : ''}"
  .card-header.p-2 id="tip-heading-#{exercise_tip.id}" role="tab"
    .card-title.mb-0
      a.collapsed aria-controls="tip-collapse-#{exercise_tip.id}" aria-expanded="false" data-parent="#tips" data-toggle="collapse" href="#tip-collapse-#{exercise_tip.id}"
@ -11,6 +11,7 @@
            = ": #{tip.title}" if tip.title?
  .card.card-collapse.collapse id="tip-collapse-#{exercise_tip.id}" aria-labelledby="tip-heading-#{exercise_tip.id}" role="tabpanel" data-exercise-tip-id=exercise_tip.id
    .card-body.p-3
+      - if tip.description?
        h5
          = t('exercises.implement.tips.description')
        = render_markdown(tip.description)
--- a/app/views/tips/_sortable_tip.html.slim
+++ b/app/views/tips/_sortable_tip.html.slim
@ -0,0 +1,8 @@
+- tip = exercise_tip.tip
+.list-group-item.d-block data-tip-id=tip.id data-id=exercise_tip.id
+  span.fa.fa-bars.mr-3
+  = tip.to_s
+  a.fa.fa-eye.ml-2 href=tip_path(tip) target='_blank'
+  a.fa.fa-times.ml-2.remove-tip href='#'
+  .list-group.nested-sortable-list class="#{exercise_tip.children.present? ? 'mt-3' : ''}"
+    = render(partial: 'tips/sortable_tip', collection: exercise_tip.children, as: :exercise_tip)
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@ -367,7 +367,9 @@ de:
        path: 'Pfad der Datei im Projektverzeichnis. Kann auch leer gelassen werden.'
    form:
      add_file: Datei hinzufügen
-      tags: "Tags"
+      add_tips: Tipps hinzufügen
+      tips: Tipps
+      tags: Tags
      click_to_collapse: "Zum Aus-/Einklappen hier klicken..."
      unpublish_warning: Mit dieser Aktion wird die Aufgabe deaktiviert. Jeder Student, der versucht sie zu implementieren wird eine Fehlermeldung bekommen, bis die Aufgabe wieder aktiviert wurde.
      no_execution_environment_selected: Bitte eine Ausführungsumgebung auswählen, bevor die Aufgabe aktiviert wird.
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@ -367,7 +367,9 @@ en:
        path: "The file's path in the project tree. Can be left blank."
    form:
      add_file: Add file
-      tags: "Tags"
+      add_tips: Add tips
+      tips: Tips
+      tags: Tags
      click_to_collapse: "Click to expand/collapse..."
      unpublish_warning: This will unpublish the exercise. Any student trying to implement it will get an error message, until it is published again.
      no_execution_environment_selected: Select an execution environment before publishing the exercise.
--- a/config/webpack/environment.js
+++ b/config/webpack/environment.js
@ -19,7 +19,8 @@ environment.plugins.prepend('Provide', new webpack.ProvidePlugin({
        vis: 'vis',
        hljs: 'highlight.js',
        d3: 'd3',
-        Sentry: '@sentry/browser'
+        Sentry: '@sentry/browser',
+        Sortable: 'sortablejs',
    })
 );

--- a/package.json
+++ b/package.json
@ -15,6 +15,7 @@
    "jstree": "^3.3.10",
    "opensans-webkit": "^1.1.0",
    "popper.js": "^1.16.1",
+    "sortablejs": "^1.12.0",
    "underscore": "^1.10.2",
    "vis": "^4.21.0",
    "webpack-merge": "^5.1.1"
--- a/yarn.lock
+++ b/yarn.lock
@ -6962,6 +6962,11 @@ sort-keys@^1.0.0:
  dependencies:
    is-plain-obj "^1.0.0"

+sortablejs@^1.12.0:
+  version "1.12.0"
+  resolved "https://registry.yarnpkg.com/sortablejs/-/sortablejs-1.12.0.tgz#ee6d7ece3598c2af0feb1559d98595e5ea37cbd6"
+  integrity sha512-bPn57rCjBRlt2sC24RBsu40wZsmLkSo2XeqG8k6DC1zru5eObQUIPPZAQG7W2SJ8FZQYq+BEJmvuw1Zxb3chqg==
+
 source-list-map@^2.0.0:
  version "2.0.1"
  resolved "https://registry.yarnpkg.com/source-list-map/-/source-list-map-2.0.1.tgz#3993bd873bfc48479cca9ea3a547835c7c154b34"