Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php

namespace Modules\ReportBuilder\Filament\Admin\Resources;

use BackedEnum;
use Filament\Resources\Resource;
use Filament\Schemas\Schema;
use Filament\Support\Icons\Heroicon;
use Filament\Tables\Table;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Pages\CreateReportTemplate;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Pages\DesignReportTemplate;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Pages\EditReportTemplate;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Pages\ListReportTemplates;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Schemas\ReportTemplateForm;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Tables\ReportTemplatesTable;
use Modules\ReportBuilder\Models\ReportTemplate;

class ReportTemplateResource extends Resource
{
protected static ?string $model = ReportTemplate::class;

protected static string|BackedEnum|null $navigationIcon = Heroicon::OutlinedDocumentText;

protected static ?string $navigationGroup = 'Reports';

protected static ?int $navigationSort = 10;

public static function getModelLabel(): string
{
return 'Report Template';
}

public static function getPluralModelLabel(): string
{
return 'Report Templates';
}

public static function getNavigationLabel(): string
{
return 'Report Templates';
}

public static function form(Schema $schema): Schema
{
return ReportTemplateForm::configure($schema);
}

public static function table(Table $table): Table
{
return ReportTemplatesTable::configure($table);
}

public static function getRelations(): array
{
return [];
}

public static function getPages(): array
{
return [
'index' => ListReportTemplates::route('/'),
'create' => CreateReportTemplate::route('/create'),
'edit' => EditReportTemplate::route('/{record}/edit'),
'design' => DesignReportTemplate::route('/{record}/design'),
];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

namespace Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Pages;

use Filament\Resources\Pages\CreateRecord;
use Illuminate\Database\Eloquent\Model;
use Modules\Core\Models\Company;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource;
use Modules\ReportBuilder\Services\ReportTemplateService;

class CreateReportTemplate extends CreateRecord
{
protected static string $resource = ReportTemplateResource::class;

public function create(bool $another = false): void
{
$this->authorizeAccess();

$this->callHook('beforeValidate');
$data = $this->form->getState();
$this->callHook('afterValidate');

$data = $this->mutateFormDataBeforeCreate($data);
$this->callHook('beforeCreate');

$this->record = $this->handleRecordCreation($data);

$this->callHook('afterCreate');
$this->rememberData();

$this->getCreatedNotification()?->send();

if ($another) {
$this->form->model($this->getRecord()::class);
$this->record = null;
$this->fillForm();

return;
}

$this->redirect($this->getRedirectUrl());
}

protected function handleRecordCreation(array $data): Model
{
$company = Company::find(session('current_company_id'));
if (!$company) {
$company = auth()->user()->companies()->first();
}
Comment on lines +46 to +49
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Extract duplicated company resolution logic.

This company resolution logic is duplicated in ListReportTemplates.php (lines 24-27). Extract it to a shared trait or helper method to maintain consistency and reduce duplication.

See the suggestion in ListReportTemplates.php for the extraction pattern.

🤖 Prompt for AI Agents
In
Modules/ReportBuilder/Filament/Admin/Resources/ReportTemplateResource/Pages/CreateReportTemplate.php
around lines 46 to 49, the company-resolution block is duplicated (also present
in ListReportTemplates.php lines 24-27); extract this logic into a shared helper
so both pages use the same implementation. Create a small trait (e.g.,
ResolvesCurrentCompany) or a static helper method in a suitable namespace under
Modules/ReportBuilder (implementing: check session('current_company_id') and
fallback to auth()->user()->companies()->first()), add proper return typing,
include unit/usage docblock, then replace the inline code in both
CreateReportTemplate.php and ListReportTemplates.php with a call to the new
trait/helper and import/use it in both classes. Ensure behavior and tests remain
unchanged.


return app(ReportTemplateService::class)->createTemplate(
$company,
$data['name'],
$data['template_type'],
[]
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
<?php

namespace Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Pages;

use Filament\Resources\Pages\Page;
use Illuminate\Support\Str;
use Livewire\Attributes\On;
use Modules\ReportBuilder\DTOs\BlockDTO;
use Modules\ReportBuilder\DTOs\GridPositionDTO;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource;
use Modules\ReportBuilder\Models\ReportTemplate;
use Modules\ReportBuilder\Services\GridSnapperService;
use Modules\ReportBuilder\Services\ReportTemplateService;
use Modules\ReportBuilder\Transformers\BlockTransformer;

class DesignReportTemplate extends Page
{
protected static string $resource = ReportTemplateResource::class;

protected static string $view = 'reportbuilder::filament.admin.resources.report-template-resource.pages.design-report-template';

public ReportTemplate $record;

public array $blocks = [];

public string $selectedBlockId = '';

public function mount(ReportTemplate $record): void
{
$this->record = $record;
$this->loadBlocks();
}

protected function loadBlocks(): void
{
$service = app(ReportTemplateService::class);
$blockDTOs = $service->loadBlocks($this->record);

$this->blocks = [];
foreach ($blockDTOs as $blockDTO) {
$blockArray = BlockTransformer::toArray($blockDTO);
$this->blocks[$blockArray['id']] = $blockArray;
}
}

#[On('drag-block')]
public function updateBlockPosition(string $blockId, array $position): void
{
if (!isset($this->blocks[$blockId])) {
return;
}

$gridSnapper = app(GridSnapperService::class);
$positionDTO = new GridPositionDTO();
$positionDTO->setX($position['x'] ?? 0)
->setY($position['y'] ?? 0)
->setWidth($position['width'] ?? 1)
->setHeight($position['height'] ?? 1);

if (!$gridSnapper->validate($positionDTO)) {
return;
}

$snappedPosition = $gridSnapper->snap($positionDTO);

$this->blocks[$blockId]['position'] = [
'x' => $snappedPosition->getX(),
'y' => $snappedPosition->getY(),
'width' => $snappedPosition->getWidth(),
'height' => $snappedPosition->getHeight(),
];
}

#[On('add-block')]
public function addBlock(string $blockType): void
{
$blockId = 'block_' . $blockType . '_' . Str::random(8);

$position = new GridPositionDTO();
$position->setX(0)->setY(0)->setWidth(6)->setHeight(4);

$block = new BlockDTO();
$block->setId($blockId)
->setType($blockType)
->setPosition($position)
->setConfig([])
->setLabel(ucfirst(str_replace('_', ' ', $blockType)))
->setIsCloneable(false)
->setDataSource('custom')
->setIsCloned(false)
->setClonedFrom(null);

$this->blocks[$blockId] = BlockTransformer::toArray($block);
}

#[On('clone-block')]
public function cloneBlock(string $blockId): void
{
if (!isset($this->blocks[$blockId])) {
return;
}

$originalBlock = $this->blocks[$blockId];

if ($originalBlock['isCloned'] === false && $originalBlock['isCloneable'] === true) {
$newBlockId = 'block_' . $originalBlock['type'] . '_' . Str::random(8);

$position = new GridPositionDTO();
$position->setX($originalBlock['position']['x'] + 1)
->setY($originalBlock['position']['y'] + 1)
->setWidth($originalBlock['position']['width'])
->setHeight($originalBlock['position']['height']);

$clonedBlock = new BlockDTO();
$clonedBlock->setId($newBlockId)
->setType($originalBlock['type'])
->setPosition($position)
->setConfig($originalBlock['config'])
->setLabel($originalBlock['label'] . ' (Clone)')
->setIsCloneable(false)
->setDataSource($originalBlock['dataSource'])
->setIsCloned(true)
->setClonedFrom($blockId);

$this->blocks[$newBlockId] = BlockTransformer::toArray($clonedBlock);
}
}
Comment on lines +97 to +127
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

LGTM: Proper clone logic with minor position concern.

The clone logic correctly validates cloneability (line 105) and sets appropriate metadata. The position offset of (+1, +1) on lines 109-110 is simple but could potentially place clones off-grid or cause overlap. Consider whether the offset should be validated or snapped immediately.

Verify that the +1 offset doesn't cause issues:


🏁 Script executed:

#!/bin/bash
# Check if cloned block positions are validated/snapped elsewhere
rg -nP -C3 '\bcloneBlock\b' --type=php

Length of output: 887



---

🏁 Script executed:

```shell
#!/bin/bash
# Search for position validation, snapping, or grid constraint logic
rg -nP -C2 '(snap|Snap|validate|Validate|constraint|Constraint|grid.*position|position.*grid)' --type=php | head -100

Length of output: 10040


🏁 Script executed:

#!/bin/bash
# Search for where blocks are saved/persisted after cloning
rg -nP -C3 'blocks\[' --type=php | grep -A3 -B3 'cloned\|clone' | head -80

Length of output: 1989


🏁 Script executed:

#!/bin/bash
# Check if BlockDTO or GridPositionDTO has validation
fd -e php | xargs rg -l 'class (BlockDTO|GridPositionDTO)' --type=php

Length of output: 193



---

🏁 Script executed:

```shell
#!/bin/bash
# Check what happens after cloneBlock - is there downstream validation?
rg -nP -C5 'cloneBlock' --type=php | grep -v test

Length of output: 1394


🏁 Script executed:

#!/bin/bash
# Check if positions are always validated/snapped before persisting blocks
rg -nP 'saveBlocks|persistBlocks|updateBlocks|save.*blocks' --type=php

Length of output: 1405


🏁 Script executed:

#!/bin/bash
# Check the moveBlock method to see the validation pattern applied there
sed -n '45,72p' Modules/ReportBuilder/Filament/Admin/Resources/ReportTemplateResource/Pages/DesignReportTemplate.php

Length of output: 894


Apply grid validation and snapping to cloned block position, consistent with updateBlockPosition.

The cloneBlock method bypasses the validation and snapping applied in updateBlockPosition (lines 45-72). When a block is cloned with the +1 offset, the position is stored without checking grid boundaries. If the original block is near the grid edge (e.g., x=gridSize-1), the cloned block's offset will exceed valid grid bounds.

The fix: validate and snap the cloned position before storing:

$position = new GridPositionDTO();
$position->setX($originalBlock['position']['x'] + 1)
    ->setY($originalBlock['position']['y'] + 1)
    ->setWidth($originalBlock['position']['width'])
    ->setHeight($originalBlock['position']['height']);

$gridSnapper = app(GridSnapperService::class);
if (!$gridSnapper->validate($position)) {
    return;
}
$position = $gridSnapper->snap($position);

Then use $position->getX() etc. when setting the cloned block's position (lines 114-117).

🤖 Prompt for AI Agents
In
Modules/ReportBuilder/Filament/Admin/Resources/ReportTemplateResource/Pages/DesignReportTemplate.php
around lines 97 to 127, the cloned block position is created by offsetting the
original position but skips the grid validation and snapping used in
updateBlockPosition, which can allow positions outside grid bounds; fix by
resolving a GridSnapperService from the container, validate the new
GridPositionDTO and return early if invalid, then snap the position before
setting it on the cloned BlockDTO and use the snapped position getters (x, y,
width, height) when assigning the cloned block's position; ensure you abort the
clone when validation fails to match updateBlockPosition behavior.


#[On('delete-block')]
public function deleteBlock(string $blockId): void
{
if (!isset($this->blocks[$blockId])) {
return;
}

unset($this->blocks[$blockId]);
}

#[On('edit-config')]
public function updateBlockConfig(string $blockId, array $config): void
{
if (!isset($this->blocks[$blockId])) {
return;
}

$this->blocks[$blockId]['config'] = array_merge(
$this->blocks[$blockId]['config'] ?? [],
$config
);
}

public function save(): void
{
$service = app(ReportTemplateService::class);
$service->persistBlocks($this->record, $this->blocks);

$this->dispatch('blocks-saved');
$this->redirect(static::getResource()::getUrl('index'));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

namespace Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Pages;

use Filament\Actions\DeleteAction;
use Filament\Resources\Pages\EditRecord;
use Illuminate\Database\Eloquent\Model;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource;
use Modules\ReportBuilder\Services\ReportTemplateService;

class EditReportTemplate extends EditRecord
{
protected static string $resource = ReportTemplateResource::class;

public function save(bool $shouldRedirect = true, bool $shouldSendSavedNotification = true): void
{
$this->authorizeAccess();

$this->callHook('beforeValidate');
$data = $this->form->getState();
$this->callHook('afterValidate');

$data = $this->mutateFormDataBeforeSave($data);
$this->callHook('beforeSave');

$this->record = $this->handleRecordUpdate($this->getRecord(), $data);

$this->callHook('afterSave');

if ($shouldSendSavedNotification) {
$this->getSavedNotification()?->send();
}

if ($shouldRedirect) {
$this->redirect($this->getRedirectUrl());
}
}

protected function getHeaderActions(): array
{
return [
DeleteAction::make()
->visible(fn () => !$this->record->is_system)
->action(function () {
app(ReportTemplateService::class)->deleteTemplate($this->record);
}),
];
}

protected function handleRecordUpdate(Model $record, array $data): Model
{
$record->update([
'name' => $data['name'],
'description' => $data['description'] ?? null,
'template_type' => $data['template_type'],
'is_active' => $data['is_active'] ?? true,
]);

return $record;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

namespace Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource\Pages;

use Filament\Actions\Action;
use Filament\Actions\CreateAction;
use Filament\Resources\Pages\ListRecords;
use Modules\Core\Models\Company;
use Modules\ReportBuilder\Filament\Admin\Resources\ReportTemplateResource;
use Modules\ReportBuilder\Services\ReportTemplateService;

class ListReportTemplates extends ListRecords
{
protected static string $resource = ReportTemplateResource::class;

protected function getHeaderActions(): array
{
return [
CreateAction::make()
->action(function (array $data) {
$company = Company::find(session('current_company_id'));
if (!$company) {
$company = auth()->user()->companies()->first();
}

app(ReportTemplateService::class)->createTemplate(
$company,
$data['name'],
$data['template_type'],
[]
);
})
Comment on lines +20 to +32
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Duplicated company resolution and creation logic.

The company resolution logic (lines 24-27) and template creation flow are duplicated from CreateReportTemplate.php (lines 46-56). This duplication can lead to inconsistencies if the logic needs to change. Consider one of these approaches:

  1. Preferred: Remove this custom CreateAction and rely solely on the CreateReportTemplate page for creation workflow.
  2. Extract company resolution to a shared trait or helper method if both approaches are needed.

If you need both creation paths, extract the shared logic:

// In a trait or helper
protected function resolveCurrentCompany(): Company
{
    $company = Company::find(session('current_company_id'));
    if (!$company) {
        $company = auth()->user()->companies()->first();
    }
    return $company;
}

Then use it in both places:

->action(function (array $data) {
    $company = $this->resolveCurrentCompany();
    
    app(ReportTemplateService::class)->createTemplate(
        $company,
        $data['name'],
        $data['template_type'],
        []
    );
})
🤖 Prompt for AI Agents
In
Modules/ReportBuilder/Filament/Admin/Resources/ReportTemplateResource/Pages/ListReportTemplates.php
around lines 23 to 35, the company resolution and template creation logic is
duplicated from CreateReportTemplate.php; remove the custom inline CreateAction
and instead rely on the existing CreateReportTemplate page for creation, or if
you must keep both flows, extract the company resolution into a shared protected
method or trait (e.g., resolveCurrentCompany()) and call that from both
locations, then call ReportTemplateService::createTemplate with the resolved
Company and incoming data; ensure you remove the duplicated lines 24–27 and
replace them with a single call to the shared helper or navigation to the
CreateReportTemplate page.

->modalWidth('full'),
];
}
}
Loading