Feat: multi-terminology support #2413
Conversation
Console (appwrite/console)Project ID: Sites (2)
Tip SSR frameworks are fully supported with configurable build runtimes |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRefactors many table-specific surfaces into a generic entity model: renames create/store symbols (showCreateTable → showCreateEntity), replaces "columns" with "fields", and exposes a centralized public entity module at $database/(entity) (helpers, views, layouts, settings). Adds terminology, analytics and dependencies helpers, a DocumentsDB entry on the SDK surface, a pluralization helper, navigation utilities, and a SvelteKit alias ($database). Tightens analytics guards and enum entries. Updates numerous routes, components, and stores to consume Entity/Field types, adds/removes multiple UI components, and adjusts load/navigation flows accordingly. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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
![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: 4
📜 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)/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/terminology.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/header.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)/views/breadcrumbs.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/dependencies.ts
🧰 Additional context used
🧬 Code graph analysis (2)
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(62-71)
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)
...sole)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts
Show resolved
Hide resolved
...sole)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts
Show resolved
Hide resolved
...le)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
Show resolved
Hide resolved
...le)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
28-28: Emptyvectordbterminology still causes downstream failures.The
vectordb: {}stub remains empty despite previous feedback. WhenuseTerminologyis called with a vectordb type, the returnedTerminologyResultwill have undefinedentity,field, andrecordproperties, causing analytics handlers to fail (as noted in your past review acknowledgment).Either:
- Remove the vectordb entry until terminology is defined, or
- Define the actual terms for vectordb
Apply this diff to remove the stub:
legacy: { entity: 'collection', field: 'attribute', record: 'document' - }, - vectordb: {} + } } as const;Or define proper terms:
- vectordb: {} + vectordb: { + entity: 'vector', + field: 'dimension', + record: 'embedding' + }
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
4-4: Consider decoupling from table-specificColumnstype.Importing
Columnsfrom the table-specific store path creates tight coupling between this general terminology module and a specific database type's implementation. Since this module aims to support multiple database types (legacy, tablesdb, documentsdb, vectordb), theFieldtype should ideally be more generic or defined locally.Consider defining a generic
Fieldtype here that can accommodate attributes/columns/fields across all database types, rather than importing from a specific implementation.Also applies to: 10-10
42-48: Consider adding explicit return types for type safety.The
transformDatabaseTermsfunction usesObject.fromEntries, which results in loose typing ({ [key: string]: any }). While the logic is correct, adding an explicit return type annotation would improve type safety and IDE support.For example:
type TerminologyVariants = { entity?: { lower: Term; title: Term }; field?: { lower: Term; title: Term }; record?: { lower: Term; title: Term }; }; const transformDatabaseTerms = (terms: Partial<TerminologyShape>): TerminologyVariants => Object.fromEntries( Object.entries(terms).map(([key, term]) => [ key, term ? createTermVariants(term) : undefined ]) ) as TerminologyVariants;
62-76: Good improvement, but validation is incomplete.The addition of the type existence check (lines 64-67) is a good improvement over the previous silent fallback. However, validation is still incomplete: when
typeexists but is not a valid key interminologyData(e.g., a typo or unsupported type), the function silently falls back to an empty object (line 69:terminologyData[type] || {}).To complete the validation, verify that the type is actually supported:
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 (!dbTerminologies || Object.keys(dbTerminologies).length === 0) { + throw new AppwriteException(`Unsupported database type: ${type}`, 500); + } - const dbTerminologies = terminologyData[type] || {}; return { source: dbTerminologies, field: dbTerminologies.field, record: dbTerminologies.record, entity: dbTerminologies.entity }; }This ensures that callers fail fast with a clear error when an unsupported or empty database type is encountered.
📜 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)/helpers/terminology.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
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)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (2)
284-287: Consider using the plural string helper.The manual pluralization logic appears twice in the template. According to the AI summary, a plural string helper has been added to the codebase. Using this helper would eliminate duplication and ensure consistent pluralization across the UI.
If a helper like
plural(count, 'index', 'indexes')orpluralize(count, 'index')is available, consider refactoring:<!-- Line 286-287 --> {length} {plural(length, 'index', 'indexes')} <!-- Line 307-308 --> {selectedIndexes.length > 1 ? 'indexes' : 'index'} <!-- becomes --> {plural(selectedIndexes.length, 'index', 'indexes')}Also applies to: 306-308
89-89: Clean up or document commented-out 'orders' column.The commented-out code for the 'orders' column appears in multiple places with the note "design doesn't have orders atm." If this feature is planned for future implementation, add a TODO or FIXME comment with context. Otherwise, consider removing the commented code to reduce clutter.
Also applies to: 203-204, 238-240
📜 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]/(entity)/views/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.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)/views/header.svelte
🔇 Additional comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (1)
332-336: Delete modal prop types are consistent
TheDeletecomponent’sselectedIndexprop already acceptsModels.ColumnIndex | string[] | nulland handles single vs. bulk deletions. No changes needed.
.../project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte (1)
50-77: Optional: stabilizeentityreference for keyed Id component
The{#key entity.$id}block will remountIdon eachentity.$idchange—ensureentityisn’t recreated on every render to avoid unnecessary remounts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/stores/navigation.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]/table-[table]/header.svelte(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/stores/navigation.ts
🔇 Additional comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte (2)
79-93: LGTM!The styles properly handle the expand/collapse animation with a smooth transition and appropriate accessibility considerations. Disabling
pointer-eventswhen collapsed prevents unintended tab interactions, and thecursor: defaultoverride ensures proper visual feedback.
37-47: Validate dynamic parameter access.The code reads
page.params[terminology.entity.lower.singular]without ensuring the matching dynamic segment exists in your routes. Confirm that every entity route defines a dynamic parameter named afterterminology.entity.lower.singular, or add a guard/fallback inbasePathto avoidundefinedIDs.
...onsole)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/routes/+layout.ts (1)
9-9: Replace remainingbasepath usages withresolve()
There are numerous instances of${base}/…andbase + '/…'across the codebase (e.g. inlib/stores/billing.ts, variousroutes/…files). To ensure consistent routing and path resolution, replace these withresolve('…').
📜 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)/views/layouts/empty.svelte(3 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]/+layout.svelte(4 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]/+page.svelte(4 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]/rows/create.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(3 hunks)src/routes/+layout.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/indexes.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/+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 (15)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
7-7: LGTM! Clean refactoring from table-specific to entity-scoped terminology.The rename from
showCreateTabletoshowCreateEntityis consistent between the import and usage, aligning with the PR's goal to generalize table-specific UI into entity-scoped abstractions. The button text "Create table" remains appropriate for this database tables context.Also applies to: 150-150
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte (1)
13-13: LGTM – SideSheet is correctly exported by$database/(entity)and all imports are consistent.src/routes/+layout.ts (3)
9-9: LGTM! Correct use of SvelteKit'sresolveutility.The import correctly switches from
basetoresolve, which is SvelteKit's recommended approach for path resolution with base path support.
34-34: LGTM! Pathname comparisons correctly useresolve().The comparisons correctly use
resolve()to generate full paths for equality checks. Since bothurl.pathnameandresolve()include the base path, these comparisons will work correctly regardless of base path configuration.Also applies to: 55-55
37-37: LGTM! Redirects correctly useresolve()for path generation.All redirect calls correctly use
resolve()to generate full paths with base path support. The combination withwithParamshelper properly preserves query parameters.Also applies to: 59-59, 67-67
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/empty.svelte (2)
14-17: LGTM: Import updates align with module restructuring.The migration to
$databasealias-based imports consolidates entity-scoped primitives and improves code organization.
27-44: Approve callback integration: Verified that the table page component (+page.svelte:236) correctly suppliesonOpenCreateColumnto the layout; no further action needed.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (4)
43-66: LGTM! Centralized imports align with entity-based refactoring.The migration of
SideSheet,CreateEntity, andCreateIndexto the centralized$database/(entity)module is consistent with the PR's goal of generalizing table-specific UI into entity-scoped abstractions.
97-99: LGTM! Comment clarification improves readability.The updated comments clearly distinguish AI-driven index and column creation scenarios.
115-115: LGTM! Callback correctly uses renamed store.The update from
showCreateTabletoshowCreateEntityis consistent with the entity-centric refactoring.
446-460: Handle errors in onCreateIndex
In your CreateIndex component (e.g. CreateIndex.svelte), wrap the SDK call in a try/catch and surface any errors to the user (toast, inline message, etc.) so failures aren’t silent.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (4)
19-20: LGTM! Import reordering improves organization.The reordering of
expandTabsandcolumnsOrderimports has no functional impact and may reflect alphabetical or logical grouping.
35-36: LGTM! Imports align with entity-based centralization.Moving
EmptySheetto the centralized$database/(entity)module is consistent with the PR's refactoring strategy.
54-55: LGTM! Icon mapping enhances column type visibility.Adding icon lookup from
columnOptionsimproves the UI by visually distinguishing column types.
246-251: LGTM! Callback correctly initializes create column sheet.The
onOpenCreateColumncallback properly sets up the create column sheet state with the necessary context (title, columns, columnsOrder).
...project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/empty.svelte
Outdated
Show resolved
Hide resolved
...nsole)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
Outdated
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte (1)
39-39: Confirm whether manual submit state is still necessary.According to past review comments, the
creatingEntityflag was identified as redundant because the Modal component handles submit state internally viasubmissionLoader. You responded that it was fixed, but the flag is still present in the current code (lines 39, 59, 73, 158-159).If the Modal's
submissionLoaderand$submittingcontext are sufficient, remove thecreatingEntityflag and usedisabled={$submitting}on the buttons instead. If there's a specific reason this manual management is still needed after the refactor, please clarify.Based on learnings
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte (2)
36-37: Add explicit type annotations for nullable state.The
idanderrorstate variables are initialized tonullwithout explicit type annotations, which may lead to type inference issues in TypeScript.Apply this diff to add explicit types:
- let id = $state(null); - let error = $state(null); + let id = $state<string | null>(null); + let error = $state<string | null>(null);
89-89: Use separate assignments for clarity.Chained assignment (
name = id = null) is less readable than 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 (1)
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
⏰ 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
...nsole)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte
Outdated
Show resolved
Hide resolved
...le)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Outdated
Show resolved
Hide resolved
...ject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/select.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte (2)
99-101: Inconsistent property access:columnsvsfields.Line 99 accesses
rowTable.columns, butrowTableis derived from the sameallTablesarray asrelatedTable(which consistently uses.fieldsthroughout this file). This inconsistency will cause a runtime error if the Entity type only exposes afieldsproperty.Apply this diff to use the correct property name:
- const hasAllColumns = rowTable.columns.every( + const hasAllColumns = rowTable.fields.every( (column: Columns) => column.key in row );
91-92: Replace.columnswith.fieldsforrowTable
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte line 99: change
toconst hasAllColumns = rowTable.columns.every(const hasAllColumns = rowTable.fields.every(src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte (1)
1-71: Fix Prettier formatting issues before merging.The pipeline reports code style violations across the entire file. Run
prettier --writeon this file to resolve formatting inconsistencies.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)
155-167: Possible crash when table.fields is undefinedtable.fields is optional in the new Entity model. makeTableColumns() is called onMount without a guard; mapping on undefined will throw. Make the map null-safe.
- const baseColumns: Column[] = table.fields.map((col: Columns) => ({ + const baseColumns: Column[] = (table?.fields ?? []).map((col: Columns) => ({ id: col.key, title: col.key, type: col.type as ColumnType, hide: !!selectedColumnsToHide?.includes(col.key), array: col?.array, width: getColumnWidth(col.key, { min: minimumWidth }), minimumWidth: minimumWidth, draggable: true, icon: getAppropriateIcon(col.type), format: 'format' in col && col?.format === 'enum' ? col.format : null, elements: 'elements' in col ? col.elements : null }));Also guard the onMount caller or rely solely on the reactive block that already checks table.fields.
♻️ Duplicate comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte (1)
19-25: HandlenullingetKeysto prevent runtime crash.
selectedIndexdefaults tonull(Line 10) and we also set it back tonullinsidecleanup(). The derivedselectedKeysimmediately callsgetKeys(selectedIndex), so anynullvalue crashes withselected.key. Please acceptnullingetKeysand return an empty array so the component stays stable.- let selectedKeys = $derived(getKeys(selectedIndex)); + let selectedKeys = $derived(getKeys(selectedIndex)); … - function getKeys(selected: Index | string[]): string[] { - return Array.isArray(selected) ? selected : [selected.key]; + function getKeys(selected: Index | string[] | null): string[] { + if (selected === null) return []; + return Array.isArray(selected) ? selected : [selected.key]; }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
453-470: Resolved: invalidate after index creationDependency invalidation after creating an index is now present. This addresses the earlier review note about stale UI.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte (1)
2-8: Fix callback type: lengths emits nulls but is typed number[].create() builds lengths with nulls for fields without a length; the callback declares number[]. This will fail type-checking and mislead consumers. Update the type (or normalize payload).
export type CreateIndexesCallbackType = { key: string; type: IndexType; fields: string[]; - lengths: number[]; + lengths: Array<number | null>; orders: string[]; };
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte (1)
79-92: LGTM! Cleanup logic is appropriate.The function properly updates navigation, notifies the user, tracks analytics, and resets state after successful entity creation.
Optional: Consider separate assignments on Line 91 for clarity.
The chained assignment
name = id = nullworks correctly but could be more explicit:- name = id = null; + name = null; + id = null;src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)
376-379: Null-safe relationship check before bulk deleteIf fields are absent, .some() will throw. Make it resilient.
- const hasAnyRelationships = table.fields.some((column: Columns) => - isRelationship(column) - ); + const hasAnyRelationships = + table?.fields?.some((column: Columns) => isRelationship(column)) ?? false;src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
144-144: Optional chaining to avoid edge-case crashDisabled check can crash if table is transiently undefined. Make it null-safe.
- disabled: page.url.pathname.endsWith(table.$id), + disabled: page.url.pathname.endsWith(table?.$id ?? ''),src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte (1)
105-114: Prefer find() over filter()[0] and simplify isSpatial check.Small clarity refactor; avoids array allocation and length juggling.
- const field = entity.fields.filter((field) => externalFieldKey === field.key); - const isSpatial = field.length && isSpatialType(field[0]); + const f = entity.fields.find((x) => externalFieldKey === x.key); + const isSpatial = !!f && isSpatialType(f);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
src/lib/helpers/string.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)/views/create.svelte(5 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/overview.svelte(3 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/settings/security.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)/indexes.svelte(12 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/store.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(19 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(8 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/deleteColumn.svelte(2 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]/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(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/editPermissions.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editRelated.svelte(5 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]/settings/+page.svelte(1 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)
✅ Files skipped from review due to trivial changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/+page.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
🧰 Additional context used
🧠 Learnings (3)
📚 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: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
📚 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 (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts (2)
src/lib/stores/sdk.ts (1)
sdk(149-172)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
toSupportiveEntity(78-91)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
Entity(10-15)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.ts (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
Entity(10-15)
🪛 GitHub Actions: Tests
src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte
[warning] 1-1: Code style issues found in the file. Run Prettier with --write to fix.
⏰ 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 (36)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte (2)
59-59: Verify tooltip placement logic.The tooltip placement switches from
'right'when collapsed toundefined(default) when expanded. Typically, constrained spaces (collapsed state) benefit from auto/default positioning to avoid clipping. Please confirm this behavior is intentional for your layout.
37-47: Verify dynamic parameter key consistency. The expressionpage.params[entityType]assumesterminology.entity.lower.singularmatches the route-param key (e.g. “table”). Confirm this alignment for all current and future entity types to avoid undefined IDs.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/editPermissions.svelte (1)
12-20: Good refactoring to prop-based component.The migration from a store-based
tablereference to a typed prop is a solid improvement. TheEntitytype import and prop signature are correctly structured.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte (7)
3-14: LGTM! Imports support the entity abstraction.The updated imports appropriately support the refactored entity creation flow with terminology abstraction, suggestions, and improved analytics.
16-24: LGTM! Props interface is cleaner and more flexible.The refactored props make the component more generic and reusable:
- Simplified
showbinding- Configurable
useSuggestionsfeature- Generic
onCreateEntitycallback enables usage across different entity types
26-33: LGTM! Terminology abstraction enables multi-entity support.The dynamic extraction of terminology enables this component to work seamlessly across different entity types (tables, collections, etc.) with appropriate labels and analytics tracking.
35-38: LGTM! State additions support the refactored flow.The addition of
idanderrorstate variables appropriately supports the new entity creation flow with custom ID handling and granular error management.
41-55: LGTM! Suggestions integration is well-structured.The function appropriately guards against cases where suggestions are disabled and correctly initializes the thinking mode with consistent entity identification.
57-77: LGTM! Entity creation flow is robust.The refactored flow correctly:
- Uses a consistent
finalIdfor both thinking mode initialization and entity creation- Resets state in a
finallyblock to ensure UI re-enablement in all scenarios- Properly handles errors with tracking and user feedback
105-162: LGTM! Effects and template leverage terminology effectively.The reactive effects appropriately handle:
- Auto-generation of IDs from entity names
- State reset when modal closes
- Suggestions cleanup on navigation
The template correctly uses dynamic terminology for labels, placeholders, and titles, making the component truly generic across entity types.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/relationship.svelte (2)
68-71: LGTM! Clean refactoring from store to prop.The migration from using a
$tablestore to accepting atableprop of typeEntityaligns well with the PR's entity-centric abstraction goals. The new approach provides better component isolation and explicit data flow.
124-244: Verify Entity type and prop reactivity
- Confirm
Models.CollectionandModels.Tableboth define$id: stringandname: string.- Ensure parent components pass the
tableprop in a way that Svelte’s reactivity picks up updates.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte (1)
23-34: Remove suggestion: dynamic parameter access is safe.
entityis derived fromgetTerminologies()as"table"or"collection", matching the[table]/[collection]route segments, and SvelteKit guarantees those params exist when this component renders.Likely an incorrect or invalid review comment.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
23-25: Helpful type exportMaking
Tablepublic unblocks the shared entity helpers without altering runtime behavior. 👍src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts (1)
5-19: Good alignment with the entity abstractionFetching the raw table once and exposing it via
toSupportiveEntitykeeps the layout contract consistent while letting downstream code rely on unified fields/recordSecurity semantics.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.ts (1)
4-39: Typing matches the shared entity modelSwapping the table parameter to the new
Entitysurface cleans up the generics aroundbuildGridQuerieswithout touching the underlying query logic. Looks great.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte (1)
13-205: Clean transition to entity fieldsThe shift to
table.fields/table.recordSecuritykeeps the row-creation flow in sync with the new terminology while preserving the existing UX. Nicely done.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte (1)
11-181: Entity-aware edit flow looks solidReusing
table.databaseId/table.$idand iterating viatable.fieldsties the edit sheet neatly into the shared entity model without disturbing the diffing logic.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (4)
85-86: Use public SpreadsheetContainer exportGood switch to public export path. Ensure $database/(entity) exposes a named export SpreadsheetContainer.
421-423: Analytics: correct event for row deleteSwitch to Click.DatabaseRowDelete looks good and aligns with the new Click enum.
701-705: Support object-based related table identifiersHandling related objects with a tableId property is a good addition.
1105-1116: Terminology update in confirmationsUsing table.name and updated phrasing is consistent with the entity-centric model.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (4)
35-36: Use public entity exportsImporting EmptySheet and columnOptions from public surfaces is aligned with the new API surface.
75-79: Reactive init guarded by fields presenceGood: compute columns only when table.fields exists.
247-253: Create column sheet wiringWiring columns and columnsOrder into the create sheet is correct and matches the new store surface.
286-287: Propagate table to CreateRowPassing {table} directly is consistent with the refactor.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (4)
121-122: Command keys routing guardUse of table?.$id in the keys condition is correct; avoids errors during initial render.
353-354: Title uses entity nameUsing table?.name in the title reflects the new terminology accurately.
481-482: Pass table to Row Permissions editorPropagating table to EditRowPermissions aligns with entity-centric flows.
506-506: Indexes suggestions integrationPassing {table} keeps suggestions aligned with the active entity.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte (3)
47-47: LGTM! Centralized imports improve maintainability.Consolidating component imports from
$database/(entity)creates a cleaner public API and makes it easier to maintain the entity abstraction boundary.
562-562: Title updated to align with terminology.The change from "Create attribute" to "Create column" maintains consistency with the multi-terminology support introduced in this PR.
547-550: Ensuredata.tableis correctly typed and defined
Unable to locate itsPageDataorload—please verifydata.tableis anEntityand always defined before rendering<DeleteColumn>.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/deleteColumn.svelte (2)
13-23: Approve DeleteColumn prop usage
All<DeleteColumn>instances now receive a requiredtableprop (e.g., in +page.svelte), so removing optional chaining ontable.nameis safe.
98-103: No null-safety issue: the table prop is guaranteed by the parent layout load, sotable.namewill always be defined.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte (1)
19-19: Verify$database/table-[table]store imports. We locatedrows/store.tsandcolumns/store.tsundersrc/routes/(console)/…/databases/…/table-[table]; ensure your$databasealias still resolves there or re-export these modules via the entity layer to avoid build errors.
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Outdated
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Show resolved
Hide resolved
...nsole)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
Outdated
Show resolved
Hide resolved
...console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte
Show resolved
Hide resolved
...onsole)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte
Outdated
Show resolved
Hide resolved
...t-[region]-[project]/databases/database-[database]/table-[table]/rows/editPermissions.svelte
Show resolved
Hide resolved
...onsole)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
254-256: Guard sample-data generation against missing fields.
table.fieldsmay be undefined. Accessing.some()and.filter()on it will throw a TypeError.Apply this diff to add null-safe defaults:
- 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)); + const filteredColumns = columns.filter((col: Columns) => col.type !== 'relationship');
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (2)
24-24: Add type annotation for tableItems.The
tableItemsarray lacks explicit typing, reducing type safety.-let tableItems = $state([]); +let tableItems = $state<Array<{ id: string; name: string; updatedAt: string }>>([]);
101-112: Consider combining effects for clarity.The two separate
$effectblocks could be combined into a single effect that handles both cleanup and initialization based onshowDelete.-$effect(() => { - if (!showDelete) { - tables = null; - tableItems = []; - } -}); - -$effect(() => { - if (showDelete) { - listTables(); - } -}); +$effect(() => { + if (showDelete) { + listTables(); + } else { + tables = null; + tableItems = []; + confirmedDeletion = false; + error = false; + } +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/breadcrumbs.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]/breadcrumbs.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte(7 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(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(18 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/breadcrumbs.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.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]/store.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
⏰ 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 (8)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts (1)
1-5: Rename aligns with new entity terminology
showCreateEntitykeeps the same behavior while matching the updated vocabulary shift. Looks good.src/routes/(console)/project-[region]-[project]/databases/database-[database]/breadcrumbs.svelte (1)
12-34: LGTM: Dynamic route resolution improves maintainability.The refactor from static hrefs to
resolveRoutemakes breadcrumbs more maintainable and consistent with the routing system. The fallback logic for organization ID (organization?.$id ?? project.teamId) is sound, assuming the type inconsistency flagged separately is resolved.src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte (1)
28-28: LGTM! Svelte 5 migration is correctly implemented.The migration to Svelte 5 runes is properly executed:
- Props now use
$props()with correct type annotation- State variables use
$state()for reactivity- Derived state replaces the reactive statement with
$derived()- Function parameter has explicit typing for better type safety
Also applies to: 31-31, 33-33, 90-90
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (2)
443-455: LGTM! Invalidation issue resolved.The index creation handler now correctly invalidates
Dependencies.TABLEafter creating the index, ensuring the UI reflects the new index without requiring a manual refresh.
492-492: ConfirmIndexesSuggestionssafely handles undefinedtableprop. Unable to locate its implementation automatically; please manually verify it guards againsttablebeing undefined on initial render.src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte (1)
69-139: Ensuredatabaseis always defined before rendering: add server-side validation in the load function (e.g. throw a 404 ifdatabaseis null) or render a fallback UI whendatabaseis missing.src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte (2)
14-18: LGTM! Props use correct Svelte 5 runes syntax.The
$bindableand$propsusage follows Svelte 5 conventions for two-way binding.
27-27: No need to guard page.data.database
The parent+layout.ts(src/routes/.../database-[database]/+layout.ts) always loads and returnsdatabase, sopage.data.databaseis guaranteed when this component mounts.Likely an incorrect or invalid review comment.
...utes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte
Outdated
Show resolved
Hide resolved
...utes/(console)/project-[region]-[project]/databases/database-[database]/backups/table.svelte
Outdated
Show resolved
Hide resolved
...routes/(console)/project-[region]-[project]/databases/database-[database]/breadcrumbs.svelte
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte
Show resolved
Hide resolved
...tes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte
Outdated
Show resolved
Hide resolved
...nsole)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.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]/(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
Outdated
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
src/routes/(console)/project-[region]-[project]/overview/platforms/+page.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 (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.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/databases/database-[database]/delete.svelte
Outdated
Show resolved
Hide resolved
...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
fix: alert aligment issue in max file size card
fix: tooltip content truncated in billing ui table
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