From 43b983d02f4a83ec8ca66c321076aa31a065a918 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 29 Sep 2026 23:57:56 +0200 Subject: [PATCH 1/2] feat(timetable): the generator input from learniq's event or two uploaded sheets --- appinfo/routes.php | 2 + lib/Controller/TimetableInputController.php | 109 ++++++++ lib/Event/TimetableActivitiesQueryEvent.php | 148 ++++++++++ lib/Service/TimetableCsvParser.php | 264 ++++++++++++++++++ lib/Service/TimetableInputBuilder.php | 202 ++++++++++++++ lib/Timetabling/SolverInput.php | 85 ++++++ .../changes/timetabling-generator/tasks.md | 6 +- .../TimetableInputControllerTest.php | 146 ++++++++++ tests/unit/Service/TimetableCsvParserTest.php | 120 ++++++++ .../Service/TimetableInputBuilderTest.php | 183 ++++++++++++ 10 files changed, 1262 insertions(+), 3 deletions(-) create mode 100644 lib/Controller/TimetableInputController.php create mode 100644 lib/Event/TimetableActivitiesQueryEvent.php create mode 100644 lib/Service/TimetableCsvParser.php create mode 100644 lib/Service/TimetableInputBuilder.php create mode 100644 lib/Timetabling/SolverInput.php create mode 100644 tests/unit/Controller/TimetableInputControllerTest.php create mode 100644 tests/unit/Service/TimetableCsvParserTest.php create mode 100644 tests/unit/Service/TimetableInputBuilderTest.php diff --git a/appinfo/routes.php b/appinfo/routes.php index 850e9e71..7303e607 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -53,4 +53,6 @@ ['name' => 'timetable#upsert', 'url' => '/api/timetable/sessions/upsert', 'verb' => 'POST'], // Admins publish a source's draft lessons in a window (timetable-draft-review). ['name' => 'timetable#publish', 'url' => '/api/timetable/sessions/publish', 'verb' => 'POST'], + // Admins upload the activities and rooms sheets for the timetable generator (timetabling-generator 2.2). + ['name' => 'timetableInput#upload', 'url' => '/api/timetable/input/upload', 'verb' => 'POST'], ]); diff --git a/lib/Controller/TimetableInputController.php b/lib/Controller/TimetableInputController.php new file mode 100644 index 00000000..cff85fe1 --- /dev/null +++ b/lib/Controller/TimetableInputController.php @@ -0,0 +1,109 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Controller; + +use OCA\Planninq\AppInfo\Application; +use OCA\Planninq\Service\SettingsService; +use OCA\Planninq\Service\TimetableCsvParser; +use OCA\Planninq\Service\TimetableInputBuilder; +use OCA\Planninq\Settings\AdminSettings; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; +use OCP\AppFramework\Http\JSONResponse; +use OCP\IRequest; + +/** + * Takes the uploaded rooms and activities sheets. + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ +class TimetableInputController extends Controller { + + /** + * Constructor. + * + * @param IRequest $request The request. + * @param TimetableCsvParser $parser Reads the sheets. + * @param TimetableInputBuilder $inputBuilder Keeps the parsed sheets. + * @param SettingsService $settingsService Answers whether the caller is an admin. + * + * @return void + */ + public function __construct( + IRequest $request, + private readonly TimetableCsvParser $parser, + private readonly TimetableInputBuilder $inputBuilder, + private readonly SettingsService $settingsService, + ) { + parent::__construct(appName: Application::APP_ID, request: $request); + }//end __construct() + + /** + * Parse and keep the rooms and activities sheets. Admins only. + * + * Nextcloud's admin middleware refuses everyone else before this runs; the + * explicit check is defence in depth, as in TimetableController::upsert(). + * Nothing is kept when either sheet has a refused line. + * + * @return JSONResponse 200 with the counts; 400 with the refused lines; 403 for a non-admin. + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + #[AuthorizedAdminSetting(settings: AdminSettings::class)] + public function upload(): JSONResponse { + if ($this->settingsService->isCurrentUserAdmin() === false) { + return new JSONResponse(['error' => 'Only an admin can upload timetable activities.'], Http::STATUS_FORBIDDEN); + } + + $roomsCsv = $this->request->getParam('rooms'); + $activitiesCsv = $this->request->getParam('activities'); + if (is_string($roomsCsv) === false || is_string($activitiesCsv) === false) { + return new JSONResponse(['error' => 'Send the rooms and the activities sheet as text.'], Http::STATUS_BAD_REQUEST); + } + + $rooms = $this->parser->parseRooms(csv: $roomsCsv); + $types = array_values(array_unique(array_column($rooms['rows'], 'type'))); + $activities = $this->parser->parseActivities(csv: $activitiesCsv, roomTypes: $types); + $errors = [ + 'rooms' => $rooms['errors'], + 'activities' => $activities['errors'], + ]; + if ($rooms['errors'] !== [] || $activities['errors'] !== []) { + return new JSONResponse(['error' => 'Some lines were refused. Nothing was kept.', 'errors' => $errors], Http::STATUS_BAD_REQUEST); + } + + $this->inputBuilder->storeUpload(rooms: $rooms['rows'], activities: $activities['rows']); + + return new JSONResponse( + [ + 'rooms' => count($rooms['rows']), + 'activities' => count($activities['rows']), + 'lessons' => array_sum(array_column($activities['rows'], 'lessonsPerWeek')), + ] + ); + }//end upload() +}//end class diff --git a/lib/Event/TimetableActivitiesQueryEvent.php b/lib/Event/TimetableActivitiesQueryEvent.php new file mode 100644 index 00000000..6dac14cc --- /dev/null +++ b/lib/Event/TimetableActivitiesQueryEvent.php @@ -0,0 +1,148 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Event; + +use OCP\EventDispatcher\Event; + +/** + * A request for the activities and rooms of one academic year. + * + * An activity row: group, subject, teacher (a Nextcloud user id), lessonsPerWeek, + * lessonLength (in periods) and roomType. A room row: reference, capacity, type. + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ +class TimetableActivitiesQueryEvent extends Event { + + /** + * Contract version of this event and its answer. + * + * @var integer + */ + public const CONTRACT_VERSION = 1; + + /** + * The activities, once an app answered. + * + * @var array>|null + */ + private ?array $activities = null; + + /** + * The rooms, once an app answered. + * + * @var array> + */ + private array $rooms = []; + + /** + * The app that answered. + * + * @var string|null + */ + private ?string $answeredBy = null; + + /** + * Constructor. + * + * @param string $academicYear The academic year asked for, such as `2026-2027`. + * + * @return void + */ + public function __construct( + private readonly string $academicYear, + ) { + parent::__construct(); + }//end __construct() + + /** + * The academic year asked for. + * + * @return string + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function getAcademicYear(): string { + return $this->academicYear; + }//end getAcademicYear() + + /** + * Answer the event with the year's activities and rooms. + * + * @param string $app The answering app, such as `learniq`. + * @param array> $activities Activity rows. + * @param array> $rooms Room rows. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function answer(string $app, array $activities, array $rooms): void { + $this->answeredBy = $app; + $this->activities = array_values($activities); + $this->rooms = array_values($rooms); + }//end answer() + + /** + * The activities, or null when no app answered. + * + * @return array>|null + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function getActivities(): ?array { + return $this->activities; + }//end getActivities() + + /** + * The rooms the answering app sent. + * + * @return array> + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function getRooms(): array { + return $this->rooms; + }//end getRooms() + + /** + * The app that answered, or null. + * + * @return string|null + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function getAnsweredBy(): ?string { + return $this->answeredBy; + }//end getAnsweredBy() +}//end class diff --git a/lib/Service/TimetableCsvParser.php b/lib/Service/TimetableCsvParser.php new file mode 100644 index 00000000..f2ce499f --- /dev/null +++ b/lib/Service/TimetableCsvParser.php @@ -0,0 +1,264 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Service; + +/** + * Parses the rooms and activities sheets into the rows the input builder reads. + * + * A result is {rows, errors}; an error is {line, field, code, message}. Codes: + * `header`, `empty`, `whole-number`, `unknown-room-type`. The header check + * ignores case and surrounding spaces; a comma or a semicolon separates columns. + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ +class TimetableCsvParser { + + /** + * The rooms sheet columns, in order. + * + * @var array + */ + public const ROOM_COLUMNS = ['reference', 'capacity', 'type']; + + /** + * The activities sheet columns, in order. + * + * @var array + */ + public const ACTIVITY_COLUMNS = ['group', 'subject', 'teacher', 'lessons per week', 'lesson length', 'room type']; + + /** + * The row key each activities column is stored under. + * + * @var array + */ + private const ACTIVITY_KEYS = [ + 'group' => 'group', + 'subject' => 'subject', + 'teacher' => 'teacher', + 'lessons per week' => 'lessonsPerWeek', + 'lesson length' => 'lessonLength', + 'room type' => 'roomType', + ]; + + /** + * Parse the rooms sheet. + * + * @param string $csv The sheet as text. + * + * @return array{rows:array,errors:array} + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function parseRooms(string $csv): array { + $rows = []; + $errors = []; + foreach ($this->records(csv: $csv, columns: self::ROOM_COLUMNS, errors: $errors) as $line => $cells) { + $capacity = $this->wholeNumber(value: $cells['capacity'], line: $line, field: 'capacity', errors: $errors); + if ($this->filled(cells: $cells, line: $line, errors: $errors) === false || $capacity === null) { + continue; + } + + $rows[] = ['reference' => $cells['reference'], 'capacity' => $capacity, 'type' => $cells['type']]; + } + + return ['rows' => $rows, 'errors' => $errors]; + }//end parseRooms() + + /** + * Parse the activities sheet; every room type must be one of the rooms' types. + * + * @param string $csv The sheet as text. + * @param array $roomTypes The room types the rooms sheet holds. + * + * @return array{rows:array>,errors:array} + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function parseActivities(string $csv, array $roomTypes): array { + $rows = []; + $errors = []; + foreach ($this->records(csv: $csv, columns: self::ACTIVITY_COLUMNS, errors: $errors) as $line => $cells) { + $lessons = $this->wholeNumber(value: $cells['lessons per week'], line: $line, field: 'lessons per week', errors: $errors); + $length = $this->wholeNumber(value: $cells['lesson length'], line: $line, field: 'lesson length', errors: $errors); + $filled = $this->filled(cells: $cells, line: $line, errors: $errors); + $known = $this->knownRoomType(type: $cells['room type'], roomTypes: $roomTypes, line: $line, errors: $errors); + if ($filled === false || $known === false || $lessons === null || $length === null) { + continue; + } + + $row = []; + foreach (self::ACTIVITY_KEYS as $column => $key) { + $row[$key] = $cells[$column]; + } + + $row['lessonsPerWeek'] = $lessons; + $row['lessonLength'] = $length; + $rows[] = $row; + } + + return ['rows' => $rows, 'errors' => $errors]; + }//end parseActivities() + + /** + * The data lines by line number, cells keyed by column; a wrong header adds one error and yields nothing. + * + * @param string $csv The sheet. + * @param array $columns The expected columns. + * @param array $errors Errors, appended to. + * + * @return array> + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + private function records(string $csv, array $columns, array &$errors): array { + $lines = preg_split('/\r\n|\r|\n/', trim($csv)); + if ($lines === false) { + $lines = []; + } + + $head = array_map(static fn (string $cell): string => mb_strtolower(trim($cell)), $this->cells(line: (string)($lines[0] ?? ''))); + if ($head !== $columns) { + $errors[] = $this->error(line: 1, field: '', code: 'header', message: 'The first line must be: '.implode(', ', $columns).'.'); + return []; + } + + $records = []; + foreach (array_slice($lines, 1, null, true) as $index => $line) { + if (trim($line) === '') { + continue; + } + + $cells = array_pad(array_map('trim', $this->cells(line: $line)), count($columns), ''); + $records[$index + 1] = array_combine($columns, array_slice($cells, 0, count($columns))); + } + + return $records; + }//end records() + + /** + * The cells of one line, split on the separator the line uses. + * + * @param string $line One line. + * + * @return array + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + private function cells(string $line): array { + $separator = ','; + if (substr_count($line, ';') > substr_count($line, ',')) { + $separator = ';'; + } + + return array_map('strval', str_getcsv($line, $separator, '"', '')); + }//end cells() + + /** + * Whether no cell is empty; each empty cell adds an error. + * + * @param array $cells The line's cells. + * @param int $line The line number. + * @param array $errors Errors, appended to. + * + * @return bool + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + private function filled(array $cells, int $line, array &$errors): bool { + $filled = true; + foreach ($cells as $field => $value) { + if ($value === '') { + $errors[] = $this->error(line: $line, field: $field, code: 'empty', message: "Line {$line}: {$field} is empty."); + $filled = false; + } + } + + return $filled; + }//end filled() + + /** + * A whole number of 1 or more, or null with an error; an empty cell is left to filled(). + * + * @param string $value The cell. + * @param int $line The line number. + * @param string $field The column. + * @param array $errors Errors, appended to. + * + * @return int|null + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + private function wholeNumber(string $value, int $line, string $field, array &$errors): ?int { + if (preg_match('/^\d+$/', $value) === 1 && (int)$value >= 1) { + return (int)$value; + } + + if ($value !== '') { + $errors[] = $this->error(line: $line, field: $field, code: 'whole-number', message: "Line {$line}: {$field} must be a whole number of 1 or more."); + } + + return null; + }//end wholeNumber() + + /** + * Whether a room type is one of the rooms' types; an unknown one adds an error. + * + * @param string $type The cell. + * @param array $roomTypes The known types. + * @param int $line The line number. + * @param array $errors Errors, appended to. + * + * @return bool + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + private function knownRoomType(string $type, array $roomTypes, int $line, array &$errors): bool { + if ($type === '' || in_array($type, $roomTypes, true) === true) { + return true; + } + + $errors[] = $this->error(line: $line, field: 'room type', code: 'unknown-room-type', message: "Line {$line}: room type {$type} is not on the rooms sheet."); + return false; + }//end knownRoomType() + + /** + * One refusal. + * + * @param int $line The line number. + * @param string $field The column, empty for the header. + * @param string $code The refusal code. + * @param string $message A readable refusal. + * + * @return array{line:int,field:string,code:string,message:string} + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + private function error(int $line, string $field, string $code, string $message): array { + return ['line' => $line, 'field' => $field, 'code' => $code, 'message' => $message]; + }//end error() +}//end class diff --git a/lib/Service/TimetableInputBuilder.php b/lib/Service/TimetableInputBuilder.php new file mode 100644 index 00000000..5526c9b3 --- /dev/null +++ b/lib/Service/TimetableInputBuilder.php @@ -0,0 +1,202 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Service; + +use OCA\Planninq\AppInfo\Application; +use OCA\Planninq\Event\TimetableActivitiesQueryEvent; +use OCA\Planninq\Timetabling\SolverInput; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\IAppConfig; + +/** + * Builds the SolverInput of a generator run. + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ +class TimetableInputBuilder { + + /** + * App config key holding the uploaded rooms sheet, parsed. + * + * @var string + */ + public const CSV_ROOMS_KEY = 'timetable_csv_rooms'; + + /** + * App config key holding the uploaded activities sheet, parsed. + * + * @var string + */ + public const CSV_ACTIVITIES_KEY = 'timetable_csv_activities'; + + /** + * Why an input is empty when nothing supplied activities. + * + * @var string + */ + public const NO_ACTIVITIES = 'No activities: learniq did not answer and no CSV was uploaded'; + + /** + * The wish fields the solver reads. + * + * @var array + */ + private const WISH_FIELDS = ['appliesTo', 'reference', 'kind', 'periods', 'limit', 'strength', 'weight']; + + /** + * Constructor. + * + * @param IEventDispatcher $dispatcher Dispatches the activities query. + * @param IAppConfig $appConfig Holds the uploaded sheets. + * @param TimetableGridService $grid The week grid. + * + * @return void + */ + public function __construct( + private readonly IEventDispatcher $dispatcher, + private readonly IAppConfig $appConfig, + private readonly TimetableGridService $grid, + ) { + }//end __construct() + + /** + * The input of a run for one academic year and the given wishes. + * + * @param string $academicYear The academic year, such as `2026-2027`. + * @param array> $wishes timetableWish objects. + * + * @return SolverInput + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function build(string $academicYear, array $wishes): SolverInput { + $periods = $this->grid->periodKeys(); + $wishes = array_map(fn (array $wish): array => $this->wish(wish: $wish), array_values($wishes)); + + $event = new TimetableActivitiesQueryEvent(academicYear: $academicYear); + $this->dispatcher->dispatchTyped($event); + + $activities = ($event->getActivities() ?? []); + $rooms = $event->getRooms(); + $source = (string)$event->getAnsweredBy(); + if ($activities === []) { + $activities = $this->stored(key: self::CSV_ACTIVITIES_KEY); + $rooms = $this->stored(key: self::CSV_ROOMS_KEY); + $source = 'csv'; + } + + if ($activities === []) { + return new SolverInput(periods: $periods, rooms: [], lessons: [], wishes: $wishes, source: 'none', reason: self::NO_ACTIVITIES); + } + + return new SolverInput(periods: $periods, rooms: array_values($rooms), lessons: $this->lessons(activities: $activities), wishes: $wishes, source: $source); + }//end build() + + /** + * Keep an uploaded rooms and activities sheet as the input for runs no app answers. + * + * @param array> $rooms Parsed room rows. + * @param array> $activities Parsed activity rows. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function storeUpload(array $rooms, array $activities): void { + $this->appConfig->setValueString(Application::APP_ID, self::CSV_ROOMS_KEY, (string)json_encode(array_values($rooms)), lazy: true); + $this->appConfig->setValueString(Application::APP_ID, self::CSV_ACTIVITIES_KEY, (string)json_encode(array_values($activities)), lazy: true); + }//end storeUpload() + + /** + * One lesson per weekly lesson of each activity, keyed `{group}:{subject}:{n}`. + * + * @param array> $activities Activity rows. + * + * @return array> + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + private function lessons(array $activities): array { + $lessons = []; + foreach ($activities as $activity) { + $key = ((string)($activity['group'] ?? '')).':'.((string)($activity['subject'] ?? '')); + $count = max(0, (int)($activity['lessonsPerWeek'] ?? 0)); + for ($number = 1; $number <= $count; $number++) { + $lessons[] = [ + 'key' => $key.':'.$number, + 'activity' => $key, + 'group' => (string)($activity['group'] ?? ''), + 'subject' => (string)($activity['subject'] ?? ''), + 'teacher' => (string)($activity['teacher'] ?? ''), + 'roomType' => (string)($activity['roomType'] ?? ''), + 'length' => max(1, (int)($activity['lessonLength'] ?? 1)), + ]; + } + } + + return $lessons; + }//end lessons() + + /** + * A wish as the solver reads it: its id and the fields that constrain. + * + * @param array $wish A timetableWish object. + * + * @return array + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + private function wish(array $wish): array { + $out = ['id' => (string)($wish['id'] ?? ($wish['@self']['id'] ?? ''))]; + foreach (self::WISH_FIELDS as $field) { + if (array_key_exists($field, $wish) === true) { + $out[$field] = $wish[$field]; + } + } + + return $out; + }//end wish() + + /** + * A stored sheet, or an empty list. + * + * @param string $key The app config key. + * + * @return array> + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + private function stored(string $key): array { + $rows = json_decode($this->appConfig->getValueString(Application::APP_ID, $key, '[]', lazy: true), true); + if (is_array($rows) === false) { + return []; + } + + return array_values(array_filter($rows, 'is_array')); + }//end stored() +}//end class diff --git a/lib/Timetabling/SolverInput.php b/lib/Timetabling/SolverInput.php new file mode 100644 index 00000000..acab68ee --- /dev/null +++ b/lib/Timetabling/SolverInput.php @@ -0,0 +1,85 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Timetabling; + +/** + * The input of one generator run. + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ +final class SolverInput { + + /** + * Constructor. + * + * @param array $periods Period keys, such as `mon-1`. + * @param array> $rooms Rooms: reference, capacity, type. + * @param array> $lessons Lessons: key, activity, group, subject, teacher, roomType, length. + * @param array> $wishes Wishes: id, appliesTo, reference, kind, periods, limit, strength, weight. + * @param string $source Where the activities came from: an app id, `csv` or `none`. + * @param string|null $reason Why the input is empty, or null. + * + * @return void + */ + public function __construct( + public readonly array $periods, + public readonly array $rooms, + public readonly array $lessons, + public readonly array $wishes, + public readonly string $source, + public readonly ?string $reason=null, + ) { + }//end __construct() + + /** + * Whether there is nothing to place. + * + * @return bool + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function isEmpty(): bool { + return $this->lessons === []; + }//end isEmpty() + + /** + * The shape a timetableScenario keeps under `input`. + * + * @return array{periods:array,rooms:array>,lessons:array>,wishes:array>,source:string} + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function toArray(): array { + return [ + 'periods' => $this->periods, + 'rooms' => $this->rooms, + 'lessons' => $this->lessons, + 'wishes' => $this->wishes, + 'source' => $this->source, + ]; + }//end toArray() +}//end class diff --git a/openspec/changes/timetabling-generator/tasks.md b/openspec/changes/timetabling-generator/tasks.md index daae39bb..c4383c4e 100644 --- a/openspec/changes/timetabling-generator/tasks.md +++ b/openspec/changes/timetabling-generator/tasks.md @@ -10,9 +10,9 @@ DECISIONS.md row 17). Sections 5 onward assume the solver answer recorded in des ## 2. Activities and the solver input -- [ ] 2.1 `TimetableActivitiesQueryEvent` (typed, ADR-041) and `TimetableInputBuilder` that turns activities, rooms, the grid and the wishes into a `SolverInput`. Verify: PHPUnit on the builder with a fixture; an event nobody answers yields an empty input and the reason "No activities: learniq did not answer and no CSV was uploaded". -- [ ] 2.2 CSV upload of activities and rooms (same columns as the event), admin only. Verify: PHPUnit on the parser (header check, lessons per week and lesson length as whole numbers, unknown room type refused with the line number); controller refuses a non-admin. -- [ ] 2.3 Draft the learniq listener contract to `~/memcap-work/build-all/for-ruben/learniq-timetable-activities-event.md`. Verify: the file names the event class, its fields and the learniq endpoint it mirrors. +- [x] 2.1 `TimetableActivitiesQueryEvent` (typed, ADR-041) and `TimetableInputBuilder` that turns activities, rooms, the grid and the wishes into a `SolverInput`. Verify: PHPUnit on the builder with a fixture; an event nobody answers yields an empty input and the reason "No activities: learniq did not answer and no CSV was uploaded". +- [x] 2.2 CSV upload of activities and rooms (same columns as the event), admin only. Verify: PHPUnit on the parser (header check, lessons per week and lesson length as whole numbers, unknown room type refused with the line number); controller refuses a non-admin. +- [x] 2.3 Draft the learniq listener contract to `~/memcap-work/build-all/for-ruben/learniq-timetable-activities-event.md`. Verify: the file names the event class, its fields and the learniq endpoint it mirrors. ## 3. Wishes diff --git a/tests/unit/Controller/TimetableInputControllerTest.php b/tests/unit/Controller/TimetableInputControllerTest.php new file mode 100644 index 00000000..8f15c62b --- /dev/null +++ b/tests/unit/Controller/TimetableInputControllerTest.php @@ -0,0 +1,146 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Tests\Unit\Controller; + +use OCA\Planninq\Controller\TimetableInputController; +use OCA\Planninq\Service\SettingsService; +use OCA\Planninq\Service\TimetableCsvParser; +use OCA\Planninq\Service\TimetableGridService; +use OCA\Planninq\Service\TimetableInputBuilder; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\IAppConfig; +use OCP\IRequest; +use PHPUnit\Framework\TestCase; +use ReflectionMethod; + +/** + * Admins only; nothing is kept when a line is refused. + */ +class TimetableInputControllerTest extends TestCase { + + /** + * The app config values, in memory. + * + * @var array + */ + private array $stored = []; + + /** + * A controller with the real parser and builder, the given body and caller. + * + * @param array $body The request parameters. + * @param bool $admin Whether the caller is an admin. + * + * @return TimetableInputController + */ + private function controller(array $body, bool $admin): TimetableInputController { + $request = $this->createMock(IRequest::class); + $request->method('getParam')->willReturnCallback(static fn (string $key): mixed => ($body[$key] ?? null)); + + $config = $this->createMock(IAppConfig::class); + $config->method('getValueString')->willReturnCallback( + fn (string $app, string $key, string $default = '', bool $lazy = false): string => ($this->stored[$key] ?? $default) + ); + $config->method('setValueString')->willReturnCallback( + function (string $app, string $key, string $value, bool $lazy = false): bool { + $this->stored[$key] = $value; + return true; + } + ); + + $settings = $this->createMock(SettingsService::class); + $settings->method('isCurrentUserAdmin')->willReturn($admin); + + return new TimetableInputController( + request: $request, + parser: new TimetableCsvParser(), + inputBuilder: new TimetableInputBuilder(dispatcher: $this->createMock(IEventDispatcher::class), appConfig: $config, grid: new TimetableGridService(appConfig: $config)), + settingsService: $settings, + ); + }//end controller() + + /** + * The two sheets of the spec's school. + * + * @return array + */ + private function sheets(): array { + return [ + 'rooms' => "reference,capacity,type\nr-101,30,classroom\n", + 'activities' => "group,subject,teacher,lessons per week,lesson length,room type\n3A,English,klaas,3,1,classroom\n3A,Maths,noor,2,2,classroom\n", + ]; + }//end sheets() + + /** + * An admin uploads both sheets: they are kept and counted. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function testAnAdminUploadIsKept(): void { + $response = $this->controller(body: $this->sheets(), admin: true)->upload(); + + self::assertSame(expected: Http::STATUS_OK, actual: $response->getStatus()); + self::assertSame(expected: ['rooms' => 1, 'activities' => 2, 'lessons' => 5], actual: $response->getData()); + self::assertCount(expectedCount: 2, haystack: json_decode($this->stored['timetable_csv_activities'], true)); + self::assertSame(expected: 'r-101', actual: json_decode($this->stored['timetable_csv_rooms'], true)[0]['reference']); + }//end testAnAdminUploadIsKept() + + /** + * A non-admin is refused and nothing is kept; the route is admin-only. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function testANonAdminIsRefused(): void { + $response = $this->controller(body: $this->sheets(), admin: false)->upload(); + + self::assertSame(expected: Http::STATUS_FORBIDDEN, actual: $response->getStatus()); + self::assertSame(expected: [], actual: $this->stored); + self::assertNotSame(expected: [], actual: (new ReflectionMethod(TimetableInputController::class, 'upload'))->getAttributes(AuthorizedAdminSetting::class)); + }//end testANonAdminIsRefused() + + /** + * A refused line keeps nothing and answers with the lines. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function testARefusedLineKeepsNothing(): void { + $body = ['activities' => "group,subject,teacher,lessons per week,lesson length,room type\n3A,Swimming,piet,1,1,pool\n"] + $this->sheets(); + $response = $this->controller(body: $body, admin: true)->upload(); + + self::assertSame(expected: Http::STATUS_BAD_REQUEST, actual: $response->getStatus()); + self::assertSame(expected: 'unknown-room-type', actual: $response->getData()['errors']['activities'][0]['code']); + self::assertSame(expected: 2, actual: $response->getData()['errors']['activities'][0]['line']); + self::assertSame(expected: [], actual: $this->stored); + + $missing = $this->controller(body: ['rooms' => 'x'], admin: true)->upload(); + self::assertSame(expected: Http::STATUS_BAD_REQUEST, actual: $missing->getStatus()); + }//end testARefusedLineKeepsNothing() +}//end class diff --git a/tests/unit/Service/TimetableCsvParserTest.php b/tests/unit/Service/TimetableCsvParserTest.php new file mode 100644 index 00000000..675fdbf0 --- /dev/null +++ b/tests/unit/Service/TimetableCsvParserTest.php @@ -0,0 +1,120 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Tests\Unit\Service; + +use OCA\Planninq\Service\TimetableCsvParser; +use PHPUnit\Framework\TestCase; + +/** + * Header check, whole numbers and room types, each refusal with its line. + */ +class TimetableCsvParserTest extends TestCase { + + /** + * A good sheet: header case and spaces ignored, semicolons accepted, blank lines skipped. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function testAGoodSheetBecomesRows(): void { + $parser = new TimetableCsvParser(); + $rooms = $parser->parseRooms(csv: "Reference,Capacity,Type\nr-101,30,classroom\n\nlab-1,16,lab\n"); + self::assertSame(expected: [], actual: $rooms['errors']); + self::assertSame(expected: [['reference' => 'r-101', 'capacity' => 30, 'type' => 'classroom'], ['reference' => 'lab-1', 'capacity' => 16, 'type' => 'lab']], actual: $rooms['rows']); + + $activities = $parser->parseActivities(csv: " Group ; Subject;Teacher;Lessons per week;Lesson length;Room type\r\n3A;Chemistry;noor;2;2;lab\r\n", roomTypes: ['classroom', 'lab']); + self::assertSame(expected: [], actual: $activities['errors']); + self::assertSame( + expected: [['group' => '3A', 'subject' => 'Chemistry', 'teacher' => 'noor', 'lessonsPerWeek' => 2, 'lessonLength' => 2, 'roomType' => 'lab']], + actual: $activities['rows'] + ); + }//end testAGoodSheetBecomesRows() + + /** + * A sheet whose first line is not the header is refused as a whole. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function testAWrongHeaderIsRefused(): void { + $result = (new TimetableCsvParser())->parseActivities(csv: "group,subject,teacher,hours\n3A,English,klaas,3\n", roomTypes: ['classroom']); + + self::assertSame(expected: [], actual: $result['rows']); + self::assertCount(expectedCount: 1, haystack: $result['errors']); + self::assertSame(expected: 1, actual: $result['errors'][0]['line']); + self::assertSame(expected: 'header', actual: $result['errors'][0]['code']); + self::assertSame(expected: 'The first line must be: group, subject, teacher, lessons per week, lesson length, room type.', actual: $result['errors'][0]['message']); + }//end testAWrongHeaderIsRefused() + + /** + * Lessons per week and lesson length must be whole numbers of 1 or more; an unknown room type is refused; each names its line. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function testEachRefusalNamesItsLine(): void { + $csv = implode( + "\n", + [ + 'group,subject,teacher,lessons per week,lesson length,room type', + '3A,English,klaas,3,1,classroom', + '3A,Maths,noor,three,1,classroom', + '3B,Maths,noor,2,1.5,classroom', + '3B,Music,piet,0,1,classroom', + '3B,Swimming,piet,1,2,pool', + '3C,,piet,1,1,classroom', + ] + ); + $result = (new TimetableCsvParser())->parseActivities(csv: $csv, roomTypes: ['classroom']); + + self::assertSame(expected: ['3A:English'], actual: array_map(static fn (array $row): string => $row['group'].':'.$row['subject'], $result['rows'])); + self::assertSame( + expected: [ + [3, 'lessons per week', 'whole-number'], + [4, 'lesson length', 'whole-number'], + [5, 'lessons per week', 'whole-number'], + [6, 'room type', 'unknown-room-type'], + [7, 'subject', 'empty'], + ], + actual: array_map(static fn (array $error): array => [$error['line'], $error['field'], $error['code']], $result['errors']) + ); + self::assertSame(expected: 'Line 6: room type pool is not on the rooms sheet.', actual: $result['errors'][3]['message']); + }//end testEachRefusalNamesItsLine() + + /** + * A room capacity must be a whole number too. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function testARoomCapacityMustBeAWholeNumber(): void { + $result = (new TimetableCsvParser())->parseRooms(csv: "reference,capacity,type\nr-101,thirty,classroom\n"); + + self::assertSame(expected: [], actual: $result['rows']); + self::assertSame(expected: 'Line 2: capacity must be a whole number of 1 or more.', actual: $result['errors'][0]['message']); + }//end testARoomCapacityMustBeAWholeNumber() +}//end class diff --git a/tests/unit/Service/TimetableInputBuilderTest.php b/tests/unit/Service/TimetableInputBuilderTest.php new file mode 100644 index 00000000..4e9cbbc4 --- /dev/null +++ b/tests/unit/Service/TimetableInputBuilderTest.php @@ -0,0 +1,183 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Tests\Unit\Service; + +require_once __DIR__ . '/../Support/RegisterSchemaValidation.php'; + +use OCA\Planninq\Event\TimetableActivitiesQueryEvent; +use OCA\Planninq\Service\TimetableGridService; +use OCA\Planninq\Service\TimetableInputBuilder; +use OCA\Planninq\Tests\Unit\Support\RegisterSchemaValidation; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\IAppConfig; +use PHPUnit\Framework\TestCase; + +/** + * The builder asks the hour plan's owner through the real event, falls back to + * the uploaded sheets, and says why an input is empty. + */ +class TimetableInputBuilderTest extends TestCase { + use RegisterSchemaValidation; + + /** + * The app config values, in memory. + * + * @var array + */ + private array $stored = []; + + /** + * A builder whose dispatcher runs $listener on the real event, as learniq's listener would. + * + * @param callable|null $listener Receives the TimetableActivitiesQueryEvent, or null for nobody listening. + * + * @return TimetableInputBuilder + */ + private function builder(?callable $listener): TimetableInputBuilder { + $dispatcher = $this->createMock(IEventDispatcher::class); + $dispatcher->method('dispatchTyped')->willReturnCallback( + static function (object $event) use ($listener): void { + if ($listener !== null && $event instanceof TimetableActivitiesQueryEvent) { + $listener($event); + } + } + ); + + $config = $this->createMock(IAppConfig::class); + $config->method('getValueString')->willReturnCallback( + fn (string $app, string $key, string $default = '', bool $lazy = false): string => ($this->stored[$key] ?? $default) + ); + $config->method('setValueString')->willReturnCallback( + function (string $app, string $key, string $value, bool $lazy = false): bool { + $this->stored[$key] = $value; + return true; + } + ); + + return new TimetableInputBuilder(dispatcher: $dispatcher, appConfig: $config, grid: new TimetableGridService(appConfig: $config)); + }//end builder() + + /** + * Learniq's hour plan fixture: 3A has three English lessons and two double maths lessons. + * + * @return array{0:array>,1:array>} + */ + private function hourPlan(): array { + return [ + [ + ['group' => '3A', 'subject' => 'English', 'teacher' => 'klaas', 'lessonsPerWeek' => 3, 'lessonLength' => 1, 'roomType' => 'classroom'], + ['group' => '3A', 'subject' => 'Maths', 'teacher' => 'noor', 'lessonsPerWeek' => 2, 'lessonLength' => 2, 'roomType' => 'classroom'], + ], + [['reference' => 'r-101', 'capacity' => 30, 'type' => 'classroom']], + ]; + }//end hourPlan() + + /** + * An answered event gives one lesson per weekly lesson, the grid's forty periods and the wishes with their ids. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function testAnAnsweredEventBecomesLessons(): void { + [$activities, $rooms] = $this->hourPlan(); + $asked = null; + $input = $this->builder( + static function (TimetableActivitiesQueryEvent $event) use ($activities, $rooms, &$asked): void { + $asked = $event->getAcademicYear(); + $event->answer(app: 'learniq', activities: $activities, rooms: $rooms); + } + )->build( + academicYear: '2026-2027', + wishes: [['@self' => ['id' => 'w-1'], 'appliesTo' => 'teacher', 'reference' => 'klaas', 'kind' => 'unavailable', 'periods' => ['wed-5'], 'strength' => 'hard', 'note' => 'not needed']] + ); + + self::assertSame(expected: '2026-2027', actual: $asked); + self::assertSame(expected: 'learniq', actual: $input->source); + self::assertNull(actual: $input->reason); + self::assertCount(expectedCount: 40, haystack: $input->periods); + self::assertSame(expected: ['mon-1', 'mon-2'], actual: array_slice($input->periods, 0, 2)); + self::assertSame(expected: 'fri-8', actual: $input->periods[39]); + self::assertSame(expected: $rooms, actual: $input->rooms); + self::assertSame(expected: ['3A:English:1', '3A:English:2', '3A:English:3', '3A:Maths:1', '3A:Maths:2'], actual: array_column($input->lessons, 'key')); + self::assertSame( + expected: ['key' => '3A:Maths:1', 'activity' => '3A:Maths', 'group' => '3A', 'subject' => 'Maths', 'teacher' => 'noor', 'roomType' => 'classroom', 'length' => 2], + actual: $input->lessons[3] + ); + self::assertSame( + expected: [['id' => 'w-1', 'appliesTo' => 'teacher', 'reference' => 'klaas', 'kind' => 'unavailable', 'periods' => ['wed-5'], 'strength' => 'hard']], + actual: $input->wishes + ); + + $scenario = ['title' => 'Autumn', 'source' => 'generated', 'weekOf' => '2026-10-05', 'windowFrom' => '2026-10-05', 'windowTo' => '2026-10-30', 'status' => 'queued', 'input' => $input->toArray()]; + self::assertSame(expected: [], actual: $this->registerSchemaErrors(slug: 'timetableScenario', payload: $scenario), message: 'the input fits the scenario schema'); + }//end testAnAnsweredEventBecomesLessons() + + /** + * Nobody answers and nothing was uploaded: an empty input and the reason. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function testAnUnansweredEventWithoutUploadGivesTheReason(): void { + $input = $this->builder(listener: null)->build(academicYear: '2026-2027', wishes: []); + + self::assertTrue(condition: $input->isEmpty()); + self::assertSame(expected: 'none', actual: $input->source); + self::assertSame(expected: 'No activities: learniq did not answer and no CSV was uploaded', actual: $input->reason); + self::assertCount(expectedCount: 40, haystack: $input->periods); + }//end testAnUnansweredEventWithoutUploadGivesTheReason() + + /** + * Nobody answers but an admin uploaded sheets: those are the input. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.2 + */ + public function testUploadedSheetsAreTheFallback(): void { + [$activities, $rooms] = $this->hourPlan(); + $builder = $this->builder(listener: null); + $builder->storeUpload(rooms: $rooms, activities: $activities); + + $input = $builder->build(academicYear: '2026-2027', wishes: []); + self::assertSame(expected: 'csv', actual: $input->source); + self::assertCount(expectedCount: 5, haystack: $input->lessons); + self::assertSame(expected: $rooms, actual: $input->rooms); + }//end testUploadedSheetsAreTheFallback() + + /** + * A narrower grid gives fewer period keys. + * + * @return void + * + * @spec openspec/changes/timetabling-generator/tasks.md#task-2.1 + */ + public function testThePeriodsFollowTheStoredGrid(): void { + $this->stored['timetable_period_grid'] = '{"days":["mon","wed"],"periods":[{"start":"08:00","end":"08:45"},{"start":"08:45","end":"09:30"}]}'; + $input = $this->builder(listener: null)->build(academicYear: '2026-2027', wishes: []); + + self::assertSame(expected: ['mon-1', 'mon-2', 'wed-1', 'wed-2'], actual: $input->periods); + }//end testThePeriodsFollowTheStoredGrid() +}//end class From e40b82c9584125eefd36a032941993380d4e60b1 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Wed, 30 Sep 2026 00:02:46 +0200 Subject: [PATCH 2/2] style(timetable): two lines under 150 characters (phpcs) --- lib/Service/TimetableCsvParser.php | 3 ++- lib/Service/TimetableInputBuilder.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Service/TimetableCsvParser.php b/lib/Service/TimetableCsvParser.php index f2ce499f..0c00df08 100644 --- a/lib/Service/TimetableCsvParser.php +++ b/lib/Service/TimetableCsvParser.php @@ -242,7 +242,8 @@ private function knownRoomType(string $type, array $roomTypes, int $line, array return true; } - $errors[] = $this->error(line: $line, field: 'room type', code: 'unknown-room-type', message: "Line {$line}: room type {$type} is not on the rooms sheet."); + $message = "Line {$line}: room type {$type} is not on the rooms sheet."; + $errors[] = $this->error(line: $line, field: 'room type', code: 'unknown-room-type', message: $message); return false; }//end knownRoomType() diff --git a/lib/Service/TimetableInputBuilder.php b/lib/Service/TimetableInputBuilder.php index 5526c9b3..02c6d4a2 100644 --- a/lib/Service/TimetableInputBuilder.php +++ b/lib/Service/TimetableInputBuilder.php @@ -114,7 +114,8 @@ public function build(string $academicYear, array $wishes): SolverInput { return new SolverInput(periods: $periods, rooms: [], lessons: [], wishes: $wishes, source: 'none', reason: self::NO_ACTIVITIES); } - return new SolverInput(periods: $periods, rooms: array_values($rooms), lessons: $this->lessons(activities: $activities), wishes: $wishes, source: $source); + $lessons = $this->lessons(activities: $activities); + return new SolverInput(periods: $periods, rooms: array_values($rooms), lessons: $lessons, wishes: $wishes, source: $source); }//end build() /**