Skip to content

Fix/resource page record binding #63

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

Merged
ManukMinasyan merged 2 commits into from
Dec 13, 2025
Merged

Fix/resource page record binding #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75aed19
feat: implement custom action resolution for board actions
ManukMinasyan Dec 13, 2025
bf10dcb
feat: add test fixtures for BoardResourcePage and TestResource
ManukMinasyan Dec 13, 2025
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
7 changes: 4 additions & 3 deletions phpunit.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
processIsolation="false"
stopOnFailure="false"
executionOrder="random"
failOnWarning="true"
failOnWarning="false"
failOnRisky="true"
failOnEmptyTestSuite="true"
beStrictAboutOutputDuringTests="true"
Expand All @@ -20,13 +20,14 @@
<directory>tests</directory>
</testsuite>
</testsuites>
<coverage>
<!-- Coverage configuration moved to test-coverage script -->
<!-- <coverage>
<report>
<html outputDirectory="build/coverage"/>
<text outputFile="build/coverage.txt"/>
<clover outputFile="build/logs/clover.xml"/>
</report>
</coverage>
</coverage> -->
<logging>
<junit outputFile="build/report.junit.xml"/>
</logging>
Expand Down
67 changes: 67 additions & 0 deletions src/BoardResourcePage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

namespace Relaticle\Flowforge;

use Filament\Actions\Action;
use Filament\Actions\Contracts\HasActions;
use Filament\Actions\Exceptions\ActionNotResolvableException;
use Filament\Forms\Contracts\HasForms;
use Filament\Resources\Pages\Page;
use Relaticle\Flowforge\Concerns\BaseBoard;
Expand All @@ -13,10 +15,75 @@
/**
* Board page for Filament resource pages.
* Extends Filament's resource Page class with kanban board functionality.
*
* CRITICAL: This class doesn't use InteractsWithRecord trait itself, but child
* classes might. To handle the trait conflict, we override getDefaultActionRecord()
* to intelligently route to either board card records or resource records based
* on whether a recordKey is present in the mounted action context.
*/
abstract class BoardResourcePage extends Page implements HasActions, HasBoard, HasForms
{
use BaseBoard;

protected string $view = 'flowforge::filament.pages.board-page';

/**
* Override Filament's action resolution to detect and route board actions.
*
* This method intercepts the action resolution flow to check if an action
* is a board action (has recordKey in context). If so, it routes to
* resolveBoardAction() which properly handles the record resolution,
* similar to how table actions are handled via resolveTableAction().
*
* This mirrors the logic in InteractsWithActions::resolveActions() but adds
* board action detection.
*
* @param array<array<string, mixed>> $actions
* @return array<Action>
*
* @throws ActionNotResolvableException
*/
protected function resolveActions(array $actions): array
{
$resolvedActions = [];

foreach ($actions as $actionNestingIndex => $action) {
if (blank($action['name'] ?? null)) {
throw new \Filament\Actions\Exceptions\ActionNotResolvableException('An action tried to resolve without a name.');
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

The code uses a fully qualified exception class name instead of relying on the imported ActionNotResolvableException at line 9. For consistency, use the imported class name instead of the fully qualified name.

Suggested change
throw new \Filament\Actions\Exceptions\ActionNotResolvableException('An action tried to resolve without a name.');
throw new ActionNotResolvableException('An action tried to resolve without a name.');

Copilot uses AI. Check for mistakes.
}

// Check if this is a board CARD action (has recordKey in context)
// Column actions have 'column' in arguments, not recordKey
// This detection happens BEFORE schema/table action detection
$recordKey = $action['context']['recordKey'] ?? null;
$columnId = $action['arguments']['column'] ?? null;

Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

The logic for detecting board card actions checks if recordKey is present AND columnId is blank. However, this logic may not handle all edge cases correctly. For instance, if an action has both recordKey and column in different contexts (e.g., a card action within a column), the condition might not work as expected. Consider adding clearer documentation about when actions have recordKey vs column, or add validation to ensure these are mutually exclusive when expected.

Suggested change
// Validation: recordKey and columnId should be mutually exclusive.
// If both are present, this is an invalid/ambiguous action context.
if (filled($recordKey) && filled($columnId)) {
throw new \Filament\Actions\Exceptions\ActionNotResolvableException(
"Ambiguous board action: both 'recordKey' and 'column' are present. " .
"Actions must have either 'recordKey' (for card actions) or 'column' (for column actions), not both."
);
}

Copilot uses AI. Check for mistakes.
// Only route to resolveBoardAction for card actions (not column actions)
if (filled($recordKey) && blank($columnId)) {
$resolvedAction = $this->resolveBoardAction($action, $resolvedActions);
} elseif (filled($action['context']['schemaComponent'] ?? null)) {
$resolvedAction = $this->resolveSchemaComponentAction($action, $resolvedActions);
} elseif (filled($action['context']['table'] ?? null)) {
$resolvedAction = $this->resolveTableAction($action, $resolvedActions);
} else {
$resolvedAction = $this->resolveAction($action, $resolvedActions);
}

if (! $resolvedAction) {
continue;
}

$resolvedAction->nestingIndex($actionNestingIndex);
$resolvedAction->boot();

$resolvedActions[] = $resolvedAction;

$this->cacheSchema(
"mountedActionSchema{$actionNestingIndex}",
$this->getMountedActionSchema($actionNestingIndex, $resolvedAction),
);
}

return $resolvedActions;
}
}
33 changes: 33 additions & 0 deletions src/Concerns/InteractsWithBoard.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,39 @@ public function getBoardQuery(): ?Builder
return $this->getBoard()->getQuery();
}

/**
* Resolve a board action (similar to resolveTableAction).
*/
protected function resolveBoardAction(array $action, array $parentActions): ?Action
{
$resolvedAction = null;

if (count($parentActions)) {
$parentAction = end($parentActions);
$resolvedAction = $parentAction->getModalAction($action['name']);
} else {
$resolvedAction = $this->cachedActions[$action['name']] ?? null;
}

if (! $resolvedAction) {
return null;
}

$recordKey = $action['context']['recordKey'] ?? $action['arguments']['recordKey'] ?? null;

if (filled($recordKey)) {
$board = $this->getBoard();
$query = $board->getQuery();

if ($query) {
$record = (clone $query)->find($recordKey);
$resolvedAction->record($record);
}
}

return $resolvedAction;
}

Comment on lines +290 to +321
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

The resolveBoardAction method is defined in the InteractsWithBoard trait, but it's being called in BoardResourcePage.resolveActions(). Since BoardResourcePage uses the BaseBoard trait which includes InteractsWithBoard, this method will be available. However, to improve code organization and follow the same pattern as resolveTableAction and resolveSchemaComponentAction (which come from Filament's InteractsWithTable and InteractsWithForms traits respectively), consider whether this method should remain in InteractsWithBoard or be moved to BoardResourcePage where it's actually called.

Suggested change
* Resolve a board action (similar to resolveTableAction).
*/
protected function resolveBoardAction(array $action, array $parentActions): ?Action
{
$resolvedAction = null;
if (count($parentActions)) {
$parentAction = end($parentActions);
$resolvedAction = $parentAction->getModalAction($action['name']);
} else {
$resolvedAction = $this->cachedActions[$action['name']] ?? null;
}
if (! $resolvedAction) {
return null;
}
$recordKey = $action['context']['recordKey'] ?? $action['arguments']['recordKey'] ?? null;
if (filled($recordKey)) {
$board = $this->getBoard();
$query = $board->getQuery();
if ($query) {
$record = (clone $query)->find($recordKey);
$resolvedAction->record($record);
}
}
return $resolvedAction;
}
*
*/

Copilot uses AI. Check for mistakes.
/**
* Get board record actions with proper context.
*/
Expand Down
41 changes: 41 additions & 0 deletions tests/Fixtures/TestBoardResourcePage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

declare(strict_types=1);

namespace Relaticle\Flowforge\Tests\Fixtures;

use Filament\Resources\Pages\Concerns\InteractsWithRecord;
use Relaticle\Flowforge\Board;
use Relaticle\Flowforge\BoardResourcePage;
use Relaticle\Flowforge\Column;

/**
* Test fixture for BoardResourcePage that uses InteractsWithRecord.
* Replicates the GitHub issue #37 scenario where a project has many tasks.
*/
class TestBoardResourcePage extends BoardResourcePage
{
use InteractsWithRecord;

protected static string $resource = TestResource::class;

public function mount(int | string $record): void
{
$this->record = $this->resolveRecord($record);
}

public function board(Board $board): Board
{
// Use $this->getRecord() to scope tasks to this project
return $board
->query($this->getRecord()->tasks()->getQuery())
->recordTitleAttribute('title')
->columnIdentifier('status')
->positionIdentifier('order_position')
->columns([
Column::make('todo')->label('To Do')->color('gray'),
Column::make('in_progress')->label('In Progress')->color('blue'),
Column::make('completed')->label('Completed')->color('green'),
]);
}
}
24 changes: 24 additions & 0 deletions tests/Fixtures/TestResource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

namespace Relaticle\Flowforge\Tests\Fixtures;

use Filament\Resources\Resource;

/**
* Minimal test resource for TestBoardResourcePage.
*/
class TestResource extends Resource
{
protected static ?string $model = Project::class;

protected static ?string $slug = 'test-projects';

public static function getPages(): array
{
return [
'board' => TestBoardResourcePage::route('/{record}/board'),
];
}
}
Comment on lines +1 to +24
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

These test fixtures (TestResource and TestBoardResourcePage) appear to be created to address GitHub issue 37, but there are no corresponding test files that use these fixtures to verify the fix. Without actual tests exercising these fixtures, it's unclear whether the implementation correctly solves the issue. Consider adding test cases that use these fixtures to verify the record binding functionality works correctly when InteractsWithRecord trait is used with BoardResourcePage.

Copilot uses AI. Check for mistakes.