Commit 20c5cf15 authored by Mahmoud Aglan's avatar Mahmoud Aglan

test(branch): check the records a screen is handed, not the markup it prints

Reported: the weekly schedule shows a schedule on a branch that was never
scheduled. Investigated against a restored tenant and the component is correct —
branch 2 gets only branch 2's sessions, and a branch with nothing gets an empty
grid. But chasing it found a real hole in the suite.

BranchScopedScreensTest searches rendered HTML for another branch's uuids. The
weekly schedule grid prints group names and times and no uuid at all, and
training_sessions was not even among the tables it collected uuids for. A
foreign session sitting in that grid would have left nothing in the markup to
search for, and the suite would have stayed green while the screen was wrong.
Several other grids and calendars are the same shape.

So this asserts one level earlier, on the objects rather than the markup. Every
component reachable at a parameterless URL is mounted under every branch, the
data handed to its views is captured through a view composer, and every model in
it that carries a branch_id must belong to the active branch — or be null only
where null still means "every branch" (people and the academy calendar), or
belong to a model that declares BRANCH_SCOPE_EXEMPT. It reads that declaration
rather than keeping a second list that would drift away from it.

Components come from the router rather than a hand-written list, so a screen
added next month is covered without anyone remembering.

1,188 component mounts across nine branches, 9.5M records inspected, no screen
handing its view another branch's record.
Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
parent e45bd6d7
<?php
namespace Tests\Feature;
use App\Domain\Shared\Context\BranchContext;
use App\Domain\Shared\Context\BranchScopeState;
use App\Domain\Shared\Models\Academy;
use App\Models\User;
use Illuminate\Database\Eloquent\Collection as EloquentCollection;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Route;
use Illuminate\Support\Facades\View;
use Tests\TestCase;
/**
* Mount every list screen and inspect the records it hands its view.
*
* DB_CONNECTION=pgsql DB_DATABASE=oc_sport_test ./vendor/bin/phpunit --filter BranchScopedComponentDataTest
*
* BranchScopedScreensTest searches the rendered HTML for another branch's uuids,
* which only works on a screen that prints uuids. The weekly schedule does not:
* its grid renders group names and times, so a session from another branch would
* have sat in that grid with nothing in the markup to give it away, and the
* suite would have stayed green. Several other grids and calendars are the same
* shape.
*
* So this asserts one level earlier, on the objects rather than the markup.
* Every model a component passes to its view is examined: if it carries a
* branch_id, it must be the active branch — or null, which is only allowed for
* the handful of models where null still means "every branch" (people, and the
* academy calendar).
*
* Components are discovered from the router rather than listed, so a screen
* added next month is covered without anyone remembering to add it here.
*/
class BranchScopedComponentDataTest extends TestCase
{
/** Models where a null branch legitimately means "every branch". */
private const SHARED_TABLES = ['employees', 'trainers', 'guardians', 'holidays'];
/**
* Components that are cross-branch by design.
*
* Short on purpose: everything absent from it is expected to hold the line.
*/
private const CROSS_BRANCH_BY_DESIGN = [
\App\Livewire\BranchSwitcher::class,
\App\Livewire\Branches\BranchList::class,
\App\Livewire\Branches\BranchForm::class,
\App\Livewire\Enrollments\TransferParticipantWizard::class,
\App\Livewire\Enrollments\TransferGroup::class,
];
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;
}
/**
* Livewire components reachable at a URL that takes no parameters.
*
* @return array<string, string> route name => component class
*/
private function listComponents(): array
{
$out = [];
foreach (Route::getRoutes() as $route) {
$name = $route->getName();
if (! $name || ! in_array('GET', $route->methods(), true)) {
continue;
}
if (str_contains($route->uri(), '{')) {
continue;
}
$action = $route->getActionName();
if (! str_starts_with($action, 'App\\Livewire\\')) {
continue;
}
$class = str_replace('@__invoke', '', $action);
if (! class_exists($class) || in_array($class, self::CROSS_BRANCH_BY_DESIGN, true)) {
continue;
}
// The member portal is scoped by membership, not by branch.
if (str_starts_with($class, 'App\\Livewire\\Portal\\')
|| str_starts_with($class, 'App\\Livewire\\Parent\\')
|| str_starts_with($class, 'App\\Livewire\\Guardian\\')) {
continue;
}
$out[$name] = $class;
}
ksort($out);
return $out;
}
/**
* Walk whatever a component handed its view and pull out every model.
*
* @return array<int, Model>
*/
private function modelsIn(mixed $value, int $depth = 0): array
{
if ($depth > 4) {
return [];
}
if ($value instanceof Model) {
return [$value];
}
if ($value instanceof EloquentCollection || $value instanceof Collection || is_array($value)) {
$out = [];
foreach ($value as $item) {
foreach ($this->modelsIn($item, $depth + 1) as $model) {
$out[] = $model;
}
}
return $out;
}
if ($value instanceof \Illuminate\Pagination\AbstractPaginator) {
return $this->modelsIn($value->items(), $depth + 1);
}
return [];
}
public function test_no_list_screen_hands_its_view_another_branch_record(): void
{
$branches = DB::table('branches')->whereNull('deleted_at')->orderBy('id')
->pluck('id')->map(fn ($id) => (int) $id)->all();
$this->assertNotEmpty($branches);
$state = app(BranchScopeState::class);
$this->actingAs($this->owner);
$leaks = [];
$inspected = 0;
$modelsSeen = 0;
foreach ($branches as $branchId) {
foreach ($this->listComponents() as $routeName => $class) {
session([BranchContext::KEY => $branchId]);
app()->forgetInstance(BranchContext::class);
$state->activate($branchId);
// Read what the view is handed rather than what the markup
// prints: the weekly schedule renders group names and times and
// no uuid at all, so a foreign session in its grid leaves no
// trace in the HTML to search for.
$seen = [];
View::composer('*', function ($view) use (&$seen) {
$seen[] = $view->getData();
});
try {
\Livewire\Livewire::test($class);
} catch (\Throwable $e) {
// A screen that will not mount — a missing permission, a
// required parameter — is another test's problem, not this
// one's. BranchScopedScreensTest already asserts nothing
// 5xx's.
continue;
}
$inspected++;
foreach ($seen as $data) {
foreach ($data as $key => $value) {
foreach ($this->modelsIn($value) as $model) {
$modelsSeen++;
$verdict = $this->verdict($model, $branchId);
if ($verdict !== null) {
$leaks[] = "branch {$branchId} / {$routeName}\${$key}: {$verdict}";
}
}
}
}
}
}
$state->deactivate();
$this->assertGreaterThan(20, $inspected, 'Almost nothing mounted — the sweep proved nothing.');
$this->assertGreaterThan(0, $modelsSeen, 'No models were inspected — the sweep proved nothing.');
$this->assertSame(
[],
array_values(array_unique($leaks)),
"Components handed their view a record from another branch:\n\n"
.implode("\n", array_unique($leaks))
);
}
/**
* Null when the model is fine, otherwise a sentence saying what is wrong.
*/
private function verdict(Model $model, int $branchId): ?string
{
if (! array_key_exists('branch_id', $model->getAttributes())) {
return null;
}
// A model that declares itself academy-wide keeps its column but is not
// scoped by it — events are the case today. Read the declaration rather
// than keeping a second list here that would drift away from it.
if (defined(get_class($model).'::BRANCH_SCOPE_EXEMPT')) {
return null;
}
$rowBranch = $model->getAttribute('branch_id');
if ($rowBranch === null) {
return in_array($model->getTable(), self::SHARED_TABLES, true)
? null
: $model->getTable().' #'.$model->getKey().' belongs to no branch at all';
}
if ((int) $rowBranch === $branchId) {
return null;
}
return $model->getTable().' #'.$model->getKey().' belongs to branch '.$rowBranch;
}
}
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