Skip to content

Commit

Permalink
add setting for short names to workaround Windows problem
Browse files Browse the repository at this point in the history
  • Loading branch information
PhilippImhof committed Aug 8, 2024
1 parent 41b10c4 commit 07878b9
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 2 deletions.
8 changes: 8 additions & 0 deletions essaydownload_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,13 @@ protected function standard_preference_fields(MoodleQuickForm $mform) {
);
$mform->addElement('advcheckbox', 'responsetext', get_string('includeresponsetext', 'quiz_essaydownload'));
$mform->addElement('advcheckbox', 'attachments', get_string('includeattachments', 'quiz_essaydownload'));

$mform->addElement(
'advcheckbox',
'shortennames',
get_string('additionalsettings', 'quiz_essaydownload'),
get_string('shortennames', 'quiz_essaydownload')
);
$mform->addHelpButton('shortennames', 'shortennames', 'quiz_essaydownload');
}
}
8 changes: 7 additions & 1 deletion essaydownload_options.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ class quiz_essaydownload_options extends quiz_essaydownload_options_parent_class
/** @var bool whether to include attachments (if there are) in the archive */
public $attachments = true;

/** @var bool how to organise the sub folders in the archive (by question or by attempt) */
/** @var bool whether to shorten file and path names to workaround a Windows issue */
public $shortennames = false;

/** @var string how to organise the sub folders in the archive (by question or by attempt) */
public $groupby = 'byattempt';

/**
Expand All @@ -81,6 +84,7 @@ public function get_initial_form_data() {
$toform->responsetext = $this->responsetext;
$toform->questiontext = $this->questiontext;
$toform->attachments = $this->attachments;
$toform->shortennames = $this->shortennames;
$toform->groupby = $this->groupby;

return $toform;
Expand All @@ -95,6 +99,7 @@ public function setup_from_form_data($fromform): void {
$this->responsetext = $fromform->responsetext;
$this->questiontext = $fromform->questiontext;
$this->attachments = $fromform->attachments;
$this->shortennames = $fromform->shortennames;
$this->groupby = $fromform->groupby;
}

Expand All @@ -105,6 +110,7 @@ public function setup_from_params() {
$this->responsetext = optional_param('responsetext', $this->responsetext, PARAM_BOOL);
$this->questiontext = optional_param('questiontext', $this->questiontext, PARAM_BOOL);
$this->attachments = optional_param('attachments', $this->attachments, PARAM_BOOL);
$this->shortennames = optional_param('shortennames', $this->shortennames, PARAM_BOOL);
$this->groupby = optional_param('groupby', $this->groupby, PARAM_ALPHA);
}

Expand Down
3 changes: 3 additions & 0 deletions lang/en/quiz_essaydownload.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/

$string['additionalsettings'] = 'Additional settings';
$string['byattempt'] = 'Attempt';
$string['byquestion'] = 'Question';
$string['errorfilename'] = 'error-{$a}.txt';
Expand All @@ -39,4 +40,6 @@
$string['plugindescription'] = 'Download text answers and attachment files submitted in response to essay questions in a quiz.';
$string['pluginname'] = 'Essay responses downloader plugin (quiz_essaydownload)';
$string['privacy:metadata'] = 'The quiz essay download plugin does not store any personal data about any user.';
$string['shortennames'] = 'Shorten archive name and subfolder names';
$string['shortennames_help'] = 'If the total path name of an extracted file is larger than 260 characters, this may cause problems with Windows\' built-in extraction tool. In this case, activating this checkbox may help. It might, however, make it more difficult to identify your students, if they have very long names.';
$string['whattoinclude'] = 'What to include';
15 changes: 14 additions & 1 deletion report.php
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,13 @@ public function get_attempts_and_names(sql_join $joins): array {

$attempts = [];
foreach ($results as $result) {
// If the user has requested short filenames, we limit the last and first name to 40
// characters each.
if ($this->options->shortennames) {
$result->lastname = substr($result->lastname, 0, 40);
$result->firstname = substr($result->firstname, 0, 40);
}

// Build the path for this attempt: <name>_<attemptid>_<date/time finished>.
$path = $result->lastname . '_' . $result->firstname . '_' . $result->attemptid;
$path = $path . '_' . date('Ymd_His', $result->timefinish);
Expand Down Expand Up @@ -299,10 +306,16 @@ public function get_details_for_attempt(int $attemptid): array {
* @return void
*/
protected function process_and_download(): void {
$quizname = $this->cm->name;
// If the user requests shorter file names, we will make sure the quiz' name is not more than
// 15 characters.
if ($this->options->shortennames) {
$quizname = substr($quizname, 0, 15);
}
// The archive's name will be <short name of course> - <quiz name> - <cmid for the quiz>.zip.
// This makes sure that the name will be unique per quiz, even if two quizzes have the same
// title. Also, we will replace spaces by underscores.
$filename = $this->course->shortname . ' - ' . $this->cm->name . ' - ' . $this->cm->id . '.zip';
$filename = $this->course->shortname . ' - ' . $quizname . ' - ' . $this->cm->id . '.zip';
$filename = self::clean_filename($filename);

// The ZIP will be created on the fly via the stream writer.
Expand Down
63 changes: 63 additions & 0 deletions tests/report_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

namespace quiz_essaydownload;

use quiz_essaydownload_options;
use quiz_essaydownload_report;

defined('MOODLE_INTERNAL') || die();
Expand Down Expand Up @@ -111,6 +112,68 @@ public function test_quiz_has_essay_questions_when_it_is_empty(): void {
self::assertFalse($report->quiz_has_essay_questions());
}

public function test_long_names_being_shortened(): void {
$this->resetAfterTest();

// Create a course and a quiz with an essay question.
$generator = $this->getDataGenerator();
$questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
$course = $generator->create_course();
$quiz = $this->create_test_quiz($course);
$quiz->name = 'ThisQuizHasAnExtremelyLongTitleBecauseLongTitlesAreJustSoCoolToHave';
quiz_essaydownload_test_helper::add_essay_question($questiongenerator, $quiz);

// Add a student with a very long name and create an attempt.
$student = \phpunit_util::get_data_generator()->create_user(
[
'firstname' => 'ExtremelyLongFirstNameForThisVerySpecificPerson',
'lastname' => 'OneThingIsSureThisLastNameIsNotGoingToEndVerySoon'
]
);
\phpunit_util::get_data_generator()->enrol_user($student->id, $course->id, 'student');
$attempt = $this->attempt_quiz($quiz, $student);

$cm = get_coursemodule_from_id('quiz', $quiz->cmid);
$report = new quiz_essaydownload_report();

list($currentgroup, $allstudentjoins, $groupstudentjoins, $allowedjoins) =
$report->init('essaydownload', 'quiz_essaydownload_form', $quiz, $cm, $course);

// Use reflection to force shortening of names.
$reflectedreport = new \ReflectionClass($report);
$reflectedoptions = $reflectedreport->getProperty('options');
$reflectedoptions->setAccessible(true);
$options = new quiz_essaydownload_options('essaydownload', $quiz, $cm, $course);
$options->shortennames = true;
$reflectedoptions->setValue($report, $options);

// Fetch the attemps using the report's API.
$fetchedattempts = $report->get_attempts_and_names($groupstudentjoins);

// There should be exactly one attempt.
self::assertCount(1, $fetchedattempts);

$i = 0;
foreach ($fetchedattempts as $fetchedid => $fetchedname) {
// The attempt is stored in a somewhat obscure way.
$attemptobj = $attempt[2]->get_attempt();

$id = $attemptobj->id;
self::assertEquals($id, $fetchedid);

$firstname = clean_filename(str_replace(' ', '_', substr($student->firstname, 0, 40)));
$lastname = clean_filename(str_replace(' ', '_', substr($student->lastname, 0, 40)));

$name = $lastname . '_' . $firstname . '_' . $id . '_' . date('Ymd_His', $attemptobj->timefinish);

// We will not compare the minutes and seconds, because there might be a small difference and
// we don't really care. If the timestamp is correct up to the hours, we can safely assume the
// conversion worked.
self::assertStringStartsWith(substr($name, 0, -4), $fetchedname);
$i++;
}
}

public function test_get_attempts_and_names_without_groups(): void {
$this->resetAfterTest();

Expand Down

0 comments on commit 07878b9

Please sign in to comment.