Feat: multi-terminology support #2413
Conversation
ConsoleProject ID: Sites (2)
Note Appwrite has a Discord community with over 16 000 members. |
WalkthroughGeneralizes table-specific UI into entity-scoped abstractions across the database console. Adds terminology, analytics, and dependency helper modules with types and Svelte context wiring; introduces many new entity-focused Svelte components, layout stores, and index barrels; centralizes exports under a new $database alias. Updates analytics (adds Click.DatabaseRowDelete, fixes AuthInvalidateSession, remaps several Submit entries and tightens tracking guards), exposes documentsDB on the SDK, adds a plural string helper, tweaks backup/restore payload handling, and renames showCreateTable → showCreateEntity across stores and usages. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
...le)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
Show resolved
Hide resolved
update: make `createTable` as a reusable component like `createEntity`.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte (1)
32-32: Consider addressing the TODO comment.Line 32 notes uncertainty about whether these should be "attributes or columns." With the new terminology system in place, this might be a good opportunity to clarify and use the appropriate terminology-driven label.
Would you like me to help generate a solution that uses terminology for this field label, or open an issue to track this refinement?
Also applies to: 38-38
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte (1)
17-17: Address the TODO comment.The TODO indicates that this component may need to handle
documentSecurityin addition torowSecurity. Please clarify whether this is planned for a future update or should be implemented now.Do you want me to open a new issue to track this task, or is this already covered by existing plans?
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts (1)
20-22: Consider error handling for missing context.If
getTerminologies()is called beforesetTerminologies()has been invoked,getContextwill returnundefined, potentially causing runtime errors in consuming components.Consider adding a guard to provide a helpful error message:
export function getTerminologies(): Terminologies { - return getContext<Terminologies>(TERMINOLOGIES_KEY); + const terminologies = getContext<Terminologies>(TERMINOLOGIES_KEY); + if (!terminologies) { + throw new Error('Terminologies context not initialized. Call setTerminologies() first.'); + } + return terminologies; }src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (1)
67-67: Remove or document commented code.The commented
orderscolumn should either be removed if not needed or documented with a reason if it's planned for future implementation.Apply this diff to remove the commented line:
{ id: 'columns' }, - // { id: 'orders' }, // design doesn't have orders atm { id: 'lengths' },Or document why it's commented:
- // { id: 'orders' }, // design doesn't have orders atm + // TODO: Add 'orders' column once design is finalized (see issue #XXXX) + // { id: 'orders' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (66)
src/lib/actions/analytics.ts(3 hunks)src/lib/components/backupRestoreBox.svelte(2 hunks)src/lib/helpers/string.ts(1 hunks)src/lib/stores/navigation.ts(1 hunks)src/lib/stores/sdk.ts(2 hunks)src/routes/(console)/project-[region]-[project]/auth/security/updateSessionInvalidation.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/dependencies.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/breadcrumbs.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte(6 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/failedModal.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/field/csvDisabled.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/field/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte(9 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/select.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/empty.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/sidesheet.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/spreadsheet.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/store.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/danger.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/index.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/name.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/permissions.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/security.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/status.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/indexes.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/options.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/activity/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/breadcrumbs.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/createColumnDropdown.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/csvDisabled.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/failedModal.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/dangerZone.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/deleteTable.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateName.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updatePermissions.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateSecurity.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateStatus.svelte(0 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/usage/[[period]]/+page.svelte(1 hunks)svelte.config.js(1 hunks)
💤 Files with no reviewable changes (9)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/breadcrumbs.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateName.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/deleteTable.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/csvDisabled.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateStatus.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updatePermissions.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/dangerZone.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/settings/updateSecurity.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/failedModal.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T04:33:19.632Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/createTable.svelte:28-33
Timestamp: 2025-09-25T04:33:19.632Z
Learning: In the Appwrite console createTable component, manual submit state management (like setting creatingTable = true) is not needed because: 1) The Modal component handles submit state internally via submissionLoader, preventing double submissions, and 2) After successful table creation, the entire view is destroyed and replaced with the populated table view, ending the component lifecycle.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte
🧬 Code graph analysis (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
Columns(9-21)src/lib/helpers/string.ts (3)
singular(20-22)plural(37-59)capitalize(8-10)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
Term(11-11)TerminologyShape(5-9)TerminologyResult(13-22)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/dependencies.ts (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
TerminologyResult(13-22)DependenciesResult(61-66)Term(11-11)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
useTerminology(59-68)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
TerminologyResult(13-22)AnalyticsResult(47-58)TerminologyShape(5-9)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
useTerminology(59-68)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (3)
AnalyticsResult(47-58)TerminologyResult(13-22)DependenciesResult(61-66)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
useTerminology(59-68)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts (1)
useAnalytics(8-51)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/dependencies.ts (1)
useDependencies(8-24)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/types.ts (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts (1)
baseTerminology(14-30)
🔇 Additional comments (63)
src/lib/components/backupRestoreBox.svelte (2)
60-62: LGTM! Clear variable naming.The destructuring with aliasing (
$collection: type) improves readability within the function scope. The comment helpfully clarifies that the payload structure still uses$collectionas an operation type identifier.
63-85: Consistent refactor throughout.All references to the renamed
typevariable are updated correctly:
- Early return guard (line 63)
- Type membership check (line 67)
- Map access (lines 68, 85)
- Type comparison (line 75)
The logic and behavior are preserved.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/usage/[[period]]/+page.svelte (4)
2-3: LGTM! Imports align with entity terminology refactor.The updated imports correctly use the PageProps type for type safety and source the Usage component from the new centralized '$database/(entity)' location, which aligns with the PR's goal of generalizing from table-specific to entity-based terminology.
5-5: LGTM! Proper use of Svelte 5 runes.The migration from
export let datatolet { data }: PageProps = $props()correctly adopts Svelte 5's runes API while maintaining type safety.
7-8: LGTM! Clean use of $derived for reactive values.The derived constants correctly use Svelte 5's
$derivedrune to compute count and total from the page data. The property access is straightforward and the naming is clear.
11-11: Ignore entity-based route suggestion. The codebase currently usestable-[table]routes and has noentity-routes or imports, so no action needed.Likely an incorrect or invalid review comment.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/header.svelte (1)
73-75: Header migration looks correct.Props, bindings, and the preference-backed expand state all line up with the new shared component’s API. 👍
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/create.svelte (1)
13-13: LGTM!The import change correctly adopts the new public module surface for
SideSheet, aligning with the PR's centralization of UI components.src/lib/stores/sdk.ts (2)
24-25: Clarify the "DocumentsDB later" comment.The comment
// DocumentsDB latersuggests this might be incomplete or a placeholder. Please clarify whether this is intended for this PR or if it should be removed or completed before merging.
135-135: Clarify the "documentsDB later" comment.The comment
// documentsDB latersuggests this might be incomplete or a placeholder. Please clarify whether this is intended for this PR or if it should be removed or completed before merging.svelte.config.js (1)
22-24: LGTM!The new
$databasealias is correctly configured and enables the centralized import pattern used throughout the PR.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/options.svelte (1)
5-5: LGTM!The import change correctly adopts the new public module surface for
SideSheet, consistent with the PR's centralization effort.src/lib/helpers/string.ts (1)
24-59: Reasonable implementation for common English pluralization.The function correctly handles common pluralization rules (sibilants, consonant+y, vowel+y, default). Note that it does not handle less common cases like words ending in "f"/"fe" (e.g., "leaf" → "leaves") or irregular plurals (e.g., "child" → "children"), but the documentation accurately describes it as handling "common English pluralization rules."
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/indexes.svelte (1)
12-12: LGTM!The import change correctly adopts the new public module surface for
SideSheet, consistent with the PR's centralization effort.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (4)
43-43: LGTM!The import changes correctly adopt the new public module surface and reflect the terminology shift from "table" to "entity."
Also applies to: 49-49, 66-66
97-99: LGTM!The updated comments improve clarity by documenting when dependency invalidation is skipped during AI-driven index and column creation.
115-115: LGTM!The store name change from
showCreateTabletoshowCreateEntityaligns with the PR's terminology shift.
446-460: LGTM!The
CreateIndexcomponent now accepts a generalizedentityprop and usesexternalFieldKeyinstead ofexternalColumnKey, aligning with the PR's terminology shift. TheonCreateIndexhandler correctly maps the internalfieldsterminology to the API'scolumnsparameter.src/lib/stores/navigation.ts (3)
12-14: LGTM!The
withPathfunction correctly joins path segments and normalizes trailing slashes.
16-21: LGTM!The type casting is necessary to align runtime values with TypeScript's conditional types. The pattern is appropriate for this use case.
23-29: LGTM!The
navigatefunction correctly composesresolveRouteandgoto, providing a convenient navigation helper.src/routes/(console)/project-[region]-[project]/auth/security/updateSessionInvalidation.svelte (1)
25-25: LGTM! Typo fix in analytics enum.The correction from
AuthInvalidateSesssion(three 's') toAuthInvalidateSession(two 's') is correct and aligns with standard spelling.Also applies to: 31-31
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/overview.svelte (1)
5-5: LGTM! Proper terminology integration.The component correctly integrates the terminology system to derive dynamic entity labels.
Also applies to: 13-14
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/field/csvDisabled.svelte (1)
1-16: LGTM! Clean wrapper component.The
CsvDisabledcomponent is well-implemented with proper typing and a clear purpose—wrapping UI elements to indicate they're disabled during CSV import.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/field/index.ts (1)
1-1: LGTM! Proper barrel export pattern.This barrel file correctly re-exports the default export from
csvDisabled.svelteas a named exportCsvDisabled, enabling the named imports used in other files.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.svelte (2)
7-7: LGTM! Proper terminology integration.The component correctly integrates the terminology system to derive display strings and paths.
Also applies to: 18-22
28-28: Verify dynamic property access is safe.
Ensure thatterminology.entity.lower.singular(e.g. "table") corresponds to an actual[table]dynamic segment in your SvelteKit route hierarchy and thatpage.params[entity]always exists when this component renders.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/createColumnDropdown.svelte (1)
7-7: CsvDisabled import matches named export –CsvDisabledis correctly exported from$database/(entity).src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.ts (1)
5-5: No action needed: Breadcrumbs import is correct Verified thatBreadcrumbsis re-exported as a named export in the entity index.src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts (1)
8-8: AllshowCreateTablereferences removed
No occurrences ofshowCreateTableremain in.tsor.sveltefiles.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (3)
86-86: LGTM: Centralized public export.The import now uses the public module alias
$database/(entity)instead of a local relative path. This aligns with the PR's goal of centralizing entity-related exports.
422-422: Verify analytics event naming consistency. Successful deletes fire Click.DatabaseRowDelete, while errors still log Submit.RowDelete—confirm both align with analytics conventions and downstream consumers expect these distinct event names.
4-4: Analytics import completeness verified. BothClick.DatabaseRowDeleteand theSubmitevents (ColumnDelete,RowUpdate,RowDelete) are exported in the analytics module.src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
7-7: LGTM: Consistent terminology refactor.The store import and usage have been updated from
showCreateTabletoshowCreateEntity, aligning with the PR's goal of generalizing from table-specific to entity-generic terminology. The change is applied consistently at both the import and usage sites.Also applies to: 149-149
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (1)
17-17: LGTM: Public module import pattern.The import now uses a named export from the public
$database/(entity)module instead of a local default import. This centralizes theSpreadsheetContainercomponent and aligns with the PR's public API surface strategy.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/activity/+page.svelte (1)
5-9: LGTM: Svelte 5 runes pattern.The component now uses the
$props()rune to destructure thedataprop instead ofexport let data. This is consistent with Svelte 5's runes API and modern component patterns. The data is still properly typed asPageDataand passed to theActivitycomponent.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/status.svelte (2)
1-40: LGTM: Well-structured status component.The component properly:
- Uses Svelte 5 runes (
$props(),$state()) for reactive state management- Integrates with the terminology system via
getTerminologies()for dynamic analytics and dependency keys- Implements proper error handling with both success and error notification paths
- Tracks analytics events using terminology-based event names
- Invalidates the correct dependency after successful updates
The cleanup flow (lines 22-29) and error handling (lines 35-37) follow best practices.
58-58: Verify button disabled logic.The Update button is disabled when
enabled === entity.enabled. Ensure this comparison works correctly for boolean values and that the button becomes enabled when the user toggles the switch to a different value than the entity's current status.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/header.svelte (2)
52-63: LGTM: Proper collapsed state handling.The tabs container correctly handles the collapsed state by:
- Transitioning opacity smoothly (300ms cubic-bezier)
- Disabling pointer events when collapsed
- Changing cursor to default for collapsed tabs
This provides good UX and prevents interaction with hidden tabs.
30-34: No action needed: dynamic param names align with entity types. All routes use bracketed segments named after the lowercase singular entity (e.g.[table],[collection]), matchingentityTypefromuseTerminology.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/index.ts (1)
1-3: LGTM: Clean barrel exports.This barrel file properly consolidates the layout components (
EmptySheet,SideSheet,SpreadsheetContainer) into a public API surface, enabling centralized imports via the$database/(entity)alias throughout the codebase.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/index.ts (1)
1-2: LGTM: Proper type and component exports.The barrel correctly exports both the
Indexesview component and theCreateIndexcomponent along with its associatedCreateIndexesCallbackTypetype. This provides a complete public API surface for index management functionality.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte (2)
46-46: LGTM: Import consolidation.The consolidation of multiple component imports into a single import from
$database/(entity)improves maintainability and aligns with the new public API surface.
482-482: LGTM: Improved modal rendering pattern.The change from conditional rendering to always rendering the FailedModal with
bind:showis cleaner and avoids unnecessary component mounting/unmounting cycles. The title update to "Create column" is consistent with the terminology refactor.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (3)
19-20: LGTM: Import and export updates.The addition of
columnsOrderto the exports and the consolidation ofEmptySheetimport from the public$database/(entity)path improve the module's API surface and maintainability.Also applies to: 35-36
54-56: LGTM: Icon derivation for columns.The addition of the
iconproperty derived fromcolumnOptionsensures consistent iconography across the UI and aligns with the enhanced column type surface.
236-241: LGTM: Enhanced empty state handler.The
onOpenCreateColumnhandler provides a cleaner API for opening the create column sheet with proper initialization. The explicit setting of title, columns, and columnsOrder ensures the sheet has all necessary context.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/init.ts (1)
28-35: LGTM: Clean composition pattern.The
buildTerminologiesfunction cleanly composes the terminology, analytics, and dependencies helpers into a cohesive object. This centralization simplifies consumption across components.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte (2)
1-89: LGTM: Well-structured indexes view.The component demonstrates good practices:
- Proper state management for modals and selection
- Permission checks before allowing index creation
- Status badge handling with error details
- Analytics tracking on user actions
- Responsive empty states via snippets
222-242: LGTM: Proper bulk selection UI.The floating action bar implementation correctly handles bulk selection state, provides clear feedback on the number of selected items, and offers cancel/delete actions.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/settings/permissions.svelte (2)
1-67: LGTM: Well-implemented permissions view.The component demonstrates solid implementation:
- Proper change detection using
symmetricDifference- Clean separation between update logic and cleanup
- Appropriate error handling with notifications and analytics
- Terminology-driven UI labels for consistency
- Disabled state when no changes to prevent unnecessary API calls
46-48: LGTM: Efficient change detection.The use of
symmetricDifferenceto detect permission changes is an efficient and correct approach for comparing arrays. The button will only be enabled when there are actual differences between the current and new permissions.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/breadcrumbs.svelte (1)
8-13: Verify dynamic parameter access
entityIdis derived viapage.params[entityType]; ensure everyterminology.entity.lower.singularvalue (e.g."table","view") corresponds to an existing[<entityType>]route segment under
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/…
so thatpage.params[entityType]never returnsundefined.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte (10)
5-14: LGTM!The import refactoring aligns well with the generalized entity approach. The use of
$databasealias and the newuseTerminologyhelper support the multi-terminology feature.
26-31: LGTM!The dynamic entity terminology derivation using
useTerminology(page)is well-implemented and supports the multi-terminology feature.
34-37: LGTM!State variable renaming from
creatingTabletocreatingEntityaligns with the generalization to support multiple entity types.
39-53: LGTM!The
enableThinkingModeForSuggestionsfunction correctly guards against enabling thinking mode whenuseSuggestionsis false, and the parameters are appropriately generalized.
72-85: Reset logic looks good.The cleanup logic correctly resets state variables and updates the navigation. The chained assignment on lines 83-84 is clear and appropriate.
Note: The analytics event on line 80 has the same potential naming issue as mentioned in the previous comment.
98-109: LGTM!The effect hook correctly manages auto-ID generation and state cleanup when the modal is closed.
111-119: LGTM!The effect hook correctly resets the suggestions table state when navigating to an entity page with the modal open.
122-157: LGTM!The modal markup correctly uses dynamic entity terminology and conditional rendering for suggestions. The button states are properly tied to
creatingEntityfor UI feedback, while the Modal component handles submission state internally to prevent double submissions.Based on learnings.
16-24: All CreateEntity call sites updated Verified that every<CreateEntity>invocation binds the newshowprop and usesonCreateEntity; no further changes needed.
55-70: No analytics event update needed
Submit.TableCreateremains the correct enum entry for this operation; no changes required.
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte
Outdated
Show resolved
Hide resolved
...oject-[region]-[project]/databases/database-[database]/(entity)/views/settings/danger.svelte
Show resolved
Hide resolved
...project-[region]-[project]/databases/database-[database]/table-[table]/settings/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte(1 hunks)
🔇 Additional comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte (1)
28-56: Clean delegation pattern and terminology-based invalidation.The refactor to delegate deletion via
onDeleteIndexesand use terminology-based dependencies (dependencies.entity.singularat line 45) aligns well with the PR's multi-terminology goals. Thecleanup()function properly resets state, tracks events, shows notifications, and invalidates the correct dependency.
...roject-[region]-[project]/databases/database-[database]/(entity)/views/indexes/delete.svelte
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
| let error = $state(null); | ||
| let touchedId = $state(false); | ||
| let error: string = $state(null); | ||
| let creatingEntity = $state(false); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify if manual submit state management is necessary.
The component now manually manages submit state with the creatingEntity flag (line 39) and uses forceShowLoader={creatingEntity} (line 156). However, retrieved learnings indicate that the Modal component handles submit state internally via submissionLoader, which should prevent double submissions and manage loading state automatically.
This manual management contradicts the established pattern and may be redundant. If the Modal's internal state is insufficient for the refactored entity creation flow, please clarify why this change was necessary.
Based on learnings
Also applies to: 156-157
🏁 Script executed:
#!/bin/bash
# Description: Verify Modal component's submissionLoader implementation and usage patterns
# Check Modal component implementation for submissionLoader
rg -nP --type=svelte -A5 -B5 'submissionLoader'
# Check other usages of Modal to see if they manually manage submit state
rg -nP --type=svelte 'forceShowLoader\s*=' Length of output: 61
🏁 Script executed:
#!/bin/bash
# Search for submissionLoader prop or usage in all Svelte components
rg -n --glob '*.svelte' 'submissionLoader' -A5 -B5
# Search for forceShowLoader usage across Svelte components
rg -n --glob '*.svelte' 'forceShowLoader' -A3 -B3Length of output: 56137
🏁 Script executed:
#!/bin/bash
# Locate the create.svelte file under entity/views
file=$(find src/routes -type f -path '*/(entity)/views/create.svelte' -print -quit)
# Show imports and initial state declarations
sed -n '1,70p' "$file"
# Show lines around creatingEntity usage
sed -n '30,60p' "$file"Length of output: 3178
🏁 Script executed:
#!/bin/bash
# Locate the create.svelte file under entity/views
file=$(find src/routes -type f -path '*/(entity)/views/create.svelte' -print -quit)
# Show Modal usage and handlers
sed -n '120,200p' "$file"Length of output: 988
Remove redundant creatingEntity state
The creatingEntity flag is never updated; Modal and Button handle loading via submissionLoader and the $submitting context. Remove the creatingEntity declaration and its disabled/forceShowLoader bindings, and if you need to disable during submit use disabled={$submitting} on the submit Button.
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte
around line 39, remove the redundant let creatingEntity = $state(false)
declaration and delete any usages/bindings to creatingEntity (e.g.,
disabled/forceShowLoader on Modal and Button). Instead rely on the existing
submissionLoader and $submitting context; if you need to disable the submit
Button during submit, set disabled={$submitting} (and remove forceShowLoader or
similar props tied to creatingEntity).
| async function createEntity() { | ||
| error = null; | ||
| try { | ||
| const table = await sdk | ||
| .forProject(page.params.region, page.params.project) | ||
| .tablesDB.createTable({ | ||
| databaseId, | ||
| tableId: id ? id : ID.unique(), | ||
| name | ||
| }); | ||
| updateAndCleanup(); | ||
| // early init setup! | ||
| enableThinkingModeForSuggestions(id, name); | ||
| await onTableCreated(table); | ||
| // create entity. | ||
| await onCreateEntity(id ? id : ID.unique(), name); | ||
| name = id = null; | ||
| showCreate = false; | ||
| creatingTable = false; | ||
| // don't wait for UI to mount! | ||
| enableThinkingModeForSuggestions(table); | ||
| // cleanup | ||
| updateAndCleanup(); | ||
| } catch (e) { | ||
| error = e.message; | ||
| trackError(e, Submit.TableCreate); | ||
| trackError(e, analyticsCreateSubmit); | ||
| } | ||
| } |
There was a problem hiding this comment.
Reset creatingEntity flag on error to prevent permanently disabled buttons.
If onCreateEntity throws an error, the creatingEntity flag remains true because updateAndCleanup() (which resets it on line 86) is only called on success. This leaves the Cancel and Create buttons permanently disabled.
Apply this diff to reset the flag in the catch block:
} catch (e) {
error = e.message;
+ creatingEntity = false;
trackError(e, analyticsCreateSubmit);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function createEntity() { | |
| error = null; | |
| try { | |
| const table = await sdk | |
| .forProject(page.params.region, page.params.project) | |
| .tablesDB.createTable({ | |
| databaseId, | |
| tableId: id ? id : ID.unique(), | |
| name | |
| }); | |
| updateAndCleanup(); | |
| // early init setup! | |
| enableThinkingModeForSuggestions(id, name); | |
| await onTableCreated(table); | |
| // create entity. | |
| await onCreateEntity(id ? id : ID.unique(), name); | |
| name = id = null; | |
| showCreate = false; | |
| creatingTable = false; | |
| // don't wait for UI to mount! | |
| enableThinkingModeForSuggestions(table); | |
| // cleanup | |
| updateAndCleanup(); | |
| } catch (e) { | |
| error = e.message; | |
| trackError(e, Submit.TableCreate); | |
| trackError(e, analyticsCreateSubmit); | |
| } | |
| } | |
| async function createEntity() { | |
| error = null; | |
| try { | |
| // early init setup! | |
| enableThinkingModeForSuggestions(id, name); | |
| // create entity. | |
| await onCreateEntity(id ? id : ID.unique(), name); | |
| // cleanup | |
| updateAndCleanup(); | |
| } catch (e) { | |
| error = e.message; | |
| creatingEntity = false; | |
| trackError(e, analyticsCreateSubmit); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/create.svelte
around lines 57 to 72, the catch block for createEntity does not reset the
creatingEntity flag so buttons remain disabled after an error; update the catch
block to clear the creatingEntity state (e.g., set creatingEntity = false or
call updateAndCleanup()) before tracking the error so the UI is re-enabled on
failure.
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)
| const enumKey = `${term.title.singular}${action}`; | ||
| return Submit[enumKey as keyof typeof Submit]; |
There was a problem hiding this comment.
Unsafe enum lookup can return undefined at runtime.
The constructed enumKey is asserted as keyof typeof Submit without validation. If the key doesn't exist in the Submit enum, the return value will be undefined at runtime rather than a valid Submit enum value, potentially causing silent failures downstream.
Consider validating the key exists before returning:
const enumKey = `${term.title.singular}${action}`;
- return Submit[enumKey as keyof typeof Submit];
+ const result = Submit[enumKey as keyof typeof Submit];
+ if (result === undefined) {
+ throw new Error(
+ `Submit enum key "${enumKey}" not found for ${termType} action "${action}"`
+ );
+ }
+ return result;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const enumKey = `${term.title.singular}${action}`; | |
| return Submit[enumKey as keyof typeof Submit]; | |
| const enumKey = `${term.title.singular}${action}`; | |
| const result = Submit[enumKey as keyof typeof Submit]; | |
| if (result === undefined) { | |
| throw new Error( | |
| `Submit enum key "${enumKey}" not found for ${termType} action "${action}"` | |
| ); | |
| } | |
| return result; |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts
around lines 17 to 18, the dynamically constructed enumKey is cast to keyof
typeof Submit without runtime validation which can yield undefined; change the
code to validate that enumKey exists on the Submit enum before returning (for
example, use Object.prototype.hasOwnProperty.call(Submit, enumKey) or check
enumKey in Submit), and if it does not exist either throw a descriptive error or
return a safe default/handle the missing case explicitly so a valid Submit value
is always returned.
| const enumKey = `Database${term.title.singular}${action}`; | ||
| return Click[enumKey as keyof typeof Click]; |
There was a problem hiding this comment.
Unsafe enum lookup can return undefined at runtime.
Similar to the submit handler, the constructed enumKey is asserted as keyof typeof Click without validation. If the key doesn't exist in the Click enum, the return value will be undefined at runtime.
Apply the same validation pattern as suggested for the submit handler:
const enumKey = `Database${term.title.singular}${action}`;
- return Click[enumKey as keyof typeof Click];
+ const result = Click[enumKey as keyof typeof Click];
+ if (result === undefined) {
+ throw new Error(
+ `Click enum key "${enumKey}" not found for ${termType} action "${action}"`
+ );
+ }
+ return result;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const enumKey = `Database${term.title.singular}${action}`; | |
| return Click[enumKey as keyof typeof Click]; | |
| const enumKey = `Database${term.title.singular}${action}`; | |
| const result = Click[enumKey as keyof typeof Click]; | |
| if (result === undefined) { | |
| throw new Error( | |
| `Click enum key "${enumKey}" not found for ${termType} action "${action}"` | |
| ); | |
| } | |
| return result; |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/analytics.ts
around lines 28-29, the constructed enumKey is cast to keyof typeof Click
without validation which may yield undefined at runtime; validate the key exists
on the Click enum (e.g. using Object.prototype.hasOwnProperty.call(Click,
enumKey) or enumKey in Click) before indexing, and if the key is missing handle
it deterministically (throw a descriptive error or return a documented safe
default enum value) so the function never returns undefined.
| field: 'attribute', | ||
| record: 'document' | ||
| }, | ||
| vectordb: {} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Empty vectordb terminology may cause downstream failures.
The vectordb entry is an empty object. When useTerminology is called with a vectordb type, the returned TerminologyResult will have undefined entity, field, and record properties. This will cause the analytics handlers in analytics.ts to throw errors when trying to create handlers (lines 14-15, 25-26 in analytics.ts), since they check for missing terminology.
If vectordb is not yet supported, consider either:
- Removing the vectordb entry until terminology is defined:
legacy: {
entity: 'collection',
field: 'attribute',
record: 'document'
- },
- vectordb: {}
+ }- Or adding explicit validation in
useTerminologyto reject unsupported types early with a clear error message.
Run this script to check if vectordb is referenced elsewhere:
🏁 Script executed:
#!/bin/bash
# Check for vectordb usage in the codebase
rg -nP --type=ts "vectordb|VectorDB" -C2Length of output: 1453
🏁 Script executed:
#!/bin/bash
# Locate analytics.ts files
fd analytics.ts -t f
# For each analytics.ts file, show the sections around handler creation
for file in $(fd analytics.ts -t f); do
echo "=== $file ==="
sed -n '1,40p' "$file"
echo "..."
sed -n '80,120p' "$file"
doneLength of output: 4232
🏁 Script executed:
#!/bin/bash
# Find all types.ts files
echo "=== Listing types.ts files ==="
fd types.ts -t f
# Inspect helpers/types.ts in the database entity helpers folder
echo "=== src/routes/.../helpers/types.ts ==="
sed -n '1,200p' $(fd helpers/types.ts -t f)
# Inspect useTerminology in terminology.ts
echo "=== src/routes/.../helpers/terminology.ts ==="
sed -n '1,200p' $(fd terminology.ts -t f)Length of output: 2442
Remove or validate empty vectordb terminology
The stub vectordb: {} produces undefined entity|field|record, leading to missing or error-throwing analytics handlers. Remove this entry until you’ve defined its terms, or update useTerminology to throw a clear error on unsupported database types.
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
around line 28, the empty stub "vectordb: {}" yields undefined
entity|field|record values and breaks analytics handlers; either remove this
empty "vectordb" entry until you define its terms, or (preferred if you need a
runtime guard) add validation in useTerminology to detect unsupported/empty
database types and throw a clear, descriptive error (e.g., "Unsupported database
type: vectordb - missing terminology definitions") so callers fail fast and
handlers don’t receive undefined values.
| export function useTerminology(page: Page): TerminologyResult { | ||
| const type = page.data?.database?.type as DatabaseType; | ||
| const dbTerminologies = terminologyData[type] || {}; | ||
| return { | ||
| source: dbTerminologies, | ||
| field: dbTerminologies.field, | ||
| record: dbTerminologies.record, | ||
| entity: dbTerminologies.entity | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate database type to prevent silent fallback to empty terminology.
When the database type is unknown or invalid, the function falls back to an empty object (dbTerminologies = {}). This returns a TerminologyResult with all optional fields undefined, which may cause confusion and errors downstream. The analytics handlers will throw errors when terminology is missing, but the root cause (invalid type) is obscured.
Add explicit type validation:
export function useTerminology(page: Page): TerminologyResult {
const type = page.data?.database?.type as DatabaseType;
+ if (type && !terminologyData[type]) {
+ console.warn(`Unknown database type: ${type}. Falling back to empty terminology.`);
+ }
const dbTerminologies = terminologyData[type] || {};
return {
source: dbTerminologies,
field: dbTerminologies.field,
record: dbTerminologies.record,
entity: dbTerminologies.entity
};
}This provides visibility when an invalid type is encountered during development.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function useTerminology(page: Page): TerminologyResult { | |
| const type = page.data?.database?.type as DatabaseType; | |
| const dbTerminologies = terminologyData[type] || {}; | |
| return { | |
| source: dbTerminologies, | |
| field: dbTerminologies.field, | |
| record: dbTerminologies.record, | |
| entity: dbTerminologies.entity | |
| }; | |
| export function useTerminology(page: Page): TerminologyResult { | |
| const type = page.data?.database?.type as DatabaseType; | |
| if (type && !terminologyData[type]) { | |
| console.warn(`Unknown database type: ${type}. Falling back to empty terminology.`); | |
| } | |
| const dbTerminologies = terminologyData[type] || {}; | |
| return { | |
| source: dbTerminologies, | |
| field: dbTerminologies.field, | |
| record: dbTerminologies.record, | |
| entity: dbTerminologies.entity | |
| }; | |
| } |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts
around lines 62-70, the function silently falls back to an empty object when
page.data?.database?.type is invalid which masks the root cause; validate that
the resolved type exists in terminologyData and if it does not, throw or log a
clear error (e.g., throw new Error(`Unknown database type: ${type}`) or use a
dev-only assert) instead of returning an empty TerminologyResult, so callers
fail fast with a descriptive message; ensure the function still returns the
expected TerminologyResult shape when the type is valid.
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.
| } = $props(); | ||
| let showCreateIndex = $state(false); | ||
| let selectedIndex: Models.ColumnIndex = $state(null); |
There was a problem hiding this comment.
Add proper type annotations to state variables.
Two type safety issues:
- Line 56:
selectedIndexis initialized tonullbut typed asModels.ColumnIndex, which doesn't includenullin its type union. - Line 59:
selectedIndexeslacks a type annotation and will be inferred asnever[].
Apply this diff to fix the type annotations:
- let selectedIndex: Models.ColumnIndex = $state(null);
+ let selectedIndex: Models.ColumnIndex | null = $state(null);
let createIndex: CreateIndex;
- let selectedIndexes = $state([]);
+ let selectedIndexes = $state<string[]>([]);Also applies to: 59-59
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/view.svelte
around lines 56 and 59, the state variables have incorrect/insufficient types:
change selectedIndex's type to include null (e.g., Models.ColumnIndex | null)
since it is initialized to null, and add an explicit array type for
selectedIndexes (e.g., Models.ColumnIndex[]) so it is not inferred as never[];
update the $state initializers accordingly to match these types.
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
Bug Fixes
Refactor
Chores