From 0cbc3e9aa542225f2408d4dffa99e68918df6419 Mon Sep 17 00:00:00 2001 From: Jeppe Bundgaard Date: Thu, 7 May 2026 13:50:32 +0200 Subject: [PATCH] Add support for archived departments with schema updates, API integration, and filtering logic - Added `archived` column and index to `departments` table, ensuring schema initialization via `departments_schema_bootstrap`. - Updated OpenAPI spec to include `archived` attribute and `filters=archived` query parameter with superuser access control. - Enhanced `Departments` API to support archived department filtering and retrieval. - Modified `ApiFixtures`, `departments_o`, and related tests to validate behavior for archived departments. - Added unit and API tests to ensure correct handling of archived departments and filter enforceability. --- openapi.yaml | 13 ++- ...ily_report_complaints_schema_bootstrap.php | 4 +- .../classes/departments_schema_bootstrap.php | 89 +++++++++++++++++++ services/nginx/app/objects/departments_o.php | 5 ++ .../nginx/app/routes/departmentsRoute.php | 66 +++++++++++++- .../app/tests/Api/DepartmentsApiTest.php | 83 ++++++++++++++++- .../app/tests/Support/Api/ApiFixtures.php | 1 + .../tests/Support/Api/ApiSchemaBootstrap.php | 41 ++++++++- 8 files changed, 295 insertions(+), 7 deletions(-) create mode 100644 services/nginx/app/classes/departments_schema_bootstrap.php diff --git a/openapi.yaml b/openapi.yaml index 5d218417..a92c0d9e 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -3395,7 +3395,7 @@ paths: tags: - Departments summary: List departments - description: Retrieve a list of all visible departments + description: Retrieve visible, active departments by default. Superuser department access may filter archived departments with `filters=archived:1`. operationId: listDepartments parameters: - name: id @@ -3406,6 +3406,11 @@ paths: - $ref: '#/components/parameters/PageParam' - $ref: '#/components/parameters/PerPageParam' - $ref: '#/components/parameters/SearchParam' + - name: filters + in: query + schema: + type: string + description: Comma-separated field filters. `archived:1` is only honored for users with superuser department access. responses: '200': description: Departments retrieved successfully @@ -15607,6 +15612,8 @@ components: type: integer visible: type: boolean + archived: + type: boolean dimension: type: integer branding: @@ -15677,6 +15684,8 @@ components: type: integer visible: type: boolean + archived: + type: boolean longitude: type: number format: float @@ -15697,6 +15706,8 @@ components: type: string visible: type: boolean + archived: + type: boolean longitude: type: number format: float diff --git a/services/nginx/app/classes/department_daily_report_complaints_schema_bootstrap.php b/services/nginx/app/classes/department_daily_report_complaints_schema_bootstrap.php index 91b4edd0..716320c8 100644 --- a/services/nginx/app/classes/department_daily_report_complaints_schema_bootstrap.php +++ b/services/nginx/app/classes/department_daily_report_complaints_schema_bootstrap.php @@ -76,11 +76,13 @@ class department_daily_report_complaints_schema_bootstrap dimension INT NOT NULL DEFAULT 0, branding INT NOT NULL DEFAULT 0, visible TINYINT(1) NOT NULL DEFAULT 1, + archived TINYINT(1) NOT NULL DEFAULT 0, longitude DECIMAL(10,7) NOT NULL DEFAULT 0, latitude DECIMAL(10,7) NOT NULL DEFAULT 0, order_priority INT NOT NULL DEFAULT 0, created_at DATETIME NULL DEFAULT CURRENT_TIMESTAMP, - updated_at DATETIME NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP + updated_at DATETIME NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + KEY idx_departments_archived (archived) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci" ); diff --git a/services/nginx/app/classes/departments_schema_bootstrap.php b/services/nginx/app/classes/departments_schema_bootstrap.php new file mode 100644 index 00000000..c6e0eed6 --- /dev/null +++ b/services/nginx/app/classes/departments_schema_bootstrap.php @@ -0,0 +1,89 @@ +query( + "ALTER TABLE departments + ADD COLUMN archived TINYINT(1) NOT NULL DEFAULT 0 + AFTER visible" + ); + } + + if (!self::indexExists($db, 'departments', self::ARCHIVED_INDEX)) { + $db->query( + "ALTER TABLE departments + ADD INDEX " . self::ARCHIVED_INDEX . " (archived)" + ); + } + + self::$initialized = true; + } + + private static function tableExists(object $db, string $table): bool + { + $table = self::escapeIdentifier($table); + $result = $db->query("SHOW TABLES LIKE '{$table}'"); + + if ($result === false || !is_object($result) || !property_exists($result, 'num_rows')) { + return false; + } + + return (int)$result->num_rows > 0; + } + + private static function columnExists(object $db, string $table, string $column): bool + { + $table = self::escapeIdentifier($table); + $column = self::escapeIdentifier($column); + $result = $db->query("SHOW COLUMNS FROM `{$table}` LIKE '{$column}'"); + + if ($result === false || !is_object($result) || !property_exists($result, 'num_rows')) { + return false; + } + + return (int)$result->num_rows > 0; + } + + private static function indexExists(object $db, string $table, string $index): bool + { + $table = self::escapeIdentifier($table); + $index = self::escapeIdentifier($index); + $result = $db->query("SHOW INDEX FROM `{$table}` WHERE Key_name = '{$index}'"); + + if ($result === false || !is_object($result) || !property_exists($result, 'num_rows')) { + return false; + } + + return (int)$result->num_rows > 0; + } + + private static function escapeIdentifier(string $value): string + { + return str_replace(['\\', "'", '`'], ['\\\\', "\\'", ''], $value); + } +} diff --git a/services/nginx/app/objects/departments_o.php b/services/nginx/app/objects/departments_o.php index 9dc3f979..9baaea50 100644 --- a/services/nginx/app/objects/departments_o.php +++ b/services/nginx/app/objects/departments_o.php @@ -3,6 +3,7 @@ namespace objects; use classes\db; +use classes\departments_schema_bootstrap; use classes\object_property; use classes\slack; use classes\stripe; @@ -20,6 +21,7 @@ class departments_o extends db public department_variables_o $variables; // The department variables object public object_property $dimension; // The dimension of the department public object_property $visible; // The visibility of the department + public object_property $archived; // Whether the department is archived public object_property $branding; // The branding of the department public object_property $longitude; // The longitude of the department (Can be null) public object_property $latitude; // The latitude of the department (Can be null) @@ -29,6 +31,7 @@ class departments_o extends db public function structure(): void { + departments_schema_bootstrap::ensureTables(); $this->setTable('departments'); } @@ -103,6 +106,7 @@ class departments_o extends db $this->dimension = new object_property($this->table, $this->id, 'dimension', 'int', false); $this->branding = new object_property($this->table, $this->id, 'branding', 'int', false); $this->visible = new object_property($this->table, $this->id, 'visible', 'int', false); + $this->archived = new object_property($this->table, $this->id, 'archived', 'boolean', false); $this->longitude = new object_property($this->table, $this->id, 'longitude', 'float', false); $this->latitude = new object_property($this->table, $this->id, 'latitude', 'float', false); $this->order_priority = new object_property($this->table, $this->id, 'order_priority', 'int', false); @@ -156,6 +160,7 @@ class departments_o extends db 'description' => $department['description'], 'id' => $department['id'], 'visible' => $department['visible'], + 'archived' => $department['archived'] ?? 0, ]; }, $departments); } diff --git a/services/nginx/app/routes/departmentsRoute.php b/services/nginx/app/routes/departmentsRoute.php index 250a415a..83ac2a65 100644 --- a/services/nginx/app/routes/departmentsRoute.php +++ b/services/nginx/app/routes/departmentsRoute.php @@ -17,6 +17,59 @@ class departmentsRoute { use route_t; + private function buildDepartmentListFilters(departments_o $departments, bool $canListArchived): string + { + global $response; + + $filters = $response->getRequestParameter('filters') ?? []; + if (is_string($filters) || is_array($filters)) { + $filters = $departments->filter_string_to_array($filters); + } else { + $filters = []; + } + + $archived = 0; + if ( + $canListArchived + && array_key_exists('archived', $filters) + && self::isTruthyBooleanValue($filters['archived']) + ) { + $archived = 1; + } + + unset($filters['visible'], $filters['archived']); + $filters['visible'] = 1; + $filters['archived'] = $archived; + + return $departments->array_to_filters($filters); + } + + private static function isTruthyBooleanValue(mixed $value): bool + { + if (is_array($value)) { + foreach ($value as $singleValue) { + if (self::isTruthyBooleanValue($singleValue)) { + return true; + } + } + return false; + } + + if (is_bool($value)) { + return $value; + } + + if (is_numeric($value)) { + return (int)$value === 1; + } + + if (is_string($value)) { + return in_array(strtolower(trim($value)), ['1', 'true', 'yes', 'on'], true); + } + + return false; + } + public function run(): void { $this->get('/departments', function () { @@ -49,6 +102,7 @@ class departmentsRoute 'description', 'economic_department_id', 'visible', + 'archived', 'longitude', 'latitude', ]) @@ -63,6 +117,7 @@ class departmentsRoute 'updated_at' => (string)$department['updated_at'], 'dimension' => (int)$department['dimension'], 'branding' => (int)$department['branding'], + 'archived' => (bool)(int)($department['archived'] ?? 0), 'longitude' => (float)$department['longitude'], 'latitude' => (float)$department['latitude'], 'order_priority' => (int)$department['order_priority'], @@ -73,9 +128,10 @@ class departmentsRoute } return $tmp_department; }, - $departments_o->forceRestrictFilters([ - 'visible' => 1, // Only show visible departments, this is to prevent showing internal system departments to the end-user. - ]) + $this->buildDepartmentListFilters( + $departments_o, + $user->hasPermission('superuser_fetch_department') + ) ) ); } else { @@ -160,6 +216,10 @@ class departmentsRoute if (self::isParametersSet(['order_priority'])) { $department->order_priority->set((int)self::getParameter('order_priority')); } + if (self::isParametersSet(['archived'])) { + $department->archived->set(self::isTruthyBooleanValue(self::getParameter('archived'))); + } + $department->objectChanged(); // Log the incident (new logs_o())->add('departments', (int)self::getParameter('id'), 1, $user->id, 'EDIT_DEPARTMENT', 'Successfully edited a department'); // Return a success message diff --git a/services/nginx/app/tests/Api/DepartmentsApiTest.php b/services/nginx/app/tests/Api/DepartmentsApiTest.php index 5234a477..2f4cb73a 100644 --- a/services/nginx/app/tests/Api/DepartmentsApiTest.php +++ b/services/nginx/app/tests/Api/DepartmentsApiTest.php @@ -20,6 +20,11 @@ it('lists only visible departments and can return a single department with the s 'name' => 'Hidden Department', 'visible' => 0, ]); + $archivedDepartment = api_fixtures()->createDepartment([ + 'name' => 'Archived Department', + 'visible' => 1, + 'archived' => 1, + ]); $webhookDepartment = api_fixtures()->createDepartment([ 'name' => 'Webhook Department', 'slack_webhook' => 'https://hooks.slack.test/example', @@ -41,7 +46,8 @@ it('lists only visible departments and can return a single department with the s expect($departmentIds) ->toContain($visibleDepartment['id']) ->toContain($webhookDepartment['id']) - ->not->toContain($hiddenDepartment['id']); + ->not->toContain($hiddenDepartment['id']) + ->not->toContain($archivedDepartment['id']); $singleResponse = api_client()->get('/departments?id=' . $webhookDepartment['id'], $session['headers']); @@ -56,6 +62,79 @@ it('lists only visible departments and can return a single department with the s ->toHaveKey('slack_webhook', 'https://hooks.slack.test/example'); }); +it('allows superusers to filter archived departments', function (): void { + api_test_covers('GET /departments', 'happy'); + + $session = api_fixtures()->createUserSession([ + 'list_departments', + 'superuser_fetch_department', + ]); + + $activeDepartment = api_fixtures()->createDepartment([ + 'name' => 'Active Department', + 'visible' => 1, + 'archived' => 0, + ]); + $archivedDepartment = api_fixtures()->createDepartment([ + 'name' => 'Archived Department', + 'visible' => 1, + 'archived' => 1, + ]); + + $response = api_client()->get('/departments?filters=archived:1', $session['headers']); + + $response + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); + + $departmentIds = array_map( + static fn(array $department): int => (int)($department['id'] ?? 0), + is_array($response->data()) ? $response->data() : [] + ); + + expect($departmentIds) + ->toContain($archivedDepartment['id']) + ->not->toContain($activeDepartment['id']); + + foreach ($response->data() as $department) { + expect((bool)($department['archived'] ?? false))->toBeTrue(); + } +}); + +it('does not allow regular department listings to reveal archived departments through filters', function (): void { + api_test_covers('GET /departments', 'auth'); + + $session = api_fixtures()->createUserSession(['list_departments']); + + $activeDepartment = api_fixtures()->createDepartment([ + 'name' => 'Regular Active Department', + 'visible' => 1, + 'archived' => 0, + ]); + $archivedDepartment = api_fixtures()->createDepartment([ + 'name' => 'Regular Archived Department', + 'visible' => 1, + 'archived' => 1, + ]); + + $response = api_client()->get('/departments?filters=archived:1', $session['headers']); + + $response + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); + + $departmentIds = array_map( + static fn(array $department): int => (int)($department['id'] ?? 0), + is_array($response->data()) ? $response->data() : [] + ); + + expect($departmentIds) + ->toContain($activeDepartment['id']) + ->not->toContain($archivedDepartment['id']); +}); + it('rejects department listing when the permission is missing', function (): void { api_test_covers('GET /departments', 'auth'); @@ -137,6 +216,7 @@ it('updates departments through the real endpoint', function (): void { 'name' => 'Updated Department', 'description' => 'Updated description', 'order_priority' => 5, + 'archived' => true, ], $session['headers']); $response @@ -151,6 +231,7 @@ it('updates departments through the real endpoint', function (): void { expect($row['name'] ?? null)->toBe('Updated Department'); expect($row['description'] ?? null)->toBe('Updated description'); expect((int)($row['order_priority'] ?? 0))->toBe(5); + expect((int)($row['archived'] ?? 0))->toBe(1); }); it('rejects invalid department update requests', function (): void { diff --git a/services/nginx/app/tests/Support/Api/ApiFixtures.php b/services/nginx/app/tests/Support/Api/ApiFixtures.php index 2bf4ad02..8af9ab18 100644 --- a/services/nginx/app/tests/Support/Api/ApiFixtures.php +++ b/services/nginx/app/tests/Support/Api/ApiFixtures.php @@ -128,6 +128,7 @@ final class ApiFixtures 'dimension' => (int)($attributes['dimension'] ?? 0), 'branding' => (int)($attributes['branding'] ?? 0), 'visible' => (int)($attributes['visible'] ?? 1), + 'archived' => (int)($attributes['archived'] ?? 0), 'latitude' => $attributes['latitude'] ?? 0.0, 'longitude' => $attributes['longitude'] ?? 0.0, 'order_priority' => (int)($attributes['order_priority'] ?? 0), diff --git a/services/nginx/app/tests/Support/Api/ApiSchemaBootstrap.php b/services/nginx/app/tests/Support/Api/ApiSchemaBootstrap.php index 92f9d758..1d13da7f 100644 --- a/services/nginx/app/tests/Support/Api/ApiSchemaBootstrap.php +++ b/services/nginx/app/tests/Support/Api/ApiSchemaBootstrap.php @@ -19,6 +19,8 @@ final class ApiSchemaBootstrap $this->execute($name, $sql); } + $this->ensureDepartmentArchiveSchema(); + foreach ($this->viewStatements() as $name => $sql) { $this->execute($name, $sql); } @@ -85,13 +87,15 @@ CREATE TABLE IF NOT EXISTS `departments` ( `dimension` INT NOT NULL DEFAULT 0, `branding` INT NOT NULL DEFAULT 0, `visible` TINYINT(1) NOT NULL DEFAULT 1, + `archived` TINYINT(1) NOT NULL DEFAULT 0, `latitude` DECIMAL(10,7) NOT NULL DEFAULT 0, `longitude` DECIMAL(10,7) NOT NULL DEFAULT 0, `order_priority` INT NOT NULL DEFAULT 0, `created_at` DATETIME NULL DEFAULT CURRENT_TIMESTAMP, `updated_at` DATETIME NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, PRIMARY KEY (`id`), - KEY `idx_departments_visible` (`visible`) + KEY `idx_departments_visible` (`visible`), + KEY `idx_departments_archived` (`archived`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci SQL, 'department_variables' => <<<'SQL' @@ -486,4 +490,39 @@ SQL, ); } } + + private function ensureDepartmentArchiveSchema(): void + { + if (!$this->columnExists('departments', 'archived')) { + $this->execute( + 'departments.archived', + 'ALTER TABLE `departments` ADD COLUMN `archived` TINYINT(1) NOT NULL DEFAULT 0 AFTER `visible`' + ); + } + + if (!$this->indexExists('departments', 'idx_departments_archived')) { + $this->execute( + 'departments.idx_departments_archived', + 'ALTER TABLE `departments` ADD INDEX `idx_departments_archived` (`archived`)' + ); + } + } + + private function columnExists(string $table, string $column): bool + { + $table = $this->db->real_escape_string($table); + $column = $this->db->real_escape_string($column); + $result = $this->db->query("SHOW COLUMNS FROM `{$table}` LIKE '{$column}'"); + + return $result !== false && $result->num_rows > 0; + } + + private function indexExists(string $table, string $index): bool + { + $table = $this->db->real_escape_string($table); + $index = $this->db->real_escape_string($index); + $result = $this->db->query("SHOW INDEX FROM `{$table}` WHERE Key_name = '{$index}'"); + + return $result !== false && $result->num_rows > 0; + } }