PG Problem Editor file selector

This changes the PG Problem Editor to not assume a file type when opened
without a file type specified.  So when the problem editor is opened
from the site navigation link it does not directly open to the blank
file template.  Instead it opens to a file chooser page. On that page,
three options are presented.  The user may choose a "New Problem
Template", a "File" and give a path relative to the course templates
directory, or a "Sample Problem" and pick a sample problem file from a
dropdown.

In order to support sample problems a new file type needed to be
implemented for the problem editor.  The sample problem parsing module
for PG is used to strip out the documentation when the problem is loaded
into the editor.

However, I observed that the current code for that used by the sample
problem viewer to offer a sample problem file for download is highly
innefficient.  It currently reads all of the sample problem files, then
reads all of the PG macros, and then parses the desired problem
stripping out documentation, and finally serves the file. So a
corresponding PG pull request adds a `getSampleProblemCode` method that
simply reads only the desired sample problem and efficiently strips out
the documentation without needing the macros and sample problem
metadata. So instead of needing to read 253 files (the number of macros
plus the number of sample problems currently), it only reads 1.

Author: drgrice1

htdocs/js/PGProblemEditor/pgproblemeditor.js

@@ -1,4 +1,57 @@
 (() => {
+	const fileChooserForm = document.forms['pg-editor-file-chooser'];
+	if (fileChooserForm) {
+		const newProblemRadio = document.getElementById('new_problem');
+
+		const sourceFilePathInput = fileChooserForm.elements['sourceFilePath'];
+		const filePathRadio = document.getElementById('file_path');
+
+		const sampleProblemFileSelect = fileChooserForm.elements['sampleProblemFile'];
+		const sampleProblemRadio = document.getElementById('sample_problem');
+
+		newProblemRadio?.addEventListener('change', () => {
+			if (newProblemRadio.checked) {
+				sampleProblemFileSelect.required = false;
+				sourceFilePathInput.required = false;
+			}
+		});
+
+		if (filePathRadio && sourceFilePathInput) {
+			const filePathSelected = () => {
+				sampleProblemFileSelect.required = false;
+				sourceFilePathInput.required = true;
+				filePathRadio.checked = true;
+			};
+			filePathRadio.addEventListener('change', () => {
+				if (filePathRadio.checked) filePathSelected();
+			});
+			sourceFilePathInput.addEventListener('focusin', filePathSelected);
+		}
+		if (sampleProblemRadio && sampleProblemFileSelect) {
+			const sampleProblemSelected = () => {
+				sampleProblemFileSelect.required = true;
+				sourceFilePathInput.required = false;
+				sampleProblemRadio.checked = true;
+			};
+			sampleProblemRadio.addEventListener('change', () => {
+				if (sampleProblemRadio.checked) sampleProblemSelected();
+			});
+			sampleProblemFileSelect.addEventListener('change', sampleProblemSelected);
+			sampleProblemFileSelect.addEventListener('focusin', sampleProblemSelected);
+		}
+
+		fileChooserForm.addEventListener('submit', (e) => {
+			if (!fileChooserForm.checkValidity()) {
+				e.preventDefault();
+				e.stopPropagation();
+			}
+
+			fileChooserForm.classList.add('was-validated');
+		});
+
+		return;
+	}
+
 	// Add a container for message toasts.
 	const toastContainer = document.createElement('div');
 	toastContainer.classList.add('toast-container', 'position-fixed', 'bottom-0', 'end-0', 'p-3');

lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm

@@ -90,6 +90,11 @@ The "stationary" source for this problem is stored in the assets/pg directory
 and defined in defaults.config as
 $webworkFiles{screenSnippets}{blankProblem}
 
+=item sample_problem
+
+This is a special case which allows one to edit a sample PG problem.  These
+are problems located in the pg/tutorial/sample_problems directory.
+
 =back
 
 =head2 Action
@@ -123,6 +128,7 @@ use WeBWorK::Utils::Files      qw(surePathToFile readFile path_is_subdir);
 use WeBWorK::Utils::Instructor qw(assignProblemToAllSetUsers addProblemToSet);
 use WeBWorK::Utils::JITAR      qw(seq_to_jitar_id jitar_id_to_seq);
 use WeBWorK::Utils::Sets       qw(format_set_name_display);
+use SampleProblemParser        qw(getSampleProblemCode generateMetadata);
 
 use constant DEFAULT_SEED => 123456;
 
@@ -191,11 +197,11 @@ sub pre_header_initialize ($c) {
 			} else {
 				$c->{file_type} = 'problem';
 			}
-		} else {
-			$c->{file_type} = 'blank_problem';
 		}
 	}
 
+	return unless $c->{file_type};
+
 	# Clean up sourceFilePath and check that sourceFilePath is relative to the templates folder
 	if ($c->{file_type} eq 'source_path_for_problem_file') {
 		$c->{sourceFilePath} = $c->getRelativeSourceFilePath($c->param('sourceFilePath'));
@@ -242,6 +248,11 @@ sub initialize ($c) {
 	$c->stash->{actionFormTitles} = ACTION_FORM_TITLES();
 	$c->stash->{hardcopyLabels}   = [];
 
+	unless ($c->{file_type}) {
+		$c->stash->{sampleProblemMetadata} = generateMetadata("$ce->{pg_dir}/tutorial/sample-problems");
+		return;
+	}
+
 	# Tell the templates if we are working on a PG file
 	$c->{is_pg} = !$c->{file_type} || ($c->{file_type} ne 'course_info' && $c->{file_type} ne 'hardcopy_theme');
 
@@ -265,7 +276,7 @@ sub initialize ($c) {
 		));
 	}
 
-	if ($c->{file_type} eq 'blank_problem') {
+	if ($c->{file_type} eq 'blank_problem' || $c->{file_type} eq 'sample_problem') {
 		$c->addbadmessage($c->maketext('This file is a template. You may use "Save As" to create a new file.'));
 	} elsif ($c->{inputFilePath} =~ /$BLANKPROBLEM$/) {
 		$c->addbadmessage($c->maketext(
@@ -298,7 +309,8 @@ sub initialize ($c) {
 				eval { $problemContents = readFile($c->{editFilePath}) };
 				$problemContents = $@ if $@;
 				$c->{inputFilePath} = $c->{editFilePath};
-
+			} elsif (path_is_subdir($c->{editFilePath}, "$ce->{pg_dir}/tutorial/sample-problems")) {
+				$problemContents = getSampleProblemCode($c->{editFilePath});
 			} else {
 				$c->stash->{file_error} = $c->maketext('The given file path is not a valid location.');
 			}
@@ -387,12 +399,14 @@ sub page_title ($c) {
 
 #  Convert initial path component to [TMPL], [COURSE], or [WW].
 sub shortPath ($c, $file) {
-	my $tmpl = $c->ce->{courseDirs}{templates};
-	my $root = $c->ce->{courseDirs}{root};
-	my $ww   = $c->ce->{webworkDirs}{root};
+	my $tmpl   = $c->ce->{courseDirs}{templates};
+	my $root   = $c->ce->{courseDirs}{root};
+	my $ww     = $c->ce->{webworkDirs}{root};
+	my $sample = $c->ce->{pg_dir} . '/tutorial/sample-problems';
 	$file =~ s|^$tmpl|[TMPL]|;
 	$file =~ s|^$root|[COURSE]|;
 	$file =~ s|^$ww|[WW]|;
+	$file =~ s|^$sample|[SAMPLE]|;
 
 	return $file;
 }
@@ -416,6 +430,7 @@ sub determineTempEditFilePath ($c, $path) {
 	my $templatesDirectory   = $c->ce->{courseDirs}{templates};
 	my $tmpEditFileDirectory = $c->getTempEditFileDirectory();
 	my $hardcopyThemesDir    = $c->ce->{webworkDirs}{hardcopyThemes};
+	my $pgRoot               = $c->ce->{pg_dir};
 
 	$c->addbadmessage($c->maketext('The path to the original file should be absolute.'))
 		unless $path =~ m|^/|;
@@ -429,6 +444,9 @@ sub determineTempEditFilePath ($c, $path) {
 		} elsif ($path eq $c->ce->{webworkFiles}{screenSnippets}{blankProblem}) {
 			# Handle the case of the blank problem in snippets.
 			$path = "$tmpEditFileDirectory/blank.$setID.$user.tmp";
+		} elsif ($path =~ m|^$pgRoot/tutorial/sample-problems/(.*\.pg)$|) {
+			# Handle the case of a sample problem.
+			$path = "$tmpEditFileDirectory/$1.$user.tmp";
 		} elsif ($path eq $c->ce->{webworkFiles}{hardcopySnippets}{setHeader}) {
 			# Handle the case of the screen header in snippets.
 			$path = "$tmpEditFileDirectory/screenHeader.$setID.$user.tmp";
@@ -450,7 +468,6 @@ sub determineTempEditFilePath ($c, $path) {
 }
 
 # Determine the original path to a file corresponding to a temporary edit file.
-# Returns a path that is relative to the template directory.
 sub determineOriginalEditFilePath ($c, $path) {
 	my $ce = $c->ce;
 
@@ -511,6 +528,8 @@ sub getFilePaths ($c) {
 		$editFilePath = "$ce->{courseDirs}{templates}/$ce->{courseFiles}{course_info}";
 	} elsif ($c->{file_type} eq 'blank_problem') {
 		$editFilePath = $ce->{webworkFiles}{screenSnippets}{blankProblem};
+	} elsif ($c->{file_type} eq 'sample_problem') {
+		$editFilePath = "$ce->{pg_dir}/tutorial/sample-problems/" . $c->param('sampleProblemFile');
 	} elsif ($c->{file_type} eq 'hardcopy_theme') {
 		$editFilePath = "$ce->{courseDirs}{hardcopyThemes}/" . $c->param('hardcopy_theme');
 		if (!-e $editFilePath) {
@@ -787,8 +806,8 @@ sub view_handler ($c) {
 				sourceFilePath => $relativeTempFilePath
 			}
 		));
-	} elsif ($c->{file_type} eq 'blank_problem') {
-		# Redirect to Problem.pm.pm.
+	} elsif ($c->{file_type} eq 'blank_problem' || $c->{file_type} eq 'sample_problem') {
+		# Redirect to Problem.pm.
 		$c->authen->flash(status_message => $c->{status_message}->join(''));
 		$c->reply_with_redirect($c->systemLink(
 			$c->url_for('problem_detail', setID => 'Undefined_Set', problemID => 1),
@@ -843,10 +862,9 @@ sub view_handler ($c) {
 	return;
 }
 
-# The hardcopy and format_code actions are handled by javascript.  These are provided just in case
-# something goes wrong and the actions are called.
-sub hardcopy_action    { }
-sub format_code_action { }
+# The format_code action is handled by javascript.  This is provided just in case
+# something goes wrong and the handler is called.
+sub format_code_handler { }
 
 sub hardcopy_handler ($c) {
 	# Redirect to problem editor page.

lib/WeBWorK/ContentGenerator/SampleProblemViewer.pm

@@ -19,7 +19,8 @@ use Mojo::Base 'WeBWorK::ContentGenerator', -signatures;
 use File::Basename qw(basename);
 use Pod::Simple::Search;
 
-use SampleProblemParser qw(parseSampleProblem generateMetadata);
+use WeBWorK::Utils::Files qw(path_is_subdir);
+use SampleProblemParser   qw(parseSampleProblem generateMetadata getSampleProblemCode);
 
 =head1 NAME
 
@@ -44,9 +45,17 @@ Render the requestedSampleProblem or one of the indexes.
 sub renderSampleProblem ($c) {
 	my $pg_root = $c->ce->{pg_dir};
 
-	(undef, my $macro_files) = Pod::Simple::Search->new->inc(0)->survey("$pg_root/macros");
-	my %macro_locations = map { basename($_) => $_ =~ s!$pg_root/macros/!!r } keys %$macro_files;
-	my $metadata        = generateMetadata("$pg_root/tutorial/sample-problems", macro_locations => \%macro_locations);
+	if ($c->stash->{filePath} =~ /\.pg$/) {
+		my $sampleProblemFile = "$pg_root/tutorial/sample-problems/" . $c->stash->{filePath};
+		return $c->render(data => $c->maketext('File not found.'))
+			unless path_is_subdir($sampleProblemFile, $c->ce->{pg_dir} . '/tutorial/sample-problems')
+			&& -r $sampleProblemFile;
+
+		# Render the .pg file as a downloadable file.
+		return $c->render_file(data => getSampleProblemCode($sampleProblemFile));
+	}
+
+	my $metadata = generateMetadata("$pg_root/tutorial/sample-problems");
 
 	if (grep { $c->stash->{filePath} eq $_ } qw(categories techniques subjects macros)) {
 		my %labels = (
@@ -79,25 +88,14 @@ sub renderSampleProblem ($c) {
 			label    => $labels{ $c->stash->{filePath} },
 			list     => $list
 		);
-	} elsif ($c->stash->{filePath} =~ /\.pg$/) {
-		unless ($metadata->{ basename($c->stash->{filePath}) }) {
-			return $c->render(data => $c->maketext('File not found.'));
-		}
-
-		# Render the .pg file as a downloadable file.
-		return $c->render_file(
-			data => parseSampleProblem(
-				"$pg_root/tutorial/sample-problems/" . $c->stash->{filePath},
-				metadata    => $metadata,
-				pod_root    => $c->url_for('pod_index'),
-				pg_doc_home => $c->url_for('sample_problem_index')
-			)->{code}
-		);
 	} else {
 		unless ($metadata->{ basename($c->stash->{filePath}) . '.pg' }) {
 			$c->render(data => $c->maketext('Sample problem not found.'));
 		}
 
+		(undef, my $macro_files) = Pod::Simple::Search->new->inc(0)->survey("$pg_root/macros");
+		my %macro_locations = map { basename($_) => $_ =~ s!$pg_root/macros/!!r } keys %$macro_files;
+
 		# Render a problem with its documentation.
 		my $problemFile = "$pg_root/tutorial/sample-problems/" . $c->stash->{filePath} . '.pg';
 		return $c->render(

templates/ContentGenerator/Instructor/PGProblemEditor.html.ep

@@ -22,13 +22,19 @@
 	% last;
 % }
 %
+% if (!$c->{file_type}) {
+	%= include("ContentGenerator/Instructor/PGProblemEditor/file_chooser");
+	% last;
+% }
+%
 % if (stash('file_error')) {
 	<div class="alert alert-danger p-1 mb-0"><%= stash('file_error') %></div>
 	% last;
 % }
 %
 % my %titles = (
 	% blank_problem                => x('Editing <strong>new problem</strong> template "[_1]".'),
+	% sample_problem               => x('Editing <strong>sample problem</strong> file "[_1]".'),
 	% set_header                   => x('Editing <strong>set header</strong> file "[_1]".'),
 	% hardcopy_header              => x('Editing <strong>hardcopy header</strong> file "[_1]".'),
 	% hardcopy_theme               => x('Editing <strong>hardcopy theme</strong> file "[_1]".'),
@@ -85,6 +91,9 @@
 	% if (not_blank($c->{tempFilePath})) {
 		<%= hidden_field temp_file_path => $c->{tempFilePath} =%>
 	% }
+	% if ($c->{file_type} eq 'sample_problem' && param('sampleProblemFile')) {
+		<%= hidden_field sampleProblemFile => param('sampleProblemFile') =%>
+	% }
 	% if ($c->{file_type} eq 'hardcopy_theme') {
 		<%= hidden_field hardcopy_theme => param('hardcopy_theme') =%>
 	% }

templates/ContentGenerator/Instructor/PGProblemEditor/add_problem_form.html.ep

@@ -1,6 +1,6 @@
 % use WeBWorK::Utils::Sets qw(format_set_name_display);
 %
-% last unless $c->{is_pg};
+% last unless $c->{is_pg} && $c->{file_type} ne 'blank_problem' && $c->{file_type} ne 'sample_problem';
 %
 % my $allSetNames = [ map { $_->[0] =~ s/^set|\.def$//gr } $db->listGlobalSetsWhere({}, 'set_id') ];
 %

templates/ContentGenerator/Instructor/PGProblemEditor/file_chooser.html.ep

@@ -0,0 +1,54 @@
+<h2 class="my-3">Choose File to Edit</h2>
+%
+<%= form_for current_route, method => 'POST', name => 'pg-editor-file-chooser',
+	class => 'needs-validation', novalidate => undef, begin =%>
+	<div class="row align-items-center mb-3">
+		<div class="col-auto">
+			<div class="form-check">
+				<%= radio_button file_type => 'blank_problem',
+					id => 'new_problem', checked => undef, class => 'form-check-input' %>
+				<%= label_for new_problem => maketext('New problem template'), class => 'form-check-label' =%>
+			</div>
+		</div>
+	</div>
+	<div class="row align-items-center mb-3">
+		<div class="col-auto">
+			<div class="form-check">
+				<%= radio_button file_type => 'source_path_for_problem_file',
+					id => 'file_path', class => 'form-check-input' %>
+				<%= label_for file_path => maketext('File:'), class => 'form-check-label' =%>
+			</div>
+		</div>
+		<div class="col-auto" dir="ltr">
+			<div class="editor-save-path input-group input-group-sm">
+				<%= label_for source_file_path => '[TMPL]/', class => 'input-group-text' =%>
+				<%= text_field sourceFilePath => '',
+					id => 'source_file_path', class => 'form-control form-control-sm', size => 60, dir  => 'ltr' =%>
+				<div class="invalid-feedback">
+					<%= maketext('Plese enter a file with path relative to the course templates directory.') %>
+				</div>
+			</div>
+		</div>
+	</div>
+	<div class="row align-items-center mb-3">
+		<div class="col-auto">
+			<div class="form-check">
+				<%= radio_button file_type => 'sample_problem',
+					id => 'sample_problem', class => 'form-check-input' %>
+				<%= label_for sample_problem => maketext('Sample problem:'), class => 'form-check-label' =%>
+			</div>
+		</div>
+		<div class="col-auto">
+			<div class="col-auto">
+				<%= select_field sampleProblemFile => [
+						[maketext('Choose Sample Problem') => '', selected => undef, disabled => undef],
+						map { [ $sampleProblemMetadata->{$_}{name} => "$sampleProblemMetadata->{$_}{dir}/$_" ] }
+							sort { $sampleProblemMetadata->{$a}{name} cmp $sampleProblemMetadata->{$b}{name} }
+							keys %$sampleProblemMetadata
+					], id => 'sample_problem_file', class => 'form-select', 'aria-labelledby' => 'sample_problem' =%>
+				<div class="invalid-feedback"><%= maketext('Please select a problem.') %></div>
+			</div>
+		</div>
+	</div>
+	<div><%= submit_button maketext('Open'), class => 'btn btn-primary' =%></div>
+<%= end =%>

templates/ContentGenerator/Instructor/PGProblemEditor/save_as_form.html.ep

@@ -7,11 +7,13 @@
 % # Don't show the save as form when editing an existing course info file.
 % last if $c->{file_type} eq 'course_info' && -e $c->{editFilePath};
 %
-% my $isBlank         = $c->{file_type} eq 'blank_problem';
+% my $isBlank         = $c->{file_type} eq 'blank_problem' || $c->{file_type} eq 'sample_problem';
 % my $isHardcopyTheme = $c->{file_type} eq 'hardcopy_theme';
 % my $templatesDir    = $ce->{courseDirs}{templates};
-% my $shortFilePath   = $isBlank         ? 'newProblem.pg' : $c->{editFilePath} =~ s|^$templatesDir/||r;
-% $shortFilePath      = $isHardcopyTheme ? 'hardcopyThemes/' . ($c->{editFilePath} =~ s|^.*\/||r) : $shortFilePath;
+% my $shortFilePath   =
+	% $isBlank           ? 'newProblem.pg'
+	% : $isHardcopyTheme ? 'hardcopyThemes/' . ($c->{editFilePath} =~ s|^.*\/||r)
+	                   % : $c->{editFilePath} =~ s|^$templatesDir/||r;
 %
 % # Suggest that modifications be saved to the "local" subdirectory if its not in a writeable directory
 % $shortFilePath = "local/$shortFilePath" unless $isBlank || $isHardcopyTheme || -w dirname($c->{editFilePath});
@@ -21,9 +23,9 @@
 %
 % my $probNum = $c->{file_type} eq 'problem' ? $c->{problemID} : 'header';
 %
-% # Don't add or replace problems to sets if the set is the Undefined_Set or if the problem is the blank_problem.
-% my $can_add_problem_to_set =
-	% not_blank($c->{setID}) && $c->{setID} ne 'Undefined_Set' && $c->{file_type} ne 'blank_problem';
+% # Don't add or replace problems to sets if the set is the Undefined_Set or
+% # if the problem is the blank_problem or a sample problem.
+% my $can_add_problem_to_set = not_blank($c->{setID}) && $c->{setID} ne 'Undefined_Set' && !$isBlank;
 %
 % my $prettyProbNum = $probNum;
 % if ($c->{setID}) {

templates/ContentGenerator/Instructor/PGProblemEditor/save_form.html.ep

@@ -1,5 +1,8 @@
 % # Can't save blank problems without changing names, and can't save if lacking write permissions.
-% last unless $c->{file_type} ne 'blank_problem' && $c->{editFilePath} !~ /newProblem\.pg$/ && -w $c->{editFilePath};
+% last if $c->{file_type} eq 'blank_problem'
+	% || $c->{file_type} eq 'sample_problem'
+	% || $c->{editFilePath} =~ /newProblem\.pg$/
+	% || !-w $c->{editFilePath};
 %
 <div>
 	<div class="mb-2">