Refactor statistic rendering to improve performance

- With these changes, we mostly pass the desired objects to the view, so that the results can be re-used there.
- Furthermore, we improve the statistics shown for teachers, so that these align more with the expectations (and the actual submissions belonging to the teacher's study groups).

Author: MrSerth

app/controllers/exercises_controller.rb

@@ -494,14 +494,14 @@ def reload
 
   def statistics
     # Show general statistic page for specific exercise
-    contributor_statistics = {'InternalUser' => {}, 'ExternalUser' => {}, 'ProgrammingGroup' => {}}
+    contributor_statistics = {InternalUser => {}, ExternalUser => {}, ProgrammingGroup => {}}
 
     query = policy_scope(Submission).select('contributor_id, contributor_type, MAX(score) AS maximum_score, COUNT(id) AS runs')
       .where(exercise_id: @exercise.id)
       .group('contributor_id, contributor_type')
 
     query.each do |tuple|
-      contributor_statistics[tuple['contributor_type']][tuple['contributor_id'].to_i] = tuple
+      contributor_statistics[tuple.contributor_type.constantize][tuple.contributor] = tuple
     end
 
     render locals: {

app/controllers/external_users_controller.rb

@@ -69,9 +69,12 @@ def statistics
 
     statistics = {}
 
-    ApplicationRecord.connection.exec_query(working_time_query(tag&.id)).each do |tuple|
+    working_time_statistics = ApplicationRecord.connection.exec_query(working_time_query(tag&.id))
+    attempted_exercises = Exercise.where(id: working_time_statistics.pluck('exercise_id'))
+    working_time_statistics.each do |tuple|
       tuple = tuple.merge('working_time' => format_time_difference(tuple['working_time']))
-      statistics[tuple['exercise_id'].to_i] = tuple
+      exercise = attempted_exercises.find {|attempted_exercise| attempted_exercise.id == tuple['exercise_id'] }
+      statistics[exercise] = tuple
     end
 
     render locals: {

app/models/exercise.rb

@@ -51,9 +51,9 @@ class Exercise < ApplicationRecord
 
   MAX_GROUP_EXERCISE_FEEDBACKS = 20
 
-  def average_percentage
-    if average_score && (maximum_score.to_d != BigDecimal('0.0')) && submissions.exists?(cause: 'submit')
-      (average_score / maximum_score * 100).round(2)
+  def average_percentage(base = submissions)
+    if average_score(base) && (maximum_score.to_d != BigDecimal('0.0')) && base.exists?(cause: 'submit')
+      (average_score(base) / maximum_score * 100).round(2)
     else
       0
     end
@@ -67,9 +67,9 @@ def finishers_percentage
     end
   end
 
-  def average_score
+  def average_score(base = submissions)
     Submission.from(
-      submissions.group(:contributor_id, :contributor_type)
+      base.group(:contributor_id, :contributor_type)
                  .select('MAX(score) as max_score')
     ).average(:max_score).to_f
   end
@@ -549,8 +549,8 @@ def solved_by?(contributor)
     maximum_score(contributor).to_i == maximum_score.to_i
   end
 
-  def finishers_count
-    Submission.from(submissions.where(score: maximum_score, cause: %w[submit assess remoteSubmit remoteAssess]).group(:contributor_id, :contributor_type).select(:contributor_id, :contributor_type), 'submissions').count
+  def finishers_count(base = submissions)
+    Submission.from(base.where(score: maximum_score, cause: %w[submit assess remoteSubmit remoteAssess]).group(:contributor_id, :contributor_type).select(:contributor_id, :contributor_type), 'submissions').count
   end
 
   def set_default_values

app/models/submission.rb

@@ -44,7 +44,7 @@ class Submission < ApplicationRecord
                                 joins(:exercise).where('submissions.updated_at > exercises.late_submission_deadline')
                               }
 
-  scope :latest, -> { order(updated_at: :desc).first }
+  scope :latest, -> { order(submissions: {updated_at: :desc}).first }
 
   validates :cause, inclusion: {in: CAUSES}
 

app/views/exercises/statistics.html.slim

@@ -5,53 +5,50 @@
   - append_javascript_pack_tag('d3-tip')
 h1 = @exercise
 
-= row(label: '.participants', value: @exercise.contributors.size)
+- statistic_base = policy_scope(Submission).where(exercise: @exercise).unscope(where: :cause)
+- participants = statistic_base.group(:contributor_id, :contributor_type).count.size
+- finishers_count = @exercise.finishers_count(statistic_base)
+- average_score = @exercise.average_score(statistic_base)
+- average_percentage = @exercise.average_percentage(statistic_base)
+
+= row(label: '.participants', value: participants)
 
 - %i[intermediate final].each do |scope|
   / i18n-tasks-use t('.intermediate_submissions') t('.final_submissions')
   = row(label: ".#{scope}_submissions") do
     /TODO: Refactor next line
-    = "#{@exercise.submissions.send(scope).count} (#{t('.users_and_programming_groups', count: Submission.from(@exercise.submissions.send(scope).group(:contributor_id, :contributor_type).select(:contributor_id, :contributor_type)).count)})"
+    = "#{statistic_base.send(scope).count} (#{t('.users_and_programming_groups', count: Submission.from(statistic_base.send(scope).group(:contributor_id, :contributor_type).select(:contributor_id, :contributor_type)).count)})"
 
 = row(label: '.finishing_rate') do
   p
-    - if @exercise.finishers_count
-      span.number
-        = @exercise.finishers_count
-      =<> t('shared.out_of')
-      span.number
-        = @exercise.contributors.size
-      =< t('exercises.statistics.external_users')
+    - if finishers_count
+      = t('.users_and_programming_groups', count: "#{finishers_count} #{t('shared.out_of')} #{participants}")
     - else
       = empty
-  - finishers_count = @exercise.contributors.size
-  - finishers_percentage = finishers_count.zero? ? 0 : (100.0 / finishers_count * @exercise.finishers_count).round(2)
+  - finishers_percentage = participants.zero? ? 0 : (100.0 / participants * finishers_count).round(2)
   p = progress_bar(finishers_percentage)
 
 = row(label: '.average_score') do
   p
-    - if @exercise.average_score
-      span.number
-        = @exercise.average_score.round(2)
+    - if average_score
+      = average_score.round(2)
       =<> t('shared.out_of')
-      span.number
-        = @exercise.maximum_score
+      = format('%.1f', @exercise.maximum_score)
     - else
       = empty
-  p = progress_bar(@exercise.average_percentage)
+  p = progress_bar(average_percentage)
 
 - if policy(@exercise).detailed_statistics?
   = row(label: '.average_worktime') do
     p = @exercise.average_working_time
 
-- {internal_users: t('.internal_users'), external_users: t('.external_users'), programming_groups: t('.programming_groups')}.each_pair do |symbol, label|
-  - submissions = policy_scope(Submission).where(contributor: @exercise.send(symbol), exercise: @exercise)
-  - if submissions.any?
-    strong = label
-    - if symbol == :external_users
+- contributor_statistics.each_pair do |user_type, user_with_submission_stats|
+  - if user_with_submission_stats.any?
+    h5 = t(".#{user_type.model_name.collection}")
+    - if user_type == ExternalUser
       - working_time_array = []
-      - @exercise.send(symbol).distinct.each do |user|
-        - working_time = @exercise.average_working_time_for(user) || 0
+      - user_with_submission_stats.each_key do |contributor|
+        - working_time = @exercise.average_working_time_for(contributor) || 0
         - working_time_array.push working_time
       hr
       .d-none#data data-working-time=ActiveSupport::JSON.encode(working_time_array)
@@ -60,9 +57,8 @@ h1 = @exercise
       hr
       #chart_2
       hr
-    - contributors = symbol.to_s.classify.constantize.where(id: submissions.joins(symbol).group(:contributor_id).select(:contributor_id).distinct)
     .table-responsive.mb-4
-      table.table.table-striped class=(contributors.present? ? 'sortable' : '')
+      table.table.table-striped class=(user_with_submission_stats.present? ? 'sortable' : '')
         thead
           tr
             th.header = t('.user')
@@ -71,12 +67,10 @@ h1 = @exercise
             th.header = t('.runs') if policy(@exercise).detailed_statistics?
             th.header = t('.worktime') if policy(@exercise).detailed_statistics?
         tbody
-          - contributors.each do |contributor|
-            - us = contributor_statistics[contributor.class.name][contributor.id] || {'maximum_score' => nil, 'runs' => nil}
-            - label = contributor.displayname.to_s
+          - user_with_submission_stats.each do |contributor, submission_stat|
             tr
-              td = link_to_if symbol == :external_users && policy(contributor).statistics?, label, {controller: 'exercises', action: 'external_user_statistics', external_user_id: contributor.id, id: @exercise.id}
-              td = us['maximum_score'] || 0
+              td = link_to_if user_type == ExternalUser && policy(contributor).statistics?, contributor.displayname, statistics_external_user_exercise_path(contributor, @exercise)
+              td = submission_stat['maximum_score'] || '0.0'
               td.align-middle
                 - latest_user_submission = @exercise.final_submission(contributor)
                 - if latest_user_submission.present?
@@ -86,5 +80,5 @@ h1 = @exercise
                     .deadline-result.unknown-result
                   - elsif latest_user_submission.after_late_deadline?
                     .deadline-result.negative-result
-              td = us['runs'] if policy(@exercise).detailed_statistics?
-              td = @exercise.average_working_time_for(contributor) || 0 if policy(@exercise).detailed_statistics?
+              td = submission_stat['runs'] if policy(@exercise).detailed_statistics?
+              td = @exercise.average_working_time_for(contributor) || '00:00:00' if policy(@exercise).detailed_statistics?

app/views/external_users/statistics.html.slim

@@ -1,27 +1,23 @@
 h1 = t('.title', user: @user.displayname)
 
-- submissions = policy_scope(Submission).where(contributor: @user)
-- exercises = Exercise.where(id: submissions.joins(:exercise).group(:exercise_id).select(:exercise_id).distinct).compact
-
-- if submissions.any?
+- if statistics.present?
   .table-responsive
-    table.table.table-striped class=(exercises.present? ? 'sortable' : '')
+    table.table.table-striped class=(statistics.present? ? 'sortable' : '')
       thead
         tr
           th.header = t('.exercise')
           th.header = t('.score')
           th.header = t('.deadline')
-          th.header = t('.runs') if policy(exercises.first).detailed_statistics?
-          th.header = t('.worktime') if policy(exercises.first).detailed_statistics?
+          th.header = t('.runs') if policy(statistics.keys.first).detailed_statistics?
+          th.header = t('.worktime') if policy(statistics.keys.first).detailed_statistics?
       tbody
-        - exercises.each do |exercise|
+        - statistics.each do |exercise, stats|
           // Grab any submission in context of study group (or all if admin). Then check for permission
-          - any_submission = submissions.where(exercise:).first
-          - if any_submission && policy(any_submission.study_group).show? && statistics[exercise.id]
-            - stats = statistics[exercise.id]
+          - any_submission = @user.submissions.where(exercise:).first
+          - if any_submission && policy(any_submission.study_group).show?
             tr
-              td = link_to exercise, controller: 'exercises', action: 'external_user_statistics', external_user_id: @user.id, id: exercise.id
-              td = stats['maximum_score'] || 0
+              td = link_to exercise, statistics_external_user_exercise_path(@user, exercise)
+              td = stats['maximum_score'] || 0.0
               td.align-middle
                 - latest_viewable_submission = exercise.final_submission(@user)
                 - if latest_viewable_submission.present?
@@ -31,7 +27,7 @@ h1 = t('.title', user: @user.displayname)
                     .deadline-result.unknown-result.within_grace_period
                   - elsif latest_viewable_submission.after_late_deadline?
                     .deadline-result.negative-result.after_late_deadline
-              td = stats['runs'] || 0 if policy(exercises.first).detailed_statistics?
-              td = stats['working_time'] || 0 if policy(exercises.first).detailed_statistics?
+              td = stats['runs'] || 0 if policy(exercise).detailed_statistics?
+              td = stats['working_time'] || '00:00:00' if policy(exercise).detailed_statistics?
 - else
   = t('exercises.external_users.statistics.no_data_available')

config/locales/en/exercise.yml

@@ -215,7 +215,7 @@ en:
       finishing_rate: Finishing Rate
       intermediate_submissions: Intermediate Submissions
       internal_users: Internal Users
-      participants: Participating Users
+      participants: Participants
       programming_groups: Programming Groups
       runs: Runs
       score: Maximum Score