From 828191ba7c6d828528142ba8c6325b1e95769168 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Tue, 27 Feb 2024 07:44:27 -0600 Subject: [PATCH 1/5] Fix setting user overrides for dates on the user detail page for set versions. This addresses issue #2340. Currently if an instructor unchecks a checkbox for a user's versioned set on the user detail page (the page you can reach by clicking on the "Assigned Sets" column for a user in the Accounts Manager), then when dates are checked (in the `checkDates` method of `UserDetail.pm`), the dates from the global set record are used. If those are valid dates (they usually will be in this case), then those valid dates are returned with no error. Thus the database will be updated. However, the logic used to determine how the database is updated does not match the logic used to check the sets, and since the override checkbox is not checked, the date in the user's versioned set record is set to NULL. So for versioned sets this makes it so that if the checkbox is not checked or the override date is not set, then the global set date is NOT used for a fallback. Instead 0 is used for a fallback. Since that is not a valid date, this forces an error, and the database is not updated. This is actually a more general issue with the logic used when the dates are checked and the logic used when the database is updated not being the same. So that is fixed. Additionally when the dates in the database are updated, there are cases where the user interface does not correctly reflect the changes made. One such case is for a regular set if the checkbox for a date is checked but the override date left empty. Then the global set date is used (meaning the user set value for that date is set to NULL in the database), however the checkbox remains checked. So the corresponding parameters are updated so that the changes are reflected when the page rerenders after saving. Note that in the case that the dates are not updated in the database due to an error in the given dates, the user interface does not update to reflect the current database state. I don't think the user interface should be updated in this case, as the instructor might want an opportunity to correct the error and not have their changes wiped out. I also noticed that the "User overrides" header was being displayed over the "Open", "Closes", "Answer" labels in the case that a set is not assigned to a user (and so there are no override fields). So that is not shown in that case anymore. I also removed the 10 year limit on dates. I am not sure why we impose this limit. There is no reason to do so. In fact, it is quite useful in many situations to have a due date that is more than 10 years in the future. By the way, I was curious if these dates were affected by the year 2038 problem. So after removing this restriction I set a due date to be in 2050. This seems to work with no issue. So it seems that 64bit integers are being used in this case which are not affected by the year 2038 problem. So we have 292 million years before this will be an issue again. --- .../ContentGenerator/Instructor/UserDetail.pm | 53 +++++++++---------- .../UserDetail/set_date_table.html.ep | 4 +- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm b/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm index e91aeac478..9065ebf42a 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm @@ -90,10 +90,14 @@ 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 (defined $c->param("set.$setID.$field.override") && $c->param("set.$setID.$field") ne '') { $userSetRecord->$field($rh_dates->{$field}); } else { $userSetRecord->$field(undef); #stop override + + # Update the parameters so the user interface reflects the changes made. + $c->param("set.$setID.$field.override", undef); + $c->param("set.$setID.$field", undef); } } $db->putUserSet($userSetRecord); @@ -110,18 +114,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 defined $c->param("set.$setID,v$ver.$field.override") + && $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); @@ -168,25 +176,27 @@ 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") - : $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("Set $setID has errors in its dates: " + . "open_date |$open_date|, due date |$due_date|, answer_date |$answer_date|"); + return { %dates, error => 1 }; } + my $error = 0; + 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; @@ -205,22 +215,7 @@ sub checkDates ($c, $setRecord, $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"); - $error = 1; - } - if ($answer_date > $cutoff) { - $c->addbadmessage("Error: answer date cannot be more than 10 years from now in set $setID"); - $error = 1; - } - - $c->addbadmessage('No date changes were saved!') if ($error); + $c->addbadmessage('No date changes were saved!') if $error; return { %dates, error => $error }; } diff --git a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep index eb36e11f85..3a69c40412 100644 --- a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep +++ b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep @@ -10,7 +10,9 @@ - - +
- <%= maketext("User overrides") =%> + % if (defined $userRecord) { + <%= maketext("User overrides") =%> + % } <%= maketext("Set values") =%> From aba6fff6ee0f1d46696a3f0fcc4ea14530e75e14 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 29 Feb 2024 20:13:29 -0600 Subject: [PATCH 2/5] More improvements to the User Detail page. The header on the user override dates now read "User Overrides" unless it is a test version in which case it is "User's Test Version Dates". The header on the dates for the global set reads "Test Dates" for tests and "Assignment Dates" for other assignments. The placeholder in the user override date inputs now reads "Test Default" for tests, and "Assignment Default" for other assignments. The date override checkboxes have been removed. Remove the "s" from the end of the "Close" date label. That was inconsistent with the other date labels. The class dates for the set are now in readonly inputs instead of just being text. This is the same as on the set detail page. One advantage of this is that it makes it easier to cut and paste those dates. There is also a small tweak to the datepicker.js (which is used on this page as well as on the sets manager page and the set detail page). When flatpickr adds the date picker, it hides the input originally on the page, and adds another input. This moves the id from the original input onto the added flatpickr input so that labels still work. The ids aren't used for anything else after the date picker javascript has found them. Also remove the placeholder that flatpickr copies to the new input since that isn't valid on a hidden input. --- htdocs/js/DatePicker/datepicker.js | 8 ++ htdocs/js/UserDetail/userdetail.js | 16 ---- .../ContentGenerator/Instructor/UserDetail.pm | 17 ++-- .../UserDetail/set_date_table.html.ep | 80 +++++++++---------- .../HelpFiles/InstructorUserDetail.html.ep | 3 +- 5 files changed, 56 insertions(+), 68 deletions(-) diff --git a/htdocs/js/DatePicker/datepicker.js b/htdocs/js/DatePicker/datepicker.js index 3cbe63dfc6..767d5ce99b 100644 --- a/htdocs/js/DatePicker/datepicker.js +++ b/htdocs/js/DatePicker/datepicker.js @@ -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'; diff --git a/htdocs/js/UserDetail/userdetail.js b/htdocs/js/UserDetail/userdetail.js index 5295406e3f..a1d990dc0b 100644 --- a/htdocs/js/UserDetail/userdetail.js +++ b/htdocs/js/UserDetail/userdetail.js @@ -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', () => { diff --git a/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm b/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm index 9065ebf42a..dedfc7a163 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm @@ -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 )]; @@ -90,14 +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") && $c->param("set.$setID.$field") ne '') { + if ($c->param("set.$setID.$field") && $c->param("set.$setID.$field") ne '') { $userSetRecord->$field($rh_dates->{$field}); } else { - $userSetRecord->$field(undef); #stop override - - # Update the parameters so the user interface reflects the changes made. - $c->param("set.$setID.$field.override", undef); - $c->param("set.$setID.$field", undef); + # Stop override + $userSetRecord->$field(undef); } } $db->putUserSet($userSetRecord); @@ -120,8 +117,8 @@ sub initialize ($c) { unless ($rh_dates->{error}) { for my $field (@{ DATE_FIELDS_ORDER() }) { $setVersionRecord->$field($rh_dates->{$field}) - if defined $c->param("set.$setID,v$ver.$field.override") - && $c->param("set.$setID,v$ver.$field") ne ''; + if ($c->param("set.$setID,v$ver.$field") + && $c->param("set.$setID,v$ver.$field") ne ''); } $db->putSetVersion($setVersionRecord); } @@ -182,7 +179,7 @@ sub checkDates ($c, $setRecord, $setID) { 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") : ($setID =~ /,v\d+$/ ? 0 : $setRecord->$field); } diff --git a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep index 3a69c40412..8a701af054 100644 --- a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep +++ b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep @@ -1,22 +1,22 @@ % 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; % - - + % if (defined $userRecord) { + + % } + % unless ($isVersioned) { + + % } % for my $field (@$fields) { % # Skip reduced credit dates for sets which don't have them. @@ -28,33 +28,24 @@ % my $globalValue = $globalRecord->$field; % - - - + + % } + % unless ($isVersioned) { + + % } % }
- % if (defined $userRecord) { - <%= maketext("User overrides") =%> - % } - - <%= maketext("Set values") =%> - + <%= $isVersioned ? maketext(q{User's Test Version Dates}) : maketext('User Overrides') =%> + > + <%= $isGateway ? maketext('Test Dates') : maketext('Assignment Dates') =%> +
- % if (defined $userRecord) { - <%= label_for "set.$setID.$field.override_id" => maketext($fieldLabels->{$field}), - class => 'form-check-label' =%> - % } else { - <%= maketext($fieldLabels->{$field}) =%> - % } - - % 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' =%> - % if (defined $userRecord) { + % if (defined $userRecord) { +
<%= text_field "set.$setID.$field" => defined $userRecord ? $userRecord->$field : $globalValue, 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'), @@ -62,15 +53,22 @@ locale => $ce->{language}, timezone => $ce->{siteDefaults}{timezone} } =%> - + + +
- % } - 		- - <%= $c->formatDateTime($globalValue, 'datetime_format_short') =%> - - + <%= text_field "set.$setID.$field.class_value" => + $c->formatDateTime($globalValue, 'datetime_format_short'), + id => "set.$setID.$field.class_value", readonly => undef, dir => 'ltr', + class => 'form-control form-control-sm w-auto', + defined $userRecord ? ('aria-labelledby' => "set.$setID.${field}_id") : () =%> +
diff --git a/templates/HelpFiles/InstructorUserDetail.html.ep b/templates/HelpFiles/InstructorUserDetail.html.ep index 557ccf7ea9..d7e2dc5ab3 100644 --- a/templates/HelpFiles/InstructorUserDetail.html.ep +++ b/templates/HelpFiles/InstructorUserDetail.html.ep @@ -27,7 +27,8 @@ . '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. ') =%> + . 'class answer date for the set, then the answer date for the student must also be set to a later date. ' + . 'If a date is left empty, then the assignment default date will be used.') =%>

<%= maketext('Click on the homework set links to edit the grades for this individual student on the homework set. ' From 4164a7cf06e0a776537a7f6a7b1359f2b0fab3f7 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Thu, 29 Feb 2024 21:32:49 -0800 Subject: [PATCH 3/5] rearrange tables in user sets page --- .../Instructor/UserDetail.html.ep | 12 +++---- .../UserDetail/set_date_table.html.ep | 34 ++++++++++--------- .../Instructor/UserDetail/set_row.html.ep | 31 +++++++++++------ 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/templates/ContentGenerator/Instructor/UserDetail.html.ep b/templates/ContentGenerator/Instructor/UserDetail.html.ep index c0305ca478..1ef3720df6 100644 --- a/templates/ContentGenerator/Instructor/UserDetail.html.ep +++ b/templates/ContentGenerator/Instructor/UserDetail.html.ep @@ -83,16 +83,12 @@ %

- - - - +
- <%= maketext("Sets assigned to [_1] ([_2])", $userName, $userID) =%> -
+ + - - + % for my $set (@{ $c->{setRecords} }) { % my $setID = $set->set_id; diff --git a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep index 8a701af054..6386618f0b 100644 --- a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep +++ b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep @@ -5,16 +5,16 @@ % my $isVersioned = $isGateway && defined $mergedRecord && $mergedRecord->can('version_id'); % $setID .= ',v' . $mergedRecord->version_id if $isVersioned; % -
<%= maketext("Sets assigned to [_1] ([_2])", $userName, $userID) =%>
<%= maketext('Assignment') %> <%= maketext('Assigned') %><%= maketext("Edit set for [_1]", $userID) %><%= maketext('Dates') %><%= maketext('Dates') %>
+
> - % if (defined $userRecord) { + % unless ($isVersioned) { % } - % unless ($isVersioned) { - % } @@ -32,13 +32,24 @@ maketext($fieldLabels->{$field}), class => 'form-label mb-0' =%> + % unless ($isVersioned) { + + % } % if (defined $userRecord) { % } - % unless ($isVersioned) { - - % } % }
- <%= $isVersioned ? maketext(q{User's Test Version Dates}) : maketext('User Overrides') =%> + <%= maketext('Assignment Dates') =%> > - <%= $isGateway ? maketext('Test Dates') : maketext('Assignment Dates') =%> + % if (defined $userRecord) { + > + <%= $isVersioned ? maketext(q{User's Test Version Dates}) : maketext('User Overrides') =%>
+ <%= text_field "set.$setID.$field.class_value" => + $c->formatDateTime($globalValue, '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") : () =%> +
<%= text_field "set.$setID.$field" => defined $userRecord ? $userRecord->$field : $globalValue, id => "set.$setID.${field}_id", - class => 'form-control w-auto' . ($field eq 'open_date' ? ' datepicker-group' : ''), + class => 'form-control w-auto' + . ($field eq 'open_date' ? ' datepicker-group' : ''), placeholder => $isGateway ? ($isVersioned && $field ne 'reduced_scoring_date' ? maketext('Required') @@ -60,15 +71,6 @@
- <%= text_field "set.$setID.$field.class_value" => - $c->formatDateTime($globalValue, 'datetime_format_short'), - id => "set.$setID.$field.class_value", readonly => undef, dir => 'ltr', - class => 'form-control form-control-sm w-auto', - defined $userRecord ? ('aria-labelledby' => "set.$setID.${field}_id") : () =%> -
diff --git a/templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep b/templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep index df4d683b12..33d26d9397 100644 --- a/templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep +++ b/templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep @@ -2,29 +2,40 @@ % % # my ($set, $userSet, $mergedSet, $version) = @_; % my $setID = $set->set_id; +% my $isGateway = $set->assignment_type =~ /gateway/; % my $version = stash 'version';
- - + % if (defined $mergedSet) { - + + % if ($isGateway) { + + + % } <%= 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 } ) =%> - + % if ($version) { <%= hidden_field "set.$setID,v$version.assignment" => 'delete' =%> % } % } else { - <%= format_set_name_display($setID) %> + + % if ($isGateway) { + + + % } + <%= format_set_name_display($setID) %> + % } + + <%= include 'ContentGenerator/Instructor/UserDetail/set_date_table', From f70b0c9b0b27491e1088c477fc9cefe649fd58cf Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Fri, 1 Mar 2024 15:49:36 -0600 Subject: [PATCH 4/5] Add a popover to the user detail date tables for test versions with advice. The advice is to consider alternatve measures to changing dates for test versions. Also remove the `ms-auto` class added to the dates table for test versions. The alignment is still not right with that. --- .../Instructor/UserDetail/set_date_table.html.ep | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep index 6386618f0b..25bd7d166d 100644 --- a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep +++ b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep @@ -5,7 +5,7 @@ % my $isVersioned = $isGateway && defined $mergedRecord && $mergedRecord->can('version_id'); % $setID .= ',v' . $mergedRecord->version_id if $isVersioned; % -> +
% unless ($isVersioned) { % } % if (defined $userRecord) { - % } @@ -48,8 +57,7 @@ <%= text_field "set.$setID.$field" => defined $userRecord ? $userRecord->$field : $globalValue, id => "set.$setID.${field}_id", - class => 'form-control w-auto' - . ($field eq 'open_date' ? ' datepicker-group' : ''), + class => 'form-control w-auto' . ($field eq 'open_date' ? ' datepicker-group' : ''), placeholder => $isGateway ? ($isVersioned && $field ne 'reduced_scoring_date' ? maketext('Required') From 5200f0caf601b71ca32798b895b33407e96b11bd Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Fri, 1 Mar 2024 07:00:55 -0600 Subject: [PATCH 5/5] Fix the help for the User Detail page. It still references the check boxes for the date overrides which no longer exist. Also translate the error messages from when the dates are checked. --- .../ContentGenerator/Instructor/UserDetail.pm | 30 ++++++++++++------- .../UserDetail/set_date_table.html.ep | 7 ++--- .../Instructor/UserDetail/set_row.html.ep | 1 - .../HelpFiles/InstructorUserDetail.html.ep | 11 +++---- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm b/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm index dedfc7a163..3913aa60ab 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm @@ -187,32 +187,40 @@ sub checkDates ($c, $setRecord, $setID) { 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: " - . "open_date |$open_date|, due date |$due_date|, answer_date |$answer_date|"); + $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 }; } my $error = 0; - 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!"); + if ($c->ce->{pg}{ansEvalDefaults}{enableReducedScoring} + && $setRecord->enable_reduced_scoring + && ($reduced_scoring_date < $open_date || $reduced_scoring_date > $due_date)) + { + $c->addbadmessage($c->maketext( + 'The reduced scoring date must be between the open date and the close date for set [_1].', $setID + )); $error = 1; } if ($due_date < $open_date) { - $c->addbadmessage("Answers cannot be due until on or after the open date in set $setID!"); + $c->addbadmessage($c->maketext('The close date must be on or after the open date for set [_1].', $setID)); $error = 1; } - 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!"); + 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 }; } diff --git a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep index 25bd7d166d..01a2496e96 100644 --- a/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep +++ b/templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep @@ -34,8 +34,6 @@ % && (!$ce->{pg}{ansEvalDefaults}{enableReducedScoring} || !$globalRecord->enable_reduced_scoring); % - % my $globalValue = $globalRecord->$field; - %
@@ -13,7 +13,16 @@ > + > + % if ($isVersioned) { + + <% =%>\ + <% =%>\ + + % } <%= $isVersioned ? maketext(q{User's Test Version Dates}) : maketext('User Overrides') =%>
<%= label_for "set.$setID.$field" . (defined $userRecord ? '_id' : '.class_value') => maketext($fieldLabels->{$field}), @@ -44,7 +42,7 @@ % unless ($isVersioned) { <%= text_field "set.$setID.$field.class_value" => - $c->formatDateTime($globalValue, 'datetime_format_short'), + $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, @@ -54,8 +52,7 @@ % if (defined $userRecord) {
- <%= text_field "set.$setID.$field" => - defined $userRecord ? $userRecord->$field : $globalValue, + <%= text_field "set.$setID.$field" => $userRecord->$field, id => "set.$setID.${field}_id", class => 'form-control w-auto' . ($field eq 'open_date' ? ' datepicker-group' : ''), placeholder => $isGateway diff --git a/templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep b/templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep index 33d26d9397..3e090476b7 100644 --- a/templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep +++ b/templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep @@ -1,6 +1,5 @@ % 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'; diff --git a/templates/HelpFiles/InstructorUserDetail.html.ep b/templates/HelpFiles/InstructorUserDetail.html.ep index d7e2dc5ab3..a3fd81b91c 100644 --- a/templates/HelpFiles/InstructorUserDetail.html.ep +++ b/templates/HelpFiles/InstructorUserDetail.html.ep @@ -23,11 +23,12 @@ . 'is deleted and cannot be recovered. Use this action cautiously!') =%>

- <%= 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.') =%>