Commit 73a61a77 authored by Mahmoud Aglan's avatar Mahmoud Aglan

test(branch): watch the SQL a dashboard runs, not just the records it renders

The two existing suites cannot catch a leaking dashboard. BranchIsolationTest
proves the scope narrows a model; BranchScopedScreensTest looks for another
branch's records in the rendered HTML. But a dashboard renders totals, not
records — a revenue widget quietly summing every branch shows a number that is
simply wrong, with no uuid anywhere to give it away, and both suites pass.

Dashboards are also where the raw query builder lives, because that is what
aggregates are written in, and a raw DB::table() goes straight past every global
scope. So this asserts at the only layer that sees both: the wire.

Two checks:

  - Every SQL statement each of sixteen dashboards runs is captured with
    DB::listen, and any query reading a branch-owned table without mentioning
    `branch_id` anywhere — its own WHERE, a join, a subquery the scope added —
    is a failure. Deliberately crude, because a strict SQL parse would be worse
    than useless here: a query that never says the word never asked.

  - The per-branch figures for participants, enrolments, invoices, payment
    totals, attendance and groups must add up to the academy-wide figure. A
    widget ignoring the branch returns the whole academy for every branch, so
    the sum comes out a multiple of the truth.

Current state: all sixteen render, 1,354 queries captured, 902 of them touching
branch-owned tables, and every one names a branch.
Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
parent d3cbd5f1
<?php
namespace Tests\Feature;
use App\Domain\Shared\Context\BranchContext;
use App\Domain\Shared\Models\Academy;
use App\Models\User;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Route;
use Tests\TestCase;
/**
* Watch the SQL a dashboard actually runs, and refuse any query that reads a
* branch-owned table without saying which branch.
*
* DB_CONNECTION=pgsql DB_DATABASE=oc_sport_test ./vendor/bin/phpunit --filter DashboardQueriesRespectBranchTest
*
* The other two suites cannot catch this. BranchIsolationTest proves the model
* scope narrows a model, and BranchScopedScreensTest looks for another branch's
* *records* in the rendered HTML — but a dashboard renders totals, not records.
* A revenue widget that quietly summed every branch would show a number that is
* simply wrong, with no uuid anywhere to give it away, and both suites would
* pass.
*
* Dashboards are also where the raw query builder lives, because that is what
* aggregates are written in — and a raw DB::table() goes straight past every
* global scope. So this asserts at the only layer that sees both: the wire.
*
* The rule is deliberately crude, because a strict SQL parse would be worse
* than useless here: any query that reads a branch-owned table must mention
* `branch_id` *somewhere* — in its own WHERE, in a join, or in a subquery the
* scope put there. A query that never says the words is one that never asked.
*/
class DashboardQueriesRespectBranchTest extends TestCase
{
/**
* Tables where every row belongs to exactly one branch, and reading them
* without a branch predicate means reading every branch.
*/
private const BRANCH_OWNED = [
'participants', 'training_groups', 'enrollments', 'training_sessions',
'attendance_records', 'invoices', 'payments', 'transactions', 'expenses',
'pos_transactions', 'cash_sessions', 'facilities', 'space_reservations',
'training_programs', 'base_prices', 'products', 'inventory_levels',
'inventory_movements', 'payslips', 'trainer_advances',
];
/**
* Every screen whose job is to show a number.
*
* Named explicitly rather than discovered, so that adding a dashboard and
* forgetting to add it here is a visible omission in review rather than a
* silent gap in coverage.
*/
private const DASHBOARDS = [
'dashboard',
'financial.overview',
'reports.hub',
'reports.attendance',
'payroll.dashboard',
'payroll.advances',
'receptionist.dashboard',
'pos.history',
'cash-sessions.list',
'inventory.movements',
'invoices.list',
'expenses.list',
'enrollments.list',
'participants.list',
'groups.list',
'attendance.list',
];
private User $owner;
protected function setUp(): void
{
parent::setUp();
if (config('database.default') !== 'pgsql') {
$this->markTestSkipped('Needs a restored Postgres tenant; see the class comment.');
}
if (! $academy = Academy::first()) {
$this->markTestSkipped('No academy in the restored tenant.');
}
app()->instance('current_academy', $academy);
$owner = User::withoutGlobalScopes()
->whereHas('primaryRole', fn ($q) => $q->where('slug', 'academy_owner'))
->first();
if (! $owner) {
$this->markTestSkipped('No academy_owner in the restored tenant.');
}
$this->owner = $owner;
}
private function branchId(): int
{
return (int) DB::table('training_groups')
->whereNotNull('branch_id')->orderBy('branch_id')->value('branch_id');
}
/**
* Does this query read a branch-owned table?
*
* Matches the quoted identifier Postgres emits ("participants") as well as
* the bare word, so `participant_id` on some other table does not count.
*/
private function tablesRead(string $sql): array
{
$hit = [];
foreach (self::BRANCH_OWNED as $table) {
if (preg_match('/(?:from|join|into|update)\s+"?'.$table.'"?\b/i', $sql)) {
$hit[] = $table;
}
}
return $hit;
}
public function test_no_dashboard_reads_a_branch_owned_table_without_naming_a_branch(): void
{
$branchId = $this->branchId();
$offenders = [];
foreach (self::DASHBOARDS as $name) {
if (! Route::has($name)) {
$this->fail("Route {$name} no longer exists — update the dashboard list.");
}
$captured = [];
DB::listen(function ($query) use (&$captured) {
$captured[] = $query->sql;
});
$response = $this->actingAs($this->owner)
->withSession([BranchContext::KEY => $branchId])
->get(route($name));
// A screen the owner cannot open tells us nothing either way.
if ($response->getStatusCode() !== 200) {
continue;
}
foreach ($captured as $sql) {
$tables = $this->tablesRead($sql);
if ($tables === []) {
continue;
}
if (stripos($sql, 'branch_id') !== false) {
continue;
}
$offenders[] = $name.' → '.implode(',', $tables).' :: '
.substr(preg_replace('/\s+/', ' ', $sql), 0, 260);
}
}
$this->assertSame(
[],
$offenders,
"These queries read a branch-owned table without naming a branch:\n\n"
.implode("\n\n", array_unique($offenders))
);
}
/**
* The counterpart to the query check: the numbers themselves must move when
* the branch does.
*
* A widget that ignored the branch would return the academy-wide figure for
* every branch, so the per-branch figures would sum to far more than the
* whole. Asserting on the underlying aggregate rather than on scraped HTML,
* because a number's position in the markup is not what is being tested.
*/
public function test_branch_totals_add_up_to_the_academy_total(): void
{
$branches = DB::table('branches')->whereNull('deleted_at')->pluck('id');
$metrics = [
'participants' => fn () => \App\Domain\Participant\Models\Participant::count(),
'enrolments' => fn () => \App\Domain\Training\Models\Enrollment::count(),
'invoices' => fn () => \App\Domain\Financial\Models\Invoice::count(),
'payments' => fn () => \App\Domain\Financial\Models\Payment::sum('amount'),
'attendance' => fn () => \App\Domain\Attendance\Models\AttendanceRecord::count(),
'groups' => fn () => \App\Domain\Training\Models\TrainingGroup::count(),
];
$state = app(\App\Domain\Shared\Context\BranchScopeState::class);
foreach ($metrics as $label => $metric) {
$state->deactivate();
$whole = $metric();
$summed = 0;
foreach ($branches as $id) {
$state->activate((int) $id);
$summed += $metric();
}
$state->deactivate();
$this->assertSame(
(int) $whole,
(int) $summed,
"The per-branch {$label} figures do not add up to the academy total — "
."either a branch is double-counting, or rows belong to no branch at all."
);
}
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment