From 25c13972a4491a0bfb02b653436ab3c6d805ac4d Mon Sep 17 00:00:00 2001
From: Kevin Pham <keevan.pham@gmail.com>
Date: Fri, 3 May 2024 11:55:08 +1000
Subject: [PATCH] fix: clean up and some optimisations

- add missing phpdocs,
- optimise coursecontext get by caching the call to the class,
- convert moodle_url to abs path
- remove double blank lines
- remove dead code
- fix file upload handling
---
 attendees.php                          |  3 -
 classes/booking_manager.php            | 36 +++++++++---
 classes/event/course_viewed.php        |  1 -
 classes/event/csv_processed.php        | 81 ++++++++++++++++++++++++++
 classes/form/confirm_bookings_form.php |  2 +-
 classes/form/upload_bookings_form.php  |  4 +-
 classes/privacy/provider.php           |  4 +-
 db/upgrade.php                         |  1 -
 lang/en/facetoface.php                 |  1 +
 lib.php                                | 22 ++-----
 sessions.php                           |  2 -
 sitenotice.php                         |  2 -
 upload.php                             | 19 +++---
 13 files changed, 132 insertions(+), 46 deletions(-)
 create mode 100644 classes/event/csv_processed.php

diff --git a/attendees.php b/attendees.php
index 1693c85..2401419 100644
--- a/attendees.php
+++ b/attendees.php
@@ -59,7 +59,6 @@
 // Load cancellations.
 $cancellations = facetoface_get_cancellations($session->id);
 
-
 /*
  * Capability checks to see if the current user can view this page
  *
@@ -341,7 +340,6 @@
 }
 echo html_writer::link($url, get_string('goback', 'facetoface')) . html_writer::end_tag('p');
 
-
 /*
  * Print unapproved requests (if user able to view)
  */
@@ -358,7 +356,6 @@
             echo html_writer::tag('p', get_string('cannotapproveatcapacity', 'facetoface'));
         }
 
-
         $action = new moodle_url('attendees.php', ['s' => $s]);
         echo html_writer::start_tag('form', ['action' => $action->out(), 'method' => 'post']);
         echo html_writer::empty_tag('input', ['type' => 'hidden', 'name' => 'sesskey', 'value' => $USER->sesskey]);
diff --git a/classes/booking_manager.php b/classes/booking_manager.php
index 1d099fb..4a4e837 100644
--- a/classes/booking_manager.php
+++ b/classes/booking_manager.php
@@ -17,8 +17,10 @@
 namespace mod_facetoface;
 
 use context_course;
-use moodle_exception;
+use context_user;
+use file_storage;
 use lang_string;
+use moodle_exception;
 
 class booking_manager {
 
@@ -31,6 +33,9 @@ class booking_manager {
     /** @var int The course id. */
     private $course;
 
+    /** @var context_course The course context. */
+    private $coursecontext;
+
     /** @var int The course id. */
     private $facetoface;
 
@@ -58,13 +63,13 @@ public function __construct($f, $records = []) {
         $this->f = $f;
         $this->facetoface = $facetoface;
         $this->course = $course;
+        $this->coursecontext = context_course::instance($course->id);
         $this->records = $records;
     }
 
     /**
      * Returns file from file system. File must exist.
      * @param int $fileitemid Item id of file stored in the current $USER's draft file area
-     * @return stored_file
      */
     public function load_from_file(int $fileitemid) {
         global $USER;
@@ -77,7 +82,7 @@ public function load_from_file(int $fileitemid) {
             throw new moodle_exception('error:cannotloadfile', 'mod_facetoface');
         }
 
-        return current($files);
+        $this->file = current($files);
     }
 
     /**
@@ -91,6 +96,10 @@ public function load_from_array(array $records) {
         return $this;
     }
 
+    /**
+     * Get the headers for the records.
+     * @return array
+     */
     public static function get_headers(): array {
         return [
             'email',
@@ -101,7 +110,11 @@ public static function get_headers(): array {
         ];
     }
 
-    private function get_iterator() {
+    /**
+     * Get an iterator for the records.
+     * @return Generator
+     */
+    private function get_iterator(): \Generator {
         if (!$this->usefile) {
             foreach ($this->records as $record) {
                 yield $record;
@@ -131,12 +144,20 @@ private function get_iterator() {
         }
     }
 
-    public function validate() {
+    /**
+     * Validate the records provided to ensure they can be processed without errors.
+     *
+     * As there are multiple dependant data points (users, sessions, capacity)
+     * that are checked. They are all in this method.
+     *
+     * @return array An array of errors.
+     */
+    public function validate(): array {
         global $DB;
         $errors = [];
         $sessioncapacitycache = [];
 
-        // Break into rows.
+        // Break into rows and validate the multiple interdependant fields together.
         foreach ($this->get_iterator() as $index => $entry) {
             $row = $index + 1;
 
@@ -200,8 +221,7 @@ public function validate() {
             }
 
             // Check user enrolment into the course.
-            $coursecontext = context_course::instance($this->course->id);
-            if (isset($userid) && !is_enrolled($coursecontext, $userid)) {
+            if (isset($userid) && !is_enrolled($this->coursecontext, $userid)) {
                 $errors[] = [$row, new lang_string('error:userisnotenrolledintocourse', 'mod_facetoface', $entry->email)];
             }
 
diff --git a/classes/event/course_viewed.php b/classes/event/course_viewed.php
index 540b929..06f2706 100644
--- a/classes/event/course_viewed.php
+++ b/classes/event/course_viewed.php
@@ -75,5 +75,4 @@ public static function get_name() {
     public function get_url() {
         return new \moodle_url('/mod/facetoface/index.php', ['id' => $this->courseid]);
     }
-
 }
diff --git a/classes/event/csv_processed.php b/classes/event/csv_processed.php
new file mode 100644
index 0000000..e4a3f70
--- /dev/null
+++ b/classes/event/csv_processed.php
@@ -0,0 +1,81 @@
+<?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/>.
+
+namespace mod_facetoface\event;
+
+/**
+ * The mod_facetoface CSV processed event.
+ *
+ * @package    mod_facetoface
+ * @author     Kevin Pham <kevinpham@catalyst-au.net>
+ * @copyright  Catalyst IT, 2024
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class csv_processed extends \core\event\base {
+
+    /**
+     * Init method.
+     *
+     * @return void
+     */
+    protected function init() {
+        $this->data['crud'] = 'r';
+        $this->data['edulevel'] = self::LEVEL_PARTICIPATING;
+        $this->data['objecttable'] = 'facetoface';
+    }
+
+    /**
+     * Returns description of what happened.
+     *
+     * @return string
+     */
+    public function get_description() {
+        return "The user with id '$this->userid' has processed an uploaded CSV file for session bookings " .
+            "in the facetoface instance with the course module id '$this->contextinstanceid'.";
+    }
+
+    /**
+     * Return localised event name.
+     *
+     * @return string
+     */
+    public static function get_name() {
+        return get_string('eventcsvprocessed', 'mod_facetoface');
+    }
+
+    /**
+     * Get URL related to the action
+     *
+     * @return \moodle_url
+     */
+    public function get_url() {
+        return new \moodle_url('/mod/facetoface/upload.php', ['f' => $this->objectid]);
+    }
+
+    /**
+     * Custom validation.
+     *
+     * @throws \coding_exception
+     * @return void
+     */
+    protected function validate_data() {
+        parent::validate_data();
+
+        if ($this->contextlevel != CONTEXT_MODULE) {
+            throw new \coding_exception('Context level must be CONTEXT_MODULE.');
+        }
+    }
+}
diff --git a/classes/form/confirm_bookings_form.php b/classes/form/confirm_bookings_form.php
index 5bda087..b79b2ec 100644
--- a/classes/form/confirm_bookings_form.php
+++ b/classes/form/confirm_bookings_form.php
@@ -54,7 +54,7 @@ public function definition() {
 
         $backurl = new moodle_url('/mod/facetoface/upload.php', ['f' => $f]);
         $htmlbuttons = $OUTPUT->render((new single_button(
-            new moodle_url('upload.php', ['f' => $f, 'fileid' => $fileid, 'process' => 1]),
+            new moodle_url('/mod/facetoface/upload.php', ['f' => $f, 'fileid' => $fileid, 'process' => 1]),
             get_string('facetoface:confirmandprocess', 'mod_facetoface'),
             'post',
             true
diff --git a/classes/form/upload_bookings_form.php b/classes/form/upload_bookings_form.php
index 084cf6c..f4d76eb 100644
--- a/classes/form/upload_bookings_form.php
+++ b/classes/form/upload_bookings_form.php
@@ -30,7 +30,7 @@
 class upload_bookings_form extends \moodleform {
 
     /**
-     * Build form for importing woekflows.
+     * Build form for importing bookings.
      *
      * {@inheritDoc}
      * @see \moodleform::definition()
@@ -42,7 +42,7 @@ public function definition() {
 
         $mform->addElement('header', 'settingsheader', get_string('upload'));
 
-        $url = new moodle_url('example.csv');
+        $url = new moodle_url('/mod/facetoface/example.csv');
         $link = html_writer::link($url, 'example.csv');
         $mform->addElement('static', 'examplecsv', get_string('facetoface:examplecsv', 'mod_facetoface'), $link);
 
diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php
index ae36179..e8ea863 100644
--- a/classes/privacy/provider.php
+++ b/classes/privacy/provider.php
@@ -55,7 +55,7 @@ class provider implements
      * @param collection $items a reference to the collection to use to store the metadata.
      * @return collection the updated collection of metadata items.
      */
-    public static function get_metadata(collection $collection) : collection {
+    public static function get_metadata(collection $collection): collection {
         $collection->add_database_table(
             'facetoface_signups',
             [
@@ -98,7 +98,7 @@ public static function get_metadata(collection $collection) : collection {
      * @param int $userid the userid.
      * @return contextlist the list of contexts containing user info for the user.
      */
-    public static function get_contexts_for_userid(int $userid) : contextlist {
+    public static function get_contexts_for_userid(int $userid): contextlist {
         // Fetch all facetoface contexts with userdata.
         $sql = "SELECT c.id
                   FROM {context} c
diff --git a/db/upgrade.php b/db/upgrade.php
index 49224a2..1981038 100644
--- a/db/upgrade.php
+++ b/db/upgrade.php
@@ -29,7 +29,6 @@
  */
 
 /**
- *
  * Sends message to administrator listing all updated
  * duplicate custom fields
  * @param array $data
diff --git a/lang/en/facetoface.php b/lang/en/facetoface.php
index c6e07ed..2dd4913 100644
--- a/lang/en/facetoface.php
+++ b/lang/en/facetoface.php
@@ -810,6 +810,7 @@
 
 /* Face-to-face events and logging */
 $string['eventaddsession'] = 'Session added';
+$string['eventcsvprocessed'] = 'CSV processed';
 $string['eventaddsessionfailed'] = 'Session add (FAILED)';
 $string['eventapproverequests'] = 'Session approve requests';
 $string['eventattendeesviewed'] = 'Session attendees viewed';
diff --git a/lib.php b/lib.php
index b3f07a8..ed41045 100644
--- a/lib.php
+++ b/lib.php
@@ -1162,7 +1162,7 @@ function facetoface_get_attendees($sessionid) {
     global $CFG, $DB;
 
     $usernamefields = facetoface_get_all_user_name_fields(true, 'u');
-    $records = $DB->get_records_sql("
+    return $DB->get_records_sql("
         SELECT u.id, {$usernamefields},
             u.email,
             su.id AS submissionid,
@@ -1213,8 +1213,6 @@ function facetoface_get_attendees($sessionid) {
             sign.timecreated ASC,
             ss.timecreated ASC
     ", [$sessionid, MDL_F2F_STATUS_BOOKED, MDL_F2F_STATUS_WAITLISTED, $sessionid, MDL_F2F_STATUS_APPROVED]);
-
-    return $records;
 }
 
 /**
@@ -1954,7 +1952,6 @@ function facetoface_user_signup($session, $facetoface, $course, $discountcode,
 
     if (!$success) {
         throw new moodle_exception('error:couldnotupdatef2frecord', 'facetoface');
-        return false;
     }
 
     // Work out which status to use.
@@ -1985,7 +1982,6 @@ function facetoface_user_signup($session, $facetoface, $course, $discountcode,
     // Update status.
     if (!facetoface_update_signup_status($usersignup->id, $newstatus, $userid)) {
         throw new moodle_exception('error:f2ffailedupdatestatus', 'facetoface');
-        return false;
     }
 
     // Add to user calendar -- if facetoface usercalentry is set to true.
@@ -2124,7 +2120,6 @@ function facetoface_send_request_notice($facetoface, $session, $userid) {
  * @param bool $usetransaction Set to true if database transactions are to be used
  *
  * @returns integer ID of newly created signup status, or false
- *
  */
 function facetoface_update_signup_status($signupid, $statuscode, $createdby, $note='', $grade=null) {
     global $DB;
@@ -2904,7 +2899,7 @@ function facetoface_cm_info_view(cm_info $coursemodule) {
                     $moreinfolink = html_writer::link(
                         $signupurl,
                         get_string($signupstr, 'facetoface'),
-                        ['class' => 'f2fsessionlinks f2fsessioninfolink'],
+                        ['class' => 'f2fsessionlinks f2fsessioninfolink']
                     );
                     $span = html_writer::tag('span', get_string('options', 'facetoface').':', ['class' => 'f2fsessionnotice']);
                 }
@@ -2963,13 +2958,13 @@ function facetoface_cm_info_view(cm_info $coursemodule) {
                     $output .= html_writer::tag(
                         'span',
                         get_string('signupforsession', 'facetoface'),
-                        ['class' => 'f2fsessionnotice'],
+                        ['class' => 'f2fsessionnotice']
                     );
                 } else {
                     $output .= html_writer::tag(
                         'span',
                         get_string('upcomingsessions', 'facetoface'),
-                        ['class' => 'f2fsessionnotice'],
+                        ['class' => 'f2fsessionnotice']
                     );
 
                     if (!empty($futuresessions)) {
@@ -3860,9 +3855,7 @@ function facetoface_get_customfielddata($sessionid) {
               JOIN {facetoface_session_data} d ON f.id = d.fieldid
               WHERE d.sessionid = ?";
 
-    $records = $DB->get_records_sql($sql, [$sessionid]);
-
-    return $records;
+    return $DB->get_records_sql($sql, [$sessionid]);
 }
 
 /**
@@ -3937,9 +3930,6 @@ function facetoface_update_trainers($sessionid, $form) {
 
                 if (!$DB->insert_record('facetoface_session_roles', $newtrainer)) {
                     throw new moodle_exception('error:couldnotaddtrainer', 'facetoface');
-                    $transaction->force_transaction_rollback();
-
-                    return false;
                 }
             } else {
                 unset($oldtrainers[$roleid][$trainer]);
@@ -3963,8 +3953,6 @@ function facetoface_update_trainers($sessionid, $form) {
                     'userid' => $trainer->id,
                 ])) {
                     throw new moodle_exception('error:couldnotdeletetrainer', 'facetoface');
-                    $transaction->force_transaction_rollback();
-                    return false;
                 }
             }
         }
diff --git a/sessions.php b/sessions.php
index 21a06ef..ecbcbb0 100644
--- a/sessions.php
+++ b/sessions.php
@@ -95,7 +95,6 @@
     'context'  => $modulecontext,
 ];
 
-
 // Handle deletions.
 if ($d && $confirm) {
     if (!confirm_sesskey()) {
@@ -361,7 +360,6 @@
 
 $pagetitle = format_string($facetoface->name);
 
-
 $PAGE->set_title($pagetitle);
 $PAGE->set_heading($course->fullname);
 
diff --git a/sitenotice.php b/sitenotice.php
index 838051c..f09ae40 100644
--- a/sitenotice.php
+++ b/sitenotice.php
@@ -97,8 +97,6 @@
         throw new moodle_exception('error:unknownbuttonclicked', 'facetoface', $returnurl);
     }
 
-
-
     $todb = new stdClass();
     $todb->name = trim($fromform->name);
     $todb->text = trim($fromform->text['text']);
diff --git a/upload.php b/upload.php
index 4629a74..144f0d2 100644
--- a/upload.php
+++ b/upload.php
@@ -23,6 +23,7 @@
 use mod_facetoface\form\upload_bookings_form;
 use mod_facetoface\form\confirm_bookings_form;
 use mod_facetoface\booking_manager;
+use mod_facetoface\event\csv_processed;
 
 $f = optional_param('f', 0, PARAM_INT); // The facetoface module ID.
 $fileid = optional_param('fileid', 0, PARAM_INT); // The fileid of the file uploaded.
@@ -40,14 +41,12 @@
     throw new moodle_exception('error:incorrectcoursemoduleid', 'facetoface');
 }
 
-
 require_course_login($course, true, $cm);
 $context = context_course::instance($course->id);
 $modulecontext = context_module::instance($cm->id);
 require_capability('mod/facetoface:editsessions', $context);
 require_capability('mod/facetoface:uploadbookings', $context);
 
-
 echo $OUTPUT->header();
 
 // Render form, which should only consist of an upload element.
@@ -85,7 +84,6 @@
         echo \core\notification::success(get_string('facetoface:uploadreadytoprocess', 'mod_facetoface'));
     }
 
-
     // Set form data to allow user to continue and process the uploaded file on their next form submit.
 } else if ($process && $fileid && $f) {
     // Form submitted, and ready for processing -> process.
@@ -98,12 +96,20 @@
         // Process entries.
         $bm->process();
 
-        echo "PROCESSED";
-        die;
+        // Logging and events trigger.
+        $params = [
+            'context'  => $modulecontext,
+            'objectid' => $f,
+        ];
+        $event = \mod_facetoface\event\csv_processed::create($params);
+        $event->add_record_snapshot('facetoface_sessions', $session);
+        $event->add_record_snapshot('facetoface', $facetoface);
+        $event->trigger();
+
         // Redirect back to start with notification.
         redirect(
             new moodle_url('/mod/facetoface/upload.php', ['f' => $f]),
-            'Successfully processed X records.',
+            'Successfully processed records.',
             null,
             notification::NOTIFY_SUCCESS);
     }
@@ -116,7 +122,6 @@
         notification::NOTIFY_ERROR);
 } else {
     $mform = new upload_bookings_form(null);
-    $data = $mform->get_data();
     $mform->set_data(['f' => $f, 'validate' => 1]);
 
     // Form not subumitted -> prep the form with current context (f2f module id).