Skip to content

Commit

Permalink
Fixes and improvements for the new "Make Archive" page.
Browse files Browse the repository at this point in the history 
Fix the action.  There needs to be a hidden "confirm" input.

Improve layouts.

Also other clean up suggested in my review of openwebwork#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.
  • Loading branch information
@drgrice1
drgrice1 committed Aug 16, 2023
1 parent 925037a commit cdba313
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 51 deletions.
18 changes: 8 additions & 10 deletions htdocs/js/FileManager/filemanager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}`;
});
}

Expand Down
54 changes: 40 additions & 14 deletions lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -356,33 +356,59 @@ 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);
}

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;
my $zip = Archive::Zip->new();
$zip->storeSymbolicLink(1);
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 {
Expand Down
52 changes: 27 additions & 25 deletions templates/ContentGenerator/Instructor/FileManager/archive.html.ep
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
% # template for the archive subpage.
<div class="card w-75 mx-auto">
% use Mojo::File qw(path);
%
<div class="card mx-auto" style="max-width:700px">
<div class="card-body">
<div class="mb-3">
<b><%= maketext('The following files have been selected for archiving. Select the type '
. 'of archive and any subset of the requested files.') =%> </b>
. 'of archive and any subset of the requested files.') =%></b>
</div>
<div class="row">
<div class="col-4">
<div class="col-12">
<div class="input-group input-group-sm mb-2">
<span class="input-group-text"><%= maketext('Archive Filename') %>:</span>
<%= text_field archive_filename => '',
Expand All @@ -20,43 +21,44 @@
<div class="input-group input-group-sm mb-2">
<span class="input-group-text"><%= maketext('Archive Type') %>:</span>
<div class="input-group-text flex-grow-1">
<label class="form-check-label me-4">
<label class="form-check-label me-2">
<%= radio_button archive_type => 'zip', checked => undef, class => 'form-check-input mx-1' =%>
<%= maketext('Zip File') =%>
</label>
<label class="form-check-label me-4">
<label class="form-check-label">
<%= radio_button archive_type => 'tgz', class => 'form-check-input mx-1' =%>
<%= maketext('Compressed Tar') =%>
</label>
<button type="button" class="btn btn-sm btn-secondary" id="select-all-files-button">
<%= maketext('Select All Files') =%>
</button>
</div>
</div>
</div>


<div class="row mb-2">
<div class="col-12">
<button type="button" class="btn btn-sm btn-secondary" id="select-all-files-button">
<%= maketext('Select All Files') =%>
</button>
</div>
</div>
%
% 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 =%>
%
<p><%= maketext('Create the archive of the select files?') %></p>
<div class="d-flex justify-content-evenly">
<%= 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' =%>
</div>
</div>
</div>
<%= hidden_field confirmed => 'MakeArchive' =%>
<%= $c->HiddenFlags =%>
4 changes: 2 additions & 2 deletions templates/ContentGenerator/Instructor/FileManager/refresh.html.ep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
dir => 'ltr', size => 17, multiple => undef =%>
</div>
<div class="col-md-4 mb-2">
<div class="d-md-flex flex-column justify-content-evenly">
<div class="d-flex flex-md-column flex-wrap flex-md-nowrap justify-content-evenly gap-1 gap-md-0">
<%= submit_button maketext('View'), id => 'View', %button =%>
<%= submit_button maketext('Edit'), id => 'Edit', %button =%>
<%= submit_button maketext('Download'), id => 'Download', %button =%>
Expand All @@ -50,7 +50,7 @@
% unless ($c->{courseName} eq 'admin') {
<%= submit_button maketext('Archive Course'), id => 'ArchiveCourse', %button =%>
% }
<div style="height: 10px"></div>
<div class="d-none d-md-block" style="height: 10px"></div>
<%= submit_button maketext('New File'), id => 'NewFile', %button =%>
<%= submit_button maketext('New Folder'), id => 'NewFolder', %button =%>
<%= submit_button maketext('Refresh'), id => 'Refresh', %button =%>
Expand Down

0 comments on commit cdba313

Please sign in to comment.