feat(share): public playlist route /p/$token (phase 1.g.2)#20
Conversation
SSR route consuming /api/v1/share/playlists/{token} (no auth).
Renders metadata, cover, empty tracklist, plus OG + Twitter tags.
Validator rejects path-injection chars (slash, dot, percent).
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughCe PR ajoute le partage public de playlists via URL tokenisée : un utilitaire de fetch public, contrats et validation de token, une server‑fn ChangesPublic Playlist Sharing
Sequence DiagramsequenceDiagram
participant User as Navigateur
participant Route as /p/$token route
participant ServerFn as getPublicPlaylist
participant Validator as isWellShapedToken
participant PublicFetch as waveflowFetchPublic
participant WaveflowAPI as Waveflow Public API
User->>Route: visite /p/⟨token⟩
Route->>ServerFn: loader(token)
ServerFn->>Validator: isWellShapedToken(token)
alt token invalide
ServerFn-->>Route: throw notFound()
else token valide
ServerFn->>PublicFetch: GET /api/v1/share/playlists/encodeURIComponent(token)
PublicFetch->>WaveflowAPI: GET (sans Authorization)
alt 200 OK
WaveflowAPI-->>PublicFetch: playlist JSON
PublicFetch-->>ServerFn: PublicPlaylist
ServerFn-->>Route: {kind: 'ok', playlist}
else 404
WaveflowAPI-->>PublicFetch: 404
PublicFetch-->>ServerFn: WaveflowServerError(404)
ServerFn-->>Route: throw notFound()
else autre erreur
WaveflowAPI-->>PublicFetch: erreur
PublicFetch-->>ServerFn: WaveflowServerError / throw
ServerFn-->>Route: {kind: 'error', message}
end
end
Route->>User: render PlaylistPanel / NotFoundPanel / ErrorPanel
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/p`.$token.tsx:
- Around line 30-35: The head helper in p.$token.tsx currently treats any non-ok
loaderData as “Playlist not found”, which mislabels transient server errors;
update the head({ loaderData }) logic to distinguish loaderData.kind === 'error'
from the 'not found' case used by ErrorPanel and return an appropriate SSR title
(e.g., "Server error · WaveFlow" or a generic "Error · WaveFlow") when kind ===
'error', leaving "Playlist not found · WaveFlow" only for the not-found branch
so preview/social titles match ErrorPanel.
- Around line 77-113: PlaylistPanel is loading cover_hash, color_id and icon_id
but never rendering the cover preview; update the JSX in PlaylistPanel to render
a cover preview component/div above the playlist name that uses
playlist.cover_hash (for image URL or data), playlist.color_id (for background
color/fallback), and playlist.icon_id (for overlay/icon) and provide sensible
fallbacks when values are missing so the preview shows on the public page;
locate the PlaylistPanel function and add the preview markup/component before
the <h1> that references those playlist properties and ensure accessible
alt/text and responsive sizing.
In `@src/server-fns/share.ts`:
- Around line 65-75: The input validation currently done in inputValidator
causes malformed tokens to throw before the handler and break the intended
public-route rendering; move the token check into the getPublicPlaylist handler
so the route can return { kind: 'not_found' } for any bad token, remove the
throw from inputValidator, and tighten isWellShapedToken's regex from
/^[A-Za-z0-9]{8,128}$/ to /^[A-Za-z0-9]{32}$/ so only 32-char alphanumeric
tokens are accepted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 34c4d726-ce4c-42f9-9257-001fea71a249
📒 Files selected for processing (4)
src/lib/server/waveflow-server.tssrc/routes/p.$token.tsxsrc/server-fns/share.test.tssrc/server-fns/share.ts
…lidator Three findings from CodeRabbit on the share preview surface. head() now distinguishes 'not_found' from 'error' so the SSR title and social preview match what ErrorPanel actually renders instead of falsely claiming the playlist is gone. PlaylistPanel renders a brand-colored cover tile derived from color_id with the playlist name initial as overlay. cover_hash stays unrendered until the server-side artwork pipeline exposes it as a public URL. getPublicPlaylist's input validator no longer throws on malformed tokens. Shape check (strict 32-char regex, matching the server's mint format) lives in the handler and returns not_found, so bad tokens render the friendly empty state instead of the framework error boundary. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai thanks for the careful review — all three findings addressed in 5de25d7. 1. 2. PlaylistPanel cover preview — added a brand-colored tile (h-32 / sm:h-40, rounded-2xl) above the 3. Token validator — Local checks green: typecheck, lint, 16/16 vitest cases on |
|
Tip For best results, initiate chat on the files or code changes.
🐇 ᵔᴥᵔ |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/p`.$token.tsx:
- Line 127: The code uses playlist.name.trim().charAt(0) which breaks
multi-codepoint graphemes (e.g., emojis); change the extraction to use an
iteration-by-codepoint approach (e.g., Array.from(...) or the spread
[...playlist.name.trim()]) to get the true first grapheme, then fall back to '♪'
and apply toUpperCase() as needed; update the expression that sets the initial
(variable initial) to use this safe-first-character logic and guard for empty
names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f6870d60-6c0a-4c5c-ae75-b17f7dafa852
📒 Files selected for processing (3)
src/routes/p.$token.tsxsrc/server-fns/share.test.tssrc/server-fns/share.ts
CodeRabbit caught that `charAt(0)` on a name starting with an emoji (or any supplementary-plane code point) returns a lone surrogate, which renders as the replacement character. Swap to `Array.from(str)[0]` so the spread iterates by code point — a name like "🎵 Mix" now overlays "🎵" instead of a broken half-surrogate. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai good catch, fixed in 3ee20f9. Switched to |
|
Tip For best results, initiate chat on the files or code changes.
🐇 ᵔᴥᵔ |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/p.$token.tsx (1)
27-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFixer un vrai 404 côté SSR quand
kind: 'not_found'Le loader rend
NotFoundPanelvia une union{ kind: 'not_found' }, mais ne déclenche ninotFound()nisetResponseStatus(404). Du coup, le SSR peut servir un statut HTTP non-404 (soft-404) malgré le contenu “playlist introuvable”.🔧 Piste de correction
+import { setResponseStatus } from '`@tanstack/react-start/server`' + export const Route = createFileRoute('/p/$token')({ loader: async ({ params }): Promise<PublicPlaylistResult> => { - return getPublicPlaylist({ data: params.token }) + const result = await getPublicPlaylist({ data: params.token }) + if (result.kind === 'not_found') setResponseStatus(404) + return result }, head: ({ loaderData }) => { if (!loaderData || loaderData.kind === 'not_found') { return { meta: [{ title: 'Playlist not found · WaveFlow' }], } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/p`.$token.tsx around lines 27 - 35, The loader currently returns getPublicPlaylist(...) directly and when the result is { kind: 'not_found' } the route renders a NotFound UI but does not set an HTTP 404; update the loader (the async loader function that calls getPublicPlaylist) to inspect the result, and if result.kind === 'not_found' call the framework's server 404 helper (e.g., notFound() or setResponseStatus(404)) before returning so SSR emits a real 404 status; keep normal return for other kinds so head and UI logic (head, NotFoundPanel) work unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/routes/p`.$token.tsx:
- Around line 27-35: The loader currently returns getPublicPlaylist(...)
directly and when the result is { kind: 'not_found' } the route renders a
NotFound UI but does not set an HTTP 404; update the loader (the async loader
function that calls getPublicPlaylist) to inspect the result, and if result.kind
=== 'not_found' call the framework's server 404 helper (e.g., notFound() or
setResponseStatus(404)) before returning so SSR emits a real 404 status; keep
normal return for other kinds so head and UI logic (head, NotFoundPanel) work
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 550a6ff8-d6e8-4dae-813b-ba967da61925
📒 Files selected for processing (1)
src/routes/p.$token.tsx
CodeRabbit caught that the route was rendering NotFoundPanel
with HTTP 200 — a soft-404 that confuses crawlers and link
unfurlers.
The loader now calls setResponseStatus(404) from
@tanstack/react-start/server when the result is { kind:
'not_found' }, guarded on typeof window === 'undefined' since
the h3 event lives in AsyncLocalStorage on the server only and
client-side navigation has nothing to set.
UI rendering is unchanged — NotFoundPanel still renders, just
with the right status code on the wire.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai right call, fixed in fe5a6ea. The loader now calls Picked |
|
Tip For best results, initiate chat on the files or code changes.
✔ 🐇 ᵔᴥᵔ |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/p`.$token.tsx:
- Line 2: Le build échoue car setResponseStatus est importé statiquement dans la
route; déplace la logique de 404 dans une Server Function créée avec
createServerFn (par ex. serverCheckToken or handleTokenNotFound) qui importe et
utilise setResponseStatus côté serveur, puis supprime l'import de
setResponseStatus du fichier de route et remplace la logique dans loader par un
simple appel/await à cette server-fn (p. ex. await
serverCheckToken(params.token)); en résumé: remove the top-level import of
setResponseStatus, implement createServerFn(...) that performs the 404 via
setResponseStatus, export it, and have loader only call that exported server
function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 085741de-17a8-4e9f-b328-74ee65baa13a
📒 Files selected for processing (1)
src/routes/p.$token.tsx
The previous attempt put setResponseStatus(404) in the route loader, which broke the CI build: TanStack Start's import protection denies @tanstack/react-start/server imports from route files (routes are bundled for the client too). Move the status call into getPublicPlaylist's handler instead. Server-fn handler bodies are server-only by construction — createServerFn splits the file so the client only sees the RPC stub — so the import is legitimate there and the build succeeds. UI behaviour and tests are unchanged. The route loader stays isomorphic; the 404 emission rides along with the result the server-fn returns. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
CI build was failing on fe5a6ea — TanStack Start's import-protection denies Fixed in 3ec4ad4 by moving |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server-fns/share.ts (1)
82-109:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRisque de navigation client cassée avec
setResponseStatus(404)dansgetPublicPlaylist(RPC)
src/server-fns/share.tsmetsetResponseStatus(404)puis retourne{ kind: 'not_found' }, maissrc/routes/p.$token.tsxappelle ce server-fn dans unloadersans try/catch et afficheNotFoundPaneluniquement siloaderData.kind === 'not_found'. Or le client-RPC TanStack traite généralement les réponses HTTP non-OK comme des erreurs → leloaderpourrait ne jamais recevoir{ kind: 'not_found' }et tomber sur l’error boundary.Utiliser le signal TanStack idiomatique
notFound()(et gérer côté client avecisNotFound(err)/ou vianotFoundComponent) au lieu de piloter le contrôle de flux viasetResponseStatus(404)dans un server-fn RPC.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server-fns/share.ts` around lines 82 - 109, The handler currently calls setResponseStatus(404) and returns { kind: 'not_found' }, which causes the TanStack RPC client to treat the response as an HTTP error; instead, replace those control-flow returns with the TanStack "notFound" signal: when isWellShapedToken(token) is false or when waveflowFetchPublic throws a WaveflowServerError with status===404, call/throw notFound() (import it from your TanStack server helpers) rather than using setResponseStatus or returning { kind: 'not_found' }; keep the existing catch logic for other errors and leave waveflowFetchPublic, isWellShapedToken and WaveflowServerError checks intact, and ensure the route loader handles notFound via isNotFound(err) / notFoundComponent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/server-fns/share.ts`:
- Around line 82-109: The handler currently calls setResponseStatus(404) and
returns { kind: 'not_found' }, which causes the TanStack RPC client to treat the
response as an HTTP error; instead, replace those control-flow returns with the
TanStack "notFound" signal: when isWellShapedToken(token) is false or when
waveflowFetchPublic throws a WaveflowServerError with status===404, call/throw
notFound() (import it from your TanStack server helpers) rather than using
setResponseStatus or returning { kind: 'not_found' }; keep the existing catch
logic for other errors and leave waveflowFetchPublic, isWellShapedToken and
WaveflowServerError checks intact, and ensure the route loader handles notFound
via isNotFound(err) / notFoundComponent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b2f2a803-e960-4c42-b2ee-c39d25cc9e86
📒 Files selected for processing (2)
src/routes/p.$token.tsxsrc/server-fns/share.ts
… hack CodeRabbit caught that emitting a 404 status from the server-fn handler breaks client-side navigation: TanStack's RPC client treats any non-OK HTTP response as a thrown Error (serverFnFetcher.ts:188), so the loader would never see the discriminated-union not_found variant and the route would fall into the framework's error boundary. Switch to the idiomatic signal: throw notFound() in the handler for both malformed-token and upstream-404 cases. TanStack serialises notFound through the RPC layer as a recognised payload, the client re-throws it as a NotFoundError, and the router routes to the route's notFoundComponent with a proper SSR 404 status. PublicPlaylistResult drops the 'not_found' variant since the 404 case now propagates out-of-band. The route gains a notFoundComponent and its head() defaults to the not-found title when loaderData is absent. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai sharp catch, fully refactored in 07fdc7c. Verified the concern: Refactor:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/p.$token.tsx (1)
33-73: 🧹 Nitpick | 🔵 TrivialPour l'unfurling social, pense à
og:url(etog:imageplus tard).Les branches
head()sont cohérentes avec les panneaux rendus, rien à redire côté logique. Note opérationnelle uniquement : sansog:urlcanonique, certains scrapers (Slack/Discord/Facebook) génèrent un aperçu moins fiable.og:imagereste légitimement différé jusqu'au pipeline d'artwork public, comme indiqué dans l'objectif du PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/p`.$token.tsx around lines 33 - 73, Add an Open Graph canonical URL to the social metadata in head(): when loaderData is present and loaderData.kind === 'playlist' (i.e., the branch that destructures const { playlist } = loaderData), include an { property: 'og:url', content: <canonical playlist URL> } meta entry (construct the canonical URL from the playlist token/ID or the same route URL used elsewhere), so scrapers like Slack/Discord/Facebook get a reliable unfurl; keep og:image deferred as noted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/routes/p`.$token.tsx:
- Around line 33-73: Add an Open Graph canonical URL to the social metadata in
head(): when loaderData is present and loaderData.kind === 'playlist' (i.e., the
branch that destructures const { playlist } = loaderData), include an {
property: 'og:url', content: <canonical playlist URL> } meta entry (construct
the canonical URL from the playlist token/ID or the same route URL used
elsewhere), so scrapers like Slack/Discord/Facebook get a reliable unfurl; keep
og:image deferred as noted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5fa983b7-7aec-4c5e-85a6-4e99dade5502
📒 Files selected for processing (2)
src/routes/p.$token.tsxsrc/server-fns/share.ts
|
Tip For best results, initiate chat on the files or code changes.
Le diagnostic sur La solution finale est propre sur tous les plans :
66/66 + build : ✔ 🐇 ᵔᴥᵔ ✏️ Learnings added
🧠 Learnings used |
@coderabbitai crée une issue pour sa stp |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: gh issue create \
--repo InstaZDLL/waveflow-web \
--title "feat(share): ajouter og:url canonique dans head() de la route /p/\$token" \
--assignee InstaZDLL \
--body "## Contexte
La route \`/p/\$token\` (introduite dans la PR `#20`) génère déjà des balises Open Graph (\`og:type\`, \`og:title\`, \`og:description\`, \`og:site_name\`) et Twitter Card dans \`head()\`, mais ne définit pas \`og:url\`.
## Problème
Sans balise \`og:url\` canonique, certains scrapers de partage social (Slack, Discord, Facebook/Meta) peuvent générer un aperçu moins fiable ou utiliser l'URL de la page de référencement plutôt que l'URL canonique de la playlist.
## Action attendue
Dans \`src/routes/p.\$token.tsx\`, dans la branche \`head()\` correspondant à \`loaderData.kind === 'ok'\`, ajouter une entrée meta :
\`\`\`ts
{ property: 'og:url', content: \`https://<domaine>/p/\${token}\` }
\`\`\`
L'URL canonique peut être construite à partir du token de route (disponible via \`params.token\` dans le loader) ou d'une constante de domaine de base déjà utilisée dans le projet.
## Hors périmètre (différé)
- \`og:image\` / \`twitter:card: summary_large_image\` : différés jusqu'à ce que le pipeline d'artwork public soit disponible (voir PR `#20`).
## Références
- PR : `#20`
- Commentaire de review : https://github.com/InstaZDLL/waveflow-web/pull/20
- Demandé par `@InstaZDLL`
"Length of output: 1467
🐇 ᵔᴥᵔ 🧠 Learnings used |
1 participant
Summary
Phase 1.g.2 — the web side of playlist sharing. Renders an SSR public preview at
/p/$token, consuming the unauthenticated GET endpoint that ships in waveflow-server PR #25.The page shows playlist metadata + cover preview + an empty tracklist for now (server-side
playlist_trackmaterialisation is a separate sub-phase). What matters for the social-share use case is already there: real<title>,<meta name=\"description\">, Open Graph (og:type=music.playlist,og:title,og:description,og:site_name) and Twitter Card meta tags emitted from the route'shead()against the loader data.What lands
src/server-fns/share.ts—getPublicPlaylistserver-fn with strict token-shape validator (32 alphanumerics, rejects path-injection chars). Returns a discriminated union so the loader distinguishes 404 from transient errors.src/lib/server/waveflow-server.ts—waveflowFetchPublicanonymous fetch helper. SameWaveflowServerErrorshape as the authed variant.src/routes/p.$token.tsx— route withloader,head()for OG/Twitter, distinctok/not_found/errorpanels.src/server-fns/share.test.ts— 15 vitest cases pinning the predicate's accept/reject matrix.Notes
summary(notsummary_large_image) until the server-side artwork pipeline exposes the cover_hash as a public URL._authedparent layout NOT applied — the token is the auth.Depends on
getPublicPlaylistreturns{ kind: 'error' }for every token.Summary by CodeRabbit
New Features
Server
Tests