Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add summary column to series scoresheets #3642

Merged
merged 7 commits into from
May 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/assets/stylesheets/components/table.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,8 @@ tr.gu-mirror {
font-size: 12px;
color: $text-secondary;
}

#scoresheet-table-wrapper .summary-col {
font-size: 12px;
color: $text-secondary;
}
12 changes: 11 additions & 1 deletion app/controllers/series_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -213,12 +217,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
Expand Down
14 changes: 14 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,20 @@ 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.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} #{'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 (#{combined_sql}) c ON c.user_id = users.id")
.reorder Submission.statuses.keys.map { |k| "c.#{k.parameterize(separator: '_')} #{direction}" }.join(',')
}
scope :order_by_activity_read_state_in_series, lambda { |direction, content_page, series|
read_states = ActivityReadState.in_series(series).of_content_page(content_page)
read_states = read_states.before_deadline(series.deadline) if series.deadline.present?
Expand Down
35 changes: 35 additions & 0 deletions app/views/series/_scoresheet_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<thead>
<tr>
<th class="user-name"><dodona-sort-button column="status_in_course_and_name" default="ASC"> <%= t('series.scoresheet.user') %></dodona-sort-button></th>
<th class="status-header ellipsis-overflow"><dodona-sort-button column="submission_statuses_in_series"> <%= t('series.scoresheet.total') %></dodona-sort-button></th>
<% activities.each do |activity| %>
<th class="status-header ellipsis-overflow" title="<%= activity.name %>">
<dodona-sort-button column="<%= activity.id %>"><%= activity.name %></dodona-sort-button>
Expand All @@ -13,6 +14,23 @@
<tbody>
<tr class="summary-row">
<td class="user-name ellipsis-overflow"><%= t('series.scoresheet.users', count: users.length) %></td>
<td class="status">
<% i = 0 %>
<% statuses.each do |status| %>
<% count = total[status] %>
<% if count > 0 %>
<% if i > 0 %>
&middot;
<% end %>
<% i += 1 %>
<span data-bs-toggle="tooltip" title="<%= count %> <%= Submission.human_enum_name(:status, status) %>">
<%= count %>
<%= status_icon(status, 12) %>
</span>
<% else %>
<% end %>
<% end %>
</td>
<% activities.each do |activity| %>
<% if activity.exercise? %>
<td class="status">
Expand Down Expand Up @@ -48,6 +66,23 @@
<% users.each do |student| %>
<tr>
<td class="user-name ellipsis-overflow"><%= link_to student.full_name, course_member_path(course, student), title: student.full_name, class: "ellipsis-overflow" %></td>
<td class="status summary-col">
<% i = 0 %>
<% statuses.each do |status| %>
<% count = summary_by_user[[student.id, status]] %>
<% if count > 0 %>
<% if i > 0 %>
&middot;
<% end %>
<% i += 1 %>
<span data-bs-toggle="tooltip" title="<%= count %> <%= Submission.human_enum_name(:status, status) %>">
<%= count %>
<%= status_icon(status, 12) %>
</span>
<% else %>
<% end %>
<% end %>
</td>
<% activities.each do |activity| %>
<% if activity.exercise? %>
<% submission = submissions[[student.id, activity.id]] %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/series/scoresheet.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
course_labels: @course_labels
} %>
<div id="scoresheet-table-wrapper">
<%= 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} %>
</div>
<% else %>
<span><%= t('.no_exercises_html', series_edit_url: edit_series_path(@series)) %></span>
Expand Down
2 changes: 1 addition & 1 deletion app/views/series/scoresheet.js.erb
Original file line number Diff line number Diff line change
@@ -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}) %>")
1 change: 1 addition & 0 deletions config/locales/views/series/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions config/locales/views/series/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 37 additions & 0 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,43 @@ def oauth_hash_for(user)
assert_equal [u4.id, 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
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
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 :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, 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
User.destroy_all
c = create :course
Expand Down