Refactor external_user_statistics to own controller action

app/controllers/exercises_controller.rb

@@ -11,8 +11,9 @@ class ExercisesController < ApplicationController
   before_action :set_execution_environments, only: %i[index create edit new update]
   before_action :set_exercise_and_authorize,
     only: MEMBER_ACTIONS + %i[clone implement working_times intervention search run statistics submit reload feedback
-                              requests_for_comments study_group_dashboard export_external_check export_external_confirm]
-  before_action :set_external_user_and_authorize, only: [:statistics]
+                              requests_for_comments study_group_dashboard export_external_check export_external_confirm
+                              external_user_statistics]
+  before_action :set_external_user_and_authorize, only: [:external_user_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]
@@ -466,57 +467,58 @@ working_time_accumulated: working_time_accumulated})
   end
 
   def statistics
-    if @external_user
-      # Render statistics page for one specific external user
-      authorize(@external_user, :statistics?)
-      if policy(@exercise).detailed_statistics?
-        @submissions = Submission.where(user: @external_user,
-          exercise_id: @exercise.id).in_study_group_of(current_user).order('created_at')
-        @show_autosaves = params[:show_autosaves] == 'true'
-        @submissions = @submissions.where.not(cause: 'autosave') unless @show_autosaves
-        interventions = UserExerciseIntervention.where('user_id = ?  AND exercise_id = ?', @external_user.id,
-          @exercise.id)
-        @all_events = (@submissions + interventions).sort_by(&:created_at)
-        @deltas = @all_events.map.with_index do |item, index|
-          delta = item.created_at - @all_events[index - 1].created_at if index.positive?
-          delta.nil? || (delta > StatisticsHelper::WORKING_TIME_DELTA_IN_SECONDS) ? 0 : delta
-        end
-        @working_times_until = []
-        @all_events.each_with_index do |_, index|
-          @working_times_until.push((format_time_difference(@deltas[0..index].sum) if index.positive?))
-        end
-      else
-        final_submissions = Submission.where(user: @external_user,
-          exercise_id: @exercise.id).in_study_group_of(current_user).final
-        @submissions = []
-        %i[before_deadline within_grace_period after_late_deadline].each do |filter|
-          relevant_submission = final_submissions.send(filter).latest
-          @submissions.push relevant_submission if relevant_submission.present?
-        end
-        @all_events = @submissions
-      end
-      render 'exercises/external_users/statistics'
-    else
-      # Show general statistic page for specific exercise
-      user_statistics = {'InternalUser' => {}, 'ExternalUser' => {}}
-      additional_filter = if policy(@exercise).detailed_statistics?
-                            ''
-                          elsif !policy(@exercise).detailed_statistics? && current_user.study_groups.count.positive?
-                            "AND study_group_id IN (#{current_user.study_groups.pluck(:id).join(', ')}) AND cause = 'submit'"
-                          else
-                            # e.g. internal user without any study groups, show no submissions
-                            'AND FALSE'
-                          end
-      query = "SELECT user_id, user_type, MAX(score) AS maximum_score, COUNT(id) AS runs
-              FROM submissions WHERE exercise_id = #{@exercise.id} #{additional_filter}
-              GROUP BY user_id, user_type;"
-      ApplicationRecord.connection.execute(query).each do |tuple|
-        user_statistics[tuple['user_type']][tuple['user_id'].to_i] = tuple
-      end
-      render locals: {
-        user_statistics: user_statistics,
-      }
+    # Show general statistic page for specific exercise
+    user_statistics = {'InternalUser' => {}, 'ExternalUser' => {}}
+    additional_filter = if policy(@exercise).detailed_statistics?
+                          ''
+                        elsif !policy(@exercise).detailed_statistics? && current_user.study_groups.count.positive?
+                          "AND study_group_id IN (#{current_user.study_groups.pluck(:id).join(', ')}) AND cause = 'submit'"
+                        else
+                          # e.g. internal user without any study groups, show no submissions
+                          'AND FALSE'
+                        end
+    query = "SELECT user_id, user_type, MAX(score) AS maximum_score, COUNT(id) AS runs
+            FROM submissions WHERE exercise_id = #{@exercise.id} #{additional_filter}
+            GROUP BY user_id, user_type;"
+    ApplicationRecord.connection.execute(query).each do |tuple|
+      user_statistics[tuple['user_type']][tuple['user_id'].to_i] = tuple
     end
+    render locals: {
+      user_statistics: user_statistics,
+    }
+  end
+
+  def external_user_statistics
+    # Render statistics page for one specific external user
+
+    if policy(@exercise).detailed_statistics?
+      @submissions = Submission.where(user: @external_user,
+        exercise_id: @exercise.id).in_study_group_of(current_user).order('created_at')
+      @show_autosaves = params[:show_autosaves] == 'true'
+      @submissions = @submissions.where.not(cause: 'autosave') unless @show_autosaves
+      interventions = UserExerciseIntervention.where('user_id = ?  AND exercise_id = ?', @external_user.id,
+        @exercise.id)
+      @all_events = (@submissions + interventions).sort_by(&:created_at)
+      @deltas = @all_events.map.with_index do |item, index|
+        delta = item.created_at - @all_events[index - 1].created_at if index.positive?
+        delta.nil? || (delta > StatisticsHelper::WORKING_TIME_DELTA_IN_SECONDS) ? 0 : delta
+      end
+      @working_times_until = []
+      @all_events.each_with_index do |_, index|
+        @working_times_until.push((format_time_difference(@deltas[0..index].sum) if index.positive?))
+      end
+    else
+      final_submissions = Submission.where(user: @external_user,
+        exercise_id: @exercise.id).in_study_group_of(current_user).final
+      @submissions = []
+      %i[before_deadline within_grace_period after_late_deadline].each do |filter|
+        relevant_submission = final_submissions.send(filter).latest
+        @submissions.push relevant_submission if relevant_submission.present?
+      end
+      @all_events = @submissions
+    end
+
+    render 'exercises/external_users/statistics'
   end
 
   def submit

app/policies/exercise_policy.rb

@@ -5,7 +5,7 @@ class ExercisePolicy < AdminOrAuthorPolicy
     admin?
   end
 
-  %i[show? feedback? statistics? rfcs_for_exercise?].each do |action|
+  %i[show? feedback? statistics? external_user_statistics? rfcs_for_exercise?].each do |action|
     define_method(action) { admin? || teacher_in_study_group? || (teacher? && @record.public?) || author? }
   end
 

config/locales/de.yml

@@ -477,6 +477,7 @@ de:
       feedback: Feedback
       requests_for_comments: Kommentaranfragen
       study_group_dashboard: Live Dashboard
+      external_user_statistics: Statistik für externe Nutzer
     show:
       is_unpublished: Aufgabe ist deaktiviert
     statistics:

config/locales/en.yml

@@ -477,6 +477,7 @@ en:
       feedback: Feedback
       requests_for_comments: Requests for Comments
       study_group_dashboard: Live Dashboard
+      external_user_statistics: External User Statistics
     show:
       is_unpublished: Exercise is unpublished
     statistics:

config/routes.rb

@@ -117,7 +117,9 @@ Rails.application.routes.draw do
   resources :user_exercise_feedbacks, except: %i[show index]
 
   resources :external_users, only: %i[index show], concerns: :statistics do
-    resources :exercises, concerns: :statistics
+    resources :exercises do
+      get :statistics, to: 'exercises#external_user_statistics', on: :member
+    end
     member do
       get :tag_statistics
     end

spec/controllers/exercises_controller_spec.rb

@@ -236,8 +236,8 @@ describe ExercisesController do
     expect_template(:statistics)
   end
 
-  describe 'GET #statistics for external users' do
-    let(:perform_request) { get :statistics, params: params }
+  describe 'GET #external_user_statistics' do
+    let(:perform_request) { get :external_user_statistics, params: params }
     let(:params) { {id: exercise.id, external_user_id: external_user.id} }
     let(:external_user) { create(:external_user) }
 
@@ -250,7 +250,7 @@ describe ExercisesController do
     context 'when viewing the default submission statistics page without a parameter' do
       it 'does not list autosaved submissions' do
         perform_request
-        expect(assigns(:submissions)).to match_array [
+        expect(assigns(:all_events).filter {|event| event.is_a? Submission }).to match_array [
           an_object_having_attributes(cause: 'run', user_id: external_user.id),
           an_object_having_attributes(cause: 'assess', user_id: external_user.id),
           an_object_having_attributes(cause: 'run', user_id: external_user.id),
@@ -263,7 +263,7 @@ describe ExercisesController do
 
       it 'lists all submissions, including autosaved submissions' do
         perform_request
-        submissions = assigns(:submissions)
+        submissions = assigns(:all_events).filter {|event| event.is_a? Submission }
         expect(submissions).to match_array Submission.all
         expect(submissions).to include an_object_having_attributes(cause: 'autosave', user_id: external_user.id)
       end