trypost/tests/Feature/Services/Social/ConnectionVerifierTest.php

1203 lines
48 KiB
PHP
Raw Permalink Normal View History

<?php
declare(strict_types=1);
use App\Enums\SocialAccount\Status;
use App\Exceptions\PlatformUnavailableException;
use App\Exceptions\TokenExpiredException;
use App\Models\SocialAccount;
use App\Services\Social\ConnectionVerifier;
use Illuminate\Http\Client\ConnectionException;
use Illuminate\Support\Facades\Http;
test('verifies account without refresh when token is not expired', function () {
Http::fake([
config('trypost.platforms.linkedin.api').'/*' => Http::response(['sub' => '123'], 200),
]);
$account = SocialAccount::factory()->linkedin()->create([
'token_expires_at' => now()->addDays(30),
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
Http::assertSentCount(1);
Http::assertSent(fn ($request) => str_contains($request->url(), config('trypost.platforms.linkedin.api').'/rest/userinfo'));
});
test('refreshes linkedin token before verifying when expired', function () {
Http::fake([
config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken' => Http::response([
'access_token' => 'new_token',
'refresh_token' => 'new_refresh_token',
'expires_in' => 5184000,
], 200),
config('trypost.platforms.linkedin.api').'/*' => Http::response(['sub' => '123'], 200),
]);
$account = SocialAccount::factory()->linkedin()->create([
'token_expires_at' => now()->subHour(),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
expect($account->fresh()->token_expires_at)->toBeGreaterThan(now());
Http::assertSent(fn ($request) => str_contains($request->url(), config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken'));
});
test('refreshing one linkedin row leaves a sibling linkedin-page row untouched', function () {
Http::fake([
config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken' => Http::response([
'access_token' => 'new_token',
'refresh_token' => 'new_refresh_token',
'expires_in' => 5184000,
], 200),
]);
$person = SocialAccount::factory()->linkedin()->create(['refresh_token' => 'old_refresh_token']);
$page = SocialAccount::factory()->linkedinPage()->create(['refresh_token' => 'old_refresh_token']);
(new ConnectionVerifier)->refreshToken($person);
expect($person->fresh()->refresh_token)->toBe('new_refresh_token');
expect($page->fresh()->refresh_token)->toBe('old_refresh_token');
});
test('refreshes x token before verifying when expired', function () {
Http::fake([
config('trypost.platforms.x.api').'/oauth2/token' => Http::response([
'access_token' => 'new_token',
'refresh_token' => 'new_refresh_token',
'expires_in' => 7200,
], 200),
config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200),
]);
$account = SocialAccount::factory()->x()->create([
'token_expires_at' => now()->subHour(),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
expect($account->fresh()->token_expires_at)->toBeGreaterThan(now());
Http::assertSent(fn ($request) => str_contains($request->url(), config('trypost.platforms.x.api').'/oauth2/token'));
});
test('refreshes bluesky token before verifying when expired', function () {
Http::fake([
config('trypost.platforms.bluesky.default_service').'/xrpc/com.atproto.server.refreshSession' => Http::response([
'accessJwt' => 'new_access_token',
'refreshJwt' => 'new_refresh_token',
], 200),
config('trypost.platforms.bluesky.default_service').'/xrpc/app.bsky.actor.getProfile*' => Http::response(['did' => 'did:plc:123'], 200),
]);
$account = SocialAccount::factory()->bluesky()->create([
'token_expires_at' => now()->subHour(),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
expect($account->fresh()->token_expires_at)->toBeGreaterThan(now());
Http::assertSent(fn ($request) => str_contains($request->url(), 'refreshSession'));
});
test('refreshes youtube token before verifying when expired', function () {
Http::fake([
config('trypost.platforms.youtube.oauth_api').'/token' => Http::response([
'access_token' => 'new_token',
'expires_in' => 3600,
], 200),
config('trypost.platforms.youtube.data_api').'/*' => Http::response(['items' => []], 200),
]);
$account = SocialAccount::factory()->youtube()->create([
'token_expires_at' => now()->subHour(),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
Http::assertSent(fn ($request) => str_contains($request->url(), config('trypost.platforms.youtube.oauth_api').'/token'));
});
test('refreshes tiktok token before verifying when expired', function () {
Http::fake([
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
config('trypost.platforms.tiktok.api').'/oauth/token/' => Http::response([
'access_token' => 'new_token',
'refresh_token' => 'new_refresh_token',
'expires_in' => 86400,
], 200),
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
config('trypost.platforms.tiktok.api').'/user/info/*' => Http::response(['data' => ['user' => []]], 200),
]);
$account = SocialAccount::factory()->tiktok()->create([
'token_expires_at' => now()->subHour(),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
Http::assertSent(fn ($request) => str_contains($request->url(), config('trypost.platforms.tiktok.api').'/oauth/token'));
});
test('refreshes pinterest token before verifying when expired', function () {
Http::fake([
config('trypost.platforms.pinterest.api').'/oauth/token' => Http::response([
'access_token' => 'new_token',
'refresh_token' => 'new_refresh_token',
'expires_in' => 86400,
], 200),
config('trypost.platforms.pinterest.api').'/user_account' => Http::response(['username' => 'test'], 200),
]);
$account = SocialAccount::factory()->pinterest()->create([
'token_expires_at' => now()->subHour(),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
Http::assertSent(fn ($request) => str_contains($request->url(), config('trypost.platforms.pinterest.api').'/oauth/token'));
});
test('refreshes threads token before verifying when expired', function () {
Http::fake([
config('trypost.platforms.threads.auth_api').'/refresh_access_token*' => Http::response([
'access_token' => 'new_token',
'expires_in' => 5184000,
], 200),
config('trypost.platforms.threads.graph_api').'/me*' => Http::response(['id' => '123', 'username' => 'test'], 200),
]);
$account = SocialAccount::factory()->threads()->create([
'token_expires_at' => now()->subHour(),
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
Http::assertSent(fn ($request) => str_contains($request->url(), 'refresh_access_token'));
});
test('throws exception when linkedin refresh fails', function () {
Http::fake([
config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken' => Http::response(['error' => 'invalid_grant'], 400),
]);
$account = SocialAccount::factory()->linkedin()->create([
'token_expires_at' => now()->subHour(),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect(fn () => $verifier->verify($account))
->toThrow(TokenExpiredException::class, 'Failed to refresh LinkedIn token');
});
test('throws exception when x refresh fails', function () {
Http::fake([
config('trypost.platforms.x.api').'/oauth2/token' => Http::response(['error' => 'invalid_grant'], 400),
]);
$account = SocialAccount::factory()->x()->create([
'token_expires_at' => now()->subHour(),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect(fn () => $verifier->verify($account))
->toThrow(TokenExpiredException::class, 'Failed to refresh X token');
});
test('does not refresh facebook token as it uses long-lived tokens', function () {
Http::fake([
config('trypost.platforms.facebook.graph_api').'/*' => Http::response(['id' => '123', 'name' => 'Test'], 200),
]);
$account = SocialAccount::factory()->facebook()->create([
'token_expires_at' => now()->subHour(),
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
// Should only call the verify endpoint, no refresh
Http::assertSentCount(1);
Http::assertSent(fn ($request) => str_contains($request->url(), config('trypost.platforms.facebook.graph_api')));
});
test('refreshes instagram token when expired', function () {
Http::fake([
config('trypost.platforms.instagram.auth_api').'/refresh_access_token*' => Http::response([
'access_token' => 'new-instagram-token',
'token_type' => 'bearer',
'expires_in' => 5184000,
], 200),
config('trypost.platforms.instagram.graph_api').'/me*' => Http::response(['id' => '123', 'username' => 'test'], 200),
]);
$account = SocialAccount::factory()->instagram()->create([
'token_expires_at' => now()->subHour(),
]);
$verifier = new ConnectionVerifier;
$result = $verifier->verify($account);
expect($result)->toBeTrue();
$account->refresh();
expect($account->access_token)->toBe('new-instagram-token');
expect($account->refresh_token)->toBe('new-instagram-token');
Http::assertSentCount(2);
Http::assertSent(fn ($request) => str_contains($request->url(), 'refresh_access_token'));
});
test('threads refresh records a 60-day expiry when the response omits expires_in', function () {
Http::fake([
config('trypost.platforms.threads.auth_api').'/refresh_access_token*' => Http::response([
'access_token' => 'new-threads-token',
], 200),
]);
$account = SocialAccount::factory()->threads()->create([
'token_expires_at' => now()->addHours(2),
]);
(new ConnectionVerifier)->refreshToken($account);
$account->refresh();
expect($account->token_expires_at)->not->toBeNull();
expect($account->token_expires_at->isAfter(now()->addDays(59)))->toBeTrue();
});
test('instagram refresh records a 60-day expiry when the response omits expires_in', function () {
Http::fake([
config('trypost.platforms.instagram.auth_api').'/refresh_access_token*' => Http::response([
'access_token' => 'new-instagram-token',
], 200),
]);
$account = SocialAccount::factory()->instagram()->create([
'token_expires_at' => now()->addHours(2),
]);
(new ConnectionVerifier)->refreshToken($account);
$account->refresh();
expect($account->token_expires_at)->not->toBeNull();
expect($account->token_expires_at->isAfter(now()->addDays(59)))->toBeTrue();
});
test('does NOT refresh proactively when token still works (lazy refresh)', function () {
Http::fake([
config('trypost.platforms.linkedin.api').'/*' => Http::response(['sub' => '123'], 200),
]);
// Token is "expiring soon" but access_token still works.
$account = SocialAccount::factory()->linkedin()->create([
'token_expires_at' => now()->addMinutes(10),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect($verifier->verify($account))->toBeTrue();
// Refresh endpoint must NOT have been called — verify worked without it.
Http::assertNotSent(fn ($request) => str_contains($request->url(), 'oauth/v2/accessToken'));
Http::assertSent(fn ($request) => str_contains($request->url(), config('trypost.platforms.linkedin.api').'/rest/userinfo'));
});
test('refreshes lazily on 401 then retries verify', function () {
Http::fake([
// First verify call returns 401, second (after refresh) returns 200.
config('trypost.platforms.linkedin.api').'/*' => Http::sequence()
->push(['error' => 'unauthorized'], 401)
->push(['sub' => '123'], 200),
config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken' => Http::response([
'access_token' => 'new_token',
'refresh_token' => 'new_refresh_token',
'expires_in' => 5184000,
], 200),
]);
$account = SocialAccount::factory()->linkedin()->create([
'token_expires_at' => now()->addHours(2),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect($verifier->verify($account))->toBeTrue();
Http::assertSent(fn ($request) => str_contains($request->url(), 'oauth/v2/accessToken'));
});
test('throws when verify returns 401 AND refresh also fails', function () {
Http::fake([
config('trypost.platforms.linkedin.api').'/*' => Http::response(['error' => 'unauthorized'], 401),
config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken' => Http::response(['error' => 'invalid_grant'], 400),
]);
$account = SocialAccount::factory()->linkedin()->create([
'token_expires_at' => now()->addHours(2),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect(fn () => $verifier->verify($account))->toThrow(TokenExpiredException::class);
});
test('forces refresh when token is hard-expired', function () {
Http::fake([
config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken' => Http::response([
'access_token' => 'new_token',
'refresh_token' => 'new_refresh_token',
'expires_in' => 5184000,
], 200),
config('trypost.platforms.linkedin.api').'/*' => Http::response(['sub' => '123'], 200),
]);
$account = SocialAccount::factory()->linkedin()->create([
'token_expires_at' => now()->subMinutes(5),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect($verifier->verify($account))->toBeTrue();
Http::assertSent(fn ($request) => str_contains($request->url(), 'oauth/v2/accessToken'));
});
test('throws when refresh fails AND token is hard-expired', function () {
Http::fake([
config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken' => Http::response(['error' => 'invalid_grant'], 400),
]);
$account = SocialAccount::factory()->linkedin()->create([
'token_expires_at' => now()->subMinutes(5),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect(fn () => $verifier->verify($account))->toThrow(TokenExpiredException::class);
});
test('5xx during refresh raises PlatformUnavailableException, not TokenExpiredException', function () {
$service = config('trypost.platforms.bluesky.default_service');
Http::fake([
"{$service}/xrpc/com.atproto.server.refreshSession" => Http::response('upstream timeout', 503),
]);
$account = SocialAccount::factory()->bluesky()->create([
'token_expires_at' => now()->subMinutes(5),
'refresh_token' => 'old_refresh_token',
'meta' => ['service' => $service],
]);
$verifier = new ConnectionVerifier;
expect(fn () => $verifier->refreshToken($account))->toThrow(PlatformUnavailableException::class);
});
test('connection failure during refresh raises PlatformUnavailableException', function () {
Http::fake([
config('trypost.platforms.youtube.oauth_api').'/token' => fn () => throw new ConnectionException('cURL error 7: connection refused'),
]);
$account = SocialAccount::factory()->youtube()->create([
'token_expires_at' => now()->subMinutes(5),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect(fn () => $verifier->refreshToken($account))->toThrow(PlatformUnavailableException::class);
});
test('does not disconnect when a concurrent refresh already rotated the token (lost-rotation race)', function () {
Http::fake([
// Our stale refresh_token is rejected — a concurrent process already used it.
config('trypost.platforms.x.api').'/oauth2/token' => Http::response(['error' => 'invalid_grant'], 400),
// But the access_token the winning refresh persisted still works.
config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200),
]);
$account = SocialAccount::factory()->x()->create([
'status' => Status::Connected,
'access_token' => 'stale-token',
'refresh_token' => 'already-rotated',
'token_expires_at' => now()->subHour(),
]);
// Simulate the concurrent refresh: a separate instance persists a fresh,
// valid token (through the encrypted cast) while our in-memory copy stays
// the stale, expired one.
SocialAccount::find($account->id)->update([
'access_token' => 'fresh-token',
'token_expires_at' => now()->addHours(2),
]);
expect((new ConnectionVerifier)->verify($account))->toBeTrue();
expect($account->fresh()->status)->toBe(Status::Connected);
Http::assertSent(fn ($request) => str_contains($request->url(), '/users/me')
&& $request->header('Authorization')[0] === 'Bearer fresh-token');
});
test('does not disconnect when the verify after a refresh 401s once but a fresh token is available', function () {
Http::fake([
// Refresh succeeds and rotates the token...
config('trypost.platforms.x.api').'/oauth2/token' => Http::response([
'access_token' => 'refreshed-token',
'refresh_token' => 'new-refresh',
'expires_in' => 7200,
], 200),
// ...but the verify that follows 401s once (the sub-commit window where a
// lock-skipped refresh reloads a not-yet-persisted token) before
// succeeding on the reload + retry.
config('trypost.platforms.x.api').'/users/me' => Http::sequence()
->push(['error' => 'unauthorized'], 401)
->push(['data' => ['id' => '123']], 200),
]);
$account = SocialAccount::factory()->x()->create([
'status' => Status::Connected,
'access_token' => 'stale-token',
'refresh_token' => 'old-refresh',
'token_expires_at' => now()->subHour(),
]);
expect((new ConnectionVerifier)->verify($account))->toBeTrue();
expect($account->fresh()->status)->toBe(Status::Connected);
});
test('4xx during refresh keeps raising TokenExpiredException', function () {
Http::fake([
config('trypost.platforms.x.api').'/oauth2/token' => Http::response(['error' => 'invalid_grant'], 400),
]);
$account = SocialAccount::factory()->x()->create([
'token_expires_at' => now()->subMinutes(5),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect(fn () => $verifier->refreshToken($account))->toThrow(TokenExpiredException::class);
});
test('429 during refresh raises PlatformUnavailableException (rate limit is transient)', function () {
Http::fake([
config('trypost.platforms.x.api').'/oauth2/token' => Http::response(['error' => 'rate_limit_exceeded'], 429),
]);
$account = SocialAccount::factory()->x()->create([
'token_expires_at' => now()->subMinutes(5),
'refresh_token' => 'old_refresh_token',
]);
$verifier = new ConnectionVerifier;
expect(fn () => $verifier->refreshToken($account))->toThrow(PlatformUnavailableException::class);
});
test('bluesky 5xx during refresh raises PlatformUnavailable even when password fallback is stored', function () {
$service = config('trypost.platforms.bluesky.default_service');
Http::fake([
"{$service}/xrpc/com.atproto.server.refreshSession" => Http::response('upstream timeout', 503),
"{$service}/xrpc/com.atproto.server.createSession" => Http::response('upstream timeout', 503),
]);
$account = SocialAccount::factory()->bluesky()->create([
'token_expires_at' => now()->subMinutes(5),
'refresh_token' => 'old_refresh_token',
'meta' => [
'service' => $service,
'identifier' => 'user.bsky.social',
'password' => encrypt('app-password'),
],
]);
$verifier = new ConnectionVerifier;
expect(fn () => $verifier->refreshToken($account))->toThrow(PlatformUnavailableException::class);
});
test('verifies discord by checking the bot is still in the guild', function () {
config(['trypost.platforms.discord.bot_token' => 'BOTTOKEN']);
Http::fake([
config('trypost.platforms.discord.api').'/guilds/*' => Http::response(['id' => '999000111'], 200),
]);
$account = SocialAccount::factory()->discord()->create([
'platform_user_id' => '999000111',
]);
expect((new ConnectionVerifier)->verify($account))->toBeTrue();
});
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
test('throws TokenExpiredException when the discord bot was removed from the guild', function () {
config(['trypost.platforms.discord.bot_token' => 'BOTTOKEN']);
Http::fake([
config('trypost.platforms.discord.api').'/guilds/*' => Http::response(['message' => 'Unknown Guild'], 404),
]);
$account = SocialAccount::factory()->discord()->create([
'platform_user_id' => '999000111',
]);
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
// Discord has no per-account refresh flow (one bot token shared across
// every connected account) — a confirmed rejection must not trigger a
// second, identical getGuild call against that shared budget.
Http::assertSentCount(1);
});
test('throws TokenExpiredException when the discord bot is no longer in the guild (403)', function () {
config(['trypost.platforms.discord.bot_token' => 'BOTTOKEN']);
Http::fake([
config('trypost.platforms.discord.api').'/guilds/*' => Http::response(['message' => 'Missing Access', 'code' => 50001], 403),
]);
$account = SocialAccount::factory()->discord()->create([
'platform_user_id' => '999000111',
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
Http::assertSentCount(1);
});
test('throws PlatformUnavailableException, not TokenExpiredException, when the shared discord bot token is rejected', function () {
config(['trypost.platforms.discord.bot_token' => 'BOTTOKEN']);
// 401 means the app-wide DISCORD_BOT_TOKEN is misconfigured — every
// Discord-connected account would fail identically. It is not evidence
// that THIS account's connection is broken, so it must not disconnect it.
Http::fake([
config('trypost.platforms.discord.api').'/guilds/*' => Http::response(['message' => '401: Unauthorized'], 401),
]);
$account = SocialAccount::factory()->discord()->create([
'platform_user_id' => '999000111',
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('instagram refresh treats a Meta rate-limit (400 OAuthException code 4) as transient, not a dead token', function () {
Http::fake([
config('trypost.platforms.instagram.auth_api').'/refresh_access_token*' => Http::response([
'error' => ['message' => 'Application request limit reached', 'type' => 'OAuthException', 'code' => 4],
], 400),
]);
$account = SocialAccount::factory()->instagram()->create([
'token_expires_at' => now()->subHour(),
]);
expect(fn () => (new ConnectionVerifier)->refreshToken($account))
->toThrow(PlatformUnavailableException::class);
});
test('threads refresh treats a Meta rate-limit (400 OAuthException code 17) as transient, not a dead token', function () {
Http::fake([
config('trypost.platforms.threads.auth_api').'/refresh_access_token*' => Http::response([
'error' => ['message' => 'User request limit reached', 'type' => 'OAuthException', 'code' => 17],
], 400),
]);
$account = SocialAccount::factory()->threads()->create([
'token_expires_at' => now()->subHour(),
]);
expect(fn () => (new ConnectionVerifier)->refreshToken($account))
->toThrow(PlatformUnavailableException::class);
});
test('instagram refresh treats code 190 as a genuinely expired token', function () {
Http::fake([
config('trypost.platforms.instagram.auth_api').'/refresh_access_token*' => Http::response([
'error' => ['message' => 'Access token has expired', 'type' => 'OAuthException', 'code' => 190],
], 400),
]);
$account = SocialAccount::factory()->instagram()->create([
'token_expires_at' => now()->subHour(),
]);
expect(fn () => (new ConnectionVerifier)->refreshToken($account))
->toThrow(TokenExpiredException::class);
});
fix: detect dead Threads/Instagram/Facebook tokens reported under non-190 codes (#254) * fix: detect dead Threads/Instagram/Facebook tokens reported under non-190 codes verifyThreads/verifyInstagram/verifyFacebook only threw TokenExpiredException for Meta error code 190, silently returning false for every other rejection (e.g. code 100 "The requested resource does not exist"). The hourly VerifyWorkspaceConnections check never saw that false, so a genuinely dead token went unflagged — no reconnect email — until the real scheduled post tried to publish and failed with the same raw error (#230). GraphError::isTransient() now isolates the known rate-limit/transient codes (1, 2, 4, 17); everything else on a failed verify/refresh is a confirmed rejection and raises TokenExpiredException, while transient/5xx/429 raises PlatformUnavailableException so the account isn't disconnected on a throttle. Also drops the unused $errorType variable from the three *PublishException classes. * fix: treat unparseable Meta failure bodies as transient, drop dead code Code review on #254 found two issues in the original fix: - The inverted classifier (`! GraphError::isTransient($body)`) treated a response body that fails to parse as JSON (WAF block page, truncated response, gateway hiccup) as a confirmed dead token, since isTransient() returns false for a body it can't recognize. That flipped a null/unparseable body from "retry later" (PlatformUnavailableException, the pre-fix behavior) to "disconnect now" (TokenExpiredException) for both the Threads/Instagram refresh classifiers and the verify path's classifyMetaVerifyFailure. Fixed by treating a null body as transient at both call sites — we have no confirmed rejection from Meta to act on. - GraphError::indicatesInvalidToken() had no remaining production callers after the refresh classifiers switched to isTransient() — removed it and its tests instead of leaving dead code behind. * test: symmetric Facebook/Instagram coverage for the shared verify classifier verifyInstagram/verifyFacebook/verifyThreads all delegate to the same classifyMetaVerifyFailure(), so the non-190 dead-token, rate-limit, 5xx, and non-JSON-body cases were only exercised end-to-end for Threads. Adds the missing Facebook (rate-limit, 5xx, non-JSON) and Instagram (non-190 dead token, 5xx) cases so each platform has direct proof, not just shared-code inference. * fix: recognize Business Use Case (BUC) rate-limit codes for Page-token accounts Verified the transient-code list against Meta's official docs. Confirmed: codes 1, 2, 4, 17, 190 match what's documented at developers.facebook.com/docs/graph-api/guides/error-handling/. But Meta runs a SECOND, separately-coded rate-limit system (Business Use Case / BUC) for Page and system-user tokens — which is exactly what our Facebook and InstagramFacebook accounts use. BUC rejections come back as a plain HTTP 400 (not 429) with codes in the 80000 range (80001 Pages API, 80005 Instagram Platform), which GraphError::isTransient() didn't recognize — meaning a throttled Facebook/InstagramFacebook Page token would have been misclassified as a confirmed dead token and disconnected. - Added 80001/80005 to GraphError::TRANSIENT_CODES, with sources. - Added GraphError::isTransientFailure(Response) to fold the status-based checks (5xx, 429) and body-based checks together into one documented method, replacing the ad-hoc multi-condition `if` that lived inline in ConnectionVerifier::classifyMetaVerifyFailure(). - isTransient() now treats a null (unparseable) body as transient directly, so the refresh-path classifiers no longer need a separate null guard. - Documented the full code table, sources, and per-platform token-type notes (Page token vs. user token, which rate-limit system applies to which platform) in GraphError's class docblock and in CLAUDE.md, so future changes here start from verified sources instead of guessing. * refactor: move Meta verify-failure classification into GraphError classifyMetaVerifyFailure() lived in ConnectionVerifier but never touched $this, SocialAccount, or the cache lock — it was a pure (Response, label) -> Exception translation, same shape as what TokenRefreshClient already owns for the refresh side. Keeping it in ConnectionVerifier broke that symmetry and split Meta error interpretation across two classes instead of the one (GraphError) whose docblock already says that's its job. Moved as GraphError::classifyVerifyFailure(), dropped the now-unused Response import from ConnectionVerifier, and added direct unit tests for the new public method alongside the existing ConnectionVerifierTest coverage that exercises it through verify(). * fix: correct Instagram Platform BUC code from 80005 to 80002 My earlier WebFetch of Meta's rate-limiting page mis-parsed the BUC code table and mapped 80005 to Instagram Platform. It's actually Lead Generation (Marketing API, which this app never calls) — Instagram Platform is 80002. Verified against a raw, unsummarized reproduction of the same official page (developers.facebook.com/docs/graph-api/overview/rate-limiting/) plus independent third-party corroboration, both pointing to 80002. Also closes a test-coverage gap flagged in review: GraphError::isTransient() now intentionally treats a parseable body with no "error" key (e.g. {"data": {...}}) as a confirmed rejection, not transient — a real behavior change from the pre-#254 code, which silently ignored that shape. Added explicit unit + integration coverage for it so the decision is asserted, not implicit.
2026-08-08 16:24:10 +00:00
test('instagram verify treats a Meta rate-limit (OAuthException code 4) as transient, not a disconnect', function () {
Http::fake([
config('trypost.platforms.instagram.graph_api').'/me*' => Http::response([
'error' => ['message' => 'Application request limit reached', 'type' => 'OAuthException', 'code' => 4],
], 400),
]);
// Not expired, so verify hits the endpoint directly (no refresh).
$account = SocialAccount::factory()->instagram()->create([
'token_expires_at' => now()->addDays(30),
]);
// A rate-limit must NOT raise TokenExpiredException (which would disconnect);
fix: detect dead Threads/Instagram/Facebook tokens reported under non-190 codes (#254) * fix: detect dead Threads/Instagram/Facebook tokens reported under non-190 codes verifyThreads/verifyInstagram/verifyFacebook only threw TokenExpiredException for Meta error code 190, silently returning false for every other rejection (e.g. code 100 "The requested resource does not exist"). The hourly VerifyWorkspaceConnections check never saw that false, so a genuinely dead token went unflagged — no reconnect email — until the real scheduled post tried to publish and failed with the same raw error (#230). GraphError::isTransient() now isolates the known rate-limit/transient codes (1, 2, 4, 17); everything else on a failed verify/refresh is a confirmed rejection and raises TokenExpiredException, while transient/5xx/429 raises PlatformUnavailableException so the account isn't disconnected on a throttle. Also drops the unused $errorType variable from the three *PublishException classes. * fix: treat unparseable Meta failure bodies as transient, drop dead code Code review on #254 found two issues in the original fix: - The inverted classifier (`! GraphError::isTransient($body)`) treated a response body that fails to parse as JSON (WAF block page, truncated response, gateway hiccup) as a confirmed dead token, since isTransient() returns false for a body it can't recognize. That flipped a null/unparseable body from "retry later" (PlatformUnavailableException, the pre-fix behavior) to "disconnect now" (TokenExpiredException) for both the Threads/Instagram refresh classifiers and the verify path's classifyMetaVerifyFailure. Fixed by treating a null body as transient at both call sites — we have no confirmed rejection from Meta to act on. - GraphError::indicatesInvalidToken() had no remaining production callers after the refresh classifiers switched to isTransient() — removed it and its tests instead of leaving dead code behind. * test: symmetric Facebook/Instagram coverage for the shared verify classifier verifyInstagram/verifyFacebook/verifyThreads all delegate to the same classifyMetaVerifyFailure(), so the non-190 dead-token, rate-limit, 5xx, and non-JSON-body cases were only exercised end-to-end for Threads. Adds the missing Facebook (rate-limit, 5xx, non-JSON) and Instagram (non-190 dead token, 5xx) cases so each platform has direct proof, not just shared-code inference. * fix: recognize Business Use Case (BUC) rate-limit codes for Page-token accounts Verified the transient-code list against Meta's official docs. Confirmed: codes 1, 2, 4, 17, 190 match what's documented at developers.facebook.com/docs/graph-api/guides/error-handling/. But Meta runs a SECOND, separately-coded rate-limit system (Business Use Case / BUC) for Page and system-user tokens — which is exactly what our Facebook and InstagramFacebook accounts use. BUC rejections come back as a plain HTTP 400 (not 429) with codes in the 80000 range (80001 Pages API, 80005 Instagram Platform), which GraphError::isTransient() didn't recognize — meaning a throttled Facebook/InstagramFacebook Page token would have been misclassified as a confirmed dead token and disconnected. - Added 80001/80005 to GraphError::TRANSIENT_CODES, with sources. - Added GraphError::isTransientFailure(Response) to fold the status-based checks (5xx, 429) and body-based checks together into one documented method, replacing the ad-hoc multi-condition `if` that lived inline in ConnectionVerifier::classifyMetaVerifyFailure(). - isTransient() now treats a null (unparseable) body as transient directly, so the refresh-path classifiers no longer need a separate null guard. - Documented the full code table, sources, and per-platform token-type notes (Page token vs. user token, which rate-limit system applies to which platform) in GraphError's class docblock and in CLAUDE.md, so future changes here start from verified sources instead of guessing. * refactor: move Meta verify-failure classification into GraphError classifyMetaVerifyFailure() lived in ConnectionVerifier but never touched $this, SocialAccount, or the cache lock — it was a pure (Response, label) -> Exception translation, same shape as what TokenRefreshClient already owns for the refresh side. Keeping it in ConnectionVerifier broke that symmetry and split Meta error interpretation across two classes instead of the one (GraphError) whose docblock already says that's its job. Moved as GraphError::classifyVerifyFailure(), dropped the now-unused Response import from ConnectionVerifier, and added direct unit tests for the new public method alongside the existing ConnectionVerifierTest coverage that exercises it through verify(). * fix: correct Instagram Platform BUC code from 80005 to 80002 My earlier WebFetch of Meta's rate-limiting page mis-parsed the BUC code table and mapped 80005 to Instagram Platform. It's actually Lead Generation (Marketing API, which this app never calls) — Instagram Platform is 80002. Verified against a raw, unsummarized reproduction of the same official page (developers.facebook.com/docs/graph-api/overview/rate-limiting/) plus independent third-party corroboration, both pointing to 80002. Also closes a test-coverage gap flagged in review: GraphError::isTransient() now intentionally treats a parseable body with no "error" key (e.g. {"data": {...}}) as a confirmed rejection, not transient — a real behavior change from the pre-#254 code, which silently ignored that shape. Added explicit unit + integration coverage for it so the decision is asserted, not implicit.
2026-08-08 16:24:10 +00:00
// it must be surfaced as PlatformUnavailableException so the caller retries
// next cycle instead of silently — and permanently — treating it as valid.
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('threads verify treats a dead token reported under a non-190 code as genuinely expired', function () {
// Meta doesn't always report a dead Threads token as code 190 — this
// reproduces the "(#100) The requested resource does not exist" case
// from issue #230, which the old code === 190-only check let through
// as a silent, un-flagged "still valid".
Http::fake([
config('trypost.platforms.threads.graph_api').'/me*' => Http::response([
'error' => ['message' => 'The requested resource does not exist', 'type' => 'OAuthException', 'code' => 100],
], 400),
]);
$account = SocialAccount::factory()->threads()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('threads verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.threads.graph_api').'/me*' => Http::response('upstream timeout', 503),
]);
$account = SocialAccount::factory()->threads()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('facebook verify treats a dead token reported under a non-190 code as genuinely expired', function () {
Http::fake([
config('trypost.platforms.facebook.graph_api').'/me*' => Http::response([
'error' => ['message' => 'The requested resource does not exist', 'type' => 'OAuthException', 'code' => 100],
], 400),
]);
$account = SocialAccount::factory()->facebook()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
// Facebook Page tokens don't expire — no per-account refresh flow — so a
// confirmed rejection must not trigger a second, identical /me call.
Http::assertSentCount(1);
fix: detect dead Threads/Instagram/Facebook tokens reported under non-190 codes (#254) * fix: detect dead Threads/Instagram/Facebook tokens reported under non-190 codes verifyThreads/verifyInstagram/verifyFacebook only threw TokenExpiredException for Meta error code 190, silently returning false for every other rejection (e.g. code 100 "The requested resource does not exist"). The hourly VerifyWorkspaceConnections check never saw that false, so a genuinely dead token went unflagged — no reconnect email — until the real scheduled post tried to publish and failed with the same raw error (#230). GraphError::isTransient() now isolates the known rate-limit/transient codes (1, 2, 4, 17); everything else on a failed verify/refresh is a confirmed rejection and raises TokenExpiredException, while transient/5xx/429 raises PlatformUnavailableException so the account isn't disconnected on a throttle. Also drops the unused $errorType variable from the three *PublishException classes. * fix: treat unparseable Meta failure bodies as transient, drop dead code Code review on #254 found two issues in the original fix: - The inverted classifier (`! GraphError::isTransient($body)`) treated a response body that fails to parse as JSON (WAF block page, truncated response, gateway hiccup) as a confirmed dead token, since isTransient() returns false for a body it can't recognize. That flipped a null/unparseable body from "retry later" (PlatformUnavailableException, the pre-fix behavior) to "disconnect now" (TokenExpiredException) for both the Threads/Instagram refresh classifiers and the verify path's classifyMetaVerifyFailure. Fixed by treating a null body as transient at both call sites — we have no confirmed rejection from Meta to act on. - GraphError::indicatesInvalidToken() had no remaining production callers after the refresh classifiers switched to isTransient() — removed it and its tests instead of leaving dead code behind. * test: symmetric Facebook/Instagram coverage for the shared verify classifier verifyInstagram/verifyFacebook/verifyThreads all delegate to the same classifyMetaVerifyFailure(), so the non-190 dead-token, rate-limit, 5xx, and non-JSON-body cases were only exercised end-to-end for Threads. Adds the missing Facebook (rate-limit, 5xx, non-JSON) and Instagram (non-190 dead token, 5xx) cases so each platform has direct proof, not just shared-code inference. * fix: recognize Business Use Case (BUC) rate-limit codes for Page-token accounts Verified the transient-code list against Meta's official docs. Confirmed: codes 1, 2, 4, 17, 190 match what's documented at developers.facebook.com/docs/graph-api/guides/error-handling/. But Meta runs a SECOND, separately-coded rate-limit system (Business Use Case / BUC) for Page and system-user tokens — which is exactly what our Facebook and InstagramFacebook accounts use. BUC rejections come back as a plain HTTP 400 (not 429) with codes in the 80000 range (80001 Pages API, 80005 Instagram Platform), which GraphError::isTransient() didn't recognize — meaning a throttled Facebook/InstagramFacebook Page token would have been misclassified as a confirmed dead token and disconnected. - Added 80001/80005 to GraphError::TRANSIENT_CODES, with sources. - Added GraphError::isTransientFailure(Response) to fold the status-based checks (5xx, 429) and body-based checks together into one documented method, replacing the ad-hoc multi-condition `if` that lived inline in ConnectionVerifier::classifyMetaVerifyFailure(). - isTransient() now treats a null (unparseable) body as transient directly, so the refresh-path classifiers no longer need a separate null guard. - Documented the full code table, sources, and per-platform token-type notes (Page token vs. user token, which rate-limit system applies to which platform) in GraphError's class docblock and in CLAUDE.md, so future changes here start from verified sources instead of guessing. * refactor: move Meta verify-failure classification into GraphError classifyMetaVerifyFailure() lived in ConnectionVerifier but never touched $this, SocialAccount, or the cache lock — it was a pure (Response, label) -> Exception translation, same shape as what TokenRefreshClient already owns for the refresh side. Keeping it in ConnectionVerifier broke that symmetry and split Meta error interpretation across two classes instead of the one (GraphError) whose docblock already says that's its job. Moved as GraphError::classifyVerifyFailure(), dropped the now-unused Response import from ConnectionVerifier, and added direct unit tests for the new public method alongside the existing ConnectionVerifierTest coverage that exercises it through verify(). * fix: correct Instagram Platform BUC code from 80005 to 80002 My earlier WebFetch of Meta's rate-limiting page mis-parsed the BUC code table and mapped 80005 to Instagram Platform. It's actually Lead Generation (Marketing API, which this app never calls) — Instagram Platform is 80002. Verified against a raw, unsummarized reproduction of the same official page (developers.facebook.com/docs/graph-api/overview/rate-limiting/) plus independent third-party corroboration, both pointing to 80002. Also closes a test-coverage gap flagged in review: GraphError::isTransient() now intentionally treats a parseable body with no "error" key (e.g. {"data": {...}}) as a confirmed rejection, not transient — a real behavior change from the pre-#254 code, which silently ignored that shape. Added explicit unit + integration coverage for it so the decision is asserted, not implicit.
2026-08-08 16:24:10 +00:00
});
test('facebook verify treats a Meta rate-limit as transient, not a disconnect', function () {
Http::fake([
config('trypost.platforms.facebook.graph_api').'/me*' => Http::response([
'error' => ['message' => 'Application request limit reached', 'type' => 'OAuthException', 'code' => 4],
], 400),
]);
$account = SocialAccount::factory()->facebook()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('facebook verify treats a Business Use Case rate-limit (Page token, code 80001) as transient, not a disconnect', function () {
// Facebook and InstagramFacebook accounts use Page tokens, which are
// throttled by BUC limits (code 80001) rather than Platform Rate Limits
// (codes 4/17) — and BUC rejections come back as a plain 400, not 429.
Http::fake([
config('trypost.platforms.facebook.graph_api').'/me*' => Http::response([
'error' => ['message' => 'There have been too many calls to this Page account.', 'code' => 80001],
], 400),
]);
$account = SocialAccount::factory()->facebook()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('instagram verify treats a Business Use Case rate-limit (code 80002) as transient, not a disconnect', function () {
Http::fake([
config('trypost.platforms.instagram.graph_api').'/me*' => Http::response([
'error' => ['message' => 'Instagram Platform rate limit reached.', 'code' => 80002],
], 400),
]);
$account = SocialAccount::factory()->instagram()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('facebook verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.facebook.graph_api').'/me*' => Http::response('upstream timeout', 503),
]);
$account = SocialAccount::factory()->facebook()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('facebook verify treats a non-JSON failure body as platform unavailable, not a confirmed dead token', function () {
Http::fake([
config('trypost.platforms.facebook.graph_api').'/me*' => Http::response('<html>blocked</html>', 400),
]);
$account = SocialAccount::factory()->facebook()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('instagram verify treats a dead token reported under a non-190 code as genuinely expired', function () {
Http::fake([
config('trypost.platforms.instagram.graph_api').'/me*' => Http::response([
'error' => ['message' => 'The requested resource does not exist', 'type' => 'OAuthException', 'code' => 100],
], 400),
]);
$account = SocialAccount::factory()->instagram()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('instagram verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.instagram.graph_api').'/me*' => Http::response('upstream timeout', 503),
]);
$account = SocialAccount::factory()->instagram()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('threads refresh treats a dead token reported under a non-190 code as genuinely expired', function () {
Http::fake([
config('trypost.platforms.threads.auth_api').'/refresh_access_token*' => Http::response([
'error' => ['message' => 'The requested resource does not exist', 'type' => 'OAuthException', 'code' => 100],
], 400),
]);
$account = SocialAccount::factory()->threads()->create([
'token_expires_at' => now()->subHour(),
]);
expect(fn () => (new ConnectionVerifier)->refreshToken($account))
->toThrow(TokenExpiredException::class);
});
test('threads verify treats a non-JSON failure body as platform unavailable, not a confirmed dead token', function () {
// A WAF block page, truncated response, or gateway hiccup can return a
// 4xx with a body that isn't parseable JSON. There's no confirmed
// rejection from Meta in that case, so it must not disconnect the account.
Http::fake([
config('trypost.platforms.threads.graph_api').'/me*' => Http::response('<html>blocked</html>', 400),
]);
$account = SocialAccount::factory()->threads()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('threads verify treats a failed response with a valid but unrecognized JSON shape as a confirmed rejection', function () {
// Unlike an unparseable body, a body that DID parse but has no "error"
// key at all is a real response from Meta, just not shaped like its
// usual error object. This is deliberately NOT treated as transient —
// an unrecognized 4xx shape still disconnects rather than being
// silently ignored, which is the exact bug this class replaced.
Http::fake([
config('trypost.platforms.threads.graph_api').'/me*' => Http::response(['data' => ['id' => '123']], 400),
]);
$account = SocialAccount::factory()->threads()->create([
'token_expires_at' => now()->addDays(30),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('threads refresh treats a non-JSON failure body as platform unavailable, not a confirmed dead token', function () {
Http::fake([
config('trypost.platforms.threads.auth_api').'/refresh_access_token*' => Http::response('<html>blocked</html>', 400),
]);
$account = SocialAccount::factory()->threads()->create([
'token_expires_at' => now()->subHour(),
]);
expect(fn () => (new ConnectionVerifier)->refreshToken($account))
->toThrow(PlatformUnavailableException::class);
});
test('instagram refresh treats a non-JSON failure body as platform unavailable, not a confirmed dead token', function () {
Http::fake([
config('trypost.platforms.instagram.auth_api').'/refresh_access_token*' => Http::response('<html>blocked</html>', 400),
]);
$account = SocialAccount::factory()->instagram()->create([
'token_expires_at' => now()->subHour(),
]);
expect(fn () => (new ConnectionVerifier)->refreshToken($account))
->toThrow(PlatformUnavailableException::class);
});
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
test('throws PlatformUnavailableException when the discord guild lookup fails transiently', function () {
config(['trypost.platforms.discord.bot_token' => 'BOTTOKEN']);
Http::fake([
config('trypost.platforms.discord.api').'/guilds/*' => Http::response(['message' => 'Internal Server Error'], 500),
]);
$account = SocialAccount::factory()->discord()->create([
'platform_user_id' => '999000111',
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('linkedin verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.linkedin.api').'/rest/userinfo' => Http::response(['message' => 'Internal Server Error'], 500),
]);
$account = SocialAccount::factory()->linkedin()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('linkedin verify throws TokenExpiredException on a bare 401', function () {
Http::fake([
config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken' => Http::response(['error' => 'invalid_grant'], 400),
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
config('trypost.platforms.linkedin.api').'/rest/userinfo' => Http::response(['message' => 'Unauthorized'], 401),
]);
$account = SocialAccount::factory()->linkedin()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('linkedin page verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.linkedin-page.api').'/rest/organizationAcls*' => Http::response(['message' => 'Internal Server Error'], 500),
]);
$account = SocialAccount::factory()->linkedinPage()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('linkedin page verify throws TokenExpiredException on a bare 401', function () {
Http::fake([
config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken' => Http::response(['error' => 'invalid_grant'], 400),
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
config('trypost.platforms.linkedin-page.api').'/rest/organizationAcls*' => Http::response(['message' => 'Unauthorized'], 401),
]);
$account = SocialAccount::factory()->linkedinPage()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('x verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.x.api').'/users/me' => Http::response(['title' => 'Internal Server Error'], 503),
]);
$account = SocialAccount::factory()->x()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('x verify throws TokenExpiredException on a bare 401', function () {
Http::fake([
config('trypost.platforms.x.api').'/oauth2/token' => Http::response(['error' => 'invalid_grant'], 400),
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
config('trypost.platforms.x.api').'/users/me' => Http::response(['title' => 'Unauthorized'], 401),
]);
$account = SocialAccount::factory()->x()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('x verify throws TokenExpiredException on an unsupported-authentication error type', function () {
Http::fake([
config('trypost.platforms.x.api').'/oauth2/token' => Http::response(['error' => 'invalid_grant'], 400),
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
config('trypost.platforms.x.api').'/users/me' => Http::response([
'type' => 'https://api.twitter.com/2/problems/unsupported-authentication',
'title' => 'Unsupported Authentication',
], 403),
]);
$account = SocialAccount::factory()->x()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('youtube verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.youtube.data_api').'/channels*' => Http::response(['error' => ['message' => 'Backend Error']], 500),
]);
$account = SocialAccount::factory()->youtube()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('youtube verify throws TokenExpiredException on a bare 401', function () {
Http::fake([
config('trypost.platforms.youtube.oauth_api').'/token' => Http::response(['error' => 'invalid_grant'], 400),
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
config('trypost.platforms.youtube.data_api').'/channels*' => Http::response(['error' => ['message' => 'Unauthorized']], 401),
]);
$account = SocialAccount::factory()->youtube()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('pinterest verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.pinterest.api').'/user_account' => Http::response(['message' => 'Internal Server Error'], 500),
]);
$account = SocialAccount::factory()->pinterest()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('pinterest verify throws TokenExpiredException on a bare 401', function () {
Http::fake([
config('trypost.platforms.pinterest.api').'/oauth/token' => Http::response(['error' => 'invalid_grant'], 400),
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
config('trypost.platforms.pinterest.api').'/user_account' => Http::response(['message' => 'Unauthorized'], 401),
]);
$account = SocialAccount::factory()->pinterest()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('tiktok verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.tiktok.api').'/user/info/*' => Http::response(['error' => ['code' => 'internal_error']], 500),
]);
$account = SocialAccount::factory()->tiktok()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('tiktok verify throws TokenExpiredException on a bare 401 from user/info', function () {
// /v2/user/info/ only needs the always-granted user.info.basic scope, so
// unlike a publish-time 401 (which can be a video.publish scope gap), a
// 401 here is unambiguous — the token itself is dead. A 401 also triggers
// verify()'s built-in refresh-and-retry, so the refresh endpoint needs a
// response too, even though the retried verify call fails identically.
Http::fake([
config('trypost.platforms.tiktok.api').'/oauth/token/' => Http::response([
'access_token' => 'new_token',
'refresh_token' => 'new_refresh_token',
'expires_in' => 86400,
], 200),
config('trypost.platforms.tiktok.api').'/user/info/*' => Http::response(['error' => ['code' => 'access_token_invalid']], 401),
]);
$account = SocialAccount::factory()->tiktok()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('tiktok verify throws TokenExpiredException on a bare 401 with no recognized error code', function () {
// Isolates ConnectionVerifier's own status-based check from
// TikTokPublishException::isConfirmedDeadToken()'s error-code check:
// 'access_token_revoked' IS present as error.code, but it's not one of
// the codes that check recognizes (access_token_invalid,
// access_token_expired, 10001, 10002) — so only the bare
// `$response->status() === 401` clause in verifyTikTok() can catch it.
Http::fake([
config('trypost.platforms.tiktok.api').'/oauth/token/' => Http::response([
'access_token' => 'new_token',
'refresh_token' => 'new_refresh_token',
'expires_in' => 86400,
], 200),
config('trypost.platforms.tiktok.api').'/user/info/*' => Http::response(['error' => ['code' => 'access_token_revoked']], 401),
]);
$account = SocialAccount::factory()->tiktok()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
// 1 refresh call (hits the faked 200) + verify called before, after, and
// once more when the refresh rotated the token (refreshThenVerify's own
// retry) — those three all hit the same faked 401, since the fake
// ignores the token used.
Http::assertSentCount(4);
});
test('bluesky verify treats a 5xx as platform unavailable, not a disconnect', function () {
Http::fake([
config('trypost.platforms.bluesky.default_service').'/xrpc/*' => Http::response(['message' => 'Internal Server Error'], 500),
]);
$account = SocialAccount::factory()->bluesky()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('bluesky verify throws TokenExpiredException when getProfile reports ExpiredToken', function () {
Http::fake([
// refreshSession and createSession (password re-auth fallback) both
// rejected, so refreshThenVerify exhausts every recovery path.
config('trypost.platforms.bluesky.default_service').'/xrpc/com.atproto.server.refreshSession' => Http::response(['error' => 'ExpiredToken'], 400),
config('trypost.platforms.bluesky.default_service').'/xrpc/com.atproto.server.createSession' => Http::response(['error' => 'InvalidRequest'], 400),
config('trypost.platforms.bluesky.default_service').'/xrpc/app.bsky.actor.getProfile*' => Http::response(['error' => 'ExpiredToken'], 400),
feat: proactive connection check for at-risk posts + SocialAccount name centralization (#256) * chore: gitignore .superpowers/ scratch workspace Holds per-plan subagent-driven-development artifacts (ledger, briefs, review packages) — scratch state, not part of the shipped codebase. * feat: add connection_warning_sent_at to post_platforms * feat: add PostAtRisk notification type and translations * fix: add user_id to NotificationPreferenceFactory definition for ->create() support * feat: add PostAtRisk mailable and email template * feat: add VerifyUpcomingPostConnections job * fix: guard VerifyUpcomingPostConnections against transient errors and cross-workspace leaks - Add a generic \Exception catch around ConnectionVerifier::verify() so a transient error (e.g. ConnectionException) on one account can't abort processing of every other at-risk account in the workspace run. - Eager-load socialAccount.workspace so markAsTokenExpired's observer chain never lazy-loads it — this only ever manifested once 2+ distinct accounts were hydrated in a single run (Eloquent only sets preventsLazyLoading on batch hydration of >1 row), which is exactly the multi-account scenario this job exists to handle. - Add covering tests: enabled=false posts are excluded, one workspace's at-risk posts never leak into another workspace's notification, and an unexpected exception on one account doesn't stop the rest of the run. * feat: add social:check-upcoming-connections command and schedule it * fix: add composite index for the 15-minute upcoming-post connection query post_platforms(status, connection_warning_sent_at) supports the filter both VerifyUpcomingPostConnections and social:check-upcoming-connections run every 15 minutes; without it, every run does a full table scan that only grows as posts accumulate. * fix: localize the PostAtRisk email's per-account line and label times as UTC The postsLabel line was the only hardcoded-English content in an otherwise fully-translated email, and it showed scheduled_at times with no timezone indicator even though the app stores everything in UTC. Add mail.post_at_risk.posts_label (pluralized, one entry per locale, mirroring each locale's existing post_at_risk.subject plural-boundary syntax) and use trans_choice() to build the line, with a literal " UTC" suffix left untranslated in every locale like a unit abbreviation. Also document why content() reassigns the public $atRiskGroups property instead of using a local variable (Mailable::buildViewData() overwrites with() data with same-named public properties). * fix: time-box the warning dedup and guard against orphaned/ownerless rows - Re-arm connection_warning_sent_at after a day instead of permanently suppressing it, so a post rescheduled back into the risk window after a stale warning is re-evaluated instead of silently skipped forever. - Exclude post_platforms with a null social_account_id from the at-risk query. With tries=1, dereferencing a null socialAccount relation would abort the whole workspace run, including already-detected broken accounts. - Resolve and check the workspace owner before stamping connection_warning_sent_at, so an ownerless workspace's posts are left un-warned (available to be picked up once it gets an owner) instead of being marked "warned" with no notification ever sent. Applied the same dedup time-boxing and null-account guard to the social:check-upcoming-connections dispatch query for consistency. * fix: PostAtRisk email is always English — drop the locale translation layer config('app.locale')/App::setLocale() is only ever set by the SetLocale web middleware, which reads a cookie off the incoming HTTP request. Every Mailable in this branch is built inside a queued job (SendNotification), which runs outside the HTTP request lifecycle entirely — no middleware, no cookie, nothing sets the locale there. So content() always resolved 'app.locale' to the static APP_LOCALE default ('en') regardless of the recipient's actual preference: the 16-locale mail.post_at_risk.* keys were dead weight from the start, matching an existing (pre-existing, out of scope here) gap in the sibling WorkspaceConnectionsDisconnected/ AccountDisconnected mailables. Replaces the trans_choice()/__() calls with plain English strings built directly in PostAtRisk, and removes the now-unused mail.post_at_risk.* block from all 16 locale files. Also strengthens the mailable test to assert the full "N post(s) scheduled: ... UTC" string, not just a fragment of it. * refactor: consolidate the two post_platforms migrations from this branch into one connection_warning_sent_at and its supporting index were added in two separate migrations (the column in the original task, the index during final review). Both are still unmerged/unshipped on this branch, so folding the index into the same migration that adds the column is safe and keeps the schema change to post_platforms as one unit instead of two. Verified with a full rollback + re-migrate cycle that the consolidated up()/down() is self-consistent. * refactor: add PostPlatform::scopeEnabled(), replace ->where('enabled', true) everywhere The raw where('enabled', true) clause was duplicated across 17 call sites in 12 files (13 including the 2 this branch added), all expressing the same rule PublishPost enforces at publish time: only enabled platforms are eligible. Added a scopeEnabled() to PostPlatform and swapped every query-builder call site to ->enabled(). Three call sites are intentionally left untouched: they filter an already-loaded relation Collection (->postPlatforms->where(...), no parens), which is Collection::where(), not a query scope — a query scope can't apply to an in-memory collection. No inverse (enabled = false) query pattern exists anywhere in the codebase — 'enabled' => false only ever appears as a write when a post is disabled/synced, never as a read filter — so no scopeDisabled() was added; nothing would call it. * test: cover re-armed post_platform where the account was reconnected The re-arm dedup fix (connection_warning_sent_at older than a day is treated as null) only had coverage for "still broken, warns again" and "too recent, stays skipped". Missing: the row gets re-evaluated (verify() is called, not skipped) but comes back healthy because the user reconnected in the meantime — nothing should change (no new warning, no notification, marker stays at its old value). * fix: dispatch-level uniqueness, index the enabled filter, close markAsTokenExpired race From a deep review pass on the whole branch: - VerifyUpcomingPostConnections now implements ShouldBeUnique (keyed on workspaceId, 300s window). withoutOverlapping() on the schedule only serializes the fast-dispatching command; a queue backlog could still let two jobs for the same workspace run concurrently, both mailing the owner for the same at-risk posts. - The composite index now covers enabled too (status, enabled, connection_warning_sent_at) — every query that uses it filters on all three, so the index previously required a heap fetch per row just to check enabled. - markAsTokenExpired() silently no-ops if it loses the account's status lock to a concurrent process (a publish attempt, the daily check). The job used to push the account into the at-risk notification regardless of whether the update actually landed. It now re-checks the account's status after the call and only warns if the transition is confirmed — a lost race just defers the account to the next run instead of sending a misleading "reconnect" email for an account whose status didn't change. Also includes an unrelated stray Pint fix (inline \Throwable -> imported) in SendNotification.php that had been sitting uncommitted. * refactor: centralize account handle/display name, expose to frontend, close review findings Adds SocialAccount::handle()/accountDisplayName() plus appended display_label/handle_label JSON fields, replacing duplicated username/display_name fallback logic scattered across platform previews, NetworkConnectGrid, PreviewTab, Calendar, and the post editor pages. Also closes the remaining findings from the final review on this branch: escapes the workspace name in PostAtRisk's intro (and drops the now-unnecessary raw-HTML rendering), fixes the tautological "dispatches once per workspace" test, adds plural/subject test coverage for PostAtRisk, raises VerifyUpcomingPostConnections' uniqueFor to cover the full schedule cadence, and updates a stale docblock. * test: cover draft-post exclusion, account status after PlatformUnavailableException Adds the two coverage gaps left open by the last review: a post still in Draft status inside the 1-hour window must not trigger a check or warning, and a PlatformUnavailableException must leave the account status untouched. Also drops the dedicated PostAtRisk XSS test — the intro is now plain Blade-escaped text, so the coverage is redundant with the framework's own escaping. * fix: close final review findings — i18n notification, empty-string fallback, missed refactor sites - Localize the in-app "post at risk" notification title in all 16 locales via trans_choice (the email stays English, unchanged) - Use ?: instead of ?? in handle()/accountDisplayName()/handleLabel() so an empty-string username/display_name still falls back, matching the old Vue || behavior - Migrate the 3 frontend sites the earlier sweep missed (Index.vue, SocialAccountsGrid.vue, ScheduleTab.vue) to display_label/handle_label - Fix avatar-initial fallback in the platform preview components to use display_label instead of raw display_name - Correct handle_label's TS type to string | null across 10 files to match the accessor's actual return type - Add test coverage for the command-level "already warned" dedup path and the in-app Notification row created alongside PostAtRisk's email * fix: notification storm, duplicate-email race, and queue payload bloat in upcoming-post checks Three correctness issues found by review, fixed after discussion: - An already-broken account could get a fresh PostAtRisk email every 15 minutes for as long as it stayed broken, if new posts kept entering the 1-hour risk window. Gated with a per-account 60-minute renotify cooldown. - Two concurrent jobs (RefreshExpiringTokens and this one) could each discover the same dead token and send their own email for it (AccountDisconnected + PostAtRisk) within the same tick. Gated with a 5-minute grace period, applied only when another process already transitioned the account before we got to it — not when we're the one making the transition. - PostAtRisk carried full SocialAccount/PostPlatform/Post model graphs on the queue payload, since SerializesModels can't reduce models nested inside a plain array/Collection to lightweight identifiers. It now carries only post_platform IDs and rehydrates at send time, with envelope()/content() sharing one memoized query so their counts can't disagree. Also replaces the account-health cache with a persisted SocialAccount.last_verified_at column, and narrows the actual platform API calls to only fire once a post's nearest scheduled_at is within 30 minutes — enough lead time to reconnect, without spending API budget checking a full hour out. * fix: replace dead unsubscribe link with notification preferences, finish display_label sweep The shared mail footer's unsubscribe link was permanently dead code (unsubscribe_url was never passed by any Mailable). Replaced it with a fixed "Manage notifications" link to the real settings page, via route('app.notifications.preferences'). Also closes out the remaining sites still computing the username/display_name fallback locally instead of reading the backend-computed display_label: 8 more Vue components (platform previews, per-platform post-editor settings, the AI post wizard, the automation Generate node config, and the analytics account selector) plus two PHP call sites (PostPlatform::getDisplayNameAttribute(), already fixed on main before this branch, and the template image generator's rendered footer text). * fix: only show "Manage notifications" on preference-driven emails The link doesn't make sense on transactional emails that always send regardless of notification preferences (password reset, email verification) or that go to recipients who may not even have an account yet (workspace invite) — and the settings page it points to requires login, which is actively broken for the first two. Split the shared footer into two Maizzle components: footer.html (plain) for the 3 transactional templates, footer-authenticated.html (adds the link) for the 6 that go through SendNotification and respect the recipient's notification preferences. * fix: lock PostAtRisk's subject to the dispatch-time count, expose handle_label from analytics PostAtRisk's subject/previewText were recomputed from a fresh DB query at send time, while the in-app notification's title (built in VerifyUpcomingPostConnections::notifyOwner()) used the count observed at dispatch time. If a post_platform row disappeared in between, the two could disagree. The count is now passed into the mailable explicitly and reused for both — the body's account/post details still rehydrate fresh from the DB, preserving the anti-staleness fix from earlier in this branch. Also adds handle_label to AnalyticsController's account payload, matching every other endpoint that serializes a SocialAccount. * fix: don't abort the whole workspace run if an account is deleted mid-verify An exception thrown inside a catch block isn't routed to a sibling catch, so $account->refresh() throwing ModelNotFoundException (the user disconnected/deleted the account in the brief window between this job loading it and handling the TokenExpiredException) escaped handle() entirely. With tries = 1, that killed the run for every other account in the same workspace, not just the deleted one. Also fixes an inconsistent placeholder in PlatformPreview.vue (handle_label: null instead of '', matching display_label). * fix: guard against deleted accounts, guarantee a non-empty account name Closes the last 4 findings from the sixth review round: - VerifyUpcomingPostConnections now skips a group whose account resolved to null (deleted between the main query and its eager-loaded relation), instead of an unguarded property access aborting the whole workspace's run - the same job's nested exception handler now covers any \Exception from markAsTokenExpired() (lock/DB failures), not just ModelNotFoundException - PostAtRisk drops a rehydrated group whose account no longer exists instead of crashing the render (verified: fails without the fix, passes with it) - AnalyticsController's handle_label field is now actually consumed by AnalyticsAccountSelector.vue instead of being unused payload Also closes a real gap: every connector requests enough OAuth scope to populate at least one of username/display_name (confirmed for TikTok, whose account.py comment implied otherwise but whose connect() scopes always include user.info.profile), so accountDisplayName()/handle()/ displayLabel/handleLabel now return a guaranteed non-empty string (falling back to the platform label only as a last resort) instead of being nullable. This removes the now-pointless @if guards around accountDisplayName() in the account-disconnected and post-at-risk email templates, and lets ~30 frontend files drop the `| null` from display_label/handle_label and the ?? undefined fallbacks that only existed to satisfy that type. * fix: drop the now-pointless ?? '' fallback on display_label in TemplateImageGenerator display_label is a guaranteed non-empty string (see 950558b4). * fix: correct social_account's TS type to nullable in Index.vue and Calendar.vue Both declared social_account as required while their own templates used optional chaining (pp.social_account?.display_label) — the type was lying. social_account_id is nullable and the account can be deleted (FK is nullOnDelete), so the field genuinely can be null. Swept every other social_account/socialAccount field in resources/js for the same mismatch; all others already declared it correctly. * Centralize avatar-initial extraction via getInitials() Replace hand-rolled .charAt(0)/.charAt(0).toUpperCase() avatar-initial logic across social account previews, the accounts grid, the analytics account selector, and the mention picker with the existing useInitials() composable already used by Avatar.vue. * Drop pointless display_label fallbacks now that it's always populated display_label is guaranteed non-empty (falls back to the platform label server-side), so || 'Channel' / || 'TryPost' / ?? platform were unreachable. * Fix cold-review findings: dead handle_label guard, slug leak, wrong post count - AnalyticsAccountSelector: the "@handle" line's guard/value must read the raw username (nullable — Facebook Pages and Telegram channels legitimately have none), not handle_label, which always resolves to something and made the guard permanently true. Drop the now-orphaned handle_label field from the analytics payload/type since nothing else in analytics used it. - PlatformPreview: the no-account-selected fallback now uses getPlatformLabel() instead of the raw platform slug, matching the backend's own last-resort label fallback. - VerifyUpcomingPostConnections: count distinct posts (post_id), not post_platform rows, so one post spanning multiple broken accounts doesn't inflate the at-risk count in the email subject and notification title. * Fix cold-review round 2: silent Telegram/Discord false negative, flaky email ordering, dead display_name - VerifyUpcomingPostConnections: ConnectionVerifier::verify() reports a dead Telegram/Discord connection by returning false rather than throwing. The job discarded that return value, so a bot removed from a channel/guild was stamped last_verified_at and silently trusted healthy for the next 40 minutes — no warning, post just fails at publish time. Route a false return through the same TokenExpiredException handling used by every other platform. - PostAtRisk: atRiskGroups() had no ORDER BY, so the per-account "N posts scheduled: H:i, H:i UTC" line rendered in arbitrary (physical row) order. Sort by scheduled_at before formatting. - Drop the orphaned display_name field from the analytics payload/type (superseded by display_label; nothing in resources/js/components/ analytics or pages/analytics read it). * Add social icons and copyright to email footers Icons match the trypost-site footer (outline @tabler/icons style, converted to PNG since email clients — notably Outlook desktop — don't render inline SVG). Reordered footer content: tagline, manage-notifications link, icons as the closing element, copyright line last. * Standardize connection-verify error classification across all 13 platforms Every platform now follows one contract: verify() returns true on a healthy connection, throws TokenExpiredException only on a confirmed dead connection, and PlatformUnavailableException on anything else (rate limit, 5xx, unrecognized). Previously most platforms silently returned false on anything but a 401, so callers (all of which only react via try/catch) could never distinguish "definitely dead" from "transient" — and Telegram/Discord never threw at all. Each platform's "is this confirmed dead" check now lives next to its existing publish-time error classifier (App\Exceptions\Social\*PublishException) instead of being re-typed inline in ConnectionVerifier, closing real, already-drifted gaps between the two paths: - TikTok and Mastodon both had a bare "status === 401/403" check shared between publish and verify, but TikTok's scope_not_authorized and Mastodon's write-scope 403 use the same status for a non-fatal scope gap, not a dead token — verify's lower-privilege endpoint keeps its own stricter check on top instead. - Telegram/Discord authenticate with one bot token shared across every connected account; a 401 means that shared token is misconfigured (an operator problem), never that one specific account is broken — excluded from both platforms' confirmed-dead checks accordingly. - Facebook/InstagramFacebook/Mastodon/Telegram/Discord have no per-account refresh flow at all, so a confirmed rejection now skips the pointless refresh-and-retry (Platform::hasTokenRefreshFlow()). Also fixes two bugs found while hardening VerifyUpcomingPostConnections: a post hard-deleted mid-run could crash the whole job for every other account in the batch (now filtered per group), and two overlapping runs of the same job could send duplicate PostAtRisk warnings (now a conditional claim on connection_warning_sent_at). * Skip paused accounts in upcoming-post connection checks, close claim race A paused (is_active=false) social account already fails at publish time before any platform API call, so it shouldn't trigger a proactive connection check or "reconnect" warning. Guard added at dispatch time (CheckUpcomingPostConnections) and re-checked fresh mid-run inside VerifyUpcomingPostConnections's per-account loop, since the job can take real wall-clock time working through a workspace and an account can be paused or deleted after the query-time guard already ran. Also wraps the connection_warning_sent_at claim in a SELECT ... FOR UPDATE transaction (ordered by id, 3 retries) to close a race between two overlapping runs of the same job double-claiming and double-emailing about the same post_platform. * Clarify "commit" wording in claim-transaction comment Reads ambiguously as a git commit on a PR diff; it means the DB transaction commit.
2026-08-09 14:10:39 +00:00
]);
$account = SocialAccount::factory()->bluesky()->create();
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
});
test('mastodon verify treats a 5xx as platform unavailable, not a disconnect', function () {
$account = SocialAccount::factory()->mastodon()->create();
$instance = $account->meta['instance'] ?? config('trypost.platforms.mastodon.default_instance');
Http::fake([
"{$instance}/api/v1/accounts/verify_credentials" => Http::response(['error' => 'Internal Server Error'], 503),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
test('mastodon verify throws TokenExpiredException on a bare 401, with no refresh attempt (mastodon tokens do not expire)', function () {
$account = SocialAccount::factory()->mastodon()->create();
$instance = $account->meta['instance'] ?? config('trypost.platforms.mastodon.default_instance');
Http::fake([
"{$instance}/api/v1/accounts/verify_credentials" => Http::response(['error' => 'The access token was revoked'], 401),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
// Mastodon has no per-account refresh flow — a confirmed rejection must
// not trigger a second, identical verify_credentials call.
Http::assertSentCount(1);
});
test('mastodon verify throws TokenExpiredException on a bare 403 from verify_credentials', function () {
// verify_credentials is the lowest-privilege read endpoint every
// authorized app token can reach, so unlike a publish-time 403 on the
// write-scoped /statuses endpoint (which stays a Permission-category
// publish failure), a 403 here means the app's authorization itself was
// revoked. MastodonPublishException::isConfirmedDeadToken() is 401-only
// by design — this is ConnectionVerifier's own additional check.
$account = SocialAccount::factory()->mastodon()->create();
$instance = $account->meta['instance'] ?? config('trypost.platforms.mastodon.default_instance');
Http::fake([
"{$instance}/api/v1/accounts/verify_credentials" => Http::response(['error' => 'This action is not allowed'], 403),
]);
expect(fn () => (new ConnectionVerifier)->verify($account))
->toThrow(TokenExpiredException::class);
Http::assertSentCount(1);
});