Merge pull request #2345 from drgrice1/bugfix/date-overrides-versione…

…d-user-set

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

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';
 

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', () => {

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,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);
@@ -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);
@@ -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 };
 }

templates/ContentGenerator/Instructor/UserDetail.html.ep

@@ -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;

templates/ContentGenerator/Instructor/UserDetail/set_date_table.html.ep

@@ -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.
@@ -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>

templates/ContentGenerator/Instructor/UserDetail/set_row.html.ep

@@ -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',

templates/HelpFiles/InstructorUserDetail.html.ep

@@ -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. '