UDM Request Page & Cloudflare Turnstile#1465
Draft
joslane wants to merge 30 commits intosillsdev:developfrom
Draft
UDM Request Page & Cloudflare Turnstile#1465joslane wants to merge 30 commits intosillsdev:developfrom
joslane wants to merge 30 commits intosillsdev:developfrom
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a User Data Management (UDM) feature: new unauthenticated product pages (manage/about), server loaders that fetch manifests and per-locale LDML, a POST API for deletion requests with Cloudflare Turnstile verification, many new udm_* localization keys, global Turnstile types, and multiple non-functional formatting/whitespace edits across repo files. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant AppServer as Server
participant ArtifactStorage as "Artifact\nStorage/Hosting"
participant TurnstileAPI as "Cloudflare\nTurnstile"
participant DBQueue as "DB / Queue (future)"
User->>Browser: Open product UDM page
Browser->>AppServer: GET product layout (locale, manifest)
AppServer->>ArtifactStorage: Fetch manifest & locale assets
ArtifactStorage-->>AppServer: Return manifest and assets
AppServer-->>Browser: Render page with localized strings & Turnstile widget
Browser->>TurnstileAPI: Request client token
TurnstileAPI-->>Browser: Return token
Browser->>AppServer: POST /api/delete-request { email, deletionType, token }
AppServer->>TurnstileAPI: Verify token with secret + client IP
TurnstileAPI-->>AppServer: Verification result
alt verification success
AppServer->>DBQueue: Enqueue/store deletion request (scaffold)
DBQueue-->>AppServer: ack
AppServer-->>Browser: 200 { success: true }
else verification failed
AppServer-->>Browser: 400/422 error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 86: Update the README sentence that currently states the `db` and
`valkey` Docker images need to be running to say instead that the `db` and
`valkey` Docker containers need to be running; locate the line referencing `db`
and `valkey` (the runtime prerequisite sentence) and replace the word "images"
with "containers" to avoid confusion for new contributors.
In `@src/lib/locales/es-419.json`:
- Around line 578-605: The Spanish UDM strings (keys like udm_manage_title,
udm_deletion_intro_1, udm_deletion_intro_2, udm_email_label, udm_scope_warning,
udm_verification_label, udm_send_code, udm_footer_text, udm_contact_support,
udm_show_more, udm_back_manage, etc.) need proper Spanish diacritics and
punctuation; update values to use accents (eliminación, verificación, sesión,
límite/retención as appropriate), ensure question marks are paired and correctly
placed (e.g., "¿Necesitas ayuda?"), and fix capitalization/spacing so the
localized copy reads naturally before release.
In `@src/lib/locales/fr-FR.json`:
- Around line 577-604: Several French UDM strings (e.g., udm_manage_title,
udm_manage_subtitle, udm_about_button, udm_deletion_request_title,
udm_deletion_intro_1, udm_deletion_intro_2, udm_email_placeholder,
udm_email_help, udm_scope_label, udm_scope_data_title, udm_scope_data_desc,
udm_scope_account_title, udm_scope_account_desc, udm_scope_warning,
udm_items_removed_title, udm_item_bookmarks, udm_item_notes,
udm_item_highlights, udm_item_reading_progress, udm_verification_label,
udm_captcha_placeholder, udm_send_code, udm_footer_text, udm_contact_support,
udm_about_title, udm_show_more, udm_back_manage) are missing proper French
accents and typography; update each value to use correct accents and punctuation
(e.g., "Gérer", "données", "À propos", "Saisissez l'adresse e‑mail associée",
"votre connexion reste active", use non‑breaking spaces before colons/» if
applicable) and ensure consistent capitalization and French contractions (e.g.,
"À propos de cette application", "vous@exemple.com", "E‑mail" or "Adresse
e-mail" as per project convention) so the strings read naturally in French.
In `@src/routes/`(unauthenticated)/+layout.server.ts:
- Around line 19-22: The code block using existsSync/readFile/JSON.parse can
throw and currently aborts layout load; wrap the read+parse in a try/catch
around the readFile and JSON.parse (referring to filePath, readFile, JSON.parse,
and ret) and on any error log a diagnostic message (or use the app logger) and
leave ret as the safe default (e.g., an empty Entries<L10NKeys,
Entries<string,string>> or previous value) instead of rethrowing; ensure the
catch covers both IO and JSON errors so load continues gracefully.
In `@src/routes/`(unauthenticated)/+layout.svelte:
- Around line 13-15: The code mutates the module-level l10nMap inside a $effect
(using createl10nMapFromEntries) which causes SSR race conditions; stop mutating
the exported l10nMap from +layout.svelte and instead create the localized map
per-request and pass it via props or Svelte context: compute const localL10n =
createl10nMapFromEntries(data.localizedNames) synchronously (outside $effect) in
the component, then either pass localL10n down as a prop to children or call
setContext('l10n', localL10n) so consumers read request-local state; remove the
assignment to the module-level l10nMap and any $effect that mutates it.
In `@src/routes/`(unauthenticated)/api/delete-request/+server.ts:
- Around line 78-82: The console.log call that prints raw PII (variables email
and ip) should be changed to avoid logging raw identifiers: update the logging
in the delete-request handler so it only records non-PII (e.g., deletionType)
and either masks or hashes sensitive values before logging (e.g., hashEmail =
hash(email) or redact IP to a subnet), and ensure you use the existing logging
utility instead of console.log if one exists (replace the console.log('Delete
request received', { email, deletionType, ip }) with a call that logs
deletionType and maskedEmail/maskedIp or hashedEmail/hashedIp).
- Around line 50-60: The Turnstile verification fetch (the call that sets
verifyResponse) needs an AbortController-based timeout (e.g., 5s) so the handler
can't hang; create an AbortController, start a setTimeout that calls
controller.abort() after 5000ms, pass controller.signal into the fetch options
(the same fetch that posts secret: env.TURNSTILE_SECRET_KEY, response:
turnstileToken, remoteip: ip), and ensure you clear the timeout (clearTimeout)
in a finally block so it doesn't leak; also handle the abort case (catch
AbortError or check for error.name === 'AbortError') and return an appropriate
error response instead of waiting indefinitely.
In `@src/routes/`(unauthenticated)/products/[id]/file/+layout.server.ts:
- Around line 101-110: Add an AbortController-based timeout wrapper (e.g., 5s)
around network fetches and use it from fetchTextFile and the manifest fetch:
implement a helper (e.g., fetchWithTimeout(url, options, ms=5000)) that creates
an AbortController, sets a timer to call controller.abort() after the timeout,
passes controller.signal into fetch, clears the timer on success, and throws or
returns an empty result on abort; then replace the raw fetch(...) calls inside
fetchTextFile (which uses findFilePath/baseUrl) and the manifest fetch call with
fetchWithTimeout so slow storage endpoints cannot block rendering.
In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte:
- Line 153: Replace the hardcoded TURNSTILE_SITE_KEY string with an
environment-backed public variable: import PUBLIC_TURNSTILE_SITE_KEY from
SvelteKit's public env (e.g. via "import { PUBLIC_TURNSTILE_SITE_KEY } from
'$env/static/public'") and assign that to the TURNSTILE_SITE_KEY constant used
in this component (and replace the other occurrences of the literal key around
the component). Ensure the component uses the imported PUBLIC_TURNSTILE_SITE_KEY
for rendering/initialization so the frontend uses the real site key that matches
the server-side TURNSTILE_SECRET_KEY configuration.
- Around line 343-349: Replace the manual click-handling radio state with
Svelte's bind:group: for both radio inputs that control deletionType, add
value="data" on the first radio and value="account" on the second, remove the
checked={deletionType === '...'} and the onclick={() => (deletionType = '...')}
handlers, and instead use bind:group={deletionType} on each input so the
deletionType variable is updated via Svelte binding.
In `@src/routes/`(unauthenticated)/products/[id]/file/about/+page.svelte:
- Line 23: Replace the hardcoded English alt="App icon" in the Svelte page with
a localized message lookup (use the same i18n helper used on the page — e.g.,
$t, getMessage, or the page's load-provided `locale` function) and use the key
udm_app_icon_alt; then add the udm_app_icon_alt entry to all locale message
catalogs with appropriate translations so the alt text is localized for each
supported language. Ensure you reference the alt attribute in the <img> element
inside src/routes/(unauthenticated)/products/[id]/file/about/+page.svelte and
add the matching udm_app_icon_alt key to each locale JSON/TS message file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4e454d3-1b22-44ba-967f-e84834ec1727
📒 Files selected for processing (19)
.codeclimate.yml.vscode/settings.jsonREADME.mddeployment/ci/docker-compose.ymldeployment/development/docker-compose.ymleslint-rules/tsconfig.jsonrenovate.jsonsrc/app.csssrc/lib/locales/en-US.jsonsrc/lib/locales/es-419.jsonsrc/lib/locales/fr-FR.jsonsrc/lib/otel/development_config.ymlsrc/lib/otel/production_config.ymlsrc/routes/(unauthenticated)/+layout.server.tssrc/routes/(unauthenticated)/+layout.sveltesrc/routes/(unauthenticated)/api/delete-request/+server.tssrc/routes/(unauthenticated)/products/[id]/file/+layout.server.tssrc/routes/(unauthenticated)/products/[id]/file/+page.sveltesrc/routes/(unauthenticated)/products/[id]/file/about/+page.svelte
README.md
Outdated
| ## Running in the Host OS | ||
|
|
||
| This is important for local development. You still need the `db` and `valkey` docker images running. | ||
| This is important for local development. You still need the `db` and `valkey` docker images running. |
Contributor
There was a problem hiding this comment.
Use “containers” instead of “images” in the runtime prerequisite sentence.
Line 86 says db and valkey Docker images need to be running; technically, running units are containers. This can confuse new contributors.
✏️ Proposed doc fix
-This is important for local development. You still need the `db` and `valkey` docker images running.
+This is important for local development. You still need the `db` and `valkey` Docker containers running.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| This is important for local development. You still need the `db` and `valkey` docker images running. | |
| This is important for local development. You still need the `db` and `valkey` Docker containers running. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 86, Update the README sentence that currently states the
`db` and `valkey` Docker images need to be running to say instead that the `db`
and `valkey` Docker containers need to be running; locate the line referencing
`db` and `valkey` (the runtime prerequisite sentence) and replace the word
"images" with "containers" to avoid confusion for new contributors.
Comment on lines
+578
to
+605
| "udm_manage_title": "Administrar mis datos", | ||
| "udm_manage_subtitle": "Solicita la eliminacion de la cuenta o de los datos de esta app.", | ||
| "udm_about_button": "Acerca de esta app", | ||
| "udm_deletion_request_title": "Solicitud de eliminacion", | ||
| "udm_deletion_intro_1": "Ingresa el correo asociado a tu cuenta para solicitar la eliminacion de datos. Enviaremos un codigo de verificacion de un solo uso para confirmar tu identidad.", | ||
| "udm_deletion_intro_2": "Una vez confirmada, tu solicitud se procesara en un plazo de 30 dias, de acuerdo con nuestras obligaciones de retencion de datos. Las eliminaciones son permanentes y no se pueden deshacer. Es posible que conservemos cierta informacion cuando la ley o el cumplimiento normativo lo exijan.", | ||
| "udm_email_label": "Correo", | ||
| "udm_email_placeholder": "tu@ejemplo.com", | ||
| "udm_email_help": "Usa el correo asociado a tu cuenta.", | ||
| "udm_scope_label": "Alcance de la eliminacion", | ||
| "udm_scope_data_title": "Eliminar mis datos", | ||
| "udm_scope_data_desc": "Tu inicio de sesion seguira activo, pero tu contenido personal se eliminara de forma permanente.", | ||
| "udm_scope_account_title": "Eliminar mi cuenta y todos los datos asociados", | ||
| "udm_scope_account_desc": "Esto eliminara permanentemente tu inicio de sesion y tu contenido guardado.", | ||
| "udm_scope_warning": "Advertencia: Las eliminaciones son permanentes y no se pueden deshacer. Algunos datos pueden conservarse por razones legales o de cumplimiento.", | ||
| "udm_items_removed_title": "Elementos que se eliminaran", | ||
| "udm_item_bookmarks": "Marcadores", | ||
| "udm_item_notes": "Notas", | ||
| "udm_item_highlights": "Resaltados", | ||
| "udm_item_reading_progress": "Progreso del plan de lectura", | ||
| "udm_verification_label": "Verificacion", | ||
| "udm_captcha_placeholder": "Marcador de posicion del captcha (Turnstile)", | ||
| "udm_send_code": "Enviar codigo de verificacion", | ||
| "udm_footer_text": "Esta pagina se proporciona para {appName} en Google Play. Necesitas ayuda? ", | ||
| "udm_contact_support": "Contactar soporte", | ||
| "udm_about_title": "Acerca de esta app", | ||
| "udm_show_more": "Mostrar mas", | ||
| "udm_back_manage": "Volver a Administrar mis datos", |
Contributor
There was a problem hiding this comment.
Spanish UDM copy should be finalized before release.
Lines 578-605 include multiple new strings without expected accents/punctuation (e.g., “eliminacion”, “verificacion”, “Necesitas ayuda?”), which reduces localization quality.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/locales/es-419.json` around lines 578 - 605, The Spanish UDM strings
(keys like udm_manage_title, udm_deletion_intro_1, udm_deletion_intro_2,
udm_email_label, udm_scope_warning, udm_verification_label, udm_send_code,
udm_footer_text, udm_contact_support, udm_show_more, udm_back_manage, etc.) need
proper Spanish diacritics and punctuation; update values to use accents
(eliminación, verificación, sesión, límite/retención as appropriate), ensure
question marks are paired and correctly placed (e.g., "¿Necesitas ayuda?"), and
fix capitalization/spacing so the localized copy reads naturally before release.
src/lib/locales/fr-FR.json
Outdated
Comment on lines
+577
to
+604
| "udm_manage_title": "Gerer mes donnees", | ||
| "udm_manage_subtitle": "Demandez la suppression du compte ou des donnees pour cette application.", | ||
| "udm_about_button": "A propos de cette application", | ||
| "udm_deletion_request_title": "Demande de suppression", | ||
| "udm_deletion_intro_1": "Saisissez l'adresse e-mail associee a votre compte pour demander la suppression des donnees. Nous enverrons un code de verification a usage unique pour confirmer votre identite.", | ||
| "udm_deletion_intro_2": "Une fois confirmee, votre demande sera traitee sous 30 jours, conformement a nos obligations de conservation des donnees. Les suppressions sont definitives et ne peuvent pas etre annulees. Certaines informations peuvent etre conservees lorsque la loi ou des obligations de conformite l'exigent.", | ||
| "udm_email_label": "E-mail", | ||
| "udm_email_placeholder": "vous@exemple.com", | ||
| "udm_email_help": "Utilisez l'adresse e-mail associee a votre compte.", | ||
| "udm_scope_label": "Portee de la suppression", | ||
| "udm_scope_data_title": "Supprimer mes donnees", | ||
| "udm_scope_data_desc": "Votre connexion reste active, mais votre contenu personnel sera supprime de facon definitive.", | ||
| "udm_scope_account_title": "Supprimer mon compte et toutes les donnees associees", | ||
| "udm_scope_account_desc": "Cela supprimera definitivement votre connexion et le contenu enregistre.", | ||
| "udm_scope_warning": "Attention: les suppressions sont definitives et ne peuvent pas etre annulees. Certaines donnees peuvent etre conservees pour des raisons juridiques ou de conformite.", | ||
| "udm_items_removed_title": "Elements a supprimer", | ||
| "udm_item_bookmarks": "Signets", | ||
| "udm_item_notes": "Notes", | ||
| "udm_item_highlights": "Surlignages", | ||
| "udm_item_reading_progress": "Progression du plan de lecture", | ||
| "udm_verification_label": "Verification", | ||
| "udm_captcha_placeholder": "Emplacement du widget captcha (Turnstile)", | ||
| "udm_send_code": "Envoyer le code de verification", | ||
| "udm_footer_text": "Cette page est fournie pour {appName} sur Google Play. Besoin d'aide ? ", | ||
| "udm_contact_support": "Contacter le support", | ||
| "udm_about_title": "A propos de cette application", | ||
| "udm_show_more": "Afficher plus", | ||
| "udm_back_manage": "Retour a Gerer mes donnees", |
Contributor
There was a problem hiding this comment.
French UDM copy needs a quick native-text QA pass.
Lines 577-604 contain several new strings missing common French accents/typography (for example, “Gerer”, “donnees”, “A propos”). This will read unpolished in production.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/locales/fr-FR.json` around lines 577 - 604, Several French UDM
strings (e.g., udm_manage_title, udm_manage_subtitle, udm_about_button,
udm_deletion_request_title, udm_deletion_intro_1, udm_deletion_intro_2,
udm_email_placeholder, udm_email_help, udm_scope_label, udm_scope_data_title,
udm_scope_data_desc, udm_scope_account_title, udm_scope_account_desc,
udm_scope_warning, udm_items_removed_title, udm_item_bookmarks, udm_item_notes,
udm_item_highlights, udm_item_reading_progress, udm_verification_label,
udm_captcha_placeholder, udm_send_code, udm_footer_text, udm_contact_support,
udm_about_title, udm_show_more, udm_back_manage) are missing proper French
accents and typography; update each value to use correct accents and punctuation
(e.g., "Gérer", "données", "À propos", "Saisissez l'adresse e‑mail associée",
"votre connexion reste active", use non‑breaking spaces before colons/» if
applicable) and ensure consistent capitalization and French contractions (e.g.,
"À propos de cette application", "vous@exemple.com", "E‑mail" or "Adresse
e-mail" as per project convention) so the strings read naturally in French.
Comment on lines
+19
to
+22
| if (existsSync(filePath)) { | ||
| const file = (await readFile(filePath)).toString(); | ||
| ret = JSON.parse(file) as Entries<L10NKeys, Entries<string, string>>; | ||
| } |
Contributor
There was a problem hiding this comment.
Handle missing/malformed locale files without crashing load.
Line 20/21 can throw (readFile/JSON.parse) and currently aborts the whole layout load.
🔧 Suggested fix
- let ret = null;
- if (existsSync(filePath)) {
- const file = (await readFile(filePath)).toString();
- ret = JSON.parse(file) as Entries<L10NKeys, Entries<string, string>>;
- }
+ let ret: Entries<L10NKeys, Entries<string, string>> | null = null;
+ try {
+ const file = await readFile(filePath, 'utf8');
+ ret = JSON.parse(file) as Entries<L10NKeys, Entries<string, string>>;
+ } catch {
+ ret = null;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(unauthenticated)/+layout.server.ts around lines 19 - 22, The
code block using existsSync/readFile/JSON.parse can throw and currently aborts
layout load; wrap the read+parse in a try/catch around the readFile and
JSON.parse (referring to filePath, readFile, JSON.parse, and ret) and on any
error log a diagnostic message (or use the app logger) and leave ret as the safe
default (e.g., an empty Entries<L10NKeys, Entries<string,string>> or previous
value) instead of rethrowing; ensure the catch covers both IO and JSON errors so
load continues gracefully.
Comment on lines
+13
to
+15
| $effect(() => { | ||
| l10nMap.value = createl10nMapFromEntries(data.localizedNames); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find module-level l10n state declaration:"
rg -n -C3 'export const l10nMap|\\$state\\(' src/lib/locales.svelte.ts
echo
echo "Find where l10nMap is assigned and whether it's done inside \$effect:"
rg -n -C4 'l10nMap\\.value\\s*=|\\$effect\\(' srcRepository: sillsdev/appbuilder-portal
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching for l10nMap in src/lib/locales.svelte.ts ==="
if [ -f "src/lib/locales.svelte.ts" ]; then
cat -n src/lib/locales.svelte.ts
else
echo "File not found, searching for locales files:"
fd -e ts -e svelte "locale" src/lib
fi
echo
echo "=== Searching for l10nMap usage ==="
rg -n "l10nMap" src
echo
echo "=== Checking the target file structure ==="
if [ -f "src/routes/(unauthenticated)/+layout.svelte" ]; then
cat -n "src/routes/(unauthenticated)/+layout.svelte"
else
echo "Looking for +layout.svelte files:"
fd -e svelte "+layout.svelte" src/routes
fiRepository: sillsdev/appbuilder-portal
Length of output: 5969
Initialize localization state outside $effect for SSR safety.
Module-level l10nMap exported from src/lib/locales.svelte.ts is mutated inside $effect in layout files. This creates a race condition under concurrent SSR requests—multiple requests overwrite the shared module state. Use request-local flow instead: pass localization data through component props or context rather than mutating module-level state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(unauthenticated)/+layout.svelte around lines 13 - 15, The code
mutates the module-level l10nMap inside a $effect (using
createl10nMapFromEntries) which causes SSR race conditions; stop mutating the
exported l10nMap from +layout.svelte and instead create the localized map
per-request and pass it via props or Svelte context: compute const localL10n =
createl10nMapFromEntries(data.localizedNames) synchronously (outside $effect) in
the component, then either pass localL10n down as a prop to children or call
setContext('l10n', localL10n) so consumers read request-local state; remove the
assignment to the module-level l10nMap and any $effect that mutates it.
Comment on lines
+78
to
+82
| console.log('Delete request received', { | ||
| email, | ||
| deletionType, | ||
| ip | ||
| }); |
Contributor
There was a problem hiding this comment.
Avoid logging raw email and client IP.
Line 78 logs direct PII (email, ip). This increases privacy/compliance exposure in logs.
🔧 Suggested fix
console.log('Delete request received', {
- email,
deletionType,
- ip
+ hasEmail: Boolean(email)
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| console.log('Delete request received', { | |
| email, | |
| deletionType, | |
| ip | |
| }); | |
| console.log('Delete request received', { | |
| deletionType, | |
| hasEmail: Boolean(email) | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(unauthenticated)/api/delete-request/+server.ts around lines 78 -
82, The console.log call that prints raw PII (variables email and ip) should be
changed to avoid logging raw identifiers: update the logging in the
delete-request handler so it only records non-PII (e.g., deletionType) and
either masks or hashes sensitive values before logging (e.g., hashEmail =
hash(email) or redact IP to a subnet), and ensure you use the existing logging
utility instead of console.log if one exists (replace the console.log('Delete
request received', { email, deletionType, ip }) with a call that logs
deletionType and maskedEmail/maskedIp or hashedEmail/hashedIp).
Comment on lines
+101
to
+110
| async function fetchTextFile(baseUrl: URL, files: string[], language: string, filename: string) { | ||
| const path = findFilePath(files, language, filename) ?? `${language}/${filename}`; | ||
| const fileUrl = new URL(path, baseUrl); | ||
| try { | ||
| const res = await fetch(fileUrl); | ||
| if (!res.ok) return ''; | ||
| return (await res.text()).trim(); | ||
| } catch { | ||
| return ''; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "+layout.server.ts" | grep -i productsRepository: sillsdev/appbuilder-portal
Length of output: 366
🏁 Script executed:
cat -n src/routes/\(unauthenticated\)/products/\[id\]/file/+layout.server.ts | head -120Repository: sillsdev/appbuilder-portal
Length of output: 5097
🏁 Script executed:
cat -n src/routes/\(unauthenticated\)/products/\[id\]/file/+layout.server.ts | sed -n '100,220p'Repository: sillsdev/appbuilder-portal
Length of output: 4673
🏁 Script executed:
cat -n src/routes/\(unauthenticated\)/products/\[id\]/file/+layout.server.ts | wc -lRepository: sillsdev/appbuilder-portal
Length of output: 73
🏁 Script executed:
cat -n src/routes/\(unauthenticated\)/products/\[id\]/file/+layout.server.ts | tail -50Repository: sillsdev/appbuilder-portal
Length of output: 2147
🏁 Script executed:
rg -i "timeout|abort|signal" src/routes/ -A 2 -B 2Repository: sillsdev/appbuilder-portal
Length of output: 4019
🏁 Script executed:
fd -type f "*fetch*" | head -20Repository: sillsdev/appbuilder-portal
Length of output: 240
Add timeout guards to manifest and text file fetches.
Lines 105 and 207 perform network fetches without timeouts in the request load handler, which blocks public page rendering. A slow storage endpoint can stall the entire page. Additionally, lines 241–246 make multiple sequential calls to fetchTextFile(), compounding the risk.
Implement a fetch wrapper with an AbortController-based timeout (e.g., 5 seconds) and apply it to both the manifest fetch (line 207) and the fetchTextFile() function (line 105).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(unauthenticated)/products/[id]/file/+layout.server.ts around
lines 101 - 110, Add an AbortController-based timeout wrapper (e.g., 5s) around
network fetches and use it from fetchTextFile and the manifest fetch: implement
a helper (e.g., fetchWithTimeout(url, options, ms=5000)) that creates an
AbortController, sets a timer to call controller.abort() after the timeout,
passes controller.signal into fetch, clears the timer on success, and throws or
returns an empty result on abort; then replace the raw fetch(...) calls inside
fetchTextFile (which uses findFilePath/baseUrl) and the manifest fetch call with
fetchWithTimeout so slow storage endpoints cannot block rendering.
| let submitting = $state(false); | ||
| let errorMessage = $state<string | null>(null); | ||
|
|
||
| const TURNSTILE_SITE_KEY = 'YOUR_SITE_KEY'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/routes/\(unauthenticated\)/products/\[id\]/file && wc -l +page.svelteRepository: sillsdev/appbuilder-portal
Length of output: 86
🏁 Script executed:
cat -n src/routes/\(unauthenticated\)/products/\[id\]/file/+page.svelte | sed -n '150,160p'Repository: sillsdev/appbuilder-portal
Length of output: 443
🏁 Script executed:
cat -n src/routes/\(unauthenticated\)/products/\[id\]/file/+page.svelte | sed -n '405,415p'Repository: sillsdev/appbuilder-portal
Length of output: 523
🏁 Script executed:
# Check for environment variable definitions in the project
rg "PUBLIC_TURNSTILE_SITE_KEY|VITE_TURNSTILE" --type svelte --type ts --type jsRepository: sillsdev/appbuilder-portal
Length of output: 100
🏁 Script executed:
# Check for .env or env config files
find . -type f \( -name ".env*" -o -name "svelte.config.*" \) | head -20Repository: sillsdev/appbuilder-portal
Length of output: 88
🏁 Script executed:
# Search for environment variable imports in svelte files using rg
rg '\$env/static/public' --type-list | grep -i svelteRepository: sillsdev/appbuilder-portal
Length of output: 459
🏁 Script executed:
# Search for $env imports more broadly
rg '\$env/static' -A 2Repository: sillsdev/appbuilder-portal
Length of output: 52
🏁 Script executed:
# Check for env file patterns
find . -maxdepth 2 -type f -name "*.md" | xargs grep -l "TURNSTILE\|environment" 2>/dev/null | head -5Repository: sillsdev/appbuilder-portal
Length of output: 81
🏁 Script executed:
# Look at the imports at the beginning of the file to understand the pattern
head -30 src/routes/\(unauthenticated\)/products/\[id\]/file/+page.svelteRepository: sillsdev/appbuilder-portal
Length of output: 1607
🏁 Script executed:
# Search for $env usage in JS/TS files
rg '\$env/' -t js -t ts | head -20Repository: sillsdev/appbuilder-portal
Length of output: 437
🏁 Script executed:
# Look for any Turnstile configuration or references
rg -i 'turnstile|sitekey' | grep -v node_modules | head -20Repository: sillsdev/appbuilder-portal
Length of output: 2164
🏁 Script executed:
# Check README for setup/environment variable documentation
head -100 ./README.mdRepository: sillsdev/appbuilder-portal
Length of output: 3794
Replace placeholder Turnstile key with environment-backed config.
Line 153 uses 'YOUR_SITE_KEY', which prevents real verification tokens from being issued. The server-side validation in delete-request/+server.ts checks for TURNSTILE_SECRET_KEY, confirming Turnstile is configured; the frontend key must also be environment-backed to function in production.
🔧 Suggested fix
import { onMount } from 'svelte';
+ import { PUBLIC_TURNSTILE_SITE_KEY } from '$env/static/public';
@@
- const TURNSTILE_SITE_KEY = 'YOUR_SITE_KEY';
+ const TURNSTILE_SITE_KEY = PUBLIC_TURNSTILE_SITE_KEY;Also applies to: 408-410
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte at line 153,
Replace the hardcoded TURNSTILE_SITE_KEY string with an environment-backed
public variable: import PUBLIC_TURNSTILE_SITE_KEY from SvelteKit's public env
(e.g. via "import { PUBLIC_TURNSTILE_SITE_KEY } from '$env/static/public'") and
assign that to the TURNSTILE_SITE_KEY constant used in this component (and
replace the other occurrences of the literal key around the component). Ensure
the component uses the imported PUBLIC_TURNSTILE_SITE_KEY for
rendering/initialization so the frontend uses the real site key that matches the
server-side TURNSTILE_SECRET_KEY configuration.
Comment on lines
+343
to
+349
| <input | ||
| type="radio" | ||
| name="deletionType" | ||
| class="radio radio-primary radio-sm mt-1" | ||
| checked={deletionType === 'data'} | ||
| onclick={() => (deletionType = 'data')} | ||
| /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l "src/routes/(unauthenticated)/products/[id]/file/+page.svelte"Repository: sillsdev/appbuilder-portal
Length of output: 134
🏁 Script executed:
sed -n '340,370p' "src/routes/(unauthenticated)/products/[id]/file/+page.svelte"Repository: sillsdev/appbuilder-portal
Length of output: 1469
🏁 Script executed:
# Get broader context around the radio buttons
sed -n '320,380p' "src/routes/(unauthenticated)/products/[id]/file/+page.svelte"Repository: sillsdev/appbuilder-portal
Length of output: 2778
🏁 Script executed:
# Find the script section and deletionType variable declaration
sed -n '1,50p' "src/routes/(unauthenticated)/products/[id]/file/+page.svelte"Repository: sillsdev/appbuilder-portal
Length of output: 2225
🏁 Script executed:
# Search for deletionType declaration
rg "deletionType" "src/routes/(unauthenticated)/products/[id]/file/+page.svelte" -B 2 -A 2Repository: sillsdev/appbuilder-portal
Length of output: 1000
🏁 Script executed:
# Search for other radio button implementations in the codebase to see if bind:group is used elsewhere
rg "bind:group" --type svelteRepository: sillsdev/appbuilder-portal
Length of output: 100
🏁 Script executed:
# Also check for other uses of checked/onclick patterns in Svelte files to see if this is a consistent pattern
rg "type=\"radio\"" --type svelte -A 3 | head -40Repository: sillsdev/appbuilder-portal
Length of output: 100
🏁 Script executed:
# Search for bind:group across all files
rg "bind:group" | head -20Repository: sillsdev/appbuilder-portal
Length of output: 2655
🏁 Script executed:
# Search for radio button patterns in .svelte files
fd "\.svelte$" | xargs rg -l "type=\"radio\"" | head -10Repository: sillsdev/appbuilder-portal
Length of output: 538
Use bind:group for radio state instead of click handlers.
The two radio buttons for deletion scope (lines 343-349 and 363-368) use onclick handlers to update state, which is inconsistent with the rest of the codebase. Replace with bind:group to match Svelte conventions and ensure consistent state management across all interaction modes.
Apply the following changes to both radio inputs:
- Add
valueattribute ("data"and"account"respectively) - Replace
checked={deletionType === 'value'}withbind:group={deletionType} - Remove the
onclick={() => (deletionType = 'value')}handler
🔧 Suggested fix
<label class="label cursor-pointer items-start justify-start gap-3 p-0 group min-w-0">
<input
type="radio"
name="deletionType"
+ value="data"
class="radio radio-primary radio-sm mt-1"
- checked={deletionType === 'data'}
- onclick={() => (deletionType = 'data')}
+ bind:group={deletionType}
/> <label class="label cursor-pointer items-start justify-start gap-3 p-0 group min-w-0">
<input
type="radio"
name="deletionType"
+ value="account"
class="radio radio-primary radio-sm mt-1"
- checked={deletionType === 'account'}
- onclick={() => (deletionType = 'account')}
+ bind:group={deletionType}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <input | |
| type="radio" | |
| name="deletionType" | |
| class="radio radio-primary radio-sm mt-1" | |
| checked={deletionType === 'data'} | |
| onclick={() => (deletionType = 'data')} | |
| /> | |
| <input | |
| type="radio" | |
| name="deletionType" | |
| value="data" | |
| class="radio radio-primary radio-sm mt-1" | |
| bind:group={deletionType} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte around lines
343 - 349, Replace the manual click-handling radio state with Svelte's
bind:group: for both radio inputs that control deletionType, add value="data" on
the first radio and value="account" on the second, remove the
checked={deletionType === '...'} and the onclick={() => (deletionType = '...')}
handlers, and instead use bind:group={deletionType} on each input so the
deletionType variable is updated via Svelte binding.
| <div class="px-5 pb-4 flex items-start gap-4"> | ||
| <img | ||
| src={iconSrc} | ||
| alt="App icon" |
Contributor
There was a problem hiding this comment.
Localize the image alt text.
Line 23 is English-only while the rest of this page is localized; this creates a minor i18n/accessibility mismatch.
Suggested change
- alt="App icon"
+ alt={m.udm_app_icon_alt()}Also add udm_app_icon_alt to the locale message catalogs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(unauthenticated)/products/[id]/file/about/+page.svelte at line
23, Replace the hardcoded English alt="App icon" in the Svelte page with a
localized message lookup (use the same i18n helper used on the page — e.g., $t,
getMessage, or the page's load-provided `locale` function) and use the key
udm_app_icon_alt; then add the udm_app_icon_alt entry to all locale message
catalogs with appropriate translations so the alt text is localized for each
supported language. Ensure you reference the alt attribute in the <img> element
inside src/routes/(unauthenticated)/products/[id]/file/about/+page.svelte and
add the matching udm_app_icon_alt key to each locale JSON/TS message file.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/routes/(unauthenticated)/products/[id]/file/+page.svelte (2)
20-103: Consider extracting color utilities to a shared module.These color manipulation functions (
hexToRgb,rgbToHex,getRelativeLuminance,contrastRatio,getReadableTextHex,lightenColor,hexToDaisyHSL) are generic and could be reused elsewhere in the codebase. Extracting them to$lib/utils/color.tswould improve maintainability and enable unit testing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte around lines 20 - 103, Extract the generic color helper functions (hexToRgb, rgbToHex, srgbChannelToLinear, getRelativeLuminance, contrastRatio, getReadableTextHex, lightenColor, hexToDaisyHSL) into a new shared module (e.g. $lib/utils/color.ts) as named exports, preserving their TypeScript signatures and behavior; then replace the local definitions in +page.svelte with imports of those named functions and update any references to use the imported names. Ensure the new module exports srgbChannelToLinear (even if internal) because getRelativeLuminance depends on it, add a simple unit-testable API surface, and run the app to confirm no runtime/type errors after changing the imports.
310-317: Consider adding client-side validation for the email input.The email input lacks a
requiredattribute and relies solely on server-side validation. Addingrequiredwould provide immediate feedback and prevent unnecessary API calls.🔧 Suggested fix
<input id="email" type="email" bind:value={email} placeholder={m.udm_email_placeholder()} class="input input-bordered w-full text-base sm:text-sm h-11 focus:border-primary focus:ring-1 focus:ring-primary focus:outline-none" name="email" + required />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte around lines 310 - 317, The email input lacks client-side validation; add the required attribute to the <input id="email" ... bind:value={email}> element and, in the form submit handler (e.g., the function handling the form submission / on:submit callback in this component), call the input's checkValidity() or use a simple regex to verify the email before making the API call; if invalid, prevent submission and show a validation message (use setCustomValidity/message or a local error state bound to the UI).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte:
- Around line 165-197: The submitDeleteRequest function currently only sets
errorMessage on failure and never signals success to the user; add a success
state (e.g., successMessage or isSubmitted) alongside existing
submitting/errorMessage state, set successMessage when the POST returns ok
inside submitDeleteRequest (and clear errorMessage), and render that success
state in the component template (near the existing error alert) so users get
feedback and duplicate submissions can be avoided; update any UI logic that
depends on submitting or form inputs to reflect the successful submission (e.g.,
disable the form or show a confirmation).
---
Nitpick comments:
In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte:
- Around line 20-103: Extract the generic color helper functions (hexToRgb,
rgbToHex, srgbChannelToLinear, getRelativeLuminance, contrastRatio,
getReadableTextHex, lightenColor, hexToDaisyHSL) into a new shared module (e.g.
$lib/utils/color.ts) as named exports, preserving their TypeScript signatures
and behavior; then replace the local definitions in +page.svelte with imports of
those named functions and update any references to use the imported names.
Ensure the new module exports srgbChannelToLinear (even if internal) because
getRelativeLuminance depends on it, add a simple unit-testable API surface, and
run the app to confirm no runtime/type errors after changing the imports.
- Around line 310-317: The email input lacks client-side validation; add the
required attribute to the <input id="email" ... bind:value={email}> element and,
in the form submit handler (e.g., the function handling the form submission /
on:submit callback in this component), call the input's checkValidity() or use a
simple regex to verify the email before making the API call; if invalid, prevent
submission and show a validation message (use setCustomValidity/message or a
local error state bound to the UI).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 043ba71d-f54f-4542-95c8-e00c02e69019
📒 Files selected for processing (1)
src/routes/(unauthenticated)/products/[id]/file/+page.svelte
Comment on lines
+165
to
+197
| async function submitDeleteRequest() { | ||
| submitting = true; | ||
| errorMessage = null; | ||
|
|
||
| try { | ||
| const token = turnstileToken; | ||
|
|
||
| if (!token) { | ||
| errorMessage = 'Please complete the verification.'; | ||
| submitting = false; | ||
| return; | ||
| } | ||
|
|
||
| const res = await fetch('/api/delete-request', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ | ||
| email, | ||
| deletionType, | ||
| turnstileToken: token | ||
| }) | ||
| }); | ||
|
|
||
| if (!res.ok) { | ||
| const data = await res.json().catch(() => null); | ||
| errorMessage = data?.message ?? 'Failed to submit request.'; | ||
| } | ||
| } catch { | ||
| errorMessage = 'Unexpected error.'; | ||
| } finally { | ||
| submitting = false; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Missing success state handling after form submission.
When res.ok is true, the function completes without providing user feedback. The user has no indication their request was received, which may lead to confusion or duplicate submissions.
🔧 Suggested fix
Add a success state variable and handle the successful response:
let errorMessage = $state<string | null>(null);
+ let successMessage = $state<string | null>(null);
// ...
async function submitDeleteRequest() {
submitting = true;
errorMessage = null;
+ successMessage = null;
try {
const token = turnstileToken;
if (!token) {
errorMessage = 'Please complete the verification.';
submitting = false;
return;
}
const res = await fetch('/api/delete-request', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
email,
deletionType,
turnstileToken: token
})
});
if (!res.ok) {
const data = await res.json().catch(() => null);
errorMessage = data?.message ?? 'Failed to submit request.';
+ } else {
+ successMessage = 'Your request has been submitted. Check your email for confirmation.';
+ email = '';
+ turnstileToken = null;
}
} catch {
errorMessage = 'Unexpected error.';
} finally {
submitting = false;
}
}Then render the success message in the template (near the error alert):
{`#if` successMessage}
<div class="alert alert-success text-sm">
{successMessage}
</div>
{/if}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte around lines
165 - 197, The submitDeleteRequest function currently only sets errorMessage on
failure and never signals success to the user; add a success state (e.g.,
successMessage or isSubmitted) alongside existing submitting/errorMessage state,
set successMessage when the POST returns ok inside submitDeleteRequest (and
clear errorMessage), and render that success state in the component template
(near the existing error alert) so users get feedback and duplicate submissions
can be avoided; update any UI logic that depends on submitting or form inputs to
reflect the successful submission (e.g., disable the form or show a
confirmation).
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/lib/locales/fr-FR.json (1)
580-607:⚠️ Potential issue | 🟡 MinorFrench UDM strings still need native-text QA before release.
The new copy still has obvious missing accents/typography (
Gerer,donnees,A propos,associee,Retour a), so the page will look unpolished to French users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/locales/fr-FR.json` around lines 580 - 607, The French UDM strings (e.g., udm_manage_title, udm_manage_subtitle, udm_about_button, udm_deletion_intro_1, udm_deletion_intro_2, udm_email_placeholder, udm_email_help, udm_scope_warning, udm_footer_text, udm_about_title, udm_back_manage) contain missing accents and typographic issues; update these entries to correct French accents and punctuation (e.g., "Gérer", "données", "À propos", "associée", "Retour à"), ensure consistent apostrophes/spacing (e.g., « » vs straight quotes if used elsewhere), and run a native French QA pass to verify grammar, diacritics, and style before merging.src/lib/locales/es-419.json (1)
581-608:⚠️ Potential issue | 🟡 MinorSpanish UDM copy still needs a final localization pass.
Several user-facing strings here still ship without standard Spanish accents/punctuation—for example
eliminacion,verificacion,sesion, andNecesitas ayuda?. That will read unfinished in production, especially on a public data-deletion flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/locales/es-419.json` around lines 581 - 608, The Spanish UDM strings contain missing accents and punctuation; update the JSON values for the listed keys (udm_manage_title, udm_manage_subtitle, udm_deletion_request_title, udm_deletion_intro_1, udm_deletion_intro_2, udm_email_label, udm_email_placeholder, udm_email_help, udm_scope_label, udm_scope_data_title, udm_scope_data_desc, udm_scope_account_title, udm_scope_account_desc, udm_scope_warning, udm_items_removed_title, udm_item_bookmarks, udm_item_notes, udm_item_highlights, udm_item_reading_progress, udm_verification_label, udm_captcha_placeholder, udm_send_code, udm_footer_text, udm_contact_support, udm_about_title, udm_show_more, udm_back_manage) to use correct Spanish diacritics and punctuation (e.g., eliminacion→eliminación, verificacion→verificación, sesion→sesión, pagina→página, mas→más, enviar codigo→enviar código, marcador de posicion→marcador de posición, add opening ¿ to questions like “¿Necesitas ayuda?”), then run a native Spanish localization/spellcheck pass to ensure consistency before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/lib/locales/es-419.json`:
- Around line 581-608: The Spanish UDM strings contain missing accents and
punctuation; update the JSON values for the listed keys (udm_manage_title,
udm_manage_subtitle, udm_deletion_request_title, udm_deletion_intro_1,
udm_deletion_intro_2, udm_email_label, udm_email_placeholder, udm_email_help,
udm_scope_label, udm_scope_data_title, udm_scope_data_desc,
udm_scope_account_title, udm_scope_account_desc, udm_scope_warning,
udm_items_removed_title, udm_item_bookmarks, udm_item_notes,
udm_item_highlights, udm_item_reading_progress, udm_verification_label,
udm_captcha_placeholder, udm_send_code, udm_footer_text, udm_contact_support,
udm_about_title, udm_show_more, udm_back_manage) to use correct Spanish
diacritics and punctuation (e.g., eliminacion→eliminación,
verificacion→verificación, sesion→sesión, pagina→página, mas→más, enviar
codigo→enviar código, marcador de posicion→marcador de posición, add opening ¿
to questions like “¿Necesitas ayuda?”), then run a native Spanish
localization/spellcheck pass to ensure consistency before committing.
In `@src/lib/locales/fr-FR.json`:
- Around line 580-607: The French UDM strings (e.g., udm_manage_title,
udm_manage_subtitle, udm_about_button, udm_deletion_intro_1,
udm_deletion_intro_2, udm_email_placeholder, udm_email_help, udm_scope_warning,
udm_footer_text, udm_about_title, udm_back_manage) contain missing accents and
typographic issues; update these entries to correct French accents and
punctuation (e.g., "Gérer", "données", "À propos", "associée", "Retour à"),
ensure consistent apostrophes/spacing (e.g., « » vs straight quotes if used
elsewhere), and run a native French QA pass to verify grammar, diacritics, and
style before merging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8d239a0-1f7f-42e5-ac87-d131b9b736ad
📒 Files selected for processing (3)
src/lib/locales/en-US.jsonsrc/lib/locales/es-419.jsonsrc/lib/locales/fr-FR.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/locales/en-US.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app.d.ts`:
- Around line 11-14: The Window augmentation declares handleTurnstileSuccess but
the page uses window.onTurnstileSuccess; update the declaration in the Window
interface to match the actual global callback name (replace or add
onTurnstileSuccess?: (token: string) => void) so the global callback used in
src/routes/(unauthenticated)/products/[id]/file/+page.svelte is properly typed;
reference the Window interface and the turnstile? property when making this
change.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/routes/(unauthenticated)/products/[id]/file/+page.svelte (2)
155-163: Local interface declaration shadows globalWindowtype.Declaring
interface WindowinsideonMountcreates a local type that shadows the globalWindow. This works but is unconventional. Consider using type assertion or augmenting the global type.♻️ Alternative approach
onMount(() => { - interface Window { - handleTurnstileSuccess?: (token: string) => void; - } - - (window as Window).handleTurnstileSuccess = (token: string) => { + (window as typeof window & { handleTurnstileSuccess?: (token: string) => void }).handleTurnstileSuccess = (token: string) => { turnstileToken = token; }; });Or declare the global augmentation in a
.d.tsfile if used elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte around lines 155 - 163, The local "interface Window" declared inside onMount shadows the global Window type; remove that local declaration and instead assign the callback with a proper type assertion or global augmentation: set (window as any).handleTurnstileSuccess = (token: string) => { turnstileToken = token; } or add a global declaration for handleTurnstileSuccess in a .d.ts file to extend Window for reuse; update the assignment in onMount (referencing onMount, handleTurnstileSuccess, and turnstileToken) to use one of these approaches so you no longer shadow the global Window type.
205-219: Redundant CSS variable assignments between inline style and$effect.The same CSS custom properties are set both in the inline
styleattribute (lines 242-253) and in the$effect(lines 207-217). The$effectupdatesdocument.documentElement, while the inline style scopes to the component's root div.This creates two sources of truth. If the intent is global theming, the inline styles are unnecessary; if scoped theming is intended, the
$effectondocumentElementmay cause side effects on other pages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte around lines 205 - 219, The $effect block currently sets global CSS variables via document.documentElement.style.setProperty while the component also sets the same variables in the root div's inline style, creating duplicate, conflicting sources of truth; decide whether the theme should be global or scoped and remove the redundant approach: if global theming is intended, delete the inline style on the component root and keep the $effect (keep $effect and the document.documentElement.style.setProperty calls), or if scoped theming is intended, remove the $effect and apply the variables only on the component root's inline style (remove the document.documentElement.style.setProperty usages in the $effect). Ensure references to primaryHex, primaryContentHex, primaryHSL, primaryContentHSL, lightBgHex remain used in whichever location you keep.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/routes/`(unauthenticated)/products/[id]/file/+page.svelte:
- Around line 155-163: The local "interface Window" declared inside onMount
shadows the global Window type; remove that local declaration and instead assign
the callback with a proper type assertion or global augmentation: set (window as
any).handleTurnstileSuccess = (token: string) => { turnstileToken = token; } or
add a global declaration for handleTurnstileSuccess in a .d.ts file to extend
Window for reuse; update the assignment in onMount (referencing onMount,
handleTurnstileSuccess, and turnstileToken) to use one of these approaches so
you no longer shadow the global Window type.
- Around line 205-219: The $effect block currently sets global CSS variables via
document.documentElement.style.setProperty while the component also sets the
same variables in the root div's inline style, creating duplicate, conflicting
sources of truth; decide whether the theme should be global or scoped and remove
the redundant approach: if global theming is intended, delete the inline style
on the component root and keep the $effect (keep $effect and the
document.documentElement.style.setProperty calls), or if scoped theming is
intended, remove the $effect and apply the variables only on the component
root's inline style (remove the document.documentElement.style.setProperty
usages in the $effect). Ensure references to primaryHex, primaryContentHex,
primaryHSL, primaryContentHSL, lightBgHex remain used in whichever location you
keep.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40460f46-7d85-4baa-95cf-7dd1ebd00eba
📒 Files selected for processing (3)
src/app.d.tssrc/routes/(unauthenticated)/products/[id]/file/+layout.server.tssrc/routes/(unauthenticated)/products/[id]/file/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app.d.ts
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
4 participants
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.
Summary by CodeRabbit
New Features
Style