fix: empty-deck and parser-error copy on the upload page#2453
Merged
Conversation
EmptyDeckError now returns code 'empty_export' with a docsLink and copy
that works for any file format, not just Notion. The previous message
("No toggles found in [filename]") read wrong for markdown, csv, and PDF
uploads since none of those use Notion toggles.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
classifyUploadError now falls back to copy that names the file context
("Something broke while reading this file…"). The generic fallback
stays in place for thrown errors (network, etc.) where mentioning "the
file" would be wrong.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Server-thrown EmptyDeckError now lands in the emptyDeck UI rather than the generic error state, matching the 200+0-cards flow. The download button is hidden when no blob is available. The default empty-deck body shows the spec copy with an inline link to common-problems. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Notion's list-block-children union includes PartialBlockObjectResponse, which lacks the type field. Reading r.type before narrowing fails tsc under the strict @notionhq/client 5.x types. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
✅ Deploy Preview for notion2anki ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
1 task
aalemayhu
added a commit
that referenced
this pull request
May 19, 2026
…ntry Playwright covers: new user sees tour, Skip hides it and calls /api/users/me/onboarded, already-onboarded user does not see tour, user created before migration cutoff does not see tour. Changelog entry covers the first-deck-success wave (PRs #2453, trial-on-reg, login-loop, and this PR — wave 4 of 4). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
aalemayhu
added a commit
that referenced
this pull request
May 19, 2026
## What Adds a four-step popover tour that appears once on a new user's first authenticated visit to `/upload`. Each step has Back / Next / Skip controls. Pressing Skip marks the user as onboarded (stored in `users.onboarded_at TIMESTAMPTZ NULL`) so the tour never appears again — including on new devices. Closes #252. Wave 4 of 4 — closes the first-deck-success spec (#2446). ## Why A new account-holder reaching `/upload` with no orientation gives up because they don't know where to start. The four-step tour surfaces the four key actions at exactly the right moment without blocking the workflow. ## How - **Migration**: `20260608000000_add_onboarded_at_to_users.js` — `users.onboarded_at TIMESTAMPTZ NULL DEFAULT NULL`. `pnpm kanel` types updated. - **Server**: `markOnboarded(userId)` in `UsersRepository` (idempotent — only flips NULL → now()), `markOnboarded` controller method reading `userId` from `res.locals` (CWE-639), new route `PATCH /api/users/me/onboarded` behind `RequireAuthentication`. - **`getLocals` response**: adds `created_at` and `onboarded_at` to the user object so the frontend can gate the tour without a second fetch. - **Client**: `OnboardingTour` component (~150 LOC, zero new deps), `markOnboarded.ts` API call. Tour gates on `created_at >= 2026-06-08` (migration timestamp) AND `onboarded_at === null`. - **Cutoff**: users created before the migration don't see the tour — no re-onboarding existing users. ## Measuring success Log line: `PATCH /api/users/me/onboarded` 204 responses at > 0 per day within the first week post-deploy. Metric to track: ratio of new users (created_at >= 2026-06-08) who complete their first conversion within 24 hours of signup. ## Testing - Vitest: 10 new unit tests in `OnboardingTour.test.tsx` covering first-time render, already-onboarded suppression, pre-migration-cutoff suppression, step navigation, Back/Next/Skip, and final-step Skip. - Playwright: 4 e2e tests in `onboarding-tour.spec.ts` covering the Skip end-to-end path and the three suppression conditions. - `StripeController.test.ts`: updated `buildUser` fixture to include `onboarded_at: null` after the kanel regen. - All 752 Vitest tests pass. Server tsc clean. ## Changelog ``` { type: 'feature', title: 'Upload page — a short guided tour walks you through your first conversion on a new account, and a clearer message shows when a file produces no cards', date: '2026-06-08' } ``` This entry covers the first-deck-success wave (#2446): this PR (tour), #2453 (empty-deck copy), trial-on-reg, and login-loop. Per the spec, the combined wave entry lands in this final PR. ## Risks - The tour only appears for new users (created_at ≥ migration timestamp). Existing users are unaffected. - `markOnboarded` is idempotent — calling it twice is safe. - Rollback: drop the `onboarded_at` column via the `down` migration; remove the route; revert the client component. Tour won't appear. - The kanel types were updated manually (kanel binary not installed in worktree). Running `pnpm kanel` post-migration will produce identical output. ## Security - Endpoint reads `userId` from `res.locals.user` (set by `RequireAuthentication` middleware) — never from the request body. CWE-639 mitigated. - No sensitive data logged or returned. - User-row mutation: `/security-review` recommended before merge. ## Goal alignment Reduces first-visit abandonment for new users, directly contributing to the 300K-user retention goal. A user who completes one successful conversion is significantly more likely to return. ## Note on sonar-scanner `sonar-scanner` was not run locally (`SONAR_TOKEN` not configured in this worktree environment). No new SQL, no HTML injection surface, no zip extraction, no user-controlled URLs — the new code path is outside Sonar's high-risk triggers. Reviewers should watch for cognitive complexity on the component. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/2anki/codesmith/server/pr/2456"><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 Sonnet 4.6 <noreply@anthropic.com>
![https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source](https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source){kind=link}
![https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img](https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img){kind=link}
![https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a](https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a){kind=link}
1 task
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
1 participant
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
When an upload produces no cards, the upload page now shows the same info card whether the server caught it (
EmptyDeckError→ 400) or the parser produced an empty 200 response. Body copy is format-agnostic — works for Notion, markdown, csv, PDF — and links to/documentation/help/common-problems. Generic parser failures get a sharper fallback that names the file context and routes the user to support.Wave PR 1 of 4 from the first-deck-success spec. Issues #721 and #731.
Why
The previous server message ("No toggles found in [filename]…") read wrong for any non-Notion upload — markdown and PDF files don't use Notion toggles, so the suggestion to "wrap your content in toggles" was incorrect for those formats. The default empty-deck body on the 200+0 path had the same Notion-centric bias. The conversion-error fallback ("Something went wrong. Try again. If the problem keeps happening, email support@2anki.net.") didn't name the file context, so users couldn't tell if it was a network issue or a parser failure.
How
EmptyDeckResponseserver-side now carriescode: 'empty_export'anddocsLink: '/documentation/help/common-problems'alongside the message. Existingempty_exportcode inUploadErrorBodyis now actually used.UploadFormroutes any 400 withcode: 'empty_export'to the existing'emptyDeck'zone-state (info card) instead of the generic error state. HelperzoneStateForUploadErrorkeeps the three upload-handler call sites consistent.renderEmptyDeckBodybranch (non-Google-Drive) renders the spec text with an inline link to common-problems.classifyUploadErrorfalls back to upload-context copy ("Something broke while reading this file. Try again, or send the file to support@2anki.net so we can fix the parser.") rather than the generic "Something went wrong." that's correct for thrown errors but wrong for upload responses.Measuring success
Empty-deck-to-docs link clicks become trackable from
/documentation/help/common-problemsreferrer. The parser-error fallback rate stays low (it's a true-fallback path); if it spikes, that's a regression elsewhere.Testing
pnpm test src/services/UploadService.test.ts— new assertions forcode, message, anddocsLink.pnpm --filter 2anki-web test:run— three new vitest cases:code: 'empty_export'lands in the emptyDeck info card (not the error state, no download button)classifyUploadErrorunit test for the parser-error fallbackRisks
EmptyDeckErrorserver-side used to render under "Something went wrong" with the WarningIcon; it now renders as the info-card empty-deck state. Visually different.Goal alignment
Activation. The first-deck-success spec frames the four upload-screen dead-ends as the cheapest activation win. Empty-deck copy that names the next step (instead of confusing non-Notion uploaders with toggle vocabulary) keeps users moving instead of bouncing.
Drive-by fix
The pre-push tsc hook caught two pre-existing errors in
src/services/NotionService/blocks/lists/BlockTable.tsx(from PR #2445, merged earlier today) wherer.type === 'table_row'doesn't narrow againstPartialBlockObjectResponse | BlockObjectResponse. One commit on this branch adds the'type' in rguard. Existing 9 BlockTable tests still pass.Notes
Need help on this PR? Tag
@codesmithwith what you need.