moodle/moodle
3
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-08-05 17:06:53 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

MDL-69112 assign: Improve parsing of uploaded feedback names

Browse source
This commit is contained in:
Eric Merrill 2020-07-31 19:06:49 -04:00
parent 07238ca511
commit 6ff0525a3f
2 changed files with 192 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

68
mod/assign/feedback/file/importziplib.php
View file

@ -59,33 +59,53 @@ class assignfeedback_file_zip_importer {
return false; return false;
} }
$info = explode('_', $fileinfo->get_filepath() . $fileinfo->get_filename(), 5); // Break the full path-name into path parts.
$pathparts = explode('/', $fileinfo->get_filepath() . $fileinfo->get_filename());
if (count($info) < 5) { while (!empty($pathparts)) {
return false; // Get the next path part and break it up by underscores.
$pathpart = array_shift($pathparts);
$info = explode('_', $pathpart, 5);
if (count($info) < 5) {
continue;
}
// Check the participant id.
$participantid = $info[1];
if (!is_numeric($participantid)) {
continue;
}
// Convert to int.
$participantid += 0;
if (empty($participants[$participantid])) {
continue;
}
// Set user, which is by reference, so is used by the calling script.
$user = $participants[$participantid];
// Set the plugin. This by reference, and is used by the calling script.
$plugin = $assignment->get_plugin_by_type($info[2], $info[3]);
if (!$plugin) {
continue;
}
// Take any remaining text in this part and put it back in the path parts array.
array_unshift($pathparts, $info[4]);
// Combine the remaining parts and set it as the filename.
// Note that filename is a 'by reference' variable, so we need to set it before returning.
$filename = implode('/', $pathparts);
return true;
} }
$participantid = $info[1]; return false;
$filename = $info[4];
$plugin = $assignment->get_plugin_by_type($info[2], $info[3]);
if (!is_numeric($participantid)) {
return false;
}
if (!$plugin) {
return false;
}
// Convert to int.
$participantid += 0;
if (empty($participants[$participantid])) {
return false;
}
$user = $participants[$participantid];
return true;
} }
/** /**

148
mod/assign/feedback/file/tests/importziplib_test.php Normal file
View file

@ -0,0 +1,148 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
/**
* Unit tests for importziplib.
*
* @package assignfeedback_file
* @copyright 2020 Eric Merrill <merrill@oakland.edu>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->dirroot . '/mod/assign/tests/generator.php');
require_once($CFG->dirroot . '/mod/assign/feedback/file/importziplib.php');
/**
* Unit tests for importziplib.
*
* @copyright 2020 Eric Merrill <merrill@oakland.edu>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class assignfeedback_importziplib_testcase extends advanced_testcase {
// Use the generator helper.
use mod_assign_test_generator;
/**
* Test the assignfeedback_file_zip_importer->is_valid_filename_for_import() method.
*/
public function test_is_valid_filename_for_import() {
// Do the initial assign setup.
$this->resetAfterTest();
$course = $this->getDataGenerator()->create_course();
$teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher');
$student = $this->getDataGenerator()->create_and_enrol($course, 'student');
$assign = $this->create_instance($course, [
'assignsubmission_onlinetext_enabled' => 1,
'assignfeedback_file_enabled' => 1,
]);
// Create an online text submission.
$this->add_submission($student, $assign);
// Now onto the file work.
$fs = get_file_storage();
// Setup a basic file we will work with. We will keep renaming and repathing it.
$record = new stdClass;
$record->contextid = $assign->get_context()->id;
$record->component = 'assignfeedback_file';
$record->filearea = ASSIGNFEEDBACK_FILE_FILEAREA;
$record->itemid = $assign->get_user_grade($student->id, true)->id;
$record->filepath = '/';
$record->filename = '1.txt';
$record->source = 'test';
$file = $fs->create_file_from_string($record, 'file content');
// The importer we will use.
$importer = new assignfeedback_file_zip_importer();
// Setup some variable we use.
$user = null;
$plugin = null;
$filename = '';
$allusers = $assign->list_participants(0, false);
$participants = array();
foreach ($allusers as $user) {
$participants[$assign->get_uniqueid_for_user($user->id)] = $user;
}
$file->rename('/import/', '.hiddenfile');
$result = $importer->is_valid_filename_for_import($assign, $file, $participants, $user, $plugin, $filename);
$this->assertFalse($result);
$file->rename('/import/', '~hiddenfile');
$result = $importer->is_valid_filename_for_import($assign, $file, $participants, $user, $plugin, $filename);
$this->assertFalse($result);
$file->rename('/import/some_path_here/', 'RandomFile.txt');
$result = $importer->is_valid_filename_for_import($assign, $file, $participants, $user, $plugin, $filename);
$this->assertFalse($result);
$file->rename('/import/', '~hiddenfile');
$result = $importer->is_valid_filename_for_import($assign, $file, $participants, $user, $plugin, $filename);
$this->assertFalse($result);
// Get the students assign id.
$studentid = $assign->get_uniqueid_for_user($student->id);
// Submissions are identified with the format:
// StudentName_StudentID_PluginType_Plugin_FilePathAndName.
// Test a string student id.
$badname = 'Student Name_StringID_assignsubmission_file_My_cool_filename.txt';
$file->rename('/import/', $badname);
$result = $importer->is_valid_filename_for_import($assign, $file, $participants, $user, $plugin, $filename);
$this->assertFalse($result);
// Test an invalid student id.
$badname = 'Student Name_' . ($studentid + 100) . '_assignsubmission_file_My_cool_filename.txt';
$file->rename('/import/', $badname);
$result = $importer->is_valid_filename_for_import($assign, $file, $participants, $user, $plugin, $filename);
$this->assertFalse($result);
// Test an invalid submission plugin.
$badname = 'Student Name_' . $studentid . '_assignsubmission_noplugin_My_cool_filename.txt';
$file->rename('/import/', $badname);
$result = $importer->is_valid_filename_for_import($assign, $file, $participants, $user, $plugin, $filename);
$this->assertFalse($result);
// Test a basic, good file.
$goodbase = 'Student Name_' . $studentid . '_assignsubmission_file_';
$file->rename('/import/', $goodbase . "My_cool_filename.txt");
$result = $importer->is_valid_filename_for_import($assign, $file, $participants, $user, $plugin, $filename);
$this->assertTrue($result);
$this->assertEquals($participants[$studentid], $user);
$this->assertEquals('My_cool_filename.txt', $filename);
$this->assertInstanceOf(assign_submission_file::class, $plugin);
// Test another good file, with some additional path and underscores.
$user = null;
$plugin = null;
$filename = '';
$file->rename('/import/some_path_here/' . $goodbase . '/some_path/', 'My File.txt');
$result = $importer->is_valid_filename_for_import($assign, $file, $participants, $user, $plugin, $filename);
$this->assertTrue($result);
$this->assertEquals($participants[$studentid], $user);
$this->assertEquals('/some_path/My File.txt', $filename);
$this->assertInstanceOf(assign_submission_file::class, $plugin);
}
}