Best Practices

Anti-Patterns

The numbered list of patterns we reject in code review, grouped by severity, with the fix for each.

This chapter is the authoritative list of patterns the project rejects in code review. Each item has a stable reference number (#1 through #24) that PR comments and the winch-code-reviewer agent cite directly. The numbers never change — if you see "blocked on #8" in a review, the item below is what was flagged.

The list is grouped by severity, not by number. Critical items block the PR; warnings should be fixed in the same PR if reasonable; suggestions are non-blocking polish. Within each group, items are sorted by their original number so the reference stays scannable.

Every entry follows the same shape: a one-paragraph description of the smell, why it bites, a ❌ example, a ✅ example, and a pointer to the file where the fix belongs. Read it once end-to-end so you recognise the names, then come back when a PR comment cites a number.

The severity legend is shared across the codebase:

Critical — block the PR; fix before merging.
Warning — fix in this PR if reasonable; otherwise create a follow-up.
Suggestion — recommend; don't block.

Critical — block the PR

These items represent rules whose violation breaks the architecture, leaves partial database writes, or causes silent data loss. Don't merge a PR with any of them open.

#1 — Direct model access across modules

Code in domain A reaches into domain B's Models\Entities\* (or its Actions or Scopes) by use-importing the class and querying it. The only sanctioned way to read or write another domain's data is through that domain's Contract.

Crossing the boundary directly couples both domains permanently — internal refactors in the upstream domain silently break the downstream caller, and there's no longer a single place to add validation, caching, or events for that read.

❌ Bad

use Domain\Provider\Models\Entities\Provider;

final class AssignOrderToProvider
{
    public static function handle(int $orderId, int $providerId): Order
    {
        $provider = Provider::find($providerId);  // reaching across the boundary

        return Order::create([...]);
    }
}

✅ Good

final class AssignOrderToProvider
{
    public static function handle(int $orderId, int $providerId): Order
    {
        $provider = app(ProviderContract::class)->getProvider($providerId);

        return Order::create([...]);
    }
}

Where to fix: any Action, Service, Controller, Job, or Listener that imports a class from another domain's Models\Entities\ namespace. See services and contracts and actions.

#5 — Fat controllers

A controller method orchestrates lookups, builds models, writes related rows, calls calculations — everything a Service or Action should own. The controller becomes the only place that knows the full flow, and the same logic can never be reused by Jobs, console commands, or other entry points.

Controllers are thin HTTP adapters: hydrate a DTO from the request, hand it to a Contract, translate the result into a response. That's it.

❌ Bad

public function store(Request $request): JsonResponse
{
    $provider = Provider::find($request->provider_id);
    $customer = Customer::find($request->customer_id);
    $order = Order::create([
        'provider_id' => $provider->id,
        'customer_id' => $customer->id,
        'amount' => $request->amount,
    ]);
    $order->lines()->createMany($request->lines);
    $order->calculateTotal();

    return response()->json($order);
}

✅ Good

public function store(StoreOrderRequest $request): JsonResponse
{
    $order = $this->orderContract->create(
        CreateOrderData::fromArray($request->validated()),
    );

    return response()->json($order);
}

Where to fix: in src/Presentation/{Cpanel}/Controllers/. Move orchestration into a Create{Entity} Action and expose it via the domain Contract. See controllers and actions.

#6 — Non-static Actions

An Action class exposes an execute(), run(), or __invoke() instance method, requiring new ...() at the call site. Project Actions are stateless and have no constructor — callers use Class::handle($data) directly.

A constructor on an Action also tempts contributors to inject Contracts there, which breaks the rule from actions: cross-domain dependencies live inside handle() via app(...), not in a constructor.

❌ Bad

class CreateOrderAction
{
    public function execute(CreateOrderData $data): Order
    {
        return Order::create($data->toArray());
    }
}

(new CreateOrderAction())->execute($data);

✅ Good

final class CreateOrder
{
    public static function handle(CreateOrderData $data): Order
    {
        return Order::create($data->toArray());
    }
}

CreateOrder::handle($data);

Where to fix: in src/Domain/{Domain}/Actions/. Make the method public static function handle(...), mark the class final, and drop the Action suffix from the class name (the namespace already encodes it). See actions.

#8 — Missing DB transaction on multi-statement write

An Action writes to two or more tables (or two or more rows across the same table) without DB::transaction(). If the second statement fails — constraint violation, deadlock, network blip — the first one is already committed and the database is in a partial state.

Any Create / Update / Delete Action that touches more than one row must wrap the writes in a transaction. Read-only Actions (Get*) do not.

❌ Bad

final class CreateOrder
{
    public static function handle(CreateOrderData $data): Order
    {
        $order = Order::create([...]);
        $order->lines()->createMany($data->lines);  // if this throws, $order is orphaned

        return $order;
    }
}

✅ Good

final class CreateOrder
{
    public static function handle(CreateOrderData $data): Order
    {
        return DB::transaction(function () use ($data) {
            $order = Order::create([...]);
            $order->lines()->createMany($data->lines);

            return $order;
        });
    }
}

A missing transaction is one of the most expensive bugs to detect after the fact — the database survives in an inconsistent state and the user often won't notice until a downstream report breaks. Wrap any multi-row write.

Where to fix: in domain Actions under src/Domain/{Domain}/Actions/. See actions for the rule and the standard Create* / Update* templates.

#15 — Direct model queries in Services

A Service class contains Eloquent queries directly (Model::query()->where(...), Model::find(...)). Services exist to satisfy a Contract by delegating to Actions; query logic belongs in an Action so it can be reused, tested, and listed in one place per domain.

This is the same critical rule as #1 viewed from the inside of a domain rather than across the boundary — a Service that queries directly is bypassing its own Action layer, which is the seam other domains will eventually need.

❌ Bad

class EmployeeService implements EmployeeContract
{
    public function getEmployee(int $id): ?Employee
    {
        return Employee::query()->where('id', $id)->first();  // forbidden in a Service
    }
}

✅ Good

class EmployeeService implements EmployeeContract
{
    public function getEmployee(int $id): ?Employee
    {
        return GetEmployee::handle($id);
    }
}

final class GetEmployee
{
    public static function handle(int $id): ?Employee
    {
        return Employee::query()->where('id', $id)->first();
    }
}

Where to fix: in src/Domain/{Domain}/Services/. The Service may call GetData{Entity}s::for(...), Get{Entity}::handle(...), etc. — anything more than that means the body belongs in an Action. See services and contracts.

#18 — camelCase in DTO properties

A *Data class declares its public readonly properties in camelCase (e.g. $providerId, $customerId). Data classes mirror DB column names, which are snake_case throughout the application, and Arrayable::toArray() plus Model::fill() rely on that alignment so Model::create($data->toArray()) works without per-field mapping.

camelCase properties silently break write Actions that pass $data->toArray() straight into Model::create(...) or update(...) — fields land in $attributes under the wrong key and become silent data loss the next time the model is read.

❌ Bad

final class CreateOrderData
{
    use Arrayable;

    public function __construct(
        public readonly int $providerId,
        public readonly int $customerId,
        public readonly int $cityId,
    ) {}
}

✅ Good

final class CreateOrderData
{
    use Arrayable;

    public function __construct(
        public readonly int $provider_id,
        public readonly int $customer_id,
        public readonly int $city_id,
    ) {}
}

Where to fix: in src/Domain/{Domain}/DataTransferObjects/. Rename properties, then update every call site that constructed the DTO by name. See DTOs.

#19 — Data classes not ending with "Data"

A DTO class is named CreateOrderDTO, ProviderDTO, OrderLineDTO, or simply Order. The project convention — established across controllers, services, and Contracts — is the suffix Data. The reviewer agent treats this as a hard rule because the naming difference cascades into wrong imports across the codebase.

A non-Data suffix also breaks habits: contributors reading a Contract signature can't tell at a glance whether the parameter is a DTO or a domain model.

❌ Bad

final class CreateOrderDTO
{
    public function __construct(
        public readonly int $provider_id,
    ) {}
}

final class ProviderDTO { /* ... */ }
final class OrderLine    { /* ... */ }

✅ Good

final class CreateOrderData
{
    public function __construct(
        public readonly int $provider_id,
    ) {}
}

final class ProviderData  { /* ... */ }
final class OrderLineData { /* ... */ }

Where to fix: in src/Domain/{Domain}/DataTransferObjects/. Class name, file name, and every import. See DTOs.

Warnings — fix in this PR if reasonable

These items don't break anything outright, but they accumulate into design problems if ignored. Fix them in the PR that introduced them; if the fix is large, file a follow-up ticket before merging.

#2 — Specific getter methods instead of one `get{Entity}`

A Contract grows methods like getProviderById, getProviderByEmail, getProviderByPhone. Each new lookup needs a new method, a new Action, and a new place in the Service — the surface area explodes for a single concept (find one provider).

The convention is a single get{Entity}(?int $id = null, ?Filter{Entity}Data $filters = null, array $with_relations = []): ?{Entity}Data. Lookups by anything other than primary key go through the Filter{Entity}Data parameter.

❌ Bad

interface ProviderContract
{
    public function getProviderById(int $id): ?ProviderData;
    public function getProviderByEmail(string $email): ?ProviderData;
    public function getProviderByPhone(string $phone): ?ProviderData;
}

✅ Good

interface ProviderContract
{
    public function getProvider(?int $id = null, ?FilterProviderData $filters = null, array $with_relations = []): ?ProviderData;
}

Where to fix: in src/Domain/{Domain}/Contracts/{Entity}Contract.php and the matching Service. Add or extend Filter{Entity}Data to carry the new lookup field. See services and contracts and DTOs.

#3 — Multi-line method signatures

Interface methods (and, by extension, Service methods) are split across multiple lines with each parameter on its own line. The project style keeps the full signature on one line — neighbour files are one line, and Pint won't reformat this for you.

The exception is constructors of Data classes, which are intentionally multi-line for readability. Method signatures elsewhere — Contracts, Services, Actions — stay on one line.

❌ Bad

public function getProvider(
    ?int $id = null,
    ?FilterProviderData $filters = null,
    array $with_relations = []
): ?ProviderData;

✅ Good

public function getProvider(?int $id = null, ?FilterProviderData $filters = null, array $with_relations = []): ?ProviderData;

Where to fix: Contract interfaces, Service implementations, and any Action whose signature would otherwise wrap. See code style.

#4 — Returning Models from Contracts

Cross-domain Contracts expose Eloquent models (Provider, Order, Admin) as their return type. The intended end state is that Contracts return read DTOs (ProviderData, OrderData) — that way the downstream domain has no path to mutate the row, no accidental N+1 from relation access, and no leak of the upstream's schema.

Existing Service templates and the canonical Contract pattern (see services and contracts) emit model return types, so the codebase is full of this pattern. A future refactor will introduce {Entity}Data read DTOs. Until that lands, don't block PRs on this pattern — flag it as informational for new Contracts and leave existing ones alone.

❌ Bad (target state)

interface ProviderContract
{
    public function getProvider(int $id): ?Provider;
}

✅ Good (target state)

interface ProviderContract
{
    public function getProvider(?int $id = null, ?FilterProviderData $filters = null): ?ProviderData;
}

Where to fix: in src/Domain/{Domain}/Contracts/ once the read-DTO migration begins. Until then, follow the existing template and don't introduce a one-off DTO without coordination.

#7 — Constructor injection in domain classes

A Service or Action declares a constructor that injects another domain's Contract (or any Laravel service). Constructor injection inside the domain layer creates tight binding at boot time, makes Actions impossible (they're not container-resolved), and obscures which collaborators a method actually needs.

Constructor injection is fine in controllers — those are container-resolved on every request and inject the owning domain's Contract. Inside a domain, resolve dependencies inline with app(SomeContract::class).

❌ Bad

class OrderService implements OrderContract
{
    public function __construct(
        private readonly ProviderContract $providerContract,
    ) {}

    public function processOrder(int $id): void
    {
        $provider = $this->providerContract->getProvider($id);
    }
}

✅ Good

class OrderService implements OrderContract
{
    public function processOrder(int $id): void
    {
        $provider = app(ProviderContract::class)->getProvider($id);
    }
}

Where to fix: in domain Services/ and Actions/. Constructor injection of cross-domain Contracts is a code smell here; inline app(...) resolution is the project convention. See services and contracts and actions.

#9 — Raw arrays instead of Data classes

A Contract or Action accepts array $data or returns array for a structured payload. Arrays carry no type information, no IDE autocomplete, no static analysis, no required-field enforcement — every caller has to guess the keys, and every renamed field becomes a silent runtime break.

Inputs to write Actions are Create{Entity}Data / Update{Entity}Data. Inputs to list / read methods are Filter{Entity}Data. Outputs of read methods are model entities (current state) or {Entity}Data (target state — see #4).

❌ Bad

public function createOrder(array $data): Order;
public function getOrderData(int $id): array;

✅ Good

public function createOrder(CreateOrderData $data): Order;
public function getOrder(?int $id = null, ?FilterOrderData $filters = null): ?OrderData;

Where to fix: in Contracts and Service signatures. Define the missing Data class in src/Domain/{Domain}/DataTransferObjects/ and update all callers. See DTOs.

#10 — Array parameter without PHPDoc array shape

A Data class has an array property (e.g. $lines, $attachments) but no @param ChildData[] $lines PHPDoc on the constructor. Without the shape annotation, static analysis can't infer what's in the array, IDEs can't autocomplete on $data->lines[0]->..., and reviewers can't tell whether the array holds DTOs, models, or scalars.

This is closely related to #11 — but here the issue is purely the missing annotation; the right fix is sometimes a nested Data class (#11) and sometimes just the docblock.

❌ Bad

final class OrderData
{
    use Arrayable;

    public function __construct(
        public readonly array $lines,
    ) {}
}

✅ Good

final class OrderData
{
    use Arrayable;

    /** @param OrderLineData[] $lines */
    public function __construct(
        public readonly array $lines,
    ) {}
}

Where to fix: in any *Data class with array-typed properties. See DTOs and code style.

#11 — Nested arrays instead of nested Data classes

A Data class accepts an array of associative arrays for a child collection ($lines = [['product_id' => 1, 'qty' => 2], ...]). Even with a @param array[] annotation, the inner shape is informal — callers don't know which keys are required, and the action that consumes the payload has to read keys defensively.

Promote the inner shape to its own Data class (OrderLineData) and annotate the parent with @param OrderLineData[] $lines. Now the schema is explicit at both levels.

❌ Bad

final class CreateOrderData
{
    use Arrayable;

    public function __construct(
        public readonly int $provider_id,
        public readonly array $lines,  // [['product_id' => ..., 'qty' => ...], ...]
    ) {}
}

✅ Good

final class CreateOrderData
{
    use Arrayable;

    /** @param OrderLineData[] $lines */
    public function __construct(
        public readonly int $provider_id,
        public readonly array $lines,
    ) {}
}

final class OrderLineData
{
    use Arrayable;

    public function __construct(
        public readonly int $product_id,
        public readonly int $qty,
    ) {}
}

Where to fix: in src/Domain/{Domain}/DataTransferObjects/. Add the child Data class, update the fromArray() builder on the parent to hydrate it. See DTOs.

#14 — Repeated filter logic without scopes

The same three-or-four-clause where chain appears in two or more places — Action queries, scopes inside other models, Service-level queries that snuck past #15. The chain encodes a domain concept ("active provider", "issued waybill") and changing the definition means hunting down every duplicate.

Extract the chain to an Eloquent local scope on the model (scopeActive, scopeIssued) and call the scope everywhere. The query reads at the level of the domain concept, not the implementation.

❌ Bad

$query->where('status', ProviderStatus::ACTIVE)
    ->where('verified_at', '!=', null)
    ->where('banned_at', null);

✅ Good

// In Provider model
public function scopeActive(Builder $query): void
{
    $query->where('status', ProviderStatus::ACTIVE)
        ->whereNotNull('verified_at')
        ->whereNull('banned_at');
}

// In Actions
$query->active();

Where to fix: in domain models (src/Domain/{Domain}/Models/Entities/) and Models/Scopes/ traits. See Eloquent and queries.

#16 — Circular dependencies between domains

Domain A's Contract calls domain B's Contract, and B's Contract calls back into A's Contract. At best this is a confusing dependency graph; at worst it deadlocks under transactions or causes infinite recursion when both sides try to "stay consistent."

Break the cycle: either move the shared concept into a third domain (Core, or a new bounded context), have one side publish a domain event the other listens to, or model the relationship explicitly with a join entity that owns the bridge.

❌ Bad

// Domain\Order\Services\OrderService
$provider = app(ProviderContract::class)->getProvider($id);

// Domain\Provider\Services\ProviderService
$orderCount = app(OrderContract::class)->countOrdersForProvider($id);
// Now Order and Provider depend on each other.

✅ Good

// Domain\Order — publishes an event on create
event(new OrderCreated($order));

// Domain\Provider — listens for it
final class IncrementProviderOrderCount
{
    public function handle(OrderCreated $event): void { /* ... */ }
}

Where to fix: in domain Providers/, Services/, and Listeners/. Look at the import graph between two domains before adding a cross-call. See domain boundaries and cross-module communication.

#20 — Loading rows just to count them

A count is computed with ->collection()->count() (or ->get()->count()) on a Get* / GetDataAbstract result — every matching row is hydrated into a Collection and counted in PHP. GetDataAbstract already exposes ->count(), which issues a single SQL COUNT(*) and transfers one integer instead of thousands of models.

On a large table the difference is real, not theoretical — a document-summary badge dropped from ~72ms to ~11ms (and stopped pulling 60k+ rows into memory) purely by switching the call. The same applies to existence: use ->exists(), never ->collection()->isNotEmpty().

❌ Bad

$count = app(DetailsDataContract::class)->getDetails($filter)->collection()->count();

✅ Good

$count = app(DetailsDataContract::class)->getDetails($filter)->count();

Where to fix: anywhere a Get* result is only counted — controllers, menu-badge closures, services, stat widgets. See Eloquent and queries.

#21 — Wildcard `whereHas()` on a polymorphic relation

An existence check on a morphTo relation is written as whereHas('contractable') with no type list. To resolve the wildcard, Eloquent first runs select distinct {relation}_type from {table} — a full scan of the whole table — on every build of the query, then a per-type EXISTS. Passing the explicit morph-target list skips the discovery scan entirely.

Keep the type list as a constant on the model (e.g. UserContract::CONTRACTABLE_TYPES, mirroring the morph map) so it lives in one place.

❌ Bad

UserContract::filterBy($data)->whereHas('contractable');
// → select distinct contractable_type from user_contracts  (full scan, every call)

✅ Good

UserContract::filterBy($data)->whereHasMorph('contractable', UserContract::CONTRACTABLE_TYPES);

Where to fix: any whereHas() / whereDoesntHave() on a morphTo, in Actions and Models/Scopes/ filter traits. See Eloquent and queries.

#22 — Service drops `$withRelations` / `$withCount`

A Service get{Entity}s() declares array $withRelations = [] (and $withCount) in its signature but calls the Action's ::for($data) without forwarding them. The parameters are silently ignored, so no caller can ever eager-load — every relation the view touches then lazy-loads per row (N+1). The bug is invisible at the call site because the signature looks correct.

❌ Bad

public function getExports(FilterExportData $data, array $withRelations = [], array $withCount = []): GetExports
{
    return GetExports::for($data);   // $withRelations / $withCount dropped on the floor
}

✅ Good

public function getExports(FilterExportData $data, array $withRelations = [], array $withCount = []): GetExports
{
    return GetExports::for($data, $withRelations, $withCount);
}

Where to fix: in src/Domain/{Domain}/Services/ — every get{Entity}s() that exposes $withRelations must pass it through to Get{Entity}s::for(...). See services and contracts.

#23 — Controllers calling Actions directly

A controller use-imports a domain Action and calls it statically (SomeAction::handle(...)), resolves one via app(SomeAction::class), or constructor-injects an Action. Controllers are HTTP adapters whose only domain dependency is the relevant {Domain}Contract — every domain call goes through a Contract method, never an Action.

This is distinct from #5: the controller can be perfectly thin — a single Action::handle($dto) line — and still violate the boundary. Calling the Action directly bypasses the domain's published gateway, where cross-domain consumers, jobs, and tests look for the operation, and where a transaction or extra orchestration gets added later. Wiring straight to an Action couples the Presentation layer to a domain internal, so renaming or splitting that Action silently breaks the controller.

❌ Bad

use Domain\Order\Actions\OrderDriverBooking\NotifyDriverBookingAssigned;

class OrderAssignmentActionController extends Controller
{
    public function assignOrder(int $id, AssignOrderRequest $request): RedirectResponse
    {
        // ...
        NotifyDriverBookingAssigned::handle($order, $assignmentType, $driverId);  // Action called from the controller
    }
}

✅ Good

use Domain\Order\Contracts\OrderDriverBookingContract;

class OrderAssignmentActionController extends Controller
{
    public function __construct(
        private readonly OrderDriverBookingContract $orderDriverBookingContract,
    ) {}

    public function assignOrder(int $id, AssignOrderRequest $request): RedirectResponse
    {
        // ...
        $this->orderDriverBookingContract->notifyAssigned($order, $assignmentType, $driverId);
    }
}

Where to fix: in src/Presentation/{Cpanel}/Controllers/. Add the method to the domain Contract, implement it in the Service (delegating to the Action), then inject and call the Contract. The messaging transport builders (Domain\Message\Actions\Sender\Mail::make(...), Whatsapp::make(...)) are the sanctioned exception — a fluent transport API used inside Actions everywhere, not a business Action. See controllers and services and contracts.

#24 — Orchestration or parsing logic in Services

A Service method contains loops, conditionals, or array traversal rather than a single delegation to an Action. The Service exists to satisfy a Contract by calling an Action — it should hold no algorithm, no iteration, no payload parsing, no decision-making. Any multi-step process, even a foreach over an input array, is business logic that belongs in an Action.

This is distinct from #15: the violation here is not a DB query but logic that becomes non-reusable when buried in the Service. Moving it to an Action makes it callable from Jobs, console commands, and Listeners without going through the full Contract stack.

❌ Bad

class WhatsappWebhookService implements WhatsappWebhookContract
{
    public function handle(array $entries): void
    {
        foreach ($entries as $entry) {
            foreach ($entry['changes'] ?? [] as $change) {
                foreach ($change['value']['statuses'] ?? [] as $status) {
                    MarkWhatsappMessageAsRead::handle($status);
                }
            }
        }
    }
}

✅ Good

// All iteration lives in the Action
final class ProcessWhatsappWebhook
{
    public static function handle(array $entries): void
    {
        foreach ($entries as $entry) {
            foreach ($entry['changes'] ?? [] as $change) {
                foreach ($change['value']['statuses'] ?? [] as $status) {
                    MarkWhatsappMessageAsRead::handle($status);
                }
            }
        }
    }
}

// Service is a pure one-line delegate
class WhatsappWebhookService implements WhatsappWebhookContract
{
    public function handle(array $entries): void
    {
        ProcessWhatsappWebhook::handle($entries);
    }
}

Where to fix: in src/Domain/{Domain}/Services/. If a Service method body is more than a single return SomeAction::handle(...) or SomeAction::for(...) call, extract the body into an Action. See actions and services and contracts.

These are style polish — the manual approach works, but the project has a clearer idiom. Reviewers should mention them; authors can choose to defer.

#12 — Not using `whereByType` macro for polymorphic filters

A Filter Scope branches manually on is_array(...) to decide between whereIn and where. The whereByType macro on the query builder does exactly that — pass it a column and a value, it picks whereIn if the value is an array and where otherwise. The branch is one method call.

❌ Bad

if (is_array($filters->status)) {
    $query->whereIn('status', $filters->status);
} else {
    $query->where('status', $filters->status);
}

✅ Good

$query->when($filters?->status, fn ($q) => $q->whereByType('status', $filters->status));

Where to fix: in Models/Scopes/ filter traits — wherever a filter value could be one-or-many. See Eloquent and queries.

#13 — Not using `whereLikeText` macro for LIKE searches

A filter scope writes where('column', 'like', "%{$value}%") inline. The whereLikeText macro centralises the wildcard handling, escaping, and case-insensitivity behaviour — one call expresses "fuzzy text search on this column."

Manual LIKE works, but the macro normalises behaviour across the codebase (especially around Arabic text and case folding) and saves the contributor from getting the wildcard escaping wrong.

❌ Bad

$query->when($filters?->email, fn ($q) => $q->where('email', 'like', "%{$filters->email}%"));
$query->when($filters?->name, fn ($q) => $q->where('admins.name', 'like', "%{$filters->name}%"));

✅ Good

$query->when($filters?->email, fn ($q) => $q->whereLikeText('email', $filters->email));
$query->when($filters?->name, fn ($q) => $q->whereLikeText('name', $filters->name));

Where to fix: in Models/Scopes/ filter traits, anywhere a free-text search field hits the database. See Eloquent and queries.

#17 — Verbose filter data creation from requests

A controller builds a Filter{Entity}Data by listing every property and pulling each one from the request manually. The list duplicates the DTO's constructor and breaks every time a new filter field is added — the controller silently drops the new field until someone remembers to update both places.

Filter{Entity}Data::fromArray() already accepts the whole request; merge per-controller overrides with + so request keys win except where the controller intentionally pins a value.

❌ Bad

$filterData = FilterEmployeeData::fromArray([
    'email' => $request->email,
    'id' => $request->filter_admin_id,
    'winch_branch_id' => $request->filter_winch_branch_id ?? $winchBranchId,
    'city_id' => $request->filter_city_id,
    // ... many more lines, growing with every new field
]);

✅ Good

$filterData = FilterEmployeeData::fromArray($request->all() + [
    'winch_branch_id' => $winchBranchId,
    'is_active' => true,
]);

Where to fix: in src/Presentation/{Cpanel}/Controllers/. The + operator preserves left-hand keys, so put controller-pinned overrides on the right and request data on the left. See controllers and DTOs.