Fix problem ids displayed for achievement items.

Instead of just storing a max problem id for each achievement set option, store the json encoded list of problem ids. Initially only the options for the first set are provided. The javascript then rebuilds the options list when a new set is selected that only includes the problem ids for that set. This also restructures the way that achievement item data is obtained from the database to be a bit more efficient. When this data is obtained gateway tests are no longer filtered out. The current code for each achievement item that does not act on gateway tests is already set up to filter these out. The achievement items that do act on gateway tests no longer need to load data from the database on their own. They can use the given list and filter out non-gateway sets. There seems to have been a minor bug in the `ResurrectGW` achievement in that it showed all gateway sets to be possibly "resurrected" when it should only show gateway sets that are past due. That has also been fixed, so now it only shows past due gateway sets for "resurrection". This fixes issue openwebwork#2281.
drgrice1 · Jan 24, 2024 · 1b6e912 · 1b6e912
1 parent b2b054f
commit 1b6e912
Show file tree

Hide file tree

Showing 19 changed files with 118 additions and 121 deletions.
diff --git a/htdocs/js/AchievementItems/achievementitems.js b/htdocs/js/AchievementItems/achievementitems.js
@@ -1,17 +1,33 @@
 (() => {
 	for (const setSelect of document.querySelectorAll('select[data-problems]')) {
 		setSelect.addEventListener('change', () => {
-			const max = parseInt(Array.from(setSelect.querySelectorAll('option'))
-				.find((option) => option.value === setSelect.value)?.dataset.max ?? '0');
+			const problemIds = JSON.parse(
+				Array.from(setSelect.querySelectorAll('option')).find((option) => option.value === setSelect.value)
+					?.dataset.problemIds ?? '[]'
+			);
 
-			document.querySelectorAll(`#${setSelect.dataset.problems} option`).forEach((option, index) => {
-				option.style.display = index < max ? '' : 'none';
-			});
+			const problemSelect = document.getElementById(setSelect.dataset.problems);
+			if (problemSelect) {
+				for (const option of problemSelect.querySelectorAll('option')) option.remove();
+				for (const id of problemIds) {
+					const option = document.createElement('option');
+					option.value = id;
+					option.text = id;
+					problemSelect.add(option);
+				}
+			}
 
 			// This is only used by the "Box of Transmogrification".
-			document.querySelectorAll(`#${setSelect.dataset.problems2} option`).forEach((option, index) => {
-				option.style.display = index < max ? '' : 'none';
-			});
+			const problemSelect2 = document.getElementById(setSelect.dataset.problems2);
+			if (problemSelect2) {
+				for (const option of problemSelect2.querySelectorAll('option')) option.remove();
+				for (const id of problemIds) {
+					const option = document.createElement('option');
+					option.value = id;
+					option.text = id;
+					problemSelect2.add(option);
+				}
+			}
 		});
 	}
 })();
diff --git a/lib/WeBWorK/AchievementItems/AddNewTestGW.pm b/lib/WeBWorK/AchievementItems/AddNewTestGW.pm
@@ -31,15 +31,13 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	my $db = $c->db;
 
-	my $effectiveUserName = $c->param('effectiveUser') // $c->param('user');
-	my @unfilteredsets = $db->getMergedSets(map { [ $effectiveUserName, $_ ] } $db->listUserSets($effectiveUserName));
 	my @openGateways;
 
 	# Find the template sets of open gateway quizzes.
-	for my $set (@unfilteredsets) {
+	for my $set (@$sets) {
 		push(@openGateways, [ format_set_name_display($set->set_id) => $set->set_id ])
 			if $set->assignment_type =~ /gateway/
 			&& $set->set_id !~ /,v\d+$/

diff --git a/lib/WeBWorK/AchievementItems/DoubleProb.pm b/lib/WeBWorK/AchievementItems/DoubleProb.pm
@@ -18,6 +18,8 @@ use Mojo::Base 'WeBWorK::AchievementItems', -signatures;
 
 # Item to make a problem worth double.
 
+use Mojo::JSON qw(encode_json);
+
 use WeBWorK::Utils qw(between x nfreeze_base64 thaw_base64 format_set_name_display);
 
 sub new ($class) {
@@ -28,32 +30,28 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	# Construct a dropdown with open sets and another with problems.
-	# Javascript ensures the appropriate number of problems are shown for the selected set.
+	# Javascript ensures the appropriate problems are shown for the selected set.
 
-	my @openSets;
-	my $maxProblems = 0;
+	my (@openSets, @initialProblemIDs);
 
 	for my $i (0 .. $#$sets) {
-		if (between($sets->[$i]->open_date, $sets->[$i]->due_date) && $sets->[$i]->assignment_type eq 'default') {
+		if (between($sets->[$i]->open_date, $sets->[$i]->due_date)
+			&& $sets->[$i]->assignment_type eq 'default'
+			&& @{ $setProblemIds->{ $sets->[$i]->set_id } })
+		{
 			push(
 				@openSets,
 				[
 					format_set_name_display($sets->[$i]->set_id) => $sets->[$i]->set_id,
-					data                                         => { max => $setProblemCount->[$i] }
+					data => { problem_ids => encode_json($setProblemIds->{ $sets->[$i]->set_id }) }
 				]
 			);
-			$maxProblems = $setProblemCount->[$i] if $setProblemCount->[$i] > $maxProblems;
+			@initialProblemIDs = @{ $setProblemIds->{ $sets->[$i]->set_id } } unless @initialProblemIDs;
 		}
 	}
 
-	my @problemIDs;
-
-	for my $i (1 .. $maxProblems) {
-		push(@problemIDs, [ $i => $i, $i > $openSets[0][3]{max} ? (style => 'display:none') : () ]);
-	}
-
 	return $c->c(
 		$c->tag(
 			'p',
@@ -71,7 +69,7 @@ sub print_form ($self, $sets, $setProblemCount, $c) {
 			$c,
 			id                  => 'dbp_problem_id',
 			label_text          => $c->maketext('Problem Number'),
-			values              => \@problemIDs,
+			values              => \@initialProblemIDs,
 			menu_container_attr => { class => 'col-3' }
 		)
 	)->join('');

diff --git a/lib/WeBWorK/AchievementItems/DoubleSet.pm b/lib/WeBWorK/AchievementItems/DoubleSet.pm
@@ -28,7 +28,7 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	my @openSets;
 
 	for my $i (0 .. $#$sets) {

diff --git a/lib/WeBWorK/AchievementItems/DuplicateProb.pm b/lib/WeBWorK/AchievementItems/DuplicateProb.pm
@@ -18,6 +18,8 @@ use Mojo::Base 'WeBWorK::AchievementItems', -signatures;
 
 # Item to turn one problem into another problem
 
+use Mojo::JSON qw(encode_json);
+
 use WeBWorK::Utils qw(between x nfreeze_base64 thaw_base64 format_set_name_display);
 
 sub new ($class) {
@@ -28,31 +30,28 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	# Show open sets and allow for a choice of two problems from the set.
+	# Javascript ensures the appropriate problems are shown for the selected set.
 
-	my @openSets;
-	my $maxProblems = 0;
+	my (@openSets, @initialProblemIDs);
 
 	for my $i (0 .. $#$sets) {
-		if (between($sets->[$i]->open_date, $sets->[$i]->due_date) && $sets->[$i]->assignment_type eq 'default') {
+		if (between($sets->[$i]->open_date, $sets->[$i]->due_date)
+			&& $sets->[$i]->assignment_type eq 'default'
+			&& @{ $setProblemIds->{ $sets->[$i]->set_id } })
+		{
 			push(
 				@openSets,
 				[
 					format_set_name_display($sets->[$i]->set_id) => $sets->[$i]->set_id,
-					data                                         => { max => $setProblemCount->[$i] }
+					data => { problem_ids => encode_json($setProblemIds->{ $sets->[$i]->set_id }) }
 				]
 			);
-			$maxProblems = $setProblemCount->[$i] if $setProblemCount->[$i] > $maxProblems;
+			@initialProblemIDs = @{ $setProblemIds->{ $sets->[$i]->set_id } } unless @initialProblemIDs;
 		}
 	}
 
-	my @problemIDs;
-
-	for my $i (1 .. $maxProblems) {
-		push(@problemIDs, [ $i => $i, $i > $openSets[0][3]{max} ? (style => 'display:none') : () ]);
-	}
-
 	return $c->c(
 		$c->tag(
 			'p',
@@ -78,15 +77,15 @@ sub print_form ($self, $sets, $setProblemCount, $c) {
 				WeBWorK::AchievementItems::form_popup_menu_row(
 					$c,
 					id                  => 'tran_problem_id',
-					values              => \@problemIDs,
+					values              => \@initialProblemIDs,
 					label_text          => $c->maketext('Copy this Problem'),
 					menu_container_attr => { class => 'col-2 ps-0' },
 					add_container       => 0
 				),
 				WeBWorK::AchievementItems::form_popup_menu_row(
 					$c,
 					id                  => 'tran_problem_id2',
-					values              => \@problemIDs,
+					values              => \@initialProblemIDs,
 					label_text          => $c->maketext('To this Problem'),
 					menu_container_attr => { class => 'col-2 ps-0' },
 					add_container       => 0

diff --git a/lib/WeBWorK/AchievementItems/ExtendDueDate.pm b/lib/WeBWorK/AchievementItems/ExtendDueDate.pm
@@ -28,7 +28,7 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	my @openSets;
 
 	for my $i (0 .. $#$sets) {

diff --git a/lib/WeBWorK/AchievementItems/ExtendDueDateGW.pm b/lib/WeBWorK/AchievementItems/ExtendDueDateGW.pm
@@ -29,15 +29,13 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	my $db = $c->db;
 
-	my $effectiveUserName = $c->param('effectiveUser') // $c->param('user');
-	my @unfilteredsets = $db->getMergedSets(map { [ $effectiveUserName, $_ ] } $db->listUserSets($effectiveUserName));
 	my @openGateways;
 
 	# Find the template sets for open tests.
-	for my $set (@unfilteredsets) {
+	for my $set (@$sets) {
 		push(@openGateways, [ format_set_name_display($set->set_id) => $set->set_id ])
 			if $set->assignment_type =~ /gateway/
 			&& $set->set_id !~ /,v\d+$/

diff --git a/lib/WeBWorK/AchievementItems/FullCreditProb.pm b/lib/WeBWorK/AchievementItems/FullCreditProb.pm
@@ -18,6 +18,8 @@ use Mojo::Base 'WeBWorK::AchievementItems', -signatures;
 
 # Item to give full credit on a single problem
 
+use Mojo::JSON qw(encode_json);
+
 use WeBWorK::Utils qw(between x nfreeze_base64 thaw_base64 format_set_name_display);
 
 sub new ($class) {
@@ -28,32 +30,28 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	# Construct a dropdown with open sets and another with problems.
-	# Javascript ensures the appropriate number of problems are shown for the selected set.
+	# Javascript ensures the appropriate problems are shown for the selected set.
 
-	my @openSets;
-	my $maxProblems = 0;
+	my (@openSets, @initialProblemIDs);
 
 	for my $i (0 .. $#$sets) {
-		if (between($sets->[$i]->open_date, $sets->[$i]->due_date) && $sets->[$i]->assignment_type eq 'default') {
+		if (between($sets->[$i]->open_date, $sets->[$i]->due_date)
+			&& $sets->[$i]->assignment_type eq 'default'
+			&& @{ $setProblemIds->{ $sets->[$i]->set_id } })
+		{
 			push(
 				@openSets,
 				[
 					format_set_name_display($sets->[$i]->set_id) => $sets->[$i]->set_id,
-					data                                         => { max => $setProblemCount->[$i] }
+					data => { problem_ids => encode_json($setProblemIds->{ $sets->[$i]->set_id }) }
 				]
 			);
-			$maxProblems = $setProblemCount->[$i] if $setProblemCount->[$i] > $maxProblems;
+			@initialProblemIDs = @{ $setProblemIds->{ $sets->[$i]->set_id } } unless @initialProblemIDs;
 		}
 	}
 
-	my @problemIDs;
-
-	for my $i (1 .. $maxProblems) {
-		push(@problemIDs, [ $i => $i, $i > $openSets[0][3]{max} ? (style => 'display:none') : () ]);
-	}
-
 	return $c->c(
 		$c->tag(
 			'p',
@@ -71,7 +69,7 @@ sub print_form ($self, $sets, $setProblemCount, $c) {
 			$c,
 			id                  => 'fcp_problem_id',
 			label_text          => $c->maketext('Problem Number'),
-			values              => \@problemIDs,
+			values              => \@initialProblemIDs,
 			menu_container_attr => { class => 'col-3' }
 		)
 	)->join('');

diff --git a/lib/WeBWorK/AchievementItems/FullCreditSet.pm b/lib/WeBWorK/AchievementItems/FullCreditSet.pm
@@ -28,7 +28,7 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	my @openSets;
 
 	for my $i (0 .. $#$sets) {

diff --git a/lib/WeBWorK/AchievementItems/HalfCreditProb.pm b/lib/WeBWorK/AchievementItems/HalfCreditProb.pm
@@ -18,6 +18,8 @@ use Mojo::Base 'WeBWorK::AchievementItems', -signatures;
 
 # Item to give half credit on a single problem.
 
+use Mojo::JSON qw(encode_json);
+
 use WeBWorK::Utils qw(between x nfreeze_base64 thaw_base64 format_set_name_display);
 
 sub new ($class) {
@@ -28,32 +30,28 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	# Construct a dropdown with open sets and another with problems.
-	# Javascript ensures the appropriate number of problems are shown for the selected set.
+	# Javascript ensures the appropriate problems are shown for the selected set.
 
-	my @openSets;
-	my $maxProblems = 0;
+	my (@openSets, @initialProblemIDs);
 
 	for my $i (0 .. $#$sets) {
-		if (between($sets->[$i]->open_date, $sets->[$i]->due_date) && $sets->[$i]->assignment_type eq 'default') {
+		if (between($sets->[$i]->open_date, $sets->[$i]->due_date)
+			&& $sets->[$i]->assignment_type eq 'default'
+			&& @{ $setProblemIds->{ $sets->[$i]->set_id } })
+		{
 			push(
 				@openSets,
 				[
 					format_set_name_display($sets->[$i]->set_id) => $sets->[$i]->set_id,
-					data                                         => { max => $setProblemCount->[$i] }
+					data => { problem_ids => encode_json($setProblemIds->{ $sets->[$i]->set_id }) }
 				]
 			);
-			$maxProblems = $setProblemCount->[$i] if $setProblemCount->[$i] > $maxProblems;
+			@initialProblemIDs = @{ $setProblemIds->{ $sets->[$i]->set_id } } unless @initialProblemIDs;
 		}
 	}
 
-	my @problemIDs;
-
-	for my $i (1 .. $maxProblems) {
-		push(@problemIDs, [ $i => $i, $i > $openSets[0][3]{max} ? (style => 'display:none') : () ]);
-	}
-
 	return $c->c(
 		$c->tag(
 			'p',
@@ -70,7 +68,7 @@ sub print_form ($self, $sets, $setProblemCount, $c) {
 		WeBWorK::AchievementItems::form_popup_menu_row(
 			$c,
 			id                  => 'hcp_problem_id',
-			values              => \@problemIDs,
+			values              => \@initialProblemIDs,
 			label_text          => $c->maketext('Problem Number'),
 			menu_container_attr => { class => 'col-3' }
 		)

diff --git a/lib/WeBWorK/AchievementItems/HalfCreditSet.pm b/lib/WeBWorK/AchievementItems/HalfCreditSet.pm
@@ -28,7 +28,7 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	my @openSets;
 
 	for my $i (0 .. $#$sets) {

diff --git a/lib/WeBWorK/AchievementItems/ReducedCred.pm b/lib/WeBWorK/AchievementItems/ReducedCred.pm
@@ -32,7 +32,7 @@ sub new ($class) {
 	}, $class;
 }
 
-sub print_form ($self, $sets, $setProblemCount, $c) {
+sub print_form ($self, $sets, $setProblemIds, $c) {
 	my @openSets;
 
 	for my $i (0 .. $#$sets) {