Skip to content

SocialAccountObserver::notifyOnboarding() lazy-loads workspace — crashes when 2+ SocialAccount rows are batch-hydrated #255

Description

@paulocastellano

Problem

App\Observers\SocialAccountObserver::notifyOnboarding() reads $socialAccount->workspace?->account without workspace being eager-loaded on the triggering model.

This is latent almost everywhere because Laravel only flips preventsLazyLoading on a hydrated batch when Builder::hydrate() loads more than one distinct row in the same query (Illuminate\Database\Eloquent\Builder::hydrate(), gated by count($items) > 1). A single-account code path never trips it. Model::shouldBeStrict(! $this->app->isProduction()) is set in app/Providers/AppServiceProvider.php:217, so:

  • Local/testing: strict mode is on → any caller that batch-processes 2+ distinct SocialAccount rows and triggers a status-changing update() (which fires the observer) throws LazyLoadingViolationException immediately.
  • Production: strict mode is off → the same code path degrades to a silent N+1 query instead of a crash.

Where this was found

Discovered while building App\Jobs\VerifyUpcomingPostConnections (PR for the proactive upcoming-post connection check). That job intentionally groups at-risk post_platforms by social_account_id and can call SocialAccount::markAsTokenExpired() on multiple distinct accounts within a single run — exactly the "2+ distinct rows hydrated, one triggers the observer" shape that surfaces the bug.

Fixed at the call site for that job only (app/Jobs/VerifyUpcomingPostConnections.php, atRiskPostPlatforms()): eager-loads socialAccount.workspace before any account in the batch can be updated. That closes the gap for this one job, not for the observer itself.

Why this needs a real fix, not another per-caller workaround

Any other current or future code path that batch-updates 2+ distinct SocialAccount rows to a different status without separately remembering to eager-load workspace will hit the same crash (locally/in tests) or silent N+1 (in production). Audited during the review of the job above — none of the other current call sites are exposed today (each operates on a single account per invocation), but that's incidental, not structural:

  • app/Jobs/RefreshSocialToken.php — single account per job, not exposed today
  • app/Jobs/PublishToSocialPlatform.php — single account per job, not exposed today
  • app/Http/Controllers/Auth/*Controller.php (OAuth connect flows) — single account per request, not exposed today
  • app/Jobs/VerifyWorkspaceConnections.php — already eager-loads workspace.owner (:34), not exposed

Suggested fix

Fix SocialAccountObserver::notifyOnboarding() itself so it doesn't depend on every caller remembering to eager-load workspace — e.g. load it inside the observer if missing ($socialAccount->loadMissing('workspace')), or restructure so the observer doesn't need the relation lazily at all.

Repro (local/testing)

Any test/factory flow that creates 2+ distinct SocialAccount rows in one query result and then triggers a status update() on more than one of them within that same hydrated batch reproduces LazyLoadingViolationException at the notifyOnboarding() call site.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions