From fc13869f5aa36d1037fc1be321fd98422c4708d2 Mon Sep 17 00:00:00 2001 From: Philipp Memmel Date: Thu, 22 Jun 2023 15:01:18 +0200 Subject: [PATCH] MDL-40084 mod_data: Rename and move entries ex-/importer classes Also: - Extend test coverage - Small bug fixes - Codestyle --- mod/data/classes/local/csv_importer.php | 40 ---- .../csv_entries_exporter.php} | 17 +- .../entries_exporter.php} | 47 ++-- .../ods_entries_exporter.php} | 12 +- .../utils.php} | 16 +- .../csv_entries_importer.php} | 19 +- .../entries_importer.php} | 19 +- mod/data/deprecatedlib.php | 136 +---------- mod/data/export.php | 6 +- mod/data/field/file/field.class.php | 11 - mod/data/field/picture/field.class.php | 11 - mod/data/import.php | 2 +- mod/data/import_form.php | 2 +- mod/data/lang/en/data.php | 2 +- ...xport_test.php => entries_export_test.php} | 128 ++++++++-- mod/data/tests/entries_exporter_test.php | 222 ++++++++++++++++++ ...mport_test.php => entries_import_test.php} | 84 +++++-- mod/data/tests/entries_importer_test.php | 205 ++++++++++++++++ mod/data/upgrade.txt | 8 +- 19 files changed, 692 insertions(+), 295 deletions(-) delete mode 100644 mod/data/classes/local/csv_importer.php rename mod/data/classes/local/{csv_exporter.php => exporter/csv_entries_exporter.php} (84%) rename mod/data/classes/local/{exporter.php => exporter/entries_exporter.php} (84%) rename mod/data/classes/local/{ods_exporter.php => exporter/ods_entries_exporter.php} (83%) rename mod/data/classes/local/{exporter_utils.php => exporter/utils.php} (93%) rename mod/data/classes/local/{mod_data_csv_importer.php => importer/csv_entries_importer.php} (94%) rename mod/data/classes/local/{importer.php => importer/entries_importer.php} (91%) rename mod/data/tests/{export_test.php => entries_export_test.php} (57%) create mode 100644 mod/data/tests/entries_exporter_test.php rename mod/data/tests/{import_test.php => entries_import_test.php} (80%) create mode 100644 mod/data/tests/entries_importer_test.php diff --git a/mod/data/classes/local/csv_importer.php b/mod/data/classes/local/csv_importer.php deleted file mode 100644 index 15ae3ae346b..00000000000 --- a/mod/data/classes/local/csv_importer.php +++ /dev/null @@ -1,40 +0,0 @@ -. - -namespace mod_data\local; - -use file_serving_exception; -use zip_archive; - -/** - * CSV importer class for importing data and - if needed - files as well from a zip archive. - * - * @package mod_data - * @copyright 2023 ISB Bayern - * @author Philipp Memmel - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class csv_importer extends importer { - - /** - * Declares the importer to use a csv file as data file. - * - * @see importer::get_import_data_file_extension() - */ - public function get_import_data_file_extension(): string { - return 'csv'; - } -} diff --git a/mod/data/classes/local/csv_exporter.php b/mod/data/classes/local/exporter/csv_entries_exporter.php similarity index 84% rename from mod/data/classes/local/csv_exporter.php rename to mod/data/classes/local/exporter/csv_entries_exporter.php index 436eb4bbd42..4159117fd04 100644 --- a/mod/data/classes/local/csv_exporter.php +++ b/mod/data/classes/local/exporter/csv_entries_exporter.php @@ -14,20 +14,20 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -namespace mod_data\local; +namespace mod_data\local\exporter; use coding_exception; use csv_export_writer; /** - * CSV exporter for mod_data. + * CSV entries exporter for mod_data. * * @package mod_data * @copyright 2023 ISB Bayern * @author Philipp Memmel * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class csv_exporter extends exporter { +class csv_entries_exporter extends entries_exporter { /** @var string[] Possible delimiter names. Only used internally to check if a valid delimiter name * has been specified. @@ -43,23 +43,26 @@ class csv_exporter extends exporter { /** * Returns the csv data exported by the csv_export_writer for further handling. * - * @see \mod_data\local\exporter::get_data_file_content() + * @see \mod_data\local\exporter\entries_exporter::get_data_file_content() */ public function get_data_file_content(): string { + global $CFG; + require_once($CFG->libdir . '/csvlib.class.php'); + return csv_export_writer::print_array($this->exportdata, $this->delimitername, '"', true); } /** - * Returns the file extension of this exporter. + * Returns the file extension of this entries exporter. * - * @see \mod_data\local\exporter::get_export_data_file_extension() + * @see \mod_data\local\exporter\entries_exporter::get_export_data_file_extension() */ public function get_export_data_file_extension(): string { return 'csv'; } /** - * Setter for the delimiter name which should be used in this csv_exporter object. + * Setter for the delimiter name which should be used in this csv_entries_exporter object. * * Calling this setter is optional, the delimiter name defaults to 'comma'. * diff --git a/mod/data/classes/local/exporter.php b/mod/data/classes/local/exporter/entries_exporter.php similarity index 84% rename from mod/data/classes/local/exporter.php rename to mod/data/classes/local/exporter/entries_exporter.php index 98abb292a9a..84ee5749957 100644 --- a/mod/data/classes/local/exporter.php +++ b/mod/data/classes/local/exporter/entries_exporter.php @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -namespace mod_data\local; +namespace mod_data\local\exporter; use file_serving_exception; use moodle_exception; @@ -28,7 +28,7 @@ use zip_archive; * @author Philipp Memmel * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -abstract class exporter { +abstract class entries_exporter { /** @var int Tracks the currently edited row of the export data file. */ private int $currentrow; @@ -46,7 +46,7 @@ abstract class exporter { private zip_archive $ziparchive; /** @var bool Tracks the state if the zip archive already has been closed. */ - private bool $ziparchiveclosed; + private bool $isziparchiveclosed; /** @var string full path of the zip archive. */ private string $zipfilepath; @@ -55,7 +55,7 @@ abstract class exporter { private array $filenamesinzip; /** - * Creates an exporter object. + * Creates an entries_exporter object. * * This object can be used to export data to different formats including files. If files are added, * everything will be bundled up in a zip archive. @@ -65,7 +65,7 @@ abstract class exporter { $this->exportdata = []; $this->exportfilename = 'Exportfile'; $this->filenamesinzip = []; - $this->ziparchiveclosed = true; + $this->isziparchiveclosed = true; } /** @@ -90,7 +90,7 @@ abstract class exporter { } /** - * Signal the exporter to finish the current row and jump to the next row. + * Signal the entries_exporter to finish the current row and jump to the next row. * * @return void */ @@ -111,7 +111,7 @@ abstract class exporter { } /** - * The exporter will prepare a data file from the rows and columns being added. + * The entries_exporter will prepare a data file from the rows and columns being added. * Overwrite this method to generate the data file as string. * * @return string the data file as a string @@ -120,9 +120,9 @@ abstract class exporter { /** * Overwrite the method to return the file extension your data file will have, for example - * return 'csv'; for a csv file exporter. + * return 'csv'; for a csv file entries_exporter. * - * @return string the file extension of the data file your exporter is using + * @return string the file extension of the data file your entries_exporter is using */ abstract protected function get_export_data_file_extension(): string; @@ -141,7 +141,7 @@ abstract class exporter { } /** - * Use this method to add a file which should be exported to the exporter. + * Use this method to add a file which should be exported to the entries_exporter. * * @param string $filename the name of the file which should be added * @param string $filecontent the content of the file as a string @@ -188,7 +188,7 @@ abstract class exporter { $this->get_data_file_content(), '/'); $this->finish_zip_archive(); - if ($this->ziparchiveclosed) { + if ($this->isziparchiveclosed) { if ($sendtouser) { send_file($this->zipfilepath, $this->exportfilename . '.zip', null, 0, false, true); return null; @@ -215,6 +215,9 @@ abstract class exporter { if (!str_ends_with($zipsubdir, '/')) { $zipsubdir .= '/'; } + if (empty($filename)) { + return false; + } return in_array($zipsubdir . $filename, $this->filenamesinzip, true); } @@ -231,13 +234,19 @@ abstract class exporter { if (!$this->file_exists($filename)) { return $filename; } - $i = 1; + $extension = pathinfo($filename, PATHINFO_EXTENSION); + $filenamewithoutextension = empty($extension) + ? $filename + : substr($filename, 0,strlen($filename) - strlen($extension) - 1); + $filenamewithoutextension = $filenamewithoutextension . '_1'; + $i = 1; + $filename = empty($extension) ? $filenamewithoutextension : $filenamewithoutextension . '.' . $extension; while ($this->file_exists($filename)) { - $extension = pathinfo($filename, PATHINFO_EXTENSION); - $filenamewithoutextension = substr($filename, 0, - strlen($filename) - strlen($extension) - 1); - $filename = $filenamewithoutextension . '_' . $i . '.' . $extension; + // In case we have already a file ending with '_XX' where XX is an ascending number, we have to + // remove '_XX' first before adding '_YY' again where YY is the successor of XX. + $filenamewithoutextension = preg_replace('/_' . $i . '$/', '_' . ($i + 1), $filenamewithoutextension); + $filename = empty($extension) ? $filenamewithoutextension : $filenamewithoutextension . '.' . $extension; $i++; } return $filename; @@ -252,7 +261,7 @@ abstract class exporter { $tmpdir = make_request_directory(); $this->zipfilepath = $tmpdir . '/' . $this->exportfilename . '.zip'; $this->ziparchive = new zip_archive(); - $this->ziparchiveclosed = !$this->ziparchive->open($this->zipfilepath); + $this->isziparchiveclosed = !$this->ziparchive->open($this->zipfilepath); } /** @@ -261,8 +270,8 @@ abstract class exporter { * @return void */ private function finish_zip_archive(): void { - if (!$this->ziparchiveclosed) { - $this->ziparchiveclosed = $this->ziparchive->close(); + if (!$this->isziparchiveclosed) { + $this->isziparchiveclosed = $this->ziparchive->close(); } } } diff --git a/mod/data/classes/local/ods_exporter.php b/mod/data/classes/local/exporter/ods_entries_exporter.php similarity index 83% rename from mod/data/classes/local/ods_exporter.php rename to mod/data/classes/local/exporter/ods_entries_exporter.php index c1b59f893b7..789201f4d61 100644 --- a/mod/data/classes/local/ods_exporter.php +++ b/mod/data/classes/local/exporter/ods_entries_exporter.php @@ -14,25 +14,25 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -namespace mod_data\local; +namespace mod_data\local\exporter; use MoodleODSWorkbook; use MoodleODSWriter; /** - * ODS exporter for mod_data. + * ODS entries exporter for mod_data. * * @package mod_data * @copyright 2023 ISB Bayern * @author Philipp Memmel * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class ods_exporter extends exporter { +class ods_entries_exporter extends entries_exporter { /** - * Returns the file extension of this exporter. + * Returns the file extension of this entries exporter. * - * @see \mod_data\local\exporter::get_export_data_file_extension() + * @see \mod_data\local\exporter\entries_exporter::get_export_data_file_extension() */ public function get_export_data_file_extension(): string { return 'ods'; @@ -41,7 +41,7 @@ class ods_exporter extends exporter { /** * Returns the ods data exported by the ODS library for further handling. * - * @see \mod_data\local\exporter::get_data_file_content() + * @see \mod_data\local\exporter\entries_exporter::get_data_file_content() */ public function get_data_file_content(): string { global $CFG; diff --git a/mod/data/classes/local/exporter_utils.php b/mod/data/classes/local/exporter/utils.php similarity index 93% rename from mod/data/classes/local/exporter_utils.php rename to mod/data/classes/local/exporter/utils.php index 97df8370104..f612c3266fc 100644 --- a/mod/data/classes/local/exporter_utils.php +++ b/mod/data/classes/local/exporter/utils.php @@ -14,13 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -namespace mod_data\local; +namespace mod_data\local\exporter; -use coding_exception; use context; use context_system; -use dml_exception; -use moodle_exception; /** * Utility class for exporting data from a mod_data instance. @@ -30,15 +27,15 @@ use moodle_exception; * @author Philipp Memmel * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class exporter_utils { +class utils { /** - * Exports the data of the mod_data instance to an exporter object which then can export it to a file format. + * Exports the data of the mod_data instance to an entries_exporter object which then can export it to a file format. * * @param int $dataid * @param array $fields * @param array $selectedfields - * @param exporter $exporter the exporter object used + * @param entries_exporter $exporter the entries_exporter object used * @param int $currentgroup group ID of the current group. This is used for * exporting data while maintaining group divisions. * @param context|null $context the context in which the operation is performed (for capability checks) @@ -48,11 +45,8 @@ class exporter_utils { * @param bool $tags whether to include tags * @param bool $includefiles whether files should be exported as well * @return void - * @throws coding_exception - * @throws dml_exception - * @throws moodle_exception */ - public static function data_exportdata(int $dataid, array $fields, array $selectedfields, exporter $exporter, + public static function data_exportdata(int $dataid, array $fields, array $selectedfields, entries_exporter $exporter, int $currentgroup = 0, context $context = null, bool $userdetails = false, bool $time = false, bool $approval = false, bool $tags = false, bool $includefiles = true): void { global $DB; diff --git a/mod/data/classes/local/mod_data_csv_importer.php b/mod/data/classes/local/importer/csv_entries_importer.php similarity index 94% rename from mod/data/classes/local/mod_data_csv_importer.php rename to mod/data/classes/local/importer/csv_entries_importer.php index 0fda284f4c3..d295b9bbf85 100644 --- a/mod/data/classes/local/mod_data_csv_importer.php +++ b/mod/data/classes/local/importer/csv_entries_importer.php @@ -14,31 +14,38 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -namespace mod_data\local; +namespace mod_data\local\importer; -use coding_exception; use context_module; use core_php_time_limit; use core_tag_tag; use core_user; use csv_import_reader; -use dml_exception; use moodle_exception; use stdClass; /** - * CSV importer class for importing data and - if needed - files as well from a zip archive. + * CSV entries_importer class for importing data and - if needed - files as well from a zip archive. * * @package mod_data * @copyright 2023 ISB Bayern * @author Philipp Memmel * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class mod_data_csv_importer extends csv_importer { +class csv_entries_importer extends entries_importer { /** @var array Log entries for successfully added records. */ private array $addedrecordsmessages = []; + /** + * Declares the entries_importer to use a csv file as data file. + * + * @see entries_importer::get_import_data_file_extension() + */ + public function get_import_data_file_extension(): string { + return 'csv'; + } + /** * Import records for a data instance from csv data. * @@ -47,8 +54,6 @@ class mod_data_csv_importer extends csv_importer { * @param string $encoding The encoding of csv data. * @param string $fielddelimiter The delimiter of the csv data. * - * @throws coding_exception - * @throws dml_exception * @throws moodle_exception */ public function import_csv(stdClass $cm, stdClass $data, string $encoding, string $fielddelimiter): void { diff --git a/mod/data/classes/local/importer.php b/mod/data/classes/local/importer/entries_importer.php similarity index 91% rename from mod/data/classes/local/importer.php rename to mod/data/classes/local/importer/entries_importer.php index b4c1501a867..0f91e5f62c7 100644 --- a/mod/data/classes/local/importer.php +++ b/mod/data/classes/local/importer/entries_importer.php @@ -14,12 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -namespace mod_data\local; +namespace mod_data\local\importer; use coding_exception; use core_php_time_limit; use file_packer; -use moodle_exception; /** * Importer class for importing data and - if needed - files as well from a zip archive. @@ -29,7 +28,7 @@ use moodle_exception; * @author Philipp Memmel * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -abstract class importer { +abstract class entries_importer { /** @var string The import file path of the file which data should be imported from. */ protected string $importfilepath; @@ -50,14 +49,14 @@ abstract class importer { private string $extracteddir; /** - * Creates an importer object. + * Creates an entries_importer object. * * This object can be used to import data from data files (like csv) and zip archives both including a data file and files to be * stored in the course module context. * * @param string $importfilepath the complete path of the import file including filename * @param string $importfilename the import file name as uploaded by the user - * @throws coding_exception + * @throws coding_exception if a wrong file type is being used */ public function __construct(string $importfilepath, string $importfilename) { $this->importfilepath = $importfilepath; @@ -71,7 +70,7 @@ abstract class importer { } /** - * Return the file extension of the import data file which is being used, for example 'csv' for a csv importer. + * Return the file extension of the import data file which is being used, for example 'csv' for a csv entries_importer. * * @return string the file extension of the export data file */ @@ -80,11 +79,11 @@ abstract class importer { /** * Returns the file content of the data file. * - * Returns the content of the file directly if the importer's file is a data file itself. If the importer's file is a zip - * archive, the content of the first found data file in the zip archive's root will be returned. + * Returns the content of the file directly if the entries_importer's file is a data file itself. + * If the entries_importer's file is a zip archive, the content of the first found data file in the + * zip archive's root will be returned. * * @return false|string the data file content as string; false, if file cannot be found/read - * @throws moodle_exception */ public function get_data_file_content(): false|string { if ($this->importfiletype !== 'zip') { @@ -109,7 +108,6 @@ abstract class importer { * @param string $filename * @param string $zipsubdir * @return false|string the file content as string, false if the file could not be found/read - * @throws moodle_exception */ public function get_file_content_from_zip(string $filename, string $zipsubdir = 'files/'): false|string { if (empty($filename)) { @@ -129,7 +127,6 @@ abstract class importer { * Extracts (if not already done and if we have a zip file to deal with) the zip file to a temporary directory. * * @return void - * @throws moodle_exception */ private function extract_zip(): void { if ($this->zipfileextracted || $this->importfiletype !== 'zip') { diff --git a/mod/data/deprecatedlib.php b/mod/data/deprecatedlib.php index 074f4b611f4..5362dca9c12 100644 --- a/mod/data/deprecatedlib.php +++ b/mod/data/deprecatedlib.php @@ -102,7 +102,7 @@ function data_export_xls($export, $dataname, $count) { } /** - * @deprecated since Moodle 4.3, exporting is now being done by \mod_data\local\csv_exporter + * @deprecated since Moodle 4.3, exporting is now being done by \mod_data\local\exporter\csv_entries_exporter * @global object * @param array $export * @param string $delimiter_name @@ -130,7 +130,7 @@ function data_export_csv($export, $delimiter_name, $database, $count, $return=fa } /** - * @deprecated since Moodle 4.3, exporting is now being done by \mod_data\local\ods_exporter + * @deprecated since Moodle 4.3, exporting is now being done by \mod_data\local\exporter\ods_entries_exporter * @global object * @param array $export * @param string $dataname @@ -168,7 +168,7 @@ function data_export_ods($export, $dataname, $count) { } /** - * @deprecated since Moodle 4.3, use \mod_data\local\exporter_utils::data_exportdata with a \mod_data\local\exporter object + * @deprecated since Moodle 4.3, use \mod_data\local\exporter\utils::data_exportdata with a \mod_data\local\exporter\entries_exporter object * @global object * @param int $dataid * @param array $fields @@ -269,7 +269,7 @@ function data_get_exportdata($dataid, $fields, $selectedfields, $currentgroup=0, } /** - * @deprecated since Moodle 4.3, importing is now being done by \mod_data\local\mod_data_csv_importer::import_csv + * @deprecated since Moodle 4.3, importing is now being done by \mod_data\local\importer\csv_importer::import_csv * Import records for a data instance from csv data. * * @param object $cm Course module of the data instance. @@ -280,129 +280,17 @@ function data_get_exportdata($dataid, $fields, $selectedfields, $currentgroup=0, * @return int Number of records added. */ function data_import_csv($cm, $data, &$csvdata, $encoding, $fielddelimiter) { - global $CFG, $DB; - debugging('Function data_import_csv has been deprecated. ' - . 'Importing is now being done by \mod_data\local\mod_data_csv_importer::import_csv.', + . 'Importing is now being done by \mod_data\local\csv_importer::import_csv.', DEBUG_DEVELOPER); - // Large files are likely to take their time and memory. Let PHP know - // that we'll take longer, and that the process should be recycled soon - // to free up memory. - core_php_time_limit::raise(); - raise_memory_limit(MEMORY_EXTRA); + // New function needs a file, not the file content, so we have to temporarily put the content into a file. + $tmpdir = make_request_directory(); + $tmpfilename = 'tmpfile.csv'; + $tmpfilepath = $tmpdir . '/tmpfile.csv'; + file_put_contents($tmpfilepath, $csvdata); - $iid = csv_import_reader::get_new_iid('moddata'); - $cir = new csv_import_reader($iid, 'moddata'); - - $context = context_module::instance($cm->id); - - $readcount = $cir->load_csv_content($csvdata, $encoding, $fielddelimiter); - $csvdata = null; // Free memory. - if (empty($readcount)) { - throw new \moodle_exception('csvfailed', 'data', "{$CFG->wwwroot}/mod/data/edit.php?d={$data->id}"); - } else { - if (!$fieldnames = $cir->get_columns()) { - throw new \moodle_exception('cannotreadtmpfile', 'error'); - } - - // Check the fieldnames are valid. - $rawfields = $DB->get_records('data_fields', array('dataid' => $data->id), '', 'name, id, type'); - $fields = array(); - $errorfield = ''; - $usernamestring = get_string('username'); - $safetoskipfields = array(get_string('user'), get_string('email'), - get_string('timeadded', 'data'), get_string('timemodified', 'data'), - get_string('approved', 'data'), get_string('tags', 'data')); - $userfieldid = null; - foreach ($fieldnames as $id => $name) { - if (!isset($rawfields[$name])) { - if ($name == $usernamestring) { - $userfieldid = $id; - } else if (!in_array($name, $safetoskipfields)) { - $errorfield .= "'$name' "; - } - } else { - // If this is the second time, a field with this name comes up, it must be a field not provided by the user... - // like the username. - if (isset($fields[$name])) { - if ($name == $usernamestring) { - $userfieldid = $id; - } - unset($fieldnames[$id]); // To ensure the user provided content fields remain in the array once flipped. - } else { - $field = $rawfields[$name]; - $filepath = "$CFG->dirroot/mod/data/field/$field->type/field.class.php"; - if (!file_exists($filepath)) { - $errorfield .= "'$name' "; - continue; - } - require_once($filepath); - $classname = 'data_field_' . $field->type; - $fields[$name] = new $classname($field, $data, $cm); - } - } - } - - if (!empty($errorfield)) { - throw new \moodle_exception('fieldnotmatched', 'data', - "{$CFG->wwwroot}/mod/data/edit.php?d={$data->id}", $errorfield); - } - - $fieldnames = array_flip($fieldnames); - - $cir->init(); - $recordsadded = 0; - while ($record = $cir->next()) { - $authorid = null; - if ($userfieldid) { - if (!($author = core_user::get_user_by_username($record[$userfieldid], 'id'))) { - $authorid = null; - } else { - $authorid = $author->id; - } - } - if ($recordid = data_add_record($data, 0, $authorid)) { // Add instance to data_record. - foreach ($fields as $field) { - $fieldid = $fieldnames[$field->field->name]; - if (isset($record[$fieldid])) { - $value = $record[$fieldid]; - } else { - $value = ''; - } - - if (method_exists($field, 'update_content_import')) { - $field->update_content_import($recordid, $value, 'field_' . $field->field->id); - } else { - $content = new stdClass(); - $content->fieldid = $field->field->id; - $content->content = $value; - $content->recordid = $recordid; - $DB->insert_record('data_content', $content); - } - } - - if (core_tag_tag::is_enabled('mod_data', 'data_records') && - isset($fieldnames[get_string('tags', 'data')])) { - $columnindex = $fieldnames[get_string('tags', 'data')]; - $rawtags = $record[$columnindex]; - $tags = explode(',', $rawtags); - foreach ($tags as $tag) { - $tag = trim($tag); - if (empty($tag)) { - continue; - } - core_tag_tag::add_item_tag('mod_data', 'data_records', $recordid, $context, $tag); - } - } - - $recordsadded++; - print get_string('added', 'moodle', $recordsadded) . ". " . get_string('entry', 'data') . " (ID $recordid)
\n"; - } - } - $cir->close(); - $cir->cleanup(true); - return $recordsadded; - } + $importer = new \mod_data\local\importer\csv_entries_importer($tmpfilepath, $tmpfilename); + $importer->import_csv($cm, $data, $encoding, $fielddelimiter); return 0; } diff --git a/mod/data/export.php b/mod/data/export.php index 8e2566a4355..1dbac6203ff 100644 --- a/mod/data/export.php +++ b/mod/data/export.php @@ -98,11 +98,11 @@ if ($mform->is_cancelled()) { $exporter = null; switch ($formdata['exporttype']) { case 'csv': - $exporter = new \mod_data\local\csv_exporter(); + $exporter = new \mod_data\local\exporter\csv_entries_exporter(); $exporter->set_delimiter_name($formdata['delimiter_name']); break; case 'ods': - $exporter = new \mod_data\local\ods_exporter(); + $exporter = new \mod_data\local\exporter\ods_entries_exporter(); break; default: throw new coding_exception('Invalid export format has been specified. ' @@ -110,7 +110,7 @@ if ($mform->is_cancelled()) { } $includefiles = !empty($formdata['includefiles']); - \mod_data\local\exporter_utils::data_exportdata($data->id, $fields, $selectedfields, $exporter, $currentgroup, $context, + \mod_data\local\exporter\utils::data_exportdata($data->id, $fields, $selectedfields, $exporter, $currentgroup, $context, $exportuser, $exporttime, $exportapproval, $tags, $includefiles); $count = $exporter->get_records_count(); $filename = clean_filename("{$data->name}-{$count}_record"); diff --git a/mod/data/field/file/field.class.php b/mod/data/field/file/field.class.php index 1df3ba99c5e..062b75a9ba1 100644 --- a/mod/data/field/file/field.class.php +++ b/mod/data/field/file/field.class.php @@ -218,15 +218,6 @@ class data_field_file extends data_field_base { $DB->update_record('data_content', $content); } - /** - * File field supports export of text. The text being exported is the filename of the stored file. - * - * @return bool true - */ - public function text_export_supported() { - return true; - } - /** * Here we export the text value of a file field which is the filename of the exported file. * @@ -262,8 +253,6 @@ class data_field_file extends data_field_base { * @param string $filecontent the content of the file to import as string * @param string $filename the filename the imported file should get * @return void - * @throws file_exception - * @throws stored_file_creation_exception */ public function import_file_value(int $contentid, string $filecontent, string $filename): void { $filerecord = [ diff --git a/mod/data/field/picture/field.class.php b/mod/data/field/picture/field.class.php index 76cf28d4bc2..57e4bc7f5a5 100644 --- a/mod/data/field/picture/field.class.php +++ b/mod/data/field/picture/field.class.php @@ -352,15 +352,6 @@ class data_field_picture extends data_field_base { } } - /** - * Picture field supports export of text. The text being exported is the filename of the stored picture. - * - * @return bool true - */ - public function text_export_supported() { - return true; - } - /** * Here we export the text value of a picture field which is the filename of the exported picture. * @@ -407,8 +398,6 @@ class data_field_picture extends data_field_base { * @param string $filecontent the content of the file to import as string * @param string $filename the filename the imported file should get * @return void - * @throws file_exception - * @throws stored_file_creation_exception */ public function import_file_value(int $contentid, string $filecontent, string $filename): void { $filerecord = [ diff --git a/mod/data/import.php b/mod/data/import.php index 7368341eb52..8adfb706ca7 100644 --- a/mod/data/import.php +++ b/mod/data/import.php @@ -93,7 +93,7 @@ if ($formdata = $form->get_data()) { throw new coding_exception('No file uploaded.'); } - $importer = new \mod_data\local\mod_data_csv_importer($uploadedfilepath, $form->get_new_filename('recordsfile')); + $importer = new \mod_data\local\importer\csv_entries_importer($uploadedfilepath, $form->get_new_filename('recordsfile')); if (!$importer->get_data_file_content()) { echo $OUTPUT->notification(get_string('errordatafilenotfound', 'data'), diff --git a/mod/data/import_form.php b/mod/data/import_form.php index 123000b5219..2c96d4075bb 100644 --- a/mod/data/import_form.php +++ b/mod/data/import_form.php @@ -14,7 +14,7 @@ class mod_data_import_form extends moodleform { $dataid = $this->_customdata['dataid']; $backtourl = $this->_customdata['backtourl']; - $mform->addElement('filepicker', 'recordsfile', get_string('csvorzipfile', 'data'), + $mform->addElement('filepicker', 'recordsfile', get_string('csvfile', 'data'), null, ['accepted_types' => ['application/zip', 'text/csv']]); $delimiters = csv_import_reader::get_delimiter_list(); diff --git a/mod/data/lang/en/data.php b/mod/data/lang/en/data.php index b9ffec132be..baa16f0e33d 100644 --- a/mod/data/lang/en/data.php +++ b/mod/data/lang/en/data.php @@ -83,7 +83,7 @@ $string['createfields'] = 'Create fields to collect different types of data.'; $string['createtemplates'] = 'Templates define the interface of your activity. Once you create fields, templates will be created automatically. Alternatively, you can use a preset, which includes fields and templates.'; $string['csstemplate'] = 'Custom CSS'; $string['csvfailed'] = 'Unable to read the raw data from the CSV file'; -$string['csvorzipfile'] = 'CSV or ZIP containing a CSV file'; +$string['csvfile'] = 'CSV or ZIP containing a CSV file'; $string['csvimport'] = 'CSV file import'; $string['csvimport_help'] = 'Entries may be imported via a plain text file with a list of field names as the first line, then the data, with one record per line.'; $string['csvwithselecteddelimiter'] = 'CSV'; diff --git a/mod/data/tests/export_test.php b/mod/data/tests/entries_export_test.php similarity index 57% rename from mod/data/tests/export_test.php rename to mod/data/tests/entries_export_test.php index ef4c172f697..36df83528d5 100644 --- a/mod/data/tests/export_test.php +++ b/mod/data/tests/entries_export_test.php @@ -16,22 +16,20 @@ namespace mod_data; -use coding_exception; use context_module; -use dml_exception; -use mod_data\local\csv_exporter; -use mod_data\local\exporter_utils; -use mod_data\local\mod_data_csv_importer; +use mod_data\local\exporter\csv_entries_exporter; +use mod_data\local\exporter\ods_entries_exporter; +use mod_data\local\exporter\utils; /** - * Unit tests for import.php. + * Unit tests for exporting entries. * * @package mod_data - * @category test - * @copyright 2019 Tobias Reischmann + * @copyright 2023 ISB Bayern + * @author Philipp Memmel * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class export_test extends \advanced_testcase { +class entries_export_test extends \advanced_testcase { /** * Get the test data. @@ -39,11 +37,11 @@ class export_test extends \advanced_testcase { * In this instance we are setting up database records to be used in the unit tests. * * @return array of test instances - * @throws coding_exception */ protected function get_test_data(): array { $this->resetAfterTest(true); + /** @var \mod_data_generator $generator */ $generator = $this->getDataGenerator()->get_plugin_generator('mod_data'); $course = $this->getDataGenerator()->create_course(); $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); @@ -91,19 +89,25 @@ class export_test extends \advanced_testcase { } /** - * Tests the exporting of the content of a mod_data instance. + * Tests the exporting of the content of a mod_data instance by using the csv_entries_exporter. * - * @covers \mod_data\local\exporter - * @covers \mod_data\local\exporter_utils::data_exportdata + * It also includes more general testing of the functionality of the entries_exporter the csv_entries_exporter + * is inheriting from. + * + * @covers \mod_data\local\exporter\entries_exporter + * @covers \mod_data\local\exporter\entries_exporter::get_records_count() + * @covers \mod_data\local\exporter\entries_exporter::send_file() + * @covers \mod_data\local\exporter\csv_entries_exporter + * @covers \mod_data\local\exporter\utils::data_exportdata */ - public function test_export(): void { + public function test_export_csv(): void { global $DB; [ 'data' => $data, 'cm' => $cm, ] = $this->get_test_data(); - $exporter = new csv_exporter(); + $exporter = new csv_entries_exporter(); $exporter->set_export_file_name('testexportfile'); $fieldrecords = $DB->get_records('data_fields', ['dataid' => $data->id], 'id'); @@ -124,16 +128,18 @@ class export_test extends \advanced_testcase { // This means file and picture fields will be exported, but only as text (which is the filename), // so we will receive a csv export file. $includefiles = false; - exporter_utils::data_exportdata($data->id, $fields, $selectedfields, $exporter, $currentgroup, $context, + utils::data_exportdata($data->id, $fields, $selectedfields, $exporter, $currentgroup, $context, $exportuser, $exporttime, $exportapproval, $tags, $includefiles); $this->assertEquals(file_get_contents(__DIR__ . '/fixtures/test_data_export_without_files.csv'), $exporter->send_file(false)); + $this->assertEquals(1, $exporter->get_records_count()); + // We now test the export including files. This will generate a zip archive. $includefiles = true; - $exporter = new csv_exporter(); + $exporter = new csv_entries_exporter(); $exporter->set_export_file_name('testexportfile'); - exporter_utils::data_exportdata($data->id, $fields, $selectedfields, $exporter, $currentgroup, $context, + utils::data_exportdata($data->id, $fields, $selectedfields, $exporter, $currentgroup, $context, $exportuser, $exporttime, $exportapproval, $tags, $includefiles); // We now write the zip archive temporary to disc to be able to parse it and assert it has the correct structure. $tmpdir = make_request_directory(); @@ -164,5 +170,91 @@ class export_test extends \advanced_testcase { fclose($filestream); } $ziparchive->close(); + unlink($tmpdir . '/testexportarchive.zip'); + } + + /** + * Tests specific ODS exporting functionality. + * + * @covers \mod_data\local\exporter\ods_entries_exporter + * @covers \mod_data\local\exporter\utils::data_exportdata + */ + public function test_export_ods(): void { + global $DB; + [ + 'data' => $data, + 'cm' => $cm, + ] = $this->get_test_data(); + + $exporter = new ods_entries_exporter(); + $exporter->set_export_file_name('testexportfile'); + $fieldrecords = $DB->get_records('data_fields', ['dataid' => $data->id], 'id'); + + $fields = []; + foreach ($fieldrecords as $fieldrecord) { + $fields[] = data_get_field($fieldrecord, $data); + } + + // We select all fields. + $selectedfields = array_map(fn($field) => $field->field->id, $fields); + $currentgroup = groups_get_activity_group($cm); + $context = context_module::instance($cm->id); + $exportuser = false; + $exporttime = false; + $exportapproval = false; + $tags = false; + // We first test the export without exporting files. + // This means file and picture fields will be exported, but only as text (which is the filename), + // so we will receive an ods export file. + $includefiles = false; + utils::data_exportdata($data->id, $fields, $selectedfields, $exporter, $currentgroup, $context, + $exportuser, $exporttime, $exportapproval, $tags, $includefiles); + $odsrows = $this->get_ods_rows_content($exporter->send_file(false)); + + // Check, if the headings match with the first row of the ods file. + $i = 0; + foreach ($fields as $field) { + $this->assertEquals($field->field->name, $odsrows[0][$i]); + $i++; + } + + // Check, if the values match with the field values. + $this->assertEquals('3', $odsrows[1][0]); + $this->assertEquals('a simple text', $odsrows[1][1]); + $this->assertEquals('samplefile.png', $odsrows[1][2]); + $this->assertEquals('samplefile.png', $odsrows[1][3]); + $this->assertEquals('picturefile.png', $odsrows[1][4]); + + // As the logic of renaming the files and building a zip archive is implemented in entries_exporter class, we do + // not need to test this for the ods_entries_exporter, because entries_export_test::test_export_csv already does this. + } + + /** + * Helper function to extract the text data as row arrays from an ODS document. + * + * @param string $content the file content + * @return array two-dimensional row/column array with the text content of the first spreadsheet + */ + private function get_ods_rows_content(string $content): array { + $file = tempnam(make_request_directory(), 'ods_'); + $filestream = fopen($file, "w"); + fwrite($filestream, $content); + $reader = new \OpenSpout\Reader\ODS\Reader(); + $reader->open($file); + /** @var \OpenSpout\Reader\ODS\Sheet[] $sheets */ + $sheets = $reader->getSheetIterator(); + $rowscellsvalues = []; + foreach ($sheets as $sheet) { + /** @var \OpenSpout\Common\Entity\Row[] $rows */ + $rows = $sheet->getRowIterator(); + foreach ($rows as $row) { + $cellvalues = []; + foreach ($row->getCells() as $cell) { + $cellvalues[] = $cell->getValue(); + } + $rowscellsvalues[] = $cellvalues; + } + } + return $rowscellsvalues; } } diff --git a/mod/data/tests/entries_exporter_test.php b/mod/data/tests/entries_exporter_test.php new file mode 100644 index 00000000000..2e30875773e --- /dev/null +++ b/mod/data/tests/entries_exporter_test.php @@ -0,0 +1,222 @@ +. + +namespace mod_data; + +use context_module; +use mod_data\local\exporter\csv_entries_exporter; +use mod_data\local\exporter\ods_entries_exporter; +use mod_data\local\exporter\utils; + +/** + * Unit tests for entries_exporter and csv_entries_exporter classes. + * + * Also {@see entries_export_test} class which provides module tests for exporting entries. + * + * @package mod_data + * @covers \mod_data\local\exporter\entries_exporter + * @covers \mod_data\local\exporter\csv_entries_exporter + * @copyright 2023 ISB Bayern + * @author Philipp Memmel + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class entries_exporter_test extends \advanced_testcase { + + /** + * Tests get_records_count method. + * + * @covers \mod_data\local\exporter\entries_exporter::get_records_count + * @dataProvider get_records_count_provider + * @param array $rows the rows from the data provider to be tested by the exporter + * @param int $expectedcount the expected count of records to be exported + */ + public function test_get_records_count(array $rows, int $expectedcount): void { + $exporter = new csv_entries_exporter(); + foreach ($rows as $row) { + $exporter->add_row($row); + } + $this->assertEquals($expectedcount, $exporter->get_records_count()); + } + + /** + * Data provider method for self::test_get_records_count. + * + * @return array data for testing + */ + public function get_records_count_provider(): array { + return [ + 'onlyheader' => [ + 'rows' => [ + ['numberfield', 'textfield', 'filefield1', 'filefield2', 'picturefield'] + ], + 'expectedcount' => 0 // Only header present, so we expect record count 0. + ], + 'onerecord' => [ + 'rows' => [ + ['numberfield', 'textfield', 'filefield1', 'filefield2', 'picturefield'], + ['3', 'a simple text', 'samplefile.png', 'samplefile_1.png', 'picturefile.png'] + ], + 'expectedcount' => 1 + ], + 'tworecords' => [ + 'rows' => [ + ['numberfield', 'textfield', 'filefield1', 'filefield2', 'picturefield'], + ['3', 'a simple text', 'samplefile.png', 'samplefile_1.png', 'picturefile.png'], + ['5', 'a supersimple text', 'anotherfile.png', 'someotherfile.png', 'andapicture.png'] + ], + 'expectedcount' => 2 + ] + ]; + } + + /** + * Tests adding of files to the exporter to be included in the exported zip archive. + * + * @dataProvider add_file_from_string_provider + * @covers \mod_data\local\exporter\entries_exporter::add_file_from_string + * @covers \mod_data\local\exporter\entries_exporter::file_exists + * @param array $files array of filename and filecontent to be tested for exporting + * @param bool $success if the exporting of files should be successful + */ + public function test_add_file_from_string(array $files, bool $success): void { + $exporter = new csv_entries_exporter(); + foreach ($files as $file) { + if (empty($file['subdir'])) { + $exporter->add_file_from_string($file['filename'], $file['filecontent']); + $this->assertEquals($exporter->file_exists($file['filename']), $success); + } else { + $exporter->add_file_from_string($file['filename'], $file['filecontent'], $file['subdir']); + $this->assertEquals($exporter->file_exists($file['filename'], $file['subdir']), $success); + } + } + } + + /** + * Data provider method for self::test_add_file_from_string. + * + * @return array data for testing + */ + public function add_file_from_string_provider(): array { + return [ + 'one file' => [ + 'files' => [ + [ + 'filename' => 'testfile.txt', + 'filecontent' => 'somecontent' + ], + ], + 'success' => true + ], + 'more files, also with subdirs' => [ + 'files' => [ + [ + 'filename' => 'testfile.txt', + 'filecontent' => 'somecontent' + ], + [ + 'filename' => 'testfile2.txt', + 'filecontent' => 'someothercontent', + 'subdir' => 'testsubdir' + ], + [ + 'filename' => 'testfile3.txt', + 'filecontent' => 'someverydifferentcontent', + 'subdir' => 'files/foo/bar' + ], + [ + 'filename' => 'testfile4.txt', + 'filecontent' => 'someverydifferentcontent', + 'subdir' => 'files/foo/bar/' + ], + [ + 'filename' => 'testfile5.txt', + 'filecontent' => 'someverydifferentcontent', + 'subdir' => '/files/foo/bar/' + ], + ], + 'success' => true + ], + 'nocontent' => [ + 'files' => [ + [ + 'filename' => '', + 'filecontent' => '' + ] + ], + 'success' => false + ] + ]; + } + + /** + * Tests if unique filenames are being created correctly. + * + * @covers \mod_data\local\exporter\entries_exporter::create_unique_filename + * @dataProvider create_unique_filename_provider + * @param string $inputfilename the name of the file which should be converted into a unique filename + * @param string $resultfilename the maybe changed $inputfilename, so that it is unique in the exporter + */ + public function test_create_unique_filename(string $inputfilename, string $resultfilename): void { + $exporter = new csv_entries_exporter(); + $exporter->add_file_from_string('test.txt', 'somecontent'); + $exporter->add_file_from_string('foo.txt', 'somecontent'); + $exporter->add_file_from_string('foo_1.txt', 'somecontent'); + $exporter->add_file_from_string('foo_2.txt', 'somecontent'); + $exporter->add_file_from_string('foo', 'somecontent'); + $exporter->add_file_from_string('foo_1', 'somecontent'); + $exporter->add_file_from_string('sample_5.txt', 'somecontent'); + $exporter->add_file_from_string('bar_1.txt', 'somecontent'); + $this->assertEquals($resultfilename, $exporter->create_unique_filename($inputfilename)); + } + + /** + * Data provider method for self::test_create_unique_filename. + * + * @return array data for testing + */ + public function create_unique_filename_provider(): array { + return [ + 'does not exist yet' => [ + 'inputfilename' => 'someuniquename.txt', + 'resultfilename' => 'someuniquename.txt' + ], + 'already exists' => [ + 'inputfilename' => 'test.txt', + 'resultfilename' => 'test_1.txt' + ], + 'already exists, other numbers as well' => [ + 'inputfilename' => 'foo.txt', + 'resultfilename' => 'foo_3.txt' + ], + 'file with _5 suffix already exists' => [ + 'inputfilename' => 'sample_5.txt', + 'resultfilename' => 'sample_5_1.txt' + ], + 'file with _1 suffix already exists' => [ + 'inputfilename' => 'bar_1.txt', + 'resultfilename' => 'bar_1_1.txt' + ], + 'file without extension unique' => [ + 'inputfilename' => 'test', + 'resultfilename' => 'test' + ], + 'file without extension not unique' => [ + 'inputfilename' => 'foo', + 'resultfilename' => 'foo_2' + ] + ]; + } +} diff --git a/mod/data/tests/import_test.php b/mod/data/tests/entries_import_test.php similarity index 80% rename from mod/data/tests/import_test.php rename to mod/data/tests/entries_import_test.php index 5bfc07625ea..66482dcbdde 100644 --- a/mod/data/tests/import_test.php +++ b/mod/data/tests/entries_import_test.php @@ -18,7 +18,7 @@ namespace mod_data; use coding_exception; use dml_exception; -use mod_data\local\mod_data_csv_importer; +use mod_data\local\importer\csv_entries_importer; use moodle_exception; use zip_archive; @@ -30,7 +30,7 @@ use zip_archive; * @copyright 2019 Tobias Reischmann * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class import_test extends \advanced_testcase { +class entries_import_test extends \advanced_testcase { /** * Set up function. @@ -102,7 +102,7 @@ class import_test extends \advanced_testcase { 'teacher' => $teacher, ] = $this->get_test_data(); - $importer = new mod_data_csv_importer(__DIR__ . '/fixtures/test_data_import.csv', + $importer = new csv_entries_importer(__DIR__ . '/fixtures/test_data_import.csv', 'test_data_import.csv'); $importer->import_csv($cm, $data, 'UTF-8', 'comma'); @@ -134,7 +134,7 @@ class import_test extends \advanced_testcase { 'student' => $student, ] = $this->get_test_data(); - $importer = new mod_data_csv_importer(__DIR__ . '/fixtures/test_data_import_with_userdata.csv', + $importer = new csv_entries_importer(__DIR__ . '/fixtures/test_data_import_with_userdata.csv', 'test_data_import_with_userdata.csv'); $importer->import_csv($cm, $data, 'UTF-8', 'comma'); @@ -175,7 +175,7 @@ class import_test extends \advanced_testcase { $fieldrecord->type = 'text'; $generator->create_field($fieldrecord, $data); - $importer = new mod_data_csv_importer(__DIR__ . '/fixtures/test_data_import_with_field_username.csv', + $importer = new csv_entries_importer(__DIR__ . '/fixtures/test_data_import_with_field_username.csv', 'test_data_import_with_field_username.csv'); $importer->import_csv($cm, $data, 'UTF-8', 'comma'); @@ -237,7 +237,7 @@ class import_test extends \advanced_testcase { $fieldrecord->type = 'text'; $generator->create_field($fieldrecord, $data); - $importer = new mod_data_csv_importer(__DIR__ . '/fixtures/test_data_import_with_userdata.csv', + $importer = new csv_entries_importer(__DIR__ . '/fixtures/test_data_import_with_userdata.csv', 'test_data_import_with_userdata.csv'); $importer->import_csv($cm, $data, 'UTF-8', 'comma'); @@ -273,12 +273,9 @@ class import_test extends \advanced_testcase { /** * Tests the import including files from a zip archive. * - * @covers \mod_data\local\importer - * @covers \mod_data\local\csv_importer + * @covers \mod_data\local\importer\entries_importer + * @covers \mod_data\local\importer\csv_entries_importer * @return void - * @throws coding_exception - * @throws moodle_exception - * @throws dml_exception */ public function test_import_with_files(): void { [ @@ -286,7 +283,7 @@ class import_test extends \advanced_testcase { 'cm' => $cm, ] = $this->get_test_data(); - $importer = new mod_data_csv_importer(__DIR__ . '/fixtures/test_data_import_with_files.zip', + $importer = new csv_entries_importer(__DIR__ . '/fixtures/test_data_import_with_files.zip', 'test_data_import_with_files.zip'); $importer->import_csv($cm, $data, 'UTF-8', 'comma'); @@ -320,19 +317,16 @@ class import_test extends \advanced_testcase { $this->assertEquals($filefield->get_file(array_keys($records)[0])->get_content(), $filefieldfilecontent); fclose($filestream); - + $this->assertCount(1, $importer->get_added_records_messages()); $ziparchive->close(); } /** * Tests the import including files from a zip archive. * - * @covers \mod_data\local\importer - * @covers \mod_data\local\csv_importer + * @covers \mod_data\local\importer\entries_importer + * @covers \mod_data\local\importer\csv_entries_importer * @return void - * @throws coding_exception - * @throws moodle_exception - * @throws dml_exception */ public function test_import_with_files_missing_file(): void { [ @@ -340,7 +334,7 @@ class import_test extends \advanced_testcase { 'cm' => $cm, ] = $this->get_test_data(); - $importer = new mod_data_csv_importer(__DIR__ . '/fixtures/test_data_import_with_files_missing_file.zip', + $importer = new csv_entries_importer(__DIR__ . '/fixtures/test_data_import_with_files_missing_file.zip', 'test_data_import_with_files_missing_file.zip'); $importer->import_csv($cm, $data, 'UTF-8', 'comma'); @@ -352,7 +346,7 @@ class import_test extends \advanced_testcase { $this->assertEquals(17, $importedcontent['ID']->content); $this->assertFalse(isset($importedcontent['filefield'])); $this->assertEquals('samplepicture.png', $importedcontent['picturefield']->content); - + $this->assertCount(1, $importer->get_added_records_messages()); $ziparchive->close(); } @@ -379,4 +373,54 @@ class import_test extends \advanced_testcase { } return $records; } + + /** + * Tests if the amount of imported records is counted properly. + * + * @covers \mod_data\local\importer\csv_entries_importer::import_csv + * @covers \mod_data\local\importer\csv_entries_importer::get_added_records_messages + * @dataProvider get_added_record_messages_provider + * @param string $datafilecontent the content of the datafile to test as string + * @param int $expectedcount the expected count of messages depending on the datafile content + */ + public function test_get_added_record_messages(string $datafilecontent, int $expectedcount): void { + [ + 'data' => $data, + 'cm' => $cm, + ] = $this->get_test_data(); + + // First we need to create the zip file from the provided data. + $tmpdir = make_request_directory(); + $datafile = $tmpdir . '/entries_import_test_datafile_tmp_' . time() . '.csv'; + file_put_contents($datafile, $datafilecontent); + + $importer = new csv_entries_importer($datafile, 'testdatafile.csv'); + $importer->import_csv($cm, $data, 'UTF-8', 'comma'); + $this->assertEquals($expectedcount, count($importer->get_added_records_messages())); + } + + /** + * Data provider method for self::test_get_added_record_messages. + * + * @return array data for testing + */ + public function get_added_record_messages_provider(): array { + return [ + 'only header' => [ + 'datafilecontent' => 'ID,Param2,filefield,picturefield' . PHP_EOL, + 'expectedcount' => 0 // One line is being assumed to be the header. + ], + 'one record' => [ + 'datafilecontent' => 'ID,Param2,filefield,picturefield' . PHP_EOL + . '5,"some short text",testfilename.pdf,testpicture.png', + 'expectedcount' => 1 + ], + 'two records' => [ + 'datafilecontent' => 'ID,Param2,filefield,picturefield' . PHP_EOL + . '5,"some short text",testfilename.pdf,testpicture.png' . PHP_EOL + . '3,"other text",testfilename2.pdf,testpicture2.png', + 'expectedcount' => 2 + ], + ]; + } } diff --git a/mod/data/tests/entries_importer_test.php b/mod/data/tests/entries_importer_test.php new file mode 100644 index 00000000000..10f11f5fc86 --- /dev/null +++ b/mod/data/tests/entries_importer_test.php @@ -0,0 +1,205 @@ +. + +namespace mod_data; + +use context_module; +use mod_data\local\exporter\csv_entries_exporter; +use mod_data\local\exporter\ods_entries_exporter; +use mod_data\local\exporter\utils; +use mod_data\local\importer\csv_entries_importer; +use zip_archive; + +/** + * Unit tests for entries_importer and csv_entries_importer class. + * + * Also {@see entries_import_test} class which provides module tests for importing entries. + * + * @package mod_data + * @covers \mod_data\local\importer\entries_importer + * @covers \mod_data\local\importer\csv_entries_importer + * @copyright 2023 ISB Bayern + * @author Philipp Memmel + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class entries_importer_test extends \advanced_testcase { + + /** + * Set up function. + */ + protected function setUp(): void { + parent::setUp(); + + global $CFG; + require_once($CFG->dirroot . '/mod/data/lib.php'); + require_once($CFG->dirroot . '/lib/datalib.php'); + require_once($CFG->dirroot . '/lib/csvlib.class.php'); + require_once($CFG->dirroot . '/search/tests/fixtures/testable_core_search.php'); + require_once($CFG->dirroot . '/mod/data/tests/generator/lib.php'); + } + + /** + * Get the test data. + * In this instance we are setting up database records to be used in the unit tests. + * + * @return array + */ + protected function get_test_data(): array { + $this->resetAfterTest(true); + + $generator = $this->getDataGenerator()->get_plugin_generator('mod_data'); + $course = $this->getDataGenerator()->create_course(); + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $this->setUser($teacher); + $student = $this->getDataGenerator()->create_and_enrol($course, 'student', array('username' => 'student')); + + $data = $generator->create_instance(array('course' => $course->id)); + $cm = get_coursemodule_from_instance('data', $data->id); + + // Add fields. + $fieldrecord = new \stdClass(); + $fieldrecord->name = 'ID'; // Identifier of the records for testing. + $fieldrecord->type = 'number'; + $generator->create_field($fieldrecord, $data); + + $fieldrecord->name = 'Param2'; + $fieldrecord->type = 'text'; + $generator->create_field($fieldrecord, $data); + + $fieldrecord->name = 'filefield'; + $fieldrecord->type = 'file'; + $generator->create_field($fieldrecord, $data); + + $fieldrecord->name = 'picturefield'; + $fieldrecord->type = 'picture'; + $generator->create_field($fieldrecord, $data); + + return [ + 'teacher' => $teacher, + 'student' => $student, + 'data' => $data, + 'cm' => $cm, + ]; + } + + /** + * Test importing files from zip archive. + * + * @covers \mod_data\local\importer\entries_importer::get_file_content_from_zip + * @covers \mod_data\local\importer\entries_importer::get_data_file_content + * @dataProvider get_file_content_from_zip_provider + * @param array $files array of filenames and filecontents to test + * @param mixed $datafilecontent the expected result returned by the method which is being tested here + */ + public function test_get_file_content_from_zip(array $files, mixed $datafilecontent): void { + // First we need to create the zip file from the provided data. + $tmpdir = make_request_directory(); + $zipfilepath = $tmpdir . '/entries_importer_test_tmp_' . time() . '.zip'; + $ziparchive = new zip_archive(); + $ziparchive->open($zipfilepath); + foreach ($files as $file) { + $localname = empty($file['subdir']) ? $file['filename'] : $file['subdir'] . '/' . $file['filename']; + $ziparchive->add_file_from_string($localname, $file['filecontent']); + } + $ziparchive->close(); + + // We now created a zip archive according to the data provider's data. We now can test the importer. + $importer = new csv_entries_importer($zipfilepath, 'testzip.zip'); + foreach ($files as $file) { + $subdir = empty($file['subdir']) ? '' : $file['subdir']; + $this->assertEquals($file['filecontent'], $importer->get_file_content_from_zip($file['filename'], $subdir)); + } + + // Test the method to retrieve the datafile content. + $this->assertEquals($datafilecontent, $importer->get_data_file_content()); + unlink($zipfilepath); + } + + /** + * Data provider method for self::test_get_file_content_from_zip. + * + * @return array data for testing + */ + public function get_file_content_from_zip_provider(): array { + return [ + 'some files in the zip archive' => [ + 'files' => [ + [ + 'filename' => 'datafile.csv', + 'filecontent' => 'some,csv,data' + ], + [ + 'filename' => 'testfile.txt', + 'filecontent' => 'somecontent', + 'subdir' => 'files' + ], + [ + 'filename' => 'testfile2.txt', + 'filecontent' => 'someothercontent', + 'subdir' => 'testsubdir' + ] + ], + // Should be identical with filecontent of 'datafile.csv' above. + 'datafilecontent' => 'some,csv,data' + ], + 'wrongly placed data file' => [ + 'files' => [ + [ + 'filename' => 'datafile.csv', + 'filecontent' => 'some,csv,data', + 'subdir' => 'wrongsubdir' + ], + [ + 'filename' => 'testfile.txt', + 'filecontent' => 'somecontent', + 'subdir' => 'files' + ], + [ + 'filename' => 'testfile2.txt', + 'filecontent' => 'someothercontent', + 'subdir' => 'testsubdir' + ] + ], + // Data file is not in the root directory, though no content should be retrieved. + 'datafilecontent' => false + ], + 'two data files where only one is allowed' => [ + 'files' => [ + [ + 'filename' => 'datafile.csv', + 'filecontent' => 'some,csv,data', + ], + [ + 'filename' => 'anothercsvfile.csv', + 'filecontent' => 'some,other,csv,data', + ], + [ + 'filename' => 'testfile.txt', + 'filecontent' => 'somecontent', + 'subdir' => 'files' + ], + [ + 'filename' => 'testfile2.txt', + 'filecontent' => 'someothercontent', + 'subdir' => 'testsubdir' + ] + ], + // There are two data files in the zip root, so the data cannot be imported. + 'datafilecontent' => false + ], + ]; + } +} diff --git a/mod/data/upgrade.txt b/mod/data/upgrade.txt index 606d37e962b..069fbff70c3 100644 --- a/mod/data/upgrade.txt +++ b/mod/data/upgrade.txt @@ -5,11 +5,11 @@ information provided here is intended especially for developers. * Function data_export_xls() has been deprecated and moved to deprecatedlib, because xls support has already been dropped. * Functions data_export_csv(), data_export_ods() and data_get_exportdata() have been deprecated due to a bigger refactoring of the way data is being exported. This is now being done by new exporter classes - \mod_data\local\csv_exporter and \mod_data\local\ods_exporter (inheriting from exporter base class - \mod_data\local\exporter) as well as \mod_data\local\exporter_utils::data_exportdata(). + \mod_data\local\exporter\csv_entries_exporter and \mod_data\local\exporter\ods_entries_exporter (inheriting from + exporter base class \mod_data\local\exporter\entries_exporter) as well as \mod_data\local\exporter\utils::data_exportdata(). * Function data_import_csv() has been deprecated and moved to deprecatedlib due to a bigger rework of the way data is - being imported. This is now being done by new importer class \mod_data\local\mod_data_csv_importer inheriting from new - classes \mod_data\local\csv_importer and \mod_data\local\importer. + being imported. This is now being done by new importer class \mod_data\local\importer\csv_importer inheriting from new + class \mod_data\local\importer. * Field base class now has two new methods file_export_supported() and export_file_value(). The method file_export_supported() can be overwritten to declare that a field type can/wants to export a file. In this case this field type will have to implement the method export_file_value() returning this file for exporting. Also: This field