Multi YouTube account connection

[NagariaHussain]

Pick one while uploading

[NagariaHussain]

Implemented in 648792d — not closing yet, see "Not verified" below.

Problem

The integration was single-account by construction: one Connected App, one Token Cache, and VMS Settings held the one channel name plus the one connecting user. Every upload on the site went to whichever channel that user had authorized.

Solution

New VMS YouTube Channel DocType — one row per connected channel (channel id, name, encrypted refresh token, connected-by, default flag).

There is still only one Connected App, deliberately: a Connected App per channel would mean registering a new OAuth redirect URI in the Google Console for each one. Instead, after each authorization the refresh token is copied onto the channel record and the Token Cache is deleted, freeing the app for the next channel. The upload worker builds Credentials(token=None, refresh_token=...) and lets google-auth mint the access token, which is what removes the per-user-token constraint that made the old design single-account.

• Settings → YouTube lists every channel, with Make default / Remove per channel and Connect another channel (reuses the stored client credentials, so it goes straight to Google's account chooser).
• The upload dialog gains a Channel select — this is the "pick one while uploading" the issue asks for. It only appears when more than one channel is connected, so the single-account experience is unchanged.
• The chosen channel is recorded on VMS Asset.youtube_channel.
• A patch converts an existing single connection into a channel row.

Not verified

The real OAuth round trip has not been tested. This site's stored client secret is rejected by Google (invalid_client), so the existing connection was already dead and the migration patch logged and skipped. What was exercised, with two seeded channels: the settings list, default swapping, the picker, and the upload path itself — the job ran with the selected channel, downloaded from R2, and only failed at Google's token endpoint.

So the routing is real, but connecting a second actual YouTube account needs a pass with valid credentials before this is closed. Also worth watching there: Google only returns a refresh token on first consent for an account — finalize throws with a "revoke third-party access" hint if it's missing, which is likewise untested against the live flow.

[NagariaHussain]

Follow-up in 978deff — still not closing, the OAuth blocker below is unchanged.

Problem

648792d routed uploads to a chosen channel and recorded it on VMS Asset.youtube_channel, but nothing read that field back. After an upload finished there was no way to see which channel the video went to.

Worse, the label that was shown was wrong. The upload dialog derived the channel name from the picker's selection, which falls back to channels[0] — the default channel. An asset uploaded to a non-default channel and revisited after a reload was described as belonging to the default one.

Solution

• New channel_display_name() returns the channel's name, or "" for an unset channel and for one since removed, so a deleted channel degrades to no label instead of a dangling name. Both get_review_data and get_youtube_upload_status go through it, so the initial and polled payloads agree.
• The Share menu's View on YouTube item now carries the channel name beneath it. That's the one place the channel still matters once the upload is done — the menu swaps to View / Re-upload at that point, so the dialog's Complete state isn't reachable from there.
• The dialog prefers the stored channel whenever an upload exists (queued, complete or errored) and only falls back to the picker for a fresh upload. Its header reads "Uploaded to X" when complete.

Verified

With two seeded channels and an asset marked complete against the non-default one: both endpoints returned the right channel, the Share menu showed it, and the reopened dialog read "Uploading to CHTEST Second" where before it read "CHTEST Default". An asset with no upload still shows the picker preselected to the default.

Still blocking closure

The real OAuth round trip remains unverified — this site's stored client secret is rejected by Google with invalid_client. Connecting a second actual YouTube account needs a pass with valid credentials.

Smaller things noticed while in here: the Privacy select renders the raw value (unlisted) rather than a label, and removing a channel in Settings has no confirmation even though it unlinks that channel from every asset that used it.

[NagariaHussain]

Follow-up in 85c4fed — still not closing, the OAuth blocker is unchanged.

Problem

Removing a channel in Settings fired the moment you clicked it. Not recoverable: the API unlinks the channel from every asset that used it, then hard-deletes the record. The footer's Disconnect YouTube was worse and equally unguarded — it wipes every channel plus the stored OAuth credentials.

Solution

One AlertDialog covering both, driven by a pendingRemoval state that is either the channel or "all".

• Single-channel copy names the channel and says what actually breaks — assets lose the label showing where they were uploaded — and that the videos on YouTube are untouched.
• The "all" copy adds that the credentials go too, so reconnecting means pasting the Client ID and Secret again.
• Title reads "Disconnect all channels?" when more than one is connected, matching the footer button's existing wording.

Verified

With two seeded channels: Remove named the right channel, Cancel left both in place, confirming removed only the non-default one, and the full disconnect emptied the list and dropped the panel back to the credentials form.

Not seen on screen: the "Disconnect all channels?" title branch — the full disconnect was exercised with one channel left, which renders the other title. Single ternary, but untested as written.

Still blocking closure

The real OAuth round trip. This site's stored client secret is rejected by Google with invalid_client, so connecting a second actual YouTube account needs a pass with valid credentials.

Still open from before: the Privacy select renders the raw value (unlisted) rather than a label.

[NagariaHussain]

Follow-up in 04b7fb2 — still not closing, the OAuth blocker is unchanged.

Problem

The Privacy select in the upload dialog rendered a bare <SelectValue />, so the trigger showed the API value — unlisted — while the dropdown items beneath it read "Unlisted". Two spellings of the same choice in one control.

Solution

Options moved to a module-level PRIVACY_OPTIONS list, with the items mapped from it and SelectValue given the same render-function child the Channel select already uses. Trigger label and item label now come from one place. Also added w-full to the trigger — without it the select sized to its content and sat narrower than the Title and Description fields above it.

Verified

Dialog opened reading "Unlisted"; picking Private changed the trigger to "Private"; the select now spans the dialog width like the other fields.

Still blocking closure

The real OAuth round trip. This site's stored client secret is rejected by Google with invalid_client, so connecting a second actual YouTube account needs a pass with valid credentials. The "Disconnect all channels?" title branch from 85c4fed is also still unseen on screen.

[NagariaHussain]

Follow-up in 97e0043 — still not closing, the OAuth blocker is unchanged.

Problem

The removal confirmation added in 85c4fed said assets would be unlinked but not how many, so it could not tell you whether you were about to lose one label or thirty. get_youtube_status had no count to give it.

Solution

• get_youtube_channels carries an asset_count per channel, from one grouped query rather than a count per channel. Same number feeds the settings panel and both dialog branches.
• The count shows on each channel row too — "Default channel · 2 assets" — so it is visible before you click Remove.
• One unlinkWarning(count, target) helper writes the sentence for both branches and switches the pronoun on the count ("which channel it went to" for one asset, "they" for several). Zero reads "No assets were uploaded to it." rather than a hypothetical.

Verified

With two seeded channels holding 1 and 2 assets: rows read "· 2 assets" / "· 1 asset" and the counts vanish when nothing is linked. Remove on the 1-asset channel read "1 asset uploaded to it will no longer show which channel it went to."; with no assets, "No assets were uploaded to it."; Disconnect all read "3 assets uploaded through them will no longer show which channel they went to."

That last one also put the "Disconnect all channels?" title branch from 85c4fed on screen for the first time — that outstanding note is now cleared.

Minor: the count is a snapshot from page load, not refetched while the dialog is open, so an upload landing in between would make it stale by one.

Still blocking closure

The real OAuth round trip. This site's stored client secret is rejected by Google with invalid_client, so connecting a second actual YouTube account needs a pass with valid credentials.

[NagariaHussain]

Follow-up in 35bfb1f — still not closing, the OAuth blocker is unchanged.

Problem

The asset count the removal confirmation quotes came from whatever get_youtube_status returned at page load, and Settings is a long-lived panel with revalidateOnFocus: false. An upload landing after load left the dialog stating a number that was already wrong — the one place where being off by one changes what you decide.

Solution

openRemoval(target) sets the pending state and revalidates, so the count refreshes as the dialog opens. pendingRemoval now holds the channel's name rather than the channel object, with the channel derived from the current list — holding the object would have pinned the stale count in state and defeated the refetch. The dialog also closes itself if the channel is gone from the refreshed list, instead of rendering an empty name and a zero count.

Verified

Two seeded channels holding 1 and 2 assets. A third asset was linked to the second channel directly in the DB, behind the page's back, with no reload. Remove then read "3 assets uploaded to it will no longer show which channel they went to." — before this change it read 2. Cancelling left the row reading "· 3 assets", and Disconnect all read "4 assets", matching 1 + 3. Regression: the singular branch still reads "1 asset", and confirming a removal still removes the right channel and promotes the other to default.

Minor: the refresh is fire-and-forget, so a slow request leaves the cached number on screen for a moment with no sign it is provisional.

Still blocking closure

The real OAuth round trip. This site's stored client secret is rejected by Google with invalid_client, so connecting a second actual YouTube account needs a pass with valid credentials.

[NagariaHussain]

Follow-up in 14e2fd0 — still not closing, the OAuth blocker is unchanged.

Problem

35bfb1f made the removal dialog revalidate the channel list as it opens, but the refresh is fire-and-forget. The dialog renders the cached count immediately and swaps in the fresh one when the request lands, so on a slow request you read — and decide on — a number that is about to be replaced, with nothing saying so.

Solution

While the refresh is in flight, the count sentence is replaced by an italic "Checking how many assets this affects..." in both the single-channel and disconnect-all branches. Only the claim that can be stale disappears; the rest of the copy stays.

Gated behind a 300ms timer rather than shown the instant isValidating flips — on a local site the request lands in tens of milliseconds, so an ungated version flashed the line on every open, a flicker worse than the staleness it warns about. The confirm button stays enabled: the count is informational, and blocking on it would make a slow network block a removal already decided on.

Verified

Two seeded channels holding 1 and 2 assets, with a temporary 2s sleep in get_youtube_status to make the pending state observable: Remove read "Checking how many assets this affects..." then settled to "2 assets uploaded to it will no longer show which channel they went to."; Disconnect all did the same and settled to "3 assets ... through them". With the sleep removed the dialog opens straight to the number with no checking line, so the fast path is unchanged. Confirming a removal still removes the right channel and promotes the other to default.

Minor: the 300ms threshold is a guess. Reads well locally; a genuinely slow connection where every open crosses it has not been tried.

Still blocking closure

The real OAuth round trip. This site's stored client secret is rejected by Google with invalid_client, so connecting a second actual YouTube account needs a pass with valid credentials.

[NagariaHussain]

Follow-up in 9b3067e — still not closing, the OAuth blocker is unchanged.

Problem

The full-disconnect confirmation promises that "the stored OAuth credentials will be removed" and that you'll need to enter the Client ID and Secret again. Neither was true — disconnect_youtube deleted the channels, Token Caches and Connected App but left both credentials in VMS Settings.

The reverse was wrong too. get_youtube_status has returned has_credentials since the multi-channel work landed, and nothing read it: removing your only channel kept the credentials but dropped you back to the two-step paste form, asking for a Client ID and Secret the site already had.

Solution

• _sync_settings_summary takes clear_credentials, passed only by disconnect_youtube. Removing a single channel keeps the credentials — the remaining channels need them, and swapping accounts shouldn't cost you your Google Cloud setup.
• Cleared by setting the fields to None on the Settings doc, not via db.set_value, so the Password field's __Auth row goes too. A raw write would have left the secret behind, which is the bug.
• When no channel is connected but credentials are stored, the panel now shows an "OAuth credentials saved" block and Connect YouTube goes straight to Google reusing them — the same path "Connect another channel" already used. An Enter different credentials button reveals the original paste form for a rotated secret.

Verified

Backend: removing one of two channels left has_credentials: true with the client id and decryptable secret intact; the full disconnect returned has_credentials: false, client id '', get_password → None.

Browser: removing the only channel landed on the saved-credentials block where it previously showed the paste form; "Enter different credentials" brought the form back; with a channel re-seeded, Disconnect all went straight to the paste form, matching its own copy.

Still blocking closure

The real OAuth round trip — this site's stored client secret is rejected by Google with invalid_client, so connecting a second actual account needs a pass with valid credentials.

New note: re-authorizing an account that is already connected updates the existing channel row in place and still toasts "YouTube connected successfully", so picking the same Google account twice looks like it did nothing.

[NagariaHussain]

Follow-up in c709584 — still not closing, the OAuth blocker is unchanged.

Problem

finalize_youtube_connection upserts on channel_id, so re-authorizing an account that is already connected updates that channel row in place. The frontend toasted "YouTube connected successfully" either way, and the channel list didn't change — so picking the same Google account twice looked like the connect had done nothing.

Solution

• is_new added to the finalize response. The backend already knows; diffing the channel list on the frontend would have to run before the mutate() and would be wrong with two tabs open.
• Reconnect reads "X was already connected — its access has been refreshed". The refresh token was replaced, which is the real reason to re-authorize (a revoked or expired grant), so it should confirm it worked rather than read as an error.
• Both branches now name the channel, so a new connection says which account was picked.

Verified

Backend with _fetch_channel stubbed and a seeded non-expired Token Cache: first finalize is_new: true / 1 channel, same channel id again is_new: false / still 1, a different channel id is_new: true / 2.

Browser, same stub: with the channel present the toast read "RECON Channel was already connected — its access has been refreshed"; with the row deleted first it read "Connected RECON Channel" and the channel reappeared.

Still blocking closure

The real OAuth round trip. This site's stored client secret is rejected by Google with invalid_client, so connecting a second actual account needs a pass with valid credentials — note the whole finalize path has only ever been exercised against a stub.

New note: deleting a VMS YouTube Channel directly rather than through disconnect_youtube_channel leaves no channel marked default — the promotion only lives in the API. Only reachable from the console today, but an on_trash would be safer.

[NagariaHussain]

Follow-up in 67c1298 — still not closing, the OAuth blocker is unchanged.

Problem

Unlinking assets from a removed channel, promoting a successor default and syncing the VMS Settings summary all lived inside disconnect_youtube_channel. Deleting a VMS YouTube Channel any other way — desk list view, console, a future bulk delete — left assets pointing at a missing record, no channel marked default, and a settings summary naming a channel that no longer existed.

Solution

Moved onto the doctype controller. on_trash unlinks the assets and promotes a successor; after_delete syncs the settings summary, which has to run once the row is gone since it reads the channel list. The successor query excludes self.name — at on_trash time the row is still there, so an unfiltered "oldest remaining" would re-elect the channel being deleted.

disconnect_youtube_channel now just deletes. disconnect_youtube keeps its own _sync_settings_summary(clear_credentials=True), since clearing credentials is not a per-channel concern.

Verified

Backend: with two channels and an asset linked to the default one, deleting that channel directly via frappe.delete_doc (no API) left the asset unlinked, the remaining channel is_default 1, and the summary reading the survivor's name. The API path then removed the last channel and the summary went to connected 0.

Browser: Remove on the non-default channel left one showing "Default channel"; Remove on that last one landed on the saved-credentials empty state, so the sync fired through the controller.

Still blocking closure

The real OAuth round trip — this site's stored client secret is rejected by Google with invalid_client.

New note: nothing enforces exactly one default on insert/save. A doc created with is_default=1 in the console would give two defaults; the picker takes the first by order, so it degrades quietly rather than failing.

[NagariaHussain]

Follow-up in 8e8ac3c — still not closing, the OAuth blocker is unchanged.

Problem

Nothing kept the "exactly one default channel" invariant on insert or save. It only held because every write went through finalize_youtube_connection or set_default_youtube_channel. A doc created with is_default=1 from the desk or console gave two defaults; clearing the flag on the only channel gave zero. The upload picker orders by is_default desc, creation asc, so both cases degrade quietly — it falls back to creation order rather than failing.

Solution

Moved the invariant onto the controller.

• validate promotes: a channel saved while no other one is marked default takes the flag. Covers the first channel connected and the someone-cleared-it case in one rule, so there is never zero.
• on_update demotes: a channel saved as default clears the flag on the others. After save rather than in validate, since a new doc's name isn't final until insert and the query has to exclude self.
• Both write with db.set_value, so demoting a peer doesn't re-enter the controller.
• finalize_youtube_connection no longer special-cases the first channel.

Verified

Backend: first channel saved default, second stayed non-default, a third created with is_default=1 demoted both others, deleting the default promoted a successor, and clearing the flag by hand then saving re-elected one.

Browser: with two seeded channels, "Make default" swapped the default and the panel re-sorted, DB showing exactly one default each time.

Still blocking closure

The real OAuth round trip — this site's stored client secret is rejected by Google with invalid_client. Connecting a second real account needs a pass with valid credentials, and the whole finalize path has only ever been exercised against a stub.

New note: set_default_youtube_channel still writes with db.set_value in a loop, bypassing the new controller rules. Consistent today, but the invariant now lives in two places.

[NagariaHussain]

Follow-up in c6e73de — still not closing, the OAuth blocker is unchanged.

Problem

8e8ac3c moved the "exactly one default channel" invariant onto the VMS YouTube Channel controller, but set_default_youtube_channel still wrote the flag itself — a db.set_value loop over every channel, setting 1 on the target and 0 on the rest. Same result, so the two implementations agreed today, but the rule lived in two places and only one would be updated next time it changed.

Solution

The API loads the chosen channel, sets is_default and saves. on_update demotes the others, so the loop is gone and the invariant has a single owner. The explicit _sync_settings_summary() call stays — the summary reads the list default-first so switching does change it, but syncing it isn't part of the invariant. The existence check stays too, since it throws a translated message where get_doc would raise DoesNotExistError.

Verified

Backend with three seeded channels: first saved default, setting the third gave exactly one default, setting the same one again was a no-op, setting the first swapped back with the settings summary following.

Browser: "Make default" on the second channel moved it to the top as "Default channel", the other dropped to "Connected" with its own button, DB showing exactly one default.

Still blocking closure

The real OAuth round trip — this site's stored client secret is rejected by Google with invalid_client. Connecting a second real account needs a pass with valid credentials, and the finalize path has only ever been exercised against a stub.