Merge pull request #750 from openHPI/misc

Misc changes

README.md

@@ -5,6 +5,8 @@ CodeOcean
 [![Code Climate](https://codeclimate.com/github/openHPI/codeocean/badges/gpa.svg)](https://codeclimate.com/github/openHPI/codeocean)
 [![Test Coverage](https://codeclimate.com/github/openHPI/codeocean/badges/coverage.svg)](https://codeclimate.com/github/openHPI/codeocean)
 
+![Learner Interface](docs/implement.png)
+
 ## Introduction
 
 CodeOcean is an educational, web-based execution and development environment for practical programming exercises designed for the use in Massive Open Online Courses (MOOCs). 
@@ -20,7 +22,7 @@ CodeOcean is mainly used in the context of MOOCs (such as those offered on openH
 
 ## Development Setup
 
-Please refer to the [Local Setup Guide](LOCAL_SETUP.md) for more details. 
+Please refer to the [Local Setup Guide](docs/LOCAL_SETUP.md) for more details. 
 
 ### Mandatory Steps
 

app/controllers/exercises_controller.rb

@@ -617,7 +617,7 @@ class ExercisesController < ApplicationController
       end
     else
       # redirect to feedback page if score is less than 100 percent
-      if @exercise.needs_more_feedback? && !@embed_options[:disable_redirect_to_feedback]
+      if @exercise.needs_more_feedback?(@submission) && !@embed_options[:disable_redirect_to_feedback]
         clear_lti_session_data(@submission.exercise_id, @submission.user_id, session[:consumer_id])
         redirect_to_user_feedback
       else

app/controllers/request_for_comments_controller.rb

@@ -111,20 +111,6 @@ class RequestForCommentsController < ApplicationController
     authorize!
   end
 
-  def create_comment_exercise
-    old = UserExerciseFeedback.find_by(exercise_id: params[:exercise_id], user_id: current_user.id, user_type: current_user.class.name)
-    if old
-      old.delete
-    end
-    uef = UserExerciseFeedback.new(comment_params)
-
-    if uef.save
-      render(json: {success: "true"})
-    else
-      render(json: {success: "false"})
-    end
-  end
-
   # DELETE /request_for_comments/1
   # DELETE /request_for_comments/1.json
   def destroy
@@ -148,8 +134,4 @@ class RequestForCommentsController < ApplicationController
     params.require(:request_for_comment).permit(:exercise_id, :file_id, :question, :requested_at, :solved, :submission_id).merge(user_id: current_user.id, user_type: current_user.class.name)
   end
 
-  def comment_params
-    params.permit(:exercise_id, :feedback_text).merge(user_id: current_user.id, user_type: current_user.class.name)
-  end
-
 end

app/controllers/user_exercise_feedbacks_controller.rb

@@ -1,7 +1,7 @@
 class UserExerciseFeedbacksController < ApplicationController
   include CommonBehavior
 
-  before_action :set_user_exercise_feedback, only: [:edit, :update, :show, :destroy]
+  before_action :set_user_exercise_feedback, only: [:edit, :update, :destroy]
 
   def comment_presets
     [[0,t('user_exercise_feedback.difficulty_easy')],
@@ -97,7 +97,26 @@ class UserExerciseFeedbacksController < ApplicationController
   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) if params[:user_exercise_feedback].present?
+    return unless params[:user_exercise_feedback].present?
+
+    exercise_id = if params[:user_exercise_feedback].nil?
+                    params[:exercise_id]
+                  else
+                    params[:user_exercise_feedback][:exercise_id]
+                  end
+
+    user_id = current_user.id
+    user_type = current_user.class.name
+    latest_submission = Submission
+                        .where(user_id: user_id, user_type: user_type, exercise_id: exercise_id)
+                        .order(created_at: :desc).first
+
+    params[:user_exercise_feedback]
+      .permit(:feedback_text, :difficulty, :exercise_id, :user_estimated_worktime)
+      .merge(user_id: user_id,
+             user_type: user_type,
+             submission: latest_submission,
+             normalized_score: latest_submission.normalized_score)
   end
 
   def validate_inputs(uef_params)

app/models/exercise.rb

@@ -46,7 +46,7 @@ class Exercise < ApplicationRecord
   @working_time_statistics = nil
   attr_reader :working_time_statistics
 
-  MAX_EXERCISE_FEEDBACKS = 20
+  MAX_GROUP_EXERCISE_FEEDBACKS = 20
 
   def average_percentage
     if average_score && (maximum_score != 0.0) && submissions.exists?(cause: 'submit')
@@ -550,8 +550,12 @@ class Exercise < ApplicationRecord
   end
   private :valid_submission_deadlines?
 
-  def needs_more_feedback?
-    user_exercise_feedbacks.size <= MAX_EXERCISE_FEEDBACKS
+  def needs_more_feedback?(submission)
+    if submission.normalized_score == 1.00
+      user_exercise_feedbacks.final.size <= MAX_GROUP_EXERCISE_FEEDBACKS
+    else
+      user_exercise_feedbacks.intermediate.size <= MAX_GROUP_EXERCISE_FEEDBACKS
+    end
   end
 
   def last_submission_per_user

app/models/submission.rb

@@ -101,9 +101,9 @@ class Submission < ApplicationRecord
   end
 
   def redirect_to_feedback?
-    # Redirect 10% of users to the exercise feedback page. Ensure, that always the
-    # same users get redirected per exercise and different users for different exercises.
-    # If desired, the number of feedbacks can be limited with exercise.needs_more_feedback?
+    # Redirect 10% of users to the exercise feedback page. Ensure, that always the same
+    # users get redirected per exercise and different users for different exercises. If
+    # desired, the number of feedbacks can be limited with exercise.needs_more_feedback?(submission)
     (user_id + exercise.created_at.to_i) % 10 == 1
   end
 

app/models/user_exercise_feedback.rb

@@ -2,10 +2,14 @@ class UserExerciseFeedback < ApplicationRecord
   include Creation
 
   belongs_to :exercise
+  belongs_to :submission, optional: true
   has_one :execution_environment, through: :exercise
 
   validates :user_id, uniqueness: { scope: [:exercise_id, :user_type] }
 
+  scope :intermediate, -> { where.not(normalized_score: 1.00) }
+  scope :final, -> { where(normalized_score: 1.00) }
+
   def to_s
     "User Exercise Feedback"
   end

app/views/exercises/_editor_file_tree.html.slim

@@ -5,7 +5,7 @@ div id='sidebar-collapsed' class=(@exercise.hide_file_tree && @tips.blank? ? ''
     = render('editor_button', classes: 'btn-block btn-primary btn enforce-top-margin', data: {:'data-cause' => 'file', :'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-plus', id: 'create-file-collapsed', label:'', title: t('exercises.editor.create_file'))
 
   - unless @embed_options[:disable_hints] or @tips.blank?
-    = render('editor_button', classes: 'btn-block btn-success btn mb-4', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-lightbulb', id: 'tips-collapsed', label:'', title: t('exercises.form.tips'))
+    = render('editor_button', classes: 'btn-block btn-secondary btn mb-4', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-lightbulb', id: 'tips-collapsed', label:'', title: t('exercises.form.tips'))
 
   - unless @embed_options[:disable_download]
     = render('editor_button', classes: 'btn-block btn-primary btn enforce-top-margin', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-download', id: 'download-collapsed', label:'', title: t('exercises.editor.download'))

app/views/exercises/_tips_content.html.slim

@@ -5,7 +5,7 @@
   = javascript_pack_tag('highlight', 'data-turbolinks-track': true)
   = stylesheet_pack_tag('highlight', media: 'all', 'data-turbolinks-track': true)
 
-#tips.card.text-white.bg-success.mt-2 role="tab" style="display: block;"
+#tips.card.mt-2 role="tab" style="display: block;"
   .card-header.py-2
     i.fa.fa-lightbulb
     = t('exercises.implement.tips.heading')

app/views/request_for_comments/show.html.slim

@@ -1,5 +1,5 @@
 .list-group
-  h4#exercise_caption.list-group-item-heading data-comment-exercise-url=create_comment_exercise_request_for_comment_path data-exercise-id="#{@request_for_comment.exercise.id}" data-rfc-id="#{@request_for_comment.id}"
+  h4#exercise_caption.list-group-item-heading data-exercise-id="#{@request_for_comment.exercise.id}" data-rfc-id="#{@request_for_comment.id}"
     - if @request_for_comment.solved?
       span.fa.fa-check aria-hidden="true"
     = link_to_if(policy(@request_for_comment.exercise).show?, @request_for_comment.exercise.title, [:implement, @request_for_comment.exercise])

config/routes.rb

@@ -17,7 +17,6 @@ Rails.application.routes.draw do
   resources :request_for_comments do
     member do
       get :mark_as_solved, defaults: { format: :json }
-      post :create_comment_exercise, defaults: { format: :json }
       post :set_thank_you_note, defaults: { format: :json }
     end
   end

db/migrate/20201019090123_add_normalized_score_and_submission_to_user_exercise_feedback.rb

@@ -0,0 +1,23 @@
+class AddNormalizedScoreAndSubmissionToUserExerciseFeedback < ActiveRecord::Migration[5.2]
+  def change
+    add_column :user_exercise_feedbacks, :normalized_score, :float
+    add_reference :user_exercise_feedbacks, :submission, foreign_key: true
+
+    # Disable automatic timestamp modification
+    ActiveRecord::Base.record_timestamps = false
+    UserExerciseFeedback.all.find_each do |uef|
+      latest_submission = Submission
+                          .where(user_id: uef.user_id, user_type: uef.user_type, exercise_id: uef.exercise_id)
+                          .where('created_at < ?', uef.updated_at)
+                          .order(created_at: :desc).first
+
+      # In the beginning, CodeOcean allowed feedback for exercises while viewing an RfC. As a RfC
+      # might be opened by any registered learner, feedback for exercises was created by learners
+      # without having any submission for this particular exercise.
+      next if latest_submission.nil?
+
+      uef.update(submission: latest_submission, normalized_score: latest_submission.normalized_score)
+    end
+    ActiveRecord::Base.record_timestamps = true
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_10_07_104221) do
+ActiveRecord::Schema.define(version: 2020_10_19_090123) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -423,6 +423,9 @@ ActiveRecord::Schema.define(version: 2020_10_07_104221) do
     t.integer "user_estimated_worktime"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.float "normalized_score"
+    t.bigint "submission_id"
+    t.index ["submission_id"], name: "index_user_exercise_feedbacks_on_submission_id"
   end
 
   create_table "user_exercise_interventions", id: :serial, force: :cascade do |t|
@@ -455,4 +458,5 @@ ActiveRecord::Schema.define(version: 2020_10_07_104221) do
   add_foreign_key "request_for_comments", "submissions", name: "request_for_comments_submissions_id_fk"
   add_foreign_key "submissions", "study_groups"
   add_foreign_key "tips", "file_types"
+  add_foreign_key "user_exercise_feedbacks", "submissions"
 end

lib/py_lint_adapter.rb

@@ -1,5 +1,5 @@
 class PyLintAdapter < TestingFrameworkAdapter
-  REGEXP = /Your code has been rated at (\d+\.?\d*)\/(\d+\.?\d*)/
+  REGEXP = /Your code has been rated at (-?\d+\.?\d*)\/(\d+\.?\d*)/
   ASSERTION_ERROR_REGEXP = /^.*?\(.*?,\ (.*?),.*?\)\ (.*?)$/m
 
   def self.framework_name
@@ -7,10 +7,17 @@ class PyLintAdapter < TestingFrameworkAdapter
   end
 
   def parse_output(output)
-    captures = REGEXP.match(output[:stdout]).captures.map(&:to_f)
-    count = captures.second
-    passed = captures.first
-    failed = count - passed
+    regex_match = REGEXP.match(output[:stdout])
+    if regex_match.blank?
+      count = 0
+      failed = 0
+    else
+      captures = regex_match.captures.map(&:to_f)
+      count = captures.second
+      passed = captures.first
+      failed = count - passed
+    end
+
     begin
       assertion_error_matches = Timeout.timeout(2.seconds) do
         output[:stdout].scan(ASSERTION_ERROR_REGEXP).map { |match|