trypost/tests/Feature/Social/TelegramConnectionTest.php

548 lines
21 KiB
PHP
Raw Permalink Normal View History

<?php
declare(strict_types=1);
use App\Enums\SocialAccount\Platform;
use App\Enums\UserWorkspace\Role;
use App\Events\TelegramChannelConnected;
use App\Events\TelegramConnectFailed;
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
use App\Exceptions\PlatformUnavailableException;
use App\Exceptions\TokenExpiredException;
use App\Models\Post;
use App\Models\PostPlatform;
use App\Models\SocialAccount;
use App\Models\User;
use App\Models\Workspace;
use App\Services\Social\ConnectionVerifier;
use App\Services\Social\Telegram\TelegramConnectCode;
Allow multiple social accounts per network via env (#286) * feat: expose self-hosted mode to the accounts UI SocialAccountObserver already bypasses the one-account-per-network guard when trypost.self_hosted is true, but the frontend had no way to know that and always collapsed a network to a single card once any account existed - so self-hosted deployments could not surface a second LinkedIn (or Instagram) connection even though the backend would allow creating it. * feat: allow connecting multiple accounts per network when self-hosted NetworkConnectGrid always collapsed a network (LinkedIn profile/page, Instagram standalone/Facebook) to a single card once any account existed, with no way to trigger another OAuth flow - even though SocialAccountObserver already allows unlimited accounts per network in self-hosted mode. A self-hoster connecting their personal LinkedIn profile had no path back to the connect flow to also add a company page (or a second company page/showcase page). Render one card per connected account instead of collapsing to the first, and keep a standing "Connect another" card available for a network's existing connections when self-hosted. Hosted mode is unchanged: still one card per network, matching the backend's still-enforced one-account-per-network limit there. * test: cover the selfHosted prop on accounts and onboarding pages Backend behavior for connecting a second identity per network in self-hosted mode was already covered (LinkedInControllerTest, NetworkUniquenessTest) - these just confirm the new prop the frontend now depends on is actually present and reflects config correctly. * style: apply prettier formatting Pre-existing drift in this file unrelated to the selfHosted change. * refactor: read selfHosted from shared Inertia props The flag is already shared by HandleInertiaRequests, so the accounts and onboarding controllers do not need to pass it again. Co-authored-by: Cursor <cursoragent@cursor.com> * feat: gate multiple social accounts with a dedicated env Cloud cannot flip SELF_HOSTED, so one-per-network is now ALLOW_MULTIPLE_SOCIAL_ACCOUNTS (default false). Co-authored-by: Cursor <cursoragent@cursor.com> * fix: tighten multiple-account gates after review Keep every connected identity visible, share occupiesNetwork, and return network_taken instead of a generic connect error. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: bind reconnect to the card and unique social identity Reconnect now updates the selected account, and a unique index plus connectIdentity keep the same platform identity from being inserted twice. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: build the OAuth URL before opening the popup Keep the popup opener URL-only so reconnect query params are assembled at the call site. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: drop dead social-account guards and slim the connect grid Skip migration cleanup that production never needs, trust the platform enum in the observer, and move card theming out of the grid. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: scope social reconnect to the current network Drop dead instanceof/isset guards and filter reconnect targets in the query so a stale session cannot update another network's card. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim connectable-identity filtering Index OAuth identities by id so reconnect and occupancy use only/except instead of hand-rolled filters. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim social identity persist helpers Drop the unused occupiesNetwork exception and persist reconnects with update() instead of fill/save/fresh. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: keep reconnect updates on the original social card Co-authored-by: Cursor <cursoragent@cursor.com> * test: run the suite with multiple social accounts enabled phpunit.xml forced ALLOW_MULTIPLE_SOCIAL_ACCOUNTS=false, overriding the true value in .env.ci. That broke eight tests across Automation, MCP, PostApi, RefreshExpiringTokens and VerifyUpcomingConnections which only needed two accounts of one network as a fixture, not as a rule under test. Match .env.ci instead. Every test that exercises the one-per-network rule already sets the config itself; the accounts index test was the only one leaning on the implicit default, so it now pins it. * fix: align the multi-account fallback with the self-hosted default allow_multiple_social_accounts fell back to env('SELF_HOSTED', false) while self_hosted itself defaults to env('SELF_HOSTED', true). A self-hosted install that never wrote SELF_HOSTED to its .env resolved to false and silently lost multiple accounts per network on upgrade, which is the opposite of what the documented fallback promises. * fix: collapse duplicate identities before adding the unique index Installs predating the index can hold the same identity twice: the network guard was bypassed for multi-account installs and Pinterest always created a fresh row. Creating the index on that data aborts migrate mid-deploy. Keep the newest row per identity and move its post_platforms over before dropping the duplicates - the FK is nullOnDelete, so deleting outright would orphan drafts and scheduled posts. * fix: refuse a reconnect that authorized a different identity connectIdentity overwrote platform_user_id with whatever the provider returned, so reconnecting a card while signed into another account repointed the row - and every draft and scheduled post bound to it - at a stranger. LinkedIn guarded this at the controller and Facebook via its filtered page list; nothing covered X, TikTok, Threads, Discord, Bluesky, Mastodon, Pinterest, Instagram or Telegram. Enforce the identity match at the single choke point every connect flow goes through. Every call site already maps NetworkAlreadyConnectedException to network_taken, so the refusal surfaces without new plumbing. Also restore the null-platform guard in the observer: occupiesNetwork type-hints a non-nullable Platform, so a row without one died with a TypeError instead of the database's NOT NULL error. * fix: filter connectable identities on every picker step YouTubeController::select re-fetched the channels and matched the posted id straight off the raw list, unlike callback and selectChannel. With a live youtube_oauth session it let a POST name any channel the Google account owns and bind it to the reconnect target. It also read the reconnect from the session while the connect below it read youtube_oauth.reconnect_id, so the two could disagree - pass the resolved account through instead. filterConnectableIdentities also short-circuited in multi-account mode, and the unique index is scoped to platform rather than network. That let one Instagram account connect twice, once directly and once via Facebook, publishing every Instagram post to it twice. The existing except() already spans networkPlatformValues(), so dropping the short-circuit closes it. * refactor: type the connect cards and drop the dead accounts grid The cards computed inferred account as a required ConnectedAccount and then pushed undefined onto it (TS2345). CI only runs eslint so it stayed green, but vue-tsc and editors flag it. SocialAccountsGrid is referenced nowhere; its reconnect button was updated in this branch without passing the card id, which would have been a bug had anything rendered it. * test: keep the suite on the cloud one-account-per-network default CI runs the Cloud build, so the suite baseline should be the Cloud default rather than the self-hosted one. Put phpunit.xml and .env.ci back to false and make the eight tests that merely need two accounts of one network as a fixture opt in for themselves. This also un-deads the config()->set(true) calls the branch had already added to AuthenticationTest, SyncAccountUsageTest, HasUsageTraitTest and SocialAccountObserverTest, which the forced true had turned into no-ops. * fix: connect standalone instagram instead of reopening the picker The picker emits an already-resolved connect method, but this branch rewired @select from openOAuthPopup to startConnect. startConnect sends a bare 'instagram' straight back into its own picker branch, so choosing "Instagram" closed the dialog and immediately reopened it - the OAuth window never opened and the standalone flow was unreachable. Only the via-Facebook button still worked. Split the URL-opening tail out of startConnect and let the dialog call that directly. * fix: reject a telegram reconnect before burning the connect code The nonce was consumed before connectIdentity ran, so posting /connect in the wrong chat spent the one-off code and forced the user to generate a new one. Check the identity first and report wrong_chat instead of network_taken, which told them to disconnect an account when the real fix was posting in the channel they were reconnecting. * fix: leave one target per post when merging duplicate accounts post_platforms has no unique on (post_id, social_account_id), so a post holding a row per duplicate account ended up with two enabled rows aimed at the surviving account and would publish to it twice. Keep one row per post, preferring a published one so history survives. * refactor: collapse the repeated connect-flow boilerplate Four shapes were copy-pasted across the connect controllers: - the session + permission guard opening 16 actions, now connectWorkspace() throwing a ConnectPopupException that renders the popup itself - the reconnected/connected ternary in 13 places, now connectedCallback() - the "nothing left to connect" branch in 4 places, now noConnectableIdentities() - validatedReconnectId() re-querying what reconnectAccount() already does Facebook, Instagram-via-Facebook and YouTube also re-queried the reconnect account three or four times per callback; it is resolved once and passed down. The three GET pickers skipped the manageAccounts check their POST siblings had, and pick it up from the shared guard. Drops the color key from connectableOptions and the matching frontend field - nothing read it. Platform::color() stays; the disconnection emails use it. * fix: keep an expired connect popup out of the error log ConnectPopupException escapes to the framework handler so it can render itself, which also meant report() ran first: every session_expired and workspace_not_found popup filed an ERROR and a Nightwatch issue for what used to be a silent return. A stale popup is a normal outcome, so it now implements ShouldntReport. The Mastodon and Threads guards also cleared their provider session after connectWorkspace(), so a workspace that vanished mid-flow left the client secret and the OAuth state behind. Clear first, then resolve. clearMastodonSession() no longer touches social_connect_workspace - whatever closes the popup already does. * fix: stop telling users to disconnect an account that is not the problem Two flows reused popup_callback.network_taken - "This workspace already has an account for this network. Disconnect it first." - for situations where that is neither true nor actionable: - reconnecting a card while signed into a different account on the provider, now wrong_account - an empty picker in multi-account mode, where every page or channel on that login is simply already connected, now all_connected NetworkAlreadyConnectedException carries the message key so the catch sites stay one line. handleCallback() also drops its $platform argument; it read $this->platform for the reconnect lookup and the identity filter either way, so a caller passing a different platform would have scoped the lookup to the wrong network. * refactor: filter linkedin identities with the shared helper The picker hand-rolled its own reconnect narrowing because the profile and the pages arrive in two different shapes. Flatten them into one pool of LinkedIn identities, run the shared filter, and split them again for the view - the same path Facebook, YouTube and Instagram already take. Side effect worth having: the picker previously only narrowed on a reconnect, so it would offer an identity that is already connected and only fail once the user picked it. It now hides taken identities up front and says so when nothing is left. * fix: keep the linkedin picker's own empty state Routing the picker through the shared filter made every empty pool look like "nothing left to take", including the pool LinkedIn never filled. A self-hoster running pages-only who administers no page was told the network was already connected, or that every account on the login was taken - both false - and the picker's own "you are not an admin of any LinkedIn page" state became unreachable. Only treat it as taken when filtering is what emptied it. Splitting the pool back also compared the person id loosely on one side and strictly on the other; one predicate now drives both. Threads had two forget() calls for a key the top of the action already clears, and YouTube's picker resolved the reconnect account twice on the failure path. * fix: keep the enabled row when collapsing duplicate post targets SyncPostPlatforms seeds a disabled post_platforms row for every account in the workspace, so the usual duplicate is one row the user actually checked next to one they never saw - both pending, both created in the same second. Ordering only by published-then-newest made that a coin flip, and PublishPost iterates enabled() only, so half the time a scheduled post would silently stop reaching that account and take its caption and per-platform meta with it. This runs once against production data and the dropped row is gone, so enabled now beats disabled. Also: the empty-pool exit from the LinkedIn picker was the only one leaving linkedin_pending - and its tokens - in the session. The rationale comments move to the docblocks they belong in, and usePage() comes out of the cards computed. * fix: stop the migration destroying publish history and automations Two ways the one-shot merge lost data that cannot be rebuilt: Surplus published post_platforms rows were deleted. Two duplicate accounts really could each have published, and each row carries the platform_post_id for a live post on the network - dropping one leaves that post unmanageable and invisible to metrics. The docblock claimed published beat everything; now the code does, and only unpublished repeats collapse. Automation nodes persist social_account_id inside a JSON column with no foreign key, so deleting the loser left RunGenerateNode skipping that target, or generating nothing at all when it was the node's only account. The ids are rewritten - current and legacy shapes both - and entries the merge just turned into duplicates are collapsed. Ordering is now total (null created_at sorts oldest on every engine, then id) so a rehearsal on a replica keeps the same rows as the real run. The LinkedIn picker also passes onboardingProgress inline: it clears linkedin_pending on the empty path, and a deferred reload would re-GET the route and swap the empty state for a session-expired popup. * fix: make the identity merge auditable and stop a second delivery Self-hosted installs run this unattended and it cannot be undone, so each collapsed group now logs the workspace, the identity, which row was kept, which were dropped, and how many post_platforms and automations it touched. down() says plainly that it drops the index only. Two narrower fixes: A post holding a published row plus an enabled unpublished row for the same account kept both, and PostPlatform::scopeEnabled() filters on `enabled` alone with no status check - so a republish would deliver the same content to that identity twice. Once a published row exists, every unpublished repeat goes. The automation dedupe ran on every automation in the workspace, not just the ones the merge rewrote. A node legitimately holding two entries for one account under different content types would be collapsed to whichever came first in the array. It now runs only where an id was actually substituted. * test: rehearse the identity merge against a messy database Every test on this migration so far covered a case someone thought to write, which is why three separate review rounds each found a defect the earlier ones missed. This builds a deliberately messy database instead - three workspaces, four networks, one to three copies of each identity, posts mixing published, pending and failed rows across the duplicates with enabled flags varying, and automations referencing them in both the current and legacy JSON shapes - then runs the real migration and asserts what must be true afterwards rather than what happens to a particular fixture. Invariants: no duplicate identity survives, no published row is ever destroyed, no post ends up enabled twice against one account, nothing in post_platforms or automations points at a deleted account, and the newest row of each identity is the one kept. The generator is seeded, so a failure reproduces, and it asserts its own output is adversarial - roughly nine duplicate groups and fourteen published rows - so it cannot quietly degrade into passing on an empty problem. Verified by mutation: dropping the automation repoint, the published guard, or the repeated-target collapse each fails exactly the invariant that covers it. * fix: stop the youtube picker refetching itself into a cleared session HandleInertiaRequests defers onboardingProgress for anyone mid-onboarding - exactly the people connecting their first accounts - so Inertia re-GETs the picker route right after it mounts. For Facebook and Instagram that re-entry is harmless and deliberately left deferred, but YouTube calls the Google API again, and fetchChannels() turns any failure into an empty list that clears the connect session and swaps the mounted picker for an error the user cannot retry from. Same guard the LinkedIn picker already got. LinkedIn also answered a reconnect that authorized a different identity with "Page not found", including in the person branch where no page is involved. Every other platform says wrong_account, which this PR added. * refactor: drop the unreachable youtube channel picker Google's own delegation screen already lists every channel on the account and makes the user pick one before it issues the token, so channels?mine=true always answers with that single channel and count($channels) === 1 always won. The picker behind it was never reached - its Vue page was deleted back in 7c00c338 (January) and nothing broke, which is the clearest evidence it was dead. Removes selectChannel(), select(), both routes, the youtube_oauth session payload and the tests that drove them. If Google ever does return more than one, the callback connects the first and logs a warning rather than routing to a screen that no longer exists. * fix: serialize connects so two popups cannot seat one network twice The observer's occupiesNetwork() is a check-then-insert with nothing holding the gap, and the new unique index covers the identity, not the network. Two tabs finishing OAuth at the same moment for *different* identities on one network both passed the exists() check and both inserted, leaving a Cloud workspace with the two accounts the rule exists to prevent. The same-identity race was already safe - the unique violation is caught and re-queried. A database constraint cannot hold this: allow_multiple_social_accounts is a runtime flag, so the rule is on for Cloud and off for self-hosted, and an index cannot read config. Lock per workspace and network instead, the way markAsDisconnected() and ConnectionVerifier already do. This covers connectIdentity(), which every OAuth flow and the Telegram action go through. A direct create() still answers to the observer alone, and a self-hosted install running file cache across several nodes locks per node. * fix: handle a busy connect lock on the telegram path Every other caller funnels LockTimeoutException into its generic \Exception catch and closes the popup with error_connecting. Telegram has no such catch, so the new lock could 500 the webhook - and because the nonce is spent before connectIdentity runs, Telegram's retry of the same update short-circuits on the consumed code and returns without dispatching anything. The dialog would spin forever on a code that can no longer be used. Also restores coverage the picker removal dropped: the deleted select tests were the only ones driving a multi-channel response, so nothing exercised the reconnect narrowing to its own card, or multi-account mode skipping an already-connected channel. Both are back against the callback, and removing the narrowing in filterConnectableIdentities fails them. * fix: stop the instagram login seating an account already held via facebook filterConnectableIdentities() drops every identity already connected on the network, which is what keeps one Instagram account from being seated twice under its two platforms. Every flow that persists an identity ran it except the direct Instagram Login callback, so the guard only held in one direction: InstagramFacebookController refused an account already connected as `instagram`, but the reverse was allowed through. With multiple accounts per network enabled the observer's network check is bypassed and the unique index does not span platforms, so authorizing the same account through the direct flow created a second row. Both then seed a post_platform row and the post goes out twice to one account. * fix: name the real reason when a linkedin profile reconnect switches member Reconnecting a card narrows the authorized identities to that card's own, so authorizing a different LinkedIn login empties the pool. selectIdentity() reported that as "Page not found." for every card, including personal profiles where no page was ever involved. A profile reconnect has no page to be missing: an empty pool there can only mean this login is a different member. Say so with the wrong_account wording select() already uses for the same condition. Page reconnects keep page_not_found, where the organization really can be absent from the login. * fix: surface the busy telegram connect instead of a generic failure The connect lock timing out dispatches its own 'busy' reason so the dialog can tell the user to retry, but the dialog only mapped network_taken and wrong_chat and fell back to error_generic for everything else. The reason reached the browser and died there, leaving "Could not start the connection" for a case that just needs another moment. * test: cover reconnect on every flow that gained it rememberConnectSession() gave Instagram, TikTok, Threads, Mastodon and Bluesky a reconnect path they did not have before — TikTok had been actively clearing social_reconnect_id on connect — and none of them had a test for it. Facebook, LinkedIn, YouTube, X, Discord, Pinterest and Telegram already did. Each now covers both halves: authorizing the same identity refreshes the existing card and reports it as a reconnect, and authorizing a different one is refused with wrong_account instead of quietly seating a stranger on the card and every post scheduled against it. * fix: repair what a reconnect leaves behind when it cannot proceed cleanly Two things connectIdentity got wrong once the reconnect path existed. A reconnect through the other variant of a network moves the card to the new platform — same identity, different API flavor. Post targets carry their own platform snapshot, and that snapshot picks the publisher, the queue and the scopes checked before publishing. Left behind, it failed every pending post on a permission the account no longer needs: an Instagram card moved to the Facebook variant still demanded instagram_business_content_publish and stopped with "Missing permissions". Pending targets now follow the card and reset a content type the new platform cannot publish; published targets keep theirs, since they record what really went out under a platform_post_id from that API. The network lock timing out also arrived as a raw LockTimeoutException, which every OAuth callback filed through its generic catch: an error log and "Error connecting account" for the exact race the lock exists to absorb. It now carries a busy messageKey through the branch each flow already handles, the same way the Telegram path already reported it. * refactor: resolve the linkedin reconnect card once per select select() already looked the card up before deciding whether the chosen identity matches it, then connectPerson() and connectOrganization() looked it up again on their own — two identical queries per submit, and two places that could disagree about what is being reconnected. The caller passes what it already holds. * test: render the grid's multi-account branch phpunit.xml forces ALLOW_MULTIPLE_SOCIAL_ACCOUNTS false and no browser test overrode it, so the card the flag exists to add never rendered anywhere. The pair pins both sides: a taken network offers no second card when multiples are off, and offers one when they are on. * test: pin why the linkedin select guards exist connectIdentity() already refuses a mismatched reconnect and answers with the same wrong_account message, so every existing test passes with the two guards in select() deleted — which is exactly how they would get deleted. What they actually buy is skipping the avatar download that building the connect payload runs first. Both now assert the fetch never happens, so the guards fail loudly instead of looking redundant. * fix: carry retrying targets through a variant move, atomically Two holes in the move added a commit ago. It only carried pending targets, but a retrying one is not finished either — the publish job reschedules itself and reads the snapshot fresh on the next attempt, so leaving it behind meant it retried against the old variant until it exhausted its budget on a permission the account no longer needs. Failed and published targets stay put; a publishing one has a job mid-flight already working from the snapshot it read. The card and its targets also moved in three separate statements, so a crash between them left exactly the split this was meant to close. They share a transaction now. * chore: drop the dusk selectors nothing reads Laravel Dusk is not installed — no laravel/dusk requirement, no DuskTestCase, no browse(). Browser tests run on pest-plugin-browser driving Playwright, and its @selector resolves to data-testid. The 45 dusk attributes left across 18 components selected nothing. CLAUDE.md was the reason they kept coming back: it told every agent to add them. Its browser-testing section now describes the setup that exists — data-testid targeting, the wait helper these tests need because assertions do not auto-wait on SPA paint, and why BrowserTestCase keeps Vite real. Verified before removing: every @selector used in tests/Browser resolves to a data-testid, seven of them through bound :data-testid, so none depended on a dusk attribute. * chore: drop the last one-account-per-network helper hasConnectedPlatform() has no callers left anywhere — app, tests, views or routes. It sat directly above getSocialAccount(), which this branch already removed, and is the same leftover from when a workspace could hold one account per platform. --------- Co-authored-by: Paulo Castellano <paulo@castellanos.llc> Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-25 10:28:14 +00:00
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Storage;
beforeEach(function () {
config([
'trypost.platforms.telegram.bot_token' => 'TESTTOKEN',
'trypost.platforms.telegram.bot_username' => 'TryPostBot',
'trypost.platforms.telegram.webhook_secret' => 'shh-secret',
]);
$this->workspace = Workspace::factory()->create();
$this->user = User::factory()->create([
'current_workspace_id' => $this->workspace->id,
'account_id' => $this->workspace->account_id,
]);
$this->workspace->members()->attach($this->user->id, ['role' => Role::Admin->value]);
$this->user->refresh();
});
function telegramUpdate(string $code, array $chat = []): array
{
return [
'channel_post' => [
'message_id' => 5,
'chat' => array_merge([
'id' => -1001234567890,
'title' => 'My Channel',
'username' => 'mychannel',
'type' => 'channel',
], $chat),
'text' => "/connect {$code}",
],
];
}
it('issues a signed connect code carrying the workspace', function () {
$response = $this->actingAs($this->user)
->postJson(route('app.social.telegram.connect'))
->assertOk()
->assertJsonStructure(['code', 'nonce', 'bot_username', 'expires_at']);
expect($response->json('bot_username'))->toBe('TryPostBot');
expect(data_get(TelegramConnectCode::decode($response->json('code')), 'workspace_id'))
->toBe($this->workspace->id);
expect($response->json('nonce'))
->toBe(data_get(TelegramConnectCode::decode($response->json('code')), 'nonce'));
});
it('links the channel when the webhook receives a matching /connect', function () {
Http::fake();
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
$account = SocialAccount::where('workspace_id', $this->workspace->id)
->where('platform', Platform::Telegram)
->first();
expect($account)->not->toBeNull();
expect($account->platform_user_id)->toBe('-1001234567890');
expect($account->display_name)->toBe('My Channel');
expect($account->username)->toBe('mychannel');
expect(data_get($account->meta, 'chat_id'))->toBe('-1001234567890');
expect(data_get($account->meta, 'connect_nonce'))
->toBe(data_get(TelegramConnectCode::decode($code), 'nonce'));
});
it('stores the channel photo as the account avatar on connect', function () {
Storage::fake();
Http::fake([
'*/botTESTTOKEN/getChat*' => Http::response(['ok' => true, 'result' => ['photo' => ['big_file_id' => 'BIGFILE']]], 200),
'*/botTESTTOKEN/getFile*' => Http::response(['ok' => true, 'result' => ['file_path' => 'photos/file_1.jpg']], 200),
'*/file/botTESTTOKEN/photos/file_1.jpg' => Http::response('image-bytes', 200, ['Content-Type' => 'image/jpeg']),
]);
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
$account = SocialAccount::where('platform', Platform::Telegram)->first();
expect($account->getRawOriginal('avatar_url'))->not->toBeNull();
});
it('links a private channel that has no username', function () {
Http::fake();
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code, ['username' => null]))
->assertNoContent();
$account = SocialAccount::where('platform', Platform::Telegram)->first();
expect($account->username)->toBeNull();
expect($account->display_name)->toBe('My Channel');
expect(data_get($account->meta, 'username'))->toBeNull();
});
it('does not connect a second telegram channel when one is already connected', function () {
Allow multiple social accounts per network via env (#286) * feat: expose self-hosted mode to the accounts UI SocialAccountObserver already bypasses the one-account-per-network guard when trypost.self_hosted is true, but the frontend had no way to know that and always collapsed a network to a single card once any account existed - so self-hosted deployments could not surface a second LinkedIn (or Instagram) connection even though the backend would allow creating it. * feat: allow connecting multiple accounts per network when self-hosted NetworkConnectGrid always collapsed a network (LinkedIn profile/page, Instagram standalone/Facebook) to a single card once any account existed, with no way to trigger another OAuth flow - even though SocialAccountObserver already allows unlimited accounts per network in self-hosted mode. A self-hoster connecting their personal LinkedIn profile had no path back to the connect flow to also add a company page (or a second company page/showcase page). Render one card per connected account instead of collapsing to the first, and keep a standing "Connect another" card available for a network's existing connections when self-hosted. Hosted mode is unchanged: still one card per network, matching the backend's still-enforced one-account-per-network limit there. * test: cover the selfHosted prop on accounts and onboarding pages Backend behavior for connecting a second identity per network in self-hosted mode was already covered (LinkedInControllerTest, NetworkUniquenessTest) - these just confirm the new prop the frontend now depends on is actually present and reflects config correctly. * style: apply prettier formatting Pre-existing drift in this file unrelated to the selfHosted change. * refactor: read selfHosted from shared Inertia props The flag is already shared by HandleInertiaRequests, so the accounts and onboarding controllers do not need to pass it again. Co-authored-by: Cursor <cursoragent@cursor.com> * feat: gate multiple social accounts with a dedicated env Cloud cannot flip SELF_HOSTED, so one-per-network is now ALLOW_MULTIPLE_SOCIAL_ACCOUNTS (default false). Co-authored-by: Cursor <cursoragent@cursor.com> * fix: tighten multiple-account gates after review Keep every connected identity visible, share occupiesNetwork, and return network_taken instead of a generic connect error. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: bind reconnect to the card and unique social identity Reconnect now updates the selected account, and a unique index plus connectIdentity keep the same platform identity from being inserted twice. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: build the OAuth URL before opening the popup Keep the popup opener URL-only so reconnect query params are assembled at the call site. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: drop dead social-account guards and slim the connect grid Skip migration cleanup that production never needs, trust the platform enum in the observer, and move card theming out of the grid. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: scope social reconnect to the current network Drop dead instanceof/isset guards and filter reconnect targets in the query so a stale session cannot update another network's card. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim connectable-identity filtering Index OAuth identities by id so reconnect and occupancy use only/except instead of hand-rolled filters. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim social identity persist helpers Drop the unused occupiesNetwork exception and persist reconnects with update() instead of fill/save/fresh. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: keep reconnect updates on the original social card Co-authored-by: Cursor <cursoragent@cursor.com> * test: run the suite with multiple social accounts enabled phpunit.xml forced ALLOW_MULTIPLE_SOCIAL_ACCOUNTS=false, overriding the true value in .env.ci. That broke eight tests across Automation, MCP, PostApi, RefreshExpiringTokens and VerifyUpcomingConnections which only needed two accounts of one network as a fixture, not as a rule under test. Match .env.ci instead. Every test that exercises the one-per-network rule already sets the config itself; the accounts index test was the only one leaning on the implicit default, so it now pins it. * fix: align the multi-account fallback with the self-hosted default allow_multiple_social_accounts fell back to env('SELF_HOSTED', false) while self_hosted itself defaults to env('SELF_HOSTED', true). A self-hosted install that never wrote SELF_HOSTED to its .env resolved to false and silently lost multiple accounts per network on upgrade, which is the opposite of what the documented fallback promises. * fix: collapse duplicate identities before adding the unique index Installs predating the index can hold the same identity twice: the network guard was bypassed for multi-account installs and Pinterest always created a fresh row. Creating the index on that data aborts migrate mid-deploy. Keep the newest row per identity and move its post_platforms over before dropping the duplicates - the FK is nullOnDelete, so deleting outright would orphan drafts and scheduled posts. * fix: refuse a reconnect that authorized a different identity connectIdentity overwrote platform_user_id with whatever the provider returned, so reconnecting a card while signed into another account repointed the row - and every draft and scheduled post bound to it - at a stranger. LinkedIn guarded this at the controller and Facebook via its filtered page list; nothing covered X, TikTok, Threads, Discord, Bluesky, Mastodon, Pinterest, Instagram or Telegram. Enforce the identity match at the single choke point every connect flow goes through. Every call site already maps NetworkAlreadyConnectedException to network_taken, so the refusal surfaces without new plumbing. Also restore the null-platform guard in the observer: occupiesNetwork type-hints a non-nullable Platform, so a row without one died with a TypeError instead of the database's NOT NULL error. * fix: filter connectable identities on every picker step YouTubeController::select re-fetched the channels and matched the posted id straight off the raw list, unlike callback and selectChannel. With a live youtube_oauth session it let a POST name any channel the Google account owns and bind it to the reconnect target. It also read the reconnect from the session while the connect below it read youtube_oauth.reconnect_id, so the two could disagree - pass the resolved account through instead. filterConnectableIdentities also short-circuited in multi-account mode, and the unique index is scoped to platform rather than network. That let one Instagram account connect twice, once directly and once via Facebook, publishing every Instagram post to it twice. The existing except() already spans networkPlatformValues(), so dropping the short-circuit closes it. * refactor: type the connect cards and drop the dead accounts grid The cards computed inferred account as a required ConnectedAccount and then pushed undefined onto it (TS2345). CI only runs eslint so it stayed green, but vue-tsc and editors flag it. SocialAccountsGrid is referenced nowhere; its reconnect button was updated in this branch without passing the card id, which would have been a bug had anything rendered it. * test: keep the suite on the cloud one-account-per-network default CI runs the Cloud build, so the suite baseline should be the Cloud default rather than the self-hosted one. Put phpunit.xml and .env.ci back to false and make the eight tests that merely need two accounts of one network as a fixture opt in for themselves. This also un-deads the config()->set(true) calls the branch had already added to AuthenticationTest, SyncAccountUsageTest, HasUsageTraitTest and SocialAccountObserverTest, which the forced true had turned into no-ops. * fix: connect standalone instagram instead of reopening the picker The picker emits an already-resolved connect method, but this branch rewired @select from openOAuthPopup to startConnect. startConnect sends a bare 'instagram' straight back into its own picker branch, so choosing "Instagram" closed the dialog and immediately reopened it - the OAuth window never opened and the standalone flow was unreachable. Only the via-Facebook button still worked. Split the URL-opening tail out of startConnect and let the dialog call that directly. * fix: reject a telegram reconnect before burning the connect code The nonce was consumed before connectIdentity ran, so posting /connect in the wrong chat spent the one-off code and forced the user to generate a new one. Check the identity first and report wrong_chat instead of network_taken, which told them to disconnect an account when the real fix was posting in the channel they were reconnecting. * fix: leave one target per post when merging duplicate accounts post_platforms has no unique on (post_id, social_account_id), so a post holding a row per duplicate account ended up with two enabled rows aimed at the surviving account and would publish to it twice. Keep one row per post, preferring a published one so history survives. * refactor: collapse the repeated connect-flow boilerplate Four shapes were copy-pasted across the connect controllers: - the session + permission guard opening 16 actions, now connectWorkspace() throwing a ConnectPopupException that renders the popup itself - the reconnected/connected ternary in 13 places, now connectedCallback() - the "nothing left to connect" branch in 4 places, now noConnectableIdentities() - validatedReconnectId() re-querying what reconnectAccount() already does Facebook, Instagram-via-Facebook and YouTube also re-queried the reconnect account three or four times per callback; it is resolved once and passed down. The three GET pickers skipped the manageAccounts check their POST siblings had, and pick it up from the shared guard. Drops the color key from connectableOptions and the matching frontend field - nothing read it. Platform::color() stays; the disconnection emails use it. * fix: keep an expired connect popup out of the error log ConnectPopupException escapes to the framework handler so it can render itself, which also meant report() ran first: every session_expired and workspace_not_found popup filed an ERROR and a Nightwatch issue for what used to be a silent return. A stale popup is a normal outcome, so it now implements ShouldntReport. The Mastodon and Threads guards also cleared their provider session after connectWorkspace(), so a workspace that vanished mid-flow left the client secret and the OAuth state behind. Clear first, then resolve. clearMastodonSession() no longer touches social_connect_workspace - whatever closes the popup already does. * fix: stop telling users to disconnect an account that is not the problem Two flows reused popup_callback.network_taken - "This workspace already has an account for this network. Disconnect it first." - for situations where that is neither true nor actionable: - reconnecting a card while signed into a different account on the provider, now wrong_account - an empty picker in multi-account mode, where every page or channel on that login is simply already connected, now all_connected NetworkAlreadyConnectedException carries the message key so the catch sites stay one line. handleCallback() also drops its $platform argument; it read $this->platform for the reconnect lookup and the identity filter either way, so a caller passing a different platform would have scoped the lookup to the wrong network. * refactor: filter linkedin identities with the shared helper The picker hand-rolled its own reconnect narrowing because the profile and the pages arrive in two different shapes. Flatten them into one pool of LinkedIn identities, run the shared filter, and split them again for the view - the same path Facebook, YouTube and Instagram already take. Side effect worth having: the picker previously only narrowed on a reconnect, so it would offer an identity that is already connected and only fail once the user picked it. It now hides taken identities up front and says so when nothing is left. * fix: keep the linkedin picker's own empty state Routing the picker through the shared filter made every empty pool look like "nothing left to take", including the pool LinkedIn never filled. A self-hoster running pages-only who administers no page was told the network was already connected, or that every account on the login was taken - both false - and the picker's own "you are not an admin of any LinkedIn page" state became unreachable. Only treat it as taken when filtering is what emptied it. Splitting the pool back also compared the person id loosely on one side and strictly on the other; one predicate now drives both. Threads had two forget() calls for a key the top of the action already clears, and YouTube's picker resolved the reconnect account twice on the failure path. * fix: keep the enabled row when collapsing duplicate post targets SyncPostPlatforms seeds a disabled post_platforms row for every account in the workspace, so the usual duplicate is one row the user actually checked next to one they never saw - both pending, both created in the same second. Ordering only by published-then-newest made that a coin flip, and PublishPost iterates enabled() only, so half the time a scheduled post would silently stop reaching that account and take its caption and per-platform meta with it. This runs once against production data and the dropped row is gone, so enabled now beats disabled. Also: the empty-pool exit from the LinkedIn picker was the only one leaving linkedin_pending - and its tokens - in the session. The rationale comments move to the docblocks they belong in, and usePage() comes out of the cards computed. * fix: stop the migration destroying publish history and automations Two ways the one-shot merge lost data that cannot be rebuilt: Surplus published post_platforms rows were deleted. Two duplicate accounts really could each have published, and each row carries the platform_post_id for a live post on the network - dropping one leaves that post unmanageable and invisible to metrics. The docblock claimed published beat everything; now the code does, and only unpublished repeats collapse. Automation nodes persist social_account_id inside a JSON column with no foreign key, so deleting the loser left RunGenerateNode skipping that target, or generating nothing at all when it was the node's only account. The ids are rewritten - current and legacy shapes both - and entries the merge just turned into duplicates are collapsed. Ordering is now total (null created_at sorts oldest on every engine, then id) so a rehearsal on a replica keeps the same rows as the real run. The LinkedIn picker also passes onboardingProgress inline: it clears linkedin_pending on the empty path, and a deferred reload would re-GET the route and swap the empty state for a session-expired popup. * fix: make the identity merge auditable and stop a second delivery Self-hosted installs run this unattended and it cannot be undone, so each collapsed group now logs the workspace, the identity, which row was kept, which were dropped, and how many post_platforms and automations it touched. down() says plainly that it drops the index only. Two narrower fixes: A post holding a published row plus an enabled unpublished row for the same account kept both, and PostPlatform::scopeEnabled() filters on `enabled` alone with no status check - so a republish would deliver the same content to that identity twice. Once a published row exists, every unpublished repeat goes. The automation dedupe ran on every automation in the workspace, not just the ones the merge rewrote. A node legitimately holding two entries for one account under different content types would be collapsed to whichever came first in the array. It now runs only where an id was actually substituted. * test: rehearse the identity merge against a messy database Every test on this migration so far covered a case someone thought to write, which is why three separate review rounds each found a defect the earlier ones missed. This builds a deliberately messy database instead - three workspaces, four networks, one to three copies of each identity, posts mixing published, pending and failed rows across the duplicates with enabled flags varying, and automations referencing them in both the current and legacy JSON shapes - then runs the real migration and asserts what must be true afterwards rather than what happens to a particular fixture. Invariants: no duplicate identity survives, no published row is ever destroyed, no post ends up enabled twice against one account, nothing in post_platforms or automations points at a deleted account, and the newest row of each identity is the one kept. The generator is seeded, so a failure reproduces, and it asserts its own output is adversarial - roughly nine duplicate groups and fourteen published rows - so it cannot quietly degrade into passing on an empty problem. Verified by mutation: dropping the automation repoint, the published guard, or the repeated-target collapse each fails exactly the invariant that covers it. * fix: stop the youtube picker refetching itself into a cleared session HandleInertiaRequests defers onboardingProgress for anyone mid-onboarding - exactly the people connecting their first accounts - so Inertia re-GETs the picker route right after it mounts. For Facebook and Instagram that re-entry is harmless and deliberately left deferred, but YouTube calls the Google API again, and fetchChannels() turns any failure into an empty list that clears the connect session and swaps the mounted picker for an error the user cannot retry from. Same guard the LinkedIn picker already got. LinkedIn also answered a reconnect that authorized a different identity with "Page not found", including in the person branch where no page is involved. Every other platform says wrong_account, which this PR added. * refactor: drop the unreachable youtube channel picker Google's own delegation screen already lists every channel on the account and makes the user pick one before it issues the token, so channels?mine=true always answers with that single channel and count($channels) === 1 always won. The picker behind it was never reached - its Vue page was deleted back in 7c00c338 (January) and nothing broke, which is the clearest evidence it was dead. Removes selectChannel(), select(), both routes, the youtube_oauth session payload and the tests that drove them. If Google ever does return more than one, the callback connects the first and logs a warning rather than routing to a screen that no longer exists. * fix: serialize connects so two popups cannot seat one network twice The observer's occupiesNetwork() is a check-then-insert with nothing holding the gap, and the new unique index covers the identity, not the network. Two tabs finishing OAuth at the same moment for *different* identities on one network both passed the exists() check and both inserted, leaving a Cloud workspace with the two accounts the rule exists to prevent. The same-identity race was already safe - the unique violation is caught and re-queried. A database constraint cannot hold this: allow_multiple_social_accounts is a runtime flag, so the rule is on for Cloud and off for self-hosted, and an index cannot read config. Lock per workspace and network instead, the way markAsDisconnected() and ConnectionVerifier already do. This covers connectIdentity(), which every OAuth flow and the Telegram action go through. A direct create() still answers to the observer alone, and a self-hosted install running file cache across several nodes locks per node. * fix: handle a busy connect lock on the telegram path Every other caller funnels LockTimeoutException into its generic \Exception catch and closes the popup with error_connecting. Telegram has no such catch, so the new lock could 500 the webhook - and because the nonce is spent before connectIdentity runs, Telegram's retry of the same update short-circuits on the consumed code and returns without dispatching anything. The dialog would spin forever on a code that can no longer be used. Also restores coverage the picker removal dropped: the deleted select tests were the only ones driving a multi-channel response, so nothing exercised the reconnect narrowing to its own card, or multi-account mode skipping an already-connected channel. Both are back against the callback, and removing the narrowing in filterConnectableIdentities fails them. * fix: stop the instagram login seating an account already held via facebook filterConnectableIdentities() drops every identity already connected on the network, which is what keeps one Instagram account from being seated twice under its two platforms. Every flow that persists an identity ran it except the direct Instagram Login callback, so the guard only held in one direction: InstagramFacebookController refused an account already connected as `instagram`, but the reverse was allowed through. With multiple accounts per network enabled the observer's network check is bypassed and the unique index does not span platforms, so authorizing the same account through the direct flow created a second row. Both then seed a post_platform row and the post goes out twice to one account. * fix: name the real reason when a linkedin profile reconnect switches member Reconnecting a card narrows the authorized identities to that card's own, so authorizing a different LinkedIn login empties the pool. selectIdentity() reported that as "Page not found." for every card, including personal profiles where no page was ever involved. A profile reconnect has no page to be missing: an empty pool there can only mean this login is a different member. Say so with the wrong_account wording select() already uses for the same condition. Page reconnects keep page_not_found, where the organization really can be absent from the login. * fix: surface the busy telegram connect instead of a generic failure The connect lock timing out dispatches its own 'busy' reason so the dialog can tell the user to retry, but the dialog only mapped network_taken and wrong_chat and fell back to error_generic for everything else. The reason reached the browser and died there, leaving "Could not start the connection" for a case that just needs another moment. * test: cover reconnect on every flow that gained it rememberConnectSession() gave Instagram, TikTok, Threads, Mastodon and Bluesky a reconnect path they did not have before — TikTok had been actively clearing social_reconnect_id on connect — and none of them had a test for it. Facebook, LinkedIn, YouTube, X, Discord, Pinterest and Telegram already did. Each now covers both halves: authorizing the same identity refreshes the existing card and reports it as a reconnect, and authorizing a different one is refused with wrong_account instead of quietly seating a stranger on the card and every post scheduled against it. * fix: repair what a reconnect leaves behind when it cannot proceed cleanly Two things connectIdentity got wrong once the reconnect path existed. A reconnect through the other variant of a network moves the card to the new platform — same identity, different API flavor. Post targets carry their own platform snapshot, and that snapshot picks the publisher, the queue and the scopes checked before publishing. Left behind, it failed every pending post on a permission the account no longer needs: an Instagram card moved to the Facebook variant still demanded instagram_business_content_publish and stopped with "Missing permissions". Pending targets now follow the card and reset a content type the new platform cannot publish; published targets keep theirs, since they record what really went out under a platform_post_id from that API. The network lock timing out also arrived as a raw LockTimeoutException, which every OAuth callback filed through its generic catch: an error log and "Error connecting account" for the exact race the lock exists to absorb. It now carries a busy messageKey through the branch each flow already handles, the same way the Telegram path already reported it. * refactor: resolve the linkedin reconnect card once per select select() already looked the card up before deciding whether the chosen identity matches it, then connectPerson() and connectOrganization() looked it up again on their own — two identical queries per submit, and two places that could disagree about what is being reconnected. The caller passes what it already holds. * test: render the grid's multi-account branch phpunit.xml forces ALLOW_MULTIPLE_SOCIAL_ACCOUNTS false and no browser test overrode it, so the card the flag exists to add never rendered anywhere. The pair pins both sides: a taken network offers no second card when multiples are off, and offers one when they are on. * test: pin why the linkedin select guards exist connectIdentity() already refuses a mismatched reconnect and answers with the same wrong_account message, so every existing test passes with the two guards in select() deleted — which is exactly how they would get deleted. What they actually buy is skipping the avatar download that building the connect payload runs first. Both now assert the fetch never happens, so the guards fail loudly instead of looking redundant. * fix: carry retrying targets through a variant move, atomically Two holes in the move added a commit ago. It only carried pending targets, but a retrying one is not finished either — the publish job reschedules itself and reads the snapshot fresh on the next attempt, so leaving it behind meant it retried against the old variant until it exhausted its budget on a permission the account no longer needs. Failed and published targets stay put; a publishing one has a job mid-flight already working from the snapshot it read. The card and its targets also moved in three separate statements, so a crash between them left exactly the split this was meant to close. They share a transaction now. * chore: drop the dusk selectors nothing reads Laravel Dusk is not installed — no laravel/dusk requirement, no DuskTestCase, no browse(). Browser tests run on pest-plugin-browser driving Playwright, and its @selector resolves to data-testid. The 45 dusk attributes left across 18 components selected nothing. CLAUDE.md was the reason they kept coming back: it told every agent to add them. Its browser-testing section now describes the setup that exists — data-testid targeting, the wait helper these tests need because assertions do not auto-wait on SPA paint, and why BrowserTestCase keeps Vite real. Verified before removing: every @selector used in tests/Browser resolves to a data-testid, seven of them through bound :data-testid, so none depended on a dusk attribute. * chore: drop the last one-account-per-network helper hasConnectedPlatform() has no callers left anywhere — app, tests, views or routes. It sat directly above getSocialAccount(), which this branch already removed, and is the same leftover from when a workspace could hold one account per platform. --------- Co-authored-by: Paulo Castellano <paulo@castellanos.llc> Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-25 10:28:14 +00:00
config([
'trypost.self_hosted' => false,
'trypost.allow_multiple_social_accounts' => false,
]);
Event::fake([TelegramConnectFailed::class]);
SocialAccount::factory()->telegram()->create([
'workspace_id' => $this->workspace->id,
'platform_user_id' => '-1009999999999',
]);
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
expect($this->workspace->socialAccounts()->count())->toBe(1);
expect(
SocialAccount::where('platform', Platform::Telegram)->where('platform_user_id', '-1001234567890')->exists()
)->toBeFalse();
Event::assertDispatched(
TelegramConnectFailed::class,
fn (TelegramConnectFailed $event): bool => $event->workspaceId === $this->workspace->id
&& $event->reason === 'network_taken',
);
});
Allow multiple social accounts per network via env (#286) * feat: expose self-hosted mode to the accounts UI SocialAccountObserver already bypasses the one-account-per-network guard when trypost.self_hosted is true, but the frontend had no way to know that and always collapsed a network to a single card once any account existed - so self-hosted deployments could not surface a second LinkedIn (or Instagram) connection even though the backend would allow creating it. * feat: allow connecting multiple accounts per network when self-hosted NetworkConnectGrid always collapsed a network (LinkedIn profile/page, Instagram standalone/Facebook) to a single card once any account existed, with no way to trigger another OAuth flow - even though SocialAccountObserver already allows unlimited accounts per network in self-hosted mode. A self-hoster connecting their personal LinkedIn profile had no path back to the connect flow to also add a company page (or a second company page/showcase page). Render one card per connected account instead of collapsing to the first, and keep a standing "Connect another" card available for a network's existing connections when self-hosted. Hosted mode is unchanged: still one card per network, matching the backend's still-enforced one-account-per-network limit there. * test: cover the selfHosted prop on accounts and onboarding pages Backend behavior for connecting a second identity per network in self-hosted mode was already covered (LinkedInControllerTest, NetworkUniquenessTest) - these just confirm the new prop the frontend now depends on is actually present and reflects config correctly. * style: apply prettier formatting Pre-existing drift in this file unrelated to the selfHosted change. * refactor: read selfHosted from shared Inertia props The flag is already shared by HandleInertiaRequests, so the accounts and onboarding controllers do not need to pass it again. Co-authored-by: Cursor <cursoragent@cursor.com> * feat: gate multiple social accounts with a dedicated env Cloud cannot flip SELF_HOSTED, so one-per-network is now ALLOW_MULTIPLE_SOCIAL_ACCOUNTS (default false). Co-authored-by: Cursor <cursoragent@cursor.com> * fix: tighten multiple-account gates after review Keep every connected identity visible, share occupiesNetwork, and return network_taken instead of a generic connect error. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: bind reconnect to the card and unique social identity Reconnect now updates the selected account, and a unique index plus connectIdentity keep the same platform identity from being inserted twice. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: build the OAuth URL before opening the popup Keep the popup opener URL-only so reconnect query params are assembled at the call site. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: drop dead social-account guards and slim the connect grid Skip migration cleanup that production never needs, trust the platform enum in the observer, and move card theming out of the grid. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: scope social reconnect to the current network Drop dead instanceof/isset guards and filter reconnect targets in the query so a stale session cannot update another network's card. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim connectable-identity filtering Index OAuth identities by id so reconnect and occupancy use only/except instead of hand-rolled filters. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim social identity persist helpers Drop the unused occupiesNetwork exception and persist reconnects with update() instead of fill/save/fresh. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: keep reconnect updates on the original social card Co-authored-by: Cursor <cursoragent@cursor.com> * test: run the suite with multiple social accounts enabled phpunit.xml forced ALLOW_MULTIPLE_SOCIAL_ACCOUNTS=false, overriding the true value in .env.ci. That broke eight tests across Automation, MCP, PostApi, RefreshExpiringTokens and VerifyUpcomingConnections which only needed two accounts of one network as a fixture, not as a rule under test. Match .env.ci instead. Every test that exercises the one-per-network rule already sets the config itself; the accounts index test was the only one leaning on the implicit default, so it now pins it. * fix: align the multi-account fallback with the self-hosted default allow_multiple_social_accounts fell back to env('SELF_HOSTED', false) while self_hosted itself defaults to env('SELF_HOSTED', true). A self-hosted install that never wrote SELF_HOSTED to its .env resolved to false and silently lost multiple accounts per network on upgrade, which is the opposite of what the documented fallback promises. * fix: collapse duplicate identities before adding the unique index Installs predating the index can hold the same identity twice: the network guard was bypassed for multi-account installs and Pinterest always created a fresh row. Creating the index on that data aborts migrate mid-deploy. Keep the newest row per identity and move its post_platforms over before dropping the duplicates - the FK is nullOnDelete, so deleting outright would orphan drafts and scheduled posts. * fix: refuse a reconnect that authorized a different identity connectIdentity overwrote platform_user_id with whatever the provider returned, so reconnecting a card while signed into another account repointed the row - and every draft and scheduled post bound to it - at a stranger. LinkedIn guarded this at the controller and Facebook via its filtered page list; nothing covered X, TikTok, Threads, Discord, Bluesky, Mastodon, Pinterest, Instagram or Telegram. Enforce the identity match at the single choke point every connect flow goes through. Every call site already maps NetworkAlreadyConnectedException to network_taken, so the refusal surfaces without new plumbing. Also restore the null-platform guard in the observer: occupiesNetwork type-hints a non-nullable Platform, so a row without one died with a TypeError instead of the database's NOT NULL error. * fix: filter connectable identities on every picker step YouTubeController::select re-fetched the channels and matched the posted id straight off the raw list, unlike callback and selectChannel. With a live youtube_oauth session it let a POST name any channel the Google account owns and bind it to the reconnect target. It also read the reconnect from the session while the connect below it read youtube_oauth.reconnect_id, so the two could disagree - pass the resolved account through instead. filterConnectableIdentities also short-circuited in multi-account mode, and the unique index is scoped to platform rather than network. That let one Instagram account connect twice, once directly and once via Facebook, publishing every Instagram post to it twice. The existing except() already spans networkPlatformValues(), so dropping the short-circuit closes it. * refactor: type the connect cards and drop the dead accounts grid The cards computed inferred account as a required ConnectedAccount and then pushed undefined onto it (TS2345). CI only runs eslint so it stayed green, but vue-tsc and editors flag it. SocialAccountsGrid is referenced nowhere; its reconnect button was updated in this branch without passing the card id, which would have been a bug had anything rendered it. * test: keep the suite on the cloud one-account-per-network default CI runs the Cloud build, so the suite baseline should be the Cloud default rather than the self-hosted one. Put phpunit.xml and .env.ci back to false and make the eight tests that merely need two accounts of one network as a fixture opt in for themselves. This also un-deads the config()->set(true) calls the branch had already added to AuthenticationTest, SyncAccountUsageTest, HasUsageTraitTest and SocialAccountObserverTest, which the forced true had turned into no-ops. * fix: connect standalone instagram instead of reopening the picker The picker emits an already-resolved connect method, but this branch rewired @select from openOAuthPopup to startConnect. startConnect sends a bare 'instagram' straight back into its own picker branch, so choosing "Instagram" closed the dialog and immediately reopened it - the OAuth window never opened and the standalone flow was unreachable. Only the via-Facebook button still worked. Split the URL-opening tail out of startConnect and let the dialog call that directly. * fix: reject a telegram reconnect before burning the connect code The nonce was consumed before connectIdentity ran, so posting /connect in the wrong chat spent the one-off code and forced the user to generate a new one. Check the identity first and report wrong_chat instead of network_taken, which told them to disconnect an account when the real fix was posting in the channel they were reconnecting. * fix: leave one target per post when merging duplicate accounts post_platforms has no unique on (post_id, social_account_id), so a post holding a row per duplicate account ended up with two enabled rows aimed at the surviving account and would publish to it twice. Keep one row per post, preferring a published one so history survives. * refactor: collapse the repeated connect-flow boilerplate Four shapes were copy-pasted across the connect controllers: - the session + permission guard opening 16 actions, now connectWorkspace() throwing a ConnectPopupException that renders the popup itself - the reconnected/connected ternary in 13 places, now connectedCallback() - the "nothing left to connect" branch in 4 places, now noConnectableIdentities() - validatedReconnectId() re-querying what reconnectAccount() already does Facebook, Instagram-via-Facebook and YouTube also re-queried the reconnect account three or four times per callback; it is resolved once and passed down. The three GET pickers skipped the manageAccounts check their POST siblings had, and pick it up from the shared guard. Drops the color key from connectableOptions and the matching frontend field - nothing read it. Platform::color() stays; the disconnection emails use it. * fix: keep an expired connect popup out of the error log ConnectPopupException escapes to the framework handler so it can render itself, which also meant report() ran first: every session_expired and workspace_not_found popup filed an ERROR and a Nightwatch issue for what used to be a silent return. A stale popup is a normal outcome, so it now implements ShouldntReport. The Mastodon and Threads guards also cleared their provider session after connectWorkspace(), so a workspace that vanished mid-flow left the client secret and the OAuth state behind. Clear first, then resolve. clearMastodonSession() no longer touches social_connect_workspace - whatever closes the popup already does. * fix: stop telling users to disconnect an account that is not the problem Two flows reused popup_callback.network_taken - "This workspace already has an account for this network. Disconnect it first." - for situations where that is neither true nor actionable: - reconnecting a card while signed into a different account on the provider, now wrong_account - an empty picker in multi-account mode, where every page or channel on that login is simply already connected, now all_connected NetworkAlreadyConnectedException carries the message key so the catch sites stay one line. handleCallback() also drops its $platform argument; it read $this->platform for the reconnect lookup and the identity filter either way, so a caller passing a different platform would have scoped the lookup to the wrong network. * refactor: filter linkedin identities with the shared helper The picker hand-rolled its own reconnect narrowing because the profile and the pages arrive in two different shapes. Flatten them into one pool of LinkedIn identities, run the shared filter, and split them again for the view - the same path Facebook, YouTube and Instagram already take. Side effect worth having: the picker previously only narrowed on a reconnect, so it would offer an identity that is already connected and only fail once the user picked it. It now hides taken identities up front and says so when nothing is left. * fix: keep the linkedin picker's own empty state Routing the picker through the shared filter made every empty pool look like "nothing left to take", including the pool LinkedIn never filled. A self-hoster running pages-only who administers no page was told the network was already connected, or that every account on the login was taken - both false - and the picker's own "you are not an admin of any LinkedIn page" state became unreachable. Only treat it as taken when filtering is what emptied it. Splitting the pool back also compared the person id loosely on one side and strictly on the other; one predicate now drives both. Threads had two forget() calls for a key the top of the action already clears, and YouTube's picker resolved the reconnect account twice on the failure path. * fix: keep the enabled row when collapsing duplicate post targets SyncPostPlatforms seeds a disabled post_platforms row for every account in the workspace, so the usual duplicate is one row the user actually checked next to one they never saw - both pending, both created in the same second. Ordering only by published-then-newest made that a coin flip, and PublishPost iterates enabled() only, so half the time a scheduled post would silently stop reaching that account and take its caption and per-platform meta with it. This runs once against production data and the dropped row is gone, so enabled now beats disabled. Also: the empty-pool exit from the LinkedIn picker was the only one leaving linkedin_pending - and its tokens - in the session. The rationale comments move to the docblocks they belong in, and usePage() comes out of the cards computed. * fix: stop the migration destroying publish history and automations Two ways the one-shot merge lost data that cannot be rebuilt: Surplus published post_platforms rows were deleted. Two duplicate accounts really could each have published, and each row carries the platform_post_id for a live post on the network - dropping one leaves that post unmanageable and invisible to metrics. The docblock claimed published beat everything; now the code does, and only unpublished repeats collapse. Automation nodes persist social_account_id inside a JSON column with no foreign key, so deleting the loser left RunGenerateNode skipping that target, or generating nothing at all when it was the node's only account. The ids are rewritten - current and legacy shapes both - and entries the merge just turned into duplicates are collapsed. Ordering is now total (null created_at sorts oldest on every engine, then id) so a rehearsal on a replica keeps the same rows as the real run. The LinkedIn picker also passes onboardingProgress inline: it clears linkedin_pending on the empty path, and a deferred reload would re-GET the route and swap the empty state for a session-expired popup. * fix: make the identity merge auditable and stop a second delivery Self-hosted installs run this unattended and it cannot be undone, so each collapsed group now logs the workspace, the identity, which row was kept, which were dropped, and how many post_platforms and automations it touched. down() says plainly that it drops the index only. Two narrower fixes: A post holding a published row plus an enabled unpublished row for the same account kept both, and PostPlatform::scopeEnabled() filters on `enabled` alone with no status check - so a republish would deliver the same content to that identity twice. Once a published row exists, every unpublished repeat goes. The automation dedupe ran on every automation in the workspace, not just the ones the merge rewrote. A node legitimately holding two entries for one account under different content types would be collapsed to whichever came first in the array. It now runs only where an id was actually substituted. * test: rehearse the identity merge against a messy database Every test on this migration so far covered a case someone thought to write, which is why three separate review rounds each found a defect the earlier ones missed. This builds a deliberately messy database instead - three workspaces, four networks, one to three copies of each identity, posts mixing published, pending and failed rows across the duplicates with enabled flags varying, and automations referencing them in both the current and legacy JSON shapes - then runs the real migration and asserts what must be true afterwards rather than what happens to a particular fixture. Invariants: no duplicate identity survives, no published row is ever destroyed, no post ends up enabled twice against one account, nothing in post_platforms or automations points at a deleted account, and the newest row of each identity is the one kept. The generator is seeded, so a failure reproduces, and it asserts its own output is adversarial - roughly nine duplicate groups and fourteen published rows - so it cannot quietly degrade into passing on an empty problem. Verified by mutation: dropping the automation repoint, the published guard, or the repeated-target collapse each fails exactly the invariant that covers it. * fix: stop the youtube picker refetching itself into a cleared session HandleInertiaRequests defers onboardingProgress for anyone mid-onboarding - exactly the people connecting their first accounts - so Inertia re-GETs the picker route right after it mounts. For Facebook and Instagram that re-entry is harmless and deliberately left deferred, but YouTube calls the Google API again, and fetchChannels() turns any failure into an empty list that clears the connect session and swaps the mounted picker for an error the user cannot retry from. Same guard the LinkedIn picker already got. LinkedIn also answered a reconnect that authorized a different identity with "Page not found", including in the person branch where no page is involved. Every other platform says wrong_account, which this PR added. * refactor: drop the unreachable youtube channel picker Google's own delegation screen already lists every channel on the account and makes the user pick one before it issues the token, so channels?mine=true always answers with that single channel and count($channels) === 1 always won. The picker behind it was never reached - its Vue page was deleted back in 7c00c338 (January) and nothing broke, which is the clearest evidence it was dead. Removes selectChannel(), select(), both routes, the youtube_oauth session payload and the tests that drove them. If Google ever does return more than one, the callback connects the first and logs a warning rather than routing to a screen that no longer exists. * fix: serialize connects so two popups cannot seat one network twice The observer's occupiesNetwork() is a check-then-insert with nothing holding the gap, and the new unique index covers the identity, not the network. Two tabs finishing OAuth at the same moment for *different* identities on one network both passed the exists() check and both inserted, leaving a Cloud workspace with the two accounts the rule exists to prevent. The same-identity race was already safe - the unique violation is caught and re-queried. A database constraint cannot hold this: allow_multiple_social_accounts is a runtime flag, so the rule is on for Cloud and off for self-hosted, and an index cannot read config. Lock per workspace and network instead, the way markAsDisconnected() and ConnectionVerifier already do. This covers connectIdentity(), which every OAuth flow and the Telegram action go through. A direct create() still answers to the observer alone, and a self-hosted install running file cache across several nodes locks per node. * fix: handle a busy connect lock on the telegram path Every other caller funnels LockTimeoutException into its generic \Exception catch and closes the popup with error_connecting. Telegram has no such catch, so the new lock could 500 the webhook - and because the nonce is spent before connectIdentity runs, Telegram's retry of the same update short-circuits on the consumed code and returns without dispatching anything. The dialog would spin forever on a code that can no longer be used. Also restores coverage the picker removal dropped: the deleted select tests were the only ones driving a multi-channel response, so nothing exercised the reconnect narrowing to its own card, or multi-account mode skipping an already-connected channel. Both are back against the callback, and removing the narrowing in filterConnectableIdentities fails them. * fix: stop the instagram login seating an account already held via facebook filterConnectableIdentities() drops every identity already connected on the network, which is what keeps one Instagram account from being seated twice under its two platforms. Every flow that persists an identity ran it except the direct Instagram Login callback, so the guard only held in one direction: InstagramFacebookController refused an account already connected as `instagram`, but the reverse was allowed through. With multiple accounts per network enabled the observer's network check is bypassed and the unique index does not span platforms, so authorizing the same account through the direct flow created a second row. Both then seed a post_platform row and the post goes out twice to one account. * fix: name the real reason when a linkedin profile reconnect switches member Reconnecting a card narrows the authorized identities to that card's own, so authorizing a different LinkedIn login empties the pool. selectIdentity() reported that as "Page not found." for every card, including personal profiles where no page was ever involved. A profile reconnect has no page to be missing: an empty pool there can only mean this login is a different member. Say so with the wrong_account wording select() already uses for the same condition. Page reconnects keep page_not_found, where the organization really can be absent from the login. * fix: surface the busy telegram connect instead of a generic failure The connect lock timing out dispatches its own 'busy' reason so the dialog can tell the user to retry, but the dialog only mapped network_taken and wrong_chat and fell back to error_generic for everything else. The reason reached the browser and died there, leaving "Could not start the connection" for a case that just needs another moment. * test: cover reconnect on every flow that gained it rememberConnectSession() gave Instagram, TikTok, Threads, Mastodon and Bluesky a reconnect path they did not have before — TikTok had been actively clearing social_reconnect_id on connect — and none of them had a test for it. Facebook, LinkedIn, YouTube, X, Discord, Pinterest and Telegram already did. Each now covers both halves: authorizing the same identity refreshes the existing card and reports it as a reconnect, and authorizing a different one is refused with wrong_account instead of quietly seating a stranger on the card and every post scheduled against it. * fix: repair what a reconnect leaves behind when it cannot proceed cleanly Two things connectIdentity got wrong once the reconnect path existed. A reconnect through the other variant of a network moves the card to the new platform — same identity, different API flavor. Post targets carry their own platform snapshot, and that snapshot picks the publisher, the queue and the scopes checked before publishing. Left behind, it failed every pending post on a permission the account no longer needs: an Instagram card moved to the Facebook variant still demanded instagram_business_content_publish and stopped with "Missing permissions". Pending targets now follow the card and reset a content type the new platform cannot publish; published targets keep theirs, since they record what really went out under a platform_post_id from that API. The network lock timing out also arrived as a raw LockTimeoutException, which every OAuth callback filed through its generic catch: an error log and "Error connecting account" for the exact race the lock exists to absorb. It now carries a busy messageKey through the branch each flow already handles, the same way the Telegram path already reported it. * refactor: resolve the linkedin reconnect card once per select select() already looked the card up before deciding whether the chosen identity matches it, then connectPerson() and connectOrganization() looked it up again on their own — two identical queries per submit, and two places that could disagree about what is being reconnected. The caller passes what it already holds. * test: render the grid's multi-account branch phpunit.xml forces ALLOW_MULTIPLE_SOCIAL_ACCOUNTS false and no browser test overrode it, so the card the flag exists to add never rendered anywhere. The pair pins both sides: a taken network offers no second card when multiples are off, and offers one when they are on. * test: pin why the linkedin select guards exist connectIdentity() already refuses a mismatched reconnect and answers with the same wrong_account message, so every existing test passes with the two guards in select() deleted — which is exactly how they would get deleted. What they actually buy is skipping the avatar download that building the connect payload runs first. Both now assert the fetch never happens, so the guards fail loudly instead of looking redundant. * fix: carry retrying targets through a variant move, atomically Two holes in the move added a commit ago. It only carried pending targets, but a retrying one is not finished either — the publish job reschedules itself and reads the snapshot fresh on the next attempt, so leaving it behind meant it retried against the old variant until it exhausted its budget on a permission the account no longer needs. Failed and published targets stay put; a publishing one has a job mid-flight already working from the snapshot it read. The card and its targets also moved in three separate statements, so a crash between them left exactly the split this was meant to close. They share a transaction now. * chore: drop the dusk selectors nothing reads Laravel Dusk is not installed — no laravel/dusk requirement, no DuskTestCase, no browse(). Browser tests run on pest-plugin-browser driving Playwright, and its @selector resolves to data-testid. The 45 dusk attributes left across 18 components selected nothing. CLAUDE.md was the reason they kept coming back: it told every agent to add them. Its browser-testing section now describes the setup that exists — data-testid targeting, the wait helper these tests need because assertions do not auto-wait on SPA paint, and why BrowserTestCase keeps Vite real. Verified before removing: every @selector used in tests/Browser resolves to a data-testid, seven of them through bound :data-testid, so none depended on a dusk attribute. * chore: drop the last one-account-per-network helper hasConnectedPlatform() has no callers left anywhere — app, tests, views or routes. It sat directly above getSocialAccount(), which this branch already removed, and is the same leftover from when a workspace could hold one account per platform. --------- Co-authored-by: Paulo Castellano <paulo@castellanos.llc> Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-25 10:28:14 +00:00
it('connects a second telegram channel when multiple social accounts are allowed', function () {
Http::fake();
Allow multiple social accounts per network via env (#286) * feat: expose self-hosted mode to the accounts UI SocialAccountObserver already bypasses the one-account-per-network guard when trypost.self_hosted is true, but the frontend had no way to know that and always collapsed a network to a single card once any account existed - so self-hosted deployments could not surface a second LinkedIn (or Instagram) connection even though the backend would allow creating it. * feat: allow connecting multiple accounts per network when self-hosted NetworkConnectGrid always collapsed a network (LinkedIn profile/page, Instagram standalone/Facebook) to a single card once any account existed, with no way to trigger another OAuth flow - even though SocialAccountObserver already allows unlimited accounts per network in self-hosted mode. A self-hoster connecting their personal LinkedIn profile had no path back to the connect flow to also add a company page (or a second company page/showcase page). Render one card per connected account instead of collapsing to the first, and keep a standing "Connect another" card available for a network's existing connections when self-hosted. Hosted mode is unchanged: still one card per network, matching the backend's still-enforced one-account-per-network limit there. * test: cover the selfHosted prop on accounts and onboarding pages Backend behavior for connecting a second identity per network in self-hosted mode was already covered (LinkedInControllerTest, NetworkUniquenessTest) - these just confirm the new prop the frontend now depends on is actually present and reflects config correctly. * style: apply prettier formatting Pre-existing drift in this file unrelated to the selfHosted change. * refactor: read selfHosted from shared Inertia props The flag is already shared by HandleInertiaRequests, so the accounts and onboarding controllers do not need to pass it again. Co-authored-by: Cursor <cursoragent@cursor.com> * feat: gate multiple social accounts with a dedicated env Cloud cannot flip SELF_HOSTED, so one-per-network is now ALLOW_MULTIPLE_SOCIAL_ACCOUNTS (default false). Co-authored-by: Cursor <cursoragent@cursor.com> * fix: tighten multiple-account gates after review Keep every connected identity visible, share occupiesNetwork, and return network_taken instead of a generic connect error. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: bind reconnect to the card and unique social identity Reconnect now updates the selected account, and a unique index plus connectIdentity keep the same platform identity from being inserted twice. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: build the OAuth URL before opening the popup Keep the popup opener URL-only so reconnect query params are assembled at the call site. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: drop dead social-account guards and slim the connect grid Skip migration cleanup that production never needs, trust the platform enum in the observer, and move card theming out of the grid. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: scope social reconnect to the current network Drop dead instanceof/isset guards and filter reconnect targets in the query so a stale session cannot update another network's card. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim connectable-identity filtering Index OAuth identities by id so reconnect and occupancy use only/except instead of hand-rolled filters. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim social identity persist helpers Drop the unused occupiesNetwork exception and persist reconnects with update() instead of fill/save/fresh. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: keep reconnect updates on the original social card Co-authored-by: Cursor <cursoragent@cursor.com> * test: run the suite with multiple social accounts enabled phpunit.xml forced ALLOW_MULTIPLE_SOCIAL_ACCOUNTS=false, overriding the true value in .env.ci. That broke eight tests across Automation, MCP, PostApi, RefreshExpiringTokens and VerifyUpcomingConnections which only needed two accounts of one network as a fixture, not as a rule under test. Match .env.ci instead. Every test that exercises the one-per-network rule already sets the config itself; the accounts index test was the only one leaning on the implicit default, so it now pins it. * fix: align the multi-account fallback with the self-hosted default allow_multiple_social_accounts fell back to env('SELF_HOSTED', false) while self_hosted itself defaults to env('SELF_HOSTED', true). A self-hosted install that never wrote SELF_HOSTED to its .env resolved to false and silently lost multiple accounts per network on upgrade, which is the opposite of what the documented fallback promises. * fix: collapse duplicate identities before adding the unique index Installs predating the index can hold the same identity twice: the network guard was bypassed for multi-account installs and Pinterest always created a fresh row. Creating the index on that data aborts migrate mid-deploy. Keep the newest row per identity and move its post_platforms over before dropping the duplicates - the FK is nullOnDelete, so deleting outright would orphan drafts and scheduled posts. * fix: refuse a reconnect that authorized a different identity connectIdentity overwrote platform_user_id with whatever the provider returned, so reconnecting a card while signed into another account repointed the row - and every draft and scheduled post bound to it - at a stranger. LinkedIn guarded this at the controller and Facebook via its filtered page list; nothing covered X, TikTok, Threads, Discord, Bluesky, Mastodon, Pinterest, Instagram or Telegram. Enforce the identity match at the single choke point every connect flow goes through. Every call site already maps NetworkAlreadyConnectedException to network_taken, so the refusal surfaces without new plumbing. Also restore the null-platform guard in the observer: occupiesNetwork type-hints a non-nullable Platform, so a row without one died with a TypeError instead of the database's NOT NULL error. * fix: filter connectable identities on every picker step YouTubeController::select re-fetched the channels and matched the posted id straight off the raw list, unlike callback and selectChannel. With a live youtube_oauth session it let a POST name any channel the Google account owns and bind it to the reconnect target. It also read the reconnect from the session while the connect below it read youtube_oauth.reconnect_id, so the two could disagree - pass the resolved account through instead. filterConnectableIdentities also short-circuited in multi-account mode, and the unique index is scoped to platform rather than network. That let one Instagram account connect twice, once directly and once via Facebook, publishing every Instagram post to it twice. The existing except() already spans networkPlatformValues(), so dropping the short-circuit closes it. * refactor: type the connect cards and drop the dead accounts grid The cards computed inferred account as a required ConnectedAccount and then pushed undefined onto it (TS2345). CI only runs eslint so it stayed green, but vue-tsc and editors flag it. SocialAccountsGrid is referenced nowhere; its reconnect button was updated in this branch without passing the card id, which would have been a bug had anything rendered it. * test: keep the suite on the cloud one-account-per-network default CI runs the Cloud build, so the suite baseline should be the Cloud default rather than the self-hosted one. Put phpunit.xml and .env.ci back to false and make the eight tests that merely need two accounts of one network as a fixture opt in for themselves. This also un-deads the config()->set(true) calls the branch had already added to AuthenticationTest, SyncAccountUsageTest, HasUsageTraitTest and SocialAccountObserverTest, which the forced true had turned into no-ops. * fix: connect standalone instagram instead of reopening the picker The picker emits an already-resolved connect method, but this branch rewired @select from openOAuthPopup to startConnect. startConnect sends a bare 'instagram' straight back into its own picker branch, so choosing "Instagram" closed the dialog and immediately reopened it - the OAuth window never opened and the standalone flow was unreachable. Only the via-Facebook button still worked. Split the URL-opening tail out of startConnect and let the dialog call that directly. * fix: reject a telegram reconnect before burning the connect code The nonce was consumed before connectIdentity ran, so posting /connect in the wrong chat spent the one-off code and forced the user to generate a new one. Check the identity first and report wrong_chat instead of network_taken, which told them to disconnect an account when the real fix was posting in the channel they were reconnecting. * fix: leave one target per post when merging duplicate accounts post_platforms has no unique on (post_id, social_account_id), so a post holding a row per duplicate account ended up with two enabled rows aimed at the surviving account and would publish to it twice. Keep one row per post, preferring a published one so history survives. * refactor: collapse the repeated connect-flow boilerplate Four shapes were copy-pasted across the connect controllers: - the session + permission guard opening 16 actions, now connectWorkspace() throwing a ConnectPopupException that renders the popup itself - the reconnected/connected ternary in 13 places, now connectedCallback() - the "nothing left to connect" branch in 4 places, now noConnectableIdentities() - validatedReconnectId() re-querying what reconnectAccount() already does Facebook, Instagram-via-Facebook and YouTube also re-queried the reconnect account three or four times per callback; it is resolved once and passed down. The three GET pickers skipped the manageAccounts check their POST siblings had, and pick it up from the shared guard. Drops the color key from connectableOptions and the matching frontend field - nothing read it. Platform::color() stays; the disconnection emails use it. * fix: keep an expired connect popup out of the error log ConnectPopupException escapes to the framework handler so it can render itself, which also meant report() ran first: every session_expired and workspace_not_found popup filed an ERROR and a Nightwatch issue for what used to be a silent return. A stale popup is a normal outcome, so it now implements ShouldntReport. The Mastodon and Threads guards also cleared their provider session after connectWorkspace(), so a workspace that vanished mid-flow left the client secret and the OAuth state behind. Clear first, then resolve. clearMastodonSession() no longer touches social_connect_workspace - whatever closes the popup already does. * fix: stop telling users to disconnect an account that is not the problem Two flows reused popup_callback.network_taken - "This workspace already has an account for this network. Disconnect it first." - for situations where that is neither true nor actionable: - reconnecting a card while signed into a different account on the provider, now wrong_account - an empty picker in multi-account mode, where every page or channel on that login is simply already connected, now all_connected NetworkAlreadyConnectedException carries the message key so the catch sites stay one line. handleCallback() also drops its $platform argument; it read $this->platform for the reconnect lookup and the identity filter either way, so a caller passing a different platform would have scoped the lookup to the wrong network. * refactor: filter linkedin identities with the shared helper The picker hand-rolled its own reconnect narrowing because the profile and the pages arrive in two different shapes. Flatten them into one pool of LinkedIn identities, run the shared filter, and split them again for the view - the same path Facebook, YouTube and Instagram already take. Side effect worth having: the picker previously only narrowed on a reconnect, so it would offer an identity that is already connected and only fail once the user picked it. It now hides taken identities up front and says so when nothing is left. * fix: keep the linkedin picker's own empty state Routing the picker through the shared filter made every empty pool look like "nothing left to take", including the pool LinkedIn never filled. A self-hoster running pages-only who administers no page was told the network was already connected, or that every account on the login was taken - both false - and the picker's own "you are not an admin of any LinkedIn page" state became unreachable. Only treat it as taken when filtering is what emptied it. Splitting the pool back also compared the person id loosely on one side and strictly on the other; one predicate now drives both. Threads had two forget() calls for a key the top of the action already clears, and YouTube's picker resolved the reconnect account twice on the failure path. * fix: keep the enabled row when collapsing duplicate post targets SyncPostPlatforms seeds a disabled post_platforms row for every account in the workspace, so the usual duplicate is one row the user actually checked next to one they never saw - both pending, both created in the same second. Ordering only by published-then-newest made that a coin flip, and PublishPost iterates enabled() only, so half the time a scheduled post would silently stop reaching that account and take its caption and per-platform meta with it. This runs once against production data and the dropped row is gone, so enabled now beats disabled. Also: the empty-pool exit from the LinkedIn picker was the only one leaving linkedin_pending - and its tokens - in the session. The rationale comments move to the docblocks they belong in, and usePage() comes out of the cards computed. * fix: stop the migration destroying publish history and automations Two ways the one-shot merge lost data that cannot be rebuilt: Surplus published post_platforms rows were deleted. Two duplicate accounts really could each have published, and each row carries the platform_post_id for a live post on the network - dropping one leaves that post unmanageable and invisible to metrics. The docblock claimed published beat everything; now the code does, and only unpublished repeats collapse. Automation nodes persist social_account_id inside a JSON column with no foreign key, so deleting the loser left RunGenerateNode skipping that target, or generating nothing at all when it was the node's only account. The ids are rewritten - current and legacy shapes both - and entries the merge just turned into duplicates are collapsed. Ordering is now total (null created_at sorts oldest on every engine, then id) so a rehearsal on a replica keeps the same rows as the real run. The LinkedIn picker also passes onboardingProgress inline: it clears linkedin_pending on the empty path, and a deferred reload would re-GET the route and swap the empty state for a session-expired popup. * fix: make the identity merge auditable and stop a second delivery Self-hosted installs run this unattended and it cannot be undone, so each collapsed group now logs the workspace, the identity, which row was kept, which were dropped, and how many post_platforms and automations it touched. down() says plainly that it drops the index only. Two narrower fixes: A post holding a published row plus an enabled unpublished row for the same account kept both, and PostPlatform::scopeEnabled() filters on `enabled` alone with no status check - so a republish would deliver the same content to that identity twice. Once a published row exists, every unpublished repeat goes. The automation dedupe ran on every automation in the workspace, not just the ones the merge rewrote. A node legitimately holding two entries for one account under different content types would be collapsed to whichever came first in the array. It now runs only where an id was actually substituted. * test: rehearse the identity merge against a messy database Every test on this migration so far covered a case someone thought to write, which is why three separate review rounds each found a defect the earlier ones missed. This builds a deliberately messy database instead - three workspaces, four networks, one to three copies of each identity, posts mixing published, pending and failed rows across the duplicates with enabled flags varying, and automations referencing them in both the current and legacy JSON shapes - then runs the real migration and asserts what must be true afterwards rather than what happens to a particular fixture. Invariants: no duplicate identity survives, no published row is ever destroyed, no post ends up enabled twice against one account, nothing in post_platforms or automations points at a deleted account, and the newest row of each identity is the one kept. The generator is seeded, so a failure reproduces, and it asserts its own output is adversarial - roughly nine duplicate groups and fourteen published rows - so it cannot quietly degrade into passing on an empty problem. Verified by mutation: dropping the automation repoint, the published guard, or the repeated-target collapse each fails exactly the invariant that covers it. * fix: stop the youtube picker refetching itself into a cleared session HandleInertiaRequests defers onboardingProgress for anyone mid-onboarding - exactly the people connecting their first accounts - so Inertia re-GETs the picker route right after it mounts. For Facebook and Instagram that re-entry is harmless and deliberately left deferred, but YouTube calls the Google API again, and fetchChannels() turns any failure into an empty list that clears the connect session and swaps the mounted picker for an error the user cannot retry from. Same guard the LinkedIn picker already got. LinkedIn also answered a reconnect that authorized a different identity with "Page not found", including in the person branch where no page is involved. Every other platform says wrong_account, which this PR added. * refactor: drop the unreachable youtube channel picker Google's own delegation screen already lists every channel on the account and makes the user pick one before it issues the token, so channels?mine=true always answers with that single channel and count($channels) === 1 always won. The picker behind it was never reached - its Vue page was deleted back in 7c00c338 (January) and nothing broke, which is the clearest evidence it was dead. Removes selectChannel(), select(), both routes, the youtube_oauth session payload and the tests that drove them. If Google ever does return more than one, the callback connects the first and logs a warning rather than routing to a screen that no longer exists. * fix: serialize connects so two popups cannot seat one network twice The observer's occupiesNetwork() is a check-then-insert with nothing holding the gap, and the new unique index covers the identity, not the network. Two tabs finishing OAuth at the same moment for *different* identities on one network both passed the exists() check and both inserted, leaving a Cloud workspace with the two accounts the rule exists to prevent. The same-identity race was already safe - the unique violation is caught and re-queried. A database constraint cannot hold this: allow_multiple_social_accounts is a runtime flag, so the rule is on for Cloud and off for self-hosted, and an index cannot read config. Lock per workspace and network instead, the way markAsDisconnected() and ConnectionVerifier already do. This covers connectIdentity(), which every OAuth flow and the Telegram action go through. A direct create() still answers to the observer alone, and a self-hosted install running file cache across several nodes locks per node. * fix: handle a busy connect lock on the telegram path Every other caller funnels LockTimeoutException into its generic \Exception catch and closes the popup with error_connecting. Telegram has no such catch, so the new lock could 500 the webhook - and because the nonce is spent before connectIdentity runs, Telegram's retry of the same update short-circuits on the consumed code and returns without dispatching anything. The dialog would spin forever on a code that can no longer be used. Also restores coverage the picker removal dropped: the deleted select tests were the only ones driving a multi-channel response, so nothing exercised the reconnect narrowing to its own card, or multi-account mode skipping an already-connected channel. Both are back against the callback, and removing the narrowing in filterConnectableIdentities fails them. * fix: stop the instagram login seating an account already held via facebook filterConnectableIdentities() drops every identity already connected on the network, which is what keeps one Instagram account from being seated twice under its two platforms. Every flow that persists an identity ran it except the direct Instagram Login callback, so the guard only held in one direction: InstagramFacebookController refused an account already connected as `instagram`, but the reverse was allowed through. With multiple accounts per network enabled the observer's network check is bypassed and the unique index does not span platforms, so authorizing the same account through the direct flow created a second row. Both then seed a post_platform row and the post goes out twice to one account. * fix: name the real reason when a linkedin profile reconnect switches member Reconnecting a card narrows the authorized identities to that card's own, so authorizing a different LinkedIn login empties the pool. selectIdentity() reported that as "Page not found." for every card, including personal profiles where no page was ever involved. A profile reconnect has no page to be missing: an empty pool there can only mean this login is a different member. Say so with the wrong_account wording select() already uses for the same condition. Page reconnects keep page_not_found, where the organization really can be absent from the login. * fix: surface the busy telegram connect instead of a generic failure The connect lock timing out dispatches its own 'busy' reason so the dialog can tell the user to retry, but the dialog only mapped network_taken and wrong_chat and fell back to error_generic for everything else. The reason reached the browser and died there, leaving "Could not start the connection" for a case that just needs another moment. * test: cover reconnect on every flow that gained it rememberConnectSession() gave Instagram, TikTok, Threads, Mastodon and Bluesky a reconnect path they did not have before — TikTok had been actively clearing social_reconnect_id on connect — and none of them had a test for it. Facebook, LinkedIn, YouTube, X, Discord, Pinterest and Telegram already did. Each now covers both halves: authorizing the same identity refreshes the existing card and reports it as a reconnect, and authorizing a different one is refused with wrong_account instead of quietly seating a stranger on the card and every post scheduled against it. * fix: repair what a reconnect leaves behind when it cannot proceed cleanly Two things connectIdentity got wrong once the reconnect path existed. A reconnect through the other variant of a network moves the card to the new platform — same identity, different API flavor. Post targets carry their own platform snapshot, and that snapshot picks the publisher, the queue and the scopes checked before publishing. Left behind, it failed every pending post on a permission the account no longer needs: an Instagram card moved to the Facebook variant still demanded instagram_business_content_publish and stopped with "Missing permissions". Pending targets now follow the card and reset a content type the new platform cannot publish; published targets keep theirs, since they record what really went out under a platform_post_id from that API. The network lock timing out also arrived as a raw LockTimeoutException, which every OAuth callback filed through its generic catch: an error log and "Error connecting account" for the exact race the lock exists to absorb. It now carries a busy messageKey through the branch each flow already handles, the same way the Telegram path already reported it. * refactor: resolve the linkedin reconnect card once per select select() already looked the card up before deciding whether the chosen identity matches it, then connectPerson() and connectOrganization() looked it up again on their own — two identical queries per submit, and two places that could disagree about what is being reconnected. The caller passes what it already holds. * test: render the grid's multi-account branch phpunit.xml forces ALLOW_MULTIPLE_SOCIAL_ACCOUNTS false and no browser test overrode it, so the card the flag exists to add never rendered anywhere. The pair pins both sides: a taken network offers no second card when multiples are off, and offers one when they are on. * test: pin why the linkedin select guards exist connectIdentity() already refuses a mismatched reconnect and answers with the same wrong_account message, so every existing test passes with the two guards in select() deleted — which is exactly how they would get deleted. What they actually buy is skipping the avatar download that building the connect payload runs first. Both now assert the fetch never happens, so the guards fail loudly instead of looking redundant. * fix: carry retrying targets through a variant move, atomically Two holes in the move added a commit ago. It only carried pending targets, but a retrying one is not finished either — the publish job reschedules itself and reads the snapshot fresh on the next attempt, so leaving it behind meant it retried against the old variant until it exhausted its budget on a permission the account no longer needs. Failed and published targets stay put; a publishing one has a job mid-flight already working from the snapshot it read. The card and its targets also moved in three separate statements, so a crash between them left exactly the split this was meant to close. They share a transaction now. * chore: drop the dusk selectors nothing reads Laravel Dusk is not installed — no laravel/dusk requirement, no DuskTestCase, no browse(). Browser tests run on pest-plugin-browser driving Playwright, and its @selector resolves to data-testid. The 45 dusk attributes left across 18 components selected nothing. CLAUDE.md was the reason they kept coming back: it told every agent to add them. Its browser-testing section now describes the setup that exists — data-testid targeting, the wait helper these tests need because assertions do not auto-wait on SPA paint, and why BrowserTestCase keeps Vite real. Verified before removing: every @selector used in tests/Browser resolves to a data-testid, seven of them through bound :data-testid, so none depended on a dusk attribute. * chore: drop the last one-account-per-network helper hasConnectedPlatform() has no callers left anywhere — app, tests, views or routes. It sat directly above getSocialAccount(), which this branch already removed, and is the same leftover from when a workspace could hold one account per platform. --------- Co-authored-by: Paulo Castellano <paulo@castellanos.llc> Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-25 10:28:14 +00:00
config(['trypost.allow_multiple_social_accounts' => true]);
SocialAccount::factory()->telegram()->create([
'workspace_id' => $this->workspace->id,
'platform_user_id' => '-1009999999999',
]);
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
expect($this->workspace->socialAccounts()->count())->toBe(2);
expect(
SocialAccount::where('platform', Platform::Telegram)->where('platform_user_id', '-1001234567890')->exists()
)->toBeTrue();
});
it('reconnects an existing telegram channel', function () {
Http::fake();
Allow multiple social accounts per network via env (#286) * feat: expose self-hosted mode to the accounts UI SocialAccountObserver already bypasses the one-account-per-network guard when trypost.self_hosted is true, but the frontend had no way to know that and always collapsed a network to a single card once any account existed - so self-hosted deployments could not surface a second LinkedIn (or Instagram) connection even though the backend would allow creating it. * feat: allow connecting multiple accounts per network when self-hosted NetworkConnectGrid always collapsed a network (LinkedIn profile/page, Instagram standalone/Facebook) to a single card once any account existed, with no way to trigger another OAuth flow - even though SocialAccountObserver already allows unlimited accounts per network in self-hosted mode. A self-hoster connecting their personal LinkedIn profile had no path back to the connect flow to also add a company page (or a second company page/showcase page). Render one card per connected account instead of collapsing to the first, and keep a standing "Connect another" card available for a network's existing connections when self-hosted. Hosted mode is unchanged: still one card per network, matching the backend's still-enforced one-account-per-network limit there. * test: cover the selfHosted prop on accounts and onboarding pages Backend behavior for connecting a second identity per network in self-hosted mode was already covered (LinkedInControllerTest, NetworkUniquenessTest) - these just confirm the new prop the frontend now depends on is actually present and reflects config correctly. * style: apply prettier formatting Pre-existing drift in this file unrelated to the selfHosted change. * refactor: read selfHosted from shared Inertia props The flag is already shared by HandleInertiaRequests, so the accounts and onboarding controllers do not need to pass it again. Co-authored-by: Cursor <cursoragent@cursor.com> * feat: gate multiple social accounts with a dedicated env Cloud cannot flip SELF_HOSTED, so one-per-network is now ALLOW_MULTIPLE_SOCIAL_ACCOUNTS (default false). Co-authored-by: Cursor <cursoragent@cursor.com> * fix: tighten multiple-account gates after review Keep every connected identity visible, share occupiesNetwork, and return network_taken instead of a generic connect error. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: bind reconnect to the card and unique social identity Reconnect now updates the selected account, and a unique index plus connectIdentity keep the same platform identity from being inserted twice. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: build the OAuth URL before opening the popup Keep the popup opener URL-only so reconnect query params are assembled at the call site. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: drop dead social-account guards and slim the connect grid Skip migration cleanup that production never needs, trust the platform enum in the observer, and move card theming out of the grid. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: scope social reconnect to the current network Drop dead instanceof/isset guards and filter reconnect targets in the query so a stale session cannot update another network's card. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim connectable-identity filtering Index OAuth identities by id so reconnect and occupancy use only/except instead of hand-rolled filters. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim social identity persist helpers Drop the unused occupiesNetwork exception and persist reconnects with update() instead of fill/save/fresh. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: keep reconnect updates on the original social card Co-authored-by: Cursor <cursoragent@cursor.com> * test: run the suite with multiple social accounts enabled phpunit.xml forced ALLOW_MULTIPLE_SOCIAL_ACCOUNTS=false, overriding the true value in .env.ci. That broke eight tests across Automation, MCP, PostApi, RefreshExpiringTokens and VerifyUpcomingConnections which only needed two accounts of one network as a fixture, not as a rule under test. Match .env.ci instead. Every test that exercises the one-per-network rule already sets the config itself; the accounts index test was the only one leaning on the implicit default, so it now pins it. * fix: align the multi-account fallback with the self-hosted default allow_multiple_social_accounts fell back to env('SELF_HOSTED', false) while self_hosted itself defaults to env('SELF_HOSTED', true). A self-hosted install that never wrote SELF_HOSTED to its .env resolved to false and silently lost multiple accounts per network on upgrade, which is the opposite of what the documented fallback promises. * fix: collapse duplicate identities before adding the unique index Installs predating the index can hold the same identity twice: the network guard was bypassed for multi-account installs and Pinterest always created a fresh row. Creating the index on that data aborts migrate mid-deploy. Keep the newest row per identity and move its post_platforms over before dropping the duplicates - the FK is nullOnDelete, so deleting outright would orphan drafts and scheduled posts. * fix: refuse a reconnect that authorized a different identity connectIdentity overwrote platform_user_id with whatever the provider returned, so reconnecting a card while signed into another account repointed the row - and every draft and scheduled post bound to it - at a stranger. LinkedIn guarded this at the controller and Facebook via its filtered page list; nothing covered X, TikTok, Threads, Discord, Bluesky, Mastodon, Pinterest, Instagram or Telegram. Enforce the identity match at the single choke point every connect flow goes through. Every call site already maps NetworkAlreadyConnectedException to network_taken, so the refusal surfaces without new plumbing. Also restore the null-platform guard in the observer: occupiesNetwork type-hints a non-nullable Platform, so a row without one died with a TypeError instead of the database's NOT NULL error. * fix: filter connectable identities on every picker step YouTubeController::select re-fetched the channels and matched the posted id straight off the raw list, unlike callback and selectChannel. With a live youtube_oauth session it let a POST name any channel the Google account owns and bind it to the reconnect target. It also read the reconnect from the session while the connect below it read youtube_oauth.reconnect_id, so the two could disagree - pass the resolved account through instead. filterConnectableIdentities also short-circuited in multi-account mode, and the unique index is scoped to platform rather than network. That let one Instagram account connect twice, once directly and once via Facebook, publishing every Instagram post to it twice. The existing except() already spans networkPlatformValues(), so dropping the short-circuit closes it. * refactor: type the connect cards and drop the dead accounts grid The cards computed inferred account as a required ConnectedAccount and then pushed undefined onto it (TS2345). CI only runs eslint so it stayed green, but vue-tsc and editors flag it. SocialAccountsGrid is referenced nowhere; its reconnect button was updated in this branch without passing the card id, which would have been a bug had anything rendered it. * test: keep the suite on the cloud one-account-per-network default CI runs the Cloud build, so the suite baseline should be the Cloud default rather than the self-hosted one. Put phpunit.xml and .env.ci back to false and make the eight tests that merely need two accounts of one network as a fixture opt in for themselves. This also un-deads the config()->set(true) calls the branch had already added to AuthenticationTest, SyncAccountUsageTest, HasUsageTraitTest and SocialAccountObserverTest, which the forced true had turned into no-ops. * fix: connect standalone instagram instead of reopening the picker The picker emits an already-resolved connect method, but this branch rewired @select from openOAuthPopup to startConnect. startConnect sends a bare 'instagram' straight back into its own picker branch, so choosing "Instagram" closed the dialog and immediately reopened it - the OAuth window never opened and the standalone flow was unreachable. Only the via-Facebook button still worked. Split the URL-opening tail out of startConnect and let the dialog call that directly. * fix: reject a telegram reconnect before burning the connect code The nonce was consumed before connectIdentity ran, so posting /connect in the wrong chat spent the one-off code and forced the user to generate a new one. Check the identity first and report wrong_chat instead of network_taken, which told them to disconnect an account when the real fix was posting in the channel they were reconnecting. * fix: leave one target per post when merging duplicate accounts post_platforms has no unique on (post_id, social_account_id), so a post holding a row per duplicate account ended up with two enabled rows aimed at the surviving account and would publish to it twice. Keep one row per post, preferring a published one so history survives. * refactor: collapse the repeated connect-flow boilerplate Four shapes were copy-pasted across the connect controllers: - the session + permission guard opening 16 actions, now connectWorkspace() throwing a ConnectPopupException that renders the popup itself - the reconnected/connected ternary in 13 places, now connectedCallback() - the "nothing left to connect" branch in 4 places, now noConnectableIdentities() - validatedReconnectId() re-querying what reconnectAccount() already does Facebook, Instagram-via-Facebook and YouTube also re-queried the reconnect account three or four times per callback; it is resolved once and passed down. The three GET pickers skipped the manageAccounts check their POST siblings had, and pick it up from the shared guard. Drops the color key from connectableOptions and the matching frontend field - nothing read it. Platform::color() stays; the disconnection emails use it. * fix: keep an expired connect popup out of the error log ConnectPopupException escapes to the framework handler so it can render itself, which also meant report() ran first: every session_expired and workspace_not_found popup filed an ERROR and a Nightwatch issue for what used to be a silent return. A stale popup is a normal outcome, so it now implements ShouldntReport. The Mastodon and Threads guards also cleared their provider session after connectWorkspace(), so a workspace that vanished mid-flow left the client secret and the OAuth state behind. Clear first, then resolve. clearMastodonSession() no longer touches social_connect_workspace - whatever closes the popup already does. * fix: stop telling users to disconnect an account that is not the problem Two flows reused popup_callback.network_taken - "This workspace already has an account for this network. Disconnect it first." - for situations where that is neither true nor actionable: - reconnecting a card while signed into a different account on the provider, now wrong_account - an empty picker in multi-account mode, where every page or channel on that login is simply already connected, now all_connected NetworkAlreadyConnectedException carries the message key so the catch sites stay one line. handleCallback() also drops its $platform argument; it read $this->platform for the reconnect lookup and the identity filter either way, so a caller passing a different platform would have scoped the lookup to the wrong network. * refactor: filter linkedin identities with the shared helper The picker hand-rolled its own reconnect narrowing because the profile and the pages arrive in two different shapes. Flatten them into one pool of LinkedIn identities, run the shared filter, and split them again for the view - the same path Facebook, YouTube and Instagram already take. Side effect worth having: the picker previously only narrowed on a reconnect, so it would offer an identity that is already connected and only fail once the user picked it. It now hides taken identities up front and says so when nothing is left. * fix: keep the linkedin picker's own empty state Routing the picker through the shared filter made every empty pool look like "nothing left to take", including the pool LinkedIn never filled. A self-hoster running pages-only who administers no page was told the network was already connected, or that every account on the login was taken - both false - and the picker's own "you are not an admin of any LinkedIn page" state became unreachable. Only treat it as taken when filtering is what emptied it. Splitting the pool back also compared the person id loosely on one side and strictly on the other; one predicate now drives both. Threads had two forget() calls for a key the top of the action already clears, and YouTube's picker resolved the reconnect account twice on the failure path. * fix: keep the enabled row when collapsing duplicate post targets SyncPostPlatforms seeds a disabled post_platforms row for every account in the workspace, so the usual duplicate is one row the user actually checked next to one they never saw - both pending, both created in the same second. Ordering only by published-then-newest made that a coin flip, and PublishPost iterates enabled() only, so half the time a scheduled post would silently stop reaching that account and take its caption and per-platform meta with it. This runs once against production data and the dropped row is gone, so enabled now beats disabled. Also: the empty-pool exit from the LinkedIn picker was the only one leaving linkedin_pending - and its tokens - in the session. The rationale comments move to the docblocks they belong in, and usePage() comes out of the cards computed. * fix: stop the migration destroying publish history and automations Two ways the one-shot merge lost data that cannot be rebuilt: Surplus published post_platforms rows were deleted. Two duplicate accounts really could each have published, and each row carries the platform_post_id for a live post on the network - dropping one leaves that post unmanageable and invisible to metrics. The docblock claimed published beat everything; now the code does, and only unpublished repeats collapse. Automation nodes persist social_account_id inside a JSON column with no foreign key, so deleting the loser left RunGenerateNode skipping that target, or generating nothing at all when it was the node's only account. The ids are rewritten - current and legacy shapes both - and entries the merge just turned into duplicates are collapsed. Ordering is now total (null created_at sorts oldest on every engine, then id) so a rehearsal on a replica keeps the same rows as the real run. The LinkedIn picker also passes onboardingProgress inline: it clears linkedin_pending on the empty path, and a deferred reload would re-GET the route and swap the empty state for a session-expired popup. * fix: make the identity merge auditable and stop a second delivery Self-hosted installs run this unattended and it cannot be undone, so each collapsed group now logs the workspace, the identity, which row was kept, which were dropped, and how many post_platforms and automations it touched. down() says plainly that it drops the index only. Two narrower fixes: A post holding a published row plus an enabled unpublished row for the same account kept both, and PostPlatform::scopeEnabled() filters on `enabled` alone with no status check - so a republish would deliver the same content to that identity twice. Once a published row exists, every unpublished repeat goes. The automation dedupe ran on every automation in the workspace, not just the ones the merge rewrote. A node legitimately holding two entries for one account under different content types would be collapsed to whichever came first in the array. It now runs only where an id was actually substituted. * test: rehearse the identity merge against a messy database Every test on this migration so far covered a case someone thought to write, which is why three separate review rounds each found a defect the earlier ones missed. This builds a deliberately messy database instead - three workspaces, four networks, one to three copies of each identity, posts mixing published, pending and failed rows across the duplicates with enabled flags varying, and automations referencing them in both the current and legacy JSON shapes - then runs the real migration and asserts what must be true afterwards rather than what happens to a particular fixture. Invariants: no duplicate identity survives, no published row is ever destroyed, no post ends up enabled twice against one account, nothing in post_platforms or automations points at a deleted account, and the newest row of each identity is the one kept. The generator is seeded, so a failure reproduces, and it asserts its own output is adversarial - roughly nine duplicate groups and fourteen published rows - so it cannot quietly degrade into passing on an empty problem. Verified by mutation: dropping the automation repoint, the published guard, or the repeated-target collapse each fails exactly the invariant that covers it. * fix: stop the youtube picker refetching itself into a cleared session HandleInertiaRequests defers onboardingProgress for anyone mid-onboarding - exactly the people connecting their first accounts - so Inertia re-GETs the picker route right after it mounts. For Facebook and Instagram that re-entry is harmless and deliberately left deferred, but YouTube calls the Google API again, and fetchChannels() turns any failure into an empty list that clears the connect session and swaps the mounted picker for an error the user cannot retry from. Same guard the LinkedIn picker already got. LinkedIn also answered a reconnect that authorized a different identity with "Page not found", including in the person branch where no page is involved. Every other platform says wrong_account, which this PR added. * refactor: drop the unreachable youtube channel picker Google's own delegation screen already lists every channel on the account and makes the user pick one before it issues the token, so channels?mine=true always answers with that single channel and count($channels) === 1 always won. The picker behind it was never reached - its Vue page was deleted back in 7c00c338 (January) and nothing broke, which is the clearest evidence it was dead. Removes selectChannel(), select(), both routes, the youtube_oauth session payload and the tests that drove them. If Google ever does return more than one, the callback connects the first and logs a warning rather than routing to a screen that no longer exists. * fix: serialize connects so two popups cannot seat one network twice The observer's occupiesNetwork() is a check-then-insert with nothing holding the gap, and the new unique index covers the identity, not the network. Two tabs finishing OAuth at the same moment for *different* identities on one network both passed the exists() check and both inserted, leaving a Cloud workspace with the two accounts the rule exists to prevent. The same-identity race was already safe - the unique violation is caught and re-queried. A database constraint cannot hold this: allow_multiple_social_accounts is a runtime flag, so the rule is on for Cloud and off for self-hosted, and an index cannot read config. Lock per workspace and network instead, the way markAsDisconnected() and ConnectionVerifier already do. This covers connectIdentity(), which every OAuth flow and the Telegram action go through. A direct create() still answers to the observer alone, and a self-hosted install running file cache across several nodes locks per node. * fix: handle a busy connect lock on the telegram path Every other caller funnels LockTimeoutException into its generic \Exception catch and closes the popup with error_connecting. Telegram has no such catch, so the new lock could 500 the webhook - and because the nonce is spent before connectIdentity runs, Telegram's retry of the same update short-circuits on the consumed code and returns without dispatching anything. The dialog would spin forever on a code that can no longer be used. Also restores coverage the picker removal dropped: the deleted select tests were the only ones driving a multi-channel response, so nothing exercised the reconnect narrowing to its own card, or multi-account mode skipping an already-connected channel. Both are back against the callback, and removing the narrowing in filterConnectableIdentities fails them. * fix: stop the instagram login seating an account already held via facebook filterConnectableIdentities() drops every identity already connected on the network, which is what keeps one Instagram account from being seated twice under its two platforms. Every flow that persists an identity ran it except the direct Instagram Login callback, so the guard only held in one direction: InstagramFacebookController refused an account already connected as `instagram`, but the reverse was allowed through. With multiple accounts per network enabled the observer's network check is bypassed and the unique index does not span platforms, so authorizing the same account through the direct flow created a second row. Both then seed a post_platform row and the post goes out twice to one account. * fix: name the real reason when a linkedin profile reconnect switches member Reconnecting a card narrows the authorized identities to that card's own, so authorizing a different LinkedIn login empties the pool. selectIdentity() reported that as "Page not found." for every card, including personal profiles where no page was ever involved. A profile reconnect has no page to be missing: an empty pool there can only mean this login is a different member. Say so with the wrong_account wording select() already uses for the same condition. Page reconnects keep page_not_found, where the organization really can be absent from the login. * fix: surface the busy telegram connect instead of a generic failure The connect lock timing out dispatches its own 'busy' reason so the dialog can tell the user to retry, but the dialog only mapped network_taken and wrong_chat and fell back to error_generic for everything else. The reason reached the browser and died there, leaving "Could not start the connection" for a case that just needs another moment. * test: cover reconnect on every flow that gained it rememberConnectSession() gave Instagram, TikTok, Threads, Mastodon and Bluesky a reconnect path they did not have before — TikTok had been actively clearing social_reconnect_id on connect — and none of them had a test for it. Facebook, LinkedIn, YouTube, X, Discord, Pinterest and Telegram already did. Each now covers both halves: authorizing the same identity refreshes the existing card and reports it as a reconnect, and authorizing a different one is refused with wrong_account instead of quietly seating a stranger on the card and every post scheduled against it. * fix: repair what a reconnect leaves behind when it cannot proceed cleanly Two things connectIdentity got wrong once the reconnect path existed. A reconnect through the other variant of a network moves the card to the new platform — same identity, different API flavor. Post targets carry their own platform snapshot, and that snapshot picks the publisher, the queue and the scopes checked before publishing. Left behind, it failed every pending post on a permission the account no longer needs: an Instagram card moved to the Facebook variant still demanded instagram_business_content_publish and stopped with "Missing permissions". Pending targets now follow the card and reset a content type the new platform cannot publish; published targets keep theirs, since they record what really went out under a platform_post_id from that API. The network lock timing out also arrived as a raw LockTimeoutException, which every OAuth callback filed through its generic catch: an error log and "Error connecting account" for the exact race the lock exists to absorb. It now carries a busy messageKey through the branch each flow already handles, the same way the Telegram path already reported it. * refactor: resolve the linkedin reconnect card once per select select() already looked the card up before deciding whether the chosen identity matches it, then connectPerson() and connectOrganization() looked it up again on their own — two identical queries per submit, and two places that could disagree about what is being reconnected. The caller passes what it already holds. * test: render the grid's multi-account branch phpunit.xml forces ALLOW_MULTIPLE_SOCIAL_ACCOUNTS false and no browser test overrode it, so the card the flag exists to add never rendered anywhere. The pair pins both sides: a taken network offers no second card when multiples are off, and offers one when they are on. * test: pin why the linkedin select guards exist connectIdentity() already refuses a mismatched reconnect and answers with the same wrong_account message, so every existing test passes with the two guards in select() deleted — which is exactly how they would get deleted. What they actually buy is skipping the avatar download that building the connect payload runs first. Both now assert the fetch never happens, so the guards fail loudly instead of looking redundant. * fix: carry retrying targets through a variant move, atomically Two holes in the move added a commit ago. It only carried pending targets, but a retrying one is not finished either — the publish job reschedules itself and reads the snapshot fresh on the next attempt, so leaving it behind meant it retried against the old variant until it exhausted its budget on a permission the account no longer needs. Failed and published targets stay put; a publishing one has a job mid-flight already working from the snapshot it read. The card and its targets also moved in three separate statements, so a crash between them left exactly the split this was meant to close. They share a transaction now. * chore: drop the dusk selectors nothing reads Laravel Dusk is not installed — no laravel/dusk requirement, no DuskTestCase, no browse(). Browser tests run on pest-plugin-browser driving Playwright, and its @selector resolves to data-testid. The 45 dusk attributes left across 18 components selected nothing. CLAUDE.md was the reason they kept coming back: it told every agent to add them. Its browser-testing section now describes the setup that exists — data-testid targeting, the wait helper these tests need because assertions do not auto-wait on SPA paint, and why BrowserTestCase keeps Vite real. Verified before removing: every @selector used in tests/Browser resolves to a data-testid, seven of them through bound :data-testid, so none depended on a dusk attribute. * chore: drop the last one-account-per-network helper hasConnectedPlatform() has no callers left anywhere — app, tests, views or routes. It sat directly above getSocialAccount(), which this branch already removed, and is the same leftover from when a workspace could hold one account per platform. --------- Co-authored-by: Paulo Castellano <paulo@castellanos.llc> Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-25 10:28:14 +00:00
config(['trypost.allow_multiple_social_accounts' => false]);
SocialAccount::factory()->telegram()->create([
'workspace_id' => $this->workspace->id,
'platform_user_id' => '-1001234567890',
]);
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
expect($this->workspace->socialAccounts()->count())->toBe(1);
expect(
SocialAccount::where('platform', Platform::Telegram)->where('platform_user_id', '-1001234567890')->count()
)->toBe(1);
});
Allow multiple social accounts per network via env (#286) * feat: expose self-hosted mode to the accounts UI SocialAccountObserver already bypasses the one-account-per-network guard when trypost.self_hosted is true, but the frontend had no way to know that and always collapsed a network to a single card once any account existed - so self-hosted deployments could not surface a second LinkedIn (or Instagram) connection even though the backend would allow creating it. * feat: allow connecting multiple accounts per network when self-hosted NetworkConnectGrid always collapsed a network (LinkedIn profile/page, Instagram standalone/Facebook) to a single card once any account existed, with no way to trigger another OAuth flow - even though SocialAccountObserver already allows unlimited accounts per network in self-hosted mode. A self-hoster connecting their personal LinkedIn profile had no path back to the connect flow to also add a company page (or a second company page/showcase page). Render one card per connected account instead of collapsing to the first, and keep a standing "Connect another" card available for a network's existing connections when self-hosted. Hosted mode is unchanged: still one card per network, matching the backend's still-enforced one-account-per-network limit there. * test: cover the selfHosted prop on accounts and onboarding pages Backend behavior for connecting a second identity per network in self-hosted mode was already covered (LinkedInControllerTest, NetworkUniquenessTest) - these just confirm the new prop the frontend now depends on is actually present and reflects config correctly. * style: apply prettier formatting Pre-existing drift in this file unrelated to the selfHosted change. * refactor: read selfHosted from shared Inertia props The flag is already shared by HandleInertiaRequests, so the accounts and onboarding controllers do not need to pass it again. Co-authored-by: Cursor <cursoragent@cursor.com> * feat: gate multiple social accounts with a dedicated env Cloud cannot flip SELF_HOSTED, so one-per-network is now ALLOW_MULTIPLE_SOCIAL_ACCOUNTS (default false). Co-authored-by: Cursor <cursoragent@cursor.com> * fix: tighten multiple-account gates after review Keep every connected identity visible, share occupiesNetwork, and return network_taken instead of a generic connect error. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: bind reconnect to the card and unique social identity Reconnect now updates the selected account, and a unique index plus connectIdentity keep the same platform identity from being inserted twice. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: build the OAuth URL before opening the popup Keep the popup opener URL-only so reconnect query params are assembled at the call site. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: drop dead social-account guards and slim the connect grid Skip migration cleanup that production never needs, trust the platform enum in the observer, and move card theming out of the grid. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: scope social reconnect to the current network Drop dead instanceof/isset guards and filter reconnect targets in the query so a stale session cannot update another network's card. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim connectable-identity filtering Index OAuth identities by id so reconnect and occupancy use only/except instead of hand-rolled filters. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim social identity persist helpers Drop the unused occupiesNetwork exception and persist reconnects with update() instead of fill/save/fresh. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: keep reconnect updates on the original social card Co-authored-by: Cursor <cursoragent@cursor.com> * test: run the suite with multiple social accounts enabled phpunit.xml forced ALLOW_MULTIPLE_SOCIAL_ACCOUNTS=false, overriding the true value in .env.ci. That broke eight tests across Automation, MCP, PostApi, RefreshExpiringTokens and VerifyUpcomingConnections which only needed two accounts of one network as a fixture, not as a rule under test. Match .env.ci instead. Every test that exercises the one-per-network rule already sets the config itself; the accounts index test was the only one leaning on the implicit default, so it now pins it. * fix: align the multi-account fallback with the self-hosted default allow_multiple_social_accounts fell back to env('SELF_HOSTED', false) while self_hosted itself defaults to env('SELF_HOSTED', true). A self-hosted install that never wrote SELF_HOSTED to its .env resolved to false and silently lost multiple accounts per network on upgrade, which is the opposite of what the documented fallback promises. * fix: collapse duplicate identities before adding the unique index Installs predating the index can hold the same identity twice: the network guard was bypassed for multi-account installs and Pinterest always created a fresh row. Creating the index on that data aborts migrate mid-deploy. Keep the newest row per identity and move its post_platforms over before dropping the duplicates - the FK is nullOnDelete, so deleting outright would orphan drafts and scheduled posts. * fix: refuse a reconnect that authorized a different identity connectIdentity overwrote platform_user_id with whatever the provider returned, so reconnecting a card while signed into another account repointed the row - and every draft and scheduled post bound to it - at a stranger. LinkedIn guarded this at the controller and Facebook via its filtered page list; nothing covered X, TikTok, Threads, Discord, Bluesky, Mastodon, Pinterest, Instagram or Telegram. Enforce the identity match at the single choke point every connect flow goes through. Every call site already maps NetworkAlreadyConnectedException to network_taken, so the refusal surfaces without new plumbing. Also restore the null-platform guard in the observer: occupiesNetwork type-hints a non-nullable Platform, so a row without one died with a TypeError instead of the database's NOT NULL error. * fix: filter connectable identities on every picker step YouTubeController::select re-fetched the channels and matched the posted id straight off the raw list, unlike callback and selectChannel. With a live youtube_oauth session it let a POST name any channel the Google account owns and bind it to the reconnect target. It also read the reconnect from the session while the connect below it read youtube_oauth.reconnect_id, so the two could disagree - pass the resolved account through instead. filterConnectableIdentities also short-circuited in multi-account mode, and the unique index is scoped to platform rather than network. That let one Instagram account connect twice, once directly and once via Facebook, publishing every Instagram post to it twice. The existing except() already spans networkPlatformValues(), so dropping the short-circuit closes it. * refactor: type the connect cards and drop the dead accounts grid The cards computed inferred account as a required ConnectedAccount and then pushed undefined onto it (TS2345). CI only runs eslint so it stayed green, but vue-tsc and editors flag it. SocialAccountsGrid is referenced nowhere; its reconnect button was updated in this branch without passing the card id, which would have been a bug had anything rendered it. * test: keep the suite on the cloud one-account-per-network default CI runs the Cloud build, so the suite baseline should be the Cloud default rather than the self-hosted one. Put phpunit.xml and .env.ci back to false and make the eight tests that merely need two accounts of one network as a fixture opt in for themselves. This also un-deads the config()->set(true) calls the branch had already added to AuthenticationTest, SyncAccountUsageTest, HasUsageTraitTest and SocialAccountObserverTest, which the forced true had turned into no-ops. * fix: connect standalone instagram instead of reopening the picker The picker emits an already-resolved connect method, but this branch rewired @select from openOAuthPopup to startConnect. startConnect sends a bare 'instagram' straight back into its own picker branch, so choosing "Instagram" closed the dialog and immediately reopened it - the OAuth window never opened and the standalone flow was unreachable. Only the via-Facebook button still worked. Split the URL-opening tail out of startConnect and let the dialog call that directly. * fix: reject a telegram reconnect before burning the connect code The nonce was consumed before connectIdentity ran, so posting /connect in the wrong chat spent the one-off code and forced the user to generate a new one. Check the identity first and report wrong_chat instead of network_taken, which told them to disconnect an account when the real fix was posting in the channel they were reconnecting. * fix: leave one target per post when merging duplicate accounts post_platforms has no unique on (post_id, social_account_id), so a post holding a row per duplicate account ended up with two enabled rows aimed at the surviving account and would publish to it twice. Keep one row per post, preferring a published one so history survives. * refactor: collapse the repeated connect-flow boilerplate Four shapes were copy-pasted across the connect controllers: - the session + permission guard opening 16 actions, now connectWorkspace() throwing a ConnectPopupException that renders the popup itself - the reconnected/connected ternary in 13 places, now connectedCallback() - the "nothing left to connect" branch in 4 places, now noConnectableIdentities() - validatedReconnectId() re-querying what reconnectAccount() already does Facebook, Instagram-via-Facebook and YouTube also re-queried the reconnect account three or four times per callback; it is resolved once and passed down. The three GET pickers skipped the manageAccounts check their POST siblings had, and pick it up from the shared guard. Drops the color key from connectableOptions and the matching frontend field - nothing read it. Platform::color() stays; the disconnection emails use it. * fix: keep an expired connect popup out of the error log ConnectPopupException escapes to the framework handler so it can render itself, which also meant report() ran first: every session_expired and workspace_not_found popup filed an ERROR and a Nightwatch issue for what used to be a silent return. A stale popup is a normal outcome, so it now implements ShouldntReport. The Mastodon and Threads guards also cleared their provider session after connectWorkspace(), so a workspace that vanished mid-flow left the client secret and the OAuth state behind. Clear first, then resolve. clearMastodonSession() no longer touches social_connect_workspace - whatever closes the popup already does. * fix: stop telling users to disconnect an account that is not the problem Two flows reused popup_callback.network_taken - "This workspace already has an account for this network. Disconnect it first." - for situations where that is neither true nor actionable: - reconnecting a card while signed into a different account on the provider, now wrong_account - an empty picker in multi-account mode, where every page or channel on that login is simply already connected, now all_connected NetworkAlreadyConnectedException carries the message key so the catch sites stay one line. handleCallback() also drops its $platform argument; it read $this->platform for the reconnect lookup and the identity filter either way, so a caller passing a different platform would have scoped the lookup to the wrong network. * refactor: filter linkedin identities with the shared helper The picker hand-rolled its own reconnect narrowing because the profile and the pages arrive in two different shapes. Flatten them into one pool of LinkedIn identities, run the shared filter, and split them again for the view - the same path Facebook, YouTube and Instagram already take. Side effect worth having: the picker previously only narrowed on a reconnect, so it would offer an identity that is already connected and only fail once the user picked it. It now hides taken identities up front and says so when nothing is left. * fix: keep the linkedin picker's own empty state Routing the picker through the shared filter made every empty pool look like "nothing left to take", including the pool LinkedIn never filled. A self-hoster running pages-only who administers no page was told the network was already connected, or that every account on the login was taken - both false - and the picker's own "you are not an admin of any LinkedIn page" state became unreachable. Only treat it as taken when filtering is what emptied it. Splitting the pool back also compared the person id loosely on one side and strictly on the other; one predicate now drives both. Threads had two forget() calls for a key the top of the action already clears, and YouTube's picker resolved the reconnect account twice on the failure path. * fix: keep the enabled row when collapsing duplicate post targets SyncPostPlatforms seeds a disabled post_platforms row for every account in the workspace, so the usual duplicate is one row the user actually checked next to one they never saw - both pending, both created in the same second. Ordering only by published-then-newest made that a coin flip, and PublishPost iterates enabled() only, so half the time a scheduled post would silently stop reaching that account and take its caption and per-platform meta with it. This runs once against production data and the dropped row is gone, so enabled now beats disabled. Also: the empty-pool exit from the LinkedIn picker was the only one leaving linkedin_pending - and its tokens - in the session. The rationale comments move to the docblocks they belong in, and usePage() comes out of the cards computed. * fix: stop the migration destroying publish history and automations Two ways the one-shot merge lost data that cannot be rebuilt: Surplus published post_platforms rows were deleted. Two duplicate accounts really could each have published, and each row carries the platform_post_id for a live post on the network - dropping one leaves that post unmanageable and invisible to metrics. The docblock claimed published beat everything; now the code does, and only unpublished repeats collapse. Automation nodes persist social_account_id inside a JSON column with no foreign key, so deleting the loser left RunGenerateNode skipping that target, or generating nothing at all when it was the node's only account. The ids are rewritten - current and legacy shapes both - and entries the merge just turned into duplicates are collapsed. Ordering is now total (null created_at sorts oldest on every engine, then id) so a rehearsal on a replica keeps the same rows as the real run. The LinkedIn picker also passes onboardingProgress inline: it clears linkedin_pending on the empty path, and a deferred reload would re-GET the route and swap the empty state for a session-expired popup. * fix: make the identity merge auditable and stop a second delivery Self-hosted installs run this unattended and it cannot be undone, so each collapsed group now logs the workspace, the identity, which row was kept, which were dropped, and how many post_platforms and automations it touched. down() says plainly that it drops the index only. Two narrower fixes: A post holding a published row plus an enabled unpublished row for the same account kept both, and PostPlatform::scopeEnabled() filters on `enabled` alone with no status check - so a republish would deliver the same content to that identity twice. Once a published row exists, every unpublished repeat goes. The automation dedupe ran on every automation in the workspace, not just the ones the merge rewrote. A node legitimately holding two entries for one account under different content types would be collapsed to whichever came first in the array. It now runs only where an id was actually substituted. * test: rehearse the identity merge against a messy database Every test on this migration so far covered a case someone thought to write, which is why three separate review rounds each found a defect the earlier ones missed. This builds a deliberately messy database instead - three workspaces, four networks, one to three copies of each identity, posts mixing published, pending and failed rows across the duplicates with enabled flags varying, and automations referencing them in both the current and legacy JSON shapes - then runs the real migration and asserts what must be true afterwards rather than what happens to a particular fixture. Invariants: no duplicate identity survives, no published row is ever destroyed, no post ends up enabled twice against one account, nothing in post_platforms or automations points at a deleted account, and the newest row of each identity is the one kept. The generator is seeded, so a failure reproduces, and it asserts its own output is adversarial - roughly nine duplicate groups and fourteen published rows - so it cannot quietly degrade into passing on an empty problem. Verified by mutation: dropping the automation repoint, the published guard, or the repeated-target collapse each fails exactly the invariant that covers it. * fix: stop the youtube picker refetching itself into a cleared session HandleInertiaRequests defers onboardingProgress for anyone mid-onboarding - exactly the people connecting their first accounts - so Inertia re-GETs the picker route right after it mounts. For Facebook and Instagram that re-entry is harmless and deliberately left deferred, but YouTube calls the Google API again, and fetchChannels() turns any failure into an empty list that clears the connect session and swaps the mounted picker for an error the user cannot retry from. Same guard the LinkedIn picker already got. LinkedIn also answered a reconnect that authorized a different identity with "Page not found", including in the person branch where no page is involved. Every other platform says wrong_account, which this PR added. * refactor: drop the unreachable youtube channel picker Google's own delegation screen already lists every channel on the account and makes the user pick one before it issues the token, so channels?mine=true always answers with that single channel and count($channels) === 1 always won. The picker behind it was never reached - its Vue page was deleted back in 7c00c338 (January) and nothing broke, which is the clearest evidence it was dead. Removes selectChannel(), select(), both routes, the youtube_oauth session payload and the tests that drove them. If Google ever does return more than one, the callback connects the first and logs a warning rather than routing to a screen that no longer exists. * fix: serialize connects so two popups cannot seat one network twice The observer's occupiesNetwork() is a check-then-insert with nothing holding the gap, and the new unique index covers the identity, not the network. Two tabs finishing OAuth at the same moment for *different* identities on one network both passed the exists() check and both inserted, leaving a Cloud workspace with the two accounts the rule exists to prevent. The same-identity race was already safe - the unique violation is caught and re-queried. A database constraint cannot hold this: allow_multiple_social_accounts is a runtime flag, so the rule is on for Cloud and off for self-hosted, and an index cannot read config. Lock per workspace and network instead, the way markAsDisconnected() and ConnectionVerifier already do. This covers connectIdentity(), which every OAuth flow and the Telegram action go through. A direct create() still answers to the observer alone, and a self-hosted install running file cache across several nodes locks per node. * fix: handle a busy connect lock on the telegram path Every other caller funnels LockTimeoutException into its generic \Exception catch and closes the popup with error_connecting. Telegram has no such catch, so the new lock could 500 the webhook - and because the nonce is spent before connectIdentity runs, Telegram's retry of the same update short-circuits on the consumed code and returns without dispatching anything. The dialog would spin forever on a code that can no longer be used. Also restores coverage the picker removal dropped: the deleted select tests were the only ones driving a multi-channel response, so nothing exercised the reconnect narrowing to its own card, or multi-account mode skipping an already-connected channel. Both are back against the callback, and removing the narrowing in filterConnectableIdentities fails them. * fix: stop the instagram login seating an account already held via facebook filterConnectableIdentities() drops every identity already connected on the network, which is what keeps one Instagram account from being seated twice under its two platforms. Every flow that persists an identity ran it except the direct Instagram Login callback, so the guard only held in one direction: InstagramFacebookController refused an account already connected as `instagram`, but the reverse was allowed through. With multiple accounts per network enabled the observer's network check is bypassed and the unique index does not span platforms, so authorizing the same account through the direct flow created a second row. Both then seed a post_platform row and the post goes out twice to one account. * fix: name the real reason when a linkedin profile reconnect switches member Reconnecting a card narrows the authorized identities to that card's own, so authorizing a different LinkedIn login empties the pool. selectIdentity() reported that as "Page not found." for every card, including personal profiles where no page was ever involved. A profile reconnect has no page to be missing: an empty pool there can only mean this login is a different member. Say so with the wrong_account wording select() already uses for the same condition. Page reconnects keep page_not_found, where the organization really can be absent from the login. * fix: surface the busy telegram connect instead of a generic failure The connect lock timing out dispatches its own 'busy' reason so the dialog can tell the user to retry, but the dialog only mapped network_taken and wrong_chat and fell back to error_generic for everything else. The reason reached the browser and died there, leaving "Could not start the connection" for a case that just needs another moment. * test: cover reconnect on every flow that gained it rememberConnectSession() gave Instagram, TikTok, Threads, Mastodon and Bluesky a reconnect path they did not have before — TikTok had been actively clearing social_reconnect_id on connect — and none of them had a test for it. Facebook, LinkedIn, YouTube, X, Discord, Pinterest and Telegram already did. Each now covers both halves: authorizing the same identity refreshes the existing card and reports it as a reconnect, and authorizing a different one is refused with wrong_account instead of quietly seating a stranger on the card and every post scheduled against it. * fix: repair what a reconnect leaves behind when it cannot proceed cleanly Two things connectIdentity got wrong once the reconnect path existed. A reconnect through the other variant of a network moves the card to the new platform — same identity, different API flavor. Post targets carry their own platform snapshot, and that snapshot picks the publisher, the queue and the scopes checked before publishing. Left behind, it failed every pending post on a permission the account no longer needs: an Instagram card moved to the Facebook variant still demanded instagram_business_content_publish and stopped with "Missing permissions". Pending targets now follow the card and reset a content type the new platform cannot publish; published targets keep theirs, since they record what really went out under a platform_post_id from that API. The network lock timing out also arrived as a raw LockTimeoutException, which every OAuth callback filed through its generic catch: an error log and "Error connecting account" for the exact race the lock exists to absorb. It now carries a busy messageKey through the branch each flow already handles, the same way the Telegram path already reported it. * refactor: resolve the linkedin reconnect card once per select select() already looked the card up before deciding whether the chosen identity matches it, then connectPerson() and connectOrganization() looked it up again on their own — two identical queries per submit, and two places that could disagree about what is being reconnected. The caller passes what it already holds. * test: render the grid's multi-account branch phpunit.xml forces ALLOW_MULTIPLE_SOCIAL_ACCOUNTS false and no browser test overrode it, so the card the flag exists to add never rendered anywhere. The pair pins both sides: a taken network offers no second card when multiples are off, and offers one when they are on. * test: pin why the linkedin select guards exist connectIdentity() already refuses a mismatched reconnect and answers with the same wrong_account message, so every existing test passes with the two guards in select() deleted — which is exactly how they would get deleted. What they actually buy is skipping the avatar download that building the connect payload runs first. Both now assert the fetch never happens, so the guards fail loudly instead of looking redundant. * fix: carry retrying targets through a variant move, atomically Two holes in the move added a commit ago. It only carried pending targets, but a retrying one is not finished either — the publish job reschedules itself and reads the snapshot fresh on the next attempt, so leaving it behind meant it retried against the old variant until it exhausted its budget on a permission the account no longer needs. Failed and published targets stay put; a publishing one has a job mid-flight already working from the snapshot it read. The card and its targets also moved in three separate statements, so a crash between them left exactly the split this was meant to close. They share a transaction now. * chore: drop the dusk selectors nothing reads Laravel Dusk is not installed — no laravel/dusk requirement, no DuskTestCase, no browse(). Browser tests run on pest-plugin-browser driving Playwright, and its @selector resolves to data-testid. The 45 dusk attributes left across 18 components selected nothing. CLAUDE.md was the reason they kept coming back: it told every agent to add them. Its browser-testing section now describes the setup that exists — data-testid targeting, the wait helper these tests need because assertions do not auto-wait on SPA paint, and why BrowserTestCase keeps Vite real. Verified before removing: every @selector used in tests/Browser resolves to a data-testid, seven of them through bound :data-testid, so none depended on a dusk attribute. * chore: drop the last one-account-per-network helper hasConnectedPlatform() has no callers left anywhere — app, tests, views or routes. It sat directly above getSocialAccount(), which this branch already removed, and is the same leftover from when a workspace could hold one account per platform. --------- Co-authored-by: Paulo Castellano <paulo@castellanos.llc> Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-25 10:28:14 +00:00
it('issues a connect code that carries the reconnect card', function () {
$account = SocialAccount::factory()->telegram()->create([
'workspace_id' => $this->workspace->id,
'platform_user_id' => '-1009999999999',
]);
$response = $this->actingAs($this->user)
->postJson(route('app.social.telegram.connect', ['reconnect' => $account->id]))
->assertOk();
expect(data_get(TelegramConnectCode::decode($response->json('code')), 'reconnect_id'))->toBe($account->id);
});
it('keeps the reconnect card on its own chat when a different channel posts the code', function () {
Http::fake();
Event::fake([TelegramConnectFailed::class]);
config(['trypost.allow_multiple_social_accounts' => false]);
$account = SocialAccount::factory()->telegram()->create([
'workspace_id' => $this->workspace->id,
'platform_user_id' => '-1009999999999',
]);
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15), $account->id);
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
Event::assertDispatched(
TelegramConnectFailed::class,
fn (TelegramConnectFailed $event): bool => $event->reason === 'wrong_chat'
);
expect($this->workspace->socialAccounts()->count())->toBe(1)
->and($account->fresh()->platform_user_id)->toBe('-1009999999999')
->and(Cache::has("telegram:connect:{$code}"))->toBeFalse();
});
it('lets the user retry in the right chat with the same code', function () {
Http::fake();
config(['trypost.allow_multiple_social_accounts' => false]);
$account = SocialAccount::factory()->telegram()->create([
'workspace_id' => $this->workspace->id,
'platform_user_id' => '-1001234567890',
'username' => 'stale',
]);
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15), $account->id);
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code, ['id' => -1009999999999]))
->assertNoContent();
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
expect($account->fresh()->platform_user_id)->toBe('-1001234567890')
->and($this->workspace->socialAccounts()->count())->toBe(1);
});
it('reconnects the card when its own chat posts the code', function () {
Http::fake();
config(['trypost.allow_multiple_social_accounts' => false]);
$account = SocialAccount::factory()->telegram()->create([
'workspace_id' => $this->workspace->id,
'platform_user_id' => '-1001234567890',
'username' => 'stale',
]);
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15), $account->id);
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
expect($this->workspace->socialAccounts()->count())->toBe(1)
->and($account->fresh()->platform_user_id)->toBe('-1001234567890')
->and($account->fresh()->id)->toBe($account->id);
});
it('consumes the code once so it cannot be replayed for another chat', function () {
Http::fake();
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code, ['id' => -1001111111111]))
->assertNoContent();
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code, ['id' => -1002222222222]))
->assertNoContent();
expect(SocialAccount::where('platform', Platform::Telegram)->count())->toBe(1);
expect(
SocialAccount::where('platform', Platform::Telegram)->where('platform_user_id', '-1001111111111')->exists()
)->toBeTrue();
});
it('stores reaction counts on the matching post from a reaction update', function () {
$account = SocialAccount::factory()->telegram()->create(['workspace_id' => $this->workspace->id]);
$post = Post::factory()->create([
'workspace_id' => $this->workspace->id,
'user_id' => $this->user->id,
]);
$postPlatform = PostPlatform::factory()->published()->create([
'post_id' => $post->id,
'social_account_id' => $account->id,
'platform' => Platform::Telegram,
'platform_post_id' => '42',
]);
$update = [
'message_reaction_count' => [
'chat' => ['id' => -1001234567890],
'message_id' => 42,
'reactions' => [
['type' => ['type' => 'emoji', 'emoji' => '👍'], 'total_count' => 12],
['type' => ['type' => 'emoji', 'emoji' => '❤️'], 'total_count' => 5],
],
],
];
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), $update)
->assertNoContent();
expect(data_get($postPlatform->fresh()->meta, 'reactions'))
->toBe([['type' => '👍', 'count' => 12], ['type' => '❤️', 'count' => 5]]);
});
it('does not store reactions on a post from a different channel with the same message id', function () {
$otherAccount = SocialAccount::factory()->telegram()->create([
'workspace_id' => $this->workspace->id,
'platform_user_id' => '-1009999999999',
'meta' => ['chat_id' => '-1009999999999', 'username' => 'other', 'type' => 'channel'],
]);
$post = Post::factory()->create(['workspace_id' => $this->workspace->id, 'user_id' => $this->user->id]);
$postPlatform = PostPlatform::factory()->published()->create([
'post_id' => $post->id,
'social_account_id' => $otherAccount->id,
'platform' => Platform::Telegram,
'platform_post_id' => '42',
]);
$update = [
'message_reaction_count' => [
'chat' => ['id' => -1001234567890],
'message_id' => 42,
'reactions' => [['type' => ['type' => 'emoji', 'emoji' => '👍'], 'total_count' => 9]],
],
];
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), $update)
->assertNoContent();
expect(data_get($postPlatform->fresh()->meta, 'reactions'))->toBeNull();
});
it('labels custom emoji reactions with a fallback', function () {
$account = SocialAccount::factory()->telegram()->create(['workspace_id' => $this->workspace->id]);
$post = Post::factory()->create(['workspace_id' => $this->workspace->id, 'user_id' => $this->user->id]);
$postPlatform = PostPlatform::factory()->published()->create([
'post_id' => $post->id,
'social_account_id' => $account->id,
'platform' => Platform::Telegram,
'platform_post_id' => '77',
]);
$update = [
'message_reaction_count' => [
'chat' => ['id' => -1001234567890],
'message_id' => 77,
'reactions' => [['type' => ['type' => 'custom_emoji', 'custom_emoji_id' => '555'], 'total_count' => 3]],
],
];
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), $update)
->assertNoContent();
expect(data_get($postPlatform->fresh()->meta, 'reactions'))
->toBe([['type' => 'Custom', 'count' => 3]]);
});
it('connects without an avatar when the channel has no photo', function () {
Http::fake([
'*/botTESTTOKEN/getChat*' => Http::response(['ok' => true, 'result' => []], 200),
]);
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
$account = SocialAccount::where('platform', Platform::Telegram)->first();
expect($account)->not->toBeNull();
expect($account->getRawOriginal('avatar_url'))->toBeNull();
});
it('rejects the webhook without the secret token', function () {
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$this->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertForbidden();
});
it('ignores the webhook for a tampered or expired code', function () {
$expired = TelegramConnectCode::issue($this->workspace->id, now()->subMinute());
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($expired))
->assertNoContent();
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate('not-a-valid-code'))
->assertNoContent();
expect(SocialAccount::where('platform', Platform::Telegram)->count())->toBe(0);
});
it('broadcasts to the workspace with the nonce when a channel connects', function () {
Event::fake([TelegramChannelConnected::class]);
Http::fake();
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$nonce = data_get(TelegramConnectCode::decode($code), 'nonce');
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
Event::assertDispatched(
TelegramChannelConnected::class,
fn (TelegramChannelConnected $event) => $event->workspaceId === $this->workspace->id
&& $event->nonce === $nonce,
);
});
it('does not broadcast when the code is tampered or already used', function () {
Event::fake([TelegramChannelConnected::class]);
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate('not-a-valid-code'))
->assertNoContent();
Event::assertNotDispatched(TelegramChannelConnected::class);
});
it('verifies a connected telegram account via getChat', function () {
config(['trypost.platforms.telegram.bot_token' => 'TESTTOKEN']);
$account = SocialAccount::factory()->telegram()->create(['workspace_id' => $this->workspace->id]);
Http::fake([
'*/botTESTTOKEN/getChat*' => Http::response(['ok' => true, 'result' => ['id' => -1001234567890]], 200),
]);
expect(app(ConnectionVerifier::class)->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
it('throws TokenExpiredException when getChat reports the chat is no longer reachable', function () {
config(['trypost.platforms.telegram.bot_token' => 'TESTTOKEN']);
$account = SocialAccount::factory()->telegram()->create(['workspace_id' => $this->workspace->id]);
Http::fake([
'*/botTESTTOKEN/getChat*' => Http::response(['ok' => false, 'description' => 'chat not found'], 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
expect(fn () => app(ConnectionVerifier::class)->verify($account))
->toThrow(TokenExpiredException::class);
// Telegram has no per-account refresh flow (one bot token shared across
// every connected account) — a confirmed rejection must not trigger a
// second, identical getChat call against that shared budget.
Http::assertSentCount(1);
});
it('throws PlatformUnavailableException when getChat fails transiently', function () {
config(['trypost.platforms.telegram.bot_token' => 'TESTTOKEN']);
$account = SocialAccount::factory()->telegram()->create(['workspace_id' => $this->workspace->id]);
Http::fake([
'*/botTESTTOKEN/getChat*' => Http::response(['ok' => false, 'description' => 'Internal Server Error'], 500),
]);
expect(fn () => app(ConnectionVerifier::class)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
it('throws PlatformUnavailableException, not TokenExpiredException, when the shared bot token is rejected', function () {
config(['trypost.platforms.telegram.bot_token' => 'TESTTOKEN']);
$account = SocialAccount::factory()->telegram()->create(['workspace_id' => $this->workspace->id]);
// 401 means the app-wide TELEGRAM_BOT_TOKEN is misconfigured — every
// Telegram-connected account would fail identically. It is not evidence
// that THIS account's connection is broken, so it must not disconnect it.
Http::fake([
'*/botTESTTOKEN/getChat*' => Http::response(['ok' => false, 'description' => 'Unauthorized'], 401),
]);
expect(fn () => app(ConnectionVerifier::class)->verify($account))
->toThrow(PlatformUnavailableException::class);
});
it('registers the webhook via the artisan command', function () {
Http::fake([
'*/botTESTTOKEN/setWebhook' => Http::response(['ok' => true, 'result' => true], 200),
]);
$this->artisan('telegram:set-webhook')->assertSuccessful();
Http::assertSent(function ($request) {
return str_contains($request->url(), '/setWebhook')
&& $request['secret_token'] === 'shh-secret'
&& str_contains($request['url'], 'telegram/webhook');
});
});
Allow multiple social accounts per network via env (#286) * feat: expose self-hosted mode to the accounts UI SocialAccountObserver already bypasses the one-account-per-network guard when trypost.self_hosted is true, but the frontend had no way to know that and always collapsed a network to a single card once any account existed - so self-hosted deployments could not surface a second LinkedIn (or Instagram) connection even though the backend would allow creating it. * feat: allow connecting multiple accounts per network when self-hosted NetworkConnectGrid always collapsed a network (LinkedIn profile/page, Instagram standalone/Facebook) to a single card once any account existed, with no way to trigger another OAuth flow - even though SocialAccountObserver already allows unlimited accounts per network in self-hosted mode. A self-hoster connecting their personal LinkedIn profile had no path back to the connect flow to also add a company page (or a second company page/showcase page). Render one card per connected account instead of collapsing to the first, and keep a standing "Connect another" card available for a network's existing connections when self-hosted. Hosted mode is unchanged: still one card per network, matching the backend's still-enforced one-account-per-network limit there. * test: cover the selfHosted prop on accounts and onboarding pages Backend behavior for connecting a second identity per network in self-hosted mode was already covered (LinkedInControllerTest, NetworkUniquenessTest) - these just confirm the new prop the frontend now depends on is actually present and reflects config correctly. * style: apply prettier formatting Pre-existing drift in this file unrelated to the selfHosted change. * refactor: read selfHosted from shared Inertia props The flag is already shared by HandleInertiaRequests, so the accounts and onboarding controllers do not need to pass it again. Co-authored-by: Cursor <cursoragent@cursor.com> * feat: gate multiple social accounts with a dedicated env Cloud cannot flip SELF_HOSTED, so one-per-network is now ALLOW_MULTIPLE_SOCIAL_ACCOUNTS (default false). Co-authored-by: Cursor <cursoragent@cursor.com> * fix: tighten multiple-account gates after review Keep every connected identity visible, share occupiesNetwork, and return network_taken instead of a generic connect error. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: bind reconnect to the card and unique social identity Reconnect now updates the selected account, and a unique index plus connectIdentity keep the same platform identity from being inserted twice. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: build the OAuth URL before opening the popup Keep the popup opener URL-only so reconnect query params are assembled at the call site. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: drop dead social-account guards and slim the connect grid Skip migration cleanup that production never needs, trust the platform enum in the observer, and move card theming out of the grid. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: scope social reconnect to the current network Drop dead instanceof/isset guards and filter reconnect targets in the query so a stale session cannot update another network's card. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim connectable-identity filtering Index OAuth identities by id so reconnect and occupancy use only/except instead of hand-rolled filters. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: slim social identity persist helpers Drop the unused occupiesNetwork exception and persist reconnects with update() instead of fill/save/fresh. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: keep reconnect updates on the original social card Co-authored-by: Cursor <cursoragent@cursor.com> * test: run the suite with multiple social accounts enabled phpunit.xml forced ALLOW_MULTIPLE_SOCIAL_ACCOUNTS=false, overriding the true value in .env.ci. That broke eight tests across Automation, MCP, PostApi, RefreshExpiringTokens and VerifyUpcomingConnections which only needed two accounts of one network as a fixture, not as a rule under test. Match .env.ci instead. Every test that exercises the one-per-network rule already sets the config itself; the accounts index test was the only one leaning on the implicit default, so it now pins it. * fix: align the multi-account fallback with the self-hosted default allow_multiple_social_accounts fell back to env('SELF_HOSTED', false) while self_hosted itself defaults to env('SELF_HOSTED', true). A self-hosted install that never wrote SELF_HOSTED to its .env resolved to false and silently lost multiple accounts per network on upgrade, which is the opposite of what the documented fallback promises. * fix: collapse duplicate identities before adding the unique index Installs predating the index can hold the same identity twice: the network guard was bypassed for multi-account installs and Pinterest always created a fresh row. Creating the index on that data aborts migrate mid-deploy. Keep the newest row per identity and move its post_platforms over before dropping the duplicates - the FK is nullOnDelete, so deleting outright would orphan drafts and scheduled posts. * fix: refuse a reconnect that authorized a different identity connectIdentity overwrote platform_user_id with whatever the provider returned, so reconnecting a card while signed into another account repointed the row - and every draft and scheduled post bound to it - at a stranger. LinkedIn guarded this at the controller and Facebook via its filtered page list; nothing covered X, TikTok, Threads, Discord, Bluesky, Mastodon, Pinterest, Instagram or Telegram. Enforce the identity match at the single choke point every connect flow goes through. Every call site already maps NetworkAlreadyConnectedException to network_taken, so the refusal surfaces without new plumbing. Also restore the null-platform guard in the observer: occupiesNetwork type-hints a non-nullable Platform, so a row without one died with a TypeError instead of the database's NOT NULL error. * fix: filter connectable identities on every picker step YouTubeController::select re-fetched the channels and matched the posted id straight off the raw list, unlike callback and selectChannel. With a live youtube_oauth session it let a POST name any channel the Google account owns and bind it to the reconnect target. It also read the reconnect from the session while the connect below it read youtube_oauth.reconnect_id, so the two could disagree - pass the resolved account through instead. filterConnectableIdentities also short-circuited in multi-account mode, and the unique index is scoped to platform rather than network. That let one Instagram account connect twice, once directly and once via Facebook, publishing every Instagram post to it twice. The existing except() already spans networkPlatformValues(), so dropping the short-circuit closes it. * refactor: type the connect cards and drop the dead accounts grid The cards computed inferred account as a required ConnectedAccount and then pushed undefined onto it (TS2345). CI only runs eslint so it stayed green, but vue-tsc and editors flag it. SocialAccountsGrid is referenced nowhere; its reconnect button was updated in this branch without passing the card id, which would have been a bug had anything rendered it. * test: keep the suite on the cloud one-account-per-network default CI runs the Cloud build, so the suite baseline should be the Cloud default rather than the self-hosted one. Put phpunit.xml and .env.ci back to false and make the eight tests that merely need two accounts of one network as a fixture opt in for themselves. This also un-deads the config()->set(true) calls the branch had already added to AuthenticationTest, SyncAccountUsageTest, HasUsageTraitTest and SocialAccountObserverTest, which the forced true had turned into no-ops. * fix: connect standalone instagram instead of reopening the picker The picker emits an already-resolved connect method, but this branch rewired @select from openOAuthPopup to startConnect. startConnect sends a bare 'instagram' straight back into its own picker branch, so choosing "Instagram" closed the dialog and immediately reopened it - the OAuth window never opened and the standalone flow was unreachable. Only the via-Facebook button still worked. Split the URL-opening tail out of startConnect and let the dialog call that directly. * fix: reject a telegram reconnect before burning the connect code The nonce was consumed before connectIdentity ran, so posting /connect in the wrong chat spent the one-off code and forced the user to generate a new one. Check the identity first and report wrong_chat instead of network_taken, which told them to disconnect an account when the real fix was posting in the channel they were reconnecting. * fix: leave one target per post when merging duplicate accounts post_platforms has no unique on (post_id, social_account_id), so a post holding a row per duplicate account ended up with two enabled rows aimed at the surviving account and would publish to it twice. Keep one row per post, preferring a published one so history survives. * refactor: collapse the repeated connect-flow boilerplate Four shapes were copy-pasted across the connect controllers: - the session + permission guard opening 16 actions, now connectWorkspace() throwing a ConnectPopupException that renders the popup itself - the reconnected/connected ternary in 13 places, now connectedCallback() - the "nothing left to connect" branch in 4 places, now noConnectableIdentities() - validatedReconnectId() re-querying what reconnectAccount() already does Facebook, Instagram-via-Facebook and YouTube also re-queried the reconnect account three or four times per callback; it is resolved once and passed down. The three GET pickers skipped the manageAccounts check their POST siblings had, and pick it up from the shared guard. Drops the color key from connectableOptions and the matching frontend field - nothing read it. Platform::color() stays; the disconnection emails use it. * fix: keep an expired connect popup out of the error log ConnectPopupException escapes to the framework handler so it can render itself, which also meant report() ran first: every session_expired and workspace_not_found popup filed an ERROR and a Nightwatch issue for what used to be a silent return. A stale popup is a normal outcome, so it now implements ShouldntReport. The Mastodon and Threads guards also cleared their provider session after connectWorkspace(), so a workspace that vanished mid-flow left the client secret and the OAuth state behind. Clear first, then resolve. clearMastodonSession() no longer touches social_connect_workspace - whatever closes the popup already does. * fix: stop telling users to disconnect an account that is not the problem Two flows reused popup_callback.network_taken - "This workspace already has an account for this network. Disconnect it first." - for situations where that is neither true nor actionable: - reconnecting a card while signed into a different account on the provider, now wrong_account - an empty picker in multi-account mode, where every page or channel on that login is simply already connected, now all_connected NetworkAlreadyConnectedException carries the message key so the catch sites stay one line. handleCallback() also drops its $platform argument; it read $this->platform for the reconnect lookup and the identity filter either way, so a caller passing a different platform would have scoped the lookup to the wrong network. * refactor: filter linkedin identities with the shared helper The picker hand-rolled its own reconnect narrowing because the profile and the pages arrive in two different shapes. Flatten them into one pool of LinkedIn identities, run the shared filter, and split them again for the view - the same path Facebook, YouTube and Instagram already take. Side effect worth having: the picker previously only narrowed on a reconnect, so it would offer an identity that is already connected and only fail once the user picked it. It now hides taken identities up front and says so when nothing is left. * fix: keep the linkedin picker's own empty state Routing the picker through the shared filter made every empty pool look like "nothing left to take", including the pool LinkedIn never filled. A self-hoster running pages-only who administers no page was told the network was already connected, or that every account on the login was taken - both false - and the picker's own "you are not an admin of any LinkedIn page" state became unreachable. Only treat it as taken when filtering is what emptied it. Splitting the pool back also compared the person id loosely on one side and strictly on the other; one predicate now drives both. Threads had two forget() calls for a key the top of the action already clears, and YouTube's picker resolved the reconnect account twice on the failure path. * fix: keep the enabled row when collapsing duplicate post targets SyncPostPlatforms seeds a disabled post_platforms row for every account in the workspace, so the usual duplicate is one row the user actually checked next to one they never saw - both pending, both created in the same second. Ordering only by published-then-newest made that a coin flip, and PublishPost iterates enabled() only, so half the time a scheduled post would silently stop reaching that account and take its caption and per-platform meta with it. This runs once against production data and the dropped row is gone, so enabled now beats disabled. Also: the empty-pool exit from the LinkedIn picker was the only one leaving linkedin_pending - and its tokens - in the session. The rationale comments move to the docblocks they belong in, and usePage() comes out of the cards computed. * fix: stop the migration destroying publish history and automations Two ways the one-shot merge lost data that cannot be rebuilt: Surplus published post_platforms rows were deleted. Two duplicate accounts really could each have published, and each row carries the platform_post_id for a live post on the network - dropping one leaves that post unmanageable and invisible to metrics. The docblock claimed published beat everything; now the code does, and only unpublished repeats collapse. Automation nodes persist social_account_id inside a JSON column with no foreign key, so deleting the loser left RunGenerateNode skipping that target, or generating nothing at all when it was the node's only account. The ids are rewritten - current and legacy shapes both - and entries the merge just turned into duplicates are collapsed. Ordering is now total (null created_at sorts oldest on every engine, then id) so a rehearsal on a replica keeps the same rows as the real run. The LinkedIn picker also passes onboardingProgress inline: it clears linkedin_pending on the empty path, and a deferred reload would re-GET the route and swap the empty state for a session-expired popup. * fix: make the identity merge auditable and stop a second delivery Self-hosted installs run this unattended and it cannot be undone, so each collapsed group now logs the workspace, the identity, which row was kept, which were dropped, and how many post_platforms and automations it touched. down() says plainly that it drops the index only. Two narrower fixes: A post holding a published row plus an enabled unpublished row for the same account kept both, and PostPlatform::scopeEnabled() filters on `enabled` alone with no status check - so a republish would deliver the same content to that identity twice. Once a published row exists, every unpublished repeat goes. The automation dedupe ran on every automation in the workspace, not just the ones the merge rewrote. A node legitimately holding two entries for one account under different content types would be collapsed to whichever came first in the array. It now runs only where an id was actually substituted. * test: rehearse the identity merge against a messy database Every test on this migration so far covered a case someone thought to write, which is why three separate review rounds each found a defect the earlier ones missed. This builds a deliberately messy database instead - three workspaces, four networks, one to three copies of each identity, posts mixing published, pending and failed rows across the duplicates with enabled flags varying, and automations referencing them in both the current and legacy JSON shapes - then runs the real migration and asserts what must be true afterwards rather than what happens to a particular fixture. Invariants: no duplicate identity survives, no published row is ever destroyed, no post ends up enabled twice against one account, nothing in post_platforms or automations points at a deleted account, and the newest row of each identity is the one kept. The generator is seeded, so a failure reproduces, and it asserts its own output is adversarial - roughly nine duplicate groups and fourteen published rows - so it cannot quietly degrade into passing on an empty problem. Verified by mutation: dropping the automation repoint, the published guard, or the repeated-target collapse each fails exactly the invariant that covers it. * fix: stop the youtube picker refetching itself into a cleared session HandleInertiaRequests defers onboardingProgress for anyone mid-onboarding - exactly the people connecting their first accounts - so Inertia re-GETs the picker route right after it mounts. For Facebook and Instagram that re-entry is harmless and deliberately left deferred, but YouTube calls the Google API again, and fetchChannels() turns any failure into an empty list that clears the connect session and swaps the mounted picker for an error the user cannot retry from. Same guard the LinkedIn picker already got. LinkedIn also answered a reconnect that authorized a different identity with "Page not found", including in the person branch where no page is involved. Every other platform says wrong_account, which this PR added. * refactor: drop the unreachable youtube channel picker Google's own delegation screen already lists every channel on the account and makes the user pick one before it issues the token, so channels?mine=true always answers with that single channel and count($channels) === 1 always won. The picker behind it was never reached - its Vue page was deleted back in 7c00c338 (January) and nothing broke, which is the clearest evidence it was dead. Removes selectChannel(), select(), both routes, the youtube_oauth session payload and the tests that drove them. If Google ever does return more than one, the callback connects the first and logs a warning rather than routing to a screen that no longer exists. * fix: serialize connects so two popups cannot seat one network twice The observer's occupiesNetwork() is a check-then-insert with nothing holding the gap, and the new unique index covers the identity, not the network. Two tabs finishing OAuth at the same moment for *different* identities on one network both passed the exists() check and both inserted, leaving a Cloud workspace with the two accounts the rule exists to prevent. The same-identity race was already safe - the unique violation is caught and re-queried. A database constraint cannot hold this: allow_multiple_social_accounts is a runtime flag, so the rule is on for Cloud and off for self-hosted, and an index cannot read config. Lock per workspace and network instead, the way markAsDisconnected() and ConnectionVerifier already do. This covers connectIdentity(), which every OAuth flow and the Telegram action go through. A direct create() still answers to the observer alone, and a self-hosted install running file cache across several nodes locks per node. * fix: handle a busy connect lock on the telegram path Every other caller funnels LockTimeoutException into its generic \Exception catch and closes the popup with error_connecting. Telegram has no such catch, so the new lock could 500 the webhook - and because the nonce is spent before connectIdentity runs, Telegram's retry of the same update short-circuits on the consumed code and returns without dispatching anything. The dialog would spin forever on a code that can no longer be used. Also restores coverage the picker removal dropped: the deleted select tests were the only ones driving a multi-channel response, so nothing exercised the reconnect narrowing to its own card, or multi-account mode skipping an already-connected channel. Both are back against the callback, and removing the narrowing in filterConnectableIdentities fails them. * fix: stop the instagram login seating an account already held via facebook filterConnectableIdentities() drops every identity already connected on the network, which is what keeps one Instagram account from being seated twice under its two platforms. Every flow that persists an identity ran it except the direct Instagram Login callback, so the guard only held in one direction: InstagramFacebookController refused an account already connected as `instagram`, but the reverse was allowed through. With multiple accounts per network enabled the observer's network check is bypassed and the unique index does not span platforms, so authorizing the same account through the direct flow created a second row. Both then seed a post_platform row and the post goes out twice to one account. * fix: name the real reason when a linkedin profile reconnect switches member Reconnecting a card narrows the authorized identities to that card's own, so authorizing a different LinkedIn login empties the pool. selectIdentity() reported that as "Page not found." for every card, including personal profiles where no page was ever involved. A profile reconnect has no page to be missing: an empty pool there can only mean this login is a different member. Say so with the wrong_account wording select() already uses for the same condition. Page reconnects keep page_not_found, where the organization really can be absent from the login. * fix: surface the busy telegram connect instead of a generic failure The connect lock timing out dispatches its own 'busy' reason so the dialog can tell the user to retry, but the dialog only mapped network_taken and wrong_chat and fell back to error_generic for everything else. The reason reached the browser and died there, leaving "Could not start the connection" for a case that just needs another moment. * test: cover reconnect on every flow that gained it rememberConnectSession() gave Instagram, TikTok, Threads, Mastodon and Bluesky a reconnect path they did not have before — TikTok had been actively clearing social_reconnect_id on connect — and none of them had a test for it. Facebook, LinkedIn, YouTube, X, Discord, Pinterest and Telegram already did. Each now covers both halves: authorizing the same identity refreshes the existing card and reports it as a reconnect, and authorizing a different one is refused with wrong_account instead of quietly seating a stranger on the card and every post scheduled against it. * fix: repair what a reconnect leaves behind when it cannot proceed cleanly Two things connectIdentity got wrong once the reconnect path existed. A reconnect through the other variant of a network moves the card to the new platform — same identity, different API flavor. Post targets carry their own platform snapshot, and that snapshot picks the publisher, the queue and the scopes checked before publishing. Left behind, it failed every pending post on a permission the account no longer needs: an Instagram card moved to the Facebook variant still demanded instagram_business_content_publish and stopped with "Missing permissions". Pending targets now follow the card and reset a content type the new platform cannot publish; published targets keep theirs, since they record what really went out under a platform_post_id from that API. The network lock timing out also arrived as a raw LockTimeoutException, which every OAuth callback filed through its generic catch: an error log and "Error connecting account" for the exact race the lock exists to absorb. It now carries a busy messageKey through the branch each flow already handles, the same way the Telegram path already reported it. * refactor: resolve the linkedin reconnect card once per select select() already looked the card up before deciding whether the chosen identity matches it, then connectPerson() and connectOrganization() looked it up again on their own — two identical queries per submit, and two places that could disagree about what is being reconnected. The caller passes what it already holds. * test: render the grid's multi-account branch phpunit.xml forces ALLOW_MULTIPLE_SOCIAL_ACCOUNTS false and no browser test overrode it, so the card the flag exists to add never rendered anywhere. The pair pins both sides: a taken network offers no second card when multiples are off, and offers one when they are on. * test: pin why the linkedin select guards exist connectIdentity() already refuses a mismatched reconnect and answers with the same wrong_account message, so every existing test passes with the two guards in select() deleted — which is exactly how they would get deleted. What they actually buy is skipping the avatar download that building the connect payload runs first. Both now assert the fetch never happens, so the guards fail loudly instead of looking redundant. * fix: carry retrying targets through a variant move, atomically Two holes in the move added a commit ago. It only carried pending targets, but a retrying one is not finished either — the publish job reschedules itself and reads the snapshot fresh on the next attempt, so leaving it behind meant it retried against the old variant until it exhausted its budget on a permission the account no longer needs. Failed and published targets stay put; a publishing one has a job mid-flight already working from the snapshot it read. The card and its targets also moved in three separate statements, so a crash between them left exactly the split this was meant to close. They share a transaction now. * chore: drop the dusk selectors nothing reads Laravel Dusk is not installed — no laravel/dusk requirement, no DuskTestCase, no browse(). Browser tests run on pest-plugin-browser driving Playwright, and its @selector resolves to data-testid. The 45 dusk attributes left across 18 components selected nothing. CLAUDE.md was the reason they kept coming back: it told every agent to add them. Its browser-testing section now describes the setup that exists — data-testid targeting, the wait helper these tests need because assertions do not auto-wait on SPA paint, and why BrowserTestCase keeps Vite real. Verified before removing: every @selector used in tests/Browser resolves to a data-testid, seven of them through bound :data-testid, so none depended on a dusk attribute. * chore: drop the last one-account-per-network helper hasConnectedPlatform() has no callers left anywhere — app, tests, views or routes. It sat directly above getSocialAccount(), which this branch already removed, and is the same leftover from when a workspace could hold one account per platform. --------- Co-authored-by: Paulo Castellano <paulo@castellanos.llc> Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-25 10:28:14 +00:00
it('reports a busy connect instead of letting the webhook fail', function () {
Http::fake();
Event::fake([TelegramConnectFailed::class]);
$code = TelegramConnectCode::issue($this->workspace->id, now()->addMinutes(15));
$lock = Cache::lock("social_connect:{$this->workspace->id}:telegram", 10);
expect($lock->get())->toBeTrue();
try {
$this->withHeader('X-Telegram-Bot-Api-Secret-Token', 'shh-secret')
->postJson(route('telegram.webhook'), telegramUpdate($code))
->assertNoContent();
} finally {
$lock->release();
}
Event::assertDispatched(
TelegramConnectFailed::class,
fn (TelegramConnectFailed $event): bool => $event->reason === 'busy'
);
expect($this->workspace->socialAccounts()->count())->toBe(0);
});