From 4f72e38aea12a922f73709569c707c3a51760d69 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Tue, 8 Aug 2023 16:47:18 -0400 Subject: [PATCH 01/18] Add zip capability to archiving/unarchiving in File Manager --- conf/site.conf.dist | 4 +++- htdocs/js/FileManager/filemanager.js | 2 +- .../Instructor/FileManager.pm | 20 ++++++++++++------- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/conf/site.conf.dist b/conf/site.conf.dist index 4a00f93e23..f5aae482c5 100644 --- a/conf/site.conf.dist +++ b/conf/site.conf.dist @@ -88,7 +88,9 @@ $externalPrograms{rm} = "/bin/rm"; $externalPrograms{mkdir} = "/bin/mkdir"; $externalPrograms{tar} = "/bin/tar"; $externalPrograms{gzip} = "/bin/gzip"; -$externalPrograms{git} = "/usr/bin/git"; +$externalPrograms{git} = "/usr/bin/git"; +$externalPrograms{unzip} = '/usr/bin/zip'; +$externalPrograms{unzip} = '/usr/bin/unzip'; #################################################### # equation rendering/hardcopy utiltiies diff --git a/htdocs/js/FileManager/filemanager.js b/htdocs/js/FileManager/filemanager.js index 8f579d6bf8..5a55c529e4 100644 --- a/htdocs/js/FileManager/filemanager.js +++ b/htdocs/js/FileManager/filemanager.js @@ -35,7 +35,7 @@ if ( numSelected === 0 || numSelected > 1 || - !/\.(tar|tar\.gz|tgz)$/.test(files.children[files.selectedIndex].value) + !/\.(tar|tar\.gz|tgz|zip)$/.test(files.children[files.selectedIndex].value) ) archiveButton.value = archiveButton.dataset.archiveText; else archiveButton.value = archiveButton.dataset.unarchiveText; diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm index 5869750e92..8b20aa124f 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm @@ -370,8 +370,8 @@ sub MakeArchive ($c) { sub UnpackArchive ($c) { my $archive = $c->getFile('unpack'); return '' unless $archive; - if ($archive !~ m/\.(tar|tar\.gz|tgz)$/) { - $c->addbadmessage($c->maketext('You can only unpack files ending in ".tgz", ".tar" or ".tar.gz"')); + if ($archive !~ m/\.(tar|tar\.gz|tgz|zip)$/) { + $c->addbadmessage($c->maketext('You can only unpack files ending in ".zip", ".tgz", ".tar" or ".tar.gz"')); } else { $c->unpack_archive($archive); } @@ -379,10 +379,16 @@ sub UnpackArchive ($c) { } sub unpack_archive ($c, $archive) { - my $z = $archive =~ m/\.tar$/ ? '' : 'z'; - my $dir = "$c->{courseRoot}/$c->{pwd}"; - my $tar = 'cd ' . shell_quote($dir) . " && $c->{ce}{externalPrograms}{tar} -vx${z}f " . shell_quote($archive); - my @files = readpipe "$tar 2>&1"; + my $dir = "$c->{courseRoot}/$c->{pwd}"; + my @files; + if ($archive =~ m/\.zip$/) { + my $unzip = 'cd ' . shell_quote($dir) . " && $c->{ce}{externalPrograms}{unzip} -u " . shell_quote($archive); + @files = readpipe "$unzip"; + } else { + my $z = $archive =~ m/\.tar$/ ? '' : 'z'; + my $tar = 'cd ' . shell_quote($dir) . " && $c->{ce}{externalPrograms}{tar} -vx${z}f " . shell_quote($archive); + @files = readpipe "$tar 2>&1"; + } if ($? == 0) { my $n = scalar(@files); @@ -520,7 +526,7 @@ sub Upload ($c) { if (-e $file) { $c->addgoodmessage($c->maketext('File "[_1]" uploaded successfully', $name)); - if ($name =~ m/\.(tar|tar\.gz|tgz)$/ && $c->getFlag('unpack')) { + if ($name =~ m/\.(tar|tar\.gz|tgz|zip)$/ && $c->getFlag('unpack')) { if ($c->unpack_archive($name) && $c->getFlag('autodelete')) { if (unlink($file)) { $c->addgoodmessage($c->maketext('Archive "[_1]" deleted', $name)) } else { $c->addbadmessage($c->maketext(q{Can't delete archive "[_1]": [_2]}, $name, $!)) } From b42783c59d7bc8fd0d0a5175012f7273ab282282 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Thu, 10 Aug 2023 08:48:44 -0400 Subject: [PATCH 02/18] replace call to zip with perl packages and include make archive --- conf/site.conf.dist | 2 - .../Instructor/FileManager.pm | 55 +++++++++++-------- .../Instructor/FileManager/refresh.html.ep | 10 +++- 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/conf/site.conf.dist b/conf/site.conf.dist index f5aae482c5..9591b1e151 100644 --- a/conf/site.conf.dist +++ b/conf/site.conf.dist @@ -89,8 +89,6 @@ $externalPrograms{mkdir} = "/bin/mkdir"; $externalPrograms{tar} = "/bin/tar"; $externalPrograms{gzip} = "/bin/gzip"; $externalPrograms{git} = "/usr/bin/git"; -$externalPrograms{unzip} = '/usr/bin/zip'; -$externalPrograms{unzip} = '/usr/bin/unzip'; #################################################### # equation rendering/hardcopy utiltiies diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm index 8b20aa124f..c496122f2c 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm @@ -26,6 +26,9 @@ use File::Path; use File::Copy; use File::Spec; use String::ShellQuote; +use Archive::Extract; +use IO::Compress::Zip qw(zip $ZipError); +use Archive::Tar; use WeBWorK::Utils qw(readDirectory readFile sortByName listFilesRecursive); use WeBWorK::Upload; @@ -344,24 +347,37 @@ sub Delete ($c) { } } -# Make a gzipped tar archive +# Make a gzipped tar or zip archive sub MakeArchive ($c) { my @files = $c->param('files'); if (scalar(@files) == 0) { $c->addbadmessage($c->maketext('You must select at least one file for the archive')); return $c->Refresh; } + my $dir = "$c->{courseRoot}/$c->{pwd}"; + chdir $dir; + my @files_to_compress; + + for my $f (@files) { + push(@files_to_compress, glob("$f/**")) if -d $f; + push(@files_to_compress, $f) if -f $f; + } - my $dir = "$c->{courseRoot}/$c->{pwd}"; - my $archive = uniqueName($dir, (scalar(@files) == 1) ? $files[0] . '.tgz' : "$c->{courseName}.tgz"); - my $tar = 'cd ' . shell_quote($dir) . " && $c->{ce}{externalPrograms}{tar} -cvzf " . shell_quote($archive, @files); - @files = readpipe $tar . ' 2>&1'; - if ($? == 0) { - my $n = scalar(@files); - $c->addgoodmessage($c->maketext('Archive "[_1]" created successfully ([quant,_2,file])', $archive, $n)); + my ($archive, $error, $ok); + if ($c->param('archive_type') eq 'zip') { + $archive = uniqueName('', scalar(@files) == 1 ? $files[0] . '.zip' : "$c->{courseName}.zip"); + $ok = zip [@files_to_compress] => $archive; + $error = $ZipError unless $ok; + } else { + $archive = uniqueName('', (scalar(@files) == 1) ? $files[0] . '.tgz' : "$c->{courseName}.tgz"); + $ok = Archive::Tar->create_archive($archive, COMPRESS_GZIP, @files_to_compress); + $error = $Archive::Tar::error unless $ok; + } + if ($ok) { + my $n = scalar(@files_to_compress); + $c->addgoodmessage($c->maketext('Archive "[_1]" created successfully ([quant, _2, file])', $archive, $n)); } else { - $c->addbadmessage( - $c->maketext(q{Can't create archive "[_1]": command returned [_2]}, $archive, systemError($?))); + $c->addbadmessage($c->maketext(q{Can't create archive "[_1]": command returned [_2]}, $archive, $error)); } return $c->Refresh; } @@ -379,23 +395,16 @@ sub UnpackArchive ($c) { } sub unpack_archive ($c, $archive) { - my $dir = "$c->{courseRoot}/$c->{pwd}"; - my @files; - if ($archive =~ m/\.zip$/) { - my $unzip = 'cd ' . shell_quote($dir) . " && $c->{ce}{externalPrograms}{unzip} -u " . shell_quote($archive); - @files = readpipe "$unzip"; - } else { - my $z = $archive =~ m/\.tar$/ ? '' : 'z'; - my $tar = 'cd ' . shell_quote($dir) . " && $c->{ce}{externalPrograms}{tar} -vx${z}f " . shell_quote($archive); - @files = readpipe "$tar 2>&1"; - } + my $dir = "$c->{courseRoot}/$c->{pwd}"; + my $arch = Archive::Extract->new(archive => "$dir/$archive"); + my $ok = $arch->extract(to => $dir); - if ($? == 0) { - my $n = scalar(@files); + if ($ok) { + my $n = scalar(@{ $arch->files }); $c->addgoodmessage($c->maketext('[quant,_1,file] unpacked successfully', $n)); return 1; } else { - $c->addbadmessage($c->maketext(q{Can't unpack "[_1]": command returned [_2]}, $archive, systemError($?))); + $c->addbadmessage($c->maketext(q{Can't unpack "[_1]": command returned [_2]}, $archive, $arch->error)); return 0; } } diff --git a/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep b/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep index e7c5db0ac6..b2efa10aec 100644 --- a/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep @@ -40,7 +40,15 @@ <%= submit_button maketext('Download'), id => 'Download', %button =%> <%= submit_button maketext('Rename'), id => 'Rename', %button =%> <%= submit_button maketext('Copy'), id => 'Copy', %button =%> - <%= submit_button maketext('Delete'), id => 'Delete', %button =%>\ + <%= submit_button maketext('Delete'), id => 'Delete', %button =%> +
+ <%= label_for archive_type => maketext('Archive Type'), class => 'col-auto col-form-label fw-bold' =%> +
+ <%= select_field archive_type => [ [ 'Zip File' => 'zip'], ['Compressed Tar' => 'tgz'] ], + id => 'archive-type', + class => 'form-select', + =%> +
<%= submit_button maketext('Make Archive'), id => 'MakeArchive', data => { archive_text => maketext('Make Archive'), From f5182e80f6b4ad12d8a0635f640c0137a6efcfd2 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Thu, 10 Aug 2023 17:51:06 -0400 Subject: [PATCH 03/18] update check_modules and add new UI for archive This adds Archive::Extract to check_modules.pl and libarchive-extract-perl to docker files. In addition, the UI now has a secondary page for creating an archive. ran perltidy --- Dockerfile | 1 + DockerfileStage1 | 1 + bin/check_modules.pl | 1 + htdocs/js/FileManager/filemanager.js | 17 +++++ .../Instructor/FileManager.pm | 52 +++++++++------- .../Instructor/FileManager/archive.html.ep | 62 +++++++++++++++++++ .../Instructor/FileManager/refresh.html.ep | 8 --- 7 files changed, 112 insertions(+), 30 deletions(-) create mode 100644 templates/ContentGenerator/Instructor/FileManager/archive.html.ep diff --git a/Dockerfile b/Dockerfile index 349bb61816..c8e32b0d9d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -71,6 +71,7 @@ RUN apt-get update \ imagemagick \ iputils-ping \ jq \ + libarchive-extract-perl \ libarchive-zip-perl \ libarray-utils-perl \ libc6-dev \ diff --git a/DockerfileStage1 b/DockerfileStage1 index d468ec0785..a844eab55e 100644 --- a/DockerfileStage1 +++ b/DockerfileStage1 @@ -33,6 +33,7 @@ RUN apt-get update \ imagemagick \ iputils-ping \ jq \ + libarchive-extract-perl \ libarchive-zip-perl \ libarray-utils-perl \ libc6-dev \ diff --git a/bin/check_modules.pl b/bin/check_modules.pl index e27900f784..b4261f51a8 100755 --- a/bin/check_modules.pl +++ b/bin/check_modules.pl @@ -65,6 +65,7 @@ =head1 DESCRIPTION ); my @modulesList = qw( + Archive::Extract Archive::Zip Array::Utils Benchmark diff --git a/htdocs/js/FileManager/filemanager.js b/htdocs/js/FileManager/filemanager.js index 5a55c529e4..e7e72552bc 100644 --- a/htdocs/js/FileManager/filemanager.js +++ b/htdocs/js/FileManager/filemanager.js @@ -42,6 +42,23 @@ } }; + // Used for the archive subpage to highlight all in the Select + const selectAllButton = document.getElementById('select-all-files-button'); + selectAllButton?.addEventListener('click', () => { + const n = document.getElementById('archive-files').options.length; + for (const opt of document.getElementById('archive-files').options) { + opt.selected = 'selected'; + } + }); + + + for (const r of document.querySelectorAll('input[name="archive_type"]')) { + r.addEventListener('click', () => { + const suffix = document.querySelector('input[name="archive_type"]:checked').value; + document.getElementById('filename_suffix').innerText = '.' + suffix; + }); + } + files?.addEventListener('change', checkFiles); if (files) checkFiles(); diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm index c496122f2c..0638353c9a 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm @@ -354,32 +354,40 @@ sub MakeArchive ($c) { $c->addbadmessage($c->maketext('You must select at least one file for the archive')); return $c->Refresh; } - my $dir = "$c->{courseRoot}/$c->{pwd}"; - chdir $dir; - my @files_to_compress; - for my $f (@files) { - push(@files_to_compress, glob("$f/**")) if -d $f; - push(@files_to_compress, $f) if -f $f; - } + my $dir = "$c->{courseRoot}/$c->{pwd}"; + if ($c->param('confirmed')) { + chdir($dir); + # remove any directories + my @files_to_compress = grep { -f $_ } @files; + + unless ($c->param('archive_filename') && scalar(@files_to_compress) > 0) { + $c->addbadmessage($c->maketext('The filename cannot be empty.')) unless $c->param('archive_filename'); + $c->addbadmessage($c->maketext('At least one file must be selected')) unless scalar(@files_to_compress) > 0; + return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); + } - my ($archive, $error, $ok); - if ($c->param('archive_type') eq 'zip') { - $archive = uniqueName('', scalar(@files) == 1 ? $files[0] . '.zip' : "$c->{courseName}.zip"); - $ok = zip [@files_to_compress] => $archive; - $error = $ZipError unless $ok; - } else { - $archive = uniqueName('', (scalar(@files) == 1) ? $files[0] . '.tgz' : "$c->{courseName}.tgz"); - $ok = Archive::Tar->create_archive($archive, COMPRESS_GZIP, @files_to_compress); - $error = $Archive::Tar::error unless $ok; - } - if ($ok) { - my $n = scalar(@files_to_compress); - $c->addgoodmessage($c->maketext('Archive "[_1]" created successfully ([quant, _2, file])', $archive, $n)); + my $archive = $c->param('archive_filename'); + my ($error, $ok); + if ($c->param('archive_type') eq 'zip') { + $archive .= '.zip'; + $ok = zip \@files_to_compress => $archive; + $error = $ZipError unless $ok; + } else { + $archive .= '.tgz'; + $ok = Archive::Tar->create_archive($archive, COMPRESS_GZIP, @files_to_compress); + $error = $Archive::Tar::error unless $ok; + } + if ($ok) { + my $n = scalar(@files); + $c->addgoodmessage($c->maketext('Archive "[_1]" created successfully ([quant,_2,file])', $archive, $n)); + } else { + $c->addbadmessage($c->maketext(q{Can't create archive "[_1]": command returned [_2]}, $archive, $error)); + } + return $c->Refresh; } else { - $c->addbadmessage($c->maketext(q{Can't create archive "[_1]": command returned [_2]}, $archive, $error)); + return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); } - return $c->Refresh; } # Unpack a gzipped tar archive diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep new file mode 100644 index 0000000000..f70061c8ed --- /dev/null +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -0,0 +1,62 @@ +% # template for the archive subpage. +
+
+
+ <%= maketext('The following files have been selected for archiving. Select the type ' + . 'of archive and any subset of the requested files.') =%> +
+
+
+
+ <%= maketext('Archive Filename') %>: + <%= text_field archive_filename => '', + 'aria-labelledby' => 'archive_filename', placeholder => maketext('Archive Filename'), + class => 'form-control', size => 30 =%> + .zip +
+
+
+
+
+ <%= maketext('Archive Type') %>: +
+ + + +
+
+
+ + + % my @files_to_compress; + % chdir($dir); + % for my $f (@$files) { + % push(@files_to_compress, glob("$f/**")) if -d $f; + % push(@files_to_compress, $f) if -f $f; + % } + % # remove any directories + % @files_to_compress = grep { -f $_ } @files_to_compress; + + <%= select_field files => \@files_to_compress, + id => 'archive-files', + class => 'form-select', + size => 20, + multiple => undef + =%> + +

<%= maketext('Create the archive of the select files?') %>

+
+ <%= submit_button maketext('Cancel'), name => 'action', class => 'btn btn-sm btn-secondary' =%> + <%= submit_button maketext('Make Archive'), name => 'action', class => 'btn btn-sm btn-primary' =%> +
+
+
+<%= $c->HiddenFlags =%> diff --git a/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep b/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep index b2efa10aec..ee2f28562c 100644 --- a/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep @@ -41,14 +41,6 @@ <%= submit_button maketext('Rename'), id => 'Rename', %button =%> <%= submit_button maketext('Copy'), id => 'Copy', %button =%> <%= submit_button maketext('Delete'), id => 'Delete', %button =%> -
- <%= label_for archive_type => maketext('Archive Type'), class => 'col-auto col-form-label fw-bold' =%> -
- <%= select_field archive_type => [ [ 'Zip File' => 'zip'], ['Compressed Tar' => 'tgz'] ], - id => 'archive-type', - class => 'form-select', - =%> -
<%= submit_button maketext('Make Archive'), id => 'MakeArchive', data => { archive_text => maketext('Make Archive'), From 7c829e6b601d5291de032a556ca278df96fb1eb9 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Tue, 15 Aug 2023 18:50:19 -0500 Subject: [PATCH 04/18] Fixes and improvements for the new "Make Archive" page. Fix the action. There needs to be a hidden "confirm" input. Improve layouts. Also other clean up suggested in my review of #2163. Don't use `chdir`. The IO::Compression::Zip module is too limited. It can not add directories to the zip file. Empty directories in particular should be. So use Archive::Zip instead. This module is already in use by webwork as well. --- htdocs/js/FileManager/filemanager.js | 18 +++---- .../Instructor/FileManager.pm | 53 ++++++++++++++----- .../Instructor/FileManager/archive.html.ep | 52 +++++++++--------- .../Instructor/FileManager/refresh.html.ep | 4 +- 4 files changed, 76 insertions(+), 51 deletions(-) diff --git a/htdocs/js/FileManager/filemanager.js b/htdocs/js/FileManager/filemanager.js index e7e72552bc..40d17ddc40 100644 --- a/htdocs/js/FileManager/filemanager.js +++ b/htdocs/js/FileManager/filemanager.js @@ -43,19 +43,17 @@ }; // Used for the archive subpage to highlight all in the Select - const selectAllButton = document.getElementById('select-all-files-button'); - selectAllButton?.addEventListener('click', () => { - const n = document.getElementById('archive-files').options.length; - for (const opt of document.getElementById('archive-files').options) { - opt.selected = 'selected'; + document.getElementById('select-all-files-button')?.addEventListener('click', () => { + for (const option of document.getElementById('archive-files').options) { + option.selected = 'selected'; } }); - - for (const r of document.querySelectorAll('input[name="archive_type"]')) { - r.addEventListener('click', () => { - const suffix = document.querySelector('input[name="archive_type"]:checked').value; - document.getElementById('filename_suffix').innerText = '.' + suffix; + for (const archiveTypeInput of document.querySelectorAll('input[name="archive_type"]')) { + archiveTypeInput.addEventListener('click', () => { + document.getElementById('filename_suffix').innerText = `.${ + document.querySelector('input[name="archive_type"]:checked').value + }`; }); } diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm index 0638353c9a..b5336da263 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm @@ -27,8 +27,8 @@ use File::Copy; use File::Spec; use String::ShellQuote; use Archive::Extract; -use IO::Compress::Zip qw(zip $ZipError); use Archive::Tar; +use Archive::Zip qw(:ERROR_CODES); use WeBWorK::Utils qw(readDirectory readFile sortByName listFilesRecursive); use WeBWorK::Upload; @@ -356,14 +356,18 @@ sub MakeArchive ($c) { } my $dir = "$c->{courseRoot}/$c->{pwd}"; + if ($c->param('confirmed')) { - chdir($dir); - # remove any directories - my @files_to_compress = grep { -f $_ } @files; + my $action = $c->param('action') || 'Cancel'; + return $c->Refresh if $action eq 'Cancel' || $action eq $c->maketext('Cancel'); - unless ($c->param('archive_filename') && scalar(@files_to_compress) > 0) { - $c->addbadmessage($c->maketext('The filename cannot be empty.')) unless $c->param('archive_filename'); - $c->addbadmessage($c->maketext('At least one file must be selected')) unless scalar(@files_to_compress) > 0; + unless ($c->param('archive_filename')) { + $c->addbadmessage($c->maketext('The filename cannot be empty.')); + return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); + } + + unless (@files > 0) { + $c->addbadmessage($c->maketext('At least one file must be selected')); return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); } @@ -371,18 +375,39 @@ sub MakeArchive ($c) { my ($error, $ok); if ($c->param('archive_type') eq 'zip') { $archive .= '.zip'; - $ok = zip \@files_to_compress => $archive; - $error = $ZipError unless $ok; + my $zip = Archive::Zip->new(); + for (@files) { + my $fullFile = "$dir/$_"; + + # Skip symbolic links for now. As of yet, I have not found a perl module that can add symbolic links to + # zip files correctly. Archive::Zip should be able to do this, but has permissions issues doing so. + next if -l $fullFile; + + if (-d $fullFile) { + $zip->addDirectory($fullFile => $_); + } else { + $zip->addFile($fullFile => $_); + } + } + $ok = $zip->writeToFileNamed("$dir/$archive") == AZ_OK; + # FIXME: This should check the error code, and give a more specific error message. + $error = 'Unable to create zip archive.' unless $ok; } else { $archive .= '.tgz'; - $ok = Archive::Tar->create_archive($archive, COMPRESS_GZIP, @files_to_compress); - $error = $Archive::Tar::error unless $ok; + my $tar = Archive::Tar->new; + $tar->add_files(map {"$dir/$_"} @files); + # Make file names in the archive relative to the current working directory. + for ($tar->get_files) { + $tar->rename($_->full_path, $_->full_path =~ s!^$dir/!!r); + } + $ok = $tar->write("$dir/$archive", COMPRESS_GZIP); + $error = $tar->error unless $ok; } if ($ok) { - my $n = scalar(@files); - $c->addgoodmessage($c->maketext('Archive "[_1]" created successfully ([quant,_2,file])', $archive, $n)); + $c->addgoodmessage( + $c->maketext('Archive "[_1]" created successfully ([quant,_2,file])', $archive, scalar(@files))); } else { - $c->addbadmessage($c->maketext(q{Can't create archive "[_1]": command returned [_2]}, $archive, $error)); + $c->addbadmessage($c->maketext(q{Can't create archive "[_1]": [_2]}, $archive, $error)); } return $c->Refresh; } else { diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep index f70061c8ed..0e1b0d3bcc 100644 --- a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -1,12 +1,13 @@ -% # template for the archive subpage. -
+% use Mojo::File qw(path); +% +
<%= maketext('The following files have been selected for archiving. Select the type ' - . 'of archive and any subset of the requested files.') =%> + . 'of archive and any subset of the requested files.') =%>
-
+
<%= maketext('Archive Filename') %>: <%= text_field archive_filename => '', @@ -20,38 +21,38 @@
<%= maketext('Archive Type') %>:
-
- - +
+
+ +
+
+ % % my @files_to_compress; - % chdir($dir); - % for my $f (@$files) { - % push(@files_to_compress, glob("$f/**")) if -d $f; - % push(@files_to_compress, $f) if -f $f; + % for my $file (@$files) { + % push(@files_to_compress, $file); + % my $path = path("$dir/$file"); + % push(@files_to_compress, @{ $path->list_tree({ hidden => 1 })->map('to_rel', $dir) }) + % if (-d $path && !-l $path); % } - % # remove any directories - % @files_to_compress = grep { -f $_ } @files_to_compress; - - <%= select_field files => \@files_to_compress, - id => 'archive-files', - class => 'form-select', - size => 20, - multiple => undef - =%> - + % + % # Select all files initially. Even those that are in previously selected directories or subdirectories. + % param('files', \@files_to_compress); + <%= select_field files => \@files_to_compress, id => 'archive-files', class => 'form-select mb-2', + size => 20, multiple => undef =%> + %

<%= maketext('Create the archive of the select files?') %>

<%= submit_button maketext('Cancel'), name => 'action', class => 'btn btn-sm btn-secondary' =%> @@ -59,4 +60,5 @@
+<%= hidden_field confirmed => 'MakeArchive' =%> <%= $c->HiddenFlags =%> diff --git a/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep b/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep index ee2f28562c..42f1053130 100644 --- a/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/refresh.html.ep @@ -34,7 +34,7 @@ dir => 'ltr', size => 17, multiple => undef =%>
-
+
<%= submit_button maketext('View'), id => 'View', %button =%> <%= submit_button maketext('Edit'), id => 'Edit', %button =%> <%= submit_button maketext('Download'), id => 'Download', %button =%> @@ -50,7 +50,7 @@ % unless ($c->{courseName} eq 'admin') { <%= submit_button maketext('Archive Course'), id => 'ArchiveCourse', %button =%> % } -
+
<%= submit_button maketext('New File'), id => 'NewFile', %button =%> <%= submit_button maketext('New Folder'), id => 'NewFolder', %button =%> <%= submit_button maketext('Refresh'), id => 'Refresh', %button =%> From b90a4663036fadcfde576f41ae74d0b76e90dd27 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 16 Aug 2023 11:02:13 -0500 Subject: [PATCH 05/18] Switch to using the Archive::Zip::SimpleZip module for creating zip archives. This modules supports symbolic links. --- Dockerfile | 2 +- DockerfileStage1 | 2 +- bin/check_modules.pl | 1 + .../Instructor/FileManager.pm | 30 ++++++------------- .../Instructor/FileManager/archive.html.ep | 2 +- 5 files changed, 13 insertions(+), 24 deletions(-) diff --git a/Dockerfile b/Dockerfile index c8e32b0d9d..f735e07e69 100644 --- a/Dockerfile +++ b/Dockerfile @@ -185,7 +185,7 @@ RUN apt-get update \ # ================================================================== # Phase 4 - Install additional Perl modules from CPAN that are not packaged for Ubuntu or are outdated in Ubuntu. -RUN cpanm install Statistics::R::IO DBD::MariaDB Mojo::SQLite@3.002 Perl::Tidy@20220613 \ +RUN cpanm install Statistics::R::IO DBD::MariaDB Mojo::SQLite@3.002 Perl::Tidy@20220613 Archive::Zip::SimpleZip \ && rm -fr ./cpanm /root/.cpanm /tmp/* # ================================================================== diff --git a/DockerfileStage1 b/DockerfileStage1 index a844eab55e..85357f3597 100644 --- a/DockerfileStage1 +++ b/DockerfileStage1 @@ -147,7 +147,7 @@ RUN apt-get update \ # ================================================================== # Phase 3 - Install additional Perl modules from CPAN that are not packaged for Ubuntu or are outdated in Ubuntu. -RUN cpanm install -n Statistics::R::IO DBD::MariaDB Mojo::SQLite@3.002 Perl::Tidy@20220613 \ +RUN cpanm install -n Statistics::R::IO DBD::MariaDB Mojo::SQLite@3.002 Perl::Tidy@20220613 Archive::Zip::SimpleZip \ && rm -fr ./cpanm /root/.cpanm /tmp/* # ================================================================== diff --git a/bin/check_modules.pl b/bin/check_modules.pl index b4261f51a8..bbb72196f5 100755 --- a/bin/check_modules.pl +++ b/bin/check_modules.pl @@ -67,6 +67,7 @@ =head1 DESCRIPTION my @modulesList = qw( Archive::Extract Archive::Zip + Archive::Zip::SimpleZip Array::Utils Benchmark Carp diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm index b5336da263..e3acf6185f 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm @@ -28,7 +28,7 @@ use File::Spec; use String::ShellQuote; use Archive::Extract; use Archive::Tar; -use Archive::Zip qw(:ERROR_CODES); +use Archive::Zip::SimpleZip qw($SimpleZipError); use WeBWorK::Utils qw(readDirectory readFile sortByName listFilesRecursive); use WeBWorK::Upload; @@ -355,7 +355,7 @@ sub MakeArchive ($c) { return $c->Refresh; } - my $dir = "$c->{courseRoot}/$c->{pwd}"; + my $dir = $c->{pwd} eq '.' ? $c->{courseRoot} : "$c->{courseRoot}/$c->{pwd}"; if ($c->param('confirmed')) { my $action = $c->param('action') || 'Cancel'; @@ -375,23 +375,13 @@ sub MakeArchive ($c) { my ($error, $ok); if ($c->param('archive_type') eq 'zip') { $archive .= '.zip'; - my $zip = Archive::Zip->new(); - for (@files) { - my $fullFile = "$dir/$_"; - - # Skip symbolic links for now. As of yet, I have not found a perl module that can add symbolic links to - # zip files correctly. Archive::Zip should be able to do this, but has permissions issues doing so. - next if -l $fullFile; - - if (-d $fullFile) { - $zip->addDirectory($fullFile => $_); - } else { - $zip->addFile($fullFile => $_); + if (my $zip = Archive::Zip::SimpleZip->new("$dir/$archive")) { + for (@files) { + $zip->add("$dir/$_", Name => $_, storelinks => 1); } + $ok = $zip->close; } - $ok = $zip->writeToFileNamed("$dir/$archive") == AZ_OK; - # FIXME: This should check the error code, and give a more specific error message. - $error = 'Unable to create zip archive.' unless $ok; + $error = $SimpleZipError unless $ok; } else { $archive .= '.tgz'; my $tar = Archive::Tar->new; @@ -430,11 +420,9 @@ sub UnpackArchive ($c) { sub unpack_archive ($c, $archive) { my $dir = "$c->{courseRoot}/$c->{pwd}"; my $arch = Archive::Extract->new(archive => "$dir/$archive"); - my $ok = $arch->extract(to => $dir); - if ($ok) { - my $n = scalar(@{ $arch->files }); - $c->addgoodmessage($c->maketext('[quant,_1,file] unpacked successfully', $n)); + if ($arch->extract(to => $dir)) { + $c->addgoodmessage($c->maketext('[quant,_1,file] unpacked successfully', scalar(@{ $arch->files }))); return 1; } else { $c->addbadmessage($c->maketext(q{Can't unpack "[_1]": command returned [_2]}, $archive, $arch->error)); diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep index 0e1b0d3bcc..ba544384c9 100644 --- a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -44,7 +44,7 @@ % for my $file (@$files) { % push(@files_to_compress, $file); % my $path = path("$dir/$file"); - % push(@files_to_compress, @{ $path->list_tree({ hidden => 1 })->map('to_rel', $dir) }) + % push(@files_to_compress, @{ $path->list_tree({ dir => 1, hidden => 1 })->map('to_rel', $dir) }) % if (-d $path && !-l $path); % } % From e7b1b0a9b8f4ec3c83e4f08f15711299aab5fa94 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Wed, 16 Aug 2023 17:37:40 -0400 Subject: [PATCH 06/18] Changes selecting files to Mojo::File and other fixes. --- .../Instructor/FileManager.pm | 87 +++++++++---------- .../Instructor/FileManager.html.ep | 1 + .../Instructor/FileManager/archive.html.ep | 2 +- 3 files changed, 43 insertions(+), 47 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm index e3acf6185f..d7c965ed34 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm @@ -25,6 +25,7 @@ WeBWorK::ContentGenerator::Instructor::FileManager.pm -- simple directory manage use File::Path; use File::Copy; use File::Spec; +use Mojo::File; use String::ShellQuote; use Archive::Extract; use Archive::Tar; @@ -347,62 +348,56 @@ sub Delete ($c) { } } -# Make a gzipped tar or zip archive +# Call the make archive template. sub MakeArchive ($c) { my @files = $c->param('files'); if (scalar(@files) == 0) { $c->addbadmessage($c->maketext('You must select at least one file for the archive')); return $c->Refresh; + } else { + return $c->include( + 'ContentGenerator/Instructor/FileManager/archive', + dir => "$c->{courseRoot}/$c->{pwd}", + files => \@files + ); } +} - my $dir = $c->{pwd} eq '.' ? $c->{courseRoot} : "$c->{courseRoot}/$c->{pwd}"; - - if ($c->param('confirmed')) { - my $action = $c->param('action') || 'Cancel'; - return $c->Refresh if $action eq 'Cancel' || $action eq $c->maketext('Cancel'); - - unless ($c->param('archive_filename')) { - $c->addbadmessage($c->maketext('The filename cannot be empty.')); - return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); - } - - unless (@files > 0) { - $c->addbadmessage($c->maketext('At least one file must be selected')); - return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); - } +# Create either a gzipped tar or zip archive. +sub CreateArchive ($c) { + my @files = $c->param('files'); + my $dir = "$c->{courseRoot}/$c->{pwd}"; + + # Save the current working directory and change to the $path directory. + my $cwd = Mojo::File->new; + chdir($dir); + unless ($c->param('archive_filename') && scalar(@files) > 0) { + $c->addbadmessage($c->maketext('The filename cannot be empty.')) unless $c->param('archive_filename'); + $c->addbadmessage($c->maketext('At least one file must be selected')) unless scalar(@files) > 0; + return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); + } - my $archive = $c->param('archive_filename'); - my ($error, $ok); - if ($c->param('archive_type') eq 'zip') { - $archive .= '.zip'; - if (my $zip = Archive::Zip::SimpleZip->new("$dir/$archive")) { - for (@files) { - $zip->add("$dir/$_", Name => $_, storelinks => 1); - } - $ok = $zip->close; - } - $error = $SimpleZipError unless $ok; - } else { - $archive .= '.tgz'; - my $tar = Archive::Tar->new; - $tar->add_files(map {"$dir/$_"} @files); - # Make file names in the archive relative to the current working directory. - for ($tar->get_files) { - $tar->rename($_->full_path, $_->full_path =~ s!^$dir/!!r); - } - $ok = $tar->write("$dir/$archive", COMPRESS_GZIP); - $error = $tar->error unless $ok; - } - if ($ok) { - $c->addgoodmessage( - $c->maketext('Archive "[_1]" created successfully ([quant,_2,file])', $archive, scalar(@files))); - } else { - $c->addbadmessage($c->maketext(q{Can't create archive "[_1]": [_2]}, $archive, $error)); - } - return $c->Refresh; + my $archive = $c->param('archive_filename'); + my ($error, $ok); + if ($c->param('archive_type') eq 'zip') { + $archive .= '.zip'; + $ok = zip \@files => $archive; + $error = $ZipError unless $ok; } else { - return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); + $archive .= '.tgz'; + $ok = Archive::Tar->create_archive($archive, COMPRESS_GZIP, @files); + $error = $Archive::Tar::error unless $ok; + } + if ($ok) { + my $n = scalar(@files); + $c->addgoodmessage($c->maketext('Archive "[_1]" created successfully ([quant,_2,file])', $archive, $n)); + } else { + $c->addbadmessage($c->maketext(q{Can't create archive "[_1]": command returned [_2]}, $archive, $error)); } + + # Change the working directory back to the original working directory. + chdir($cwd); + return $c->Refresh; } # Unpack a gzipped tar archive diff --git a/templates/ContentGenerator/Instructor/FileManager.html.ep b/templates/ContentGenerator/Instructor/FileManager.html.ep index e10aa557be..fd6406c9c6 100644 --- a/templates/ContentGenerator/Instructor/FileManager.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager.html.ep @@ -44,6 +44,7 @@ % x('Make Archive') => 'MakeArchive', % x('Unpack Archive') => 'UnpackArchive', % x('Archive Course') => 'Refresh', + % x('Create Archive') => 'CreateArchive' % ); % % # Add translated action names to the method map. diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep index ba544384c9..ee5aab8c24 100644 --- a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -56,7 +56,7 @@

<%= maketext('Create the archive of the select files?') %>

<%= submit_button maketext('Cancel'), name => 'action', class => 'btn btn-sm btn-secondary' =%> - <%= submit_button maketext('Make Archive'), name => 'action', class => 'btn btn-sm btn-primary' =%> + <%= submit_button maketext('Create Archive'), name => 'action', class => 'btn btn-sm btn-primary' =%>
From c854a71288456a4e64aa7a567883d824d067bdfc Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 16 Aug 2023 16:48:13 -0500 Subject: [PATCH 07/18] Fix the broken stuff. --- .../Instructor/FileManager.pm | 32 +------------------ .../Instructor/FileManager.html.ep | 1 - .../Instructor/FileManager/archive.html.ep | 2 +- 3 files changed, 2 insertions(+), 33 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm index d7c965ed34..827899dd36 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm @@ -25,7 +25,6 @@ WeBWorK::ContentGenerator::Instructor::FileManager.pm -- simple directory manage use File::Path; use File::Copy; use File::Spec; -use Mojo::File; use String::ShellQuote; use Archive::Extract; use Archive::Tar; @@ -348,20 +347,13 @@ sub Delete ($c) { } } -# Call the make archive template. +# Make a gzipped tar or zip archive sub MakeArchive ($c) { my @files = $c->param('files'); if (scalar(@files) == 0) { $c->addbadmessage($c->maketext('You must select at least one file for the archive')); return $c->Refresh; - } else { - return $c->include( - 'ContentGenerator/Instructor/FileManager/archive', - dir => "$c->{courseRoot}/$c->{pwd}", - files => \@files - ); } -} # Create either a gzipped tar or zip archive. sub CreateArchive ($c) { @@ -376,28 +368,6 @@ sub CreateArchive ($c) { $c->addbadmessage($c->maketext('At least one file must be selected')) unless scalar(@files) > 0; return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); } - - my $archive = $c->param('archive_filename'); - my ($error, $ok); - if ($c->param('archive_type') eq 'zip') { - $archive .= '.zip'; - $ok = zip \@files => $archive; - $error = $ZipError unless $ok; - } else { - $archive .= '.tgz'; - $ok = Archive::Tar->create_archive($archive, COMPRESS_GZIP, @files); - $error = $Archive::Tar::error unless $ok; - } - if ($ok) { - my $n = scalar(@files); - $c->addgoodmessage($c->maketext('Archive "[_1]" created successfully ([quant,_2,file])', $archive, $n)); - } else { - $c->addbadmessage($c->maketext(q{Can't create archive "[_1]": command returned [_2]}, $archive, $error)); - } - - # Change the working directory back to the original working directory. - chdir($cwd); - return $c->Refresh; } # Unpack a gzipped tar archive diff --git a/templates/ContentGenerator/Instructor/FileManager.html.ep b/templates/ContentGenerator/Instructor/FileManager.html.ep index fd6406c9c6..e10aa557be 100644 --- a/templates/ContentGenerator/Instructor/FileManager.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager.html.ep @@ -44,7 +44,6 @@ % x('Make Archive') => 'MakeArchive', % x('Unpack Archive') => 'UnpackArchive', % x('Archive Course') => 'Refresh', - % x('Create Archive') => 'CreateArchive' % ); % % # Add translated action names to the method map. diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep index ee5aab8c24..ba544384c9 100644 --- a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -56,7 +56,7 @@

<%= maketext('Create the archive of the select files?') %>

<%= submit_button maketext('Cancel'), name => 'action', class => 'btn btn-sm btn-secondary' =%> - <%= submit_button maketext('Create Archive'), name => 'action', class => 'btn btn-sm btn-primary' =%> + <%= submit_button maketext('Make Archive'), name => 'action', class => 'btn btn-sm btn-primary' =%>
From 9b38a068e2e2760a0f328b121c55e2af2282b628 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Thu, 24 Aug 2023 14:42:28 -0400 Subject: [PATCH 08/18] Tweaks to the file manager make archive page. If the chose archvie file exists, then don't blindly overwrite it. Add a checkbox to that allows the user to select to overwrite the file. Only replace the existing archive file if that checkbox is checked. Otherwise show a message that the file exists, and don't overwrite. If a single file is selected (or single directory initially selected) then use that filename (without extension) for a default for the archive name. --- .../Instructor/FileManager.pm | 64 +++++++++++++++---- .../Instructor/FileManager/archive.html.ep | 18 +++++- 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm index 827899dd36..5c054d33de 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm @@ -355,17 +355,59 @@ sub MakeArchive ($c) { return $c->Refresh; } -# Create either a gzipped tar or zip archive. -sub CreateArchive ($c) { - my @files = $c->param('files'); - my $dir = "$c->{courseRoot}/$c->{pwd}"; - - # Save the current working directory and change to the $path directory. - my $cwd = Mojo::File->new; - chdir($dir); - unless ($c->param('archive_filename') && scalar(@files) > 0) { - $c->addbadmessage($c->maketext('The filename cannot be empty.')) unless $c->param('archive_filename'); - $c->addbadmessage($c->maketext('At least one file must be selected')) unless scalar(@files) > 0; + my $dir = $c->{pwd} eq '.' ? $c->{courseRoot} : "$c->{courseRoot}/$c->{pwd}"; + + if ($c->param('confirmed')) { + my $action = $c->param('action') || 'Cancel'; + return $c->Refresh if $action eq 'Cancel' || $action eq $c->maketext('Cancel'); + + unless ($c->param('archive_filename')) { + $c->addbadmessage($c->maketext('The filename cannot be empty.')); + return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); + } + + unless (@files > 0) { + $c->addbadmessage($c->maketext('At least one file must be selected')); + return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); + } + + my $archive = $c->param('archive_filename') . '.' . $c->param('archive_type'); + + if (-e "$dir/$archive" && !$c->param('overwrite')) { + $c->addbadmessage($c->maketext( + 'The file [_1] exists. Check "Overwrite existing archive" to force this file to be replaced.', + $archive + )); + return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); + } + + my ($error, $ok); + if ($c->param('archive_type') eq 'zip') { + if (my $zip = Archive::Zip::SimpleZip->new("$dir/$archive")) { + for (@files) { + $zip->add("$dir/$_", Name => $_, storelinks => 1); + } + $ok = $zip->close; + } + $error = $SimpleZipError unless $ok; + } else { + my $tar = Archive::Tar->new; + $tar->add_files(map {"$dir/$_"} @files); + # Make file names in the archive relative to the current working directory. + for ($tar->get_files) { + $tar->rename($_->full_path, $_->full_path =~ s!^$dir/!!r); + } + $ok = $tar->write("$dir/$archive", COMPRESS_GZIP); + $error = $tar->error unless $ok; + } + if ($ok) { + $c->addgoodmessage( + $c->maketext('Archive "[_1]" created successfully ([quant,_2,file])', $archive, scalar(@files))); + } else { + $c->addbadmessage($c->maketext(q{Can't create archive "[_1]": [_2]}, $archive, $error)); + } + return $c->Refresh; + } else { return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); } } diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep index ba544384c9..2fdf389ee5 100644 --- a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -1,6 +1,6 @@ % use Mojo::File qw(path); % -
+
<%= maketext('The following files have been selected for archiving. Select the type ' @@ -10,7 +10,7 @@
<%= maketext('Archive Filename') %>: - <%= text_field archive_filename => '', + <%= text_field archive_filename => @$files == 1 ? $files->[0] =~ s/\..*$//r : '', 'aria-labelledby' => 'archive_filename', placeholder => maketext('Archive Filename'), class => 'form-control', size => 30 =%> .zip @@ -32,6 +32,18 @@
+
+
+
+
+ +
+
+
+
-
-
% % my @files_to_compress; % for my $file (@$files) { diff --git a/templates/HelpFiles/InstructorFileManager.html.ep b/templates/HelpFiles/InstructorFileManager.html.ep index bf2ecb0fb5..a99ad49b76 100644 --- a/templates/HelpFiles/InstructorFileManager.html.ep +++ b/templates/HelpFiles/InstructorFileManager.html.ep @@ -20,8 +20,8 @@ <%= maketext('This allows for the viewing, downloading, uploading and other management ' . 'of files in the course. Select a file or set of files (using CTRL or SHIFT) and click ' . 'the desired button on the right. Many actions can only be done with a single file (like ' - . 'view). Selecting a directory or set of files and clicking "Make Archive" creates a compressed ' - . 'tar file with the name COURSE_NAME.tgz' ) =%> + . 'view). Selecting a directory or set of files and clicking "Make Archive" allows the creation ' + . 'of a compressed tar or zip file.') =%>

<%= maketext('The list of files include regular files, directories (ending in a "/") ' From 3e4a523d0e21b65ebc79c7efd743f7e87bfc4a9c Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Fri, 25 Aug 2023 13:52:52 -0400 Subject: [PATCH 10/18] Switch archive type from radio button to select --- .../Instructor/FileManager/archive.html.ep | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep index 5e2ac8cc49..2f95188b22 100644 --- a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -13,22 +13,7 @@ <%= text_field archive_filename => @$files == 1 ? $files->[0] =~ s/\..*$//r : 'webwork_files', 'aria-labelledby' => 'archive_filename', placeholder => maketext('Archive Filename'), class => 'form-control', size => 30 =%> - .zip -

-
-
-
-
- <%= maketext('Archive Type') %>: -
- - + <%= select_field archive_type => ['zip', 'tgz'], class => 'form-select' =%>
From 8e302d952e0fd8378cb678b7903b1933c84d7a63 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Sun, 27 Aug 2023 20:25:10 -0400 Subject: [PATCH 11/18] Some accessibility changes to the archive type select --- htdocs/js/FileManager/filemanager.js | 8 -------- .../Instructor/FileManager/archive.html.ep | 17 +++++++++++------ 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/htdocs/js/FileManager/filemanager.js b/htdocs/js/FileManager/filemanager.js index e85e7da509..5a55c529e4 100644 --- a/htdocs/js/FileManager/filemanager.js +++ b/htdocs/js/FileManager/filemanager.js @@ -42,14 +42,6 @@ } }; - for (const archiveTypeInput of document.querySelectorAll('input[name="archive_type"]')) { - archiveTypeInput.addEventListener('click', () => { - document.getElementById('filename_suffix').innerText = `.${ - document.querySelector('input[name="archive_type"]:checked').value - }`; - }); - } - files?.addEventListener('change', checkFiles); if (files) checkFiles(); diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep index 2f95188b22..76059dc5c2 100644 --- a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -2,18 +2,23 @@ %
-
+
<%= maketext('The following files have been selected for archiving. Select the type ' . 'of archive and any subset of the requested files.') =%>
- <%= maketext('Archive Filename') %>: + <%= text_field archive_filename => @$files == 1 ? $files->[0] =~ s/\..*$//r : 'webwork_files', - 'aria-labelledby' => 'archive_filename', placeholder => maketext('Archive Filename'), + id => 'archive-filename', placeholder => maketext('Archive Filename'), class => 'form-control', size => 30 =%> - <%= select_field archive_type => ['zip', 'tgz'], class => 'form-select' =%> + <%= select_field archive_type => [ + [ 'File Type' => '', disabled => undef, id => 'archive-type-label' ], + [ '.zip' => 'zip', selected => undef ], + [ '.tgz' => 'tgz' ] + ], + class => 'form-select', style => 'max-width: 7em', 'aria-labelledby' => 'archive-type-label' =%>
@@ -41,9 +46,9 @@ % # Select all files initially. Even those that are in previously selected directories or subdirectories. % param('files', \@files_to_compress) unless param('confirmed'); <%= select_field files => \@files_to_compress, id => 'archive-files', class => 'form-select mb-2', - size => 20, multiple => undef =%> + 'arialabelled-by' => 'files-label', size => 20, multiple => undef =%> % -

<%= maketext('Create the archive of the select files?') %>

+

<%= maketext('Create archive of the selected files?') %>

<%= submit_button maketext('Cancel'), name => 'action', class => 'btn btn-sm btn-secondary' =%> <%= submit_button maketext('Make Archive'), name => 'action', class => 'btn btn-sm btn-primary' =%> From 43b82fea60dbb0f070e70f67438ff7d0b574d501 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Mon, 28 Aug 2023 15:45:07 -0400 Subject: [PATCH 12/18] Add text-end to archive file name input, File Type -> Archive Type and wrap in maketext --- .../ContentGenerator/Instructor/FileManager/archive.html.ep | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep index 76059dc5c2..b9ac245c13 100644 --- a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -12,9 +12,9 @@ <%= text_field archive_filename => @$files == 1 ? $files->[0] =~ s/\..*$//r : 'webwork_files', id => 'archive-filename', placeholder => maketext('Archive Filename'), - class => 'form-control', size => 30 =%> + class => 'form-control text-end', size => 30 =%> <%= select_field archive_type => [ - [ 'File Type' => '', disabled => undef, id => 'archive-type-label' ], + [ maketext('Archive Type') => '', disabled => undef, id => 'archive-type-label' ], [ '.zip' => 'zip', selected => undef ], [ '.tgz' => 'tgz' ] ], From 3dd549601821cd8330055385f7af191e8bad3901 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Mon, 28 Aug 2023 16:55:54 -0500 Subject: [PATCH 13/18] Change the way that the archive type is determined. This adds a "By extension" option to the archive type dropdown which is the default option. If that option is selected, then the archive type will be determined by the extension given in the file name. If the file name does not have a valid archive extension, then the zip archive type is assumed and that extension is added. If zip or tar is selected then the zip or tgz extension is added to the filename (if it doesn't already have that extension), and of course that archive type is used. Note that describes the server side behavior. Javascript also changes the extension in the input client side. Note that the `.tar.gz` extension is also supported if explicitly given for the filename extension. Note that at or above the large breakpoint the archive type dropdown and "Overwrite existing archive" checkbox will be on the second line together. Below the large breakpoint, they will be on separate lines. Also `dir="ltr"` is added to both the archive filename input and the files select. Also remove the "Create archive of the selected files?" question. The presented answers are not valid for that question, and the question isn't needed with the instructions at the top, as well as the fact that the buttons are clear as to what they will do. --- htdocs/js/FileManager/filemanager.js | 13 +++++++++ .../Instructor/FileManager.pm | 15 +++++++++-- .../Instructor/FileManager/archive.html.ep | 27 +++++++++++-------- 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/htdocs/js/FileManager/filemanager.js b/htdocs/js/FileManager/filemanager.js index 5a55c529e4..02c3a50aa8 100644 --- a/htdocs/js/FileManager/filemanager.js +++ b/htdocs/js/FileManager/filemanager.js @@ -45,6 +45,19 @@ files?.addEventListener('change', checkFiles); if (files) checkFiles(); + const archiveFilenameInput = document.getElementById('archive-filename'); + const archiveTypeSelect = document.getElementById('archive-type'); + if (archiveFilenameInput && archiveTypeSelect) { + archiveTypeSelect.addEventListener('change', () => { + if (archiveTypeSelect.value) { + archiveFilenameInput.value = archiveFilenameInput.value.replace( + /\.(zip|tgz|tar.gz)$/, + `.${archiveTypeSelect.value}` + ); + } + }); + } + const file = document.getElementById('file'); const uploadButton = document.getElementById('Upload'); const checkFile = () => (uploadButton.disabled = file.value === ''); diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm index 5c054d33de..80efa6d973 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm @@ -371,7 +371,18 @@ sub MakeArchive ($c) { return $c->include('ContentGenerator/Instructor/FileManager/archive', dir => $dir, files => \@files); } - my $archive = $c->param('archive_filename') . '.' . $c->param('archive_type'); + my $archive_type = + $c->param('archive_type') || ($c->param('archive_filename') =~ /\.(zip|tgz|tar.gz)$/ ? $1 : 'zip'); + + my $archive = $c->param('archive_filename'); + + # Add the correct extension to the archive filename unless it already has it. If the extension for + # the other archive type is given, then change it to the extension for this archive type. + if ($archive_type eq 'zip') { + $archive =~ s/(\.(tgz|tar.gz))?$/.zip/ unless $archive =~ /\.zip$/; + } else { + $archive =~ s/(\.zip)?$/.tgz/ unless $archive =~ /\.(tgz|tar.gz)$/; + } if (-e "$dir/$archive" && !$c->param('overwrite')) { $c->addbadmessage($c->maketext( @@ -382,7 +393,7 @@ sub MakeArchive ($c) { } my ($error, $ok); - if ($c->param('archive_type') eq 'zip') { + if ($archive_type eq 'zip') { if (my $zip = Archive::Zip::SimpleZip->new("$dir/$archive")) { for (@files) { $zip->add("$dir/$_", Name => $_, storelinks => 1); diff --git a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep index b9ac245c13..fa4f637ac0 100644 --- a/templates/ContentGenerator/Instructor/FileManager/archive.html.ep +++ b/templates/ContentGenerator/Instructor/FileManager/archive.html.ep @@ -10,20 +10,26 @@
- <%= text_field archive_filename => @$files == 1 ? $files->[0] =~ s/\..*$//r : 'webwork_files', + <%= text_field archive_filename => + @$files == 1 ? $files->[0] =~ s/\..*$/.zip/r : 'webwork_files.zip', id => 'archive-filename', placeholder => maketext('Archive Filename'), - class => 'form-control text-end', size => 30 =%> - <%= select_field archive_type => [ - [ maketext('Archive Type') => '', disabled => undef, id => 'archive-type-label' ], - [ '.zip' => 'zip', selected => undef ], - [ '.tgz' => 'tgz' ] - ], - class => 'form-select', style => 'max-width: 7em', 'aria-labelledby' => 'archive-type-label' =%> + class => 'form-control', size => 30, dir => 'ltr' =%>
-
+
+
+ + <%= select_field archive_type => [ + [ maketext('By extension') => '', selected => undef ], + [ 'zip' => 'zip' ], + [ 'tar' => 'tgz' ] + ], + class => 'form-select', id => 'archive-type' =%> +
+
+