The worst architecture decisions don't announce themselves. They ship on a Friday afternoon, pass code review, and make perfect sense for the application you're building today. Then eighteen months later, you're staring at a 900-line controller method, a model with forty-seven relationships, and a deployment pipeline that breaks every time someone touches a config file.
None of those problems arrived suddenly. They accumulated quietly, one reasonable decision at a time.
I've reviewed a lot of Laravel codebases at the point where growth starts punishing earlier shortcuts. The pattern is consistent: the mistakes aren't dramatic. They're convenient. They're the path of least resistance when the app is small, the team is small, and nobody can justify spending a day on structure that "we might not even need."
Here are the ten I see most often, what they look like before they hurt, and why they eventually do.
1. Treating controllers as the application
This is the most common one, and it's almost inevitable early on. The controller receives the request, validates the input, queries the database, calls a few services, sends a notification, and returns a response. For a small app, this is fine. Laravel's routing and controller conventions make it feel natural.
The problem is that controllers are an HTTP-layer concept. They should translate between the outside world and your application. When they become your application, every piece of business logic becomes coupled to the request/response cycle.
// This looks reasonable at first
class SubscriptionController extends Controller
{
public function upgrade(Request $request, Team $team)
{
$request->validate([
'plan' => ['required', 'in:pro,enterprise'],
]);
$plan = Plan::where('name', $request->plan)->firstOrFail();
if ($team->members()->count() > $plan->max_members) {
return response()->json(['error' => 'Too many members'], 422);
}
$team->update(['plan_id' => $plan->id]);
$team->owner->notify(new PlanUpgraded($team, $plan));
activity()
->performedOn($team)
->causedBy($request->user())
->log("Upgraded to {$plan->name}");
return response()->json(['status' => 'upgraded']);
}
}
Nothing here is wrong in isolation. But now imagine you need to upgrade a team's plan from a CLI command, a webhook handler, or an admin panel. You can't reuse any of this logic without either duplicating it or faking an HTTP request.
The fix isn't a service class for its own sake. It's extracting the action into something that doesn't know about HTTP:
class UpgradeTeamPlan
{
public function handle(Team $team, Plan $plan, User $initiatedBy): void
{
if ($team->members()->count() > $plan->max_members) {
throw new TeamExceedsPlanLimit($team, $plan);
}
$team->update(['plan_id' => $plan->id]);
$team->owner->notify(new PlanUpgraded($team, $plan));
activity()
->performedOn($team)
->causedBy($initiatedBy)
->log("Upgraded to {$plan->name}");
}
}
Now the controller is thin. The CLI command is thin. The webhook handler is thin. The logic lives in one place, and it doesn't care how it was invoked.
You don't need a formal "Actions" directory or a package to do this. You need the discipline to ask: would this logic make sense if there were no HTTP request? If not, it belongs in the controller. If yes, it doesn't.
2. Letting Eloquent models become service layers
Eloquent models are seductive. They're right there, they have access to the database, and Laravel makes it trivial to add methods. So you add a scopeActive, then a sendWelcomeEmail, then a calculateMonthlyRevenue, then a syncWithExternalCrm.
Six months later, your User model is 800 lines and does everything except make coffee.
class User extends Authenticatable
{
// 23 relationships
// 14 scopes
// 9 "helper" methods that actually contain business logic
// 4 methods that send emails
// 2 methods that call external APIs
public function calculateLifetimeValue(): float
{
return $this->invoices()
->where('status', 'paid')
->sum('amount')
- $this->refunds()->sum('amount');
}
public function sendOnboardingSequence(): void
{
// 30 lines of email scheduling logic
}
public function syncToStripe(): void
{
// 25 lines of API calls
}
}
The problem isn't that models have methods. The problem is that models are persistence objects. Their job is to represent a row in a table and its immediate relationships. When they accumulate business logic, three things happen:
Testing becomes painful. You can't test calculateLifetimeValue without booting the model, its relationships, and potentially the database.
Reusability dies. That Stripe sync logic can't be extracted into a queue job without dragging the entire model along.
The model becomes a God object. Every feature touches it. Every merge conflict involves it. Every new developer is afraid to change it.
The fix is to separate what a model is from what you can do with a model. Relationships, scopes that filter queries, and simple accessors belong on the model. Anything that orchestrates multiple steps, calls external services, or contains conditional business rules belongs elsewhere.
3. Validation in the wrong layer
Laravel's Form Requests are excellent. But they're designed for HTTP input validation, and teams often stretch them beyond that.
class StoreInvoiceRequest extends FormRequest
{
public function rules(): array
{
return [
'customer_id' => ['required', 'exists:customers,id'],
'amount' => ['required', 'numeric', 'min:0.01'],
'due_date' => ['required', 'date', 'after:today'],
];
}
public function withValidator($validator): void
{
$validator->after(function ($validator) {
// 40 lines of business rule validation
// that queries the database and checks
// account status, subscription limits,
// and billing cycles
});
}
}
The issue: this validation logic is now coupled to HTTP. If you create invoices from a queue job, a console command, or an API-to-API sync, you either duplicate the rules or skip them entirely.
Input validation (types, formats, presence) belongs in Form Requests. Business rule validation—the kind that requires querying the database, checking account state, or evaluating conditional logic—belongs in the domain layer, where it can be enforced regardless of the entry point.
A practical split:
- Form Request: "Is this input structurally valid?"
- Domain logic: "Is this operation allowed given the current state of the system?"
The second question shouldn't depend on whether the request came from a browser.
4. N+1 queries that hide behind convenience
This one doesn't look like an architecture mistake. It looks like clean code.
// In a controller or resource
$projects = Project::with('client')->get();
return ProjectResource::collection($projects);
That's fine. But then inside ProjectResource:
class ProjectResource extends JsonResource
{
public function toArray(Request $request): array
{
return [
'id' => $this->id,
'name' => $this->name,
'client' => $this->client->name,
'status' => $this->status->label,
'tasks_count' => $this->tasks()->count(),
'latest_activity' => $this->activities()->latest()->first()?->description,
'team' => $this->team->map(fn($member) => $member->name),
];
}
}
You just introduced four additional queries per project. For 20 projects, that's 81 queries. For 200, it's 801. The page still loads. The tests still pass. Nobody notices until production traffic triples and your database CPU hits 95%.
The architectural mistake isn't the missing with(). It's that the resource layer is making database decisions that should be made at the query layer. Resources should format data. They shouldn't be deciding what to fetch.
The fix is to push eager loading up to the query:
$projects = Project::with([
'client',
'status',
'team',
'activities' => fn($query) => $query->latest()->limit(1),
])->withCount('tasks')->get();
And in the resource, use the already-loaded data:
'latest_activity' => $this->activities->first()?->description,
'tasks_count' => $this->tasks_count,
This feels like a micro-optimization when you have five projects. It's an architectural decision when you have five thousand.
5. Synchronous everything until it's not
Laravel makes it trivially easy to do things synchronously. Send an email. Call an API. Generate a PDF. Resize an image. When you have ten users, all of these complete in under a second and nobody thinks twice.
The mistake isn't doing things synchronously. The mistake is structuring your code so that switching to async later requires rewriting the calling code.
// Deep inside a controller or action
$pdf = PdfGenerator::create($invoice);
Storage::put("invoices/{$invoice->id}.pdf", $pdf);
$client->notify(new InvoiceReady($invoice));
$analytics->track('invoice_generated', [
'invoice_id' => $invoice->id,
'amount' => $invoice->amount,
]);
Now imagine the PDF generation starts taking eight seconds. Or the analytics service goes down and blocks the request. Or the client notification fails and you need to retry it.
Every one of those problems requires you to find this code, wrap it in a job, and redeploy. If the logic were already structured as a dispatched job, the transition would be invisible.
The rule I use: if an operation can fail without the user needing to know about it, it should be a queued job from day one. The overhead of dispatch(new GenerateInvoicePdf($invoice)) is negligible. The overhead of retrofitting async behavior into a synchronous codebase is enormous.
6. Config files as business logic storage
Laravel's config system is designed for environment-specific settings: database credentials, API keys, feature flags. It's not designed for business logic.
But it's convenient to put things there:
// config/pricing.php
return [
'plans' => [
'starter' => [
'price' => 29,
'max_projects' => 10,
'max_members' => 5,
'features' => ['basic_reports', 'email_support'],
],
'pro' => [
'price' => 99,
'max_projects' => 100,
'max_members' => 25,
'features' => ['advanced_reports', 'priority_support', 'api_access'],
],
],
];
Then in your code:
$limits = config("pricing.plans.{$plan}.max_projects");
This works. Until pricing becomes dynamic. Until you need per-customer overrides. Until the sales team wants to change a price without a deployment. Until you need an audit trail of who changed what and when.
Config values are loaded once per request and cached by config:cache in production. They can't be changed at runtime. They can't be versioned per-customer. They can't trigger events when they change.
The fix depends on your needs. If pricing is truly static and developer-managed, config is fine. If it's business-managed, it belongs in the database with an admin interface, a Plan model, and proper change tracking.
The architectural mistake is using the wrong storage mechanism for the wrong kind of data, then discovering the mismatch when requirements change.
7. Middleware that does too much
Middleware is designed for request filtering: authentication, CORS, rate limiting, locale detection. It's a pipeline. Each middleware should make a yes/no decision about whether the request continues.
The mistake is putting business logic in middleware:
class EnsureTeamHasActiveSubscription
{
public function handle(Request $request, Closure $next): Response
{
$team = $request->route('team');
if (!$team->subscription) {
abort(402, 'No active subscription');
}
if ($team->subscription->isPastDue()) {
// Log something, maybe send a reminder
$team->owner->notify(new SubscriptionPastDue($team));
}
if ($team->subscription->isTrial() && $team->subscription->trialEndsInDays() < 3) {
// Inject trial warning into the request somehow
$request->attributes->set('trial_warning', true);
}
return $next($request);
}
}
This middleware is now doing three things: authorizing, notifying, and injecting state. The notification is a side effect that's hard to test. The injected attribute is invisible to anyone reading the controller. And the whole thing is coupled to the route structure.
Middleware should gate. It should say "yes, continue" or "no, stop." The moment it starts modifying request state, triggering side effects, or encoding business rules, it's in the wrong layer.
8. Ignoring the database schema as an architectural decision
This one is subtle. Laravel's migrations make it easy to add columns, so teams add columns. A status string here. A metadata JSON blob there. A boolean flag for every feature toggle.
The schema accumulates decisions without anyone thinking about them architecturally.
The specific patterns that cause pain later:
String columns for enumerable states. $user->status === 'active' works until you need to query "all users who aren't inactive or pending." Now you're doing whereNotIn against a list of strings that lives in a PHP constant somewhere. An enum column or a lookup table makes the state machine explicit and queryable.
JSON columns for data you need to query. Laravel's JSON column support is great for truly schemaless data. But if you find yourself writing where('metadata->priority', 'high'), that field isn't metadata. It's a column. It belongs in the schema, with an index.
Missing indexes on foreign keys. Laravel doesn't automatically index foreign key columns in every migration. If you have team_id on a projects table with two million rows and no index, every query that filters by team is doing a full table scan. This doesn't hurt at 500 rows. It's catastrophic at 500,000.
No soft-delete strategy. Adding SoftDeletes to a model is one line. Dealing with the consequences—unique constraints on soft-deleted records, query scopes that forget to include trashed items, database bloat—lasts forever. Decide early whether you need soft deletes and what the retention policy is.
The schema is the foundation. Every layer above it inherits its limitations.
9. No boundary between your code and third-party services
Every application talks to external services. Stripe. Twilio. S3. A CRM. An email provider.
The mistake isn't using those services. The mistake is calling them directly from your business logic:
class InvoiceController extends Controller
{
public function charge(Request $request, Invoice $invoice)
{
// ... validation ...
$paymentIntent = \Stripe\PaymentIntent::create([
'amount' => $invoice->amount_cents,
'currency' => 'usd',
'customer' => $invoice->customer->stripe_id,
'payment_method' => $request->payment_method,
'confirm' => true,
]);
if ($paymentIntent->status === 'succeeded') {
$invoice->markAsPaid();
}
return response()->json(['status' => $paymentIntent->status]);
}
}
Now your payment logic is coupled to Stripe's SDK, Stripe's API shape, and Stripe's error handling. If you need to add a second payment provider, support a fallback, mock payments in tests, or handle Stripe's webhook events, every change ripples through your controllers.
The fix is a boundary. Not necessarily a full "adapter pattern" with interfaces and dependency injection ceremonies—just a class that your code talks to, which talks to Stripe:
class StripePaymentGateway implements PaymentGateway
{
public function charge(Invoice $invoice, string $paymentMethod): PaymentResult
{
try {
$intent = PaymentIntent::create([
'amount' => $invoice->amount_cents,
'currency' => $invoice->currency,
'customer' => $invoice->customer->stripe_id,
'payment_method' => $paymentMethod,
'confirm' => true,
]);
return PaymentResult::fromStripeIntent($intent);
} catch (CardException $e) {
return PaymentResult::failed($e->getMessage());
}
}
}
The controller calls $this->payments->charge($invoice, $method). It doesn't know or care that Stripe exists. Tests can swap in a fake. A future provider swap is one class, not a codebase-wide find-and-replace.
10. No event strategy until you need one
Laravel's event system is powerful. Model events, custom events, listeners, subscribers, queued listeners, event broadcasting. The mistake isn't ignoring events—it's using them inconsistently.
Some teams fire events for everything. Others use events for nothing. The ones that struggle are the teams that use events sometimes, in ways that aren't predictable.
The architectural question isn't "should I use events?" It's: what is the contract between the thing that happened and the things that react to it?
If you fire InvoicePaid and three listeners react to it—one sends an email, one updates analytics, one syncs to accounting—you need to know:
- Are listeners synchronous or queued?
- What happens if one listener fails? Do the others still run?
- Can listeners be added or removed without changing the code that fires the event?
- Is the event payload a contract, or can listeners reach back into the model?
If you can't answer those questions, you don't have an event system. You have scattered side effects.
The practical approach: define your events as explicit contracts. Give them a payload that contains everything a listener needs, so listeners don't need to re-query. Decide on sync vs. queued per listener, deliberately. And treat the event class as a public API—changing its payload is a breaking change.
class InvoicePaid
{
public function __construct(
public readonly Invoice $invoice,
public readonly Payment $payment,
public readonly Carbon $paidAt,
) {}
}
Listeners receive this. They don't receive a bare model ID and go hunting. The event is the contract.
The pattern underneath all ten
Every mistake on this list shares a common root: decisions that are local and convenient but global and expensive.
A fat controller is convenient for the developer writing it today. It's expensive for every developer who needs to reuse, test, or modify that logic later.
A model that does everything is convenient when you're adding the fifth method. It's expensive when you're trying to extract a microservice or write an integration test.
Synchronous execution is convenient when you have ten users. It's expensive when you have ten thousand and need to retrofit a queue.
The fix for all of them is the same habit: before writing a piece of logic, ask where does this belong? Not "what's the fastest way to make this work?" but "if this code is still here in two years, will it still make sense?"
That question costs ten seconds. The mistakes it prevents cost months.
Top comments (0)