Introduce a new DeadlineScope for Submissions

With this scope, we extend the submissions shown to teachers. Now, a teacher has always access to the latest scored submission and the submission with the highest score (in addition to all "submit" submissions). Furthermore, for these two categories (latest / highest scored), we also respect the three deadlines (before submission deadline, within grace period, after late submission deadline) and show a respective submission.

The SubmissionsController#index has not been extended with this new scope, since the performance is too bad (and it is not used frequently).

Author: MrSerth

app/controllers/exercises_controller.rb

@@ -496,7 +496,8 @@ def statistics
     # Show general statistic page for specific exercise
     contributor_statistics = {InternalUser => {}, ExternalUser => {}, ProgrammingGroup => {}}
 
-    query = policy_scope(Submission).select('contributor_id, contributor_type, MAX(score) AS maximum_score, COUNT(id) AS runs')
+    query = SubmissionPolicy::DeadlineScope.new(current_user, Submission).resolve
+      .select('contributor_id, contributor_type, MAX(score) AS maximum_score, COUNT(id) AS runs')
       .where(exercise_id: @exercise.id)
       .group('contributor_id, contributor_type')
 
@@ -512,10 +513,15 @@ def statistics
   def external_user_statistics
     # Render statistics page for one specific external user
 
-    submissions = policy_scope(Submission).where(contributor: @external_user, exercise: @exercise)
-      .order(:created_at)
+    submissions = SubmissionPolicy::DeadlineScope.new(current_user, Submission).resolve
+      .where(contributor: @external_user, exercise: @exercise)
+      .order(submissions: {updated_at: :desc})
       .includes(:exercise, testruns: [:testrun_messages, {file: [:file_type]}], files: [:file_type])
 
+    # From here on, we switch to sort by `created_at`. This is important for the working time estimation,
+    # since a submission is updated after the corresponding testrun finishes.
+    # Sorting by `updated_at` would lead to wrong working time estimations (but is more efficient for the database).
+
     if policy(@exercise).detailed_statistics?
       @show_autosaves = params[:show_autosaves] == 'true' || submissions.where.not(cause: 'autosave').none?
 
@@ -549,11 +555,7 @@ def external_user_statistics
         end
       end
     else
-      @all_events = []
-      %i[before_deadline within_grace_period after_late_deadline].each do |filter|
-        relevant_submission = submissions.send(filter).latest
-        @all_events.push relevant_submission if relevant_submission.present?
-      end
+      @all_events = submissions.sort_by(&:created_at)
     end
 
     render 'exercises/external_users/statistics'

app/controllers/external_users_controller.rb

@@ -22,7 +22,12 @@ def show
   end
 
   def working_time_query(tag = nil)
+    deadline_scope_conditions = SubmissionPolicy::DeadlineScope.new(current_user, Submission).resolve
+
     "
+    WITH filtered_submissions AS (
+      #{deadline_scope_conditions.to_sql}
+    )
     SELECT contributor_id,
            bar.exercise_id,
            max(score) as maximum_score,
@@ -41,16 +46,17 @@ def working_time_query(tag = nil)
          (SELECT contributor_id,
                  exercise_id,
                  max(score) AS score,
-                 id,
-                 (created_at - lag(created_at) over (PARTITION BY contributor_id, exercise_id
-                                                     ORDER BY created_at)) AS working_time
-          FROM submissions
+                 filtered_submissions.id,
+                 (filtered_submissions.updated_at - lag(filtered_submissions.updated_at) over (PARTITION BY contributor_id, exercise_id
+                                                     ORDER BY filtered_submissions.updated_at)) AS working_time
+          FROM filtered_submissions
+          JOIN exercises ON filtered_submissions.exercise_id = exercises.id
           WHERE #{ExternalUser.sanitize_sql(['contributor_id = ?', @user.id])}
             AND contributor_type = 'ExternalUser'
-          #{current_user.admin? ? '' : "AND #{ExternalUser.sanitize_sql(['study_group_id IN (?)', current_user.study_groups.pluck(:id)])} AND cause = 'submit'"}
           GROUP BY exercise_id,
                    contributor_id,
-                   id
+                   filtered_submissions.id,
+                   filtered_submissions.updated_at
           ) AS foo
       ) AS bar
     #{tag.nil? ? '' : " JOIN exercise_tags et ON et.exercise_id = bar.exercise_id AND #{ExternalUser.sanitize_sql(['et.tag_id = ?', tag])}"}

app/policies/submission_policy.rb

@@ -23,7 +23,11 @@ def index?
   end
 
   def show?
-    admin? || author? || author_in_programming_group? || (teacher_in_study_group? && CausesScope.new(@user, @record).resolve.include?(@record.cause))
+    return true if admin? || author? || author_in_programming_group?
+    # Performance optimization: If this submission is within the CausesScope, we can skip the expensive DeadlineScope
+    return true if teacher_in_study_group? && CausesScope.new(@user, @record).resolve.include?(@record.cause)
+
+    teacher_in_study_group? && DeadlineScope.new(@user, Submission).resolve.exists?(@record.id)
   end
 
   class Scope < Scope
@@ -49,4 +53,69 @@ def resolve
       end
     end
   end
+
+  class DeadlineScope < Scope
+    def resolve
+      resolved_scope = super
+      return resolved_scope unless @user.teacher?
+
+      latest_before_deadline = latest_submissions_assessed.before_deadline.arel
+      latest_within_grace_period = latest_submissions_assessed.within_grace_period.arel
+      latest_after_late_deadline = latest_submissions_assessed.after_late_deadline.arel
+      highest_before_deadline = latest_submissions_assessed(highest_scored: true).before_deadline.arel
+      highest_within_grace_period = latest_submissions_assessed(highest_scored: true).within_grace_period.arel
+      highest_after_late_deadline = latest_submissions_assessed(highest_scored: true).after_late_deadline.arel
+
+      # Yes, we construct a huge union of seven relations: all three deadlines, all three highest scores and the resolved scope
+      all_unions = construct_union(
+        latest_before_deadline,
+        latest_within_grace_period,
+        latest_after_late_deadline,
+        highest_before_deadline,
+        highest_within_grace_period,
+        highest_after_late_deadline,
+        # Dirty hack, since resolved_scope.arel will loose the bindings, and replace them with $1, $2, ...
+        Arel.sql(resolved_scope.to_sql)
+      )
+
+      # Convert the union to a relation
+      Submission.from(all_unions.as(Submission.arel_table.name))
+    end
+
+    private
+
+    # This method is used to get the latest submission that was assessed or remote assessed.
+    # By default, it will simply return the one with the last time stamp per exercise and contributor.
+    # However, with the optional parameter, the highest scored submission that was scored the latest will be returned.
+    def latest_submissions_assessed(highest_scored: false)
+      submission_table = Submission.arel_table
+
+      desired_table_order = [
+        submission_table[:exercise_id],
+        submission_table[:contributor_type],
+        submission_table[:contributor_id],
+        highest_scored ? submission_table[:score].desc.nulls_last : nil,
+        submission_table[:updated_at].desc,
+      ].compact
+
+      Submission.from(
+        submission_table.project(
+          Arel.sql('DISTINCT ON (submissions.exercise_id, submissions.contributor_type, submissions.contributor_id) submissions.*')
+        )
+        .order(*desired_table_order)
+        .where(submission_table[:cause].in(%w[assess remoteAssess]))
+        .where(submission_table[:study_group_id].in(@user.study_group_ids_as_teacher))
+        .as(submission_table.name)
+      )
+    end
+
+    def construct_union(*args)
+      return nil if args.empty?
+      return args.first if args.size == 1
+
+      args.reduce do |union, arg|
+        Arel::Nodes::Union.new(union, arg)
+      end
+    end
+  end
 end

app/views/exercises/statistics.html.slim

@@ -42,6 +42,14 @@ h1 = @exercise
   = row(label: '.average_worktime') do
     p = @exercise.average_working_time
 
+- unless policy(@exercise).detailed_statistics?
+  .lead
+    .card.border-info-subtle.mb-3
+      .card-header
+        i.fa-solid.fa-circle-info.text-info
+        strong.text-info
+          => t('.final_submissions_only', count: contributor_statistics.values.sum(&:count))
+        = t('.final_submissions_only_explanation')
 - contributor_statistics.each_pair do |user_type, user_with_submission_stats|
   - if user_with_submission_stats.any?
     h5 = t(".#{user_type.model_name.collection}")

config/locales/de/exercise.yml

@@ -212,6 +212,8 @@ de:
       deadline: Abgabefrist
       external_users: Externe Personen
       final_submissions: Finale Abgaben
+      final_submissions_only: 'Details zu %{count} Teilnehmenden und Programmiergruppen:'
+      final_submissions_only_explanation: Die folgenden Ansichten zeigen ausschließlich finale Abgaben und Bewertungen. Dies umfasst explizit eingereichte Lösungen sowie die jeweils letzte Bewertung vor Ablauf der Abgabefrist.
       finishing_rate: Abschlussrate
       intermediate_submissions: Intermediäre Abgaben
       internal_users: Interne Personen

config/locales/en/exercise.yml

@@ -212,6 +212,8 @@ en:
       deadline: Deadline
       external_users: External Users
       final_submissions: Final Submissions
+      final_submissions_only: 'Details for %{count} distinct users and programming groups only:'
+      final_submissions_only_explanation: The following views display only the final submissions and assessments. This includes explicitly submitted solutions and the most recent assessment before the submission deadline was reached.
       finishing_rate: Finishing Rate
       intermediate_submissions: Intermediate Submissions
       internal_users: Internal Users