Skip to content

Ensure coordinator fallback preserves access when API is down #191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Ensure coordinator fallback preserves access when API is down #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions src/backend/api_or_sql/funcs.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,55 @@ function get_coordinator()
$query = "SELECT id, user FROM coordinator order by id";
//---
$u_data = super_function($api_params, [], $query);
// Ensure all coordinator rows expose an `active` flag for downstream
// consumers. The SQL fallback currently only returns the `id` and
// `user` columns, so we inject a default active=1 flag in that case to
// match the API payload.
$u_data = normalize_coordinator_rows($u_data);
Copy link

@gemini-code-assist gemini-code-assist bot Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The get_coordinator function now normalizes the data, and coordinator_active_index also normalizes the data. When called as coordinator_active_index(get_coordinator()) in src/header.php, the data is normalized twice. While this is not incorrect, it is inefficient. Consider removing the normalization from one of the functions to avoid redundant work. A good approach might be to remove this line and let coordinator_active_index be responsible for normalization, which aligns with its test case and its role as a 'reusable helper'.

// ---
$coordinator = $u_data;
// ---
return $u_data;
}

function normalize_coordinator_rows(array $rows): array
{
return array_map(static function ($row) {
if (!is_array($row)) {
return $row;
}

$active = $row['active'] ?? null;
if ($active === null || $active === '') {
$active = 1;
}
$row['active'] = (int) $active;

return $row;
}, $rows);
}

function coordinator_active_index(array $rows): array
{
$normalized = normalize_coordinator_rows($rows);
$coordinators = [];

foreach ($normalized as $row) {
if (!is_array($row)) {
continue;
}

$user = $row['user'] ?? '';
if ($user === '') {
continue;
}

$coordinators[$user] = (int) ($row['active'] ?? 0);
Copy link

@gemini-code-assist gemini-code-assist bot Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since normalize_coordinator_rows is called at the beginning of this function, the active key is guaranteed to exist and be an integer for every array row. Therefore, the null coalescing operator (?? 0) and the (int) cast are redundant. The ?? 0 is also confusing as it suggests a default of 0, while normalize_coordinator_rows provides a default of 1 for missing active keys.

$coordinators[$user] = $row['active'];

}

return $coordinators;
}

function get_user_pages($user_main, $year_y, $lang_y)
{
// ---
Expand Down
3 changes: 2 additions & 1 deletion src/header.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
include_once __DIR__ . '/head.php';
//---
use function TD\Render\Html\banner_alert;
use function SQLorAPI\Funcs\coordinator_active_index;
use function SQLorAPI\Funcs\get_coordinator;
//---
$coordinators = array_column(get_coordinator(), 'active', 'user');
$coordinators = coordinator_active_index(get_coordinator());

$user_in_coord = false;
if (($coordinators[$GLOBALS['global_username']] ?? 0) == 1) {
Expand Down
40 changes: 40 additions & 0 deletions tests/CoordinatorFallbackTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

declare(strict_types=1);

use PHPUnit\Framework\TestCase;
use function SQLorAPI\Funcs\coordinator_active_index;
use function SQLorAPI\Funcs\normalize_coordinator_rows;

final class CoordinatorFallbackTest extends TestCase
{
public function testNormalizeCoordinatorRowsAddsDefaultActive(): void
{
$rows = [
['user' => 'Alice'],
['user' => 'Bob', 'active' => '0'],
['user' => 'Carol', 'active' => 1],
];

$normalized = normalize_coordinator_rows($rows);

$this->assertSame(1, $normalized[0]['active']);
$this->assertSame(0, $normalized[1]['active']);
$this->assertSame(1, $normalized[2]['active']);
}

public function testCoordinatorActiveIndexTreatsMissingActiveAsActive(): void
{
$rows = [
['user' => 'Alice'],
['user' => 'Bob', 'active' => 0],
];

$index = coordinator_active_index($rows);

$this->assertSame([
'Alice' => 1,
'Bob' => 0,
], $index);
}
}