-
Notifications
You must be signed in to change notification settings - Fork 208
Feat: multi-terminology support #2413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Console (appwrite/console)Project ID: Sites (2)
Tip GraphQL API works alongside REST and WebSocket protocols |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 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
update: make `createTable` as a reusable component like `createEntity`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| export type CreateIndexesCallbackType = { | ||
| key: string; | ||
| type: IndexType; | ||
| fields: string[]; | ||
| lengths: number[]; | ||
| orders: string[]; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix lengths type to match emitted payload
CreateIndexesCallbackType.lengths is declared as number[], but the create() payload builds lengths with potential null entries (when a field has no length). TypeScript will flag the assignment, and the callback signature misrepresents the real shape. Update the type to accept null (or normalize the payload) so the code type-checks and downstream consumers can handle the optional length explicitly. Example:
export type CreateIndexesCallbackType = {
key: string;
type: IndexType;
fields: string[];
- lengths: number[];
+ lengths: Array<number | null>;
orders: string[];
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type CreateIndexesCallbackType = { | |
| key: string; | |
| type: IndexType; | |
| fields: string[]; | |
| lengths: number[]; | |
| orders: string[]; | |
| }; | |
| export type CreateIndexesCallbackType = { | |
| key: string; | |
| type: IndexType; | |
| fields: string[]; | |
| lengths: Array<number | null>; | |
| orders: string[]; | |
| }; |
🤖 Prompt for AI Agents
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/indexes/create.svelte
lines 2-8: the CreateIndexesCallbackType currently declares lengths as number[]
but the create() payload emits nulls for fields without a length; change the
type to (number | null)[] so it accurately reflects the payload (or
alternatively normalize the payload to replace nulls with undefined/0 before
calling the callback), and update any call sites/consumers to handle the
nullable entries accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ItzNotABug Comment is legit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
...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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be in a "helpers" context? Can't it just be defined for all entities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would just mix up the logic with stranded TS files while the views has its own directory structure. Mainly for organizing TS ops in a specific directory so its easier to manage. Maybe helpers isn't the right term but injections seems more broad and overkill to me.
src/routes/(console)/project-[region]-[project]/auth/security/updateSessionInvalidation.svelte
Outdated
Show resolved
Hide resolved
| export type CreateIndexesCallbackType = { | ||
| key: string; | ||
| type: IndexType; | ||
| fields: string[]; | ||
| lengths: number[]; | ||
| orders: string[]; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ItzNotABug Comment is legit
| // only for tests | ||
| // database.type = 'documentsdb'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ItzNotABug Comment is legit
Fix: Prevnet early dpeloynt success screen


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