fix: Slots. Again.

document_services.py

@@ -703,7 +703,7 @@ def makepath(p: str, symbol: str):
         im_symbol = use.split('\\')[-1].replace(';', '')
         found = False
         if im_symbol.startswith('{'):
-            for subsymbol in im_symbol.split(','):
+            for subsymbol in im_symbol[1:-1].split(','):
                 if subsymbol.strip() == symbol:
                     found = True
                     break
@@ -800,6 +800,8 @@ def main():
 
     if sys.argv[1] == "-":
         print(data)
+    elif sys.argv[1] == "/dev/null":
+        pass
     else:
         declaration = f"const funcs = {data}"
 

lbplanner/classes/helpers/slot_helper.php

@@ -109,7 +109,7 @@ public static function get_all_slots(): array {
 
         $slotsobj = [];
         foreach ($slots as $slot) {
-            array_push($slotsobj, new slot(...$slot));
+            array_push($slotsobj, slot::from_db($slot));
         }
 
         return $slotsobj;
@@ -125,15 +125,15 @@ public static function get_supervisor_slots(int $supervisorid): array {
         global $DB;
 
         $slots = $DB->get_records_sql(
-            'SELECT slot.* FROM {'.self::TABLE_SLOTS.'} as slot'.
-            'INNER JOIN '.self::TABLE_SUPERVISORS.' as supervisor ON supervisor.slotid=slot.id'.
+            'SELECT slot.* FROM {'.self::TABLE_SLOTS.'} as slot '.
+            'INNER JOIN {'.self::TABLE_SUPERVISORS.'} as supervisor ON supervisor.slotid=slot.id '.
             'WHERE supervisor.userid=?',
             [$supervisorid]
         );
 
         $slotsobj = [];
         foreach ($slots as $slot) {
-            array_push($slotsobj, new slot(...$slot));
+            array_push($slotsobj, slot::from_db($slot));
         }
 
         return $slotsobj;
@@ -151,7 +151,7 @@ public static function get_slots_by_room(string $room): array {
 
         $slotsobj = [];
         foreach ($slots as $slot) {
-            array_push($slotsobj, new slot(...$slot));
+            array_push($slotsobj, slot::from_db($slot));
         }
 
         return $slotsobj;
@@ -167,7 +167,7 @@ public static function get_slot(int $slotid): slot {
         global $DB;
         $slot = $DB->get_record(self::TABLE_SLOTS, ['id' => $slotid]);
 
-        return new slot(...$slot);
+        return slot::from_db($slot);
     }
 
     /**
@@ -197,7 +197,7 @@ public static function get_slot_filter(int $filterid): slot_filter {
         global $DB;
         $filter = $DB->get_record(self::TABLE_SLOT_FILTERS, ['id' => $filterid]);
 
-        return new slot_filter(...$filter);
+        return slot_filter::from_db($filter);
     }
 
     /**
@@ -265,7 +265,7 @@ public static function get_filters_for_slot(int $slotid): array {
 
         $filtersobj = [];
         foreach ($filters as $filter) {
-            array_push($filtersobj, new slot_filter(...$filter));
+            array_push($filtersobj, slot_filter::from_db($filter));
         }
 
         return $filtersobj;
@@ -291,12 +291,12 @@ public static function filter_slots_for_user(array $allslots, \stdClass $user):
             $filters = $slot->get_filters();
             foreach ($filters as $filter) {
                 // Checking for course ID.
-                if (!is_null($filter->courseid) && !in_array($filter->courseid, $mycourseids)) {
+                if (!in_array($filter->courseid, $mycourseids)) {
                     continue;
                 }
                 // TODO: replace address with cohorts.
                 // Checking for vintage.
-                if (!is_null($filter->vintage) && $user->address !== $filter->vintage) {
+                if ($user->address !== $filter->vintage) {
                     continue;
                 }
                 // If all filters passed, add slot to my slots and break.
@@ -331,23 +331,23 @@ public static function filter_slots_for_time(array $allslots, int $range): array
     /**
      * calculates when a slot is to happen next
      * @param slot $slot the slot
-     * @param DateTimeInterface $now the point in time representing now
+     * @param DateTimeImmutable $now the point in time representing now
      * @return DateTimeImmutable the next time this slot will occur
      */
-    public static function calculate_slot_datetime(slot $slot, DateTimeInterface $now): DateTimeImmutable {
-        $utctz = new DateTimeZone('UTC');
+    public static function calculate_slot_datetime(slot $slot, DateTimeImmutable $now): DateTimeImmutable {
         $slotdaytime = self::SCHOOL_UNITS[$slot->startunit];
-        // NOTE: format and fromFormat use different date formatting conventions.
-        $slotdatetime = DateTime::createFromFormat('YY-MM-DD tHH:MM', $now->format('Y-m-d ').$slotdaytime, $utctz);
         // Move to next day this weekday occurs (doesn't move if it's the same as today).
-        $slotdatetime->modify('this '.WEEKDAY::name_from($slot->weekday));
+        $slotdatetime = $now->modify('this '.WEEKDAY::name_from($slot->weekday)." {$slotdaytime}");
+        if ($slotdatetime === false) {
+            throw new \coding_exception('error while calculating slot datetime');
+        }
 
         // Check if slot is before now (because time of day and such) and move it a week into the future if so.
         if ($now->diff($slotdatetime)->invert === 1) {
-            $slotdatetime->add(new DateInterval('P1W'));
+            $slotdatetime = $slotdatetime->modify('+1 week');
         }
 
-        return new DateTimeImmutable($slotdatetime, $utctz);
+        return DateTimeImmutable::createFromInterface($slotdatetime);
     }
 
     /**
@@ -362,7 +362,7 @@ public static function amend_date_with_unit_time(int $unit, DateTimeInterface $d
         $utctz = new DateTimeZone('UTC');
         $daytime = self::SCHOOL_UNITS[$unit];
 
-        return DateTimeImmutable::createFromFormat('YY-MM-DD tHH:MM', $date->format('Y-m-d ').$daytime, $utctz);
+        return DateTimeImmutable::createFromFormat('Y-m-d G:i', $date->format('Y-m-d ').$daytime, $utctz);
     }
 
     /**
@@ -377,9 +377,7 @@ public static function check_slot_supervisor(int $supervisorid, int $slotid): bo
         global $DB;
 
         $context = context_system::instance();
-        if (
-            has_capability(CAPABILITY::SLOTMASTER, $context, $supervisorid)
-        ) {
+        if (has_capability(CAPABILITY::SLOTMASTER, $context, $supervisorid)) {
             return true;
         }
 

lbplanner/classes/model/reservation.php

@@ -91,13 +91,13 @@ public function __construct(int $id, int $slotid, DateTimeImmutable $date, int $
 
     /**
      * Initializes reservation object from a DB object
-     * @param array $obj the DB obj
+     * @param \stdClass $obj the DB obj
      * @return reservation the reservation obj
      */
-    public static function from_obj(array $obj): self {
+    public static function from_obj(\stdClass $obj): self {
         $utctz = new DateTimeZone('UTC');
-        $obj['date'] = new DateTimeImmutable($obj['date'], $utctz);
-        return new self(...$obj);
+        $date = new DateTimeImmutable($obj->date, $utctz);
+        return new self($obj->id, $obj->slotid, $date, $obj->userid, $obj->reserverid);
     }
 
     /**
@@ -203,7 +203,7 @@ public function prepare_for_db(): object {
         $obj = new \stdClass();
 
         $obj->slotid = $this->slotid;
-        $obj->date = $this->date;
+        $obj->date = $this->date->format('Y-m-d');
         $obj->userid = $this->userid;
         $obj->reserverid = $this->reserverid;
 

lbplanner/classes/model/slot.php

@@ -89,22 +89,49 @@ class slot {
      */
     public function __construct(int $id, int $startunit, int $duration, int $weekday, string $room, int $size) {
         $this->id = $id;
-        assert($startunit > 0);
+        if ($startunit <= 0 || $startunit > slot_helper::SCHOOL_UNIT_MAX) {
+            throw new \moodle_exception("slot's startunit must be >0 and <=".slot_helper::SCHOOL_UNIT_MAX.", but is {$startunit}");
+        }
+        if ($duration <= 0 || ($startunit + $duration - 1) > slot_helper::SCHOOL_UNIT_MAX) {
+            throw new \moodle_exception(
+                "slot's duration must be >0 and can't exceed ".slot_helper::SCHOOL_UNIT_MAX." with startunit, but is {$duration}"
+            );
+        }
         $this->startunit = $startunit;
-        assert($duration > 0);
-        assert($duration + $startunit < count(slot_helper::SCHOOL_UNITS));
         $this->duration = $duration;
         $this->weekday = WEEKDAY::from($weekday);
-        assert(strlen($room) > 0 && strlen($room) <= slot_helper::ROOM_MAXLENGTH);
+        if (strlen($room) <= 0 && strlen($room) > slot_helper::ROOM_MAXLENGTH) {
+            throw new \moodle_exception(
+                "room name's length must be >0 and <=".slot_helper::ROOM_MAXLENGTH.", but is ".strlen($room)
+            );
+        }
         $this->room = $room;
-        assert($size >= 0);  // Make it technically possible to not allow any students in a room to temporarily disable the slot.
+        if ($size < 0) {
+            throw new \moodle_exception('room size must be >0');
+        }
         $this->size = $size;
         $this->fullness = null;
         $this->forcuruser = null;
         $this->supervisors = null;
         $this->filters = null;
     }
 
+    /**
+     * Creates a slot object from a DB result
+     * @param \stdClass $obj the DB object
+     * @return slot the resulting slot object
+     */
+    public static function from_db(\stdClass $obj): self {
+        return new slot(
+            $obj->id,
+            $obj->startunit,
+            $obj->duration,
+            $obj->weekday,
+            $obj->room,
+            $obj->size,
+        );
+    }
+
     /**
      * Mark the object as freshly created and sets the new ID
      * @param int $id the new ID after inserting into the DB
@@ -184,11 +211,10 @@ public function get_forcuruser(): bool {
     public function get_supervisors(): array {
         global $DB;
         if (is_null($this->supervisors)) {
-            $this->supervisors = $DB->get_records(
+            $this->supervisors = $DB->get_fieldset(
                 slot_helper::TABLE_SUPERVISORS,
+                'userid',
                 ['slotid' => $this->id],
-                '',
-                ['userid']
             );
         }
 
@@ -228,7 +254,7 @@ public function check_overlaps(slot $other, bool $checkroom): bool {
         // Now only three variants are left: one entirely inside the other, or both intersecting partially.
         // In either case, if one of the startunits is inside the other's range, then we know the time ranges overlap.
         // Logically, only the one that starts later can be inside the other's range.
-        $thisbeforeother = $this->startunit < $other;
+        $thisbeforeother = $this->startunit < $other->startunit;
         $a = $thisbeforeother ? $this : $other;
         $b = $thisbeforeother ? $other : $this;
 

lbplanner/classes/model/slot_filter.php

@@ -42,11 +42,11 @@ class slot_filter {
     /**
      * @var ?int $id ID of linked course or null if any
      */
-    public ?int $courseid;
+    public int $courseid;
     /**
      * @var ?string $vintage linked class or null if any
      */
-    public ?string $vintage;
+    public string $vintage;
 
     /**
      * Constructs new slot_filter
@@ -55,18 +55,29 @@ class slot_filter {
      * @param ?int $courseid ID of linked course or null if any
      * @param ?string $vintage linked class or null if any
      */
-    public function __construct(int $id, int $slotid, ?int $courseid, ?string $vintage) {
-        assert(!(is_null($courseid) && is_null($vintage)));
-        if (!is_null($vintage)) {
-            assert(strlen($vintage) <= 7);
-        }
+    public function __construct(int $id, int $slotid, int $courseid, string $vintage) {
+        assert(strlen($vintage) <= 7);
 
         $this->id = $id;
         $this->slotid = $slotid;
         $this->courseid = $courseid;
         $this->vintage = $vintage;
     }
 
+    /**
+     * Creates a slot_filter object from a DB result
+     * @param \stdClass $obj the DB object
+     * @return slot_filter the resulting slot_filter object
+     */
+    public static function from_db(\stdClass $obj): self {
+        return new slot_filter(
+            $obj->id,
+            $obj->slotid,
+            $obj->courseid,
+            $obj->vintage,
+        );
+    }
+
     /**
      * Mark the object as freshly created and sets the new ID
      * @param int $id the new ID after inserting into the DB
@@ -123,16 +134,12 @@ public static function api_structure(): external_single_structure {
                 'courseid' => new external_value(
                     PARAM_INT,
                     'ID of course to filter for (or null if "any")',
-                    VALUE_REQUIRED,
-                    null,
-                    NULL_ALLOWED
+                    VALUE_REQUIRED
                 ),
                 'vintage' => new external_value(
-                    PARAM_INT,
+                    PARAM_TEXT,
                     'class name to filter for (or null if "any")',
-                    VALUE_REQUIRED,
-                    null,
-                    NULL_ALLOWED
+                    VALUE_REQUIRED
                 ),
             ]
         );

lbplanner/db/services.php

@@ -333,8 +333,8 @@
         'ajax' => true,
     ],
     'local_lbplanner_slots_update_slot' => [
-        'classname' => 'update_slot\slots_update_slot',
-        'methodname' => 'create_slot',
+        'classname' => 'local_lbplanner_services\slots_update_slot',
+        'methodname' => 'update_slot',
         'classpath' => 'local/lbplanner/services/slots/update_slot.php',
         'description' => 'Update a slot\'s values',
         'type' => 'write',
@@ -438,6 +438,7 @@
             'local_lbplanner_slots_get_student_slots',
             'local_lbplanner_slots_get_supervisor_slots',
             'local_lbplanner_slots_remove_slot_supervisor',
+            'local_lbplanner_slots_update_slot',
         ],
         'restrictedusers' => 0,
         'enabled' => 1,

lbplanner/services/courses/get_all_courses.php

@@ -46,7 +46,7 @@ public static function get_all_courses_parameters(): external_function_parameter
     public static function get_all_courses(): array {
         global $USER;
         $user = user::from_mdlobj($USER);
-        if (!($user->get_capabilitybitmask() & CAPABILITY_FLAG::SLOTMASTER)) {
+        if (!($user->get_capabilitybitmask() & (CAPABILITY_FLAG::SLOTMASTER | CAPABILITY_FLAG::TEACHER))) {
             throw new \moodle_exception('access denied: must be slotmaster');
         }
 

lbplanner/services/slots/book_reservation.php

@@ -83,8 +83,11 @@ public static function book_reservation(int $slotid, string $date, int $userid):
         );
 
         $now = new DateTimeImmutable();
-        $dateobj = DateTimeImmutable::createFromFormat("YY-MM-DD", $date);
-        $td = $dateobj->diff($now);
+        $dateobj = DateTimeImmutable::createFromFormat("Y-m-d", $date);
+        if ($dateobj === false) {
+            throw new \moodle_exception("invalid date formatting: got '{$date}', must be YYYY-MM-DD");
+        }
+        $td = $now->diff($dateobj);
 
         if ($td->invert) {
             throw new \moodle_exception('Can\'t reserve date in the past');
@@ -93,7 +96,7 @@ public static function book_reservation(int $slotid, string $date, int $userid):
         $maxdays = null;
         $student = null;
 
-        if ($userid === $USER->id) {
+        if ($userid === intval($USER->id)) {
             // Student reserving slot for themself.
 
             $maxdays = slot_helper::RESERVATION_RANGE_USER;