Feat: multi-terminology support #2413
Conversation
ConsoleProject ID: Sites (2)
Note Appwrite has a Discord community with over 16 000 members. |
WalkthroughConverts many table-specific surfaces to a generic entity model: renames create/store symbols (showCreateTable → showCreateEntity), replaces "columns" with "fields", and introduces a centralized public entity module at $database/(entity) (helpers, views, layouts, settings). Adds terminology, analytics/dependencies helpers, a DocumentsDB entry on the SDK surface, a pluralization helper, navigation utilities, and a SvelteKit alias ($database). Adjusts analytics enums and guards, updates numerous routes/components/stores to use Entity/Field types, and adds/removes multiple UI components and index/backup/settings flows accordingly. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
...le)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
Show resolved
Hide resolved
update: make `createTable` as a reusable component like `createEntity`.
![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
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte (1)
32-32: Consider addressing the TODO comment.Line 32 notes uncertainty about whether these should be "attributes or columns." With the new terminology system in place, this might be a good opportunity to clarify and use the appropriate terminology-driven label.
Would you like me to help generate a solution that uses terminology for this field label, or open an issue to track this refinement?
Also applies to: 38-38
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte (1)
17-17: Address the TODO comment.The TODO indicates that this component may need to handle
documentSecurityin addition torowSecurity. Please clarify whether this is planned for a future update or should be implemented now.Do you want me to open a new issue to track this task, or is this already covered by existing plans?
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts (1)
20-22: Consider error handling for missing context.If
getTerminologies()is called beforesetTerminologies()has been invoked,getContextwill returnundefined, potentially causing runtime errors in consuming components.Consider adding a guard to provide a helpful error message:
export function getTerminologies(): Terminologies { - return getContext<Terminologies>(TERMINOLOGIES_KEY); + const terminologies = getContext<Terminologies>(TERMINOLOGIES_KEY); + if (!terminologies) { + throw new Error('Terminologies context not initialized. Call setTerminologies() first.'); + } + return terminologies; }src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (1)
67-67: Remove or document commented code.The commented
orderscolumn should either be removed if not needed or documented with a reason if it's planned for future implementation.Apply this diff to remove the commented line:
{ id: 'columns' }, - // { id: 'orders' }, // design doesn't have orders atm { id: 'lengths' },Or document why it's commented:
- // { id: 'orders' }, // design doesn't have orders atm + // TODO: Add 'orders' column once design is finalized (see issue #XXXX) + // { id: 'orders' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (66)
src/lib/actions/analytics.ts(3 hunks)src/lib/components/backupRestoreBox.svelte(2 hunks)src/lib/helpers/string.ts(1 hunks)src/lib/stores/navigation.ts(1 hunks)src/lib/stores/sdk.ts(2 hunks)src/routes/(console)/project-[region]-[project]/auth/security/updateSessionInvalidation.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/dependencies.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/breadcrumbs.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte(6 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/failedModal.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/field/csvDisabled.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/field/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte(9 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/select.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/empty.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/sidesheet.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/spreadsheet.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/store.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/danger.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/name.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/permissions.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/status.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/indexes.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/options.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/activity/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/breadcrumbs.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/createColumnDropdown.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/csvDisabled.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/failedModal.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/dangerZone.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/deleteTable.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateName.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updatePermissions.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateSecurity.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateStatus.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/usage/[[period]]/+page.svelte(1 hunks)svelte.config.js(1 hunks)
💤 Files with no reviewable changes (9)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/breadcrumbs.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateName.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/deleteTable.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/csvDisabled.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateStatus.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updatePermissions.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/dangerZone.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateSecurity.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/failedModal.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T04:33:19.632Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/createTable.svelte:28-33
Timestamp: 2025-09-25T04:33:19.632Z
Learning: In the Appwrite console createTable component, manual submit state management (like setting creatingTable = true) is not needed because: 1) The Modal component handles submit state internally via submissionLoader, preventing double submissions, and 2) After successful table creation, the entire view is destroyed and replaced with the populated table view, ending the component lifecycle.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte
🧬 Code graph analysis (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
Columns(9-21)src/lib/helpers/string.ts (3)
singular(20-22)plural(37-59)capitalize(8-10)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
Term(11-11)TerminologyShape(5-9)TerminologyResult(13-22)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/dependencies.ts (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
TerminologyResult(13-22)DependenciesResult(61-66)Term(11-11)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
useTerminology(59-68)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
TerminologyResult(13-22)AnalyticsResult(47-58)TerminologyShape(5-9)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
useTerminology(59-68)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
AnalyticsResult(47-58)TerminologyResult(13-22)DependenciesResult(61-66)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
useTerminology(59-68)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts (1)
useAnalytics(8-51)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/dependencies.ts (1)
useDependencies(8-24)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
baseTerminology(14-30)
🔇 Additional comments (63)
src/lib/components/backupRestoreBox.svelte (2)
60-62: LGTM! Clear variable naming.The destructuring with aliasing (
$collection: type) improves readability within the function scope. The comment helpfully clarifies that the payload structure still uses$collectionas an operation type identifier.
63-85: Consistent refactor throughout.All references to the renamed
typevariable are updated correctly:
- Early return guard (line 63)
- Type membership check (line 67)
- Map access (lines 68, 85)
- Type comparison (line 75)
The logic and behavior are preserved.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/usage/[[period]]/+page.svelte (4)
2-3: LGTM! Imports align with entity terminology refactor.The updated imports correctly use the PageProps type for type safety and source the Usage component from the new centralized '$database/(entity)' location, which aligns with the PR's goal of generalizing from table-specific to entity-based terminology.
5-5: LGTM! Proper use of Svelte 5 runes.The migration from
export let datatolet { data }: PageProps = $props()correctly adopts Svelte 5's runes API while maintaining type safety.
7-8: LGTM! Clean use of $derived for reactive values.The derived constants correctly use Svelte 5's
$derivedrune to compute count and total from the page data. The property access is straightforward and the naming is clear.
11-11: Ignore entity-based route suggestion. The codebase currently usestable-[table]routes and has noentity-routes or imports, so no action needed.Likely an incorrect or invalid review comment.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte (1)
73-75: Header migration looks correct.Props, bindings, and the preference-backed expand state all line up with the new shared component’s API. 👍
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte (1)
13-13: LGTM!The import change correctly adopts the new public module surface for
SideSheet, aligning with the PR's centralization of UI components.src/lib/stores/sdk.ts (2)
24-25: Clarify the "DocumentsDB later" comment.The comment
// DocumentsDB latersuggests this might be incomplete or a placeholder. Please clarify whether this is intended for this PR or if it should be removed or completed before merging.
135-135: Clarify the "documentsDB later" comment.The comment
// documentsDB latersuggests this might be incomplete or a placeholder. Please clarify whether this is intended for this PR or if it should be removed or completed before merging.svelte.config.js (1)
22-24: LGTM!The new
$databasealias is correctly configured and enables the centralized import pattern used throughout the PR.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/options.svelte (1)
5-5: LGTM!The import change correctly adopts the new public module surface for
SideSheet, consistent with the PR's centralization effort.src/lib/helpers/string.ts (1)
24-59: Reasonable implementation for common English pluralization.The function correctly handles common pluralization rules (sibilants, consonant+y, vowel+y, default). Note that it does not handle less common cases like words ending in "f"/"fe" (e.g., "leaf" → "leaves") or irregular plurals (e.g., "child" → "children"), but the documentation accurately describes it as handling "common English pluralization rules."
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/indexes.svelte (1)
12-12: LGTM!The import change correctly adopts the new public module surface for
SideSheet, consistent with the PR's centralization effort.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (4)
43-43: LGTM!The import changes correctly adopt the new public module surface and reflect the terminology shift from "table" to "entity."
Also applies to: 49-49, 66-66
97-99: LGTM!The updated comments improve clarity by documenting when dependency invalidation is skipped during AI-driven index and column creation.
115-115: LGTM!The store name change from
showCreateTabletoshowCreateEntityaligns with the PR's terminology shift.
446-460: LGTM!The
CreateIndexcomponent now accepts a generalizedentityprop and usesexternalFieldKeyinstead ofexternalColumnKey, aligning with the PR's terminology shift. TheonCreateIndexhandler correctly maps the internalfieldsterminology to the API'scolumnsparameter.src/lib/stores/navigation.ts (3)
12-14: LGTM!The
withPathfunction correctly joins path segments and normalizes trailing slashes.
16-21: LGTM!The type casting is necessary to align runtime values with TypeScript's conditional types. The pattern is appropriate for this use case.
23-29: LGTM!The
navigatefunction correctly composesresolveRouteandgoto, providing a convenient navigation helper.src/routes/(console)/project-[region]-[project]/auth/security/updateSessionInvalidation.svelte (1)
25-25: LGTM! Typo fix in analytics enum.The correction from
AuthInvalidateSesssion(three 's') toAuthInvalidateSession(two 's') is correct and aligns with standard spelling.Also applies to: 31-31
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte (1)
5-5: LGTM! Proper terminology integration.The component correctly integrates the terminology system to derive dynamic entity labels.
Also applies to: 13-14
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/field/csvDisabled.svelte (1)
1-16: LGTM! Clean wrapper component.The
CsvDisabledcomponent is well-implemented with proper typing and a clear purpose—wrapping UI elements to indicate they're disabled during CSV import.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/field/index.ts (1)
1-1: LGTM! Proper barrel export pattern.This barrel file correctly re-exports the default export from
csvDisabled.svelteas a named exportCsvDisabled, enabling the named imports used in other files.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte (2)
7-7: LGTM! Proper terminology integration.The component correctly integrates the terminology system to derive display strings and paths.
Also applies to: 18-22
28-28: Verify dynamic property access is safe.
Ensure thatterminology.entity.lower.singular(e.g. "table") corresponds to an actual[table]dynamic segment in your SvelteKit route hierarchy and thatpage.params[entity]always exists when this component renders.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/createColumnDropdown.svelte (1)
7-7: CsvDisabled import matches named export –CsvDisabledis correctly exported from$database/(entity).src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts (1)
5-5: No action needed: Breadcrumbs import is correct Verified thatBreadcrumbsis re-exported as a named export in the entity index.src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts (1)
8-8: AllshowCreateTablereferences removed
No occurrences ofshowCreateTableremain in.tsor.sveltefiles.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (3)
86-86: LGTM: Centralized public export.The import now uses the public module alias
$database/(entity)instead of a local relative path. This aligns with the PR's goal of centralizing entity-related exports.
422-422: Verify analytics event naming consistency. Successful deletes fire Click.DatabaseRowDelete, while errors still log Submit.RowDelete—confirm both align with analytics conventions and downstream consumers expect these distinct event names.
4-4: Analytics import completeness verified. BothClick.DatabaseRowDeleteand theSubmitevents (ColumnDelete,RowUpdate,RowDelete) are exported in the analytics module.src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
7-7: LGTM: Consistent terminology refactor.The store import and usage have been updated from
showCreateTabletoshowCreateEntity, aligning with the PR's goal of generalizing from table-specific to entity-generic terminology. The change is applied consistently at both the import and usage sites.Also applies to: 149-149
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (1)
17-17: LGTM: Public module import pattern.The import now uses a named export from the public
$database/(entity)module instead of a local default import. This centralizes theSpreadsheetContainercomponent and aligns with the PR's public API surface strategy.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/activity/+page.svelte (1)
5-9: LGTM: Svelte 5 runes pattern.The component now uses the
$props()rune to destructure thedataprop instead ofexport let data. This is consistent with Svelte 5's runes API and modern component patterns. The data is still properly typed asPageDataand passed to theActivitycomponent.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/status.svelte (2)
1-40: LGTM: Well-structured status component.The component properly:
- Uses Svelte 5 runes (
$props(),$state()) for reactive state management- Integrates with the terminology system via
getTerminologies()for dynamic analytics and dependency keys- Implements proper error handling with both success and error notification paths
- Tracks analytics events using terminology-based event names
- Invalidates the correct dependency after successful updates
The cleanup flow (lines 22-29) and error handling (lines 35-37) follow best practices.
58-58: Verify button disabled logic.The Update button is disabled when
enabled === entity.enabled. Ensure this comparison works correctly for boolean values and that the button becomes enabled when the user toggles the switch to a different value than the entity's current status.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte (2)
52-63: LGTM: Proper collapsed state handling.The tabs container correctly handles the collapsed state by:
- Transitioning opacity smoothly (300ms cubic-bezier)
- Disabling pointer events when collapsed
- Changing cursor to default for collapsed tabs
This provides good UX and prevents interaction with hidden tabs.
30-34: No action needed: dynamic param names align with entity types. All routes use bracketed segments named after the lowercase singular entity (e.g.[table],[collection]), matchingentityTypefromuseTerminology.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/index.ts (1)
1-3: LGTM: Clean barrel exports.This barrel file properly consolidates the layout components (
EmptySheet,SideSheet,SpreadsheetContainer) into a public API surface, enabling centralized imports via the$database/(entity)alias throughout the codebase.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/index.ts (1)
1-2: LGTM: Proper type and component exports.The barrel correctly exports both the
Indexesview component and theCreateIndexcomponent along with its associatedCreateIndexesCallbackTypetype. This provides a complete public API surface for index management functionality.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte (2)
46-46: LGTM: Import consolidation.The consolidation of multiple component imports into a single import from
$database/(entity)improves maintainability and aligns with the new public API surface.
482-482: LGTM: Improved modal rendering pattern.The change from conditional rendering to always rendering the FailedModal with
bind:showis cleaner and avoids unnecessary component mounting/unmounting cycles. The title update to "Create column" is consistent with the terminology refactor.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (3)
19-20: LGTM: Import and export updates.The addition of
columnsOrderto the exports and the consolidation ofEmptySheetimport from the public$database/(entity)path improve the module's API surface and maintainability.Also applies to: 35-36
54-56: LGTM: Icon derivation for columns.The addition of the
iconproperty derived fromcolumnOptionsensures consistent iconography across the UI and aligns with the enhanced column type surface.
236-241: LGTM: Enhanced empty state handler.The
onOpenCreateColumnhandler provides a cleaner API for opening the create column sheet with proper initialization. The explicit setting of title, columns, and columnsOrder ensures the sheet has all necessary context.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts (1)
28-35: LGTM: Clean composition pattern.The
buildTerminologiesfunction cleanly composes the terminology, analytics, and dependencies helpers into a cohesive object. This centralization simplifies consumption across components.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (2)
1-89: LGTM: Well-structured indexes view.The component demonstrates good practices:
- Proper state management for modals and selection
- Permission checks before allowing index creation
- Status badge handling with error details
- Analytics tracking on user actions
- Responsive empty states via snippets
222-242: LGTM: Proper bulk selection UI.The floating action bar implementation correctly handles bulk selection state, provides clear feedback on the number of selected items, and offers cancel/delete actions.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/permissions.svelte (2)
1-67: LGTM: Well-implemented permissions view.The component demonstrates solid implementation:
- Proper change detection using
symmetricDifference- Clean separation between update logic and cleanup
- Appropriate error handling with notifications and analytics
- Terminology-driven UI labels for consistency
- Disabled state when no changes to prevent unnecessary API calls
46-48: LGTM: Efficient change detection.The use of
symmetricDifferenceto detect permission changes is an efficient and correct approach for comparing arrays. The button will only be enabled when there are actual differences between the current and new permissions.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/breadcrumbs.svelte (1)
8-13: Verify dynamic parameter access
entityIdis derived viapage.params[entityType]; ensure everyterminology.entity.lower.singularvalue (e.g."table","view") corresponds to an existing[<entityType>]route segment under
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/…
so thatpage.params[entityType]never returnsundefined.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte (10)
5-14: LGTM!The import refactoring aligns well with the generalized entity approach. The use of
$databasealias and the newuseTerminologyhelper support the multi-terminology feature.
26-31: LGTM!The dynamic entity terminology derivation using
useTerminology(page)is well-implemented and supports the multi-terminology feature.
34-37: LGTM!State variable renaming from
creatingTabletocreatingEntityaligns with the generalization to support multiple entity types.
39-53: LGTM!The
enableThinkingModeForSuggestionsfunction correctly guards against enabling thinking mode whenuseSuggestionsis false, and the parameters are appropriately generalized.
72-85: Reset logic looks good.The cleanup logic correctly resets state variables and updates the navigation. The chained assignment on lines 83-84 is clear and appropriate.
Note: The analytics event on line 80 has the same potential naming issue as mentioned in the previous comment.
98-109: LGTM!The effect hook correctly manages auto-ID generation and state cleanup when the modal is closed.
111-119: LGTM!The effect hook correctly resets the suggestions table state when navigating to an entity page with the modal open.
122-157: LGTM!The modal markup correctly uses dynamic entity terminology and conditional rendering for suggestions. The button states are properly tied to
creatingEntityfor UI feedback, while the Modal component handles submission state internally to prevent double submissions.Based on learnings.
16-24: All CreateEntity call sites updated Verified that every<CreateEntity>invocation binds the newshowprop and usesonCreateEntity; no further changes needed.
55-70: No analytics event update needed
Submit.TableCreateremains the correct enum entry for this operation; no changes required.
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte
Outdated
Show resolved
Hide resolved
...oject-[region]-[project]/databases/database-[database]/(entity)/views/settings/danger.svelte
Show resolved
Hide resolved
...project-[region]-[project]/databases/database-[database]/table-[table]/settings/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte(1 hunks)
🔇 Additional comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte (1)
28-56: Clean delegation pattern and terminology-based invalidation.The refactor to delegate deletion via
onDeleteIndexesand use terminology-based dependencies (dependencies.entity.singularat line 45) aligns well with the PR's multi-terminology goals. Thecleanup()function properly resets state, tracks events, shows notifications, and invalidates the correct dependency.
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte (3)
113-121: Clarify the comment about not checking entity type.The comment on line 114 states "we don't have to check for entity type!" but doesn't explain why this is safe or under what conditions this assumption holds. Adding context would help future maintainers understand whether this logic needs to be revisited when adding new entity types.
64-64: Consider using nullish coalescing operator for cleaner code.The ternary expression
id ? id : ID.unique()can be simplified using the nullish coalescing operator for better readability.Apply this diff:
- await onCreateEntity(id ? id : ID.unique(), name); + await onCreateEntity(id ?? ID.unique(), name);
85-85: Consider separating chained assignment for clarity.The chained assignment
name = id = nullis valid but may be less readable. Consider splitting it into separate statements.Apply this diff:
- name = id = null; + name = null; + id = null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/actions/analytics.ts(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T04:33:19.632Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/createTable.svelte:28-33
Timestamp: 2025-09-25T04:33:19.632Z
Learning: In the Appwrite console createTable component, manual submit state management (like setting creatingTable = true) is not needed because: 1) The Modal component handles submit state internally via submissionLoader, preventing double submissions, and 2) After successful table creation, the entire view is destroyed and replaced with the populated table view, ending the component lifecycle.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte
...nsole)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte
Show resolved
Hide resolved
...nsole)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
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]/(entity)/views/indexes/create.svelte (1)
78-89: Spatial order must be NONE; don’t require order for Spatial.Prevent ASC/DESC for Spatial and avoid failing required validation.
- let orderOptions = $derived.by(() => - selectedType === IndexType.Spatial - ? [ - { value: 'ASC', label: 'ASC' }, - { value: 'DESC', label: 'DESC' }, - { value: null, label: 'NONE' } - ] - : [ - { value: 'ASC', label: 'ASC' }, - { value: 'DESC', label: 'DESC' } - ] - ); + let orderOptions = $derived.by(() => + selectedType === IndexType.Spatial + ? [{ value: null, label: 'NONE' }] + : [ + { value: 'ASC', label: 'ASC' }, + { value: 'DESC', label: 'DESC' } + ] + );- <InputSelect + <InputSelect options={orderOptions} - required + required={selectedType !== IndexType.Spatial} id={`order-${index}`} label={index === 0 ? 'Order' : undefined} bind:value={field.order} placeholder="Select order" />Also applies to: 258-265
♻️ Duplicate comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (1)
59-59: Add type annotation for selectedIndexes.The variable
selectedIndexeslacks a type annotation and is bound toselectedRowsin the Spreadsheet component (line 195), which expectsstring[]for row IDs.Apply this diff to add the type annotation:
- let selectedIndexes = $state([]); + let selectedIndexes = $state<string[]>([]);src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte (4)
32-38: Prop default/type alignment for externalFieldKey — resolved.Setting default to null and typing as string | null fixes the mismatch. LGTM.
2-8: Fixlengthstype: payload emits nulls but type is number[].
lengthsis constructed with nulls in create(); type it accordingly.export type CreateIndexesCallbackType = { key: string; type: IndexType; fields: string[]; - lengths: number[]; + lengths: Array<number | null>; orders: string[]; };
91-97: Always collapse to a single row on Spatial.Current effect only collapses when first value is empty; ensure a single row consistently.
- if (selectedType === IndexType.Spatial && !fieldList.at(0).value) { - fieldList = [{ value: '', order: null, length: null }]; - } + if (selectedType === IndexType.Spatial) { + fieldList = [{ value: fieldList.at(0)?.value ?? '', order: null, length: null }]; + }
154-176: Harden validation and build payload deterministically (avoid 'undefined' orders, ensure Spatial orders = []).Validate all rows; ensure orders array aligns with fields; normalize lengths.
- const fieldType = terminology.field.lower.singular; - - if (!key || !selectedType || (selectedType !== IndexType.Spatial && addFieldDisabled)) { + const fieldType = terminology.field.lower.singular; + // Validate all rows, not just the last one + const invalidRow = + fieldList.some((f) => !f.value) || + (selectedType !== IndexType.Spatial && + fieldList.some( + (f) => f.order !== IndexOrder.ASC && f.order !== IndexOrder.DESC + )); + if (!key || !selectedType || invalidRow) { addNotification({ type: 'error', - message: `Selected ${fieldType} key or type invalid` + message: `Selected ${fieldType} key, type, or order invalid` }); - throw new Error(`Selected ${fieldType} key or type invalid`); + throw new Error(`Selected ${fieldType} key, type, or order invalid`); } @@ - const orders = fieldList - .map((field) => field.order) - .filter((order: IndexOrder) => order !== null) - .map((order) => String(order)); - - await onCreateIndex({ - key, - type: selectedType, - fields: fieldList.map((a) => a.value), - lengths: fieldList.map((a) => (a.length ? Number(a.length) : null)), - orders: orders.length ? orders : [] - }); + const fields = fieldList.map((f) => f.value); + const lengths = fieldList.map((f) => + f.length != null && f.length !== '' ? Number(f.length) : null + ); + const orders = + selectedType === IndexType.Spatial + ? [] + : fieldList.map((f) => (f.order === IndexOrder.DESC ? 'DESC' : 'ASC')); + + await onCreateIndex({ key, type: selectedType, fields, lengths, orders });
🧹 Nitpick comments (7)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (1)
175-175: Use Click enum for analytics event.The create button uses a string literal
"create_index"for the event, while the delete action (line 269) correctly uses theClick.DatabaseIndexDeleteenum. Using the enum consistently improves type safety and maintainability.Apply this diff if a corresponding enum value exists:
<Button secondary - event="create_index" + event={Click.DatabaseIndexCreate} disabled={!entity.fields?.length} on:click={() => (showCreateIndex = true)}>Alternatively, verify if the event should use
Click.DatabaseIndexCreateor a similar enum value from the analytics module.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte (2)
56-61: Preference key is table-specific; make it terminology-drivenUse a dynamic key so expansion state doesn’t collide across terminologies.
- $effect(() => { - if (nonSheetPages) expandTabs.set(true); - else { - expandTabs.set(preferences.getKey('tableHeaderExpanded', true)); - } - }); + $effect(() => { + if (nonSheetPages) { + expandTabs.set(true); + } else { + const prefKey = `${terminology.entity.lower.singular}HeaderExpanded`; + expandTabs.set(preferences.getKey(prefKey, true)); + } + });
11-13: Decouple from table store pathImporting
expandTabsfrom'$database/table-[table]/store'couples the generic entity Header to the table implementation. Move this to an entity-level store (e.g.,'$database/(entity)/store') to align with multi-terminology.- import { expandTabs } from '$database/table-[table]/store'; + import { expandTabs } from '$database/(entity)/store'; // create/centralize if not presentsrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte (4)
19-19: Prefer entity-centric imports over table‑prefixed paths.To align with the new terminology, consider moving/aliasing helpers to an entity path (e.g., $database/(entity)/rows/store and /fields/store) to avoid mixed “table” imports.
Also applies to: 26-27
110-122: Avoid duplicate key assignment.initialize() sets key, then it’s overwritten by generateIndexKey() in the effect. Keep a single source of truth.
function initialize() { - const field = entity.fields.filter((field) => externalFieldKey === field.key); + const field = entity.fields.filter((field) => externalFieldKey === field.key); const isSpatial = field.length && isSpatialType(field[0]); const order = isSpatial ? null : IndexOrder.ASC; @@ - key = `index_${entity.indexes.length + 1}`; + // key is set by generateIndexKey() in the open effect }Also applies to: 141-147
111-114: Use find() instead of filter()[0] for single lookup.Simplifies intent and avoids array handling.
-const field = entity.fields.filter((field) => externalFieldKey === field.key); -const isSpatial = field.length && isSpatialType(field[0]); +const field = entity.fields.find((f) => externalFieldKey === f.key); +const isSpatial = !!field && isSpatialType(field);
198-201: Safer error message extraction.Handle non-Error throwables.
- addNotification({ - type: 'error', - message: err.message - }); + addNotification({ + type: 'error', + message: err instanceof Error ? err.message : String(err) + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte(8 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
PR: appwrite/console#2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte (1)
22-33: No issues with dynamic param access. Thepage.params[entity]pattern mirrors the one inheader.svelte, which is already vetted and confirms the route-param keys align with your terminology strings.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte (1)
6-21: Clean handoff to public Header; navigation and tabs look correctGood use of
$app/staterunes, resolved paths, and permission-filtered tabs. The refactor meaningfully simplifies this route.Also applies to: 24-62, 64-64
...nsole)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Show resolved
Hide resolved
.../project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte
Outdated
Show resolved
Hide resolved
.../project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte
Show resolved
Hide resolved
.../project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
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]/settings/+page.svelte (1)
50-55: Guard againstnullbefore updating the database name.
databaseNameis now typed asstring | null, but it is passed straight intotablesDB.update, whosenameparameter expects astring. This will fail TypeScript checks (and could sendnullat runtime if the form is ever submitted while empty). Add an early return that validatesdatabaseNamebefore calling the SDK so the type is narrowed tostring.async function updateName() { + if (!databaseName) { + addError('name', 'Name is required', 'error'); + return; + } try { await sdk.forProject(page.params.region, page.params.project).tablesDB.update({ databaseId: page.params.database, name: databaseName });
♻️ Duplicate comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
20-20: Error type inconsistency remains unfixed.The
errorvariable is still initialized asnullbut assigned boolean values (trueon line 68,falseon line 108), causing type inconsistency. This issue was flagged in a previous review but has not been addressed.Apply one of these fixes:
Option 1 (simpler): Use boolean throughout:
-let error = $state(null); +let error = $state(false);Option 2 (more informative): Use error messages:
-let error = $state(null); +let error = $state<string | null>(null); // Then in catch block (line 68): -error = true; +error = err.message || 'Failed to load tables';Also applies to: 68-68, 108-108
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (1)
59-59: Restore explicit typing forselectedIndexes.Without an explicit generic,
$state([])infersnever[], which clashes with the Spreadsheet bindings expecting string IDs and re-introduces the type error flagged in the earlier review. Please add the concrete element type.-let selectedIndexes = $state([]); +let selectedIndexes = $state<string[]>([]);src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte (1)
2-8: FixCreateIndexesCallbackType.lengthsto allow nullable entries.
create()buildslengthswithnullvalues for fields without a length, so the currentnumber[]annotation misrepresents the payload and breaks type-checking. Please align the type definition with the emitted data.export type CreateIndexesCallbackType = { key: string; type: IndexType; fields: string[]; - lengths: number[]; + lengths: Array<number | null>; orders: string[]; };src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
254-256: Guard table.fields before .some()/.filter()table.fields may be undefined; calling array methods will throw. Use a safe default.
- let columns = table.fields; - const hasAnyRelationships = columns.some((column: Columns) => isRelationship(column)); - const filteredColumns = columns.filter((col: Columns) => col.type !== 'relationship'); + let columns = table?.fields ?? []; + const hasAnyRelationships = + columns?.some((column: Columns) => isRelationship(column)) ?? false; + const filteredColumns = (columns ?? []).filter( + (col: Columns) => col.type !== 'relationship' + );
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte (1)
17-20: Consider destructuring terminology properties for clarity.While the current approach works, destructuring the specific terminology properties could make the dependencies more explicit and slightly improve readability.
Apply this diff if you prefer more explicit destructuring:
- const { terminology } = getTerminologies(); - - const records = terminology.record.lower.plural; - const entity = terminology.entity.lower.singular; + const { terminology } = getTerminologies(); + const { record: { lower: { plural: records } }, entity: { lower: { singular: entity } } } = terminology;Or alternatively, use intermediate destructuring:
const { terminology } = getTerminologies(); - const records = terminology.record.lower.plural; - const entity = terminology.entity.lower.singular; + const { record, entity: entityTerm } = terminology; + const records = record.lower.plural; + const entity = entityTerm.lower.singular;src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte (1)
6-7: Use withPath for subpaths (consistency with navigation helpers)Currently sub-tabs build hrefs via string concat. Prefer withPath for consistency and to avoid double slashes.
- import { type Entity, Header } from '$database/(entity)'; + import { type Entity, Header } from '$database/(entity)'; + import { withPath } from '$lib/stores/navigation'; @@ [ { - href: path, + href: path, title: 'Rows', event: 'rows', hasChildren: true }, { - href: `${path}/columns`, + href: withPath(path, '/columns'), title: 'Columns', event: 'columns' }, { - href: `${path}/indexes`, + href: withPath(path, '/indexes'), title: 'Indexes', event: 'indexes' }, { - href: `${path}/activity`, + href: withPath(path, '/activity'), title: 'Activity', event: 'activity', hasChildren: true }, { - href: `${path}/usage`, + href: withPath(path, '/usage'), title: 'Usage', event: 'usage', hasChildren: true }, { - href: `${path}/settings`, + href: withPath(path, '/settings'), title: 'Settings', event: 'settings', disabled: !$canWriteTables }Also applies to: 25-56
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
124-146: Optional: use route ids instead of pathname for command guardsYou already noted this. Using page.route.id avoids false positives with trailing slashes/hash.
Minimal example:
- disabled: page.url.pathname.endsWith(table?.$id), + disabled: page.route.id?.endsWith('table-[table]') @@ - disabled: page.url.pathname.endsWith('columns'), + disabled: page.route.id?.endsWith('table-[table]/columns') @@ - disabled: page.url.pathname.endsWith('indexes'), + disabled: page.route.id?.endsWith('table-[table]/indexes') @@ - disabled: page.url.pathname.endsWith('activity'), + disabled: page.route.id?.endsWith('table-[table]/activity') @@ - disabled: page.url.pathname.endsWith('usage'), + disabled: page.route.id?.endsWith('table-[table]/usage') @@ - disabled: page.url.pathname.endsWith('settings') || !$canWriteTables, + disabled: page.route.id?.endsWith('table-[table]/settings') || !$canWriteTables,Also applies to: 149-156, 158-165, 167-174, 176-183, 185-192
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte(8 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/table.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(18 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
PR: appwrite/console#2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (10)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/table.svelte (5)
44-50: LGTM!The migration to
$props()and$derivedfor consuming page data is correct and aligns with Svelte 5 patterns.
52-66: Past issue resolved!The
newDatabaseInfotype now correctly allowsnullfor both fields, fixing the TypeScript strict null check issue flagged in the previous review.
98-98: LGTM!Adding
awaittoinvalidate()calls ensures data refresh completes before UI state reset, preventing potential race conditions.Also applies to: 131-131
167-173: LGTM!The
disableButtonderived logic correctly validates both restore options: requiring a name for new databases and preventing ID conflicts, while requiring explicit confirmation for same-database overwrites.
113-114: LGTM!The
$effectcleanup hook and consistent use of the deriveddatabaseobject throughout the file (replacing previous$databasestore references) align well with the Svelte 5 migration pattern.Also applies to: 175-184, 374-374, 384-384
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
103-111: Cleanup effect properly resets all state.The cleanup effect now correctly resets
confirmedDeletionanderrorwhen the modal closes, addressing the previous review concern. This ensures the modal state is fresh each time it opens.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte (1)
2-2: Correct use of SvelteKit 5 page stateUsing page from $app/state directly (page.params, page.route.id) is right for SvelteKit 5.
Based on learnings
Also applies to: 36-36
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte (1)
8-20: LGTM: route resolution and Header integrationresolveRoute with page.params and wiring Header with tabs/table/link look correct.
Based on learnings
Also applies to: 22-23, 64-64
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (2)
443-455: Good: invalidate table after index creationRefreshing Dependencies.TABLE post-create prevents stale UI.
439-455: Skip decoupling for Header; no external usages
Header is only imported in table’s header.svelte (line 64) and not used elsewhere; coupling concerns are non-applicable.Likely an incorrect or invalid review comment.
...nsole)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte
Show resolved
Hide resolved
...nsole)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte
Show resolved
Hide resolved
...nsole)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Show resolved
Hide resolved
.../project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte
Show resolved
Hide resolved
...e)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte
Show resolved
Hide resolved
...nsole)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
102-117: Guard unknown database types to avoid silent empty terminologyAdd explicit validation instead of falling back to {}.
export function useTerminology(page: Page): TerminologyResult { const type = page.data?.database?.type as DatabaseType; if (!type) { // strict check because this should always be available! throw new AppwriteException('Database type is required', 500); } - const dbTerminologies = terminologyData[type] || {}; + if (!(type in terminologyData)) { + throw new AppwriteException(`Unknown database type: ${String(type)}`, 500); + } + const dbTerminologies = terminologyData[type]; return { type, source: dbTerminologies, field: dbTerminologies.field, record: dbTerminologies.record, entity: dbTerminologies.entity }; }Based on learnings
🧹 Nitpick comments (10)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte (2)
36-51: Add error handling toloadEntityCount.The function lacks error handling for API failures. If
listTablesorlistCollectionsthrows, the error will propagate unhandled.Consider wrapping the switch statement in a try-catch block:
async function loadEntityCount() { const params = { databaseId: database.$id, queries: [Query.limit(1)] }; const projectSdk = sdk.forProject(page.params.region, page.params.project); + try { switch (terminology.type) { case 'tablesdb': { const { total } = await projectSdk.tablesDB.listTables(params); return total; } case 'documentsdb': { const { total } = await projectSdk.documentsDB.listCollections(params); return total; } default: return 0; } + } catch (error) { + console.error('Failed to load entity count:', error); + return 0; + } }
129-135: Add error handling to the await block.The template's
{#await}block lacks a{:catch}clause. IfloadEntityCount()fails, users will see a skeleton that never resolves.Add error handling to gracefully handle failures:
{#await loadEntityCount()} <Skeleton variant="line" width="100%" height={19.5} /> {:then count} {@const entity = terminology.entity.title} {count} {count === 1 ? entity.singular : entity.plural} +{:catch error} + <span class="u-color-text-danger">Failed to load count</span> {/await}src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
82-98: Add error handling toloadEntities.Similar to
loadEntityCountin settings, this function lacks error handling for API failures. Unhandled errors could break the navigation UI.Wrap the API calls in a try-catch block:
async function loadEntities() { const projectSdk = sdk.forProject(region, project); const params = { databaseId, queries: [Query.orderDesc(''), Query.limit(100)] }; + try { switch (terminology.type) { case 'tablesdb': { const { total, tables } = await projectSdk.tablesDB.listTables(params); entities = { total, entities: tables.map(toSupportiveEntity) }; break; } case 'documentsdb': { const { total, collections } = await projectSdk.documentsDB.listCollections(params); entities = { total, entities: collections.map(toSupportiveEntity) }; break; } } + } catch (error) { + console.error('Failed to load entities:', error); + entities = { total: 0, entities: [] }; + } }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table.svelte (4)
113-118: Use terminology-aware title in the confirmation modalHardcoded “Delete tables”. Make it dynamic.
-<Confirm title="Delete tables" bind:open={showDelete} onSubmit={handleDelete} disabled={deleting}> +<Confirm title={`Delete ${entityPlural}`} bind:open={showDelete} onSubmit={handleDelete} disabled={deleting}>
27-33: Nit: rename selection state for clarityVariable still named selectedTables while rendering generic entities.
-let selectedTables: string[] = $state([]); +let selectedEntities: string[] = $state([]); ... -bind:selectedRows={selectedTables} +bind:selectedRows={selectedEntities} ... -const promises = selectedTables.map((id) => { /* ... */ }) +const promises = selectedEntities.map((id) => { /* ... */ })
37-43: Branch delete by database type and manage loading state
- Add a
deletingreactive boolean and set it true before and false after- Replace flat
tablesDB.deleteTablecalls withswitch (terminology.type) { case 'tablesdb': return projectSdk.tablesDB.deleteTable(params); case 'documentsdb': return projectSdk.documentsDB.delete(params); default: return Promise.reject(new Error(`Unsupported database type: ${terminology.type}`)); }- Use
entitySingular/entityPluralin success message instead of hard-coded “table(s)”
45-51: Use terminology-driven dependency invalidation
Replace the hard-codedDependencies.TABLESwith the plural from your entity terminology:- await invalidate(Dependencies.TABLES); + import { getTerminologies } from '$database/(entity)'; + const { dependencies } = getTerminologies(); + await invalidate(dependencies.entity.plural);src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
62-67: Make DependenciesResult tolerant of optional termsField/record are optional in TerminologyResult; mirror that with Partial mapping.
-export type DependenciesResult = { - [K in keyof Omit<TerminologyResult, 'source' | 'type'>]: { - singular: Dependencies; - plural: Dependencies; - }; -}; +export type DependenciesResult = Partial< + Record< + keyof Omit<TerminologyResult, 'source' | 'type'>, + { singular: Dependencies; plural: Dependencies } + > +>;src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (2)
141-146: Use terminology-aware labels (“entities”) instead of “tables” in UIReplace hardcoded “tables” in text and headers.
-{#if entityItems.length > 0} - The following tables and all data associated with <b>{database.name}</b>, will be +{#if entityItems.length > 0} + The following {terminology.entity.lower.plural} and all data associated with <b>{database.name}</b>, will be ... -<Table.Header.Cell {root}>Table</Table.Header.Cell> +<Table.Header.Cell {root}>{terminology.entity.title.singular}</Table.Header.Cell>Also applies to: 160-165
176-179: Use Svelte event syntax on:click instead of DOM onclickConsistency with Svelte patterns and SSR.
-<button class="u-underline" onclick={listTables} type="button"> +<button class="u-underline" on:click={listTables} type="button"> ... -<button - class="u-underline" - onclick={() => { +<button + class="u-underline" + on:click={() => { entityItems = entityItems.slice(0, 3); }} type="button">Also applies to: 185-191
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte(6 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/grid.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte(12 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table.svelte(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
PR: appwrite/console#2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts
🧬 Code graph analysis (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts (1)
load(8-24)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
EntityList(24-27)DatabaseType(8-8)toSupportiveEntity(83-96)src/lib/stores/database.ts (1)
database(11-11)src/lib/stores/sdk.ts (1)
sdk(149-172)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (2)
Table(23-25)Columns(9-21)src/lib/helpers/string.ts (3)
singular(20-22)plural(37-65)capitalize(8-10)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
Term(11-11)TerminologyShape(5-9)TerminologyResult(13-23)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts (1)
src/lib/stores/navigation.ts (2)
withPath(12-21)resolveRoute(23-28)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (2)
DatabaseType(8-8)baseTerminology(29-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (10)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte (2)
7-7: LGTM: Dynamic terminology integration aligns with PR goals.The integration of dynamic terminology via
getTerminologies()successfully replaces hardcoded strings with configurable terms, supporting the multi-terminology feature. The conditional rendering based onfeatureActiveappropriately adapts messaging for cloud vs. non-cloud environments.Also applies to: 24-25, 30-31
18-20: Guard against optionalfieldterminology
For types likevectordb,terminology.fieldcan be undefined, soterminology.field.lowerwill throw. Use optional chaining (e.g.terminology.field?.lower) or supply a default before destructuring.⛔ Skipped due to learnings
Learnt from: ItzNotABug PR: appwrite/console#2413 File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28 Timestamp: 2025-10-07T14:17:11.597Z Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts (1)
39-47: LGTM! Clean route construction.The
buildEntityRoutefunction provides a centralized way to build entity-specific routes. The implementation correctly usesresolveRoutefor the base path andwithPathto append the entity segment, ensuring consistent route construction across the database UI.src/routes/(console)/project-[region]-[project]/databases/database-[database]/grid.svelte (2)
21-24: Acknowledge the bindable workaround.The
onMounthack silences the "declared but never read" warning for the write-onlyshowCreatebindable prop. This is a known Svelte 5 limitation when a bindable is only written to but never read within the component.While this workaround is acceptable for now, consider tracking this as a temporary solution that can be removed once Svelte improves its bindable prop analysis.
29-42: LGTM! Terminology-driven rendering.The grid correctly uses the new entity-based data model and terminology system. The migration from
data.tablestodata.entitiesand the use ofbuildEntityRoutefor link construction align well with the PR's entity-first approach.src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte (1)
81-152: LGTM! Proper use of derived state and terminology.The template correctly uses the derived
databaseobject and terminology-driven labels. The update and delete flows are well-structured and align with the entity-first model.src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
42-49: LGTM! Comprehensive entity-based refactor.The migration from tables to entities is well-executed. The use of terminology-driven labels,
buildEntityRoutefor navigation, and the derived route patterns all align with the PR's objectives. The dynamic param access at line 49 is acceptable given the controlled terminology system.Also applies to: 75-80, 125-236
src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte (1)
13-51: LGTM! Excellent use of derived routes and terminology.The refactor successfully eliminates hard-coded path strings in favor of
resolveRouteandwithPath. The derivedtabsarray dynamically adapts to the entity type via terminology, and the consistent use of navigation helpers improves maintainability.src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
86-99: Remove incorrect database deletion suggestion
The code intentionally usesprojectSdk.tablesDB.deletefor SQL databases andprojectSdk.documentsDB.deletefor document databases; no changes needed.Likely an incorrect or invalid review comment.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts (1)
10-10: Reject terminology-driven refactor here—getTerminologies() (Svelte getContext) isn’t available in +page.ts.Likely an incorrect or invalid review comment.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte
Show resolved
Hide resolved
...utes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table.svelte (2)
37-60: Fix delete API for non-table databases.Line 38 still calls
tablesDB.deleteTable(...)even when the active database is a documents-backed one (terminology.type === 'documentsdb'or'legacy'). In those cases the SDK call 404s and nothing gets removed. Branch on the database type and invoke the matching delete endpoint before awaiting the promises.- const promises = selectedTables.map((tableId) => - sdk.forProject(page.params.region, page.params.project).tablesDB.deleteTable({ - databaseId: page.params.database, - tableId - }) - ); + const projectSdk = sdk.forProject(page.params.region, page.params.project); + const promises = selectedTables.map((entityId) => { + if (terminology.type === 'tablesdb') { + return projectSdk.tablesDB.deleteTable({ + databaseId: page.params.database, + tableId: entityId + }); + } + + return projectSdk.documentsDB.deleteCollection({ + databaseId: page.params.database, + collectionId: entityId + }); + });
46-49: Align delete messaging with terminology.The success toast (Line 48) and confirmation title (Line 113) still hard-code “table(s)”. When the database is serving collections those strings are wrong. Reuse the derived entity labels so the UI matches the active terminology.
addNotification({ type: 'success', - message: `${selectedTables.length} table${selectedTables.length > 1 ? 's' : ''} deleted` + message: `${selectedTables.length} ${ + selectedTables.length > 1 ? entityPlural : entitySingular + } deleted` }); @@ -<Confirm title="Delete tables" bind:open={showDelete} onSubmit={handleDelete} disabled={deleting}> +<Confirm + title={`Delete ${terminology.entity.title.plural}`} + bind:open={showDelete} + onSubmit={handleDelete} + disabled={deleting}>Also applies to: 113-116
src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte (1)
60-77: Update correct SDK for DocumentsDB rename.
updateName()always callstablesDB.update. For a DocumentsDB database this endpoint doesn’t exist, so rename will fail. Branch onterminology.type(or database.type) and call the matching SDK client before invalidating.
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
21-21: Fix type inconsistency for error state.The
errorvariable is initialized asnullbut later assignedtrue(boolean) at line 80, causing type inconsistency. This issue was previously flagged but remains unfixed.Apply this diff to fix the type:
-let error = $state(null); +let error = $state(false);Also applies to: 80-80
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte(6 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/grid.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte(12 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table.svelte(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
PR: appwrite/console#2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.tssrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte
🧬 Code graph analysis (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts (1)
load(8-24)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
EntityList(24-27)DatabaseType(8-8)toSupportiveEntity(83-96)src/lib/stores/database.ts (1)
database(11-11)src/lib/stores/sdk.ts (1)
sdk(149-172)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts (1)
src/lib/stores/navigation.ts (2)
withPath(12-21)resolveRoute(23-28)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (2)
Table(23-25)Columns(9-21)src/lib/helpers/string.ts (3)
singular(20-22)plural(37-65)capitalize(8-10)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
Term(11-11)TerminologyShape(5-9)TerminologyResult(13-23)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (2)
DatabaseType(8-8)baseTerminology(29-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (9)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (4)
86-119: LGTM! Terminology-based deletion aligns with refactor goals.The
handleDeletefunction correctly implements terminology-based switching for deletion, mirroring the pattern used inlistTables. The use ofresolveRoutefor navigation and deriveddatabase.namealigns well with the entity-centric refactor.
122-136: LGTM! Effects properly handle modal lifecycle.The cleanup and initialization effects correctly reset all state variables and trigger data loading. The previously flagged issue about resetting
confirmedDeletionhas been properly addressed.
139-195: LGTM! Template correctly uses entity-centric model.The template properly references
entityItems, deriveddatabase.name, and uses Svelte 5 event handler patterns (onclick). All UI text and rendering logic aligns with the entity-centric refactor.
46-84: SDK methods verified
BothprojectSdk.tablesDB.listTablesandprojectSdk.documentsDB.listCollectionsare declared insrc/lib/stores/sdk.tsand used successfully; no further action required.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte (5)
7-7: LGTM: Import uses the new alias.The import correctly uses the
$database/(entity)alias introduced in this PR for consolidated entity helpers.
22-26: LGTM: Title correctly uses dynamic terminology.The title properly uses
field.singularinstead of the hardcoded "column" string, consistent with the multi-terminology support objective.
28-32: LGTM: Subtitle correctly uses dynamic terminology.The subtitle properly uses
field.pluralandentityinstead of hardcoded "columns" and table reference strings, consistent with the multi-terminology support objective.
6-6: RetaintableColumnSuggestionsstore name
Internal identifiers can remain stable while UI text is dynamic, so no rename is needed.
18-20: Omit null guards for terminology extraction
setTerminologies(page)in the parent layout always provides a validterminologywith populatedentityandfieldfor supported databases, so optional chaining here is unnecessary.Likely an incorrect or invalid review comment.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts
Outdated
Show resolved
Hide resolved
...utes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
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]/settings/+page.svelte (1)
51-66: Fix updateName to honor the active database type.
loadEntityCountnow goes throughdatabaseSdk, butupdateNamestill hardcodestablesDB.update. On documentsDB (or any future type) this will throw becausetablesDBdoesn’t exist, breaking renames for those databases. Update the mutation to route through the same terminology-aware SDK (or branch onterminology.type) before shipping.
♻️ Duplicate comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
20-20: Fix error state type inconsistency.The
errorvariable is still initialized asnullbut assigned boolean values (trueat line 66,falseat line 105), causing type inconsistency. This issue was previously flagged but remains unresolved.Apply this diff to use a consistent boolean type:
-let error = $state(null); +let error = $state(false);Or, if you want to capture error details:
-let error = $state(null); +let error = $state<string | null>(null);And update line 66:
- error = true; + error = err.message ?? 'Failed to load tables';Also applies to: 66-66, 105-105
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte (1)
11-13: Decouple generic Header from table-specific store and keyThis (entity)-level component imports expandTabs from table-[table]/store and persists 'tableHeaderExpanded'. That couples a generic view to a table-only store/key.
Suggestions:
- Move expandTabs to $database/(entity)/store (shared), or accept an
expandedprop and let parents wire persistence.- Rename the preference key to something generic (e.g., 'entityHeaderExpanded').
Also applies to: 65-70, 73-101
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (2)
242-246: Use terminology for the “All …” labelHardcoded “All tables” breaks multi‑terminology.
Apply:
- { - name: 'All tables', + { + name: `All ${entityTypePlural}`, leadingIcon: IconTable, href: databaseBaseRoute }
81-85: Fix invalid Query.orderDesc(''): empty field will 400Provide a real field or omit ordering.
Apply:
- entities = await databasesSdk.listEntities({ - databaseId, - queries: [Query.orderDesc(''), Query.limit(100)] - }); + entities = await databasesSdk.listEntities({ + databaseId, + queries: [Query.orderDesc('$createdAt'), Query.limit(100)] + });src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts (1)
23-23: Fix invalid Query.orderDesc(''): empty field will 400Use a real attribute or drop the sort.
Apply:
- queries: [Query.limit(limit), Query.offset(offset), Query.orderDesc('')] + queries: [Query.limit(limit), Query.offset(offset), Query.orderDesc('$updatedAt')]
🧹 Nitpick comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
131-134: Consider showing more specific error information.When an error occurs loading tables, the UI falls back to a generic confirmation message. Displaying the actual error message would improve the user experience and aid debugging.
If you capture the error message (as suggested earlier):
{:else if error} <p class="text"> - Are you sure you want to delete <b>{database.name}</b>? + Failed to load tables: {error}. Are you sure you want to delete <b>{database.name}</b>? </p>src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte (1)
48-66: Consider moving hardcoded strings to terminology for consistency.The template contains English strings like "has been updated" (line 24) and "security" (lines 49, 51) that are not sourced from the terminology system. For full multi-terminology support, these strings could also be moved to the terminology configuration.
Example for the notification message:
// In terminology configuration success: { updated: 'has been updated' }Then use it:
addNotification({ message: `${entity.name} ${terminology.success.updated}`, type: 'success' });src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (3)
49-52: Improve error messages for better debugging.The error messages for unsupported database types are generic and don't provide helpful context for debugging. Consider including the actual type value and available options.
Apply this diff to improve error messages:
case 'vectordb': - throw new Error(`Database type not supported yet`); + throw new Error(`Database type 'vectordb' is not supported yet. The backend API is not finalized.`); default: - throw new Error(`Unknown database type`); + throw new Error(`Unknown database type: '${type}'. Supported types: legacy, tablesdb, documentsdb`);
63-66: Improve error messages for better debugging.Same issue as in
listEntities- error messages lack helpful context.Apply this diff:
case 'vectordb': - throw new Error(`Database type not supported yet`); + throw new Error(`Database type 'vectordb' is not supported yet. The backend API is not finalized.`); default: - throw new Error(`Unknown database type`); + throw new Error(`Unknown database type: '${type}'. Supported types: legacy, tablesdb, documentsdb`);
15-19: Consider function overloads for better type safety.The current union-type signature is flexible but loses type safety. When passing strings, TypeScript won't enforce that
databaseTypeis required. Function overloads would provide compile-time guarantees.Consider this refactor using TypeScript function overloads:
export function useDatabasesSdk( regionOrPage: Page, projectOrTerminology: TerminologyResult ): DatabaseSdkResult; export function useDatabasesSdk( regionOrPage: string, projectOrTerminology: string, databaseType: DatabaseType ): DatabaseSdkResult; export function useDatabasesSdk( regionOrPage: string | Page, projectOrTerminology: string | TerminologyResult, databaseType?: DatabaseType ): DatabaseSdkResult { // implementation... }This makes
databaseTyperequired when using strings, enforced at compile time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte(6 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte(12 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/storage/bucket-[bucket]/settings/+page.svelte(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/routes/(console)/project-[region]-[project]/storage/bucket-[bucket]/settings/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
PR: appwrite/console#2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
PR: appwrite/console#2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts
🧬 Code graph analysis (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts (1)
load(8-24)src/lib/stores/database.ts (1)
database(11-11)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
DatabaseType(8-8)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (1)
useDatabasesSdk(15-70)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
EntityList(24-27)DatabaseType(8-8)toSupportiveEntity(83-96)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
TerminologyResult(13-23)src/lib/stores/sdk.ts (1)
sdk(149-172)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (4)
1-12: LGTM! Imports updated for entity terminology support.The imports correctly reflect the shift to entity-based APIs (EntityList, getTerminologies) and use of SvelteKit navigation utilities (resolveRoute).
43-70: LGTM! Entity-based API integration is correct.The function properly uses
databaseSdk.listEntities()and handles pagination with the new entity terminology. The mapping logic for entity items is sound.
72-97: LGTM! Deletion flow updated for terminology-driven APIs.The delete handler correctly uses
databaseSdk.deleteEntity()and navigates viaresolveRoute, aligning with the entity-based refactoring. Error handling and success notifications are appropriate.
100-114: LGTM! Effects properly manage modal lifecycle.The cleanup effect correctly resets all state when the modal closes, including the previously missing
confirmedDeletionreset. The initialization effect appropriately triggers table loading when the modal opens.
...s/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts
Show resolved
Hide resolved
...s/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts
Show resolved
Hide resolved
...ect-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte
Show resolved
Hide resolved
...ect-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.json(1 hunks)src/lib/stores/sdk.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/stores/sdk.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
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 (5)
src/routes/(console)/project-[region]-[project]/overview/platforms/+page.svelte (1)
29-49: Update continuePlatform call in onboard.svelte
The call at src/routes/(console)/project-[region]-[project]/overview/onboard.svelte:60 still passes a numeric index to theplatform: Platformparameter—replace it with a validPlatform(e.g.platforms[type]).src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
254-275: Uninitializedcolumnsvariable when filtered columns existWhen
filteredColumnshas items (non-relationship columns exist), thecolumnsvariable is never assigned. Later at line 282,generateFakeRecords(columns, ...)receives undefined, which likely causes errors.Apply this diff to ensure
columnsis always initialized:let columns: Columns[]; const currentFields = table?.fields ?? []; const hasAnyRelationships = currentFields.some((field: Field) => isRelationship(field)); const filteredColumns = currentFields.filter( (field: Field) => field.type !== 'relationship' ); if (!filteredColumns.length) { try { columns = await generateColumns($project, page.params.database, page.params.table); await invalidate(Dependencies.TABLE); trackEvent(Submit.ColumnCreate, { type: 'faker' }); } catch (e) { addNotification({ type: 'error', message: e.message }); $spreadsheetLoading = false; return; } + } else { + columns = filteredColumns; }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte (1)
211-225: Disabled-state compares Field as Column → map to relational first.compareColumns expects a Columns-like shape. Currently passing Field; this can miscompute disabled-state.
- return relatedTable.fields.every((column: Columns) => - compareColumns(column, workValue, row) - ); + return relatedTable.fields + .map(toRelationalField) + .every((column) => compareColumns(column, workValue, row)); @@ - return relatedTable.fields.every((column: Columns) => - compareColumns(column, workValue, row) - ); + return relatedTable.fields + .map(toRelationalField) + .every((column) => compareColumns(column, workValue, row));Also applies to: 232-234
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte (1)
93-107: Guard array comparisons to avoid runtime errors on null/undefined.Array.from on null/undefined throws; also add safety for to-many relations.
- if (column.array) { - return !symmetricDifference(Array.from(workColumn), Array.from(currentColumn)).length; - } + if (column.array) { + const workArr = Array.isArray(workColumn) ? workColumn : []; + const currArr = Array.isArray(currentColumn) ? currentColumn : []; + return !symmetricDifference(Array.from(workArr), Array.from(currArr)).length; + } @@ - if (isRelationshipToMany(column as Models.ColumnRelationship)) { - const workIds = workColumn.map((doc: string | Record<string, unknown>) => + if (isRelationshipToMany(column as Models.ColumnRelationship)) { + if (!Array.isArray(workColumn) || !Array.isArray(currentColumn)) { + return workColumn === currentColumn; + } + const workIds = workColumn.map((doc: string | Record<string, unknown>) =>src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)
102-107: Declaretableand avoid recreating therowsstore (compile/runtime issues).
tableis assigned but never declared → TS compile error.rows = writable(data.rows)inside a reactive statement recreates the store on every invalidation → state loss and unnecessary churn.Apply this refactor:
@@ export let data: PageData; export let showRowCreateSheet: { show: boolean; row: Partial<Models.Row> | null; }; -$: table = data.table; -$: rows = writable(data.rows); +let table: Models.Table; +const rows = writable(data.rows); +$: table = data.table; +$: rows.set(data.rows); $: if (rows) { paginatedRows.clear(); paginatedRows.setPage(1, $rows.rows); }
♻️ Duplicate comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
254-259: Critical: Guard against undefined table.fieldsThis issue was previously flagged but remains unaddressed. If
table.fieldsis undefined, calling.some()and.filter()oncurrentFieldswill throw a TypeError, breaking sample data generation.Apply this diff to add null-safe defaults:
- let columns: Columns[]; - const currentFields = table.fields; - const hasAnyRelationships = currentFields.some((field: Field) => isRelationship(field)); - const filteredColumns = currentFields.filter( - (field: Field) => field.type !== 'relationship' - ); + let columns: Columns[]; + const currentFields = table?.fields ?? []; + const hasAnyRelationships = currentFields.some((field: Field) => isRelationship(field)); + const filteredColumns = currentFields.filter( + (field: Field) => field.type !== 'relationship' + );src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
82-84: Fix: hasColumns can throw when fields is undefined (duplicate).Make it null-safe to avoid reading .length of undefined.
- $: hasColumns = !!table.fields.length; + $: hasColumns = !!table?.fields?.length; $: hasValidColumns = table?.fields?.some((field: Field) => field.status === 'available');src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (1)
52-56: Re: null-safe guard for optional/mixed fields (echoing earlier review)Earlier feedback suggested guarding with Array.isArray and optional field properties. Current optional chaining is fine, but this is safer if elements can be partial.
- ...(entity?.fields - ?.filter((field) => field.status === 'available' && isRelationship(field)) - ?.map((field) => Query.select([`${field.key}.*`])) ?? []), + ...(Array.isArray(entity?.fields) + ? entity!.fields + .filter((f) => (f as any)?.status === 'available' && isRelationship(f as any)) + .map((f) => Query.select([`${(f as any).key}.*`])) + : []),
🧹 Nitpick comments (8)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
23-41: Types look fine; consider centralizing to avoid drift.Attributes/Collection redefined here may diverge from the entity surface. Prefer exporting these from $database/(entity) and reusing.
Would you like me to generate a small follow-up to re-export and update imports?
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte (1)
48-51: toRelationalField preserves status; filter-before-map isn’t required
toRelationalField simply returns the raw Field as Columns, so.map(toRelationalField).filter(c => c.status === 'available')correctly selects available columns. Optionally guard against undefinedtable.fieldsby using(table.fields ?? []).src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (2)
7-19: Make to-many detection explicit and avoid double negativesSimplify logic, cover unknown sides, and keep a safe fallback.
-export function isRelationshipToMany(field: Field) { - if (!field) return false; - if (!isRelationship(field)) return false; - - const column = field as Models.ColumnRelationship | Models.AttributeRelationship; - - if (!column?.relationType) return false; - if (column?.side === 'child') { - return !['oneToOne', 'oneToMany'].includes(column?.relationType); - } else { - return !['oneToOne', 'manyToOne'].includes(column?.relationType); - } -} +export function isRelationshipToMany(field: Field) { + if (!field || !isRelationship(field)) return false; + const rel = field as Models.ColumnRelationship | Models.AttributeRelationship; + const { relationType, side } = rel; + if (!relationType) return false; + if (relationType === 'manyToMany') return true; + if (relationType === 'oneToOne') return false; + if (relationType === 'manyToOne') return side === 'child'; + if (relationType === 'oneToMany') return side !== 'child'; + return false; +}
33-47: Guard non-string types before calling toLowerCaseWhen receiving UI Column, or an unexpected shape, type may not be a string. Add a typeof check to avoid runtime errors.
export function isSpatialType( field: Columns | Attributes | Column ): field is | Models.ColumnPoint | Models.ColumnLine | Models.ColumnPolygon | Models.AttributePoint | Models.AttributeLine | Models.AttributePolygon { - if (!field) return false; + if (!field || typeof (field as any).type !== 'string') return false; const spatialTypes = ['point', 'linestring', 'polygon']; - return spatialTypes.includes(field.type.toLowerCase()); + const type = (field as any).type.toLowerCase(); + return spatialTypes.includes(type); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (4)
378-378: Null‑safe guard when checking relationships (defensive).Avoid property access on a possibly undefined
table:-const hasAnyRelationships = table.fields.some(isRelationship); +const hasAnyRelationships = table?.fields?.some(isRelationship) ?? false;
659-659: GuardbuildWildcardColumnsQuery(table)whentableis undefined.Prevent constructing queries with an undefined source:
-...buildWildcardColumnsQuery(table) +...(table ? buildWildcardColumnsQuery(table) : [])Also applies to: 686-686
743-745: Reactive rebuild should guard on existence and reduce churn.Guard against undefined
tableand avoid repeated rebuilds when fields are identical:-$: if (table.fields) { +$: if (table?.fields?.length) { makeTableColumns(); }Optional: compute a hash of
table.fieldsto rebuild only when it changes materially.
910-910: Verify related row object shape (tableId) and prefer dot access.If the related row includes
tableId, dot access is clearer:-$databaseRelatedRowSheetOptions.tableId = row[columnId]?.['tableId']; +$databaseRelatedRowSheetOptions.tableId = row[columnId]?.tableId;Please confirm the property name is indeed
tableId(and not$tableId/$collectionIddepending on backend).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(18 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(13 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts(1 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/settings/domains/table.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
🧬 Code graph analysis (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (2)
Field(16-16)Entity(10-14)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (2)
Columns(9-21)Attributes(23-35)src/lib/helpers/types.ts (1)
Column(44-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (13)
src/routes/(console)/project-[region]-[project]/settings/domains/table.svelte (2)
95-95: Note: This change appears unrelated to the PR's stated objectives.While this is a valuable bug fix, it doesn't relate to the multi-terminology support feature described in the PR summary. Consider whether this fix should be in a separate PR or if the PR description needs updating.
95-95: Approve typo fix—no remaining 'verifiying' occurrences detected. The correction ensures the Retry action is properly hidden during domain verification.src/routes/(console)/project-[region]-[project]/overview/platforms/+page.svelte (1)
10-10: LGTM! Type import improves type safety.The import of
PlatformTypefrom the Appwrite console SDK is correct and enables stronger typing for thecontinuePlatformfunction.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (3)
46-59: createTableColumns mapping looks good.Icon/format/elements derivation aligns with Field surface.
248-253: Nice UX: pre-populating create-column sheet state.Good use of onOpenCreateColumn to hydrate sheet inputs.
287-289: Prop passthrough alignment: CreateRow now receives Entity.Matches the child’s updated prop type.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte (1)
194-207: recordSecurity is correctly exposed and used.
The terminology helper maps raw.rowSecurity → recordSecurity (terminology.ts), and create.svelte, editPermissions.svelte, and the security view all reference table.recordSecurity. No changes needed.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (1)
21-26: Type guards look goodThe relationship and string discriminators are correct and concise.
Also applies to: 28-31
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (5)
155-169: LGTM: Columns built fromtable.fields.Clean mapping with icon, width, and enum support looks correct.
609-609: LGTM: Usetable.$idwhen updating rows.Correctly references the current table ID for the update call.
701-702: LGTM: Handle object form for related table IDs.Supporting
{ tableId: string }improves robustness of display-name resolution.
1105-1105: LGTM: Confirmation messages usetable.name.Accurate and user‑friendly deletion prompts.
Also applies to: 1112-1112, 1115-1115
85-85: ```bash
#!/bin/bash
echo "== tsconfig.json alias for $database =="
rg -n '$database' -g 'tsconfig*.json'echo "== vite.config.* alias for $database =="
rg -n '$database' -g 'vite.config.*'echo "== svelte.config.* alias for $database =="
rg -n '$database' -g 'svelte.config.*'</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
...ole)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte
Show resolved
Hide resolved
...oject-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte
Show resolved
Hide resolved
...e)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
Show resolved
Hide resolved
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 (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte (1)
93-107: Harden array/relationship comparisons against undefined valuesArray and relation-to-many paths assume arrays; undefined/null will crash (Array.from(undefined), workColumn.map). Default to empty arrays.
Apply:
- if (column.array) { - return !symmetricDifference(Array.from(workColumn), Array.from(currentColumn)).length; - } + if (column.array) { + return !symmetricDifference( + Array.from(workColumn ?? []), + Array.from(currentColumn ?? []) + ).length; + } if (isRelationship(column)) { if (isRelationshipToMany(column as Models.ColumnRelationship)) { - const workIds = workColumn.map((doc: string | Record<string, unknown>) => + const workIds = (Array.isArray(workColumn) ? workColumn : []).map( + (doc: string | Record<string, unknown>) => typeof doc === 'string' ? doc : doc.$id - ); + ); - const relatedIds = currentColumn.map((doc: string | Record<string, unknown>) => + const relatedIds = (Array.isArray(currentColumn) ? currentColumn : []).map( + (doc: string | Record<string, unknown>) => typeof doc === 'string' ? doc : doc.$id - ); + ); return !symmetricDifference(workIds, relatedIds).length; } else {src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
66-71: System columns filtering logic is invertedYou likely want to exclude hidden (selected) columns from filterable set, same as $id. Currently it keeps only hidden system columns.
- const systemColumns = [ + const systemColumns = [ { id: '$createdAt', title: '$createdAt', type: 'datetime' as ColumnType }, { id: '$updatedAt', title: '$updatedAt', type: 'datetime' as ColumnType } - ].filter((col) => !!selected.includes(col.id)); + ].filter((col) => !selected.includes(col.id));
♻️ Duplicate comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
81-86: Still using invalid empty-string ordering.The
Query.orderDesc('')call with an empty string violates the API contract and will result in a 400 error from the backend. This issue was flagged in a previous review and remains unresolved.Apply this diff to fix:
async function loadEntities() { entities = await databasesSdk.listEntities({ databaseId, - queries: [Query.orderDesc(''), Query.limit(100)] + queries: [Query.orderDesc('$createdAt'), Query.limit(100)] }); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (1)
49-60: Guard for optional fields on Entity to avoid runtime/type pitfalls.Entity.fields is optional, and each Field is Partial, meaning
statusandkeyproperties may be undefined. The current implementation uses optional chaining for the array but doesn't guard against undefined properties within each field object.This concern was flagged in a previous review. Apply type guards to ensure
field.statusandfield.keyare defined before accessing them:export function buildWildcardColumnsQuery(entity: Entity | null = null): string[] { return [ - ...(entity?.fields - ?.filter((field) => field.status === 'available' && isRelationship(field)) - ?.map((field) => Query.select([`${field.key}.*`])) ?? []), + ...(Array.isArray(entity?.fields) + ? entity.fields + .filter((field) => field?.status === 'available' && isRelationship(field)) + .map((field) => Query.select([`${field?.key}.*`])) + : []), Query.select(['*']) ]; }src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
104-119: Avoid silent fallback on unknown database typesCurrently, unknown types return empty terms, making entity/field/record undefined at runtime.
Consider validating and failing fast:
export function useTerminology(page: Page): TerminologyResult { const type = page.data?.database?.type as DatabaseType; if (!type) { // strict check because this should always be available! throw new AppwriteException('Database type is required', 500); } - const dbTerminologies = terminologyData[type] || {}; + const dbTerminologies = terminologyData[type]; + if (!dbTerminologies?.entity) { + throw new AppwriteException(`Unsupported or undefined database type: ${type}`, 500); + } return { type, source: dbTerminologies, field: dbTerminologies.field, record: dbTerminologies.record, entity: dbTerminologies.entity }; }src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
20-22: Fix error state type inconsistencyerror is initialized as null, then assigned booleans. Use a consistent boolean or string|null throughout.
Example:
-let error = $state(null); +let error = $state(false); ... -} catch (err) { - error = true; +} catch (err) { + error = true; }Also applies to: 99-108
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
82-84: Fix: hasColumns can throw when fields is undefinedUse null-safe check.
- $: hasColumns = !!table.fields.length; + $: hasColumns = !!table?.fields?.length;
🧹 Nitpick comments (6)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (1)
33-47: Add type guard forfield.typeto prevent runtime errors.Although
fieldis typed as a union of concrete types (not Partial), defensive programming suggests verifyingfield.typeexists before accessing it, especially since the function accepts multiple type variants including the genericColumntype from$lib/helpers/types.Apply this diff:
export function isSpatialType( field: Columns | Attributes | Column ): field is | Models.ColumnPoint | Models.ColumnLine | Models.ColumnPolygon | Models.AttributePoint | Models.AttributeLine | Models.AttributePolygon { - if (!field) return false; + if (!field?.type) return false; const spatialTypes = ['point', 'linestring', 'polygon']; return spatialTypes.includes(field.type.toLowerCase()); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
27-45: Clarify the intentional vectordb stubAdd a brief inline comment stating that vectordb is intentionally empty until backend API finalizes, to prevent future removals.
Apply:
legacy: { entity: 'collection', field: 'attribute', record: 'document' }, - vectordb: {} + // Intentionally empty; backend API not finalized and not returned by any database type yet. + vectordb: {}src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
43-55: Rename listTables → listEntities for clarityFunction now fetches entities, not tables. Renaming improves readability.
-async function listTables() { +async function listEntities() { ... - if (showDelete) { - listTables(); + if (showDelete) { + listEntities(); } ... - <button class="u-underline" onclick={listTables} type="button"> + <button class="u-underline" onclick={listEntities} type="button">Also applies to: 111-114, 154-167
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte (1)
47-51: Make initialization resilient if table.fields is absentIf a raw Models.Table slips through, table.fields will be undefined. Support both shapes.
- const availableColumns = table.fields - .map(toRelationalField) - .filter((column: Columns) => column.status === 'available'); + const rawFields = (table as any)?.fields ?? (table as any)?.columns ?? []; + const availableColumns = rawFields + .map(toRelationalField) + .filter((column: Columns) => column.status === 'available');src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (2)
378-379: Null-safe relationship checkAvoid .some on possibly-undefined fields.
- const hasAnyRelationships = table.fields.some(isRelationship); + const hasAnyRelationships = table?.fields?.some(isRelationship) ?? false;
621-628: Use matching analytics event on update failureFailure branch tracks RowUpdatePermissions; action here is RowUpdate.
- trackError(error, Submit.RowUpdatePermissions); + trackError(error, Submit.RowUpdate);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte(6 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte(12 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(18 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(13 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts(1 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/settings/domains/table.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
PR: appwrite/console#2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
🧬 Code graph analysis (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (2)
Field(16-16)Entity(10-14)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (2)
Columns(9-21)Attributes(23-35)src/lib/helpers/types.ts (1)
Column(44-61)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (5)
Collection(37-39)Table(41-43)Attributes(23-35)Columns(9-21)indexes(46-46)src/lib/helpers/string.ts (3)
singular(20-22)plural(37-65)capitalize(8-10)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
Term(11-11)TerminologyShape(5-9)TerminologyResult(13-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (7)
src/routes/(console)/project-[region]-[project]/settings/domains/table.svelte (1)
95-95: Approve typo correction in domain.status checkNo remaining “verifiying” occurrences found; Retry action is now correctly hidden during active verification.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (2)
21-26: Add type guard forfield.typeto prevent runtime errors.Since
Fieldis defined asPartial<Attributes> | Partial<Columns>, thetypeproperty may be undefined. Accessingfield?.typewithout verification can lead to unexpected behavior or TypeScript errors under strict type checking.Apply this diff to add a type guard:
export function isRelationship( field: Field ): field is Models.ColumnRelationship | Models.AttributeRelationship { if (!field) return false; - return field?.type === 'relationship'; + return field?.type === 'relationship'; + if (!field?.type) return false; + return field.type === 'relationship'; }Wait, let me provide a cleaner fix:
export function isRelationship( field: Field ): field is Models.ColumnRelationship | Models.AttributeRelationship { - if (!field) return false; - return field?.type === 'relationship'; + if (!field?.type) return false; + return field.type === 'relationship'; }Likely an incorrect or invalid review comment.
28-31: Add type guard forfield.typeto prevent runtime errors.Since
Fieldis defined asPartial<Attributes> | Partial<Columns>, thetypeproperty may be undefined. Accessingfield?.typewithout verification can lead to unexpected behavior.Apply this diff:
export function isString(field: Field): field is Models.ColumnString | Models.AttributeString { - if (!field) return false; - return field?.type === 'string'; + if (!field?.type) return false; + return field.type === 'string'; }Likely an incorrect or invalid review comment.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
23-43: Typed attribute/collection/table normalization looks goodThe Attributes union and the Omit-based Collection/Table wrappers correctly mirror SDK models while enabling local specialization.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte (1)
27-35: VerifyEntitypassed to CreateRow
tableprop changed fromModels.TabletoEntity. Confirm all<CreateRow>call sites (e.g.,+page.svelte:286) now pass anEntityinstance. If any still supply rawModels.Table, add a defensive fallback increate.svelteto wrap it into anEntity.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (2)
120-126: Verify: command toggles the correct “Create row” UIThis now sets $showCreateEntity, while the page uses $showRowCreateSheet for the CreateRow sheet. Ensure $showCreateEntity is wired to open the same sheet on this route.
446-458: Nice: index creation now invalidates table dependenciesRefreshing Dependencies.TABLE after createIndex prevents stale UI. LGTM.
...nsole)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
Outdated
Show resolved
Hide resolved
...oject-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte
Outdated
Show resolved
Hide resolved
...e)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
Show resolved
Hide resolved
| import CreateWeb from './createWeb.svelte'; | ||
| import { createPlatform } from './wizard/store'; | ||
| import { Click, trackEvent } from '$lib/actions/analytics'; | ||
| import { PlatformType } from '@appwrite.io/console'; |
There was a problem hiding this comment.
🧩 Analysis chain
Type safety improvement looks good—verify downstream compatibility.
The introduction of PlatformType and the updated continuePlatform signature improve type safety by replacing the generic string type with a more specific enum or union type.
Ensure that:
- All call sites of
continuePlatformhave been updated to passPlatformTypevalues instead of raw strings. - The
createPlatform.setmethod (line 38) andwizard.start(line 46) expectPlatformTypefor their respectivetypeandplatformfields.
Run the following script to verify all call sites:
Also applies to: 29-49
🏁 Script executed:
#!/bin/bash
# Description: Find all call sites of continuePlatform and verify they pass PlatformType
# Search for continuePlatform invocations
rg -n --type=ts --type=svelte -A 3 -B 1 'continuePlatform\s*\('Length of output: 30
🏁 Script executed:
#!/bin/bash
# Description: Find all call sites of continuePlatform in .svelte and .ts files
rg -n 'continuePlatform\s*\(' -C3 --glob '*.svelte' --glob '*.ts'Length of output: 1548
Update call sites to use PlatformType
In src/routes/(console)/project-[region]-[project]/overview/onboard.svelte:60, continuePlatform(type, …) still passes type: number but the signature now expects a PlatformType. Change openPlatformWizard to accept and forward a PlatformType enum value.
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/overview/platforms/+page.svelte
around line 10, update the openPlatformWizard function to accept and forward a
PlatformType enum instead of a number: change its parameter type to PlatformType
and pass that value to continuePlatform (or any downstream call) so the call
site uses the PlatformType enum value; ensure the existing import { PlatformType
} is used and update any local usages/annotations to reflect the new
PlatformType parameter.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte (1)
99-101: Critical: Inconsistent property access on Entity type.Line 99 accesses
rowTable.columns, but therelatedTablevariable is typed asEntity | null(line 31), and the broader refactoring convertscolumnstofields. The code should referencerowTable.fieldsinstead ofrowTable.columnsto align with the Entity type and maintain consistency with the rest of the file.Apply this diff to fix the property access:
- const hasAllColumns = rowTable.columns.every( + const hasAllColumns = rowTable.fields.every( (column: Columns) => column.key in row );src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (1)
7-18: Fix to-many detection for child-side relationships.When
field.side === 'child'andrelationType === 'manyToOne', this returnstrue, so single-object relationships are treated as arrays. That cascades into broken UI/logic whereverisRelationshipToManygates list handling. Limit the child branch tomanyToManyonly and handle parent/undefined sides separately.export function isRelationshipToMany(field: Field) { if (!field) return false; if (!isRelationship(field)) return false; const column = field as Models.ColumnRelationship | Models.AttributeRelationship; - if (!column?.relationType) return false; - if (column?.side === 'child') { - return !['oneToOne', 'oneToMany'].includes(column?.relationType); - } else { - return !['oneToOne', 'manyToOne'].includes(column?.relationType); - } + const relationType = column?.relationType; + if (!relationType) return false; + + if (column?.side === 'child') { + return relationType === 'manyToMany'; + } + + return relationType === 'oneToMany' || relationType === 'manyToMany'; }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte (1)
93-96: Guard array comparisons to avoid runtime errors.Array.from on undefined/null throws. Normalize before diffing.
Apply:
- if (column.array) { - return !symmetricDifference(Array.from(workColumn), Array.from(currentColumn)).length; - } + if (column.array) { + const workArr = Array.isArray(workColumn) + ? workColumn + : workColumn != null + ? [workColumn] + : []; + const currArr = Array.isArray(currentColumn) + ? currentColumn + : currentColumn != null + ? [currentColumn] + : []; + return !symmetricDifference(workArr, currArr).length; + }
♻️ Duplicate comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
20-23: Unify error state typing.Initialized as null, later assigned boolean. Keep a consistent type.
Example:
-let error = $state(null); +let error = $state(false); ... $effect(() => { if (!showDelete) { entities = null; entityItems = []; - error = false; + error = false; confirmedDeletion = false; } });Alternatively, use a message string (string | null) and render it.
Also applies to: 99-109
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (1)
24-33: Validate inputs; avoid non-null assertions and empty fallbacks.The databaseType! assertion and '' fallbacks for region/project are unsafe and were previously flagged. Add explicit validation.
Apply the earlier suggested guard:
- if (typeof regionOrPage === 'object' && typeof projectOrTerminology === 'object') { - type = projectOrTerminology.type; - region = regionOrPage?.params?.region || ''; - project = regionOrPage?.params?.project || ''; - } else { - type = databaseType!; + if (typeof regionOrPage === 'object' && typeof projectOrTerminology === 'object') { + type = projectOrTerminology.type; + region = regionOrPage?.params?.region || ''; + project = regionOrPage?.params?.project || ''; + } else { + if (!databaseType) throw new Error('databaseType is required when using string arguments'); + type = databaseType; region = regionOrPage as string; project = projectOrTerminology as string; } + if (!region || !project) throw new Error('region and project are required');src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
81-85: Fix invalid Query.orderDesc('') usage.Empty field causes a 400. Use a concrete field (e.g., $updatedAt or $createdAt) or drop ordering.
Apply:
- entities = await databasesSdk.listEntities({ - databaseId, - queries: [Query.orderDesc(''), Query.limit(100)] - }); + entities = await databasesSdk.listEntities({ + databaseId, + queries: [Query.orderDesc('$updatedAt'), Query.limit(100)] + });src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
82-84: Fix: Inconsistent null-safety fortable.fieldsLine 82 accesses
table.fields.lengthwithout optional chaining, which can throw iffieldsis undefined. Line 83 correctly uses optional chainingtable?.fields?.some(...). Apply consistent null-safe access on both lines.Apply this diff to make line 82 null-safe:
- $: hasColumns = !!table.fields.length; + $: hasColumns = !!table?.fields?.length; $: hasValidColumns = table?.fields?.some((field: Field) => field.status === 'available');src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
254-259: Fix: Guard sample-data generation against undefined fieldsAccessing
.some()and.filter()ontable.fieldswhen it's undefined will throw. Add null-safe defaults before calling array methods.Apply this diff to add null-safe guards:
- let columns: Columns[]; - const currentFields = table.fields; - const hasAnyRelationships = currentFields.some((field: Field) => isRelationship(field)); - const filteredColumns = currentFields.filter( - (field: Field) => field.type !== 'relationship' - ); + let columns: Columns[]; + const currentFields = table?.fields ?? []; + const hasAnyRelationships = currentFields.some((field: Field) => isRelationship(field)); + const filteredColumns = currentFields.filter( + (field: Field) => field.type !== 'relationship' + );
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)
910-910: Prefer dot notation for static property access.Line 910 uses bracket notation for a static property name
['tableId'], which is inconsistent with the dot notation used at line 701 for the same property. While both work, dot notation is more idiomatic for static properties.Consider this minor style improvement:
- $databaseRelatedRowSheetOptions.tableId = - row[columnId]?.['tableId']; + $databaseRelatedRowSheetOptions.tableId = + row[columnId]?.tableId;src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
23-39: Centralize Attributes/Collection types under the entity module.These public types are better housed in $database/(entity) (and re-exported here if needed) to avoid duplication and drift across table- and entity-oriented surfaces.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
117-124: Use terminology for labels (“tables”).Make the description and header terminology-driven.
Apply:
- {#if entityItems.length > 0} - The following tables and all data associated with <b>{database.name}</b>, will be + {#if entityItems.length > 0} + The following {entityTypePlural} and all data associated with <b>{database.name}</b> will beAnd add imports/derived terms near the top:
-import { type EntityList, getTerminologies } from '$database/(entity)'; +import { type EntityList, getTerminologies, useTerminology } from '$database/(entity)'; ... +const terminology = $derived(useTerminology(page)); +const entityTypePlural = $derived(terminology.entity.lower.plural);Also applies to: 138-141
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte (1)
146-150: Ensure relationship type handling is correct for non-table entities.compareColumns assumes Models.ColumnRelationship. If this component can render Entities from documentsdb, this cast is wrong (AttributeRelationship). Prefer comparing against a normalized relational field (e.g., toRelationalField) for isDisabled, or constrain this component to tables only.
Example approach:
- Precompute normalizedFields = table.fields.map(toRelationalField) and use that in isDisabled/comparisons.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte(6 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte(12 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(18 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(13 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts(1 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/settings/domains/table.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
PR: appwrite/console#2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
🧬 Code graph analysis (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
EntityList(22-25)DatabaseType(8-8)toSupportiveEntity(81-94)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
TerminologyResult(13-23)src/lib/stores/sdk.ts (1)
sdk(149-172)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (2)
Field(16-16)Entity(10-14)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (2)
Columns(9-21)Attributes(23-35)src/lib/helpers/types.ts (1)
Column(44-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (13)
src/routes/(console)/project-[region]-[project]/overview/platforms/+page.svelte (1)
29-39: Nice upgrade to typed platform payload.Switching the
typeargument toPlatformTypekeeps the store state aligned with the shared SDK surface. 👍src/routes/(console)/project-[region]-[project]/settings/domains/table.svelte (1)
95-95: Typo fix correct — no other occurrences found.
Searches confirm the change fully resolves the logic error and no strayverifiyinginstances remain.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte (1)
350-352: VerifyrelatedTable.fieldsitems before casting
Thefieldsarray is typed asField = Partial<Attributes> | Partial<Columns>, which may not guaranteetwoWayorkey. Ensure each item truly implementsModels.ColumnRelationship—for example, use a type guard ((f): f is Models.ColumnRelationship => 'twoWay' in f && 'key' in f)—before calling.filterand.map.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (5)
85-85: LGTM: Import migration to public API surface.The import path change from a local module to the centralized public export path
$database/(entity)aligns with the PR's goal of creating a generic entity model. This improves code reusability and establishes a clear public API boundary.
102-102: LGTM: Prop-based reactivity pattern.The migration from store-based (
$table) to prop-based (tablefromdata) reactivity is handled correctly. The reactive statement at line 743 ensures that column definitions are regenerated whenevertable.fieldschanges, maintaining proper UI updates.Also applies to: 743-745
1105-1115: LGTM: Consistent template bindings.The update from
$table.nametotable.namein the delete confirmation dialog is consistent with the migration to prop-based reactivity throughout the file. The bindings will update correctly when the parent data changes.
155-169: Verifyfield.typealigns withColumnTypevalues
Unable to locate theFieldtype definition—manually confirm eachfield.typematches a value incolumnTypesto avoid invalid casts.
422-422: Use the correct analytics event for row deletion. Replaced the nonexistentSubmit.RowDeletewithClick.DatabaseRowDelete('click_row_delete'). Ensure analytics dashboards and queries reference this event.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (2)
46-58: LGTM: Clean field-to-column mapping with icon supportThe
createTableColumnsfunction correctly maps fields to column objects and derives icons fromcolumnOptions. The implementation is clear and handles the optionalselectedparameter appropriately.
248-253: LGTM: Proper initialization of column creation sheetThe
onOpenCreateColumnhandler correctly initializes all necessary properties (show,title,columns,columnsOrder) for the create column sheet in the no-columns scenario.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (3)
89-93: LGTM: Type-safe route resolutionThe
basePathreactive statement usingresolveRouteis a clean improvement over manual path building. This ensures type-safe route construction that updates reactively withpage.params.
446-458: LGTM: Proper dependency invalidation after index creationThe
onCreateIndexcallback correctly creates the index via SDK and invalidatesDependencies.TABLEafterward (line 457), ensuring the UI reflects the new index. This addresses the concern raised in the previous review.Based on past review comments.
124-125: LGTM: Consistent optional chaining and entity creationLine 124 now uses
table?.$idwith optional chaining for consistency (addressing the previous review comment), and line 125 correctly uses$showCreateEntityto align with the new entity-based architecture.Based on past review comments.
| const handleDelete = async () => { | ||
| try { | ||
| await sdk | ||
| .forProject(page.params.region, page.params.project) | ||
| .tablesDB.delete({ databaseId }); | ||
| await databaseSdk.delete({ | ||
| databaseId: page.params.database | ||
| }); | ||
There was a problem hiding this comment.
Call the correct API to delete a database.
This modal deletes a database. Route it to databases.delete (not entity delete).
Apply one of:
- If you adopt the sdk helper change:
- await databaseSdk.delete({
- databaseId: page.params.database
- });
+ await databaseSdk.deleteDatabase(page.params.database);- Or call the SDK directly:
- await databaseSdk.delete({
- databaseId: page.params.database
- });
+ await sdk.forProject(page.params.region, page.params.project)
+ .databases.delete(page.params.database);🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte
around lines 72 to 77, the delete handler is calling the wrong API
(entity.delete) — update it to call the databases.delete endpoint instead;
either use the provided sdk helper for databases.delete if available or call the
SDK directly (databases.delete) with the correct payload (databaseId:
page.params.database), and ensure you await the call and handle errors/response
the same way as before.
| <button class="u-underline" onclick={listTables} type="button"> | ||
| Show more | ||
| </button> |
There was a problem hiding this comment.
Fix Svelte event handlers: use on:click, not onclick.
Current buttons won’t fire in Svelte.
Apply:
- <button class="u-underline" onclick={listTables} type="button">
+ <button class="u-underline" on:click={listTables} type="button">
Show more
</button>
...
- <button
- class="u-underline"
- onclick={() => {
- entityItems = entityItems.slice(0, 3);
- }}
- type="button">
+ <button
+ class="u-underline"
+ on:click={() => {
+ entityItems = entityItems.slice(0, 3);
+ }}
+ type="button">
Show less
</button>Also applies to: 163-171
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte
around lines 154-156 (and similarly lines 163-171), the buttons use plain HTML
onclick attributes which do not work in Svelte; replace onclick={...} with
Svelte's event directive on:click={...} for each button handler (e.g., change
onclick={listTables} to on:click={listTables}) and ensure any other inline
onclick usages in those ranges are converted the same way.
...oject-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte
Outdated
Show resolved
Hide resolved
...oject-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
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]/+layout.svelte (1)
254-282: Critical: Null-safety and uninitialized columns when fields exist.Three issues in the fake data generation logic:
- Line 255:
table.fieldsaccessed without null/undefined check will throw iftableortable.fieldsis undefined.- Lines 254-282: When non-relationship fields already exist (
filteredColumns.length > 0), the code skips column generation but never assignsfilteredColumnstocolumns. This causesgenerateFakeRecords(columns, ...)at line 282 to receive an empty array instead of the actual fields.- Type mismatch:
filteredColumnsisField[]butgenerateFakeRecordsexpectsColumns[].Apply this diff to fix all three issues:
- let columns: Columns[] = []; - const currentFields = table.fields; + let columns: Columns[]; + const currentFields = table?.fields ?? []; const hasAnyRelationships = currentFields.some((field: Field) => isRelationship(field)); const filteredColumns = currentFields.filter( (field: Field) => field.type !== 'relationship' - ); + ) as Columns[]; if (!filteredColumns.length) { try { columns = await generateColumns($project, page.params.database, page.params.table); await invalidate(Dependencies.TABLE); trackEvent(Submit.ColumnCreate, { type: 'faker' }); } catch (e) { addNotification({ type: 'error', message: e.message }); $spreadsheetLoading = false; return; } + } else { + columns = filteredColumns; } /* let the columns be processed! */ await sleep(1250);
♻️ Duplicate comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
20-22: Fix error state type inconsistency.The
errorvariable is initialized asnullbut later assigned boolean values (trueat line 66,falseat line 109), causing type inconsistency.Apply this diff to use a consistent boolean type:
-let error = $state(null); +let error = $state(false); let confirmedDeletion = $state(false); let isLoadingRowsCount = $state(false);Alternatively, if you want to capture error messages:
-let error = $state(null); +let error = $state<string | null>(null); let confirmedDeletion = $state(false); let isLoadingRowsCount = $state(false);Then update line 66:
} catch (err) { - error = true; + error = err instanceof Error ? err.message : String(err); } finally {And update line 129 to display the error message if using the string approach.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (3)
155-169: Guard against undefined fields (see existing comment).The past review comment at lines 155-169 already flagged this issue:
table.fields.map()will throw iffieldsis undefined. While line 743 adds a reactive guard, it doesn't protect the map operation itself withinmakeTableColumns().
422-422: Analytics namespace inconsistency (see existing comment).The past review comment at line 4 already flagged this: row-delete success uses
Click.DatabaseRowDeletewhile column deletes useSubmit.ColumnDelete. For consistency, this should useSubmit.RowDelete.
743-745: Incomplete guard coverage formakeTableColumns.While this reactive statement guards subsequent calls to
makeTableColumns(), it doesn't protect:
- The initial call in
onMount()at line 136- The actual
.map()operation insidemakeTableColumns()(already flagged in past review at lines 155-169)Consider adding a guard in
onMount:onMount(async () => { columnsOrder.set(preferences.getColumnOrder(tableId)); columnsWidth.set(preferences.getColumnWidths(tableId)); - makeTableColumns(); + if (table?.fields) makeTableColumns();
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (1)
140-147: Consider renaming loop variable for consistency.The loop variable is named
table, but you're iterating overentityItems. Consider renaming it toentityfor clarity and consistency with the new entity-centric terminology.Apply this diff:
-{#each entityItems as table} +{#each entityItems as entity} <Table.Row.Base {root}> - <Table.Cell {root}>{table.name}</Table.Cell> + <Table.Cell {root}>{entity.name}</Table.Cell> <Table.Cell {root}> - <DualTimeView time={table.updatedAt} /> + <DualTimeView time={entity.updatedAt} /> </Table.Cell> </Table.Row.Base> {/each}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lib/helpers/string.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(18 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(15 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/helpers/string.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T04:31:05.213Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/options.svelte:33-42
Timestamp: 2025-09-25T04:31:05.213Z
Learning: Svelte 5 uses `onclick` instead of `on:click` for event handlers. Event handlers are now properties like any other rather than directives that require the `on:` prefix.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (10)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (2)
1-495: Missing terminology initialization.The AI summary states that
setTerminologies(page)should be invoked reactively, but there's no import ofsetTerminologiesfrom$database/(entity)helpers and no reactive statement calling it. This is likely required for the multi-terminology support feature.Please verify whether terminology initialization is needed and, if so, add the missing import and reactive statement:
+ import { setTerminologies } from '$database/(entity)';And add the reactive statement after the existing reactive declarations (e.g., after line 93):
+ $: setTerminologies(page);
446-458: LGTM! Dependency invalidation now present.The
onCreateIndexcallback now correctly invalidatesDependencies.TABLEafter creating an index (line 457), ensuring the UI reflects the new index. This addresses the previous review comment.src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (2)
152-152: Theonclicksyntax is correct for Svelte 5.A previous review comment incorrectly suggested changing
onclicktoon:click. However, Svelte 5 usesonclick(as a property) instead of theon:clickdirective used in earlier versions. The current implementation is correct.Based on learnings.
74-76: Ignore—databaseSdk.deletecorrectly invokes the per‐type database deletion endpoint. TheuseDatabasesSdkimplementation dispatches totablesDB.deleteordocumentsDB.delete, which remove the database resource as intended.Likely an incorrect or invalid review comment.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (6)
700-701: LGTM: Safe property access pattern.The use of
'tableId' in relatedTablebefore accessing the property is a defensive programming practice that prevents runtime errors.
913-913: LGTM: Consistent bracket notation for dynamic property access.The use of bracket notation
row[columnId]?.['tableId']with optional chaining is appropriate for dynamic property access.
658-658: buildWildcardColumnsQuery signature matches usage
The function is defined asbuildWildcardColumnsQuery(entity: Entity | null = null), so passingtableto it is correct.
85-85: VerifySpreadsheetContainerexport from$database/(entity)
EnsureSpreadsheetContaineris re-exported in the public API surface (e.g. in the entity index file); add the export if it’s missing.
1108-1108: Remove stale review comment ontable.nameusage.The
spreadsheet.sveltecomponent contains notableorselectedRowForDeletebindings, so verifyingtable.namehere is not applicable.Likely an incorrect or invalid review comment.
608-608: table.$id usage is correct Verified thattableis sourced fromdata.table(type Models.Table) which exposes an$id: stringmatching thepage.params.tablevalue; no change required.
| const hasAnyRelationships = $table.columns.some((column) => | ||
| isRelationship(column) | ||
| ); | ||
| const hasAnyRelationships = table.fields.some(isRelationship); |
There was a problem hiding this comment.
Guard table.fields before calling .some().
Similar to the issue at lines 155-169, this will throw if table.fields is undefined.
Apply this diff to add a guard:
- const hasAnyRelationships = table.fields.some(isRelationship);
+ const hasAnyRelationships = table.fields?.some(isRelationship) ?? false;📝 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.
| const hasAnyRelationships = table.fields.some(isRelationship); | |
| const hasAnyRelationships = table.fields?.some(isRelationship) ?? false; |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
around line 378, calling table.fields.some(isRelationship) can throw if
table.fields is undefined; guard it first by checking that table.fields is an
array (e.g. Array.isArray(table.fields) && table.fields.some(isRelationship) or
using optional chaining with a default false), so the expression safely returns
false when fields is missing.
| {#if !isRelationship(structureColumn)} | ||
| <SortButton | ||
| onSort={sort} | ||
| column={column.id} | ||
| state={sortState} /> | ||
| {/if} |
There was a problem hiding this comment.
Guard against undefined structureColumn.
structureColumn (from line 802) could be undefined if the column ID isn't found in $columns. Calling isRelationship(structureColumn) without a null check may cause issues if the function doesn't handle undefined inputs.
Add a guard:
- {#if !isRelationship(structureColumn)}
+ {#if structureColumn && !isRelationship(structureColumn)}
<SortButton
onSort={sort}
column={column.id}
state={sortState} />
{/if}📝 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.
| {#if !isRelationship(structureColumn)} | |
| <SortButton | |
| onSort={sort} | |
| column={column.id} | |
| state={sortState} /> | |
| {/if} | |
| {#if structureColumn && !isRelationship(structureColumn)} | |
| <SortButton | |
| onSort={sort} | |
| column={column.id} | |
| state={sortState} /> | |
| {/if} |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
around lines 828 to 833, calling isRelationship(structureColumn) can receive
undefined if the column ID isn't found in $columns; update the conditional to
first check that structureColumn is truthy before calling isRelationship (e.g.,
structureColumn && !isRelationship(structureColumn) or use optional
chaining/explicit null guard) so the SortButton rendering is skipped when
structureColumn is undefined.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (2)
239-243: Terminology applied to “All …” label — good.
This resolves the earlier hardcoded “All tables” issue.
78-83: Fix invalid Query.orderDesc('') — causes 400s.Use a concrete field or drop ordering.
- entities = await databasesSdk.listEntities({ - databaseId: page.params.database, - queries: [Query.orderDesc(''), Query.limit(100)] - }); + entities = await databasesSdk.listEntities({ + databaseId: page.params.database, + queries: [Query.orderDesc('$createdAt'), Query.limit(100)] + });If you only sort client-side by name anyway, consider removing the order query entirely.
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
181-183: Guard against double slashes when composing hrefs.If action.href can start with '/',
withPath(databaseBaseRoute, \/${action.href}`)` will produce '//' in the middle. Sanitize or ensure hrefs are always relative without a leading slash.Example:
-{@const href = withPath(databaseBaseRoute, `/${action.href}`)} +{@const href = withPath( + databaseBaseRoute, + action.href.startsWith('/') ? action.href : `/${action.href}` +)}src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte (1)
16-16: Drop optional chaining ondatabasein header.svelte. The layout load always supplies a defineddatabase, so replacedatabase?.nameanddatabase?.$idwithdatabase.nameanddatabase.$id.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte(12 hunks)
🔇 Additional comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
47-48: Verify dynamic param lookup and route-id matcher.Using terminology-driven keys for params and route-id suffixes is clever but brittle. Confirm:
- page.params[entityTypeSingular] exists for all entity screens.
- page.route.id consistently ends with
${entityTypeSingular}-[${entityTypeSingular}].Also applies to: 61-63
src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte (3)
7-14: LGTM! Route derivation improves maintainability.The migration from manual path concatenation to
resolveRouteandwithPathaddresses the architectural concern raised in previous reviews. The use of$derivedfor reactive path construction is clean and follows Svelte 5 best practices.
48-68: LGTM! Template structure is clean and well-organized.The template implementation is solid:
- Line 49:
{@const}properly scopesbaseDatabasesPathto where it's needed- Line 63: The
isTabSelectedcall correctly passesbaseDatabasePathandtabsto handle thehasChildrenlogic for nested routesThe optional chaining at lines 52 and 55 is consistent with the database derivation but should be verified (see earlier comment).
18-45: Confirm analytics supports dynamic tab event names. Dynamicevent: terminology.entity.lower.pluralvalues aren’t referenced in any mapping or tracking code—verify with your analytics setup that these generated event names are handled or normalized to avoid breaking dashboards.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte (3)
62-76: Critical: relatedTable used before initialization.The code calls
buildWildcardColumnsQuery(relatedTable)at line 69 whenrelatedTableis stillnull. The entity is only fetched afterward at lines 73-76. This will causebuildWildcardColumnsQueryto fail or produce incorrect results.Apply this diff to fetch the entity first:
if (isSingleStore()) { + // Fetch the entity first + relatedTable = await databaseSdk.getEntity({ + databaseId, + entityId: tableId + }); + const fetchedRow = await sdk .forProject(page.params.region, page.params.project) .tablesDB.getRow({ databaseId, tableId: tableId, rowId: rows as string, queries: buildWildcardColumnsQuery(relatedTable) }); - // cannot use page.data.entities! - relatedTable = await databaseSdk.getEntity({ - databaseId, - entityId: tableId - }); fetchedRows = [fetchedRow];
86-104: Fix reference to non-existent page.data.tables.Line 88 references
page.data.tableswhich doesn't exist. The layout only providespage.data.entities(used correctly at line 103).Apply this diff:
const uniqueTableIds = [...new Set(existingRows.map((row) => row.$tableId))]; const missingTableIds = uniqueTableIds.filter((tableId) => { - return !page.data.tables?.find((table: Models.Table) => table.$id === tableId); + return !page.data.entities?.find((entity: Entity) => entity.$id === tableId); });
111-113: Use fields instead of columns for Entity compatibility.The code accesses
rowTable.columnsbut Entity objects usefields. Additionally, the type annotation should beFieldinstead ofColumns.Apply this diff:
- const hasAllColumns = rowTable.columns.every( - (column: Columns) => column.key in row + const hasAllColumns = rowTable.fields.every( + (field: Field) => field.key in row );
♻️ Duplicate comments (7)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte (1)
354-358: Type narrowing should use Field instead of Models.ColumnRelationship.This issue was flagged in a previous review. The filter should use
Fieldtype with a runtime guard to check fortwoWayproperty instead of casting toModels.ColumnRelationship.Apply this diff:
{@const twoWayKeys = new Set( relatedTable.fields - .filter((column: Models.ColumnRelationship) => column.twoWay) - .map((c) => c.key) + .filter((field: Field) => field.type === 'relationship' && 'twoWay' in field && field.twoWay) + .map((f) => f.key) )}src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts (1)
23-23: Provide a valid sort attribute forQuery.orderDesc.Passing an empty string yields an invalid order-by and the list call will fail. Use a real attribute (e.g.
$updatedAt).Apply this diff:
- queries: [Query.limit(limit), Query.offset(offset), Query.orderDesc('')] + queries: [ + Query.limit(limit), + Query.offset(offset), + Query.orderDesc('$updatedAt') + ]src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte (1)
39-39: Remove redundantcreatingEntitystate.The
creatingEntityflag is unnecessary because the Modal component handles submit state internally viasubmissionLoader, which prevents double submissions and manages the loading indicator. The manual state management on lines 39, 59, 75, 159-160 duplicates functionality already provided by the Modal.Based on learnings
Remove the
creatingEntitydeclaration and its usage:- let creatingEntity = $state(false);async function createEntity() { error = null; - creatingEntity = true; try { const finalId = id || ID.unique(); // early init setup! enableThinkingModeForSuggestions(finalId, name); // create entity. await onCreateEntity(finalId, name); // cleanup updateAndCleanup(); } catch (e) { error = e.message; trackError(e, analyticsCreateSubmit); - } finally { - creatingEntity = false; } }<svelte:fragment slot="footer"> - <Button secondary disabled={creatingEntity} on:click={() => (show = false)}>Cancel</Button> + <Button secondary on:click={() => (show = false)}>Cancel</Button> - <Button submit disabled={creatingEntity} submissionLoader forceShowLoader={creatingEntity} + <Button submit submissionLoader >Create</Button> </svelte:fragment>src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (4)
4-4: Analytics namespace inconsistency persists.This issue was previously flagged. Row-delete success still uses
Click.DatabaseRowDeletewhile column deletes useSubmit.ColumnDelete(line 467) and row-delete errors useSubmit.RowDelete(line 441). For consistency across success/error events, change toSubmit.RowDelete.Also applies to: 422-422
155-169: Guardtable.fieldsbefore mapping - issue persists.This issue was previously flagged. While line 743 guards the reactive call with
if (table.fields) makeTableColumns(), the function is also called inonMountat line 136 without any guard, sotable.fields.map()can still throw if fields is undefined at mount time.Apply this diff to guard the mapping:
function makeTableColumns() { const selectedColumnsToHide = preferences.getCustomTableColumns(tableId); + const fields = table?.fields ?? []; - const baseColumns: Column[] = table.fields.map((field: Field) => { + const baseColumns: Column[] = fields.map((field: Field) => { return { id: field.key, title: field.key,Also guard the onMount call:
onMount(async () => { columnsOrder.set(preferences.getColumnOrder(tableId)); columnsWidth.set(preferences.getColumnWidths(tableId)); - makeTableColumns(); + if (table?.fields) makeTableColumns(); sortState.set(data.currentSort as SortState);
378-378: Guardtable.fields.some()- issue persists.This issue was previously flagged. Calling
table.fields.some(isRelationship)will throw iftable.fieldsis undefined.Apply this diff:
- const hasAnyRelationships = table.fields.some(isRelationship); + const hasAnyRelationships = table.fields?.some(isRelationship) ?? false;
802-802: Guard against undefinedstructureColumn- issue persists.This issue was previously flagged.
structureColumn(from line 802) could be undefined if the column ID isn't found in$columns. The conditional at lines 828-833 callsisRelationship(structureColumn)without first checking ifstructureColumnexists.Apply this diff:
- {#if !isRelationship(structureColumn)} + {#if structureColumn && !isRelationship(structureColumn)} <SortButton onSort={sort} column={column.id} state={sortState} /> {/if}Also applies to: 828-833
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte (1)
22-49: Improve type safety for the tabs array.The tabs array has an inconsistent shape: only the settings tab includes a
disabledproperty, while the others omit it. The.filter((tab) => !tab.disabled)relies onundefinedbeing falsy, which works at runtime but may cause TypeScript to complain. Consider addingdisabled: falseexplicitly to all tabs or conditionally including the settings tab in the array to make the structure uniform.Example refactor to conditionally include the settings tab:
const tabs = $derived( - [ + [ { href: baseDatabasePath, title: terminology.entity.title.plural, event: terminology.entity.lower.plural, hasChildren: true }, { href: withPath(baseDatabasePath, '/backups'), title: 'Backups', event: 'backups', hasChildren: true }, { href: withPath(baseDatabasePath, '/usage'), title: 'Usage', event: 'usage', hasChildren: true }, - { + ...($canWriteDatabases ? [{ href: withPath(baseDatabasePath, '/settings'), event: 'settings', - title: 'Settings', - disabled: !$canWriteDatabases + title: 'Settings' - } - ].filter((tab) => !tab.disabled) + }] : []) + ] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/routes/(console)/project-[region]-[project]/databases/+page.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/relationship.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte(11 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(14 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/routes/(console)/project-[region]-[project]/databases/+page.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/relationship.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-10T12:47:52.618Z
Learnt from: ItzNotABug
PR: appwrite/console#2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts:17-21
Timestamp: 2025-10-10T12:47:52.618Z
Learning: In the database type handling for the console project, the 'legacy' database type maps to the same API as 'tablesdb' (not 'documentsdb'). Legacy is what tablesDB is based upon - they share the same API with just naming differences. The console does not use legacy SDKs in practice.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
PR: appwrite/console#2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts
📚 Learning: 2025-09-25T04:33:19.632Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/createTable.svelte:28-33
Timestamp: 2025-09-25T04:33:19.632Z
Learning: In the Appwrite console createTable component, manual submit state management (like setting creatingTable = true) is not needed because: 1) The Modal component handles submit state internally via submissionLoader, preventing double submissions, and 2) After successful table creation, the entire view is destroyed and replaced with the populated table view, ending the component lifecycle.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte
🧬 Code graph analysis (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.ts (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts (1)
load(6-22)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts (1)
load(8-24)src/lib/stores/database.ts (1)
database(11-11)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
DatabaseType(8-8)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (1)
useDatabasesSdk(16-94)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (4)
Entity(10-14)EntityList(22-25)DatabaseType(8-8)toSupportiveEntity(81-94)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
TerminologyResult(13-23)src/lib/stores/sdk.ts (1)
sdk(149-172)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts (3)
src/routes/(console)/project-[region]-[project]/databases/+page.ts (1)
load(12-42)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts (1)
load(8-24)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts (1)
useDatabasesSdk(16-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (16)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte (4)
17-23: LGTM!The new imports for Entity, Field, getTerminologies, and toRelationalField from the entity module are appropriate for the multi-terminology conversion.
185-222: LGTM!The refactor from
ColumnstoFieldtype is correct and the logic properly handles field arrays and relationships using the new type.
224-250: LGTM!The function correctly uses
relatedTable.fieldswithFieldtype annotations throughout, and the comparison logic is sound.
361-408: LGTM!The template correctly uses
relatedTable.fieldsandFieldtypes, and properly wraps fields withtoRelationalField()for the ColumnItem component.src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte (2)
10-20: LGTM! Derived routing and terminology integration.The use of
resolveRouteto constructbaseDatabasePathandbaseDatabasesPath, along withuseTerminology(page)and$derived(page.data.database), is a solid improvement for maintainability and reactivity.
62-68: LGTM! Tab rendering logic is correct.The use of
tab.eventandisTabSelected(tab, page.url.pathname, baseDatabasePath, tabs)correctly integrates with the derived tabs and base path. ThehasChildren: trueflag on the first three tabs should properly handle child route selection.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte (2)
61-67: LGTM! ID consistency properly maintained.The finalId computation ensures the same ID is used for both
enableThinkingModeForSuggestionsandonCreateEntity, preventing the ID mismatch issue flagged in previous reviews.
72-72: Verify error object hasmessageproperty.The code assumes the caught exception
ehas amessageproperty. While this is true for standard JavaScript Error objects, it may not hold for all thrown values in TypeScript.Consider adding a type guard or fallback:
} catch (e) { - error = e.message; + error = e instanceof Error ? e.message : String(e); trackError(e, analyticsCreateSubmit); } finally {src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (7)
85-85: LGTM!The import refactoring to use the public
$database/(entity)module aligns with the PR's multi-terminology support objectives.
608-608: LGTM!The change from
tableIdtotable.$idis appropriate, assumingtableis guaranteed to exist (see verification request for line 102).
658-658: LGTM!The change from
$tabletotableinbuildWildcardColumnsQueryis consistent with the refactoring approach.Also applies to: 685-685
700-702: LGTM!The defensive type checking for
relatedTablehandles both string and object cases safely.
743-745: LGTM!The reactive guard
if (table.fields)prevents callingmakeTableColumns()when fields is undefined. However, theonMountcall at line 136 still needs a similar guard (see earlier comment).
1108-1108: LGTM!The change from
$table.nametotable.nameis consistent with the refactoring approach and should be safe in these deletion confirmation dialogs.Also applies to: 1115-1115, 1118-1118
102-102: Verify thatdata.tableis always defined
The reactive aliastable = data.tableand subsequent uses (table.$id,table.name,table.fields, etc.) will throw ifdata.tableis undefined. Confirm your load (or module export) always returns a non-nulltableor add appropriate guards/defaults.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte (1)
127-133: Self-assignment pattern and documentation are correctThe added comment clearly explains the reactivity trigger, and since runes aren’t enabled in v5.25.3, the
relatedList = relatedList;assignment is still required.
| delete: (params: { databaseId: string }) => Promise<{}>; | ||
| }; |
There was a problem hiding this comment.
Fix delete API to include the entity identifier.
delete only forwards { databaseId }, but both tablesDB.delete and documentsDB.delete require the entity ID (tableId/collectionId). Runtime calls will fail with missing identifier.
Apply this diff:
- delete: (params: { databaseId: string }) => Promise<{}>;
+ delete: (params: { databaseId: string; entityId: string }) => Promise<{}>;
@@
- case 'legacy': /* databases api */
- case 'tablesdb':
- return await baseSdk.tablesDB.delete(params);
- case 'documentsdb':
- return await baseSdk.documentsDB.delete(params);
+ case 'legacy': /* databases api */
+ case 'tablesdb':
+ return await baseSdk.tablesDB.delete({
+ databaseId: params.databaseId,
+ tableId: params.entityId
+ });
+ case 'documentsdb':
+ return await baseSdk.documentsDB.delete({
+ databaseId: params.databaseId,
+ collectionId: params.entityId
+ });Also applies to: 80-87
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/sdk.ts
around lines 13-14 (also applies to lines 80-87), the delete API currently only
accepts { databaseId } but must also accept the entity identifier
(tableId/collectionId); update the delete signature to accept { databaseId,
entityId } (or named tableId/collectionId as appropriate) and forward both
databaseId and entityId to tablesDB.delete/documentsDB.delete calls, and update
any type annotations and call sites accordingly so the runtime receives the
required identifier.
| <CoverTitle href={baseDatabasesPath} style="margin-inline-start: -2.5rem;"> | ||
| {database?.name} | ||
| </CoverTitle> | ||
|
|
||
| <Id value={$database.$id}>{$database.$id}</Id> | ||
| <Id value={database?.$id}>{database?.$id}</Id> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify that optional chaining for database is intentional.
The use of database?.name and database?.$id suggests that page.data.database can be undefined. Confirm this is the expected data loading behavior (e.g., during initial render or error states). If database should always be present, consider adding error handling or a loading state instead of silently rendering empty values.
🏁 Script executed:
#!/bin/bash
# Description: Check if database is guaranteed to be present in page.data or if optional chaining is needed.
# Search for database loader/data definitions in database-[database] routes
rg -n -A5 'export (const|let|async function) (load|data)' -g '+page*.ts' -g '+layout*.ts' src/routes/\(console\)/project-*/databases/database-\[database\]/ | head -50
# Search for usages of page.data.database to see if null/undefined checks are common
rg -n 'page\.data\.database\?' -g '*.svelte' src/routes/\(console\)/project-*/databases/database-\[database\]/ | head -20Length of output: 6293
3 participants
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
Refactor
Bug Fixes
Chores