From d4af9b7616a0035284c5ea93cec7a7a03aab0fcd Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Fri, 11 Aug 2023 17:45:02 -0500 Subject: [PATCH] Improvements to the course directory update method. This always creates a course directory if it is missing. It also attempts to fix permissions if those are not correct. It does not attempt to change ownership or the group as that will almost always fail, and will need to be done manually. The directories that can be copied from the model course are copied if a directory is created. This is only done if the path did not exist to begin with. --- conf/defaults.config | 4 + lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 4 +- lib/WeBWorK/Utils.pm | 2 +- lib/WeBWorK/Utils/CourseIntegrityCheck.pm | 135 ++++++++++-------- .../archive_course_confirm.html.ep | 3 +- 5 files changed, 87 insertions(+), 61 deletions(-) diff --git a/conf/defaults.config b/conf/defaults.config index 5c9cb4b210..257d3f9023 100644 --- a/conf/defaults.config +++ b/conf/defaults.config @@ -339,6 +339,10 @@ $webworkURLs{AuthorHelpURL} ='https://webwork.maa.org/wiki/Category:Au # Defaults for course-specific locations (directories and URLs) ################################################################################ +# Developer note: Make sure to keep the list of directories in the updateCourseDirectories method +# of lib/WeBWorK/Utils/CourseIntegrityCheck.pm up to date with the keys of the $courseDirs variable +# defined below. + # The root directory of the current course. (The ID of the current course is # available in $courseName.) $courseDirs{root} = "$webworkDirs{courses}/$courseName"; diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index 2c36bc6dba..eea7f41b73 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -1563,8 +1563,8 @@ sub do_upgrade_course ($c) { 'li', $c->tag( 'span', - class => $_->[2] ? 'text-success' : 'text-danger', - $_->[1] + class => $_->[1] ? 'text-success' : 'text-danger', + $_->[0] ) ) } @$dir_update_messages diff --git a/lib/WeBWorK/Utils.pm b/lib/WeBWorK/Utils.pm index 8a799b884b..b8b9e29b7d 100644 --- a/lib/WeBWorK/Utils.pm +++ b/lib/WeBWorK/Utils.pm @@ -252,7 +252,7 @@ Creates a directory with the given name, permission bits, and group ID. sub createDirectory { my ($dirName, $permission, $numgid) = @_; - $permission = (defined($permission)) ? $permission : '0770'; + $permission //= 0770; my $errors = ''; mkdir($dirName, $permission) or $errors .= "Can't do mkdir($dirName, $permission): $!\n" . caller(3); diff --git a/lib/WeBWorK/Utils/CourseIntegrityCheck.pm b/lib/WeBWorK/Utils/CourseIntegrityCheck.pm index 70a9b6b2eb..01d7969560 100644 --- a/lib/WeBWorK/Utils/CourseIntegrityCheck.pm +++ b/lib/WeBWorK/Utils/CourseIntegrityCheck.pm @@ -25,6 +25,8 @@ with database schema and that course directory structure is correct. use strict; use warnings; +use Mojo::File qw(path); + use WeBWorK::Debug; use WeBWorK::Utils qw/createDirectory/; use WeBWorK::Utils::CourseManagement qw/listCourses/; @@ -324,85 +326,104 @@ sub checkCourseDirectories { =item $CIchecker->updateCourseDirectories($courseName); -Creates some course directories automatically. +Check to see if all course directories exist and have the correct permissions. -=cut +If a directory does not exist, then it is copied from the model course if the +corresponding directory exists in the model course, and is created otherwise. -# FIXME: This method needs work. It should give better messages, and should at least attempt to fix permissions if -# possible. It also should deal with some of the other course directories that it skips. +If the permissions are not correct, then an attempt is made to correct the +permissions. The permissions are expected to match the course root directory. +If the permissions of the course root directory are not correct, then that will +need to be manually fixed. This method does not check that. + +=cut sub updateCourseDirectories { my $self = shift; my $ce = $self->{ce}; - my @courseDirectories = keys %{ $ce->{courseDirs} }; + my @messages; - #FIXME this is hardwired for the time being. - my %updateable_directories = (html_temp => 1, mailmerge => 1, tmpEditFileDir => 1, hardcopyThemes => 1); + # All $courseDirs keys should be listed here except for root. + # The keys of the $courseDirs hash can not be used directly for lack of order. + my @course_dirs = ( + 'templates', 'DATA', 'html', 'logs', + 'scoring', 'achievements', 'email', 'hardcopyThemes', + 'macros', 'tmpEditFileDir', 'mailmerge', 'achievements_html', + 'html_images', 'html_temp' + ); - my @messages; + # These are the directories in the model course that can be copied if not found in this course. + my %model_course_dirs = ( + templates => 'templates', + html => 'html', + achievements => 'templates/achievements', + email => 'templates/email', + achievements_html => 'html/achievements' + ); + + my $permissions = path($ce->{courseDirs}{root})->stat->mode & 0777; + + for my $dir (@course_dirs) { + my $path = path($ce->{courseDirs}{$dir}); + next if -r $path && -w $path && -x $path; + + my $path_exists_initially = -e $path; + + # Create the directory if it doesn't exist. + if (!$path_exists_initially) { + eval { + $path->make_path({ mode => $permissions }); + push(@messages, [ "Created directory $path.", 1 ]); + }; + if ($@) { + push(@messages, [ "Failed to create directory $path.", 0 ]); + next; + } + } + + # Fix permissions if those are not correct. + if (($path->stat->mode & 0777) != $permissions) { + eval { + $path->chmod($permissions); + push(@messages, [ "Changed permissions for directory $path.", 1 ]); + }; + push(@messages, [ "Failed to change permissions for directory $path.", 0 ]) if $@; + } - for my $dir (sort @courseDirectories) { - # Hack for upgrading the achievements directory. - if ($dir eq 'achievements') { - my $modelCourseAchievementsDir = "$ce->{webworkDirs}{courses}/modelCourse/templates/achievements"; - my $modelCourseAchievementsHtmlDir = "$ce->{webworkDirs}{courses}/modelCourse/html/achievements"; - my $courseAchievementsDir = $ce->{courseDirs}{achievements}; - my $courseAchievementsHtmlDir = $ce->{courseDirs}{achievements_html}; - my $courseTemplatesDir = $ce->{courseDirs}{templates}; - my $courseHtmlDir = $ce->{courseDirs}{html}; - unless (-e $modelCourseAchievementsDir && -e $modelCourseAchievementsHtmlDir) { + # If the path did not exist to begin with and there is a corresponding model course directory, + # then copy the contents of the model course directory. + if (!$path_exists_initially && $model_course_dirs{$dir}) { + my $modelCoursePath = "$ce->{webworkDirs}{courses}/modelCourse/$model_course_dirs{$dir}"; + if (!-r $modelCoursePath) { push( @messages, [ 'Your modelCourse in the "courses" directory is out of date or missing. Please update it from ' - . 'webwork/webwork2/courses.dist directory before upgrading the other courses. Cannot find ' - . "MathAchievements directory $modelCourseAchievementsDir nor MathAchievements picture " - . "directory $modelCourseAchievementsHtmlDir", + . "the webwork2/courses.dist directory. Cannot find directory $modelCoursePath. The " + . "directory $path has been created, but may be missing the files it should contain.", 0 ] ); - } else { - unless (-e $courseAchievementsDir && -e $courseAchievementsHtmlDir) { - push(@messages, - [ "Attempting to update the achievements directory for $ce->{courseDirs}{root}", 1 ]); - if (-e $courseAchievementsDir) { - push(@messages, [ 'Achievements directory is already present', 1 ]); - } else { - system "cp -RPpi $modelCourseAchievementsDir $courseTemplatesDir"; - push(@messages, [ 'Achievements directory created', 1 ]); - } - if (-e $courseAchievementsHtmlDir) { - push(@messages, [ 'Achievements html directory is already present', 1 ]); + next; + } + + eval { + for (path($modelCoursePath)->list_tree({ dir => 1 })->each) { + my $destPath = $_ =~ s!$modelCoursePath!$path!r; + if (-l $_) { + symlink(readlink $_, $destPath); + } elsif (-d $_) { + path($destPath)->make_path({ mode => $permissions }); } else { - system "cp -RPpi $modelCourseAchievementsHtmlDir $courseHtmlDir "; - push(@messages, [ 'Achievements html directory created', 1 ]); + $_->copy_to($destPath); } } - } + push(@messages, [ "Copied model course directory $modelCoursePath to $path.", 1 ]); + }; + push(@messages, [ "Failed to copy model course directory $modelCoursePath to $path: $@.", 0 ]) if $@; } - next unless exists $updateable_directories{$dir}; - my $path = $ce->{courseDirs}->{$dir}; - unless (-e $path) { # If the directory does not exist, create it. - my $parentDirectory = $path; - $parentDirectory =~ s|/$||; # Remove a trailing forward slash - $parentDirectory =~ s|/[^/]*$||; # Remove last node - my ($perms, $groupID) = (stat $parentDirectory)[ 2, 5 ]; - if (-w $parentDirectory) { - createDirectory($path, $perms, $groupID) - or push(@messages, [ "Failed to create directory at $path.", 0 ]); - } else { - push( - @messages, - [ - "Permissions error. Can't create directory at $path. " - . "Lack write permission on $parentDirectory.", - 0 - ] - ); - } - } } return \@messages; diff --git a/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep index a5597adfd8..7f8af988cf 100644 --- a/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep @@ -39,7 +39,8 @@ % if ($directories_ok) {