Spatial type attributes #2338
Spatial type attributes #2338
Conversation
* updated line , point default types * spreadsheet cell for spatial types open in sidesheet
- Introduced InputPolygon component for polygon spatial data input. - Updated InputLine and InputPoint components to support index-based point addition and deletion. - Modified line and point components to handle default spatial data more effectively. - Enhanced column components to manage spatial data types including polygon.
…ate related logic
…perators and updating related components
…nhance handling of disable conditions
… improving button states for better user experience
ConsoleProject ID: Sites (2)
Note Appwrite has a Discord community with over 16 000 members. |
WalkthroughAdds spatial geometry support (point, linestring, polygon) across the console: new input components InputPoint/InputLine/InputPolygon; editors and create/update handlers for point/line/polygon column types and column-options entries; extends Columns/type unions and helpers getDefaultSpatialData/getSingleRingPolygon; exposes isSpatialType guard; renders spatial values as JSON in UI (spreadsheet, columns, columns listing); expands filters with many spatial/distance operators and threads an optional distance parameter through addFilter → Operator.toQuery; updates faker to generate spatial data; bumps an @appwrite.io/console package URL. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 31
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/helpers/types.ts (1)
84-85: Prefer precise tuple types for geometry over recursive arrays.Recursive alias is too loose; tuple aliases improve DX and validation.
Apply:
-export type NestedNumberArray = Array<number | NestedNumberArray>; +export type PointTuple = [number, number]; +export type LineStringTuple = PointTuple[]; +export type PolygonTuple = LineStringTuple[]; + +// Back-compat if needed, but prefer the specific tuples above. +export type NestedNumberArray = PointTuple | LineStringTuple | PolygonTuple;If InputPolygon currently references NestedNumberArray, keep it but migrate call sites to the tuple types over time.
src/lib/components/filters/filters.svelte (1)
122-131: Disable Apply until distance is provided for distance operators.
Button can be enabled with geometry set but missing distance.- $: isButtonDisabled = + $: isButtonDisabled = $queriesAreDirty || (quickFilters && displayQuickFilters && enableApply) ? false : !selectedColumn || !operatorKey || - (!value && - !arrayValues.length && - operatorKey !== ValidOperators.IsNotNull && - operatorKey !== ValidOperators.IsNull); + (isDistanceOperator + ? value === null || distanceValue === null + : (!value && + !arrayValues.length && + operatorKey !== ValidOperators.IsNotNull && + operatorKey !== ValidOperators.IsNull));
🧹 Nitpick comments (14)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts (1)
29-37: Minor: avoid re-allocating the spatial types array on each call.Hoist to a module-level Set for tiny perf/readability win.
Example:
const SPATIAL_TYPES = new Set(['point', 'linestring', 'polygon']); export function isSpatialType(column: Columns): column is Models.ColumnPoint | Models.ColumnLine | Models.ColumnPolygon { if (!column) return false; return SPATIAL_TYPES.has(column.type.toLowerCase()); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte (1)
332-332: Show spatial defaults in monospace and with truncation/tooltip.Raw JSON works but can overflow. Use a monospace style and tooltip for long values.
- {isSpatialType(column) ? JSON.stringify(_default) : _default} + {#if isSpatialType(column)} + <Tooltip placement="bottom" portal disabled> + <Typography.Monospace truncate>{JSON.stringify(_default)}</Typography.Monospace> + <span slot="tooltip" style:white-space="pre-wrap">{JSON.stringify(_default, null, 2)}</span> + </Tooltip> + {:else} + {_default} + {/if}src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (2)
910-913: Rendering spatial values: OK; consider monospace + tooltip for readability.Large coordinates will wrap; a tooltip with pretty JSON improves UX.
- <Typography.Text truncate> - {JSON.stringify(row[columnId])} - </Typography.Text> + <Tooltip placement="bottom" portal disabled> + <Typography.Monospace truncate>{JSON.stringify(row[columnId])}</Typography.Monospace> + <span slot="tooltip" style:white-space="pre-wrap">{JSON.stringify(row[columnId], null, 2)}</span> + </Tooltip>
287-309: Add icons for spatial types to avoid falling back to text icon.Currently point/linestring/polygon show IconText. Reuse IconLocationMarker unless you have dedicated glyphs.
Outside the changed lines:
function getAppropriateIcon(type: string): ComponentType { switch (type) { // ... case 'point': case 'linestring': case 'polygon': return IconLocationMarker; } }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (2)
52-54: Rename misleading variable.This component handles polygons, not points. Rename for clarity.
-let showDefaultPointDummyData = false; +let showDefaultPolygonDummyData = false;Adjust subsequent references accordingly.
121-133: Condition flags for default UI: align with renamed variable and simplify.Update references to the renamed flag and consider simplifying nullable logic.
- {#if !data.default && ((!editing && !defaultPolygonAdded) || (editing && !showDefaultPointDummyData))} + {#if !data.default && ((!editing && !defaultPolygonAdded) || (editing && !showDefaultPolygonDummyData))} ... - <InputPolygon - values={(defaultPolygonAdded && showDefaultPointDummyData) ? getDefaultSpatialData("polygon") : data.default} - nullable={defaultPolygonAdded && showDefaultPointDummyData} + <InputPolygon + values={(defaultPolygonAdded && showDefaultPolygonDummyData) ? getDefaultSpatialData("polygon") : data.default} + nullable={defaultPolygonAdded && showDefaultPolygonDummyData}Also applies to: 135-141
src/lib/elements/forms/index.ts (1)
34-36: Optional: keep barrel exports ordered consistently.
Consider grouping InputPoint/InputLine/InputPolygon near the other Input* exports (alphabetical or by feature) to reduce churn in future diffs.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/store.ts (1)
120-146: Spatial column options wired correctly; minor copy nit.
Handlers and types match. Nit: sentenceName for Line is 'linestring' while name is 'Line'. If sentenceName feeds user-facing text (“Add a … column”), consider 'line' for consistency with other human-readable labels.- sentenceName: 'linestring', + sentenceName: 'line',src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
195-219: De-duplicate spatial defaults across the appThis helper should be the single source of truth. Content filter UI (content.svelte) currently hardcodes different defaults. Import and use this function there for consistency.
src/lib/components/filters/content.svelte (1)
185-196: Distance input UX — add label and basic validation guard in handlerMinor: label improves clarity; handler should refuse applying a distance operator without a point value to avoid silent no-ops if HTML validation is bypassed.
- <div style="margin-top: 8px;"> - <InputNumber + <div style="margin-top: 8px;"> + <InputNumber + label="Distance" id="distance" bind:value={distanceValue} placeholder="Enter distance" step={0.001} min={0} required /> </div>And in addFilterAndReset():
- if (isDistanceOperator && distanceValue !== null && value !== null) { + if (isDistanceOperator) { + if (distanceValue == null || value == null || !Array.isArray(value) || value.length !== 2) { + return; // guard: require a point [x,y] and numeric distance + } addFilter(columnsArray, columnId, operatorKey, value, arrayValues, distanceValue); - } else { + } else { addFilter(columnsArray, columnId, operatorKey, value, arrayValues); }src/lib/elements/forms/inputPolygon.svelte (1)
9-9: TypenullableexplicitlyAdd a boolean type for clarity and tooling.
-export let nullable; +export let nullable: boolean;src/lib/elements/forms/inputPoint.svelte (1)
24-30: Defensive parse to avoid NaN writesIf the input is cleared,
Number.parseFloat('')becomesNaN. Consider guarding.- on:change={(e) => onChangePoint(index, Number.parseFloat(`${e.detail}`))} /> + on:change={(e) => { + const n = Number(e.detail); + onChangePoint(index, Number.isFinite(n) ? n : 0); + }} />src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (1)
83-84: Remove stray console.logAvoid noisy logs in production UIs.
- $:console.log(editing) + // removed debug logsrc/lib/components/filters/store.ts (1)
195-209: Consider grouping spatial types for better maintainabilityThe spatial types (Point, Line, Polygon) are repeated across multiple operator definitions. Consider creating a constant array to improve maintainability.
const SPATIAL_TYPES = [ValidTypes.Point, ValidTypes.Line, ValidTypes.Polygon] as const; // Then use it in operator definitions: [ ValidOperators.Equal, { query: Query.equal, types: [ ValidTypes.String, ValidTypes.Integer, ValidTypes.Double, ValidTypes.Boolean, ValidTypes.Enum, ...SPATIAL_TYPES ] } ]Also applies to: 211-224, 263-277
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
package.json(1 hunks)src/lib/components/filters/content.svelte(5 hunks)src/lib/components/filters/filters.svelte(7 hunks)src/lib/components/filters/store.ts(9 hunks)src/lib/elements/forms/index.ts(1 hunks)src/lib/elements/forms/inputLine.svelte(1 hunks)src/lib/elements/forms/inputPoint.svelte(1 hunks)src/lib/elements/forms/inputPolygon.svelte(1 hunks)src/lib/helpers/types.ts(2 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/line.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/store.ts(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/createIndex.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/column.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/line.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/point.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/polygon.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/store.ts(1 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]/store.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
src/lib/helpers/types.ts (1)
ColumnType(43-43)
src/lib/components/filters/store.ts (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
columns(28-28)src/lib/helpers/types.ts (1)
Column(44-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (18)
package.json (1)
25-25: Locked & spatial support verified
pnpm-lock.yaml is pinned to@appwrite.io/console@78c7b8c, and code includesIndexType.Spatial,createPolygonColumn/updatePolygonColumn, and the spatial model columns.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte (1)
18-18: LGTM: import wiring for isSpatialType.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (2)
19-21: LGTM: import isSpatialType into spreadsheet.
962-964: Disabling inline edit for spatial types: LGTM.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (2)
64-72: Maintain closure after inserts: LGTM.Inserting before the last vertex and re-closing ring is correct.
101-104: LGTM: sensible default polygon template.src/lib/elements/forms/index.ts (1)
34-36: Exports for spatial inputs look good.
Re-exporting the three Svelte components is correct.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/store.ts (3)
26-28: Icon choice OK.
IconGlobe is a reasonable, consistent pick for spatial types.
31-58: Option type extensions align with spatial support.
The added name/type unions look consistent with the rest (float maps to 'double', etc.).
12-14: Approve component imports
Verified thatsubmitPoint/updatePoint,submitLine/updateLine, andsubmitPolygon/updatePolygonare indeed exported from their respective Svelte components, so the imports instore.tsare correct.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (2)
18-20: Spatial column variants added — LGTMUnion now includes Models.ColumnPoint/Line/Polygon. Looks consistent with the models layer.
195-219: Close polygon rings and add explicit return type for getDefaultSpatialData
- Add an explicit return signature
: [number, number] | [number, number][] | [number, number][][]- Ensure each polygon ring is explicitly closed (first and last coords match) to align with GeoJSON-style expectations; confirm with the backend whether closed rings are required or should remain open before merging.
src/lib/components/filters/content.svelte (1)
67-74: Operator detection — LGTMUsing ValidOperators to derive isDistanceOperator is clear and maintainable.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/column.svelte (2)
10-12: Spatial editors imported — LGTM
41-45: Type map additions — LGTMMapping of point/linestring/polygon to dedicated editors looks correct.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte (1)
64-70: Null-safety for mutations
pushCoordinate/deleteCoordinateassumedata.defaultis initialized. Either guard or rely on the conditional rendering pattern so these are only reachable whendata.defaultexists.If you want guards:
- function pushCoordinate(_: number) { - data.default.push(getDefaultSpatialData('point') as number[]); - } + function pushCoordinate(_: number) { + if (!data.default) return; + data.default.push(getDefaultSpatialData('point') as number[]); + data.default = [...data.default]; + } @@ - function deleteCoordinate(_: number) { - if (data.default.length > 2) data.default.pop(); - } + function deleteCoordinate(_: number) { + if (!data.default) return; + if (data.default.length > 2) { + data.default.pop(); + data.default = [...data.default]; + } + }src/lib/components/filters/store.ts (2)
118-142: LGTM! Well-structured enum additions for spatial supportThe new spatial operators and types are properly integrated into the existing enum structure, maintaining consistency with the existing naming patterns.
Also applies to: 144-155
289-291: No SQL injection risk for spatial operators
Column names here are sourced from validatedModels.Table.columnskeys (not arbitrary user input) and the raw strings follow Appwrite’s query DSL, not SQL. Closing this comment.
| {:else if column.type === 'point' || column.type === 'linestring' || column.type === 'polygon'} | ||
| <InputPoint | ||
| values={value || [0, 0]} | ||
| onChangePoint={(index, newValue) => { | ||
| if (!value) value = [0, 0]; | ||
| value[index] = newValue; | ||
| }} /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Wrong input component for linestring/polygon and non-reactive mutation
- InputPoint is used for linestring/polygon, causing shape mismatch and likely runtime errors.
- Direct mutation (value[index] = …) won’t trigger updates reliably; reassign to a new array.
- {:else if column.type === 'point' || column.type === 'linestring' || column.type === 'polygon'}
- <InputPoint
- values={value || [0, 0]}
- onChangePoint={(index, newValue) => {
- if (!value) value = [0, 0];
- value[index] = newValue;
- }} />
+ {:else if column.type === 'point'}
+ <InputPoint
+ values={value ?? [0, 0]}
+ onChangePoint={(index, newValue) => {
+ const next = [...(value ?? [0, 0])];
+ next[index] = newValue;
+ value = next;
+ }} />
+ {:else if column.type === 'linestring'}
+ <InputLine
+ values={value ?? [[0, 0], [1, 0]]}
+ onChangeLine={(next) => {
+ value = next;
+ }} />
+ {:else if column.type === 'polygon'}
+ <InputPolygon
+ values={value ?? [[[0, 0],[1, 0],[1, 1],[0, 1],[0, 0]]]}
+ onChangePolygon={(next) => {
+ value = next;
+ }} />Run to confirm the event/prop API for these inputs (adjust handlers/bindings accordingly):
🏁 Script executed:
#!/bin/bash
# Inspect spatial input components for prop and event names
fd -t f 'input(point|line|polygon).svelte' src | xargs -I{} sh -c 'echo "---- {} ----"; rg -n "export let|createEventDispatcher|dispatch|onChange" -n {} -C2'Length of output: 2774
Use correct spatial inputs and reassign arrays for reactivity (content.svelte:172-178)
-
point
•<InputPoint>is fine, but replace in-place mutation with:const next = [...(value ?? [0,0])]; next[index] = newValue; value = next;
-
linestring
• Swap to<InputLine values={value ?? [[0,0],[1,0]]}
• Use its exported onChangePoint —not onChangeLine—e.g.:onChangePoint={(ptIdx, coordIdx, newVal) => { const next = value?.map(line => [...line]) ?? [[0,0],[1,0]]; next[ptIdx][coordIdx] = newVal; value = next; }}• Wire up
onAddPoint,onDeletePoint,disableDeleteLine,disableDeletePointsIndexperinputLine.svelteAPI. -
polygon
• Swap to<InputPolygon values={value ?? [[[0,0],[1,0],[1,1],[0,1],[0,0]]]}
• Use its exported onChangePoint —not onChangePolygon—e.g.:onChangePoint={(lineIdx, ptIdx, coordIdx, newVal) => { const next = value ?.map(line => line.map(pt => [...pt])) ?? [[[0,0],[1,0],[1,1],[0,1],[0,0]]]; next[lineIdx][ptIdx][coordIdx] = newVal; value = next; }}• Also supply
onAddPoint,onAddLine,onDeletePoint,disableDeletePointsIndexperinputPolygon.svelteAPI.
Ensures correct component shapes and triggers Svelte reactivity by assigning new arrays.
🤖 Prompt for AI Agents
In src/lib/components/filters/content.svelte around lines 172-178, replace the
current in-place mutations and incorrect component/handler usage with the proper
spatial inputs and immutable updates: keep InputPoint but when handling
onChangePoint create a shallow-copied array and assign value = next; for
linestrings use InputLine with a default values fallback like [[0,0],[1,0]] and
use the exported onChangePoint (not onChangeLine) to clone the nested arrays,
update the specific coordinate, and reassign value to the new structure; for
polygons use InputPolygon with a default fallback like
[[[0,0],[1,0],[1,1],[0,1],[0,0]]] and use its exported onChangePoint (not
onChangePolygon) to deep-clone lines and points, update the specific coordinate,
reassign value, and also wire up the onAddPoint, onDeletePoint, onAddLine,
disableDeleteLine/disableDeletePointsIndex props per the inputLine/inputPolygon
APIs so all updates replace arrays (value = next) to trigger Svelte reactivity.
| // Check if the current operator is a distance-based operator | ||
| $: isDistanceOperator = operatorKey && [ | ||
| ValidOperators.DistanceEqual, | ||
| ValidOperators.DistanceNotEqual, | ||
| ValidOperators.DistanceGreaterThan, | ||
| ValidOperators.DistanceLessThan | ||
| ].includes(operatorKey as ValidOperators); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure isDistanceOperator is strictly boolean.
Current expression can yield null; make it a boolean to avoid subtle bugs.
- $: isDistanceOperator = operatorKey && [
+ $: isDistanceOperator = operatorKey
+ ? [
ValidOperators.DistanceEqual,
ValidOperators.DistanceNotEqual,
ValidOperators.DistanceGreaterThan,
ValidOperators.DistanceLessThan
- ].includes(operatorKey as ValidOperators);
+ ].includes(operatorKey as ValidOperators)
+ : false;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check if the current operator is a distance-based operator | |
| $: isDistanceOperator = operatorKey && [ | |
| ValidOperators.DistanceEqual, | |
| ValidOperators.DistanceNotEqual, | |
| ValidOperators.DistanceGreaterThan, | |
| ValidOperators.DistanceLessThan | |
| ].includes(operatorKey as ValidOperators); | |
| // Check if the current operator is a distance-based operator | |
| $: isDistanceOperator = operatorKey | |
| ? [ | |
| ValidOperators.DistanceEqual, | |
| ValidOperators.DistanceNotEqual, | |
| ValidOperators.DistanceGreaterThan, | |
| ValidOperators.DistanceLessThan | |
| ].includes(operatorKey as ValidOperators) | |
| : false; |
🤖 Prompt for AI Agents
In src/lib/components/filters/filters.svelte around lines 50 to 57, the reactive
assignment for isDistanceOperator can evaluate to null/undefined; change it to
always produce a strict boolean by converting the expression to a boolean (e.g.,
use !! or Boolean) and ensure operatorKey is guarded before calling includes so
the final value is true or false only.
...egion]-[project]/databases/database-[database]/table-[table]/rows/columns/types/point.svelte
Outdated
Show resolved
Hide resolved
...egion]-[project]/databases/database-[database]/table-[table]/rows/columns/types/point.svelte
Outdated
Show resolved
Hide resolved
...egion]-[project]/databases/database-[database]/table-[table]/rows/columns/types/point.svelte
Outdated
Show resolved
Hide resolved
...ion]-[project]/databases/database-[database]/table-[table]/rows/columns/types/polygon.svelte
Outdated
Show resolved
Hide resolved
| <InputPolygon | ||
| values={value} | ||
| nullable={false} | ||
| onAddLine={pushLine} | ||
| onAddPoint={pushCoordinate} | ||
| onDeletePoint={deleteCoordinate} | ||
| onChangePoint={handlePointChange} /> |
There was a problem hiding this comment.
Guard editor when value is absent
As with Line/Point, render InputPolygon only when value exists to avoid child crashes.
- <InputPolygon
- values={value}
- nullable={false}
- onAddLine={pushLine}
- onAddPoint={pushCoordinate}
- onDeletePoint={deleteCoordinate}
- onChangePoint={handlePointChange} />
+ {#if value}
+ <InputPolygon
+ values={value}
+ nullable={false}
+ onAddLine={pushLine}
+ onAddPoint={pushCoordinate}
+ onDeletePoint={deleteCoordinate}
+ onChangePoint={handlePointChange} />
+ {/if}🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/polygon.svelte
around lines 87 to 93, the InputPolygon component is rendered unconditionally
which can crash when value is undefined; wrap the InputPolygon render in a
conditional that checks value exists (e.g., only render when value is truthy) so
children receive a valid value prop, keeping the same props and handlers
unchanged.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (8)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (3)
6-11: Use Models.ColumnPolygon in APIs, props, and store typesAll polygon-related signatures use ColumnPoint. Switch to ColumnPolygon to avoid type mismatches downstream.
export async function submitPolygon( databaseId: string, tableId: string, key: string, - data: Partial<Models.ColumnPoint> + data: Partial<Models.ColumnPolygon> ) { export async function updatePolygon( databaseId: string, tableId: string, - data: Partial<Models.ColumnPoint>, + data: Partial<Models.ColumnPolygon>, originalKey?: string ) { -export let data: Partial<Models.ColumnPoint> = { +export let data: Partial<Models.ColumnPolygon> = { required: false, default: null }; -} = createConservative<Partial<Models.ColumnPoint>>({ +} = createConservative<Partial<Models.ColumnPolygon>>({ required: false, ...data });Also applies to: 20-26, 45-49, 93-100
82-91: Fix delete handler: accept (ringIndex, pointIndex), maintain closure, drop tiny ringsCurrent code removes the closing vertex and never re-closes the ring. Also the handler signature mismatches InputPolygon’s two-arg callback.
-function deleteCoordinate(ringIndex: number) { - const ring = data.default?.at(ringIndex); - - ring.splice(ring.length - 1, 1); - if (ring.length === 0) { - data.default.splice(ringIndex, 1); - } - - data.default = [...(data.default || [])]; -} +function deleteCoordinate(ringIndex: number, pointIndex: number) { + const ring = data.default?.[ringIndex]; + if (!ring) return; + // Deleting the closing point == delete first + if (pointIndex === ring.length - 1) pointIndex = 0; + ring.splice(pointIndex, 1); + if (ring.length < 4) { + // fewer than 4 points (including closure) => drop ring + data.default?.splice(ringIndex, 1); + } else { + // keep closed: last mirrors first + ring[ring.length - 1] = [...ring[0]]; + } + data.default = [...(data.default || [])]; +} @@ - onDeletePoint={deleteCoordinate} + onDeletePoint={(ringIndex, pointIndex) => deleteCoordinate(ringIndex, pointIndex)}Also applies to: 139-141
142-147: Preserve polygon closure when first/last vertex is editedMirror first↔last vertex to keep rings closed.
onChangePoint={(lineIndex, pointIndex, coordIndex, newValue) => { if (data.default && data.default[lineIndex] && data.default[lineIndex][pointIndex]) { data.default[lineIndex][pointIndex][coordIndex] = newValue; + const ring = data.default[lineIndex]; + const lastIdx = ring.length - 1; + if (pointIndex === 0) { + ring[lastIdx] = [...ring[0]]; + } else if (pointIndex === lastIdx) { + ring[0] = [...ring[lastIdx]]; + } data.default = [...data.default]; } }}src/lib/elements/forms/inputLine.svelte (2)
39-41: Optional delete-callback guardAvoid calling an undefined prop.
- onDeletePoint={() => onDeletePoint(index)} + onDeletePoint={() => onDeletePoint?.(index)}
33-43: Guard against undefinedvaluesin {#each}Iterating over undefined crashes; use a fallback.
- {#each values as value, index} + {#each (values ?? []) as value, index} <InputPointsrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/polygon.svelte (2)
36-46: Deleting a coordinate breaks ring closure; fix and keep minimum ringMaintain closure and drop rings that fall below 4 points.
-function deleteCoordinate(ringIndex: number) { - if (!value) return; - - const ring = value.at(ringIndex); - - ring.splice(ring.length - 1, 1); - if (ring.length === 0) { - value.splice(ringIndex, 1); - } - value = [...value]; -} +function deleteCoordinate(ringIndex: number, pointIndex: number) { + if (!value) return; + const ring = value[ringIndex]; + if (!ring) return; + if (pointIndex === ring.length - 1) pointIndex = 0; + ring.splice(pointIndex, 1); + if (ring.length < 4) { + value.splice(ringIndex, 1); + } else { + ring[ring.length - 1] = [...ring[0]]; + } + value = [...value]; +}
86-93: Guard child render whenvalueis absentWrap InputPolygon to avoid crashes when
valueis undefined.- <InputPolygon - values={value} - nullable={false} - onAddLine={pushLine} - onAddPoint={pushCoordinate} - onDeletePoint={deleteCoordinate} - onChangePoint={handlePointChange} /> + {#if value} + <InputPolygon + values={value} + nullable={false} + onAddLine={pushLine} + onAddPoint={pushCoordinate} + onDeletePoint={deleteCoordinate} + onChangePoint={handlePointChange} /> + {/if}src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (1)
114-118: Fix Svelte reactivity and handle null default in onChangePointNested mutation won’t trigger updates; editing with no default won’t persist changes.
Apply:
- onChangePoint={(index, newValue) => { - if (data.default) { - data.default[index] = newValue; - } - }} /> + onChangePoint={(index, newValue) => { + const base = Array.isArray(data.default) + ? data.default + : (getDefaultSpatialData('point') as number[]); + const next = [...base]; + if (index < 0 || index >= next.length) return; + next[index] = newValue; + data = { ...data, default: next }; + }} />
🧹 Nitpick comments (8)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (1)
52-54: Naming nit: “Point” → “Polygon”Variable names mention “Point” while editing polygons; can confuse future readers.
-let showDefaultPointDummyData = false; +let showDefaultPolygonDummyData = false; @@ -$: showDefaultPointDummyData = $required ? true : false; +$: showDefaultPolygonDummyData = !!$required; @@ - values={defaultPolygonAdded && showDefaultPointDummyData + values={defaultPolygonAdded && showDefaultPolygonDummyData ? getDefaultSpatialData('polygon') : data.default} - nullable={defaultPolygonAdded && showDefaultPointDummyData} + nullable={defaultPolygonAdded && showDefaultPolygonDummyData}Also applies to: 110-111, 135-138
src/lib/elements/forms/inputLine.svelte (1)
22-28: Delete-disable logic likely inverted; please verify intent
disable = disable && allowLineDeleteenables deletion whenallowLineDeleteis false. If the intent was “disallow deletion when allowLineDelete is explicitly false,” use OR semantics.- let disable = index < minDeletableIndex; - if (allowLineDelete !== undefined) { - disable = disable && allowLineDelete; - } + let disable = index < minDeletableIndex; + if (allowLineDelete !== undefined) { + // if explicitly not allowed, keep disabled regardless of index + disable = disable || allowLineDelete === false; + } return disable;src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (3)
79-82: Required/default logic is inconsistentYou null-out default when required is true (Line 79 via handleDefaultState), but also set the dummy flag to true when required is true (Line 81), leading to UI showing a dummy while default is cleared. Confirm intended behavior; if default should be hidden when required, flip the flag.
- $: showDefaultPointDummyData = $required ? true : false; + $: showDefaultPointDummyData = !$required;
83-83: Drop debug logRemove reactive console.log to avoid noise.
- $: console.log(editing);
100-105: Simplify Add Point visibility logicReadability: compute once and reuse.
+ $: canShowAdd = + !data.default && + ((!editing && !defaultPointAdded) || (editing && !showDefaultPointDummyData)); @@ - {#if !data.default && ((!editing && !defaultPointAdded) || (editing && !showDefaultPointDummyData))} + {#if canShowAdd} <Button secondary on:click={handleAddDefault}>src/lib/components/filters/store.ts (3)
300-306: Expand DistanceNotEqual to support all geometry types.Similar to DistanceEqual, this operator should support Line and Polygon types for comprehensive spatial distance filtering.
[ ValidOperators.DistanceNotEqual, { query: Query.distanceNotEqual, - types: [ValidTypes.Point] + types: [ValidTypes.Point, ValidTypes.Line, ValidTypes.Polygon] } ],
307-313: Expand DistanceGreaterThan to support all geometry types.[ ValidOperators.DistanceGreaterThan, { query: Query.distanceGreaterThan, - types: [ValidTypes.Point] + types: [ValidTypes.Point, ValidTypes.Line, ValidTypes.Polygon] } ],
314-320: Expand DistanceLessThan to support all geometry types.[ ValidOperators.DistanceLessThan, { query: Query.distanceLessThan, - types: [ValidTypes.Point] + types: [ValidTypes.Point, ValidTypes.Line, ValidTypes.Polygon] } ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/lib/components/filters/content.svelte(5 hunks)src/lib/components/filters/filters.svelte(7 hunks)src/lib/components/filters/store.ts(9 hunks)src/lib/elements/forms/inputLine.svelte(1 hunks)src/lib/elements/forms/inputPoint.svelte(1 hunks)src/lib/elements/forms/inputPolygon.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/line.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/point.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/polygon.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/lib/elements/forms/inputPoint.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/line.svelte
- src/lib/components/filters/content.svelte
- src/lib/elements/forms/inputPolygon.svelte
- src/lib/components/filters/filters.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/point.svelte
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/components/filters/store.ts (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
columns(28-28)src/lib/helpers/types.ts (1)
Column(44-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (7)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (1)
86-93: Confirmdata.arrayavailabilityVerify Models.ColumnPoint includes
array; otherwise the Required checkbox may be incorrectly disabled.src/lib/components/filters/store.ts (6)
321-327: Intersects operator properly supports all geometry types.The Intersects operator correctly includes Point, Line, and Polygon types, which aligns with PostGIS ST_Intersects capabilities.
328-334: NotIntersects operator properly supports all geometry types.The operator correctly supports all spatial geometry types.
335-341: Overlaps operator properly supports all geometry types.The operator implementation correctly includes all spatial geometry types in its constraints.
342-348: NotOverlaps operator properly supports all geometry types.The operator correctly includes all spatial geometry types.
349-355: Touches operator properly supports all geometry types.The operator implementation correctly includes all spatial geometry types.
356-362: NotTouches operator properly supports all geometry types.The operator correctly supports all spatial geometry types in its type constraints.
.../project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte
Show resolved
Hide resolved
| await sdk.forProject(page.params.region, page.params.project).tablesDB.updatePointColumn({ | ||
| databaseId, | ||
| tableId, | ||
| key: originalKey, | ||
| required: data.required, | ||
| xdefault: data?.default, | ||
| newKey: data.key !== originalKey ? data.key : undefined | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mirror normalization/return in updatePoint
Keep payloads consistent when updating.
export async function updatePoint(
@@
- ) {
- await sdk.forProject(page.params.region, page.params.project).tablesDB.updatePointColumn({
+ ) {
+ const xdefault =
+ Array.isArray(data?.default) &&
+ data.default.length === 2 &&
+ data.default.every((n) => typeof n === 'number' && Number.isFinite(n))
+ ? data.default
+ : null;
+ return await sdk.forProject(page.params.region, page.params.project).tablesDB.updatePointColumn({
databaseId,
tableId,
key: originalKey,
required: data.required,
- xdefault: data?.default,
+ xdefault,
newKey: data.key !== originalKey ? data.key : undefined
});
}📝 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.
| await sdk.forProject(page.params.region, page.params.project).tablesDB.updatePointColumn({ | |
| databaseId, | |
| tableId, | |
| key: originalKey, | |
| required: data.required, | |
| xdefault: data?.default, | |
| newKey: data.key !== originalKey ? data.key : undefined | |
| }); | |
| } | |
| const xdefault = | |
| Array.isArray(data?.default) && | |
| data.default.length === 2 && | |
| data.default.every((n) => typeof n === 'number' && Number.isFinite(n)) | |
| ? data.default | |
| : null; | |
| return await sdk.forProject(page.params.region, page.params.project).tablesDB.updatePointColumn({ | |
| databaseId, | |
| tableId, | |
| key: originalKey, | |
| required: data.required, | |
| xdefault, | |
| newKey: data.key !== originalKey ? data.key : undefined | |
| }); | |
| } |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte
around lines 26 to 34, the updatePoint call omits normalizing and returning the
mirror field and thus produces an inconsistent payload compared to create/update
elsewhere; update the payload to include a normalized mirror value (e.g.,
compute mirror = normalizeMirror(data.mirror) or mirror: data.mirror ? true :
false) and send it in the updatePoint request, and ensure the component
returns/uses the same normalized mirror value from the response so payload shape
and values remain consistent with other column operations.
| function handlePointChange( | ||
| lineIndex: number, | ||
| pointIndex: number, | ||
| coordIndex: number, | ||
| newValue: number | ||
| ) { | ||
| if (value && value[lineIndex] && value[lineIndex][pointIndex]) { | ||
| value[lineIndex][pointIndex][coordIndex] = newValue; | ||
| value = [...value]; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Keep rings closed when editing endpoints
Mirror first/last vertex on change.
if (value && value[lineIndex] && value[lineIndex][pointIndex]) {
value[lineIndex][pointIndex][coordIndex] = newValue;
+ const ring = value[lineIndex];
+ const lastIdx = ring.length - 1;
+ if (pointIndex === 0) {
+ ring[lastIdx] = [...ring[0]];
+ } else if (pointIndex === lastIdx) {
+ ring[0] = [...ring[lastIdx]];
+ }
value = [...value];
}📝 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.
| function handlePointChange( | |
| lineIndex: number, | |
| pointIndex: number, | |
| coordIndex: number, | |
| newValue: number | |
| ) { | |
| if (value && value[lineIndex] && value[lineIndex][pointIndex]) { | |
| value[lineIndex][pointIndex][coordIndex] = newValue; | |
| value = [...value]; | |
| } | |
| } | |
| function handlePointChange( | |
| lineIndex: number, | |
| pointIndex: number, | |
| coordIndex: number, | |
| newValue: number | |
| ) { | |
| if (value && value[lineIndex] && value[lineIndex][pointIndex]) { | |
| // update the edited coordinate | |
| value[lineIndex][pointIndex][coordIndex] = newValue; | |
| // ensure the ring remains closed when endpoints change | |
| const ring = value[lineIndex]; | |
| const lastIdx = ring.length - 1; | |
| if (pointIndex === 0) { | |
| // mirror changes on the first point to the closing point | |
| ring[lastIdx] = [...ring[0]]; | |
| } else if (pointIndex === lastIdx) { | |
| // mirror changes on the closing point to the first point | |
| ring[0] = [...ring[lastIdx]]; | |
| } | |
| // trigger reactivity by cloning the outer array | |
| value = [...value]; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/polygon.svelte
around lines 48-58, the handler updates a single vertex but does not keep
polygon rings closed; when the edited point is the first (index 0) or the last
(index length-1) you must mirror the change to the opposite endpoint so the ring
remains closed. Fix by immutably cloning the outer array and the specific line
and point arrays before mutating, update the targeted coordinate, and if
pointIndex === 0 or pointIndex === lastIndex also update the corresponding
mirrored point (set last to match first or first to match last) for the same
coordIndex and coordinate values; finally assign the new outer array back to
value.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/store.ts (1)
44-59: Unify spatial type literals with central ColumnType
Replace the duplicated'point' | 'linestring' | 'polygon'union in src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/store.ts (lines 57–60) withimport type { ColumnType } from 'src/lib/helpers/types'; type SpatialType = Extract<ColumnType, 'point' | 'linestring' | 'polygon'>;and use SpatialType in place of the manual union.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.json(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/store.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/store.ts (1)
12-14: Verify exports in spatial editor components
Ensure each ofsrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/{point,line,polygon}.svelte
- default-exports its component
- named-exports
submitPoint/updatePoint,submitLine/updateLine,submitPolygon/updatePolygon
with signatures(databaseId, tableId, key, data)and(databaseId, tableId, data, originalKey).
I wasn’t able to locate these files via script—please verify manually.
...ole)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/store.ts
Outdated
Show resolved
Hide resolved
...ole)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/store.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/createIndex.svelte (4)
119-138: Send orders only for Key; validate completeness; filter strictly.
orderscurrently includes''and is sent for non-Key types. Tighten and fail fast for partial orders.- const orders = columnList.map((a) => a.order).filter((order) => order !== null); + const orders = + selectedType === IndexType.Key + ? columnList + .map((a) => a.order) + .filter((o): o is 'ASC' | 'DESC' => o === 'ASC' || o === 'DESC') + : []; + + if (selectedType === IndexType.Key && orders.length !== columnList.length) { + addNotification({ type: 'error', message: 'Order is required for all Key index columns' }); + throw new Error('Order is required for all Key index columns'); + } @@ - ...(orders.length ? { orders } : {}) + ...(selectedType === IndexType.Key && orders.length ? { orders } : {})
210-216: Hide Order for Spatial; require only for Key.This aligns UI with backend constraints and prevents accidental
null/"null"orders.- <InputSelect - options={orderOptions} - required - id={`order-${index}`} - label={index === 0 ? 'Order' : undefined} - bind:value={column.order} - placeholder="Select order" /> + {#if selectedType !== IndexType.Spatial} + <InputSelect + options={orderOptions} + required={selectedType === IndexType.Key} + id={`order-${index}`} + label={index === 0 ? 'Order' : undefined} + bind:value={column.order} + placeholder="Select order" /> + {/if}
52-64: Remove “NONE”; hide Order for Spatial to avoid invalid payloads.Providing
{ value: null, label: 'NONE' }risks binding"null"(string) and sending an invalid order. Spatial indexes don’t take order—hide the control instead of offering a sentinel.Apply:
- // order options derived from selected type - let orderOptions = $derived.by(() => - selectedType === IndexType.Spatial - ? [ - { value: 'ASC', label: 'ASC' }, - { value: 'DESC', label: 'DESC' }, - { value: null, label: 'NONE' } - ] - : [ - { value: 'ASC', label: 'ASC' }, - { value: 'DESC', label: 'DESC' } - ] - ); + // Static order options; Order UI hidden for Spatial (see below) + let orderOptions = $derived([ + { value: 'ASC', label: 'ASC' }, + { value: 'DESC', label: 'DESC' } + ]);And wrap the Order select (see Lines 210–216 comment) to render only when
selectedType !== IndexType.Spatial.
85-94: Fix spatial detection and auto-select the correct index type.Using
filteryields an array;column.length && isSpatialType(column[0])producesnumber|boolean. Also, if the preselected column is spatial,selectedTypeshould beSpatial, otherwise a “Key” index withorder=nullwill be sent.- const column = $table.columns.filter((column) => externalColumnKey === column.key); - const isSpatial = column.length && isSpatialType(column[0]); + const col = $table.columns.find((c) => externalColumnKey === c.key); + const isSpatial = !!col && isSpatialType(col); const order = isSpatial ? null : 'ASC'; columnList = externalColumnKey ? [{ value: externalColumnKey, order, length: null }] : [{ value: '', order, length: null }]; - selectedType = IndexType.Key; - key = `index_${$indexes.length + 1}`; + selectedType = isSpatial ? IndexType.Spatial : IndexType.Key; + // key is assigned via generateIndexKey() on open; avoid duplicate assignment here
🧹 Nitpick comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/createIndex.svelte (5)
30-41: Harden columnOptions against undefined table; minor robustness.If
$tableis ever transiently undefined during route transitions, this will throw. Guard with optional chaining/fallback.- let columnOptions = $derived( - $table.columns + let columnOptions = $derived( + ($table?.columns ?? []) .filter((column) => { if (selectedType === IndexType.Spatial) { return isSpatialType(column); // keep only spatial } return !isRelationship(column) && !isSpatialType(column); // keep non-relationship and non-spatial }) .map((column) => ({ value: column.key, label: column.key })) );
43-43: Unify order type: prefer null over empty string for “unset”.
order: ''complicates checks later and can leak into payloads. Usenullfor “not chosen” and gate on'ASC'|'DESC'.- let columnList = $state([{ value: '', order: '', length: null }]); + let columnList = $state([{ value: '', order: null, length: null }]);
96-101: Only require order for Key indexes; allow add for others.
addColumnDisabledcurrently blocks add when last column lacks order even forUnique/Fulltext. Scope the order requirement toKey.- const addColumnDisabled = $derived( - selectedType === IndexType.Spatial || - !columnList.at(-1)?.value || - (!columnList.at(-1)?.order && columnList.at(-1)?.order !== null) - ); + const addColumnDisabled = $derived(() => { + const last = columnList.at(-1); + const needsOrder = + selectedType === IndexType.Key && + (last?.order !== 'ASC' && last?.order !== 'DESC'); + return selectedType === IndexType.Spatial || !last?.value || needsOrder; + });
107-117: Avoid double index-key assignment during init.
initialize()setskey, then the open-effect sets it again viagenerateIndexKey(). Keep a single source of truth (prefer the generator).- if (showCreateIndex && !initializedForOpen) { - initialize(); - key = generateIndexKey(); + if (showCreateIndex && !initializedForOpen) { + initialize(); // initialize should not assign key + key = generateIndexKey(); initializedForOpen = true; }And inside
initialize()remove the directkey = ...assignment (see Lines 85–94 diff).
172-174: New column seed: align with null-as-unset.If you adopt the null-as-unset suggestion, seed
orderwithnullfor consistency.- columnList = [...columnList, { value: '', order: '', length: null }]; + columnList = [...columnList, { value: '', order: null, length: null }];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/createIndex.svelte(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/createIndex.svelte (4)
12-12: Import looks correct.Importing
isRelationshipandisSpatialTypefrom the rows store is appropriate for the filtering logic below.
48-49: Index type additions look good.
FulltextandSpatialentries are consistent with the rest of the UI.
66-72: Good: reset columnList when Spatial is selected.Resets to a single column and clears order/length—which aligns with spatial constraints.
195-204: System fields gated correctly for non-spatial types.Hiding
$id/$createdAt/$updatedAtfor Spatial is correct.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/helpers/faker.ts (1)
201-219: Optional: deduplicate point generation.A tiny helper reduces repetition and clarifies intent.
Apply this diff:
+// Local helper to generate a [lon, lat] tuple +const randomPoint = (): [number, number] => [faker.location.longitude(), faker.location.latitude()]; @@ - case 'point': { - return [faker.location.longitude(), faker.location.latitude()]; - } + case 'point': { + return randomPoint(); + } @@ - case 'line': { - return Array.from({ length: 5 }, () => [ - faker.location.longitude(), - faker.location.latitude() - ]); - } + case 'line': { + return Array.from({ length: 5 }, randomPoint); + } @@ - case 'polygon': { - const points = Array.from({ length: 5 }, () => [ - faker.location.longitude(), - faker.location.latitude() - ]); + case 'polygon': { + const points = Array.from({ length: 5 }, randomPoint); points.push(points[0]); // close polygon return [points]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/helpers/faker.ts(5 hunks)src/lib/helpers/types.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/helpers/types.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/helpers/faker.ts (2)
src/lib/helpers/types.ts (1)
NestedNumberArray(84-84)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
Columns(9-21)
🪛 ESLint
src/lib/helpers/faker.ts
[error] 213-213: 'points' is never reassigned. Use 'const' instead.
(prefer-const)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (5)
src/lib/helpers/faker.ts (5)
6-6: Type‑only import is correct.Importing
NestedNumberArrayas a type keeps it erased at runtime. LGTM.
121-127: Array branch typing looks consistent.Return type and
itemselement union align with spatial arrays. LGTM.
142-145: Return type update for spatial values is appropriate.Extending to
NestedNumberArrayis necessary for point/line/polygon. LGTM.
205-211: Incorrect — keep 'linestring' as the discriminant.
Used consistently across the codebase (e.g. src/lib/helpers/types.ts, src/lib/helpers/faker.ts:205, src/lib/components/filters/store.ts:166); do not change to 'line'.Likely an incorrect or invalid review comment.
201-204: Confirm coordinate order ([lon, lat] vs [lat, lon])Helper returns [faker.location.longitude(), faker.location.latitude()] → [lon, lat] (GeoJSON). Verify downstream consumers (UI/map libs/API/DB) expect [lon, lat] or convert to [lat, lon].
Location: src/lib/helpers/faker.ts:201-204
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte (1)
52-59: Make updates to data.default reactive (immutably reassign).Mutating nested props (data.default = ...) can miss updates depending on how parents consume data. Reassign the object to guarantee change detection. This was suggested earlier as well.
function handleDefaultState(hideDefault: boolean) { if (hideDefault) { savedDefault = data.default; - data.default = null; + data = { ...data, default: null }; } else { - data.default = savedDefault ?? getDefaultSpatialData('linestring'); + data = { ...data, default: savedDefault ?? getDefaultSpatialData('linestring') }; } } @@ on:change={(e) => { if (e.detail) { $required = false; - } else data.default = null; + } else { + data = { ...data, default: null }; + } }}Also applies to: 101-107
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte (2)
61-63: Guard against null before pushing a coordinate.If defaultChecked is toggled on and the effect hasn’t initialized data.default yet, pushCoordinate will throw. Initialize defensively.
- function pushCoordinate() { - data.default.push(getDefaultSpatialData('point') as number[]); - } + function pushCoordinate() { + if (!data.default) data.default = getDefaultSpatialData('linestring') as number[][]; + data.default.push(getDefaultSpatialData('point') as number[]); + }
118-118: Minor: pass the handler directly.Avoid creating a new function each render.
- onAddPoint={() => pushCoordinate()} + onAddPoint={pushCoordinate}
📜 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]/table-[table]/columns/line.svelte(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte (3)
12-18: LGTM: Using page from $app/state here is correct.Accessing page.params directly (no $ prefix) is valid with $app/state in SvelteKit ≥ 2.12.
20-34: Verify updateLine callers always pass originalKey.If originalKey is undefined, the API call may fail. Either enforce it in the signature or fall back to data.key.
Option if you want a defensive fallback:
- key: originalKey, + key: originalKey ?? data.key,
77-85: Re-check coupling “Required” → default visibility.Calling handleDefaultState($required) ties default UI/state to the required flag. Product-wise, defaults and required are usually independent. Confirm this matches UX across other column editors; otherwise, gate the coupling behind a prop.
...)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte
Outdated
Show resolved
Hide resolved
...)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
lets add some spatial fake columns as well when there are no columns. 1/2 are fine. you can random on either line, polygon, or point.
* updated line settings
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/lib/helpers/faker.ts (2)
87-90: Allow null in row value type to match generators.
generateValueForColumncan returnnull(e.g., empty enum), causing a TS mismatch when assigning torow.Apply this diff:
- let row: Record< - string, - string | number | boolean | Array<string | number | boolean | NestedNumberArray> - > = { + let row: Record< + string, + string | number | boolean | null | Array<string | number | boolean | NestedNumberArray | null> + > = {
235-241: Prefer const forpoints; keep ring closure.
Satisfies ESLint prefer-const and preserves semantics.Apply this diff:
- case 'polygon': { - let points = Array.from({ length: 5 }, () => [ + case 'polygon': { + const points = Array.from({ length: 5 }, () => [ faker.location.longitude(), faker.location.latitude() ]); points.push(points[0]); // close polygon return [points]; }
🧹 Nitpick comments (3)
src/lib/helpers/faker.ts (3)
102-107: Randomize spatial fields in the “no columns” sample row (per earlier feedback).
Show one spatial field (Point/Line/Polygon) instead of always both; keeps the preview concise and varied.Apply this diff:
- active: faker.datatype.boolean(), - location: [faker.location.longitude(), faker.location.latitude()], - route: Array.from({ length: 5 }, () => [ - faker.location.longitude(), - faker.location.latitude() - ]) + active: faker.datatype.boolean(), + ...((): Record<string, NestedNumberArray> => { + const pick = faker.number.int({ min: 0, max: 2 }); + if (pick === 0) { + return { location: [faker.location.longitude(), faker.location.latitude()] }; + } + if (pick === 1) { + const n = faker.number.int({ min: 2, max: 7 }); + return { + route: Array.from({ length: n }, () => [ + faker.location.longitude(), + faker.location.latitude() + ]) + }; + } + const ring = Array.from({ length: 5 }, () => [ + faker.location.longitude(), + faker.location.latitude() + ]); + ring.push(ring[0]); // close polygon + return { area: [ring] }; + })()
227-232: Vary LineString vertex count; ensure minimum of 2.
Small realism bump and aligns with spec minimum.Apply this diff:
- case 'linestring': { - return Array.from({ length: 5 }, () => [ + case 'linestring': { + const n = faker.number.int({ min: 2, max: 7 }); + return Array.from({ length: n }, () => [ faker.location.longitude(), faker.location.latitude() ]); }
52-63: Spatial sample columns — add Polygon or make selection dynamic.
src/lib/helpers/faker.ts currently creates Point + Line only (≈lines 52–58). repo search found only createPointColumn/createLineColumn; verify your pinned SDK exposes client.tablesDB.createPolygonColumn before adding Polygon. Alternatively randomize 1–2 spatial column types (Point/Line/Polygon) for demo variety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/helpers/faker.ts(7 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/helpers/faker.ts (2)
src/lib/helpers/types.ts (1)
NestedNumberArray(84-84)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
Columns(9-21)
🔇 Additional comments (5)
src/lib/helpers/faker.ts (5)
6-6: Type import is correct and well-scoped.
Keeps geometry payloads strongly typed without runtime cost.
46-51: Adding a boolean sample column looks good.
Matches existing sample schema patterns.
145-149: Return type aligns with produced values (including null for enums and nested numeric arrays).
No action needed here.
164-166: Signature for geometry-capable single-value generator looks good.
Covers all produced cases.
223-225: Point coordinates use [lng, lat] ordering — correct.
Matches common spatial conventions.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/helpers/files.ts (2)
50-57: tar.gz detection is incorrect;.split('.').pop() === 'tar'won’t match*.tar.gzThis will misclassify valid
.tar.gz/.tgzuploads and re‑pack them unnecessarily. Also considerapplication/x-gzip.- if ( - files?.length === 1 && - files.item(0).type === 'application/gzip' && - files.item(0).name.split('.').pop() === 'tar' - ) { + if ( + files?.length === 1 && + (files.item(0).type === 'application/gzip' || files.item(0).type === 'application/x-gzip') && + (files.item(0).name.toLowerCase().endsWith('.tar.gz') || files.item(0).name.toLowerCase().endsWith('.tgz')) + ) {
31-44: Return type mismatch: may return(FileData | null)[]whereFileData[]is declared
Promise.all(filePromises)currently resolves to(FileData|null)[]. This violates the function signature and risks downstream NPEs.- return Promise.all(filePromises); + const results = await Promise.all(filePromises); + return results.filter((f): f is FileData => f !== null);
🧹 Nitpick comments (4)
src/lib/helpers/files.ts (4)
74-77: Ensure error notifications render strings
message: ecan be a non‑string. Normalize to a string for consistent UI.- addNotification({ - type: 'error', - message: e - }); + addNotification({ + type: 'error', + message: e instanceof Error ? e.message : String(e) + });
46-46: Annotate return type for clarityHelps callers and avoids implicit
any/union inference.-export async function gzipUpload(files: FileList) { +export async function gzipUpload(files: FileList): Promise<File | undefined> {
58-58: Typo in commentMinor doc fix.
- // Else process the file to mantain the folder structure and create a .tar.gz file + // Else process the file to maintain the folder structure and create a .tar.gz file
88-90: Avoid shadowing outerfileparamImproves readability.
- filteredFiles.forEach((file) => { - dataTransfer.items.add(file); + filteredFiles.forEach((f) => { + dataTransfer.items.add(f);
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/helpers/faker.ts (1)
215-221: Handle dedicated email/ip/url column types (not only string+format).
ColumnsincludesColumnEmail,ColumnIp,ColumnUrl. Without explicit cases, these fall through todefaultand may generate invalid values.Apply:
case 'boolean': { return faker.datatype.boolean(); } + case 'email': { + return faker.internet.email(); + } + case 'ip': { + return faker.internet.ip(); + } + case 'url': { + return faker.internet.url(); + } + case 'datetime': { return faker.date.recent({ days: 365 }).toISOString(); }
♻️ Duplicate comments (1)
src/lib/helpers/faker.ts (1)
87-90: Allow nulls in row value type to match generator outputs.
generateValueForColumncan returnnull(e.g., empty enum). Therowtype excludesnull, causing a TS mismatch.Apply:
- let row: Record< - string, - string | number | boolean | Array<string | number | boolean | NestedNumberArray> - > = { + let row: Record< + string, + string | number | boolean | null | Array<string | number | boolean | NestedNumberArray | null> + > = {
🧹 Nitpick comments (1)
src/lib/helpers/faker.ts (1)
46-63: Spatial defaults: consider including Polygon or randomizing the spatial pair.Right now we always create Point+Line. To better exercise spatial paths (and to match prior review intent), consider randomly picking two among point/line/polygon or including a polygon column as well. This improves coverage of polygon editors and serializers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
src/lib/helpers/faker.ts(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/helpers/faker.ts (2)
src/lib/helpers/types.ts (1)
NestedNumberArray(84-84)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
Columns(9-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (3)
src/lib/helpers/faker.ts (3)
235-241: Polygon generation LGTM (closed ring; const fix).Ring closure and
prefer-constaddressed correctly; shape matches [ [ [lon, lat], ... ] ] with closure.
227-233: Incorrect — no mismatch; runtime type is 'linestring'.rg shows src/routes/.../columns/store.ts:135 contains
type: 'linestring'and src/routes/.../store.ts:19 containsModels.ColumnLine; notype: 'line'was found. No change required.Likely an incorrect or invalid review comment.
102-107: ```shell
#!/bin/bash
set -euo pipefail
echo "Finding faker helper file(s)..."
rg -n --hidden -S "faker.ts$" --glob '!node_modules' -g '**/*' || trueecho
echo "Printing first matching faker.ts (if any)"
f=$(rg --hidden -S --files | rg "faker.ts$" | head -n1 || true)
if [ -n "$f" ]; then
echo "== file: $f =="
sed -n '1,240p' "$f" || true
else
echo "no faker.ts found"
fiecho
echo "Searching for uses of 'location' arrays, 'route', and faker.location across repo (including .svelte files)"
rg -n --hidden -S "location:\s*[" --glob '!node_modules' -g '/' -C3 || true
rg -n --hidden -S "route:\s[" --glob '!node_modules' -g '/' -C3 || true
rg -n --hidden -S "faker.location" --glob '!node_modules' -g '**/' -C3 || trueecho
echo "Searching for longitude/latitude identifiers and patterns"
rg -n --hidden -S "longitude(|faker.location.longitude|.longitude\b" --glob '!node_modules' -g '/*' -C3 || true
rg -n --hidden -S "latitude(|faker.location.latitude|.latitude\b" --glob '!node_modules' -g '/' -C3 || true
rg -n --hidden -S "\b(lat|latitude)\b" --glob '!node_modules' -g '**/' -C2 || true
rg -n --hidden -S "\b(lon|lng|longitude)\b" --glob '!node_modules' -g '**/*' -C2 || trueecho
echo "Searching for mapping/GeoJSON usage (mapbox, leaflet, L.latLng, ol., coordinates, GeoJSON)"
rg -n --hidden -S "mapbox|L.map|L.marker|L.latLng|leaflet|OpenLayers|ol.|coordinates\b|GeoJSON|FeatureCollection|Feature" --glob '!node_modules' -g '**/*' -C3 || trueecho
echo "Done."</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (4)
6-19: Normalize and return SDK response in submitPoint.Prevents invalid payloads and lets callers await the result.
export async function submitPoint( @@ - ) { - await sdk.forProject(page.params.region, page.params.project).tablesDB.createPointColumn({ + ) { + const xdefault = + Array.isArray(data?.default) && + data.default.length === 2 && + data.default.every((n) => typeof n === 'number' && Number.isFinite(n)) + ? (data.default as number[]) + : null; + return await sdk.forProject(page.params.region, page.params.project).tablesDB.createPointColumn({ databaseId, tableId, key, required: data.required, - xdefault: data?.default + xdefault }); }
20-34: Mirror normalization/return in updatePoint and guard key.Keep create/update consistent; avoid sending undefined key.
export async function updatePoint( @@ - ) { - await sdk.forProject(page.params.region, page.params.project).tablesDB.updatePointColumn({ + ) { + const xdefault = + Array.isArray(data?.default) && + data.default.length === 2 && + data.default.every((n) => typeof n === 'number' && Number.isFinite(n)) + ? (data.default as number[]) + : null; + return await sdk.forProject(page.params.region, page.params.project).tablesDB.updatePointColumn({ databaseId, tableId, - key: originalKey, + key: originalKey ?? data.key!, required: data.required, - xdefault: data?.default, + xdefault, newKey: data.key !== originalKey ? data.key : undefined }); }Run to confirm all call sites pass originalKey as intended:
#!/bin/bash # Verify updatePoint invocations provide originalKey rg -nP '\bupdatePoint\(' -C2 --type=ts --glob '!**/dist/**'
52-59: Reassign object in toggle handler to trigger reactivity.Direct nested writes may not notify consumers.
function handleDefaultState(hideDefault: boolean) { if (hideDefault) { savedDefault = data.default; - data.default = null; + data = { ...data, default: null }; } else { - data.default = savedDefault ?? getDefaultSpatialData('point'); + data = { ...data, default: savedDefault ?? getDefaultSpatialData('point') }; } }
94-101: Wire “Default value” checkbox to the toggle handler; avoid direct nested mutation.Ensures symmetry and reactivity.
on:change={(e) => { - if (e.detail) { - $required = false; - handleDefaultState(false); - } else { - data.default = null; - } + if (e.detail) $required = false; + // e.detail true => show default; false => hide default + handleDefaultState(!e.detail); }}src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (3)
52-59: Reassign object in toggle handler to trigger reactivity.Avoid silent UI stale state on toggle.
function handleDefaultState(hideDefault: boolean) { if (hideDefault) { savedDefault = data.default; - data.default = null; + data = { ...data, default: null }; } else { - data.default = savedDefault ?? getDefaultSpatialData('polygon'); + data = { ...data, default: savedDefault ?? getDefaultSpatialData('polygon') }; } }
83-92: Delete the selected vertex and maintain ring closure; accept both indices.Current handler always trims the closing vertex and can break closure.
-function deleteCoordinate(ringIndex: number) { - const ring = data.default?.at(ringIndex); - - ring.splice(ring.length - 1, 1); - if (ring.length === 0) { - data.default.splice(ringIndex, 1); - } - - data.default = [...(data.default || [])]; -} +function deleteCoordinate(ringIndex: number, pointIndex: number) { + const ring = data.default?.[ringIndex]; + if (!ring) return; + if (pointIndex === ring.length - 1) pointIndex = 0; // treat closing point as first + ring.splice(pointIndex, 1); + if (ring.length < 4) { + data.default?.splice(ringIndex, 1); + } else { + ring[ring.length - 1] = [...ring[0]]; // keep closed + } + data.default = [...(data.default || [])]; +} @@ - onDeletePoint={deleteCoordinate} + nullable={!defaultChecked} + onDeletePoint={(ringIndex, pointIndex) => deleteCoordinate(ringIndex, pointIndex)} @@ - onChangePoint={(lineIndex, pointIndex, coordIndex, newValue) => { + onChangePoint={(lineIndex, pointIndex, coordIndex, newValue) => { if (data.default && data.default[lineIndex] && data.default[lineIndex][pointIndex]) { data.default[lineIndex][pointIndex][coordIndex] = newValue; + // preserve closure when editing first/last vertex + const ring = data.default[lineIndex]; + const lastIdx = ring.length - 1; + if (pointIndex === 0) ring[lastIdx] = [...ring[0]]; + else if (pointIndex === lastIdx) ring[0] = [...ring[lastIdx]]; data.default = [...data.default]; } }}Also applies to: 138-148
121-127: Wire “Default value” checkbox to toggle; avoid direct mutation.Keeps state consistent and reactive.
on:change={(e) => { - if (e.detail) { - $required = false; - handleDefaultState(false); - } else { - data.default = null; - } + if (e.detail) $required = false; + handleDefaultState(!e.detail); }}
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (1)
73-76: Don’t auto-enable default when “Required” is unchecked.Coupling these can send xdefault while the checkbox is off; let the checkbox drive default state.
$effect(() => { - data.required = $required; - handleDefaultState($required); + data.required = $required; + // If becoming required while a default is active, clear it; otherwise do nothing. + if ($required && defaultChecked) { + defaultChecked = false; + data = { ...data, default: null }; + } });src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (2)
6-19: Normalize and return in submitPolygon.Validate rings and numbers before sending; return the SDK result.
export async function submitPolygon( @@ - ) { - await sdk.forProject(page.params.region, page.params.project).tablesDB.createPolygonColumn({ + ) { + const isNum = (n: unknown) => typeof n === 'number' && Number.isFinite(n); + const valid = + Array.isArray(data?.default) && + data.default.every( + (ring) => + Array.isArray(ring) && + ring.length >= 4 && + ring[0]?.length === 2 && + ring[ring.length - 1]?.length === 2 && + ring[0][0] === ring[ring.length - 1][0] && + ring[0][1] === ring[ring.length - 1][1] && + ring.every((pt) => Array.isArray(pt) && pt.length === 2 && isNum(pt[0]) && isNum(pt[1])) + ); + const xdefault = valid ? data!.default : null; + return await sdk.forProject(page.params.region, page.params.project).tablesDB.createPolygonColumn({ databaseId, tableId, key, required: data.required, - xdefault: data?.default + xdefault }); }
98-101: Decouple default from required to avoid unintended xdefault.Don’t auto-create defaults when not required; only clear when becoming required.
$effect(() => { - data.required = $required; - handleDefaultState($required); + data.required = $required; + if ($required && defaultChecked) { + defaultChecked = false; + data = { ...data, default: null }; + } });
📜 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]/table-[table]/columns/point.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (2)
112-119: LGTM: nested update correctly reassigns to trigger UI updates.The array spread after mutation fixes reactivity.
3-3: Keep import from $app/state — it’s correct here
- Repo uses @sveltejs/kit >= 2.12 (package.json shows ^2.36.2); SvelteKit added $app/state in 2.12 and it replaces/deprecates $app/stores, so the import is valid. (svelte.dev)
- Caveat: module-level code relying on page can be unsafe on the server — $app/state values are only readable during rendering, so exported functions that might run outside render (submitPoint/updatePoint) should either accept region/project as explicit params or be moved into the instance script to guarantee availability of page.params. (svelte.dev)
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte
Likely an incorrect or invalid review comment.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (1)
20-34: Normalize/validate polygon default and return SDK result in updatePolygonMirror create: validate data.default (rings ≥4, closed, numeric), set xdefault = valid ? data.default : null, use key: originalKey ?? data.key, and return the SDK call.
export async function updatePolygon( @@ - ) { - await sdk.forProject(page.params.region, page.params.project).tablesDB.updatePolygonColumn({ + ) { + const isNum = (n: unknown) => typeof n === 'number' && Number.isFinite(n); + const valid = + Array.isArray(data?.default) && + data.default.every( + (ring) => + Array.isArray(ring) && + ring.length >= 4 && + ring[0]?.length === 2 && + ring[ring.length - 1]?.length === 2 && + ring[0][0] === ring[ring.length - 1][0] && + ring[0][1] === ring[ring.length - 1][1] && + ring.every((pt) => Array.isArray(pt) && pt.length === 2 && isNum(pt[0]) && isNum(pt[1])) + ); + const xdefault = valid ? data!.default : null; + return await sdk.forProject(page.params.region, page.params.project).tablesDB.updatePolygonColumn({ databaseId, tableId, - key: originalKey, + key: originalKey ?? data.key!, required: data.required, - xdefault: data?.default, + xdefault, newKey: data.key !== originalKey ? data.key : undefined }); }Verify all call sites pass originalKey (run locally):
rg -nP '\bupdatePolygon\(' -C2 --glob '!**/dist/**' --glob '!**/node_modules/**'
| @@ -0,0 +1,149 @@ | |||
| <script module lang="ts"> | |||
| import { sdk } from '$lib/stores/sdk'; | |||
| import { page } from '$app/state'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
Verify import: SvelteKit’s page store is from $app/stores (not $app/state).
Confirm alias or switch; module context caveat applies.
🏁 Script executed:
#!/bin/bash
rg -nP "from '\\$app/state'" -C2 --glob '!**/dist/**'
fd -a 'tsconfig*.json' 'jsconfig*.json' | xargs -I{} sh -c 'echo "--- {} ---"; cat "{}"'Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== tool check ==="
command -v rg >/dev/null 2>&1 && rg --version | head -n1 || echo "rg: not found"
command -v fd >/dev/null 2>&1 && fd --version 2>/dev/null | head -n1 || echo "fd: not found"
echo "=== searching for $app/state and $app/stores imports/usages ==="
if command -v rg >/dev/null 2>&1; then
rg -n --hidden -uu -S -C2 "from '\$app/state'|from \"\$app/state\"" || true
rg -n --hidden -uu -S -C2 "from '\$app/stores'|from \"\$app/stores\"" || true
rg -n --hidden -uu -S -C2 "\$app/state|\$app/stores" || true
else
grep -RIn --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=.git "from '\$app/state'\|from \"\$app/state\"" . || true
grep -RIn --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=.git "from '\$app/stores'\|from \"\$app/stores\"" . || true
fi
echo "=== package.json (top-level & any found within 3 levels) ==="
find . -maxdepth 3 -type f -name package.json -print -exec sed -n '1,240p' {} \;
echo "=== searching for polygon.svelte file (as referenced in review) ==="
find . -type f -name 'polygon.svelte' -print -exec sed -n '1,240p' {} \;Length of output: 87
🏁 Script executed:
#!/bin/bash
set -e -o pipefail
echo '=== tool check ==='
command -v rg >/dev/null 2>&1 && rg --version | head -n1 || echo 'rg: not found'
command -v fd >/dev/null 2>&1 && fd --version 2>/dev/null | head -n1 || echo 'fd: not found'
echo '=== searching for $app/state and $app/stores imports/usages ==='
if command -v rg >/dev/null 2>&1; then
rg -n --hidden -uu -S -C2 "from '\$app/state'|from \"\$app/state\"" || true
rg -n --hidden -uu -S -C2 "from '\$app/stores'|from \"\$app/stores\"" || true
rg -n --hidden -uu -S -C2 "\$app/state|\$app/stores" || true
else
grep -RIn --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=.git "from '\$app/state'\|from \"\$app/state\"" . || true
grep -RIn --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=.git "from '\$app/stores'\|from \"\$app/stores\"" . || true
fi
echo '=== searching for polygon.svelte file (as referenced in review) ==='
find . -type f -name 'polygon.svelte' -print -exec sed -n '1,240p' {} \; || trueLength of output: 7755
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== ripgrep versions ==="
rg --version | head -n1 || true
echo
echo "=== matches for \$app/state and \$app/stores ==="
rg -n --hidden -uu -S "\$app/state" || true
rg -n --hidden -uu -S "\$app/stores" || true
echo
echo "=== search for alias / resolver / paths patterns ==="
rg -n --hidden -uu -S "resolve\.alias|alias\s*:|paths\s*:|\$app/state|\$app/stores" || true
echo
echo "=== tsconfig / jsconfig (<=3 levels) ==="
find . -maxdepth 3 -type f \( -name 'tsconfig*.json' -o -name 'jsconfig*.json' \) -print -exec sed -n '1,240p' {} \; || true
echo
echo "=== svelte.config / vite.config (<=3 levels) ==="
find . -maxdepth 3 -type f \( -name 'svelte.config.*' -o -name 'vite.config.*' -o -name 'vite.*.config.*' \) -print -exec sed -n '1,240p' {} \; || true
echo
echo "=== files exporting 'page' ==="
rg -n --hidden -uu -S "export\s+.*page|export\s*\{\s*page\s*\}" || true
echo
echo "=== files named state.* under src, lib, app (if present) ==="
for d in src lib app; do
if [ -d "$d" ]; then
find "$d" -type f -name 'state.*' -print -exec sed -n '1,240p' {} \; || true
fi
doneLength of output: 30763
Change import to $app/stores (not $app/state)
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (module script) — replace
import { page } from '$app/state';
with
import { page } from '$app/stores';
svelte.config.js defines no alias for $app/state, so the current import will break the build.
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte
around line 3, the module script imports page from the non-existent alias
$app/state; replace that import with page from '$app/stores' so the
SvelteKit-provided store is used. Update the import statement in the module
script to import { page } from '$app/stores'; and ensure there are no other
references to $app/state in the file or project.
...roject-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte
Show resolved
Hide resolved
What does this PR do?
Screencast.from.2025-09-11.19-07-19.webm
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
Enhancements
Chores