Merge remote-tracking branch 'origin/master' into error-info

# Conflicts:
#	app/controllers/concerns/submission_scoring.rb
#	app/views/application/_navigation.html.slim
#	config/locales/de.yml
#	config/locales/en.yml
#	db/schema.rb

app/controllers/application_controller.rb

@@ -10,6 +10,7 @@ class ApplicationController < ActionController::Base
   rescue_from Pundit::NotAuthorizedError, with: :render_not_authorized
 
   def current_user
+    ::NewRelic::Agent.add_custom_attributes({ external_user_id: session[:external_user_id], session_user_id: session[:user_id] })
     @current_user ||= ExternalUser.find_by(id: session[:external_user_id]) || login_from_session || login_from_other_sources
   end
 

app/controllers/comments_controller.rb

@@ -1,5 +1,5 @@
 class CommentsController < ApplicationController
-  before_action :set_comment, only: [:show, :edit, :update, :destroy_by_id]
+  before_action :set_comment, only: [:show, :edit, :update, :destroy]
 
   # to disable authorization check: comment the line below back in
  # skip_after_action :verify_authorized
@@ -21,6 +21,7 @@ class CommentsController < ApplicationController
         comment.username = comment.user.displayname
         comment.date = comment.created_at.strftime('%d.%m.%Y %k:%M')
         comment.updated = (comment.created_at != comment.updated_at)
+        comment.editable = comment.user == current_user
       }
     else
       @comments = []
@@ -50,12 +51,14 @@ class CommentsController < ApplicationController
   def create
     @comment = Comment.new(comment_params_without_request_id)
 
-    if comment_params[:request_id]
-      UserMailer.got_new_comment(@comment, RequestForComment.find(comment_params[:request_id]), current_user).deliver_now
-    end
-
     respond_to do |format|
       if @comment.save
+        if comment_params[:request_id]
+          request_for_comment = RequestForComment.find(comment_params[:request_id])
+          send_mail_to_author @comment, request_for_comment
+          send_mail_to_subscribers @comment, request_for_comment
+        end
+
         format.html { redirect_to @comment, notice: 'Comment was successfully created.' }
         format.json { render :show, status: :created, location: @comment }
       else
@@ -83,7 +86,8 @@ class CommentsController < ApplicationController
 
   # DELETE /comments/1
   # DELETE /comments/1.json
-  def destroy_by_id
+  def destroy
+    authorize!
     @comment.destroy
     respond_to do |format|
       format.html { head :no_content, notice: 'Comment was successfully destroyed.' }
@@ -91,30 +95,45 @@ class CommentsController < ApplicationController
     end
   end
 
-  def destroy
-    @comments = Comment.where(file_id: params[:file_id], row: params[:row], user: current_user)
-    @comments.each { |comment| authorize comment; comment.destroy }
-    respond_to do |format|
-      #format.html { redirect_to comments_url, notice: 'Comments were successfully destroyed.' }
-      format.html { head :no_content, notice: 'Comments were successfully destroyed.' }
-      format.json { head :no_content }
-    end
-  end
-
   private
-    # Use callbacks to share common setup or constraints between actions.
-    def set_comment
-      @comment = Comment.find(params[:id])
-    end
+
+  # Use callbacks to share common setup or constraints between actions.
+  def set_comment
+    @comment = Comment.find(params[:id])
+  end
 
   def comment_params_without_request_id
     comment_params.except :request_id
   end
 
-    # Never trust parameters from the scary internet, only allow the white list through.
-    def comment_params
-      #params.require(:comment).permit(:user_id, :file_id, :row, :column, :text)
-      # fuer production mode, damit böse menschen keine falsche user_id uebergeben:
-      params.require(:comment).permit(:file_id, :row, :column, :text, :request_id).merge(user_id: current_user.id, user_type: current_user.class.name)
+  # Never trust parameters from the scary internet, only allow the white list through.
+  def comment_params
+    #params.require(:comment).permit(:user_id, :file_id, :row, :column, :text)
+    # fuer production mode, damit böse menschen keine falsche user_id uebergeben:
+    params.require(:comment).permit(:file_id, :row, :column, :text, :request_id).merge(user_id: current_user.id, user_type: current_user.class.name)
+  end
+
+  def send_mail_to_author(comment, request_for_comment)
+    if current_user != request_for_comment.user
+      UserMailer.got_new_comment(comment, request_for_comment, current_user).deliver_now
     end
+  end
+
+  def send_mail_to_subscribers(comment, request_for_comment)
+    request_for_comment.commenters.each do |commenter|
+      already_sent_mail = false
+      subscriptions = Subscription.where(
+          :request_for_comment_id => request_for_comment.id,
+          :user_id => commenter.id, :user_type => commenter.class.name,
+          :deleted => false)
+      subscriptions.each do |subscription|
+        if (subscription.subscription_type == 'author' and current_user == request_for_comment.user) or subscription.subscription_type == 'all'
+          unless subscription.user == current_user or already_sent_mail
+            UserMailer.got_new_comment_for_subscription(comment, subscription, current_user).deliver_now
+            already_sent_mail = true
+          end
+        end
+      end
+    end
+  end
 end

app/controllers/concerns/lti.rb

@@ -42,12 +42,12 @@ module Lti
   private :external_user_email
 
   def external_user_name(provider)
+    # save person_name_full if supplied. this is the display_name, if it is set.
+    # else only save the firstname, we don't want lastnames (family names)
     if provider.lis_person_name_full
       provider.lis_person_name_full
-    elsif provider.lis_person_name_given && provider.lis_person_name_family
-      "#{provider.lis_person_name_given} #{provider.lis_person_name_family}"
     else
-      provider.lis_person_name_given || provider.lis_person_name_family
+      provider.lis_person_name_given
     end
   end
   private :external_user_name
@@ -104,7 +104,7 @@ module Lti
   private :return_to_consumer
 
   def send_score(exercise_id, score, user_id)
-    ::NewRelic::Agent.add_custom_parameters({ score: score, session: session })
+    ::NewRelic::Agent.add_custom_attributes({ score: score, session: session })
     fail(Error, "Score #{score} must be between 0 and #{MAXIMUM_SCORE}!") unless (0..MAXIMUM_SCORE).include?(score)
 
     if session[:consumer_id]

app/controllers/concerns/submission_scoring.rb

@@ -8,7 +8,7 @@ module SubmissionScoring
         output = execute_test_file(file, submission)
         assessment = assessor.assess(output)
         passed = ((assessment[:passed] == assessment[:count]) and (assessment[:score] > 0))
-        testrun_output = passed ? nil : output[:stderr]
+        testrun_output = passed ? nil : 'message: ' + output[:message].to_s + "\n stdout: " + output[:stdout].to_s + "\n stderr: " + output[:stderr].to_s
         if !testrun_output.blank?
           submission.exercise.execution_environment.error_templates.each do |template|
             pattern = Regexp.new(template.signature).freeze
@@ -17,7 +17,7 @@ module SubmissionScoring
             end
           end
         end
-        Testrun.new(submission: submission, file: file, passed: passed, output: testrun_output).save
+        Testrun.new(submission: submission, cause: 'assess', file: file, passed: passed, output: testrun_output).save
         output.merge!(assessment)
         output.merge!(filename: file.name_with_extension, message: feedback_message(file, output[:score]), weight: file.weight)
       end

app/controllers/exercise_collections_controller.rb

@@ -0,0 +1,51 @@
+class ExerciseCollectionsController < ApplicationController
+  include CommonBehavior
+
+  before_action :set_exercise_collection, only: [:show, :edit, :update, :destroy]
+
+  def index
+    @exercise_collections = ExerciseCollection.all.paginate(:page => params[:page])
+    authorize!
+  end
+
+  def show
+  end
+
+  def new
+    @exercise_collection = ExerciseCollection.new
+    authorize!
+  end
+
+  def create
+    @exercise_collection = ExerciseCollection.new(exercise_collection_params)
+    authorize!
+    create_and_respond(object: @exercise_collection)
+  end
+
+  def destroy
+    authorize!
+    destroy_and_respond(object: @exercise_collection)
+  end
+
+  def edit
+  end
+
+  def update
+    update_and_respond(object: @exercise_collection, params: exercise_collection_params)
+  end
+
+  private
+
+  def set_exercise_collection
+    @exercise_collection = ExerciseCollection.find(params[:id])
+    authorize!
+  end
+
+  def authorize!
+    authorize(@exercise_collection || @exercise_collections)
+  end
+
+  def exercise_collection_params
+    params[:exercise_collection].permit(:name, :exercise_ids => [])
+  end
+end

app/controllers/exercises_controller.rb

@@ -20,7 +20,7 @@ class ExercisesController < ApplicationController
   end
   private :authorize!
 
-  def max_intervention_count
+  def max_intervention_count_per_day
     3
   end
 
@@ -166,7 +166,7 @@ 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
+    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)
 
     user_intervention_group = UserGroupSeparator.getInterventionGroup(current_user)
@@ -203,7 +203,7 @@ class ExercisesController < ApplicationController
             if match = lti_json.match(/^.*courses\/([a-z0-9\-]+)\/sections/)
               match.captures.first
             else
-              java_course_token
+              ""
             end
           else
             ""
@@ -344,7 +344,7 @@ class ExercisesController < ApplicationController
   end
 
   def transmit_lti_score
-    ::NewRelic::Agent.add_custom_parameters({ submission: @submission.id, normalized_score: @submission.normalized_score })
+    ::NewRelic::Agent.add_custom_attributes({ submission: @submission.id, normalized_score: @submission.normalized_score })
     response = send_score(@submission.exercise_id, @submission.normalized_score, @submission.user_id)
 
     if response[:status] == 'success'

app/controllers/request_for_comments_controller.rb

@@ -1,4 +1,5 @@
 class RequestForCommentsController < ApplicationController
+  include SubmissionScoring
   before_action :set_request_for_comment, only: [:show, :edit, :update, :destroy, :mark_as_solved, :set_thank_you_note]
 
   skip_after_action :verify_authorized
@@ -22,7 +23,7 @@ class RequestForCommentsController < ApplicationController
                           request_for_comments.submission_id, request_for_comments.row_number') # ugly, but rails wants it this way
                   .select('request_for_comments.*, max(comments.updated_at) as last_comment')
                   .search(params[:q])
-    @request_for_comments = @search.result.order('created_at DESC').paginate(page: params[:page])
+    @request_for_comments = @search.result.order('created_at DESC').paginate(page: params[:page], total_entries: @search.result.length)
     authorize!
   end
 
@@ -68,11 +69,8 @@ class RequestForCommentsController < ApplicationController
   def set_thank_you_note
     authorize!
     @request_for_comment.thank_you_note = params[:note]
-    commenters = []
-    @request_for_comment.comments.distinct.to_a.each {|comment|
-      commenters.append comment.user
-    }
-    commenters = commenters.uniq {|user| user.id}
+
+    commenters = @request_for_comment.commenters
     commenters.each {|commenter| UserMailer.send_thank_you_note(@request_for_comment, commenter).deliver_now}
 
     respond_to do |format|
@@ -110,6 +108,10 @@ class RequestForCommentsController < ApplicationController
     @request_for_comment = RequestForComment.new(request_for_comment_params)
     respond_to do |format|
       if @request_for_comment.save
+        # create thread here and execute tests. A run is triggered from the frontend and does not need to be handled here.
+        Thread.new do
+          score_submission(@request_for_comment.submission)
+        end
         format.json { render :show, status: :created, location: @request_for_comment }
       else
         format.html { render :new }

app/controllers/submissions_controller.rb

@@ -13,8 +13,12 @@ class SubmissionsController < ApplicationController
   before_action :set_mime_type, only: [:download_file, :render_file]
   skip_before_action :verify_authenticity_token, only: [:download_file, :render_file]
 
-  def max_message_buffer_size
-    500
+  def max_run_output_buffer_size
+    if(@submission.cause == 'requestComments')
+      5000
+    else
+      500
+    end
   end
 
   def authorize!
@@ -210,7 +214,7 @@ class SubmissionsController < ApplicationController
   end
 
   def handle_message(message, tubesock, container)
-    @message_buffer ||= ""
+    @run_output ||= ""
     # Handle special commands first
     if (/^#exit/.match(message))
       # Just call exit_container on the docker_client.
@@ -219,19 +223,19 @@ class SubmissionsController < ApplicationController
       # kill_socket is called in the "on close handler" of the websocket to the container
       @docker_client.exit_container(container)
     elsif /^#timeout/.match(message)
-      @message_buffer = 'timeout: ' + @message_buffer # add information that this run timed out to the buffer
+      @run_output = 'timeout: ' + @run_output # add information that this run timed out to the buffer
     else
       # Filter out information about run_command, test_command, user or working directory
       run_command = @submission.execution_environment.run_command % command_substitutions(params[:filename])
       test_command = @submission.execution_environment.test_command % command_substitutions(params[:filename])
       if !(/root|workspace|#{run_command}|#{test_command}/.match(message))
-        @message_buffer += message if @message_buffer.size <= max_message_buffer_size
         parse_message(message, 'stdout', tubesock)
       end
     end
   end
 
   def parse_message(message, output_stream, socket, recursive = true)
+    parsed = '';
     begin
       parsed = JSON.parse(message)
       if(parsed.class == Hash && parsed.key?('cmd'))
@@ -270,13 +274,16 @@ class SubmissionsController < ApplicationController
         socket.send_data JSON.dump(parsed)
         Rails.logger.info('parse_message sent: ' + JSON.dump(parsed))
       end
+    ensure
+      # save the data that was send to the run_output if there is enough space left. this will be persisted as a testrun with cause "run"
+      @run_output += JSON.dump(parsed) if @run_output.size <= max_run_output_buffer_size
     end
   end
 
   def save_run_output
-    if !@message_buffer.blank?
-      @message_buffer = @message_buffer[(0..max_message_buffer_size-1)] # trim the string to max_message_buffer_size chars
-      Testrun.create(file: @file, submission: @submission, output: @message_buffer)
+    if !@run_output.blank?
+      @run_output = @run_output[(0..max_run_output_buffer_size-1)] # trim the string to max_message_buffer_size chars
+      Testrun.create(file: @file, cause: 'run', submission: @submission, output: @run_output)
     end
   end
 

app/controllers/subscriptions_controller.rb

@@ -0,0 +1,62 @@
+class SubscriptionsController < ApplicationController
+
+  def authorize!
+    authorize(@subscription || @subscriptions)
+  end
+  private :authorize!
+
+  # POST /subscriptions.json
+  def create
+    @subscription = Subscription.new(subscription_params)
+    respond_to do |format|
+      if @subscription.save
+        format.json { render json: @subscription, status: :created }
+      else
+        format.json { render json: @subscription.errors, status: :unprocessable_entity }
+      end
+    end
+    authorize!
+  end
+
+  # DELETE /subscriptions/1
+  # DELETE /subscriptions/1.json
+  def destroy
+    begin
+      @subscription = Subscription.find(params[:id])
+    rescue
+      skip_authorization
+      respond_to do |format|
+        format.html { redirect_to request_for_comments_url, alert: t('subscriptions.subscription_not_existent') }
+        format.json { render json: {message: t('subscriptions.subscription_not_existent')}, status: :not_found }
+      end
+    else
+      authorize!
+      rfc = @subscription.try(:request_for_comment)
+      @subscription.deleted = true
+      if @subscription.save
+        respond_to do |format|
+          format.html { redirect_to request_for_comment_url(rfc), notice: t('subscriptions.successfully_unsubscribed') }
+          format.json { render json: {message: t('subscriptions.successfully_unsubscribed')}, status: :ok}
+        end
+      else
+        respond_to do |format|
+          format.html { redirect_to request_for_comment_url(rfc), :flash => { :danger => t('shared.message_failure') } }
+          format.json { render json: {message: t('shared.message_failure')}, status: :internal_server_error}
+        end
+      end
+    end
+  end
+
+  def set_subscription
+    @subscription = Subscription.find(params[:id])
+    authorize!
+  end
+  private :set_subscription
+
+  def subscription_params
+    current_user_id = current_user.try(:id)
+    current_user_class_name = current_user.try(:class).try(:name)
+    params[:subscription].permit(:request_for_comment_id, :subscription_type).merge(user_id: current_user_id, user_type: current_user_class_name, deleted: false)
+  end
+  private :subscription_params
+end