Conversation
update: move things around.
update: db page to svelte5.
optimize: imports, singular sources of truth.
fix: height issues on mobile.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/customId.svelte (1)
46-68:⚠️ Potential issue | 🟠 MajorPrevent update-loop when
showis false.With
syncFromset, the auto-sync effect still runs while hidden and fights theshowreset effect, causingidto flip betweennulland the formatted value (recursive updates). Gate auto-sync byshowto avoid this.🔧 Proposed fix
$effect(() => { - if (syncFrom && !touchedId) { + if (!show) return; + if (syncFrom && !touchedId) { const newId = toIdFormat(syncFrom); if (id !== newId) { id = newId; } } });
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/create/+page.svelte:
- Line 123: The current nullish coalescing assignment on $createDatabaseStore.id
(using "??=") won't generate an ID when id is an empty string; change the logic
to treat empty string as missing — e.g., replace the "??=" with a check for
falsy/empty and assign ID.unique() when $createDatabaseStore.id is empty (or use
logical OR assignment "||="), so that ID.unique() is used when id === ''
(reference: $createDatabaseStore.id and ID.unique()).
- Around line 153-159: Remove the stray debug print by deleting the
console.log(error) call and implement a type guard in the catch block so you
safely extract a message for addNotification; e.g., check if error instanceof
Error then use error.message, otherwise use String(error) (or a default string),
and send that into addNotification({ type: 'error', message: ... }); optionally
replace the removed debug print with console.error(...) if runtime logging is
required.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte:
- Around line 101-102: The unconditional assignment selectedPolicyGroup =
'custom' is wiping preset checkbox selections by triggering the reactive block
that clears presets; change this so it only runs outside the backups flow (or
add a guard in the reactive block that unchecks presets). Locate the
selectedPolicyGroup assignment and either wrap it in a conditional that checks
the current tab/flow (skip when in backups) or modify the reactive block that
clears presets to early-return when the current route/tab is backups so
multi-select choices in the backups tab are preserved.
- Around line 211-221: The onMount block is mutating the shared presetPolicies
store (via presetPolicies.update) which permanently removes entries; instead
create a local derived value for rendering (e.g., visiblePresets or
displayPolicies) that reads presetPolicies and filters based on
isFromBackupsTab, use that in the template, and remove the presetPolicies.update
calls from onMount; reference the existing onMount, presetPolicies and
isFromBackupsTab symbols and ensure only the local derived/writable is changed
for display while the shared presetPolicies store remains untouched.
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/databases/create/+page.svelte (1)
253-282: LGTM with minor note.The self-hosted backup promo section is well-structured with responsive layout handling.
Minor: The inline
style:width="100vw"on line 256 is overridden by the.backups-promoclass which setswidth: 220px. Consider removing the inline style if it's not serving a purpose.src/lib/components/customId.svelte (1)
29-37: Replace inline comment with a named constant.Keeps comments minimal and makes the constraint explicit in code.
♻️ Suggested change
+const ID_MAX_LENGTH = 36; function toIdFormat(str: string): string { return str .toLowerCase() .replace(/[^a-z0-9\-_. ]+/g, '') .replace(/ /g, '_') .replace(/^-+/, '') .replace(/\.+$/, '') .replace(/_{2,}/g, '_') - .slice(0, 36); // max length + .slice(0, ID_MAX_LENGTH); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte (2)
289-345: Remove non-essential inline HTML comments.These comments don’t document complex logic or TODOs and add noise to the template. As per coding guidelines:
**/*.{js,ts,svelte}: Use minimal comments; only comment TODOs or complex logic`.🧹 Suggested cleanup
- <!-- custom policy card selector --> <Card.Selector @@ - <!-- none option --> <Card.Selector
185-209: TypeselectedPolicyGroupas nullable.The variable is assigned
nullon line 185 but typed asstring. Whilestrictmode is currently disabled intsconfig.json, the type should still reflect the actual values assigned (string | null) for code clarity and future-proofing.🧩 Proposed type fix
- let selectedPolicyGroup: string = null; + let selectedPolicyGroup: string | null = null;
|
|
||
| async function createDatabase() { | ||
| try { | ||
| $createDatabaseStore.id ??= ID.unique(); |
There was a problem hiding this comment.
Bug: Empty string won't trigger ID generation.
The nullish coalescing assignment ??= only assigns when the value is null or undefined. Since the store initializes id as an empty string '', this condition will never trigger when the user doesn't provide a custom ID, likely causing an API error.
🐛 Proposed fix
- $createDatabaseStore.id ??= ID.unique();
+ $createDatabaseStore.id ||= ID.unique();Or for explicit clarity:
- $createDatabaseStore.id ??= ID.unique();
+ if (!$createDatabaseStore.id) {
+ $createDatabaseStore.id = ID.unique();
+ }📝 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.
| $createDatabaseStore.id ??= ID.unique(); | |
| $createDatabaseStore.id ||= ID.unique(); |
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/create/+page.svelte
at line 123, The current nullish coalescing assignment on
$createDatabaseStore.id (using "??=") won't generate an ID when id is an empty
string; change the logic to treat empty string as missing — e.g., replace the
"??=" with a check for falsy/empty and assign ID.unique() when
$createDatabaseStore.id is empty (or use logical OR assignment "||="), so that
ID.unique() is used when id === '' (reference: $createDatabaseStore.id and
ID.unique()).
| } catch (error) { | ||
| console.log(error); | ||
| addNotification({ | ||
| type: 'error', | ||
| message: error.message | ||
| }); | ||
| } |
There was a problem hiding this comment.
Remove debug statement and add error type guard.
console.log(error)should be removed or replaced with proper error logging.error.messageaccess assumes error is an object with a message property; consider type narrowing.
🛠️ Proposed fix
} catch (error) {
- console.log(error);
addNotification({
type: 'error',
- message: error.message
+ message: error instanceof Error ? error.message : 'Failed to create database'
});
}📝 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.
| } catch (error) { | |
| console.log(error); | |
| addNotification({ | |
| type: 'error', | |
| message: error.message | |
| }); | |
| } | |
| } catch (error) { | |
| addNotification({ | |
| type: 'error', | |
| message: error instanceof Error ? error.message : 'Failed to create database' | |
| }); | |
| } |
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/create/+page.svelte
around lines 153 - 159, Remove the stray debug print by deleting the
console.log(error) call and implement a type guard in the catch block so you
safely extract a message for addNotification; e.g., check if error instanceof
Error then use error.message, otherwise use String(error) (or a default string),
and send that into addNotification({ type: 'error', message: ... }); optionally
replace the removed debug print with console.error(...) if runtime logging is
required.
| selectedPolicyGroup = 'custom'; | ||
|
|
There was a problem hiding this comment.
Avoid clearing preset checkbox selections in backups tab.
Setting selectedPolicyGroup = 'custom' unconditionally triggers the reactive block that unchecks all presets, which wipes multi-select choices in the backups tab. Scope this to the non-backups flow (or guard the reactive block) so preset selections survive.
🛠️ Proposed fix (scope to non-backups tab)
- selectedPolicyGroup = 'custom';
+ if (!isFromBackupsTab) {
+ selectedPolicyGroup = 'custom';
+ }🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte
around lines 101 - 102, The unconditional assignment selectedPolicyGroup =
'custom' is wiping preset checkbox selections by triggering the reactive block
that clears presets; change this so it only runs outside the backups flow (or
add a guard in the reactive block that unchecks presets). Locate the
selectedPolicyGroup assignment and either wrap it in a conditional that checks
the current tab/flow (skip when in backups) or modify the reactive block that
clears presets to early-return when the current route/tab is backups so
multi-select choices in the backups tab are preserved.
| onMount(() => { | ||
| if (isFromBackupsTab) { | ||
| presetPolicies.update((preset) => { | ||
| return preset.filter((policy) => policy.id !== 'none'); | ||
| }); | ||
| } else { | ||
| presetPolicies.update((preset) => { | ||
| return preset.filter((policy) => policy.id !== 'hourly'); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Don’t mutate the shared presetPolicies store on mount.
Filtering the store here permanently removes policies for the rest of the app/session, which can break later views (e.g., open non-backups after backups and “none” is gone). Use a local derived list for display instead.
🛠️ Suggested approach (derive visible presets locally)
+ let visiblePresetPolicies: UserBackupPolicy[] = [];
+ $: visiblePresetPolicies = $presetPolicies.filter((policy) =>
+ isFromBackupsTab ? policy.id !== 'none' : policy.id !== 'hourly'
+ );
-
- onMount(() => {
- if (isFromBackupsTab) {
- presetPolicies.update((preset) => {
- return preset.filter((policy) => policy.id !== 'none');
- });
- } else {
- presetPolicies.update((preset) => {
- return preset.filter((policy) => policy.id !== 'hourly');
- });
- }
- });- {`#each` $presetPolicies as policy, index (index)}
+ {`#each` visiblePresetPolicies as policy, index (index)}🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte
around lines 211 - 221, The onMount block is mutating the shared presetPolicies
store (via presetPolicies.update) which permanently removes entries; instead
create a local derived value for rendering (e.g., visiblePresets or
displayPolicies) that reads presetPolicies and filters based on
isFromBackupsTab, use that in the template, and remove the presetPolicies.update
calls from onMount; reference the existing onMount, presetPolicies and
isFromBackupsTab symbols and ensure only the local derived/writable is changed
for display while the shared presetPolicies store remains untouched.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/routes/`(console)/project-[region]-[project]/databases/empty.svelte:
- Around line 75-81: The snippet invocation databaseTypeCard currently declares
parameters without TypeScript types; update the snippet parameter list to use
inline TypeScript types (e.g., add types for type, title, subtitle, image)
following the existing pattern used elsewhere like titleSnippet(title: string)
so the snippet header becomes a typed parameter object (include optional/union
types where appropriate, e.g., subtitle optional and image typed as string |
undefined).
🧹 Nitpick comments (3)
src/lib/stores/viewport.ts (1)
26-28: Normalize min/max to avoid silent always-false results.
If callers passmin > max, the derived store never turns true. Consider swapping the bounds.Suggested change
export function isViewPortWidthInRange(min: number, max: number) { - return derived(viewportWidth, ($width) => $width >= min && $width <= max); + const [from, to] = min <= max ? [min, max] : [max, min]; + return derived(viewportWidth, ($width) => $width >= from && $width <= to); }src/routes/(console)/project-[region]-[project]/databases/empty.svelte (2)
8-9: Remove or document commented-out code.The commented-out MongoDB imports and derived variable appear to be dead code. If this is intentional scaffolding for future work, add a TODO comment explaining the intent. Otherwise, consider removing to keep the codebase clean.
Also applies to: 29-29
119-143: Consider consolidating duplicate CSS properties.The media queries and
.adaptive-heightclass share similar styles (max-height,object-fit,border-radius). Consider using a shared class or CSS custom properties to reduce duplication, though this is a minor optimization.
| {#snippet databaseTypeCard({ type, title, subtitle, image = undefined })} | ||
| <Card | ||
| isButton | ||
| radius="s" | ||
| padding="none" | ||
| {disabled} | ||
| on:click={() => onDatabaseTypeSelected?.(type)}> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Card component
echo "=== Locating Card component ==="
find . -type f -name "*Card*" | grep -E "\.(svelte|ts|tsx)$" | head -20Repository: appwrite/console
Length of output: 1559
🏁 Script executed:
#!/bin/bash
# Examine the target file
echo "=== Checking target file ==="
cat -n src/routes/\(console\)/project-\[region\]-\[project\]/databases/empty.svelte | sed -n '70,120p'Repository: appwrite/console
Length of output: 2193
🏁 Script executed:
#!/bin/bash
# Search for on:click and onclick patterns in Svelte files
echo "=== Event directive patterns in .svelte files ==="
rg "on:click|onclick" --type svelte -A 2 -B 2 | head -100Repository: appwrite/console
Length of output: 157
🏁 Script executed:
#!/bin/bash
# Examine the Card component
echo "=== Card component at src/routes/(public)/card/Card.svelte ==="
cat -n src/routes/\(public\)/card/Card.svelte | head -80Repository: appwrite/console
Length of output: 3544
🏁 Script executed:
#!/bin/bash
# Search for on:click patterns in all svelte files
echo "=== Event directive patterns in svelte files ==="
rg "on:click|onclick" --glob "*.svelte" -A 2 -B 2 | head -150Repository: appwrite/console
Length of output: 10286
🏁 Script executed:
#!/bin/bash
# Check for snippet parameter type annotations in the codebase
echo "=== Snippet parameters with types in codebase ==="
rg "{`#snippet`" --glob "*.svelte" -A 1 | head -100Repository: appwrite/console
Length of output: 211
🏁 Script executed:
#!/bin/bash
# Check imports in the target file
echo "=== Imports in target file ==="
head -30 src/routes/\(console\)/project-\[region\]-\[project\]/databases/empty.svelteRepository: appwrite/console
Length of output: 1248
🏁 Script executed:
#!/bin/bash
# Search for Card component in lib/components
echo "=== Looking for Card.svelte in lib/components ==="
find src/lib/components -name "*[Cc]ard*.svelte" -type fRepository: appwrite/console
Length of output: 632
🏁 Script executed:
#!/bin/bash
# Check snippet patterns without special characters
echo "=== Searching for snippet keyword ==="
rg "snippet.*\{" --glob "*.svelte" -A 1 | head -80Repository: appwrite/console
Length of output: 2196
🏁 Script executed:
#!/bin/bash
# Examine the Card component
echo "=== Card component at src/lib/components/card.svelte ==="
cat -n src/lib/components/card.svelteRepository: appwrite/console
Length of output: 2760
🏁 Script executed:
#!/bin/bash
# Look for snippets with parameter types more carefully
echo "=== Snippets with type examples ==="
rg "snippet.*\(" --glob "*.svelte" -A 2 | grep -E "snippet|:" | head -60Repository: appwrite/console
Length of output: 7938
Add TypeScript type annotations to snippet parameters.
The snippet parameters lack type annotations. Per the coding guidelines for .svelte files, use TypeScript inline types for type definitions. Examples in the codebase show this pattern (e.g., {#snippet titleSnippet(title: string)}). Adding types here would improve IDE support and catch errors early:
-{`#snippet` databaseTypeCard({ type, title, subtitle, image = undefined })}
+{`#snippet` databaseTypeCard({ type, title, subtitle, image = undefined }: { type: DatabaseType; title: string; subtitle: string; image?: string })}📝 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.
| {#snippet databaseTypeCard({ type, title, subtitle, image = undefined })} | |
| <Card | |
| isButton | |
| radius="s" | |
| padding="none" | |
| {disabled} | |
| on:click={() => onDatabaseTypeSelected?.(type)}> | |
| {`#snippet` databaseTypeCard({ type, title, subtitle, image = undefined }: { type: DatabaseType; title: string; subtitle: string; image?: string })} | |
| <Card | |
| isButton | |
| radius="s" | |
| padding="none" | |
| {disabled} | |
| on:click={() => onDatabaseTypeSelected?.(type)}> |
🤖 Prompt for AI Agents
In `@src/routes/`(console)/project-[region]-[project]/databases/empty.svelte
around lines 75 - 81, The snippet invocation databaseTypeCard currently declares
parameters without TypeScript types; update the snippet parameter list to use
inline TypeScript types (e.g., add types for type, title, subtitle, image)
following the existing pattern used elsewhere like titleSnippet(title: string)
so the snippet header becomes a typed parameter object (include optional/union
types where appropriate, e.g., subtitle optional and image typed as string |
undefined).
update: disable form when submit is in progress.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/customId.svelte (1)
29-71:⚠️ Potential issue | 🟠 MajorAvoid auto‑sync loops and ensure generated IDs start with alphanumeric.
toIdFormat()can yield""or a leading_/.. WithsyncFromtruthy this can bounceidbetween""andnull, and it can prefill invalid IDs. Guard empty results and strip leading non‑alphanumerics.🔧 Suggested fix
function toIdFormat(str: string): string { return str .toLowerCase() .replace(/[^a-z0-9\-_. ]+/g, '') .replace(/ /g, '_') - .replace(/^-+/, '') + .replace(/^[^a-z0-9]+/, '') .replace(/\.+$/, '') .replace(/_{2,}/g, '_') .slice(0, 36); // max length } $effect(() => { if (syncFrom && !touchedId) { const newId = toIdFormat(syncFrom); - if (id !== newId) { - id = newId; - } + if (!newId) { + if (id !== null) id = null; + return; + } + if (id !== newId) id = newId; } });
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/create/+page.svelte:
- Around line 214-222: The Cancel button currently calls
resetCreateDatabaseStore() immediately which clears the form before the user
confirms; change the Cancel Button's click handler to only set showExitModal =
true (remove the resetCreateDatabaseStore() call) so that form reset only
happens in the onExit handler (the existing onExit function should remain
responsible for calling resetCreateDatabaseStore()) and the modal dismissal
preserves user input.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte:
- Line 152: selectedPolicyGroup is assigned null but declared as string; update
its type annotation (the declaration of selectedPolicyGroup) to allow null
(e.g., string | null) so the assignment at the top (selectedPolicyGroup = null)
and any subsequent null checks are type-safe; locate the let/const declaration
named selectedPolicyGroup in the Svelte script block and change its type
accordingly.
- Around line 140-142: getPolicyById can return undefined when IDs are
randomized, so add null guards wherever its result is used: before calling
markPolicyChecked and before accessing .id or .label on the returned policy.
Specifically, update usages of getPolicyById(...) that are passed into
markPolicyChecked and any code that reads .id/.label to first check the result
(e.g., const policy = getPolicyById(id); if (!policy) return/skip/choose a
sensible default) or use optional chaining with a clear fallback value; keep
references to getPolicyById, markPolicyChecked and the presetPolicies lookup so
the changes are localized to those call sites.
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/databases/create/+page.svelte (2)
39-40: Suspicious type assertions for database type.Line 39's
?? (null as DatabaseType)is redundant sincesearchParams.get()already returnsstring | null. CastingnulltoDatabaseTypeis a type lie that undermines type safety.♻️ Proposed fix
- const typeFromParams = page.url.searchParams.get('type') ?? (null as DatabaseType); - let type = $state(typeFromParams ?? 'tablesdb') as DatabaseType; + const typeFromParams = page.url.searchParams.get('type') as DatabaseType | null; + let type = $state<DatabaseType>(typeFromParams ?? 'tablesdb');
102-103: The function returns void—the assignment suggestion is not applicable.
cronExpression()is intentionally designed for side effects and returns void. The first suggested fix won't work. If clarity is desired, consider adding a brief comment:const totalPoliciesPromise = totalPolicies.map((policy) => { + // Mutates policy.schedule based on frequency configuration cronExpression(policy);src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte (1)
32-32: Consider propagatingdisabledto all interactive controls.If
disabledis meant to freeze edits during submit, it’s currently only applied to the daily InputSwitch. You may want to pass it through to the preset checkboxes/Card.Selector and action buttons for consistent lockout.Also applies to: 271-271
| <Button | ||
| secondary | ||
| disabled={$isSubmitting} | ||
| on:click={() => { | ||
| showExitModal = true; | ||
| resetCreateDatabaseStore(); | ||
| }}> | ||
| Cancel | ||
| </Button> |
There was a problem hiding this comment.
Bug: Premature form reset before exit confirmation.
Calling resetCreateDatabaseStore() when clicking Cancel clears the form data before the user confirms exit via the modal. If the user dismisses the exit modal to continue editing, their input is already lost. The reset in onExit (line 172) is sufficient.
🐛 Proposed fix
<Button
secondary
disabled={$isSubmitting}
on:click={() => {
showExitModal = true;
- resetCreateDatabaseStore();
}}>
Cancel
</Button>📝 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.
| <Button | |
| secondary | |
| disabled={$isSubmitting} | |
| on:click={() => { | |
| showExitModal = true; | |
| resetCreateDatabaseStore(); | |
| }}> | |
| Cancel | |
| </Button> | |
| <Button | |
| secondary | |
| disabled={$isSubmitting} | |
| on:click={() => { | |
| showExitModal = true; | |
| }}> | |
| Cancel | |
| </Button> |
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/create/+page.svelte
around lines 214 - 222, The Cancel button currently calls
resetCreateDatabaseStore() immediately which clears the form before the user
confirms; change the Cancel Button's click handler to only set showExitModal =
true (remove the resetCreateDatabaseStore() call) so that form reset only
happens in the onExit handler (the existing onExit function should remain
responsible for calling resetCreateDatabaseStore()) and the modal dismissal
preserves user input.
| const getPolicyById = (id: string) => { | ||
| return $presetPolicies.find((p) => p.id === id); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the createPolicy.svelte file
cat -n "src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte" | head -300Repository: appwrite/console
Length of output: 12619
🏁 Script executed:
# Continue reading the file to see the call sites mentioned in the review
cat -n "src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte" | sed -n '240,350p'Repository: appwrite/console
Length of output: 5683
🏁 Script executed:
# Now let's check the store where presetPolicies is defined
cat -n "src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/store.ts" 2>/dev/null || cat -n "src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/store.js" 2>/dev/null || find . -name "store.ts" -o -name "store.js" | grep backupsRepository: appwrite/console
Length of output: 2248
🏁 Script executed:
# Verify the exact behavior at line 164 - does it change all IDs or just one?
cat -n "src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte" | sed -n '160,170p'Repository: appwrite/console
Length of output: 527
🏁 Script executed:
# Check the onMount filter logic more carefully and trace the flow
cat -n "src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte" | sed -n '212,223p'Repository: appwrite/console
Length of output: 514
🏁 Script executed:
# Let me verify the exact flow - check if line 246 is executed even when dailyPolicy becomes undefined
# Also check markPolicyChecked to see if it handles undefined
cat -n "src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte" | sed -n '108,119p'Repository: appwrite/console
Length of output: 536
🏁 Script executed:
# Final verification: trace the exact problem scenario
# When $currentPlan?.backupPolicies === 1 AND isFromBackupsTab = true:
# 1. onMount filters 'none' from store
# 2. Line 164 randomizes ALL IDs including 'daily'
# 3. Line 246 tries to find 'daily' by original ID, gets undefined
# 4. Line 274 accesses undefined.id → runtime error
# Let's verify if any guards exist between line 246 and line 274
cat -n "src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte" | sed -n '244,276p'Repository: appwrite/console
Length of output: 1797
🏁 Script executed:
# Let me trace through one more scenario - what happens when you pass undefined to markPolicyChecked
# Check if policy parameter has a type guard
cat -n "src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte" | sed -n '108,119p'Repository: appwrite/console
Length of output: 536
Add null guards for getPolicyById return values at lines 246, 309-310.
When policy IDs are randomized (line 164), getPolicyById('daily'/'none') returns undefined. Line 274 then passes undefined to markPolicyChecked, which accesses policy.label and throws. Similarly, lines 314–316 and 338–341 access .id and .label on potentially undefined values.
🛠️ Suggested fixes
Line 246:
- {`@const` dailyPolicy = getPolicyById('daily')}
-
- {`#if` isFromBackupsTab}
+ {`@const` dailyPolicy = getPolicyById('daily')}
+ {`#if` isFromBackupsTab}
<!-- ... -->
{:else}
- <InputSwitch on:change={(event) => markPolicyChecked(event, dailyPolicy)}>
+ {`#if` dailyPolicy}
+ <InputSwitch on:change={(event) => markPolicyChecked(event, dailyPolicy)}>
+ ...
+ </InputSwitch>
+ {/if}Lines 309–344:
{:else}
{`@const` none = getPolicyById('none')}
{`@const` dailPreset = getPolicyById('daily')}
- <Card.Selector
- id={dailPreset.id}
- ...
+ {`#if` none && dailPreset}
+ <Card.Selector
+ id={dailPreset.id}
+ ...
+ </Card.Selector>
+ <!-- other selectors -->
+ {/if}🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte
around lines 140 - 142, getPolicyById can return undefined when IDs are
randomized, so add null guards wherever its result is used: before calling
markPolicyChecked and before accessing .id or .label on the returned policy.
Specifically, update usages of getPolicyById(...) that are passed into
markPolicyChecked and any code that reads .id/.label to first check the result
(e.g., const policy = getPolicyById(id); if (!policy) return/skip/choose a
sensible default) or use optional chaining with a clear fallback value; keep
references to getPolicyById, markPolicyChecked and the presetPolicies lookup so
the changes are localized to those call sites.
| resetFormVariables(); | ||
| showCustomPolicy = false; | ||
| listOfCustomPolicies = []; | ||
| selectedPolicyGroup = null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check TypeScript configuration for strict null checking
rg -n "strictNullChecks|\"strict\"" -g 'tsconfig*.json'Repository: appwrite/console
Length of output: 101
🏁 Script executed:
# Read the Svelte file to verify the code at lines 152 and 186
fd 'createPolicy.svelte' --type fRepository: appwrite/console
Length of output: 165
🏁 Script executed:
# Read the file around lines 152 and 186
wc -l src/routes/'(console)'/project-'[region]'-'[project]'/databases/database-'[database]'/backups/createPolicy.svelteRepository: appwrite/console
Length of output: 169
🏁 Script executed:
# Read the file content around the target lines
cat -n src/routes/'(console)'/project-'[region]'-'[project]'/databases/database-'[database]'/backups/createPolicy.svelte | sed -n '145,160p'Repository: appwrite/console
Length of output: 622
🏁 Script executed:
# Read around line 186
cat -n src/routes/'(console)'/project-'[region]'-'[project]'/databases/database-'[database]'/backups/createPolicy.svelte | sed -n '180,195p'Repository: appwrite/console
Length of output: 715
🏁 Script executed:
# Search for similar variable declarations in the file with null assignments
cat -n src/routes/'(console)'/project-'[region]'-'[project]'/databases/database-'[database]'/backups/createPolicy.svelte | grep -E "(let|var|const).*:\s*(string|boolean|number)" | head -20Repository: appwrite/console
Length of output: 475
Add null to selectedPolicyGroup's type definition.
Line 186 declares selectedPolicyGroup: string but both line 152 and line 186 assign null to it. The type annotation should reflect what the variable actually holds.
🛠️ Proposed fix
- let selectedPolicyGroup: string = null;
+ let selectedPolicyGroup: string | null = null;Also applies to: Line 152 (the assignment will be valid once the declaration is fixed).
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte
at line 152, selectedPolicyGroup is assigned null but declared as string; update
its type annotation (the declaration of selectedPolicyGroup) to allow null
(e.g., string | null) so the assignment at the top (selectedPolicyGroup = null)
and any subsequent null checks are type-safe; locate the let/const declaration
named selectedPolicyGroup in the Svelte script block and change its type
accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)
552-581:⚠️ Potential issue | 🟠 MajorSet
databaseColumnSheetOptions.columnfor header actions.Update/delete flows rely on this store (the delete modal reads
.key). If it’s stale or null, the sheet can open with the wrong column or throw. Set it once when handling header actions.🐛 Proposed fix
if (type === 'header') { if (!columnId || !column) { return; } + $databaseColumnSheetOptions.column = column; if (action === 'update') { $databaseColumnSheetOptions.show = true; $databaseColumnSheetOptions.isEdit = true; $databaseColumnSheetOptions.title = 'Update column'; } if (action === 'column-left' || action === 'column-right') { const neighbour = columnId; const to = action === 'column-left' ? 'left' : 'right'; - $databaseColumnSheetOptions.column = column; $databaseColumnSheetOptions.direction = { neighbour: columnId, to: action === 'column-left' ? 'left' : 'right' };
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/spreadsheet.svelte:
- Line 6: The import using the non-approved alias "$database" (importing
SideSheet) violates the guideline limiting imports to $lib, $routes or $themes;
change the import of SideSheet to use one of the approved aliases (for example
import SideSheet from an equivalent module under $routes or $lib — e.g. move or
re-export the module under $lib or $routes and update the import for SideSheet
accordingly) so the code no longer references "$database" and continues to
resolve the same SideSheet symbol.
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/spreadsheet.svelte (4)
46-46: Consider using a plainSetinstead ofSvelteSetfor the element cache.
cachedElementsis only used internally to track observed DOM elements for the ResizeObserver and is never bound to the template. Since reactivity isn't needed here, a plainSet<Element>would be more appropriate and slightly more efficient.♻️ Proposed fix
- import { SvelteSet } from 'svelte/reactivity';- let cachedElements = new SvelteSet<Element>(); + let cachedElements = new Set<Element>();
160-161: Consider using Svelte 5 event handler syntax for consistency.The component uses Svelte 5 runes (
$state,$effect,$props,$bindable), but the window event listener uses the legacy Svelte 4on:resizesyntax. For consistency, consider using the Svelte 5 syntax.♻️ Proposed fix
-<svelte:window on:resize={handleResize} /> +<svelte:window onresize={handleResize} />
167-167: Simplify the conditional class check.The
typeof noSqlEditor !== 'undefined'check is verbose. Snippets are truthy when defined, so a simple truthiness check suffices.♻️ Proposed fix
- class:has-json-editor={typeof noSqlEditor !== 'undefined'}> + class:has-json-editor={noSqlEditor}>
170-174: Consider simplifying the nested class structure.The outer
divat Line 170 has classno-sql-editor, and the innerdivat Line 172 has classno-sql-editor desktop. This redundancy could be confusing. Consider using justdesktopfor the inner div since it's already inside.no-sql-editor.♻️ Proposed fix
<div class="no-sql-editor"> {`#if` !$isSmallViewport} - <div class="no-sql-editor desktop" style:height={spreadsheetHeight}> + <div class="desktop" style:height={spreadsheetHeight}> {`@render` noSqlEditor?.()} </div>Then update the CSS selector accordingly:
- &:has(.no-sql-editor:empty), - &:has(.no-sql-editor.desktop:empty) { + &:has(.no-sql-editor:empty), + &:has(.no-sql-editor > .desktop:empty) { grid-template-columns: 1fr; }
| import { scrollStore, sheetHeightStore } from './store'; | ||
| import { onMount, onDestroy, type Snippet, tick } from 'svelte'; | ||
| import { isSmallViewport } from '$lib/stores/viewport'; | ||
| import { SideSheet } from '$database/(entity)'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and display svelte.config.js
echo "=== svelte.config.js ==="
find . -maxdepth 2 -name 'svelte.config.js' -type f -exec cat {} \;
echo -e "\n=== vite.config files ==="
find . -maxdepth 2 -name 'vite.config.*' -type f -exec cat {} \;
echo -e "\n=== tsconfig.json ==="
find . -maxdepth 2 -name 'tsconfig.json' -type f -exec cat {} \;Repository: appwrite/console
Length of output: 3308
Use a standard path alias ($lib, $routes, or $themes) instead of $database.
The $database path alias used in line 6 is configured in svelte.config.js, but it violates the coding guideline that specifies only $lib, $routes, and $themes should be used for imports in .{js,ts,svelte} files. Either import from a standard alias or update the coding guidelines to formally approve $database as an additional standard alias.
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/spreadsheet.svelte
at line 6, The import using the non-approved alias "$database" (importing
SideSheet) violates the guideline limiting imports to $lib, $routes or $themes;
change the import of SideSheet to use one of the approved aliases (for example
import SideSheet from an equivalent module under $routes or $lib — e.g. move or
re-export the module under $lib or $routes and update the import for SideSheet
accordingly) so the code no longer references "$database" and continues to
resolve the same SideSheet symbol.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/helpers/errorMessages.ts:
- Around line 37-41: The getKeyName function's regex doesn't accept escaped
quotes inside quoted keys (e.g., "key\"name"), so update the pattern to allow
escaped characters and return the unescaped key; replace the current match regex
in getKeyName with one that captures escaped sequences such as
/^(?:"((?:\\.|[^"\\])*)"|'((?:\\.|[^'\\])*)'|([A-Za-z_$][\w$]*))$/ and, if a
quoted capture is returned (match[1] or match[2]), unescape backslashes and
quotes (e.g., replace /\\(.)/g with '$1') before returning the key, otherwise
return the identifier capture (match[3]) or null.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte:
- Around line 36-43: The build fails because the file uses CodeMirror's Text
type (used at the occurrences around view editor logic) but doesn't import it,
causing TypeScript to pick the DOM Text type; fix this by adding an import of
Text from '@codemirror/state' (e.g., alongside EditorState, EditorSelection,
Transaction, Compartment, Extension) — use an `import type { Text } from
'@codemirror/state'` or include Text in the existing import so all usages (the
variables/functions referencing Text where methods like lineAt(), line(), or
iterChanges are called) use the correct CodeMirror Text type.
🧹 Nitpick comments (8)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/helpers/errorMessages.ts (3)
4-8: Export theSmartErrortype for consumers.
makeErrorMessagereturnsSmartError, but the type isn't exported. Callers cannot properly type variables holding the result.Suggested fix
-type SmartError = { +export type SmartError = { message: string; hint?: string; range?: { from: number; to: number }; };
193-193: Consider avoiding full document string copy for large documents.
state.doc.toString()creates a complete copy of the document in memory. For very large JSON documents, this adds memory pressure. CodeMirror'sTexttype supports efficient iteration viaiterLines()or character-by-character access without materializing the full string.
613-630: Consider unifying detector function signatures.The detectors array mixes functions that use
statedirectly with wrappers like(_state, ctx) => detectCommentToken(ctx). Sincestateis already used to buildcontext, consider having all detectors take onlyErrorContextand addingstateto the context if any detector truly needs it.This would simplify the array to a cleaner form:
const detectors: Array<(ctx: ErrorContext) => SmartError | null> = [ detectCommentToken, detectUnclosedStringAcrossLines, // ... ];src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts (2)
68-68: Add explicit type annotation toexpandTabsstore.The store is initialized with
nullbut lacks a type annotation, which results in an inferred type ofWritable<null>. If this store is meant to hold other values, TypeScript won't catch type mismatches.✨ Suggested improvement
-export const expandTabs = writable(null); +export const expandTabs = writable<string | null>(null);Adjust the type (e.g.,
string,boolean, or a specific union) based on the expected values.
155-167: Consider adding explicit return type and aligning withSortState.The function's inferred return type may not align with the
SortStatetype defined above:
- Line 166 returns
column: null, butSortState.columnisstring | undefined(optional), notstring | null- The
direction: 'default'value assumesSortDirectionincludes'default'as a valid literalAdding an explicit return type would catch any mismatches at compile time.
✨ Suggested improvement
-export function extractSortFromQueries(parsedQueries: Map<TagValue, string>) { +export function extractSortFromQueries(parsedQueries: Map<TagValue, string>): SortState { for (const [tagValue, queryString] of parsedQueries.entries()) { if (queryString.includes('orderAsc') || queryString.includes('orderDesc')) { const isAsc = queryString.includes('orderAsc'); return { column: tagValue.value, - direction: isAsc ? 'asc' : 'desc' + direction: (isAsc ? 'asc' : 'desc') as SortDirection }; } } - return { column: null, direction: 'default' }; + return { column: undefined, direction: 'default' as SortDirection }; }Adjust based on whether
SortDirectionactually includes'default'and the expected semantics for "no column".src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte (1)
13-17: Prefer$routesalias for local route imports.
Switching the relative imports to$routeskeeps routing imports consistent and avoids path churn.♻️ Suggested update
-import { showCreateEntity, randomDataModalState, resetSampleFieldsConfig } from './store'; -import { entityColumnSuggestions } from './(suggestions)/store'; -import { showCreateBackup, showCreatePolicy } from './backups/store'; +import { + showCreateEntity, + randomDataModalState, + resetSampleFieldsConfig +} from '$routes/(console)/project-[region]-[project]/databases/database-[database]/store'; +import { entityColumnSuggestions } from '$routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/store'; +import { showCreateBackup, showCreatePolicy } from '$routes/(console)/project-[region]-[project]/databases/database-[database]/backups/store';As per coding guidelines, use $lib, $routes, and $themes path aliases for imports.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/sonners/suggestions.svelte (1)
84-86: Drop the inline CSS comment to keep comments minimal.♻️ Suggested change
- & > :global:first-child { - display: unset; /* stack with flex-start in FAB */ + & > :global:first-child { + display: unset;As per coding guidelines: Use minimal comments; only comment TODOs or complex logic.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte (1)
52-77: Prefer $routes alias for local editor imports.These relative imports should use the $routes alias to comply with project import conventions. As per coding guidelines:
**/*.{js,ts,svelte}: Use $lib, $routes, and $themes path aliases for imports.
| function getKeyName(lineText: string, colonIndex: number): string | null { | ||
| const keyPart = lineText.slice(0, colonIndex).trim(); | ||
| const match = keyPart.match(/^(?:"([^"]+)"|'([^']+)'|([A-Za-z_$][\w$]*))$/); | ||
| return match ? match[1] || match[2] || match[3] || null : null; | ||
| } |
There was a problem hiding this comment.
Regex doesn't handle escaped quotes in key names.
The pattern [^"]+ and [^']+ won't match keys containing escaped quotes like "key\"name". This may cause getKeyName to return null for valid keys, leading to less informative error messages.
Suggested fix to handle escaped quotes
function getKeyName(lineText: string, colonIndex: number): string | null {
const keyPart = lineText.slice(0, colonIndex).trim();
- const match = keyPart.match(/^(?:"([^"]+)"|'([^']+)'|([A-Za-z_$][\w$]*))$/);
+ const match = keyPart.match(/^(?:"((?:[^"\\]|\\.)*)"|'((?:[^'\\]|\\.)*)'|([A-Za-z_$][\w$]*))$/);
return match ? match[1] || match[2] || match[3] || null : null;
}🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/helpers/errorMessages.ts
around lines 37 - 41, The getKeyName function's regex doesn't accept escaped
quotes inside quoted keys (e.g., "key\"name"), so update the pattern to allow
escaped characters and return the unescaped key; replace the current match regex
in getKeyName with one that captures escaped sequences such as
/^(?:"((?:\\.|[^"\\])*)"|'((?:\\.|[^'\\])*)'|([A-Za-z_$][\w$]*))$/ and, if a
quoted capture is returned (match[1] or match[2]), unescape backslashes and
quotes (e.g., replace /\\(.)/g with '$1') before returning the key, otherwise
return the identifier capture (match[3]) or null.
...oject]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte:
- Around line 74-84: Replace hardcoded "table"/"table-" strings and static
labels with the existing terminology object: use terminology-provided labels
(e.g., terminology.labels.createEntity / terminology.labels.goToEntities /
terminology.labels.findEntities) instead of "Create table", "Go to tables",
"Find tables", and replace URL checks like page.url.pathname.includes('table-')
with a dynamic check using the terminology value (e.g.,
page.url.pathname.includes(`${terminology.entity}-`) or whichever terminology
field gives the entity slug) so the disabled logic and key hints use terminology
consistently; update all occurrences referenced in this diff (the callback that
sets $showCreateEntity and goto, the keys assignment, and the disabled
conditions at the shown lines and the other instances you noted) to reference
terminology and databaseId/project/base/page symbols already in scope.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte:
- Around line 930-943: The saved selection positions
(currentSelection.anchor/head) from the old document may be out-of-range after
replacing content, causing CodeMirror errors; in the block that builds the new
content (serializeData(updatedData)) and calls editorView.dispatch, remove the
explicit selection field so CodeMirror can automatically map the cursor through
the change instead of applying stale positions — update the dispatch call in the
editorView handling (where currentSelection/currentContent are captured and
editorView.dispatch is invoked) to only provide changes: { from: 0, to:
currentContent.length, insert: newContent } and omit selection.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/spreadsheet.svelte:
- Around line 280-329: hadErrors is unused dead code in handleDelete — remove it
and its conditional guard instead of keeping a flag that's never set; delete the
hadErrors declaration and the surrounding if (!hadErrors) { ... } block (which
wraps addNotification success), leaving the success notification in the try path
(so success is shown only when no exception occurs), and keep the existing catch
which already shows errors and short-circuits; update references to hadErrors
(if any) accordingly.
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/extensions/highlighting.ts (1)
117-167:createSystemFieldStylePluginscans all lines on every doc change — consider viewport scoping for large documents.Unlike
createNestedKeyPluginwhich only processes visible ranges, this plugin iterates every line in the document on each change. For typical document sizes this is fine, but if the editor could host very large JSON documents, consider scoping toview.visibleRangesthe same way.src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte (1)
600-665:SvelteMapused in non-reactive contexts — use plainMapinstead.
checkSystemFieldsMatch(line 602) andapplySystemFieldsPatch(line 690) useSvelteMap, but these are local helper functions where Svelte reactivity tracking adds overhead for no benefit. A plainMapis more appropriate.♻️ Suggested fix
- const foundValues = new SvelteMap<string, string>(); + const foundValues = new Map<string, string>();- const hits = new SvelteMap<string, Hit>(); + const hits = new Map<string, Hit>();
| label: 'Create table', | ||
| callback() { | ||
| $showCreateEntity = true; | ||
| if (!page.url.pathname.endsWith(databaseId)) { | ||
| goto( | ||
| `${base}/project-${page.params.region}-${project}/databases/database-${databaseId}` | ||
| ); | ||
| } | ||
| }, | ||
| keys: page.url.pathname.endsWith(databaseId) ? ['c'] : ['c', 'c'], | ||
| disabled: page.url.pathname.includes('table-') || !$canWriteTables, |
There was a problem hiding this comment.
Hardcoded 'table-' and label strings won't adapt to DocumentsDB entities.
The command labels ("Create table", "Go to tables", "Find tables") and the disabled checks using page.url.pathname.includes('table-') are hardcoded for TablesDB. Since this PR adds DocumentsDB support (where entities are "collections" with collection- URL segments), these should use the terminology object that's already available at line 39.
The same 'table-' pattern appears in disabled conditions at lines 128, 140, 152, and 164–165.
Example fix for one command
{
- label: 'Create table',
+ label: `Create ${terminology.entity.lower.singular}`,
callback() {
$showCreateEntity = true;
if (!page.url.pathname.endsWith(databaseId)) {
goto(
`${base}/project-${page.params.region}-${project}/databases/database-${databaseId}`
);
}
},
keys: page.url.pathname.endsWith(databaseId) ? ['c'] : ['c', 'c'],
- disabled: page.url.pathname.includes('table-') || !$canWriteTables,
+ disabled: page.url.pathname.includes(`${terminology.entity.lower.singular}-`) || !$canWriteTables,
group: 'databases',
icon: IconPlus
},Apply the same approach to "Go to tables" (line 121), "Find tables" (line 171), and all other includes('table-') checks. Based on learnings, the 'legacy' database type maps to the same API as 'tablesdb', so the terminology-driven approach ensures correct behavior for all database types.
📝 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.
| label: 'Create table', | |
| callback() { | |
| $showCreateEntity = true; | |
| if (!page.url.pathname.endsWith(databaseId)) { | |
| goto( | |
| `${base}/project-${page.params.region}-${project}/databases/database-${databaseId}` | |
| ); | |
| } | |
| }, | |
| keys: page.url.pathname.endsWith(databaseId) ? ['c'] : ['c', 'c'], | |
| disabled: page.url.pathname.includes('table-') || !$canWriteTables, | |
| label: `Create ${terminology.entity.lower.singular}`, | |
| callback() { | |
| $showCreateEntity = true; | |
| if (!page.url.pathname.endsWith(databaseId)) { | |
| goto( | |
| `${base}/project-${page.params.region}-${project}/databases/database-${databaseId}` | |
| ); | |
| } | |
| }, | |
| keys: page.url.pathname.endsWith(databaseId) ? ['c'] : ['c', 'c'], | |
| disabled: page.url.pathname.includes(`${terminology.entity.lower.singular}-`) || !$canWriteTables, |
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte
around lines 74 - 84, Replace hardcoded "table"/"table-" strings and static
labels with the existing terminology object: use terminology-provided labels
(e.g., terminology.labels.createEntity / terminology.labels.goToEntities /
terminology.labels.findEntities) instead of "Create table", "Go to tables",
"Find tables", and replace URL checks like page.url.pathname.includes('table-')
with a dynamic check using the terminology value (e.g.,
page.url.pathname.includes(`${terminology.entity}-`) or whichever terminology
field gives the entity slug) so the disabled logic and key hints use terminology
consistently; update all occurrences referenced in this diff (the callback that
sets $showCreateEntity and goto, the keys assignment, and the disabled
conditions at the shown lines and the other instances you noted) to reference
terminology and databaseId/project/base/page symbols already in scope.
|
|
||
| // Manually update the editor content | ||
| const newContent = serializeData(updatedData); | ||
|
|
||
| if (editorView) { | ||
| // Save current cursor position | ||
| const currentSelection = editorView.state.selection.main; | ||
| const currentContent = editorView.state.doc.toString(); | ||
|
|
||
| editorView.dispatch({ | ||
| changes: { from: 0, to: currentContent.length, insert: newContent }, | ||
| selection: { anchor: currentSelection.anchor, head: currentSelection.head } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Selection may exceed new document length after content replacement.
When applying suggested attributes, the saved currentSelection.anchor/head from the old document are used directly in the dispatch with new (potentially shorter or differently structured) content. If the cursor was near the end of the old content and the new content is shorter, CodeMirror may throw or produce unexpected behavior.
🛡️ Suggested fix
- const currentSelection = editorView.state.selection.main;
const currentContent = editorView.state.doc.toString();
editorView.dispatch({
- changes: { from: 0, to: currentContent.length, insert: newContent },
- selection: { anchor: currentSelection.anchor, head: currentSelection.head }
+ changes: { from: 0, to: currentContent.length, insert: newContent }
});Omitting selection lets CodeMirror map the cursor through the change automatically, which is safer than preserving stale positions.
📝 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.
| // Manually update the editor content | |
| const newContent = serializeData(updatedData); | |
| if (editorView) { | |
| // Save current cursor position | |
| const currentSelection = editorView.state.selection.main; | |
| const currentContent = editorView.state.doc.toString(); | |
| editorView.dispatch({ | |
| changes: { from: 0, to: currentContent.length, insert: newContent }, | |
| selection: { anchor: currentSelection.anchor, head: currentSelection.head } | |
| }); | |
| } | |
| // Manually update the editor content | |
| const newContent = serializeData(updatedData); | |
| if (editorView) { | |
| // Save current cursor position | |
| const currentContent = editorView.state.doc.toString(); | |
| editorView.dispatch({ | |
| changes: { from: 0, to: currentContent.length, insert: newContent } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte
around lines 930 - 943, The saved selection positions
(currentSelection.anchor/head) from the old document may be out-of-range after
replacing content, causing CodeMirror errors; in the block that builds the new
content (serializeData(updatedData)) and calls editorView.dispatch, remove the
explicit selection field so CodeMirror can automatically map the cursor through
the change instead of applying stale positions — update the dispatch call in the
editorView handling (where currentSelection/currentContent are captured and
editorView.dispatch is invoked) to only provide changes: { from: 0, to:
currentContent.length, insert: newContent } and omit selection.
| async function handleDelete() { | ||
| showDelete = false; | ||
| let hadErrors = false; | ||
|
|
||
| try { | ||
| if (selectedDocumentForDelete) { | ||
| await databaseSdk.deleteRecord({ | ||
| databaseId, | ||
| entityId: collectionId, | ||
| recordId: selectedDocumentForDelete | ||
| }); | ||
| } else { | ||
| if (selectedDocuments.length) { | ||
| for (const batch of chunks(selectedDocuments, 100)) { | ||
| await databaseSdk.deleteRecords({ | ||
| databaseId, | ||
| entityId: collectionId, | ||
| queries: [Query.equal('$id', batch)] | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| await invalidate(Dependencies.DOCUMENTS); | ||
| trackEvent(Click.DatabaseRowDelete); | ||
|
|
||
| if (!hadErrors) { | ||
| // error is already shown above! | ||
| addNotification({ | ||
| type: 'success', | ||
| message: `${selectedDocuments.length ? selectedDocuments.length : 1} document${selectedDocuments.length > 1 ? 's' : ''} deleted` | ||
| }); | ||
| } | ||
|
|
||
| spreadsheetRenderKey.set( | ||
| hash([ | ||
| data.documents.total.toString(), | ||
| ...(selectedDocuments as string[]), | ||
| selectedDocumentForDelete | ||
| ]) | ||
| ); | ||
| } catch (error) { | ||
| addNotification({ type: 'error', message: error.message }); | ||
| trackError(error, Submit.DocumentDelete); | ||
| } finally { | ||
| selectedDocuments = []; | ||
| showDelete = false; | ||
| selectedDocumentForDelete = null; | ||
| } | ||
| } |
There was a problem hiding this comment.
hadErrors is dead code — always false.
hadErrors is initialized to false on line 282 and never set to true. The if (!hadErrors) guard on line 306 is therefore always truthy. If the intent was to accumulate errors during batch deletion (lines 293–299), the catch block short-circuits on the first error before hadErrors could be set.
♻️ Suggested cleanup
async function handleDelete() {
showDelete = false;
- let hadErrors = false;
try {
// ... deletion logic ...
await invalidate(Dependencies.DOCUMENTS);
trackEvent(Click.DatabaseRowDelete);
- if (!hadErrors) {
- // error is already shown above!
- addNotification({
- type: 'success',
- message: `${selectedDocuments.length ? selectedDocuments.length : 1} document${selectedDocuments.length > 1 ? 's' : ''} deleted`
- });
- }
+ addNotification({
+ type: 'success',
+ message: `${selectedDocuments.length ? selectedDocuments.length : 1} document${selectedDocuments.length > 1 ? 's' : ''} deleted`
+ });🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/spreadsheet.svelte
around lines 280 - 329, hadErrors is unused dead code in handleDelete — remove
it and its conditional guard instead of keeping a flag that's never set; delete
the hadErrors declaration and the surrounding if (!hadErrors) { ... } block
(which wraps addNotification success), leaving the success notification in the
try path (so success is shown only when no exception occurs), and keep the
existing catch which already shows errors and short-circuits; update references
to hadErrors (if any) accordingly.
1 participant
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Improvements