From 3ff79c7dca4bfa6562db16ff110f4bb7c92f11dc Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 8 Sep 2026 19:12:06 -0600 Subject: [PATCH 1/2] Let record navigation follow a named list, and be overridable Record navigation reads its siblings from the controller's primary list. That is the right default, but it makes navigation unavailable on any controller whose primary list is deliberately a subset: a queue filtered to pending records has no neighbours for a record outside it, so the buttons silently disappear on the contexts that view those records. `recordNavigation` now also accepts the name of a list definition, resolved per form context using the same lookup `initForm()` already uses for `"{$context}[form]"`. A controller can navigate an archive list on `preview` while `update` keeps navigating the queue: preview: recordNavigation: archive Resolving it per context also makes `recordNavigation: false` work inside a context, which it did not before -- only the top level key was read. Separately, `formRenderRecordNavigation()` now resolves the getter through `$this->controller`, as `formExtendFields()`, `formExtendRefreshData()` and the other extension points on this behavior already do. Without that a controller cannot override `formGetRecordNavigation()` at all: the behavior calls its own copy, so the override is never reached and the only way to influence navigation is to reimplement the render helper verbatim. Defaults are unchanged. The two tests covering them pass with or without this change; the three covering the new behaviour fail without it. --- modules/backend/behaviors/FormController.php | 25 ++- ...FormControllerRecordNavigationListTest.php | 190 ++++++++++++++++++ 2 files changed, 212 insertions(+), 3 deletions(-) create mode 100644 modules/backend/tests/behaviors/FormControllerRecordNavigationListTest.php diff --git a/modules/backend/behaviors/FormController.php b/modules/backend/behaviors/FormController.php index d26664c7a9..b59d0812c4 100644 --- a/modules/backend/behaviors/FormController.php +++ b/modules/backend/behaviors/FormController.php @@ -668,11 +668,14 @@ public function formRenderSecondaryTabs() * default), the controller also implements the ListController behavior, and * an existing record is being viewed. * + * Resolved through the controller so that `formGetRecordNavigation()` can be + * overridden there, as with the other extension points on this behavior. + * * @return string HTML markup (empty string when navigation is unavailable) */ public function formRenderRecordNavigation(): string { - $navigation = $this->formGetRecordNavigation(); + $navigation = $this->controller->formGetRecordNavigation(); if ($navigation === null || $navigation['current'] === null) { return ''; } @@ -693,12 +696,26 @@ public function formRenderRecordNavigation(): string * position is resolved in PHP — no driver-specific SQL — so it behaves * identically across every database Winter supports. * + * `recordNavigation` accepts `false` to disable navigation, or the name of a + * list definition to navigate that list instead of the primary one, and may be + * set per form context. A controller whose primary list is filtered to a subset + * -- an open queue, say -- can then still offer navigation on a context that + * views records outside it: + * + * preview: + * recordNavigation: archive + * * @param \Winter\Storm\Database\Model|null $model * @return array{previous: mixed, next: mixed, current: int|null, total: int}|null */ public function formGetRecordNavigation($model = null): ?array { - if (!$this->getConfig('recordNavigation', true)) { + $navigation = $this->getConfig( + "{$this->context}[recordNavigation]", + $this->getConfig('recordNavigation', true) + ); + + if (!$navigation) { return null; } @@ -712,7 +729,9 @@ public function formGetRecordNavigation($model = null): ?array } $this->controller->makeLists(); - $listWidget = $this->controller->listGetWidget(); + $listWidget = $this->controller->listGetWidget( + is_string($navigation) ? $navigation : null + ); if (!$listWidget) { return null; } diff --git a/modules/backend/tests/behaviors/FormControllerRecordNavigationListTest.php b/modules/backend/tests/behaviors/FormControllerRecordNavigationListTest.php new file mode 100644 index 0000000000..51f711254a --- /dev/null +++ b/modules/backend/tests/behaviors/FormControllerRecordNavigationListTest.php @@ -0,0 +1,190 @@ +controller`. Without that a controller cannot override + * `formGetRecordNavigation()` at all: the behavior calls its own copy, so the + * override is never reached and the only way to influence navigation is to + * reimplement the render helper verbatim. + * + * @see modules/backend/behaviors/FormController.php + */ +class NavigationController extends Controller +{ + public $implement = [ + \Backend\Behaviors\FormController::class, + \Backend\Behaviors\ListController::class, + ]; + + public $formConfig = [ + 'name' => 'User', + 'modelClass' => User::class, + 'form' => ['fields' => ['login' => ['label' => 'Login']]], + 'update' => ['title' => 'Edit'], + 'preview' => [ + 'title' => 'View', + // The point of the feature: this context navigates a different list + 'recordNavigation' => 'archive', + ], + ]; + + public $listConfig = [ + 'index' => [ + 'modelClass' => User::class, + 'list' => ['columns' => ['login' => ['label' => 'Login']]], + ], + 'archive' => [ + 'modelClass' => User::class, + 'list' => ['columns' => ['login' => ['label' => 'Login']]], + ], + ]; + + /** + * The primary list is a subset -- superusers only -- while the archive holds + * everybody. This is the shape that leaves navigation unavailable today. + */ + public function listExtendQuery($query, $definition) + { + if ($definition === 'index') { + $query->where('is_superuser', true); + } + + return $query->orderBy('id'); + } +} + +/** + * Disables navigation outright, to prove `false` still wins. + */ +class NavigationDisabledController extends NavigationController +{ + public $formConfig = [ + 'name' => 'User', + 'modelClass' => User::class, + 'form' => ['fields' => ['login' => ['label' => 'Login']]], + 'preview' => ['recordNavigation' => false], + ]; +} + +/** + * Overrides the getter, which only takes effect if the render helper resolves it + * through the controller. + */ +class NavigationOverrideController extends NavigationController +{ + public array $overrideCalls = []; + + public function formGetRecordNavigation($model = null): ?array + { + $this->overrideCalls[] = $model ? $model->getKey() : null; + + return ['previous' => 41, 'next' => 43, 'current' => 2, 'total' => 3]; + } +} + +class FormControllerRecordNavigationListTest extends PluginTestCase +{ + protected User $inBothLists; + + protected User $archivedOnly; + + public function setUp(): void + { + parent::setUp(); + + $this->inBothLists = (new UserFixture)->asSuperUser(); + $this->inBothLists->login = 'in-both'; + $this->inBothLists->email = 'in-both@example.com'; + $this->inBothLists->forceSave(); + + // In the archive only -- the record with no neighbours in the primary list + $this->archivedOnly = new UserFixture; + $this->archivedOnly->login = 'archive-only'; + $this->archivedOnly->email = 'archive-only@example.com'; + $this->archivedOnly->forceSave(); + + $this->actingAs((new UserFixture)->asSuperUser()); + } + + public function testThePrimaryListIsUsedByDefault(): void + { + $controller = new NavigationController; + $controller->initForm($this->inBothLists, 'update'); + + $navigation = $controller->formGetRecordNavigation($this->inBothLists); + + $this->assertNotNull($navigation); + $this->assertNotNull($navigation['current'], 'the record should be found in the primary list'); + } + + public function testARecordOutsideThePrimaryListHasNoPositionInIt(): void + { + // The behaviour this feature exists to answer: navigation is unavailable + // because the primary list is a subset that excludes this record + $controller = new NavigationController; + $controller->initForm($this->archivedOnly, 'update'); + + $navigation = $controller->formGetRecordNavigation($this->archivedOnly); + + $this->assertNull($navigation['current']); + $this->assertSame('', $controller->formRenderRecordNavigation()); + } + + public function testAContextCanNavigateANamedList(): void + { + $controller = new NavigationController; + $controller->initForm($this->archivedOnly, 'preview'); + + $navigation = $controller->formGetRecordNavigation($this->archivedOnly); + + $this->assertNotNull($navigation['current'], 'the archive list contains this record'); + $this->assertSame( + User::count(), + $navigation['total'], + 'the total should come from the archive list, not the filtered primary one' + ); + } + + public function testNavigationCanStillBeDisabled(): void + { + $controller = new NavigationDisabledController; + $controller->initForm($this->inBothLists, 'preview'); + + $this->assertNull($controller->formGetRecordNavigation($this->inBothLists)); + $this->assertSame('', $controller->formRenderRecordNavigation()); + } + + public function testTheGetterCanBeOverriddenByTheController(): void + { + $controller = new NavigationOverrideController; + $controller->initForm($this->archivedOnly, 'update'); + + // Reached directly ... + $this->assertSame(2, $controller->formGetRecordNavigation($this->archivedOnly)['current']); + + // ... and, the half that did not work, through the render helper. Without the + // controller resolving the getter this renders nothing, because the behavior + // calls its own copy and finds no position in the primary list. + $this->assertNotSame('', $controller->formRenderRecordNavigation()); + $this->assertNotEmpty($controller->overrideCalls); + } +} From 6233ac21a5991af4c7696295acff51782ac47aa7 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 8 Sep 2026 19:21:30 -0600 Subject: [PATCH 2/2] Update modules/backend/tests/behaviors/FormControllerRecordNavigationListTest.php Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .../tests/behaviors/FormControllerRecordNavigationListTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/backend/tests/behaviors/FormControllerRecordNavigationListTest.php b/modules/backend/tests/behaviors/FormControllerRecordNavigationListTest.php index 51f711254a..71e8353781 100644 --- a/modules/backend/tests/behaviors/FormControllerRecordNavigationListTest.php +++ b/modules/backend/tests/behaviors/FormControllerRecordNavigationListTest.php @@ -158,7 +158,7 @@ public function testAContextCanNavigateANamedList(): void $this->assertNotNull($navigation['current'], 'the archive list contains this record'); $this->assertSame( - User::count(), + User::query()->count(), $navigation['total'], 'the total should come from the archive list, not the filtered primary one' );