From f3feb8545d0ff4155e5d1c4e1fdaac02263fd61b Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 20 May 2022 14:17:41 +0200 Subject: [PATCH 1/5] Create total column --- .../stylesheets/components/table.css.scss | 5 +++ app/controllers/series_controller.rb | 6 ++++ app/views/series/_scoresheet_table.html.erb | 35 +++++++++++++++++++ app/views/series/scoresheet.html.erb | 2 +- app/views/series/scoresheet.js.erb | 2 +- config/locales/views/series/en.yml | 1 + config/locales/views/series/nl.yml | 1 + 7 files changed, 50 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/components/table.css.scss b/app/assets/stylesheets/components/table.css.scss index fdf220dc31..8327a9b59b 100644 --- a/app/assets/stylesheets/components/table.css.scss +++ b/app/assets/stylesheets/components/table.css.scss @@ -121,3 +121,8 @@ tr.gu-mirror { font-size: 12px; color: $text-secondary; } + +#scoresheet-table-wrapper .summary-col { + font-size: 12px; + color: $text-secondary; +} diff --git a/app/controllers/series_controller.rb b/app/controllers/series_controller.rb index 75eef419d6..c3984cab0f 100644 --- a/app/controllers/series_controller.rb +++ b/app/controllers/series_controller.rb @@ -207,12 +207,18 @@ def scoresheet @submission_counts = Hash.new(0) @read_state_counts = Hash.new(0) + @summary_by_user = Hash.new(0) + @total = Hash.new(0) @users.each do |user| @activities.each do |activity| if activity.exercise? && @submissions[[user.id, activity.id]].present? @submission_counts[[activity.id, @submissions[[user.id, activity.id]].status]] += 1 + @summary_by_user[[user.id, @submissions[[user.id, activity.id]].status]] += 1 + @total[@submissions[[user.id, activity.id]].status] += 1 elsif activity.content_page? && @read_states[[user.id, activity.id]].present? @read_state_counts[activity.id] += 1 + @summary_by_user[[user.id, :correct]] += 1 + @total[:correct] += 1 end end end diff --git a/app/views/series/_scoresheet_table.html.erb b/app/views/series/_scoresheet_table.html.erb index 3cb4832f11..7b4b791bdd 100644 --- a/app/views/series/_scoresheet_table.html.erb +++ b/app/views/series/_scoresheet_table.html.erb @@ -3,6 +3,7 @@ <%= t('series.scoresheet.user') %> + <%= t('series.scoresheet.total') %> <% activities.each do |activity| %> <%= activity.name %> @@ -13,6 +14,23 @@ <%= t('series.scoresheet.users', count: users.length) %> + + <% i = 0 %> + <% statuses.each do |status| %> + <% count = total[status] %> + <% if count > 0 %> + <% if i > 0 %> + · + <% end %> + <% i += 1 %> + + <%= count %> + <%= status_icon(status, 12) %> + + <% else %> + <% end %> + <% end %> + <% activities.each do |activity| %> <% if activity.exercise? %> @@ -48,6 +66,23 @@ <% users.each do |student| %> <%= link_to student.full_name, course_member_path(course, student), title: student.full_name, class: "ellipsis-overflow" %> + + <% i = 0 %> + <% statuses.each do |status| %> + <% count = summary_by_user[[student.id, status]] %> + <% if count > 0 %> + <% if i > 0 %> + · + <% end %> + <% i += 1 %> + + <%= count %> + <%= status_icon(status, 12) %> + + <% else %> + <% end %> + <% end %> + <% activities.each do |activity| %> <% if activity.exercise? %> <% submission = submissions[[student.id, activity.id]] %> diff --git a/app/views/series/scoresheet.html.erb b/app/views/series/scoresheet.html.erb index 4dce1b3661..07c8c2d1a1 100644 --- a/app/views/series/scoresheet.html.erb +++ b/app/views/series/scoresheet.html.erb @@ -35,7 +35,7 @@ course_labels: @course_labels } %>
- <%= render partial: "scoresheet_table", locals: {course: @course, series: @series, activities: @activities, users: @users, submissions: @submissions, read_states: @read_states, statuses: @statuses, read_state_counts: @read_state_counts, submission_counts: @submission_counts} %> + <%= render partial: "scoresheet_table", locals: {course: @course, series: @series, activities: @activities, users: @users, submissions: @submissions, read_states: @read_states, statuses: @statuses, read_state_counts: @read_state_counts, submission_counts: @submission_counts, summary_by_user: @summary_by_user, total: @total} %>
<% else %> <%= t('.no_exercises_html', series_edit_url: edit_series_path(@series)) %> diff --git a/app/views/series/scoresheet.js.erb b/app/views/series/scoresheet.js.erb index ee1ff9d4be..e582d0824e 100644 --- a/app/views/series/scoresheet.js.erb +++ b/app/views/series/scoresheet.js.erb @@ -1 +1 @@ -$("#scoresheet-table-wrapper").html("<%= escape_javascript(render partial: 'scoresheet_table', locals: {course: @course, series: @series, activities: @activities, users: @users, submissions: @submissions, read_states: @read_states, statuses: @statuses, read_state_counts: @read_state_counts, submission_counts: @submission_counts}) %>") +$("#scoresheet-table-wrapper").html("<%= escape_javascript(render partial: 'scoresheet_table', locals: {course: @course, series: @series, activities: @activities, users: @users, submissions: @submissions, read_states: @read_states, statuses: @statuses, read_state_counts: @read_state_counts, submission_counts: @submission_counts, summary_by_user: @summary_by_user, total: @total}) %>") diff --git a/config/locales/views/series/en.yml b/config/locales/views/series/en.yml index 4d8ade3d8f..303a60a9aa 100644 --- a/config/locales/views/series/en.yml +++ b/config/locales/views/series/en.yml @@ -61,6 +61,7 @@ en: course_view: Course scoresheet select_view: Select view status: "Status %{activity}" + total: Total users: zero: No users one: 1 user diff --git a/config/locales/views/series/nl.yml b/config/locales/views/series/nl.yml index 96d6462090..645f9f07f0 100644 --- a/config/locales/views/series/nl.yml +++ b/config/locales/views/series/nl.yml @@ -61,6 +61,7 @@ nl: course_view: Statusoverzicht van cursus select_view: Weergave selecteren status: "Status %{activity}" + total: Totaal users: zero: Geen gebruikers one: 1 gebruiker From db6d9ffe30c58aa4b0f1def64e86b596100a339d Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 20 May 2022 15:27:34 +0200 Subject: [PATCH 2/5] Add sort on scoresheet total column --- app/controllers/series_controller.rb | 6 +++++- app/models/user.rb | 11 +++++++++++ app/views/series/_scoresheet_table.html.erb | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/controllers/series_controller.rb b/app/controllers/series_controller.rb index c3984cab0f..5da05c4ab0 100644 --- a/app/controllers/series_controller.rb +++ b/app/controllers/series_controller.rb @@ -18,8 +18,12 @@ class SeriesController < ApplicationController has_scope :order_by, using: %i[column direction], only: :scoresheet, type: :hash do |controller, scope, value| column, direction = value if %w[ASC DESC].include?(direction) - if column == 'status_in_course_and_name' + case column + when 'status_in_course_and_name' scope.order_by_status_in_course_and_name direction + when 'submission_statuses_in_series' + series = Series.find(controller.params[:id]) + scope.order_by_submission_statuses_in_series direction, series else series = Series.find(controller.params[:id]) if series.activities.exists? id: column diff --git a/app/models/user.rb b/app/models/user.rb index 77429ee24d..191085b250 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -142,6 +142,17 @@ class User < ApplicationRecord joins("LEFT JOIN (#{submissions.to_sql}) submissions ON submissions.user_id = users.id") .reorder 'submissions.status': direction } + scope :order_by_submission_statuses_in_series, lambda { |direction, series| + submissions = Submission.in_series(series) + submissions = submissions.before_deadline(series.deadline) if series.deadline.present? + submissions = submissions.group(:user_id, :exercise_id).most_recent + # Create columns for each status with the count of submissions that have that status + cols = Submission.statuses.map { |k, v| "COUNT(CASE WHEN status = #{v} THEN 1 END) AS #{k.parameterize(separator: '_')}" } + submissions = submissions.group(:user_id).select :user_id, cols + # Order by those status columns + joins("LEFT JOIN (#{submissions.to_sql}) submissions ON submissions.user_id = users.id") + .reorder Submission.statuses.keys.map { |k| "submissions.#{k.parameterize(separator: '_')} #{direction}" }.join(',') + } scope :order_by_solved_exercises_in_series, lambda { |direction, series| submissions = Submission.in_series(series) submissions = submissions.before_deadline(series.deadline) if series.deadline.present? diff --git a/app/views/series/_scoresheet_table.html.erb b/app/views/series/_scoresheet_table.html.erb index 7b4b791bdd..0dddac6aeb 100644 --- a/app/views/series/_scoresheet_table.html.erb +++ b/app/views/series/_scoresheet_table.html.erb @@ -3,7 +3,7 @@ <%= t('series.scoresheet.user') %> - <%= t('series.scoresheet.total') %> + <%= t('series.scoresheet.total') %> <% activities.each do |activity| %> <%= activity.name %> From 19c556d0c358f25b5e8d396f72792fba3f1bdd0e Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 20 May 2022 15:33:58 +0200 Subject: [PATCH 3/5] Add tests on new order method --- test/models/user_test.rb | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 1205841b9a..a1b3835b5e 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -366,6 +366,34 @@ def oauth_hash_for(user) assert_equal [u3.id, u2.id, u1.id], User.in_course(c).order_by_solved_exercises_in_series('DESC', s).pluck(:id) end + test 'should be able to order by submission statuses in series' do + c = create :course + s = create :series, course: c + e1 = create :exercise + e2 = create :exercise + SeriesMembership.create series: s, activity: e1 + SeriesMembership.create series: s, activity: e2 + u1 = create :user + CourseMembership.create user: u1, course: c, status: 'student' + u2 = create :user + CourseMembership.create user: u2, course: c, status: 'student' + create :wrong_submission, user: u2, course: c, exercise: e1 + u3 = create :user + CourseMembership.create user: u3, course: c, status: 'student' + create :correct_submission, user: u3, course: c, exercise: e1 + u4 = create :user + CourseMembership.create user: u4, course: c, status: 'student' + create :correct_submission, user: u4, course: c, exercise: e1 + create :wrong_submission, user: u4, course: c, exercise: e2 + u5 = create :user + CourseMembership.create user: u5, course: c, status: 'student' + create :correct_submission, user: u5, course: c, exercise: e1 + create :correct_submission, user: u5, course: c, exercise: e2 + + assert_equal [u1.id, u2.id, u3.id, u4.id, u5.id], User.in_course(c).order_by_submission_statuses_in_series('ASC', s).pluck(:id) + assert_equal [u5.id, u4.id, u3.id, u2.id, u1.id], User.in_course(c).order_by_submission_statuses_in_series('DESC', s).pluck(:id) + end + test 'should be able to order by progress' do User.destroy_all c = create :course From fdde7ec3511759e7cec134216327e99c9fccea28 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 23 May 2022 10:50:06 +0200 Subject: [PATCH 4/5] Display correct sum including read states --- app/controllers/series_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/series_controller.rb b/app/controllers/series_controller.rb index 5da05c4ab0..8c60cf76d4 100644 --- a/app/controllers/series_controller.rb +++ b/app/controllers/series_controller.rb @@ -221,8 +221,8 @@ def scoresheet @total[@submissions[[user.id, activity.id]].status] += 1 elsif activity.content_page? && @read_states[[user.id, activity.id]].present? @read_state_counts[activity.id] += 1 - @summary_by_user[[user.id, :correct]] += 1 - @total[:correct] += 1 + @summary_by_user[[user.id, 'correct']] += 1 + @total['correct'] += 1 end end end From 9be4b17f94079250a61858cee49616ed67b15f0f Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 23 May 2022 11:55:41 +0200 Subject: [PATCH 5/5] Add read states to sort --- app/models/user.rb | 13 ++++++++----- test/models/user_test.rb | 15 ++++++++++++--- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 191085b250..8eda41d795 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -145,13 +145,16 @@ class User < ApplicationRecord scope :order_by_submission_statuses_in_series, lambda { |direction, series| submissions = Submission.in_series(series) submissions = submissions.before_deadline(series.deadline) if series.deadline.present? - submissions = submissions.group(:user_id, :exercise_id).most_recent + submissions = submissions.group(:user_id, :exercise_id).most_recent.select(:user_id, :status) + read_states = ActivityReadState.in_series(series) + read_states = read_states.before_deadline(series.deadline) if series.deadline.present? + read_states = read_states.select(:user_id, 'NULL AS status') # Create columns for each status with the count of submissions that have that status - cols = Submission.statuses.map { |k, v| "COUNT(CASE WHEN status = #{v} THEN 1 END) AS #{k.parameterize(separator: '_')}" } - submissions = submissions.group(:user_id).select :user_id, cols + cols = Submission.statuses.map { |k, v| "COUNT(CASE WHEN status = #{v} #{'OR status is NULL' if k == 'correct'} THEN 1 END) AS #{k.parameterize(separator: '_')}" } + combined_sql = "(SELECT user_id, #{cols.join(',')} FROM ((#{read_states.to_sql}) UNION ALL (#{submissions.to_sql})) AS c GROUP BY user_id)" # Order by those status columns - joins("LEFT JOIN (#{submissions.to_sql}) submissions ON submissions.user_id = users.id") - .reorder Submission.statuses.keys.map { |k| "submissions.#{k.parameterize(separator: '_')} #{direction}" }.join(',') + joins("LEFT JOIN (#{combined_sql}) c ON c.user_id = users.id") + .reorder Submission.statuses.keys.map { |k| "c.#{k.parameterize(separator: '_')} #{direction}" }.join(',') } scope :order_by_solved_exercises_in_series, lambda { |direction, series| submissions = Submission.in_series(series) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index a1b3835b5e..8917946449 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -371,8 +371,12 @@ def oauth_hash_for(user) s = create :series, course: c e1 = create :exercise e2 = create :exercise + a1 = create :content_page + a2 = create :content_page SeriesMembership.create series: s, activity: e1 SeriesMembership.create series: s, activity: e2 + SeriesMembership.create series: s, activity: a1 + SeriesMembership.create series: s, activity: a2 u1 = create :user CourseMembership.create user: u1, course: c, status: 'student' u2 = create :user @@ -388,10 +392,15 @@ def oauth_hash_for(user) u5 = create :user CourseMembership.create user: u5, course: c, status: 'student' create :correct_submission, user: u5, course: c, exercise: e1 - create :correct_submission, user: u5, course: c, exercise: e2 + create :activity_read_state, user: u5, course: c, activity: a1 + u6 = create :user + CourseMembership.create user: u6, course: c, status: 'student' + create :correct_submission, user: u6, course: c, exercise: e1 + create :activity_read_state, user: u6, course: c, activity: a1 + create :activity_read_state, user: u6, course: c, activity: a2 - assert_equal [u1.id, u2.id, u3.id, u4.id, u5.id], User.in_course(c).order_by_submission_statuses_in_series('ASC', s).pluck(:id) - assert_equal [u5.id, u4.id, u3.id, u2.id, u1.id], User.in_course(c).order_by_submission_statuses_in_series('DESC', s).pluck(:id) + assert_equal [u1.id, u2.id, u3.id, u4.id, u5.id, u6.id], User.in_course(c).order_by_submission_statuses_in_series('ASC', s).pluck(:id) + assert_equal [u6.id, u5.id, u4.id, u3.id, u2.id, u1.id], User.in_course(c).order_by_submission_statuses_in_series('DESC', s).pluck(:id) end test 'should be able to order by progress' do