From 56da9e6a7aabbff3dae1a57e60c90d0e8f50edd3 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Wed, 3 Jun 2026 22:32:44 -0700 Subject: [PATCH 1/3] add-on conf files --- conf/defaults.config | 3 + lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 9 ++- lib/WeBWorK/Utils/CourseManagement.pm | 25 +++++- .../CourseAdmin/add_course_form.html.ep | 76 ++++++++++++++----- templates/HelpFiles/AdminAddCourse.html.ep | 7 +- 5 files changed, 92 insertions(+), 28 deletions(-) diff --git a/conf/defaults.config b/conf/defaults.config index 4c769c02c2..79c2726fc2 100644 --- a/conf/defaults.config +++ b/conf/defaults.config @@ -277,6 +277,9 @@ $webworkDirs{bin} = "$webworkDirs{root}/bin"; # Location of configuration files. $webworkDirs{conf} = "$webworkDirs{root}/conf"; +# Location of add-on configuration files. +$webworkDirs{addOnConf} = "$webworkDirs{conf}/addon"; + # Location of assets (tex, pg, themes) $webworkDirs{assets} = "$webworkDirs{root}/assets"; diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index ff67697c4b..9aaa1dd3a0 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -377,9 +377,12 @@ sub do_add_course ($c) { # Include any optional arguments, including a template course and the course title and course institution. my %optional_arguments; if ($copy_from_course ne '') { - %optional_arguments = map { $_ => 1 } $c->param('copy_component'); - $optional_arguments{copyFrom} = $copy_from_course; - $optional_arguments{copyConfig} = $c->param('copy_config_file'); + %optional_arguments = map { $_ => 1 } $c->param('copy_component'); + $optional_arguments{copyFrom} = $copy_from_course; + $optional_arguments{copyConfig} = + $c->param('copy_config_file') || ($c->param('add_on_conf') && $c->param('add_on_conf') eq '*'); + $optional_arguments{addOnConf} = + $c->param('add_on_conf') && $c->param('add_on_conf') ne '*' ? [ $c->param('add_on_conf') ] : []; } if ($add_courseTitle ne '') { $optional_arguments{courseTitle} = $add_courseTitle; diff --git a/lib/WeBWorK/Utils/CourseManagement.pm b/lib/WeBWorK/Utils/CourseManagement.pm index 3d002d86d6..ce81e0b4eb 100644 --- a/lib/WeBWorK/Utils/CourseManagement.pm +++ b/lib/WeBWorK/Utils/CourseManagement.pm @@ -424,7 +424,12 @@ sub addCourse { my $courseEnvFile = $ce->{courseFiles}{environment}; open my $fh, ">:utf8", $courseEnvFile or die "failed to open $courseEnvFile for writing.\n"; - writeCourseConf($fh); + my $addOnConf = $options{addOnConf} // []; + my $relConfFolder = File::Spec->abs2rel($ce->{webworkDirs}{addOnConf}, $ce->{webworkDirs}{root}); + for (@$addOnConf) { + $_ = File::Spec->catfile($relConfFolder, $_); + } + writeCourseConf($fh, $addOnConf); close $fh; } @@ -1172,24 +1177,36 @@ sub protectQString { return $string; } -=item writeCourseConf($fh) +=item writeCourseConf($fh, $addOnConf) Writes an essentially empty course.conf file to $fh, a file handle. System administrators can use this file to override global settings for a course. +$addOnConf should be an array reference of config files to tack on at the end. =back =cut sub writeCourseConf { - my ($fh) = @_; + my ($fh, $addOnConf) = @_; - print $fh <<'EOF'; + my $content = <<'EOF'; #!perl # This file is used to override the global WeBWorK course environment for this course. EOF + + if ($addOnConf ne '') { + for my $conf (@$addOnConf) { + $content .= <<"EOF"; + +include('$conf'); +EOF + } + } + + print $fh $content; } sub get_SeedCE diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index d6e1ee1413..ac9ff7d93c 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -217,26 +217,62 @@ <%= maketext('course institution (will override "Institution" input above)') =%> -
- -
+ % my $addOnConfFolder = $ce->{webworkDirs}{addOnConf}; + % my @addOnConfFiles; + % if (-d $addOnConfFolder) { + % @addOnConfFiles = glob "$addOnConfFolder/*.conf"; + % for (0..$#addOnConfFiles){ + % $addOnConfFiles[$_] =~ s/^.*\/|\.conf$//g; + % } + % } + % if (@addOnConfFiles) { +
+ <%= label_for add_on_conf => maketext('Configuration File:'), + class => 'col-auto col-form-label fw-bold' =%> +
+ <%= select_field add_on_conf => [ + c( + maketext('Use Default') + => [ [ maketext('Distribution Default Config File') => '' ] ] + ), + c( + maketext('Source Course') + => [ [ maketext("Source Course's Config File") => '*' ] ] + ), + c( + maketext('Append to Distribution Default') + => [ map { [ $_ => "$_.conf" ] } @addOnConfFiles ] + ) + ], + id => 'add_on_conf', + multiple => undef, + size => 8, + class => 'form-select' =%> +
+
+ % } else { +
+ +
+ % } <%= hidden_field last_page_was_add_course => 1 =%> <%= $c->hidden_fields('number_of_additional_users') =%> diff --git a/templates/HelpFiles/AdminAddCourse.html.ep b/templates/HelpFiles/AdminAddCourse.html.ep index 6e7e38c98b..8cdf53a3e6 100644 --- a/templates/HelpFiles/AdminAddCourse.html.ep +++ b/templates/HelpFiles/AdminAddCourse.html.ep @@ -18,10 +18,15 @@ . 'creating future courses, or manage and email course users. Note, by default these new users will be ' . '"Dropped" and unable to login to the [_1] course.', $ce->{admin_course_id}) =%>

-

+

<%= maketext('You may choose a course to copy components from. Select the course and which components to copy. ' . 'If the course is not a true course (like the modelCourse) then only the templates and html folders, ' . 'and the simple and course config files can be copied. The "simple config" file contains the settings ' . 'made in the "Course Config" page. The "course config" file should only be copied if you know what you ' . 'are doing.') =%>

+

+ <%= maketext('If there are .conf files in the [_1] folder, you may select a number of these to include at the ' + . 'end of the course.conf file. This only applies when not copying a course.conf file from another course.', + $ce->{webworkDirs}{addOnConf}) =%> +

From 01dda642e2f5d2a0d7c3ccb6c5e8682865ebb5a5 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Thu, 4 Jun 2026 21:42:33 -0700 Subject: [PATCH 2/3] javascript for Add Course add-on-conf multiselect --- htdocs/js/CourseAdmin/restrict_select.js | 34 +++++++++++++++++++ .../CourseAdmin/add_course_form.html.ep | 10 ++++-- 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 htdocs/js/CourseAdmin/restrict_select.js diff --git a/htdocs/js/CourseAdmin/restrict_select.js b/htdocs/js/CourseAdmin/restrict_select.js new file mode 100644 index 0000000000..8e5550ee34 --- /dev/null +++ b/htdocs/js/CourseAdmin/restrict_select.js @@ -0,0 +1,34 @@ +(() => { + const addOnConfSelect = document.getElementById('add_on_conf'); + const addOnConfOptgroups = [...addOnConfSelect.querySelectorAll('optgroup')]; + + // Track previously selected options to identify the newly clicked option + let previousSelection = []; + + addOnConfSelect.addEventListener('change', (event) => { + const currentSelection = Array.from(addOnConfSelect.selectedOptions); + + // Find the option the user just clicked/selected + const newlySelected = currentSelection.find((option) => !previousSelection.includes(option)); + + if (newlySelected) { + // Find the parent optgroup + const parent = newlySelected.closest('optgroup'); + + // Loop through all options in the other groups and unselect them as appropriate + addOnConfOptgroups.forEach((group) => { + Array.from(group.children).forEach((option) => { + if ( + option !== newlySelected && + (parent.dataset.single || (!parent.dataset.single && group.dataset.single)) + ) { + option.selected = false; + } + }); + }); + } + + // Update tracking variable for the next change event + previousSelection = Array.from(addOnConfSelect.selectedOptions); + }); +})(); diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index ac9ff7d93c..cdadb9175d 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -1,5 +1,9 @@ % use WeBWorK::Utils::CourseManagement qw(listCourses); % +% content_for js => begin + <%= javascript getAssetURL($ce, 'js/CourseAdmin/restrict_select.js'), defer => undef =%> +% end +% % # Create an array of permission values for the permission selects. % my $permissionLevels = []; % for my $role (sort { $ce->{userRoles}{$a} <=> $ce->{userRoles}{$b} } keys %{ $ce->{userRoles} }) { @@ -233,11 +237,13 @@ <%= select_field add_on_conf => [ c( maketext('Use Default') - => [ [ maketext('Distribution Default Config File') => '' ] ] + => [ [ maketext('Distribution Default Config File') => '' ] ], + 'data-single' => 'true' ), c( maketext('Source Course') - => [ [ maketext("Source Course's Config File") => '*' ] ] + => [ [ maketext("Source Course's Config File") => '*' ] ], + 'data-single' => 'true' ), c( maketext('Append to Distribution Default') From 863a143e145b70642457fdf82261de5aae017f5f Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Sat, 6 Jun 2026 05:45:50 -0500 Subject: [PATCH 3/3] Suggested clean up and minor issue fixes. The JavaScript needs to not attempt to do anything with the `add_on_conf` select if it is not present in the page, as that causes errors. For instance if the `$webworkDirs{addOnConf}` directory is not present, or is empty. With modern JavaScript, don't use the array `forEach` method unless it gives a convenient and brief one liner. Generally prefer a `for of` loop as it is more readable. Another advantage is that `for of` loops work on `HTMLCollection`s and `forEach` does not. The result of a `querySelectorAll` call or the `children` of an `Element` are `HTMLCollection`s, and so a `for of` loop works directly, while `Array.from` is needed to use `forEach`. This is a minor inefficiency in this case, but that does require constructing an array from the `HTMLCollection`. I rewrote the `writeCourseConf` POD to clarify its usage some with the new optional parameter. In the `writeCourseConf` method, it should be checked that the `$addOnConf` variable is an array reference, and not that it is not equal to the empty string. Any numeric value or non empty string is not equal to the empty string and would cause an error if `$addOnConf` is set to any of those. The check should be that `ref $addOnConf eq 'ARRAY'`. That is the only condition that guarantees an error will not occur and that the value of the variable will work inside the conditional block. Minor cleanup in the `templates/ContentGenerator/CourseAdmin/add_course_form.html.ep` file. There is no need to copy the value of `$ce->{webworkDirs}{addOnConf}` to a local variable. Particularly since that can be used directly even in string interpolation. --- htdocs/js/CourseAdmin/restrict_select.js | 20 ++++++++++--------- lib/WeBWorK/Utils/CourseManagement.pm | 14 ++++++------- .../CourseAdmin/add_course_form.html.ep | 7 +++---- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/htdocs/js/CourseAdmin/restrict_select.js b/htdocs/js/CourseAdmin/restrict_select.js index 8e5550ee34..984ff1ce17 100644 --- a/htdocs/js/CourseAdmin/restrict_select.js +++ b/htdocs/js/CourseAdmin/restrict_select.js @@ -1,31 +1,33 @@ (() => { const addOnConfSelect = document.getElementById('add_on_conf'); - const addOnConfOptgroups = [...addOnConfSelect.querySelectorAll('optgroup')]; + if (!addOnConfSelect) return; + + const addOnConfOptgroups = addOnConfSelect.querySelectorAll('optgroup'); // Track previously selected options to identify the newly clicked option let previousSelection = []; - addOnConfSelect.addEventListener('change', (event) => { - const currentSelection = Array.from(addOnConfSelect.selectedOptions); - + addOnConfSelect.addEventListener('change', () => { // Find the option the user just clicked/selected - const newlySelected = currentSelection.find((option) => !previousSelection.includes(option)); + const newlySelected = Array.from(addOnConfSelect.selectedOptions).find( + (option) => !previousSelection.includes(option) + ); if (newlySelected) { // Find the parent optgroup const parent = newlySelected.closest('optgroup'); // Loop through all options in the other groups and unselect them as appropriate - addOnConfOptgroups.forEach((group) => { - Array.from(group.children).forEach((option) => { + for (const group of addOnConfOptgroups) { + for (const option of group.children) { if ( option !== newlySelected && (parent.dataset.single || (!parent.dataset.single && group.dataset.single)) ) { option.selected = false; } - }); - }); + } + } } // Update tracking variable for the next change event diff --git a/lib/WeBWorK/Utils/CourseManagement.pm b/lib/WeBWorK/Utils/CourseManagement.pm index ce81e0b4eb..097b837d89 100644 --- a/lib/WeBWorK/Utils/CourseManagement.pm +++ b/lib/WeBWorK/Utils/CourseManagement.pm @@ -1179,9 +1179,10 @@ sub protectQString { =item writeCourseConf($fh, $addOnConf) -Writes an essentially empty course.conf file to $fh, a file handle. System -administrators can use this file to override global settings for a course. -$addOnConf should be an array reference of config files to tack on at the end. +Writes the course.conf file to C<$fh>, a file handle. System administrators can +use this file to override global settings for a course. If C<$addOnConf> is +provided, then it should be a reference to an array of config files to be +included at the end of the course.conf file. =back @@ -1197,12 +1198,9 @@ sub writeCourseConf { EOF - if ($addOnConf ne '') { + if (ref $addOnConf eq 'ARRAY') { for my $conf (@$addOnConf) { - $content .= <<"EOF"; - -include('$conf'); -EOF + $content .= "\ninclude('$conf');"; } } diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index cdadb9175d..2a4be2cbd7 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -221,11 +221,10 @@ <%= maketext('course institution (will override "Institution" input above)') =%> - % my $addOnConfFolder = $ce->{webworkDirs}{addOnConf}; % my @addOnConfFiles; - % if (-d $addOnConfFolder) { - % @addOnConfFiles = glob "$addOnConfFolder/*.conf"; - % for (0..$#addOnConfFiles){ + % if (-d $ce->{webworkDirs}{addOnConf}) { + % @addOnConfFiles = glob "$ce->{webworkDirs}{addOnConf}/*.conf"; + % for (0 .. $#addOnConfFiles){ % $addOnConfFiles[$_] =~ s/^.*\/|\.conf$//g; % } % }