Merge remote-tracking branch 'origin/master' into exercise-anomaly-detection

# Conflicts:
#	db/schema.rb

app/assets/javascripts/editor/editor.js.erb

@@ -580,7 +580,7 @@ configureEditors: function () {
      * */
     initializeInterventionTimer: function() {
 
-        if ($('#editor').data('rfc-interventions') == true || $('#editor').data('break-interventions') == true) { // split in break or rfc intervention
+        if ($('#editor').data('rfc-interventions') || $('#editor').data('break-interventions')) { // split in break or rfc intervention
             window.onblur = function() { window.blurred = true; };
             window.onfocus = function() { window.blurred = false; };
 

app/assets/stylesheets/exercises.css.scss

@@ -95,14 +95,43 @@ a.file-heading {
   left: 0;
 }
 
-.feedback {
-  .text {
+.feedback-page {
+  .header {
+    font-weight: bold;
     margin-bottom: 10px;
   }
-  .difficulty {
-    font-weight: bold;
+
+  .value {
+    border: 1px solid grey;
+    padding: 10px;
+    margin-bottom: 10px;
   }
-  .worktime {
+
+  .no-feedback {
     font-weight: bold;
+    margin-top: 50px;
+  }
+
+  .feedback-header {
+    display: flex;
+
+    .username {
+      flex-grow: 1;
+      font-weight: bold;
+    }
+
+    .date {}
+  }
+
+  .feedback {
+    .text {
+      margin-bottom: 10px;
+    }
+    .difficulty {
+      font-weight: bold;
+    }
+    .worktime {
+      font-weight: bold;
+    }
   }
 }

app/controllers/exercises_controller.rb

@@ -24,6 +24,9 @@ class ExercisesController < ApplicationController
     3
   end
 
+  def max_intervention_count_per_exercise
+    1
+  end
 
   def java_course_token
     "702cbd2a-c84c-4b37-923a-692d7d1532d0"
@@ -148,8 +151,7 @@ class ExercisesController < ApplicationController
   private :user_by_code_harbor_token
 
   def exercise_params
-    params[:exercise][:expected_worktime_seconds] = params[:exercise][:expected_worktime_minutes].to_i * 60
-    params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :hide_file_tree, :allow_file_creation, :allow_auto_completion, :title, :expected_difficulty, :expected_worktime_seconds, files_attributes: file_attributes, :tag_ids => []).merge(user_id: current_user.id, user_type: current_user.class.name)
+    params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :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)
   end
   private :exercise_params
 
@@ -171,17 +173,19 @@ class ExercisesController < ApplicationController
   def implement
     redirect_to(@exercise, alert: t('exercises.implement.no_files')) unless @exercise.files.visible.exists?
     user_solved_exercise = @exercise.has_user_solved(current_user)
-    user_got_enough_interventions = UserExerciseIntervention.where(user: current_user).where("created_at >= ?", Time.zone.now.beginning_of_day).count >= max_intervention_count_per_day
-    is_java_course = @course_token && @course_token.eql?(java_course_token)
+    count_interventions_today = UserExerciseIntervention.where(user: current_user).where("created_at >= ?", Time.zone.now.beginning_of_day).count
+    user_got_intervention_in_exercise = UserExerciseIntervention.where(user: current_user, exercise: @exercise).size >= max_intervention_count_per_exercise
+    user_got_enough_interventions = count_interventions_today >= max_intervention_count_per_day or user_got_intervention_in_exercise
+    is_java_course = @course_token and @course_token.eql?(java_course_token)
 
     user_intervention_group = UserGroupSeparator.getInterventionGroup(current_user)
 
     case user_intervention_group
       when :no_intervention
       when :break_intervention
-        @show_break_interventions = (user_solved_exercise || !is_java_course || user_got_enough_interventions) ? "false" : "true"
+        @show_break_interventions = (not user_solved_exercise and is_java_course and not user_got_enough_interventions) ? "true" : "false"
       when :rfc_intervention
-        @show_rfc_interventions = (user_solved_exercise || !is_java_course || user_got_enough_interventions) ? "false" : "true"
+        @show_rfc_interventions = (not user_solved_exercise and is_java_course and not user_got_enough_interventions) ? "true" : "false"
     end
 
     @search = Search.new
@@ -387,10 +391,13 @@ class ExercisesController < ApplicationController
       # otherwise an internal user could be shown a false rfc here, since current_user.id is polymorphic, but only makes sense for external users when used with rfcs.)
       # redirect 10 percent pseudorandomly to the feedback page
       if current_user.respond_to? :external_id
-        if ((current_user.id + @submission.exercise.created_at.to_i) % 10 == 1)
+        if @submission.redirect_to_feedback?
           redirect_to_user_feedback
           return
-        elsif rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise, user_id: current_user.id).first
+        end
+
+        rfc = @submission.own_unsolved_rfc
+        if rfc
           # set a message that informs the user that his own RFC should be closed.
           flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_own_rfc')
           flash.keep(:notice)
@@ -400,24 +407,30 @@ class ExercisesController < ApplicationController
             format.json { render(json: {redirect: url_for(rfc)}) }
           end
           return
+        end
 
         # else: show open rfc for same exercise if available
-        elsif rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise).where.not(question: nil).order("RANDOM()").find { | rfc_element |(rfc_element.comments_count < 5) }
+        rfc = @submission.unsolved_rfc
+        unless rfc.nil?
           # set a message that informs the user that his score was perfect and help in RFC is greatly appreciated.
           flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_rfc')
           flash.keep(:notice)
 
           respond_to do |format|
-            format.html { redirect_to(rfc) }
-            format.json { render(json: {redirect: url_for(rfc)}) }
+            format.html {redirect_to(rfc)}
+            format.json {render(json: {redirect: url_for(rfc)})}
           end
           return
         end
       end
     else
       # redirect to feedback page if score is less than 100 percent
-       redirect_to_user_feedback
-      return
+       if @exercise.needs_more_feedback?
+         redirect_to_user_feedback
+       else
+         redirect_to_lti_return_path
+       end
+       return
     end
     redirect_to_lti_return_path
   end

app/controllers/submissions_controller.rb

@@ -390,7 +390,7 @@ class SubmissionsController < ApplicationController
     content += "#{request.base_url}/evaluate\n"
     @submission.files.each do |file|
       file_path = file.path.to_s == '' ? file.name_with_extension : File.join(file.path, file.name_with_extension)
-      content += "#{file_path}=#{file.id.to_s}\n"
+      content += "#{file_path}=#{file.file_id.to_s}\n"
     end
     File.open(path, "w+") do |f|
       f.write(content)

app/controllers/user_exercise_feedbacks_controller.rb

@@ -2,6 +2,7 @@ class UserExerciseFeedbacksController < ApplicationController
   include CommonBehavior
 
   before_action :set_user_exercise_feedback, only: [:edit, :update]
+  before_action :set_user_exercise_feedback_by_id, only: [:show, :destroy]
 
   def comment_presets
     [[0,t('user_exercise_feedback.difficulty_easy')],
@@ -19,10 +20,15 @@ class UserExerciseFeedbacksController < ApplicationController
      [4,t('user_exercise_feedback.estimated_time_more_30')]]
   end
 
-  def authorize!
-    authorize(@uef)
+  def index
+    @search = UserExerciseFeedback.all.search params[:q]
+    @uefs = @search.result.includes(:execution_environment).order(:id).paginate(page: params[:page])
+    authorize!
+  end
+
+  def show
+    authorize!
   end
-  private :authorize!
 
   def create
     @exercise = Exercise.find(uef_params[:exercise_id])
@@ -49,7 +55,8 @@ class UserExerciseFeedbacksController < ApplicationController
   end
 
   def destroy
-    destroy_and_respond(object: @tag)
+    authorize!
+    destroy_and_respond(object: @uef)
   end
 
   def edit
@@ -58,11 +65,6 @@ class UserExerciseFeedbacksController < ApplicationController
     authorize!
   end
 
-  def uef_params
-    params[:user_exercise_feedback].permit(:feedback_text, :difficulty, :exercise_id, :user_estimated_worktime).merge(user_id: current_user.id, user_type: current_user.class.name)
-  end
-  private :uef_params
-
   def new
     @texts = comment_presets.to_a
     @times = time_presets.to_a
@@ -89,6 +91,12 @@ class UserExerciseFeedbacksController < ApplicationController
     end
   end
 
+  private
+
+  def authorize!
+    authorize(@uef || @uefs)
+  end
+
   def to_s
     name
   end
@@ -98,6 +106,14 @@ class UserExerciseFeedbacksController < ApplicationController
     @uef = UserExerciseFeedback.find_by(exercise_id: params[:user_exercise_feedback][:exercise_id], user: current_user)
   end
 
+  def set_user_exercise_feedback_by_id
+    @uef = UserExerciseFeedback.find(params[:id])
+  end
+
+  def uef_params
+    params[:user_exercise_feedback].permit(:feedback_text, :difficulty, :exercise_id, :user_estimated_worktime).merge(user_id: current_user.id, user_type: current_user.class.name)
+  end
+
   def validate_inputs(uef_params)
     begin
       if uef_params[:difficulty].to_i < 0 || uef_params[:difficulty].to_i >= comment_presets.size
@@ -112,4 +128,4 @@ class UserExerciseFeedbacksController < ApplicationController
     end
   end
 
-end
+end

app/models/exercise.rb

@@ -37,6 +37,8 @@ class Exercise < ActiveRecord::Base
 
   @working_time_statistics = nil
 
+  MAX_EXERCISE_FEEDBACKS = 20
+
 
   def average_percentage
     if average_score and maximum_score != 0.0 and submissions.exists?(cause: 'submit')
@@ -362,4 +364,8 @@ class Exercise < ActiveRecord::Base
   end
   private :valid_main_file?
 
+  def needs_more_feedback?
+    user_exercise_feedbacks.size <= MAX_EXERCISE_FEEDBACKS
+  end
+
 end

app/models/submission.rb

@@ -18,6 +18,8 @@ class Submission < ActiveRecord::Base
   validates :cause, inclusion: {in: CAUSES}
   validates :exercise_id, presence: true
 
+  MAX_COMMENTS_ON_RECOMMENDED_RFC = 5
+
   def build_files_hash(files, attribute)
     files.map(&attribute.to_proc).zip(files).to_h
   end
@@ -53,4 +55,16 @@ class Submission < ActiveRecord::Base
   def to_s
     Submission.model_name.human
   end
+
+  def redirect_to_feedback?
+    ((user_id + exercise.created_at.to_i) % 10 == 1) && exercise.needs_more_feedback?
+  end
+
+  def own_unsolved_rfc
+    RequestForComment.unsolved.where(exercise_id: exercise, user_id: user_id).first
+  end
+
+  def unsolved_rfc
+    RequestForComment.unsolved.where(exercise_id: exercise).where.not(question: nil).order("RANDOM()").find { | rfc_element |(rfc_element.comments_count < MAX_COMMENTS_ON_RECOMMENDED_RFC) }
+  end
 end

app/models/user_exercise_feedback.rb

@@ -2,10 +2,11 @@ class UserExerciseFeedback < ActiveRecord::Base
   include Creation
 
   belongs_to :exercise
+  has_one :execution_environment, through: :exercise
 
   validates :user_id, uniqueness: { scope: [:exercise_id, :user_type] }
 
   def to_s
     "User Exercise Feedback"
   end
-end
+end

app/policies/user_exercise_feedback_policy.rb

@@ -1,8 +1,4 @@
-class UserExerciseFeedbackPolicy < ApplicationPolicy
-  def author?
-    @user == @record.author
-  end
-  private :author?
+class UserExerciseFeedbackPolicy < AdminOrAuthorPolicy
 
   def create?
     everyone
@@ -12,8 +8,4 @@ class UserExerciseFeedbackPolicy < ApplicationPolicy
     everyone
   end
 
-  [:show? ,:destroy?, :edit?, :update?].each do |action|
-    define_method(action) { admin? || author?}
-  end
-
 end

app/views/application/_navigation.html.slim

@@ -8,7 +8,7 @@
         - if current_user.admin?
           li = link_to(t('breadcrumbs.dashboard.show'), admin_dashboard_path)
           li.divider
-        - models = [ExecutionEnvironment, Exercise, ExerciseCollection, ProxyExercise, Tag, Consumer, CodeHarborLink,
+        - models = [ExecutionEnvironment, Exercise, ExerciseCollection, ProxyExercise, Tag, Consumer, CodeHarborLink, UserExerciseFeedback,
                 ErrorTemplate, ErrorTemplateAttribute, ExternalUser, FileType, FileTemplate, InternalUser].sort_by {|model| model.model_name.human(count: 2) }
         - models.each do |model|
           - if policy(model).index?

app/views/exercises/_form.html.slim

@@ -35,9 +35,6 @@
   .form-group
     = f.label(t('activerecord.attributes.exercise.difficulty'))
     = f.number_field :expected_difficulty, in: 1..10, step: 1
-  .form-group
-    = f.label(t('activerecord.attributes.exercise.worktime'))
-    = f.number_field "expected_worktime_minutes", value: @exercise.expected_worktime_seconds / 60, in: 1..1000, step: 1
 
   h2 = t('exercises.form.tags')
   ul.list-unstyled.panel-group

app/views/exercises/external_users/statistics.html.slim

@@ -54,8 +54,6 @@ h1 = "#{@exercise} (external user #{@external_user})"
                   -submission_or_intervention.testruns.each do |run|
                     - if run.passed
                       .unit-test-result.positive-result  title=run.output
-                    - elsif run.failed
-                      .unit-test-result.negative-result title=run.output
                     - else
                       .unit-test-result.unknown-result title=run.output
                 td = Time.at(deltas[1..index].inject(:+)).utc.strftime("%H:%M:%S") if index > 0

app/views/exercises/feedback.html.slim

@@ -1,15 +1,24 @@
-h1 = @exercise
+h1 = link_to(@exercise, exercise_path(@exercise))
 
-ul.list-unstyled.panel-group#files
-  - @feedbacks.each do |feedback|
-    li.panel.panel-default
-      .panel-heading role="tab" id="heading"
-        div.clearfix
-          span = feedback.user.name
-      .panel-collapse role="tabpanel"
-        .panel-body.feedback
-          .text = feedback.feedback_text
-          .difficulty = "#{t('user_exercise_feedback.difficulty')} #{feedback.difficulty}" if feedback.difficulty
-          .worktime = "#{t('user_exercise_feedback.working_time')} #{feedback.user_estimated_worktime}" if feedback.user_estimated_worktime
+.feedback-page
+  .header = t('activerecord.attributes.exercise.description')
+  .value = render_markdown(@exercise.description)
 
-= render('shared/pagination', collection: @feedbacks)
+  .header = t('activerecord.models.user_exercise_feedback.other')
+  - if @feedbacks.nil? or @feedbacks.size == 0
+    .no-feedback = t('user_exercise_feedback.no_feedback')
+
+  ul.list-unstyled.panel-group
+    - @feedbacks.each do |feedback|
+      li.panel.panel-default
+        .panel-heading role="tab" id="heading"
+          div.clearfix.feedback-header
+            span.username = link_to(feedback.user.name, statistics_external_user_exercise_path(id: @exercise.id, external_user_id: feedback.user.id))
+            span.date = feedback.created_at
+        .panel-collapse role="tabpanel"
+          .panel-body.feedback
+            .text = feedback.feedback_text
+            .difficulty = "#{t('user_exercise_feedback.difficulty')} #{feedback.difficulty}" if feedback.difficulty
+            .worktime = "#{t('user_exercise_feedback.working_time')} #{feedback.user_estimated_worktime}" if feedback.user_estimated_worktime
+
+  = render('shared/pagination', collection: @feedbacks)

app/views/exercises/index.html.slim

@@ -18,7 +18,6 @@ h1 = Exercise.model_name.human(count: 2)
         th = t('activerecord.attributes.exercise.maximum_score')
         th = t('activerecord.attributes.exercise.tags')
         th = t('activerecord.attributes.exercise.difficulty')
-        th = t('activerecord.attributes.exercise.worktime')
         th
           = t('activerecord.attributes.exercise.public')
           - if policy(Exercise).batch_update?
@@ -34,7 +33,6 @@ h1 = Exercise.model_name.human(count: 2)
           td = exercise.maximum_score
           td = exercise.exercise_tags.count
           td = exercise.expected_difficulty
-          td = (exercise.expected_worktime_seconds / 60).ceil
           td.public data-value=exercise.public? = symbol_for(exercise.public?)
           td = link_to(t('shared.edit'), edit_exercise_path(exercise)) if policy(exercise).edit?
           td = link_to(t('.implement'), implement_exercise_path(exercise)) if policy(exercise).implement?

app/views/exercises/show.html.slim

@@ -20,7 +20,6 @@ h1
 = row(label: 'exercise.embedding_parameters') do
   = content_tag(:input, nil, class: 'form-control', readonly: true, value: embedding_parameters(@exercise))
 = row(label: 'exercise.difficulty', value: @exercise.expected_difficulty)
-= row(label: 'exercise.worktime', value: "#{@exercise.expected_worktime_seconds/60} min")
 = row(label: 'exercise.tags', value: @exercise.exercise_tags.map{|et| "#{et.tag.name} (#{et.factor})"}.sort.join(", "))
 
 h2 = t('activerecord.attributes.exercise.files')

app/views/user_exercise_feedbacks/index.html.slim

@@ -0,0 +1,27 @@
+h1 = UserExerciseFeedback.model_name.human(count: 2)
+
+= render(layout: 'shared/form_filters') do |f|
+  .form-group
+    = f.label(:execution_environment_id_eq, t('activerecord.attributes.exercise.execution_environment'), class: 'sr-only')
+    = f.collection_select(:execution_environment_id_eq, ExecutionEnvironment.with_exercises, :id, :name, class: 'form-control', prompt: t('activerecord.attributes.exercise.execution_environment'))
+  .form-group
+    = f.label(:exercise_title_cont, t('activerecord.attributes.request_for_comments.exercise'), class: 'sr-only')
+    = f.search_field(:exercise_title_cont, class: 'form-control', placeholder: t('activerecord.attributes.request_for_comments.exercise'))
+
+.table-responsive
+  table.table
+    thead
+      tr
+        th colspan=2 = t('activerecord.attributes.user_exercise_feedback.user')
+        th = t('activerecord.attributes.user_exercise_feedback.exercise')
+        th colspan=2 = t('shared.actions')
+    tbody
+      - @uefs.each do |uef|
+        tr
+          td = uef.user.id
+          td = uef.user.name
+          td = link_to(uef.exercise.title, uef.exercise)
+          td = link_to(t('shared.show'), uef)
+          td = link_to(t('shared.destroy'), uef, data: {confirm: t('shared.confirm_destroy')}, method: :delete)
+
+= render('shared/pagination', collection: @uefs)

app/views/user_exercise_feedbacks/show.html.slim

@@ -0,0 +1,7 @@
+h2 = @uef
+
+= row(label: 'activerecord.attributes.user_exercise_feedback.exercise', value: link_to(@uef.exercise.title, @uef.exercise))
+= row(label: 'user_exercise_feedback.user', value: @uef.user)
+= row(label: 'activerecord.attributes.user_exercise_feedback.feedback_text', value: @uef.feedback_text)
+= row(label: 'user_exercise_feedback.difficulty', value: @uef.difficulty)
+= row(label: 'user_exercise_feedback.working_time', value: @uef.user_estimated_worktime)