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

Display Grace Day usage on submission history table, improve management of assessment penalty settings #1990

Merged
merged 17 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from 12 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
25 changes: 25 additions & 0 deletions app/assets/javascripts/edit_assessment.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,31 @@
}
});

// Penalties tab
$('#unlimited_submissions').on('change', function() {
$('#assessment_max_submissions').prop('disabled', $(this).prop('checked'));
});

$('#unlimited_grace_days').on('change', function() {
$('#assessment_max_grace_days').prop('disabled', $(this).prop('checked'));
});

$('#use_default_late_penalty').on('change', function() {
const $latePenaltyField = $('#assessment_late_penalty_attributes_value').parent();
$latePenaltyField.find('input').prop('disabled', $(this).prop('checked'));
$latePenaltyField.find('select').prop('disabled', $(this).prop('checked'));
});

$('#use_default_version_threshold').on('change', function() {
$('#assessment_version_threshold').prop('disabled', $(this).prop('checked'));
});

$('#use_default_version_penalty').on('change', function() {
const $versionPenaltyField = $('#assessment_version_penalty_attributes_value').parent();
$versionPenaltyField.find('input').prop('disabled', $(this).prop('checked'));
$versionPenaltyField.find('select').prop('disabled', $(this).prop('checked'));
});

});

})();
Expand Down
54 changes: 50 additions & 4 deletions app/controllers/assessments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -773,13 +773,37 @@ def edit
params[:active_tab] = "basic"
end

# make sure the penalties are set up
@assessment.late_penalty ||= Penalty.new(kind: "points")
@assessment.version_penalty ||= Penalty.new(kind: "points")

@has_annotations = @assessment.submissions.any? { |s| !s.annotations.empty? }

@is_positive_grading = @assessment.is_positive_grading

# warn instructors if the assessment is configured to allow late submissions
# but the settings do not make sense
if @assessment.end_at > @assessment.due_at
warn_message = "Late submissions are allowed, but"
if @assessment.max_grace_days == 0
flash.now[:error] ||= warn_message
flash.now[:error] += "<br>- Max grace days = 0: students can't use grace days"
end
if @assessment.effective_late_penalty.value == 0
flash.now[:error] ||= warn_message
flash.now[:error] += "<br>- Late penalty = 0: late submissions made \
without grace days are not penalized"
end
flash.now[:html_safe] = true
end
damianhxy marked this conversation as resolved.
Show resolved Hide resolved

# Used for the penalties tab
@has_unlimited_submissions = @assessment.max_submissions == -1
@has_unlimited_grace_days = @assessment.max_grace_days.nil?
@uses_default_version_threshold = @assessment.version_threshold == -1
@uses_default_late_penalty = @assessment.late_penalty.nil?
@uses_default_version_penalty = @assessment.version_penalty.nil?

# make sure the penalties are set up
# placed after the check above, so that effective_late_penalty displays the correct result
@assessment.late_penalty ||= Penalty.new(kind: "points")
@assessment.version_penalty ||= Penalty.new(kind: "points")
end

action_auth_level :update, :instructor
Expand Down Expand Up @@ -983,6 +1007,28 @@ def edit_assessment_params
@assessment.version_penalty&.destroy
end

if ActiveModel::Type::Boolean.new.cast(params[:unlimited_submissions]) == true
ass[:max_submissions] = -1
end

if ActiveModel::Type::Boolean.new.cast(params[:unlimited_grace_days]) == true
ass[:max_grace_days] = ""
end

if ActiveModel::Type::Boolean.new.cast(params[:use_default_late_penalty]) == true
ass.delete(:late_penalty_attributes)
damianhxy marked this conversation as resolved.
Show resolved Hide resolved
@assessment.late_penalty&.destroy
end

if ActiveModel::Type::Boolean.new.cast(params[:use_default_version_penalty]) == true
ass.delete(:version_penalty_attributes)
@assessment.version_penalty&.destroy
end

if ActiveModel::Type::Boolean.new.cast(params[:use_default_version_threshold]) == true
ass[:version_threshold] = -1
end

ass.delete(:name)
ass.delete(:config_file)
ass.delete(:embedded_quiz_form)
Expand Down
4 changes: 2 additions & 2 deletions app/form_builders/form_builder_with_date_time_input.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ def score_adjustment_input(name, *args)
options = args.extract_options!

fields = fields_for name do |f|
(f.vanilla_text_field :value, class: "score-box", placeholder: "10") +
(f.vanilla_text_field :value, class: "score-box", placeholder: options[:placeholder] || "", disabled: options[:disabled]) +
(@template.content_tag :div do
f.select(:kind, { "points" => "points", "%" => "percent" }, {},
class: "carrot")
class: "carrot", disabled: options[:disabled])
end)
end

Expand Down
4 changes: 2 additions & 2 deletions app/models/score_adjustment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ def kind
def to_s
case self[:kind]
when POINTS
type_str = "points"
type_str = " points"
when PERCENT
type_str = "%"
else
raise ArgumentError
end

"#{format('%+g', value)} #{type_str}"
"#{format('%g', value)}#{type_str}"
end
end
52 changes: 37 additions & 15 deletions app/views/assessments/_edit_penalties.html.erb
Original file line number Diff line number Diff line change
@@ -1,31 +1,53 @@
<%= f.text_field :max_submissions,
help_text: "The maximum number of times a student can submit the assessment.
Set this to -1 to allow unlimited submissions.",
placeholder: "E.g. 10" %>
help_text: "The maximum number of times a student can submit the assessment. \
<br>If set to -1, unlimited submissions are allowed.".html_safe,
placeholder: "10",
disabled: @has_unlimited_submissions %>
<%= label_tag(:unlimited_submissions) do %>
<%= check_box_tag :unlimited_submissions, "1", @has_unlimited_submissions %>
<%= content_tag("span", "Allow unlimited submissions.") %>
<% end %>

<%= f.text_field :max_grace_days,
help_text: "Maximum number of grace days that a student can spend on this assessment. E.g., 2. If left
blank, all of the remaining available course grace days can be spent on this assessment.",
placeholder: "Leave blank for no grace day limit" %>
help_text: "Maximum number of grace days that a student can spend on this assessment. \
<br>If blank, all available grace days can be spent on this assessment.".html_safe,
disabled: @has_unlimited_grace_days %>
<%= label_tag(:unlimited_grace_days) do %>
<%= check_box_tag :unlimited_grace_days, "1", @has_unlimited_grace_days %>
<%= content_tag("span", "Allow unlimited grace days.") %>
<% end %>

<%= f.score_adjustment_input :late_penalty,
optional: true,
help_text: "The penalty applied to late submissions after a student runs out of grace days.
It represents the number of points or a percentage of the total score removed per day, and must be a non-negative
number. If left blank, the course default is used.",
placeholder: "E.g. 15%" %>
number.<br>If blank, the course default is used.".html_safe,
disabled: @uses_default_late_penalty %>
<%= label_tag(:use_default_late_penalty) do %>
<%= check_box_tag :use_default_late_penalty, "1", @uses_default_late_penalty %>
<%= content_tag("span", "Use the course default of deducting #{@assessment.course.late_penalty} per day.") %>
<% end %>

<%= f.text_field :version_threshold, help_text: "The number of unpenalized submissions allowed. After this threshold,
each additional submission is penalized according to the version penalty. If set to -1, no submissions are penalized.
If this is left blank, the course default is used.",
placeholder: "Leave blank to use course default." %>
<%= f.text_field :version_threshold,
help_text: "The number of unpenalized submissions allowed. After this threshold,
each additional submission is penalized according to the version penalty.<br>If set to -1, no submissions are penalized.".html_safe,
disabled: @uses_default_version_threshold %>
<%= label_tag(:use_default_version_threshold) do %>
<%= check_box_tag :use_default_version_threshold, "1", @uses_default_version_threshold %>
<%= content_tag("span", "Use the course default of #{@assessment.course.version_threshold}.") %>
<% end %>

<%= f.score_adjustment_input :version_penalty, optional: true,
help_text: "The penalty applied to submissions with version greater than the version
threshold. It represents the number of points or the percentage of the total score removed per version above the
threshold, and must be a non-negative number. For example, if this is set to 1 point and the version threshold to 3,
the fifth version of a student's submissions would be docked 2 points.",
placeholder: "Leave blank to use course default." %>
threshold, and must be a non-negative number.
<br>For example, if this is set to 1 point and the version threshold to 3, the fifth version of a student's submissions would be docked 2 points.
<br>If blank, the course default is used.".html_safe,
disabled: @uses_default_version_penalty %>
<%= label_tag(:use_default_version_penalty) do %>
<%= check_box_tag :use_default_version_penalty, "1", @uses_default_version_penalty %>
<%= content_tag("span", "Use the course default of deducting #{@assessment.course.version_penalty} per submission.") %>
<% end %>

<div class="action_buttons">
<%= f.submit "Save", name: "penalties" %>
Expand Down
9 changes: 6 additions & 3 deletions app/views/assessments/_submission_history_row.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,12 @@
<% end %>

<% if @course.grace_days >= 0 then %>
<td title="Submitted <%= submission.days_late %> days late">
<span class="nobr">
<%= submission.days_late.to_s + " day".pluralize(submission.days_late) %>
<td>
<span class="nobr" title="Submitted <%= submission.days_late %> day(s) late">
<%= submission.days_late.to_s + " day".pluralize(submission.days_late) %>
</span>
<span class="nobr" title="Used <%= submission.grace_days_used %> grace day(s)">
(<%= submission.grace_days_used.to_s + " day".pluralize(submission.grace_days_used) %>)
</span>
</td>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/assessments/_submission_history_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<% end %>

<% if @course.grace_days >= 0 %>
<th>Late Days Used</th>
<th>Days Late<br>(Grace Days Used)</th>
<% end %>

<% if @assessment.version_penalty? %>
Expand Down
15 changes: 0 additions & 15 deletions app/views/assessments/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,6 @@
<div class="row">
<div class="col s12">
<%= form_for [@course, @assessment], url: edit_course_assessment_path(@course, @assessment) + "/#{params[:active_tab]}", builder: FormBuilderWithDateTimeInput do |f| %>
<% if @course.errors.any? %>
<ul>
<% @course.errors.full_messages.each do |msg| %>
<li><%= msg %></li>
<% end %>
</ul>
<% end %>

<% if @assessment.errors.any? %>
<ul>
<% @assessment.errors.full_messages.each do |msg| %>
<li><%= msg %></li>
<% end %>
</ul>
<% end %>
<div id="tab_basic">
<%= render "edit_basic", f: f %>
</div>
Expand Down
7 changes: 3 additions & 4 deletions app/views/courses/_courseFields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,15 @@
<%= f.score_adjustment_input :late_penalty, optional: true, help_text: "The penalty applied to late submissions after
a student runs out of grace days. It represents an amount of points or a percentage of the total score removed per
day, and must be a non-negative number. This field can be overriden on a per-assessment basis.",
placeholder: "E.g. 15%" %>
placeholder: "10" %>

<%= f.text_field :version_threshold, help_text: "If a submission's version is greater than this threshold, it is
penalized according to the version penalty. If set to -1, no submissions are penalized. This is required, but can be
overridden on a per-assessment basis.", placeholder: "E.g. 10" %>
penalized according to the version penalty. If set to -1, no submissions are penalized. This field can be overriden on a per-assessment basis.", placeholder: "10" %>

<%= f.score_adjustment_input :version_penalty, optional: true, help_text: "The penalty applied to submissions with
versions greater than the version threshold, i.e. number of points or percentage of the total score removed per
version above the threshold. For example, if this is set to 1 point and the version threshold to 3, the fifth
version of a student's submissions would be docked 2 points.", placeholder: "E.g. 15%" %>
version of a student's submissions would be docked 2 points. This field can be overriden on a per-assessment basis.", placeholder: "10" %>

<%= f.date_select :start_date, help_text: "When the course becomes active.",
less_than: "course_end_date" %>
Expand Down