Merge pull request openwebwork#2386 from drgrice1/sets-manager-set-de…

…f-list-improvements

Rework how set definition files from OPL and Contrib are shown.

conf/defaults.config

@@ -1042,20 +1042,14 @@ $pg{additionalPGEditorDisplayModes} = [
 # Whether the homework editor pages should show options for conditional release
 $options{enableConditionalRelease} = 0;
 
-# In the set manager, how deep to search within templates for .def files.
-# Note that this does not apply to the Library and Contrib directories.
-# Those directories are not searched in any case (see below).
-# 0 means only within templates.
+##########################################################################################
+# Searching for set.def files to import
+##########################################################################################
+# In the set manager and the library browser, the directory depth to search
+# within templates for .def files.  Note that this does not apply to the Library
+# and Contrib directories.  Those directories are not searched in any case.
 $options{setDefSearchDepth} = 4;
 
-# In the set manager, also list OPL or Contrib set defintion files.   Note
-# that the directories are not searched, but these lists are loaded from the
-# files htdocs/DATA/library-set-defs.json and htdocs/DATA/contrib-set-defs.json
-# which are generated by running bin/generate-OPL-set-def-lists.pl (which is
-# also run if you run bin/OPL-update).
-$options{useOPLdefFiles} = 1;
-$options{useContribDefFiles} = 1;
-
 ##########################################################################################
 #### Default settings for the problem editor pages
 ##########################################################################################

conf/localOverrides.conf.dist

@@ -646,12 +646,11 @@ $mail{feedbackRecipients}    = [
 ################################################################################
 # Searching for set.def files to import
 ################################################################################
-## Uncomment below so that when the Library Browser searches for set def
-## files, it searches beyond templates; it can search deeper subfolders of
-## templates, and optionally also descend into Library
+# In the set manager and the library browser, the directory depth to search
+# within templates for .def files.  Note that this does not apply to the Library
+# and Contrib directories.  Those directories are not searched in any case.
 
-#$options{setDefSearchDepth}=4; #search down 4 levels
-#$options{useOPLdefFiles}=1;
+#$options{setDefSearchDepth} = 4;
 
 ################################################################################
 # Permission overrides (e.g. "admin", "professor", "ta", "student", "guest"

htdocs/js/ProblemSetList/problemsetlist.js

@@ -235,7 +235,7 @@
 					}
 				})
 			],
-			onReady(selectedDates) {
+			onReady() {
 				// Flatpickr hides the original input and adds the alternate input after it.  That messes up the
 				// bootstrap input group styling.  So move the now hidden original input after the created alternate
 				// input to fix that.
@@ -277,4 +277,42 @@
 			}
 		});
 	}
+
+	const importSourceSelect = document.getElementById('import_source_select');
+	const listOPLSetsCheck = document.getElementById('list_opl_sets');
+	const listContribSetsCheck = document.getElementById('list_contrib_sets');
+
+	if (importSourceSelect && (listOPLSetsCheck || listContribSetsCheck)) {
+		const allOptions = Array.from(importSourceSelect.options);
+
+		const updateAvailableOptions = () => {
+			// Save the currently selected options.
+			const selectedDefs = Array.from(importSourceSelect.selectedOptions);
+
+			for (const option of Array.from(importSourceSelect.options)) option.value && option.remove();
+			for (const option of allOptions) {
+				if (option.value.startsWith('Library/')) {
+					if (listOPLSetsCheck.checked) importSourceSelect.add(option);
+				} else if (option.value.startsWith('Contrib/')) {
+					if (listContribSetsCheck.checked) importSourceSelect.add(option);
+				} else importSourceSelect.add(option);
+			}
+
+			// Reselect the options that were selected before if still available. Otherwise
+			// select the first option which should be the "Select filenames below" option.
+			let foundSelected = false;
+			for (const option of importSourceSelect.options) {
+				if (selectedDefs.includes(option)) {
+					foundSelected = true;
+					option.selected = true;
+				}
+			}
+			if (!foundSelected) allOptions[0].selected = true;
+		};
+
+		listOPLSetsCheck?.addEventListener('change', updateAvailableOptions);
+		listContribSetsCheck?.addEventListener('change', updateAvailableOptions);
+
+		updateAvailableOptions();
+	}
 })();

htdocs/js/SetMaker/setmaker.js

@@ -509,4 +509,39 @@
 	document
 		.querySelectorAll('.lb-problem-add [data-bs-toggle], .lb-problem-icons [data-bs-toggle=tooltip]')
 		.forEach((el) => new bootstrap.Tooltip(el, { fallbackPlacements: [] }));
+
+	// If this is the browse setdef panel, then set up the check boxes for including OPL or Contrib sets.
+	const setDefSelect = document.getElementsByName('library_sets')[0];
+	const listOPLSetsCheck = document.getElementById('list_opl_sets');
+	const listContribSetsCheck = document.getElementById('list_contrib_sets');
+
+	if (setDefSelect && (listOPLSetsCheck || listContribSetsCheck)) {
+		const allOptions = Array.from(setDefSelect.options);
+
+		const updateAvailableOptions = () => {
+			// Save the currently selected option.
+			let selectedDef = setDefSelect.selectedOptions[0];
+
+			for (const option of Array.from(setDefSelect.options)) option.value && option.remove();
+			for (const option of allOptions) {
+				if (option.value.startsWith('Library/')) {
+					if (listOPLSetsCheck.checked) setDefSelect.add(option);
+					else if (selectedDef === option) selectedDef = null;
+				} else if (option.value.startsWith('Contrib/')) {
+					if (listContribSetsCheck.checked) setDefSelect.add(option);
+					else if (selectedDef === option) selectedDef = null;
+				} else setDefSelect.add(option);
+			}
+
+			// Reselect the option that was selected before if it is still available. Otherwise
+			// select the first option which should be the "Select a Set Defintion File" option.
+			if (selectedDef) selectedDef.selected = true;
+			else allOptions[0].selected = true;
+		};
+
+		listOPLSetsCheck?.addEventListener('change', updateAvailableOptions);
+		listContribSetsCheck?.addEventListener('change', updateAvailableOptions);
+
+		updateAvailableOptions();
+	}
 })();

lib/WeBWorK/ContentGenerator/Instructor/SetMaker.pm

@@ -471,7 +471,7 @@ sub pre_header_initialize ($c) {
 	$count++ if ($j > 0);
 
 	# Default of which problem selector to display
-	my $browse_which = $c->param('browse_which') || 'browse_npl_library';
+	my $browse_which = $c->param('browse_which') || 'browse_opl';
 
 	# Check for problem lib buttons
 	my $browse_lib = '';
@@ -484,7 +484,7 @@ sub pre_header_initialize ($c) {
 
 	# Start the logic through if elsif elsif ...
 	debug("browse_lib",         $c->param("$browse_lib"));
-	debug("browse_npl_library", $c->param("browse_npl_library"));
+	debug("browse_opl",         $c->param("browse_opl"));
 	debug("browse_course_sets", $c->param("browse_course_sets"));
 	debug("browse_setdefs",     $c->param("browse_setdefs"));
 	# Asked to browse certain problems
@@ -493,8 +493,8 @@ sub pre_header_initialize ($c) {
 		$c->{current_library_set} = "";
 		$use_previous_problems    = 0;
 		@pg_files                 = ();
-	} elsif ($c->param('browse_npl_library')) {
-		$browse_which             = 'browse_npl_library';
+	} elsif ($c->param('browse_opl')) {
+		$browse_which             = 'browse_opl';
 		$c->{current_library_set} = "";
 		$use_previous_problems    = 0;
 		@pg_files                 = ();

lib/WeBWorK/Utils/Instructor.pm

@@ -671,33 +671,45 @@ sub getDefList {
 
 	my @found_set_defs;
 
-	# get_set_defs_wanted is a closure over @found_set_defs
-	my $get_set_defs_wanted = sub {
-		if ($File::Find::dir =~ /^$topdir\/Library/ || $File::Find::dir =~ /^$topdir\/Contrib/) {
-			$File::Find::prune = 1;
-			return;
-		}
-		if (@{ [ $File::Find::dir =~ /\//g ] } > $max_depth) { $File::Find::prune = 1; return; }
-		push @found_set_defs, $_ =~ s|^$topdir/?||r if m|/set[^/]*\.def$|;
-	};
-
-	find({ wanted => $get_set_defs_wanted, follow_fast => 1, no_chdir => 1, follow_skip => 2 }, $topdir);
+	find(
+		{
+			wanted => sub {
+				if ($File::Find::dir =~ /^$topdir\/Library/
+					|| $File::Find::dir =~ /^$topdir\/Contrib/
+					|| $File::Find::dir =~ /^$topdir\/capaLibrary/)
+				{
+					$File::Find::prune = 1;
+					return;
+				}
+				if (@{ [ $File::Find::dir =~ /\//g ] } > $max_depth) { $File::Find::prune = 1; return; }
+				push @found_set_defs, $_ =~ s|^$topdir/?||r if m|/set[^/]*\.def$|;
+			},
+			follow_fast => 1,
+			no_chdir    => 1,
+			follow_skip => 2
+		},
+		$topdir
+	);
 
 	# Load the OPL set definition files from the list file.
 	push(@found_set_defs, loadSetDefListFile("$ce->{webworkDirs}{htdocs}/DATA/library-set-defs.json"))
-		if ($ce->{options}{useOPLdefFiles});
+		if -d "$ce->{courseDirs}{templates}/Library" && -r "$ce->{courseDirs}{templates}/Library";
 
 	# Load the Contrib set definition files from the list file.
 	push(@found_set_defs, loadSetDefListFile("$ce->{webworkDirs}{htdocs}/DATA/contrib-set-defs.json"))
-		if ($ce->{options}{useContribDefFiles});
+		if -d "$ce->{courseDirs}{templates}/Contrib" && -r "$ce->{courseDirs}{templates}/Contrib";
 
+	my @lib_order;
 	my @depths;
 	my @caps;
 	for (@found_set_defs) {
+		push(@lib_order, $_ =~ m|^Library/| ? 2 : $_ =~ m|^Contrib/| ? 3 : 1);
 		push @depths, scalar(@{ [ $_ =~ /\//g ] });
 		push @caps,   uc($_);
 	}
-	return @found_set_defs[ sort { $depths[$a] <=> $depths[$b] || $caps[$a] cmp $caps[$b] } 0 .. $#found_set_defs ];
+	return @found_set_defs[
+		sort { $lib_order[$a] <=> $lib_order[$b] || $depths[$a] <=> $depths[$b] || $caps[$a] cmp $caps[$b] }
+		0 .. $#found_set_defs ];
 }
 
 =back

templates/ContentGenerator/Instructor/ProblemSetList/import_form.html.ep

@@ -15,7 +15,7 @@
 			class => 'col-form-label col-form-label-sm col-md-auto' =%>
 		<div class="col-auto">
 			<%= select_field 'action.import.source' => [
-					[ maketext('Enter filenames below') => '', selected => undef ],
+					[ maketext('Select filenames below') => '', selected => undef, disabled => undef ],
 					@$setDefList
 				],
 				id => 'import_source_select', class => 'form-select form-select-sm', dir => 'ltr',
@@ -25,6 +25,25 @@
 				: () =%>
 		</div>
 	</div>
+	%
+	% my $listLibrarySets = -d "$ce->{courseDirs}{templates}/Library" && -r "$ce->{courseDirs}{templates}/Library";
+	% my $listContribSets = -d "$ce->{courseDirs}{templates}/Contrib" && -r "$ce->{courseDirs}{templates}/Contrib";
+	% if ($listLibrarySets || $listContribSets) {
+		<div class="mb-2">
+			% if ($listLibrarySets) {
+				<div class="form-check form-check-inline">
+					<%= check_box list_opl_sets => 0, id => 'list_opl_sets', class => 'form-check-input' =%>
+					<%= label_for list_opl_sets => maketext('List OPL Sets'), class => "form-check-label" =%>
+				</div>
+			% }
+			% if ($listContribSets) {
+				<div class="form-check form-check-inline">
+					<%= check_box list_contrib_sets => 0, id => 'list_contrib_sets', class => 'form-check-input' =%>
+					<%= label_for list_contrib_sets => maketext('List Contrib Sets'), class => "form-check-label" =%>
+				</div>
+			% }
+		</div>
+	% }
 	<div class="row mb-2">
 		<%= label_for import_text => maketext('Import sets with names') . ':',
 			class => 'col-form-label col-form-label-sm col-md-auto' =%>

templates/ContentGenerator/Instructor/SetMaker/browse_library_panel_advanced.html.ep

@@ -104,7 +104,8 @@
 			<%= submit_button maketext('Reset'), name => 'library_reset',
 				class => 'btn btn-secondary btn-sm mb-1 ms-sm-0 ms-2 library-panel-btn' =%>
 			<%= submit_button maketext('Basic Search'), name => 'library_basic',
-				class => 'btn btn-secondary btn-sm mb-1 ms-sm-0 ms-2 library-panel-btn' =%>
+				class => 'btn btn-secondary btn-sm mb-2 ms-sm-0 ms-2 library-panel-btn' =%>
+			<%= include 'ContentGenerator/Instructor/SetMaker/library-include-checks' =%>
 		</div>
 	</div>
 	<%= include 'ContentGenerator/Instructor/SetMaker/view_problems_line', internal_name => 'lib_view' =%>

templates/ContentGenerator/Instructor/SetMaker/browse_library_panel_simple.html.ep

@@ -40,7 +40,8 @@
 		</div>
 		<div class="col-md-3 col-sm-4 mb-1 d-flex flex-sm-column justify-content-sm-start justify-content-center">
 			<%= submit_button maketext('Advanced Search'), name => 'library_advanced',
-				class => 'btn btn-secondary btn-sm library-panel-btn' =%>
+				class => 'btn btn-secondary btn-sm library-panel-btn mb-2' =%>
+			<%= include 'ContentGenerator/Instructor/SetMaker/library-include-checks' =%>
 		</div>
 	</div>
 	<%= include 'ContentGenerator/Instructor/SetMaker/view_problems_line', internal_name => 'lib_view' =%>

templates/ContentGenerator/Instructor/SetMaker/browse_setdef_panel.html.ep

@@ -15,10 +15,34 @@
 	<div class="mb-2">
 		<label class="col-form-label-sm"><%= maketext('Browse from:') %></label>
 		<%= select_field library_sets => [
-				$selected_library eq '' ? [ maketext('Select a Set Definition File') => '', selected => undef, disabled => 'disabled' ] : (),
+				[
+					maketext('Select a Set Definition File') => '',
+					$selected_library ? () : (selected => undef),
+					disabled => undef
+				],
 				@list_of_set_defs
 			],
 			class => 'form-select form-select-sm d-inline w-auto' =%>
 	</div>
+	%
+	% my $listLibrarySets = -d "$ce->{courseDirs}{templates}/Library" && -r "$ce->{courseDirs}{templates}/Library";
+	% my $listContribSets = -d "$ce->{courseDirs}{templates}/Contrib" && -r "$ce->{courseDirs}{templates}/Contrib";
+	% if ($listLibrarySets || $listContribSets) {
+		<div class="mb-2 font-sm">
+			% if ($listLibrarySets) {
+				<div class="form-check form-check-inline">
+					<%= check_box list_opl_sets => 0, id => 'list_opl_sets', class => 'form-check-input' =%>
+					<%= label_for list_opl_sets => maketext('List OPL Sets'), class => "form-check-label" =%>
+				</div>
+			% }
+			% if ($listContribSets) {
+				<div class="form-check form-check-inline">
+					<%= check_box list_contrib_sets => 0, id => 'list_contrib_sets', class => 'form-check-input' =%>
+					<%= label_for list_contrib_sets => maketext('List Contrib Sets'), class => "form-check-label" =%>
+				</div>
+			% }
+		</div>
+	% }
+	%
 	<%= include 'ContentGenerator/Instructor/SetMaker/view_problems_line', internal_name => 'view_setdef_set' =%>
 </div>

templates/ContentGenerator/Instructor/SetMaker/library-include-checks.html.ep

@@ -0,0 +1,14 @@
+% if (-d "$ce->{courseDirs}{templates}/Contrib" && -r "$ce->{courseDirs}{templates}/Contrib") {
+	<div class="form-check mb-2 font-sm">
+		% param('includeOPL', 'on') unless defined param('includeOPL');
+		<%= check_box includeOPL => 'on', id => 'includeOPL', class => 'form-check-input' =%>
+		<%= label_for includeOPL => maketext('Include OPL'), class => 'form-check-label' =%>
+		<%= hidden_field includeOPL => 0 =%>
+	</div>
+	<div class="form-check font-sm">
+		<%= check_box includeContrib => 'on', id => 'includeContrib', class => 'form-check-input' =%>
+		<%= label_for includeContrib => maketext('Include Contrib'), class => 'form-check-label' =%>
+	</div>
+% } else {
+	<%= hidden_field includeOPL => 1 =%>
+% }

templates/ContentGenerator/Instructor/SetMaker/top_row.html.ep

@@ -39,9 +39,9 @@
 <div class="d-flex justify-content-center">
 	<label class="col-form-label-sm mb-2 me-1"><%= maketext('Browse') %></label>
 	<div class="d-flex flex-wrap justify-content-center">
-		<%= submit_button maketext('Open Problem Library'), name => 'browse_npl_library',
+		<%= submit_button maketext('Open Problem Library'), name => 'browse_opl',
 			class => 'browse-lib-btn btn btn-secondary btn-sm mb-2 mx-1',
-			$browse_which eq 'browse_npl_library' ? (disabled => undef) : () =%>
+			$browse_which eq 'browse_opl' ? (disabled => undef) : () =%>
 		<%= submit_button maketext('Course Files'), name => 'browse_local',
 			class => 'browse-lib-btn btn btn-secondary btn-sm mb-2 mx-1',
 			$browse_which eq 'browse_local' ? (disabled => undef) : () =%>
@@ -73,7 +73,7 @@
 % } elsif ($browse_which eq 'browse_course_sets') {
 	<%= include 'ContentGenerator/Instructor/SetMaker/browse_course_sets_panel',
 		selected_library => $c->{current_library_set} =%>
-% } elsif ($browse_which eq 'browse_npl_library') {
+% } elsif ($browse_which eq 'browse_opl') {
 	<%= include 'ContentGenerator/Instructor/SetMaker/browse_library_panel' =%>
 % } elsif ($browse_which eq 'browse_setdefs') {
 	<%= include 'ContentGenerator/Instructor/SetMaker/browse_setdef_panel',

templates/ContentGenerator/Instructor/SetMaker/view_problems_line.html.ep

@@ -1,12 +1,11 @@
 % use WeBWorK::PG;
 %
-% my $contrib_exists = -r $ce->{courseDirs}{templates} . '/Contrib';
-%
 <div class="d-flex flex-wrap justify-content-center">
 	<%= submit_button maketext('View Problems'), name => $internal_name, class => 'btn btn-secondary btn-sm mb-2' =%>
 	%
 	<div class="d-inline-block ms-2 mb-2">
-		<%= label_for 'problem_displaymode' => maketext('Display Mode:'), class => 'col-form-label col-form-label-sm' =%>
+		<%= label_for 'problem_displaymode' => maketext('Display Mode:'),
+			class => 'col-form-label col-form-label-sm' =%>
 		<%= select_field 'problem_displaymode' => [
 				(
 					map { [ $_ => $_, $_ eq $ce->{pg}{options}{displayMode} ? (selected => undef) : () ] }
@@ -25,26 +24,8 @@
 		<%= select_field max_shown => [ 5, 10, 15, [ 20 => 20, selected => undef ], 25, 30, 50, 'All' ],
 			id => 'max_shown', class => 'form-select form-select-sm d-inline w-auto' =%>
 	</div>
-	% # Option of whether to show hints and solutions
+	% # Options for whether to show hints and solutions.
 	<div class="d-inline-block ms-2 mb-2">
-		% if ($contrib_exists) {
-			<div class="form-check form-check-inline ms-2">
-				<label class="form-check-label col-form-label-sm">
-					% param('includeOPL', 'on') unless defined param('includeOPL');
-					<%= check_box includeOPL => 'on', class => 'form-check-input me-1' =%>
-					<%= maketext('Include OPL') =%>
-					<%= hidden_field includeOPL => 0 =%>
-				</label>
-			</div>
-			<div class="form-check form-check-inline ms-2">
-				<label class="form-check-label col-form-label-sm">
-					<%= check_box includeContrib => 'on', class => 'form-check-input me-1' =%>
-					<%= maketext('Include Contrib') =%>
-				</label>
-			</div>
-		% } else {
-			<%= hidden_field includeOPL => 1 =%>
-		% }
 		<div class="form-check form-check-inline ms-2">
 			<label class="form-check-label col-form-label-sm">
 				<%= check_box showHints => 'on', class => 'form-check-input me-1' =%>