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

Fix setting user overrides for dates on the user detail page for set versions. #2345

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
8 changes: 8 additions & 0 deletions htdocs/js/DatePicker/datepicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,14 @@
// input to fix that.
this.altInput.after(this.input);

// Move the id of the now hidden input onto the added input so the labels still work.
this.altInput.id = this.input.id;

// Remove the placeholder from the hidden input. Flatpickr has copied that to the added input, and
// that isn't valid on a hidden input.
this.input.removeAttribute('id');
this.input.removeAttribute('placeholder');

// Make the alternate input left-to-right even for right-to-left languages.
this.altInput.dir = 'ltr';

Expand Down
16 changes: 0 additions & 16 deletions htdocs/js/UserDetail/userdetail.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,6 @@
}
});

// Make the date override checkboxes checked or unchecked appropriately
// as determined by the value of the date input when that value changes.
document
.querySelectorAll('input[type="text"][data-override],input[type="hidden"][data-override]')
.forEach((input) => {
const overrideCheck = document.getElementById(input.dataset.override);
if (!overrideCheck) return;
const changeHandler = () => (overrideCheck.checked = input.value != '');
input.addEventListener('change', changeHandler);
// Attach the keyup and blur handlers to the flatpickr alternate input.
input.previousElementSibling?.addEventListener('keyup', changeHandler);
input.previousElementSibling?.addEventListener('blur', () => {
if (input.previousElementSibling.value == '') overrideCheck.checked = false;
});
});

// If the "Assign All Sets to Current User" button is clicked, then check all assignments.
document.getElementsByName('assignAll').forEach((button) => {
button.addEventListener('click', () => {
Expand Down
74 changes: 37 additions & 37 deletions lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use WeBWorK::Debug;
use constant DATE_FIELDS => {
open_date => x('Open:'),
reduced_scoring_date => x('Reduced:'),
due_date => x('Closes:'),
due_date => x('Close:'),
answer_date => x('Answer:')
};
use constant DATE_FIELDS_ORDER => [qw(open_date reduced_scoring_date due_date answer_date )];
Expand Down Expand Up @@ -90,10 +90,11 @@ sub initialize ($c) {
unless ($rh_dates->{error}) {
# If no error update database
for my $field (@{ DATE_FIELDS_ORDER() }) {
if (defined $c->param("set.$setID.$field.override")) {
if ($c->param("set.$setID.$field") && $c->param("set.$setID.$field") ne '') {
$userSetRecord->$field($rh_dates->{$field});
} else {
$userSetRecord->$field(undef); #stop override
# Stop override
$userSetRecord->$field(undef);
}
}
$db->putUserSet($userSetRecord);
Expand All @@ -110,18 +111,22 @@ sub initialize ($c) {
if (defined $action) {
if ($action eq 'assigned') {
# This version is not to be deleted.
# Check to see if we're resetting the dates for this version.
# Check to see if the dates have been changed for this version.
# Note that dates are never reset (set to NULL) for a set version.
my $rh_dates = $c->checkDates($setVersionRecord, "$setID,v$ver");
unless ($rh_dates->{error}) {
for my $field (@{ DATE_FIELDS_ORDER() }) {
if (defined($c->param("set.$setID,v$ver.$field.override"))) {
$setVersionRecord->$field($rh_dates->{$field});
} else {
$setVersionRecord->$field(undef);
}
$setVersionRecord->$field($rh_dates->{$field})
if ($c->param("set.$setID,v$ver.$field")
&& $c->param("set.$setID,v$ver.$field") ne '');
}
$db->putSetVersion($setVersionRecord);
}
# Reset the date inputs to the date in the database if they were empty.
for my $field (@{ DATE_FIELDS_ORDER() }) {
$c->param("set.$setID,v$ver.$field", $setVersionRecord->$field)
unless $c->param("set.$setID,v$ver.$field");
}
} elsif ($action eq 'delete') {
# Delete this version.
$db->deleteSetVersion($editForUserID, $setID, $ver);
Expand Down Expand Up @@ -168,59 +173,54 @@ sub initialize ($c) {
}

sub checkDates ($c, $setRecord, $setID) {
my $error = 0;

# For each of the dates, use the override date if set. Otherwise use the value from the global set.
# Except in the case that this is a set version. In that case use 0 which will result in an error below.
# This will prevent the dates for the set version from being changed to invalid values in that case.
my %dates;
for my $field (@{ DATE_FIELDS_ORDER() }) {
$dates{$field} =
(defined $c->param("set.$setID.$field.override") && $c->param("set.$setID.$field") ne '')
($c->param("set.$setID.$field") && $c->param("set.$setID.$field") ne '')
? $c->param("set.$setID.$field")
: $setRecord->$field;
: ($setID =~ /,v\d+$/ ? 0 : $setRecord->$field);
}

my ($open_date, $reduced_scoring_date, $due_date, $answer_date) = map { $dates{$_} } @{ DATE_FIELDS_ORDER() };

unless ($answer_date && $due_date && $open_date) {
$c->addbadmessage("set $setID has errors in its dates: answer_date |$answer_date|, "
. "due date |$due_date|, open_date |$open_date|");
$error = 1;
$c->addbadmessage($c->maketext(
'Set [_1] has errors in its dates. Open Date: [_2] , Close Date: [_3], Answer Date: [_4]',
$setID,
map { $_ ? $c->formatDateTime($_, 'datetime_format_short') : $c->maketext('required') }
($open_date, $due_date, $answer_date)
));
return { %dates, error => 1 };
}

if ($answer_date < $due_date || $answer_date < $open_date) {
$c->addbadmessage("Answers cannot be made available until on or after the due date in set $setID!");
$error = 1;
}

if ($due_date < $open_date) {
$c->addbadmessage("Answers cannot be due until on or after the open date in set $setID!");
$error = 1;
}
my $error = 0;

if ($c->ce->{pg}{ansEvalDefaults}{enableReducedScoring}
&& $setRecord->enable_reduced_scoring
&& ($reduced_scoring_date < $open_date || $reduced_scoring_date > $due_date))
{
$c->addbadmessage("The reduced scoring date should be between the open date and the due date in set $setID!");
$c->addbadmessage($c->maketext(
'The reduced scoring date must be between the open date and the close date for set [_1].', $setID
));
$error = 1;
}

# Make sure the dates are not more than 10 years in the future.
my $cutoff = time + 31_556_926 * 10;
if ($open_date > $cutoff) {
$c->addbadmessage("Error: open date cannot be more than 10 years from now in set $setID");
$error = 1;
}
if ($due_date > $cutoff) {
$c->addbadmessage("Error: due date cannot be more than 10 years from now in set $setID");
if ($due_date < $open_date) {
$c->addbadmessage($c->maketext('The close date must be on or after the open date for set [_1].', $setID));
$error = 1;
}
if ($answer_date > $cutoff) {
$c->addbadmessage("Error: answer date cannot be more than 10 years from now in set $setID");

if ($answer_date < $due_date) {
$c->addbadmessage(
$c->maketext('Answers cannot be made available until on or after the close date for set [_1].', $setID)
);
$error = 1;
}

$c->addbadmessage('No date changes were saved!') if ($error);
$c->addbadmessage($c->maketext('Not saving dates for [_1]!', $setID)) if $error;

return { %dates, error => $error };
}
Expand Down
12 changes: 4 additions & 8 deletions templates/ContentGenerator/Instructor/UserDetail.html.ep
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,12 @@
</div>
%
<div class="table-responsive">
<table class="table table-bordered table-sm font-sm align-middle">
<tr>
<th class="text-center" colspan="3">
<%= maketext("Sets assigned to [_1] ([_2])", $userName, $userID) =%>
</th>
</tr>
<table class="table table-bordered table-sm font-sm align-middle w-auto caption-top">
<caption><%= maketext("Sets assigned to [_1] ([_2])", $userName, $userID) =%></caption>
<tr>
<th class="text-center"><%= maketext('Assignment') %></th>
<th class="text-center"><%= maketext('Assigned') %></th>
<th><%= maketext("Edit set for [_1]", $userID) %></th>
<th><%= maketext('Dates') %></th>
<th class="text-center"><%= maketext('Dates') %></th>
</tr>
% for my $set (@{ $c->{setRecords} }) {
% my $setID = $set->set_id;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,31 @@
% my $setID = $globalRecord->set_id;
%
% # Modify set id to include the version if this is a versioned set.
% my $isVersioned = 0;
% if (defined $mergedRecord && $mergedRecord->assignment_type =~ /gateway/ && $mergedRecord->can('version_id')) {
% $setID .= ',v' . $mergedRecord->version_id;
% $isVersioned = 1;
% }
% my $isGateway = $globalRecord->assignment_type =~ /gateway/;
% my $isVersioned = $isGateway && defined $mergedRecord && $mergedRecord->can('version_id');
% $setID .= ',v' . $mergedRecord->version_id if $isVersioned;
%
<table>
<tr>
<th scope="col" colspan="3">
<%= maketext("User overrides") =%>
</th>
<th scope="col">
<%= maketext("Set values") =%>
</th>
% unless ($isVersioned) {
<th scope="col" colspan="2">
<%= maketext('Assignment Dates') =%>
</th>
% }
% if (defined $userRecord) {
<th scope="col" <%== defined $userRecord && !$isVersioned ? '' : 'colspan="2"' %>>
% if ($isVersioned) {
<a tabindex="0" class="help-popup" role="button" data-bs-toggle="popover" data-bs-trigger="focus"
data-bs-content="<%= maketext(
'If the test was timed, granting the user an additional version '
. 'may be preferred to changing its dates.' )%>">
<i class="icon fa-solid fa-question-circle fa-lg" aria-hidden="true"></i><% =%>\
<span class="visually-hidden"><%= maketext('Help Icon') %></span><% =%>\
</a>
% }
<%= $isVersioned ? maketext(q{User's Test Version Dates}) : maketext('User Overrides') =%>
</th>
% }
</tr>
% for my $field (@$fields) {
% # Skip reduced credit dates for sets which don't have them.
Expand All @@ -23,52 +34,48 @@
% && (!$ce->{pg}{ansEvalDefaults}{enableReducedScoring} || !$globalRecord->enable_reduced_scoring);
%
<tr>
% my $globalValue = $globalRecord->$field;
%
<td class="px-1 text-nowrap">
% if (defined $userRecord) {
<%= label_for "set.$setID.$field.override_id" => maketext($fieldLabels->{$field}),
class => 'form-check-label' =%>
% } else {
<%= maketext($fieldLabels->{$field}) =%>
% }
</td>
<td class="px-1 text-nowrap">
% if (defined $userRecord) {
<%= check_box "set.$setID.$field.override" => $field,
id => "set.$setID.$field.override_id", class => 'form-check-input',
(defined $mergedRecord ? $mergedRecord->$field : $globalValue) ne $globalValue
|| ($isVersioned && $field ne 'reduced_scoring_date')
? (checked => undef)
: () =%>
% }
<%= label_for "set.$setID.$field" . (defined $userRecord ? '_id' : '.class_value') =>
maketext($fieldLabels->{$field}),
class => 'form-label mb-0' =%>
</td>
<td class="px-1 text-nowrap">
% if (defined $userRecord) {
% unless ($isVersioned) {
<td class="px-1 text-nowrap">
<%= text_field "set.$setID.$field.class_value" =>
$c->formatDateTime($globalRecord->$field, 'datetime_format_short'),
id => "set.$setID.$field.class_value", readonly => undef, dir => 'ltr',
class => 'form-control-plaintext form-control-sm w-auto',
size => 16,
defined $userRecord ? ('aria-labelledby' => "set.$setID.${field}_id") : () =%>
</td>
% }
% if (defined $userRecord) {
<td class="px-1 text-nowrap">
<div class="input-group input-group-sm flex-nowrap flatpickr">
<%= text_field "set.$setID.$field" =>
defined $userRecord ? $userRecord->$field : $globalValue,
<%= text_field "set.$setID.$field" => $userRecord->$field,
id => "set.$setID.${field}_id",
placeholder => maketext('None Specified'),
class => 'form-control w-auto' . ($field eq 'open_date' ? ' datepicker-group' : ''),
placeholder => $isGateway
? ($isVersioned && $field ne 'reduced_scoring_date'
? maketext('Required')
: maketext('Test Default'))
: maketext('Assignment Default'),
$isVersioned && $field ne 'reduced_scoring_date' ? (required => undef) : (),
data => {
override => "set.$setID.$field.override_id",
input => undef,
done_text => maketext('Done'),
today_text => maketext('Today'),
now_text => maketext('Now'),
locale => $ce->{language},
timezone => $ce->{siteDefaults}{timezone}
} =%>
<a class="btn btn-secondary btn-sm" data-toggle><i class="fas fa-calendar-alt"></i></a>
<a class="btn btn-secondary btn-sm" data-toggle tabindex="0" role="button"
aria-label="<%= $c->maketext('Pick date and time') %>">
<i class="fas fa-calendar-alt"></i>
</a>
</div>
% }
</td>
<td class="px-1 text-nowrap">
<span dir="ltr">
<%= $c->formatDateTime($globalValue, 'datetime_format_short') =%>
</span>
</td>
</td>
% }
</tr>
% }
</table>
32 changes: 21 additions & 11 deletions templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep
Original file line number Diff line number Diff line change
@@ -1,30 +1,40 @@
% use WeBWorK::Utils qw(format_set_name_display);
%
% # my ($set, $userSet, $mergedSet, $version) = @_;
% my $setID = $set->set_id;
% my $isGateway = $set->assignment_type =~ /gateway/;
% my $version = stash 'version';
<tr>
<td class="text-center">
<label class="form-check-label">
<%= check_box $version ? "set.$setID,v$version.assignment" : "set.$setID.assignment" => 'assigned',
class => 'form-check-input', defined $mergedSet ? (checked => undef) : () =%>
</label>
</td>
<td class="text-center">
<th class="text-center" scope="row">
% if (defined $mergedSet) {
<b dir="ltr">
<span dir="ltr">
% if ($isGateway) {
<i class="icon fa-solid fa-list-check" title="<%= maketext('Test/Quiz') %>"></i>
<span class="visually-hidden"><%= maketext('Test/Quiz') %></span>
% }
<%= link_to format_set_name_display($version ? "$setID (version $version)" : $setID) =>
$c->systemLink(
url_for('instructor_set_detail', setID => $setID . ($version ? ",v$version" : '')),
params => { editForUser => $userID }
) =%>
</b>
</span>
% if ($version) {
<%= hidden_field "set.$setID,v$version.assignment" => 'delete' =%>
% }
% } else {
<b dir="ltr"><%= format_set_name_display($setID) %></b>
<span dir="ltr">
% if ($isGateway) {
<i class="icon fa-solid fa-list-check" title="<%= maketext('Test/Quiz') %>"></i>
<span class="visually-hidden"><%= maketext('Test/Quiz') %></span>
% }
<%= format_set_name_display($setID) %>
</span>
% }
</th>
<td class="text-center">
<label class="form-check-label">
<%= check_box $version ? "set.$setID,v$version.assignment" : "set.$setID.assignment" => 'assigned',
class => 'form-check-input', defined $mergedSet ? (checked => undef) : () =%>
</label>
</td>
<td class="text-center">
<%= include 'ContentGenerator/Instructor/UserDetail/set_date_table',
Expand Down
12 changes: 7 additions & 5 deletions templates/HelpFiles/InstructorUserDetail.html.ep
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@
. 'is deleted and cannot be recovered. Use this action cautiously!') =%>
</p>
<p>
<%= maketext('Change the due dates for an individual student on this page. Check the checkbox and enter the new '
. 'date in order to override the date. (You can copy the date format from the date in the right hand column '
. 'which indicates the date when the homework set is due for the whole class.) Note that you should ensure '
. 'that the close date is before the answer date. If the close date for a student is extended until after the '
. 'class answer date for the set, then the answer date for the student must also be set to a later date. ') =%>
<%= maketext('Change the due dates for an individual student on this page. Enter a new date in order to '
. 'override the date. (You can copy the date format from the date in the left column which indicates '
. 'the date when the homework set is due for the whole class.) Note that you should ensure that the close '
. 'date is before the answer date. If the close date for a student is extended until after the class answer '
. 'date for the set, then the answer date for the student must also be set to a later date. If reduced '
. 'scoring is enabled for the set, then the reduced scoring date must be between the open and close dates. '
. 'If a date is left empty, then the assignment default date will be used.') =%>
</p>
<p class="mb-0">
<%= maketext('Click on the homework set links to edit the grades for this individual student on the homework set. '
Expand Down