feat: Google Drive upload history#2214
Closed
aalemayhu wants to merge 1 commit into
Closed
Conversation
The google_drive_uploads table has been write-only since August 2024. This spec defines a two-phase delivery: PR 1 surfaces the history on the Downloads page (read + delete + last_converted_at migration); PR 2 adds one-click re-convert once the OAuth re-auth story is worked out.
|
1 task
Contributor
Author
|
Superseded by #2305 — the original draft branch was renamed from |
aalemayhu
added a commit
that referenced
this pull request
May 15, 2026
## What Adds a "From Google Drive" section to the Downloads page below "From Dropbox". Authenticated users see the files they've picked from Drive with file icon, formatted size, relative time since last conversion, an "Open in Drive ↗" link, and a × to remove the row from history. The Drive file itself is never touched. A `last_converted_at timestamptz` column lands on `google_drive_uploads` via an additive migration, and the existing `saveFiles` upsert UPDATE branch now refreshes it on every re-conversion so the recency sort reflects the user's actual conversion history, not the first time they picked the file. ## Why The `google_drive_uploads` table has been write-only since August 2024 — Drive users have repeatedly re-picked the same file from the Drive Picker because nothing surfaced their history. This closes the loop for returning Drive users, the same way #2300 did for Dropbox. Target: +5pp return-upload rate among users with `google_drive_uploads` rows within 4 weeks. Phase-2 gate: ≥15% of section viewers click "Open in Drive" before we invest in one-click re-convert. ## How - Migration: additive `last_converted_at timestamptz default now()` (nullable so historical rows keep NULL and sort last) + `owner` index. - `GoogleDriveRepository.getByOwner`: selects only the columns the UI needs, filters `mimeType != 'application/vnd.google-apps.folder'`, orders by `last_converted_at DESC NULLS LAST`. - `GoogleDriveRepository.deleteByIdAndOwner(id: string, owner: number)`: parameterized; controller pre-validates the string PK with `/^[A-Za-z0-9_-]+$/` so anything outside the Drive ID alphabet is rejected at the boundary. - `GoogleDriveRepository.saveFiles` upsert UPDATE path now sets `last_converted_at = now()` — otherwise repeat converters would still see their first-upload timestamp. - `GetGoogleDriveUploadsUseCase` + `DeleteGoogleDriveUploadUseCase`: map rows to an explicit typed response (no raw DB rows through `res.json`). - `UploadController.getGoogleDriveUploads` + `.deleteGoogleDriveUpload`: owner always from `res.locals`; URL/query/body never trusted for owner. - Routes: `GET /api/upload/google_drive/mine` and `DELETE /api/upload/google_drive/mine/:id` behind `RequireAuthentication`. - Web: `useGoogleDriveUploads` hook, `GoogleDriveHistorySection`, `GoogleDriveHistoryEntry`. Section hidden when empty; error state matches VOICE.md. Icon `src` is sanitized to a small allowlist of Google CDNs (`drive-thirdparty.googleusercontent.com`, `ssl.gstatic.com`, `lh3.googleusercontent.com`) with `onError` swap to `/icons/file-generic.svg`. The "Open in Drive" `href` is sanitized to `drive.google.com` and `docs.google.com` https origins; any other URL becomes a disabled "Link unavailable" span. Size formatter handles Kanel's `string` type for `sizeBytes`. **Open question resolutions:** - Q1 (`url` safety): Trusted only after origin allowlist + `https:` check at render time. No OAuth tokens have ever been stored in the `url` column historically; the allowlist is defense in depth. - Q2 (`owner` index): Added in the migration — missing on the original 2024 create migration. - Q3 (paywall): History available to all authenticated users — matches the Dropbox phase-1 decision so the Downloads page stays consistent. ## Measuring success `GET /api/upload/google_drive/mine` returning 200s in server logs for authenticated sessions. Secondary: return-upload rate among users with `google_drive_uploads` rows shows +5pp within 4 weeks; ≥15% of section viewers click "Open in Drive" in their first session (validation gate for phase 2 / re-convert). ## Testing - Server (Jest, all green): `GoogleDriveRepository.test.ts` (6 — owner guards on get+delete, folder exclusion, last_converted_at ordering, parameterized id with crafted string), `GetGoogleDriveUploadsUseCase.test.ts` (3 — shape mapping that drops owner/description/embedUrl, passthrough, empty), `DeleteGoogleDriveUploadUseCase.test.ts` (3 — delegation, 404 on 0 deleted, success), `GoogleDriveController.test.ts` (8 — 401 guards, 400 on missing/invalid string id, 404 on missing row, 200 with clean id). - Web (Vitest, all green): `useGoogleDriveUploads.test.tsx` (4 — loading→empty, populated with hasMore, error path, deleteUpload by string key). `DownloadsPage.test.tsx` updated to mock the new hook. - `/check`: server tsc clean, web typecheck clean, 476 Vitest tests pass, Biome lint clean. - Full server suite: 1214 tests passing under `src/`. ## Risks - `last_converted_at` is not yet reflected in Kanel-generated `GoogleDriveUploads.ts`. The repository defines its own `GoogleDriveUploadRow` type for the read path, and the UPDATE in `saveFiles` uses `database.fn.now()` directly — no Kanel-typed write field. `pnpm kanel` should be run on the dev DB after the migration applies to refresh the type. - Rollback: the migration has a clean `down` (drop index then column). The API routes are additive — removing them is safe. ## Sonar Sonar scanner not run locally (token not configured in this environment) — flagging for reviewers so a Sonar bounce isn't a surprise. ## Goal alignment Direct line to the 300K-user goal: re-conversions are the fastest way to grow weekly conversions per user, and the Drive Picker step is the dominant friction for returning users with embedded slides / Docs. The feature is intentionally read-first so we can validate the click-through gate before building the (harder) OAuth re-auth re-convert. --- <details> <summary>Trio synthesis (from original spec PR #2214)</summary> - **PM:** Read-only history list with "Open in Drive" link; "Convert again" deferred (OAuth token not stored); gate on ≥15% click-through before building re-convert. - **Designer:** Fourth section on `/downloads`, below "From Dropbox", using a table layout (File / Size / Added / Actions) matching FinishedJobs; hide when empty; sanitize `url` to Drive origins only. - **Engineer:** S+ effort; string PK needs regex validation; upsert semantics make `created_at` alone misleading — `last_converted_at` is the right recency signal; `sizeBytes` arrives as string from Kanel. - **Conflict:** PM suggested sorting by `lastEditedUtc DESC`; Engineer chose `last_converted_at`. **Resolution:** `last_converted_at` — Drive's edit time is the file owner's clock, not the user's conversion history. - **Resolution:** PR 1 (this PR) ships read + delete + `last_converted_at` migration. PR 2 (re-convert) deferred until phase-1 metrics clear the gate. </details> <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/2anki/codesmith/server/pr/2305"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



What
Adds a "From Google Drive" section to the Downloads page below "From Dropbox". Authenticated users see the files they've picked from Drive with file icon, formatted size, relative time since last conversion, an "Open in Drive ↗" link, and a × to remove the row from history. The Drive file itself is never touched.
A
last_converted_at timestamptzcolumn lands ongoogle_drive_uploadsvia an additive migration, and the existingsaveFilesupsert UPDATE branch now refreshes it on every re-conversion so the recency sort reflects the user's actual conversion history, not the first time they picked the file.Why
The
google_drive_uploadstable has been write-only since August 2024 — Drive users have repeatedly re-picked the same file from the Drive Picker because nothing surfaced their history. This closes the loop for returning Drive users, the same way #2300 did for Dropbox. Target: +5pp return-upload rate among users withgoogle_drive_uploadsrows within 4 weeks. Phase-2 gate: ≥15% of section viewers click "Open in Drive" before we invest in one-click re-convert.How
last_converted_at timestamptz default now()(nullable so historical rows keep NULL and sort last) +ownerindex.GoogleDriveRepository.getByOwner: selects only the columns the UI needs, filtersmimeType != 'application/vnd.google-apps.folder', orders bylast_converted_at DESC NULLS LAST.GoogleDriveRepository.deleteByIdAndOwner(id: string, owner: number): parameterized; controller pre-validates the string PK with/^[A-Za-z0-9_-]+$/so anything outside the Drive ID alphabet is rejected at the boundary.GoogleDriveRepository.saveFilesupsert UPDATE path now setslast_converted_at = now()— otherwise repeat converters would still see their first-upload timestamp.GetGoogleDriveUploadsUseCase+DeleteGoogleDriveUploadUseCase: map rows to an explicit typed response (no raw DB rows throughres.json).UploadController.getGoogleDriveUploads+.deleteGoogleDriveUpload: owner always fromres.locals; URL/query/body never trusted for owner.GET /api/upload/google_drive/mineandDELETE /api/upload/google_drive/mine/:idbehindRequireAuthentication.useGoogleDriveUploadshook,GoogleDriveHistorySection,GoogleDriveHistoryEntry. Section hidden when empty; error state matches VOICE.md. Iconsrcis sanitized to a small allowlist of Google CDNs (drive-thirdparty.googleusercontent.com,ssl.gstatic.com,lh3.googleusercontent.com) withonErrorswap to/icons/file-generic.svg. The "Open in Drive"hrefis sanitized todrive.google.comanddocs.google.comhttps origins; any other URL becomes a disabled "Link unavailable" span. Size formatter handles Kanel'sstringtype forsizeBytes.Open question resolutions:
urlsafety): Trusted only after origin allowlist +https:check at render time. No OAuth tokens have ever been stored in theurlcolumn historically; the allowlist is defense in depth.ownerindex): Added in the migration — missing on the original 2024 create migration.Measuring success
GET /api/upload/google_drive/minereturning 200s in server logs for authenticated sessions. Secondary: return-upload rate among users withgoogle_drive_uploadsrows shows +5pp within 4 weeks; ≥15% of section viewers click "Open in Drive" in their first session (validation gate for phase 2 / re-convert).Testing
GoogleDriveRepository.test.ts(6 — owner guards on get+delete, folder exclusion, last_converted_at ordering, parameterized id with crafted string),GetGoogleDriveUploadsUseCase.test.ts(3 — shape mapping that drops owner/description/embedUrl, passthrough, empty),DeleteGoogleDriveUploadUseCase.test.ts(3 — delegation, 404 on 0 deleted, success),GoogleDriveController.test.ts(8 — 401 guards, 400 on missing/invalid string id, 404 on missing row, 200 with crafted clean id).useGoogleDriveUploads.test.tsx(4 — loading→empty, populated with hasMore, error path, deleteUpload by string key).DownloadsPage.test.tsxupdated to mock the new hook./check: server tsc clean, web typecheck clean, 476 Vitest tests pass, Biome lint clean.src/.Risks
last_converted_atis not yet reflected in Kanel-generatedGoogleDriveUploads.ts. The repository defines its ownGoogleDriveUploadRowtype for the read path, and the UPDATE insaveFilesusesdatabase.fn.now()directly — no Kanel-typed write field.pnpm kanelshould be run on the dev DB after the migration applies to refresh the type.down(drop index then column). The API routes are additive — removing them is safe.Sonar
Sonar scanner not run locally (token not configured in this environment) — flagging for reviewers so a Sonar bounce isn't a surprise.
Goal alignment
Direct line to the 300K-user goal: re-conversions are the fastest way to grow weekly conversions per user, and the Drive Picker step is the dominant friction for returning users with embedded slides / Docs. The feature is intentionally read-first so we can validate the click-through gate before building the (harder) OAuth-re-auth re-convert.
Trio synthesis (from original spec PR #2214)
/downloads, below "From Dropbox", using a table layout (File / Size / Added / Actions) matching FinishedJobs; hide when empty; sanitizeurlto Drive origins only.created_atalone misleading —last_converted_atis the right recency signal;sizeBytesarrives as string from Kanel.lastEditedUtc DESC; Engineer choselast_converted_at. Resolution:last_converted_at— Drive's edit time is the file owner's clock, not the user's conversion history.last_converted_atmigration. PR 2 (re-convert) deferred until phase-1 metrics clear the gate.