feat: enhance data source and page value management#7
Conversation
- Added support for managing columns in data sources, including creating, updating, and deleting columns via new API endpoints. - Updated data source and page components to handle column data, allowing for dynamic rendering of column values. - Improved data validation and error handling for column operations. - Refactored existing API endpoints to include column data in responses for data sources and pages.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces a comprehensive column management system for data sources with API endpoints for column CRUD operations, React components for displaying and editing columns in a data table, supporting hooks for API interactions, and type definitions for endpoints. Additionally updates page management to support values, restructures page details response, and integrates Mantine's modal provider for modal dialogs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant DataViewTable as DataViewTable Component
participant ColumnFormModal as ColumnFormModal
participant useDataViewColumns as useDataViewColumns Hook
participant API as Column API Routes
User->>DataViewTable: Click "Add Column"
DataViewTable->>ColumnFormModal: Open modal with empty state
User->>ColumnFormModal: Enter column name & type
User->>ColumnFormModal: Click Create
ColumnFormModal->>useDataViewColumns: Call createColumn(name, type)
useDataViewColumns->>API: POST /data-sources/:id/columns
API-->>useDataViewColumns: { id, name, type }
useDataViewColumns-->>ColumnFormModal: Success
ColumnFormModal->>DataViewTable: Close & refresh (onSubmit)
DataViewTable-->>DataViewTable: Mutate dataSource columns
DataViewTable-->>User: Table updates with new column
sequenceDiagram
actor User
participant DataTableRow as DataTableRow Component
participant EditableTextCell as EditableTextCell
participant usePageValueUpdate as usePageValueUpdate Hook
participant API as Page Values API Route
User->>DataTableRow: Edit cell value in row
DataTableRow->>EditableTextCell: Show editable cell
User->>EditableTextCell: Blur (confirm value)
EditableTextCell->>DataTableRow: Call onCellUpdate(columnId, {type, value})
DataTableRow->>usePageValueUpdate: Call updateValue(pageId, columnId, value)
usePageValueUpdate->>API: PATCH /pages/:id/values
API-->>usePageValueUpdate: Success
usePageValueUpdate->>DataTableRow: Mutate pages cache
DataTableRow-->>User: Cell value persisted
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
danships
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (5)
src/lib/hooks/api/use-pages.ts (1)
23-28: LGTM! Query parameter construction is correct.The conditional appending of
includeValues=trueto the query string is implemented correctly and consistently with the existingusePagesByParentimplementation in this file.For a future refactor, consider using
URLSearchParamsfor cleaner query string construction throughout this file:const params = new URLSearchParams({ dataSourceId }); if (selectedOptions.includeValues) { params.set('includeValues', 'true'); } const url = `${GET_PAGES_ENDPOINT}?${params.toString()}`;This would improve readability and maintainability, though the current string concatenation approach is functional and consistent with the existing code.
src/types/api/endpoints/get-pages.ts (1)
12-12: Simplify the includeValues schema definition.The pattern
z.coerce.boolean().optional().default(false)is redundant. According to Zod 4 documentation,.default()already handlesundefinedinputs by returning the default value, making.optional()unnecessary in this context.Apply this diff:
- includeValues: z.coerce.boolean().optional().default(false), + includeValues: z.coerce.boolean().default(false),src/components/molecules/data-table-column-header.tsx (2)
14-14: Remove unnecessary fallback for CSS module class.CSS modules are statically typed and the class will always be defined. The
?? ''fallback is unnecessary and adds noise to the code.Apply this diff:
- <Table.Th className={styles['columnHeaderHover'] ?? ''}> + <Table.Th className={styles.columnHeaderHover}>
17-17: Remove unnecessary fallback for CSS module class.CSS modules are statically typed and the class will always be defined. The
?? ''fallback is unnecessary.Apply this diff:
- <Box className={styles['columnHeaderActions'] ?? ''}> + <Box className={styles.columnHeaderActions}>src/lib/hooks/api/use-data-view-pages.ts (1)
10-24: Consider adding error feedback for invalid inputs.The function silently returns when the name is empty or a mutation is in progress. While this prevents invalid operations, it provides no feedback to the caller about why the operation was skipped.
Consider one of these approaches:
- Return a boolean indicating success:
const createPage = useCallback( async (name: string): Promise<boolean> => { const trimmedName = name.trim(); if (!trimmedName || inProgress) { return false; } await post<CreatePageResponse, CreatePageBody>(CREATE_PAGE_ENDPOINT, { name: trimmedName, emoji: null, parentId: dataSourceId, }); mutate(); return true; }, [dataSourceId, inProgress, post, mutate] );
- Or throw an error for invalid inputs:
if (!trimmedName) { throw new Error('Page name cannot be empty'); } if (inProgress) { throw new Error('Another operation is in progress'); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (50)
.cursor/rules/basics.mdc(1 hunks)commitlint.config.cjs(1 hunks)package.json(1 hunks)src/app/api/v1/data-sources/[id]/columns/[columnId]/route.ts(1 hunks)src/app/api/v1/data-sources/[id]/columns/route.ts(1 hunks)src/app/api/v1/data-sources/[id]/route.ts(2 hunks)src/app/api/v1/data-sources/route.ts(4 hunks)src/app/api/v1/pages/[id]/route.ts(2 hunks)src/app/api/v1/pages/[id]/values/route.ts(1 hunks)src/app/api/v1/pages/route.ts(1 hunks)src/app/layout-client.tsx(2 hunks)src/app/pages/[id]/page.tsx(1 hunks)src/components/atoms/column-header-actions.tsx(1 hunks)src/components/atoms/editable-boolean-cell.tsx(1 hunks)src/components/atoms/editable-page-name-cell.module.css(1 hunks)src/components/atoms/editable-page-name-cell.tsx(1 hunks)src/components/atoms/editable-text-cell.tsx(1 hunks)src/components/molecules/column-form-modal.tsx(1 hunks)src/components/molecules/data-table-column-header.module.css(1 hunks)src/components/molecules/data-table-column-header.tsx(1 hunks)src/components/molecules/data-table-row.tsx(1 hunks)src/components/molecules/new-page-row.tsx(1 hunks)src/components/organisms/data-view-render.tsx(1 hunks)src/components/organisms/data-view-table.tsx(1 hunks)src/lib/hooks/api/use-create-data-source-column.ts(1 hunks)src/lib/hooks/api/use-data-source.ts(1 hunks)src/lib/hooks/api/use-data-sources.ts(1 hunks)src/lib/hooks/api/use-data-view-columns.ts(1 hunks)src/lib/hooks/api/use-data-view-pages.ts(1 hunks)src/lib/hooks/api/use-delete-data-source-column.ts(1 hunks)src/lib/hooks/api/use-page-blocks.ts(1 hunks)src/lib/hooks/api/use-page-details.ts(1 hunks)src/lib/hooks/api/use-page-value-update.ts(1 hunks)src/lib/hooks/api/use-pages.ts(1 hunks)src/lib/hooks/api/use-update-data-source-column.ts(1 hunks)src/lib/hooks/api/use-update-page.ts(1 hunks)src/lib/utils/find-index-and-value.ts(1 hunks)src/types/api/endpoints/create-data-source-column.ts(1 hunks)src/types/api/endpoints/create-data-source.ts(1 hunks)src/types/api/endpoints/delete-data-source-column.ts(1 hunks)src/types/api/endpoints/get-page-blocks.ts(1 hunks)src/types/api/endpoints/get-page-details.ts(2 hunks)src/types/api/endpoints/get-pages.ts(1 hunks)src/types/api/endpoints/index.ts(1 hunks)src/types/api/endpoints/update-data-source-column.ts(1 hunks)src/types/api/endpoints/update-data-source.ts(1 hunks)src/types/api/endpoints/update-page-values.ts(1 hunks)src/types/api/endpoints/update-page.ts(1 hunks)src/types/api/entities.ts(1 hunks)src/types/schemas/entities/container.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Prefer TypeScript type aliases (
type) overinterfacewhen defining shapesPrefer type aliases over interface in TypeScript.
Files:
src/lib/hooks/api/use-delete-data-source-column.tssrc/types/api/endpoints/get-page-blocks.tssrc/components/atoms/column-header-actions.tsxsrc/components/atoms/editable-boolean-cell.tsxsrc/app/api/v1/pages/[id]/values/route.tssrc/types/api/endpoints/index.tssrc/types/api/endpoints/create-data-source.tssrc/components/molecules/data-table-column-header.tsxsrc/lib/hooks/api/use-data-source.tssrc/types/api/endpoints/delete-data-source-column.tssrc/types/api/endpoints/update-data-source.tssrc/components/molecules/new-page-row.tsxsrc/lib/hooks/api/use-update-page.tssrc/types/api/entities.tssrc/components/molecules/data-table-row.tsxsrc/lib/hooks/api/use-data-view-columns.tssrc/lib/hooks/api/use-page-value-update.tssrc/lib/hooks/api/use-update-data-source-column.tssrc/components/atoms/editable-page-name-cell.tsxsrc/lib/hooks/api/use-create-data-source-column.tssrc/lib/hooks/api/use-page-details.tssrc/types/api/endpoints/get-page-details.tssrc/app/api/v1/data-sources/route.tssrc/types/api/endpoints/create-data-source-column.tssrc/lib/hooks/api/use-data-view-pages.tssrc/app/api/v1/pages/[id]/route.tssrc/components/atoms/editable-text-cell.tsxsrc/app/api/v1/data-sources/[id]/columns/route.tssrc/lib/hooks/api/use-pages.tssrc/types/schemas/entities/container.tssrc/app/api/v1/data-sources/[id]/columns/[columnId]/route.tssrc/app/api/v1/pages/route.tssrc/types/api/endpoints/update-page-values.tssrc/types/api/endpoints/update-data-source-column.tssrc/components/organisms/data-view-table.tsxsrc/app/api/v1/data-sources/[id]/route.tssrc/components/molecules/column-form-modal.tsxsrc/lib/hooks/api/use-data-sources.tssrc/lib/utils/find-index-and-value.tssrc/app/layout-client.tsxsrc/types/api/endpoints/get-pages.tssrc/lib/hooks/api/use-page-blocks.tssrc/types/api/endpoints/update-page.tssrc/components/organisms/data-view-render.tsxsrc/app/pages/[id]/page.tsx
src/types/api/endpoints/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define Zod schemas and exported types for new endpoints under src/types/api/endpoints/.
Files:
src/types/api/endpoints/get-page-blocks.tssrc/types/api/endpoints/index.tssrc/types/api/endpoints/create-data-source.tssrc/types/api/endpoints/delete-data-source-column.tssrc/types/api/endpoints/update-data-source.tssrc/types/api/endpoints/get-page-details.tssrc/types/api/endpoints/create-data-source-column.tssrc/types/api/endpoints/update-page-values.tssrc/types/api/endpoints/update-data-source-column.tssrc/types/api/endpoints/get-pages.tssrc/types/api/endpoints/update-page.ts
src/components/@(atoms|molecules|organisms|templates)/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Organize UI components following Atomic Design: atoms, molecules, organisms, templates
Files:
src/components/atoms/column-header-actions.tsxsrc/components/atoms/editable-boolean-cell.tsxsrc/components/molecules/data-table-column-header.tsxsrc/components/molecules/new-page-row.tsxsrc/components/molecules/data-table-row.tsxsrc/components/atoms/editable-page-name-cell.tsxsrc/components/atoms/editable-text-cell.tsxsrc/components/organisms/data-view-table.tsxsrc/components/molecules/column-form-modal.tsxsrc/components/organisms/data-view-render.tsx
src/{components,app}/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Use Mantine UI 8 components for building React UI
Files:
src/components/atoms/column-header-actions.tsxsrc/components/atoms/editable-boolean-cell.tsxsrc/components/molecules/data-table-column-header.tsxsrc/components/molecules/new-page-row.tsxsrc/components/molecules/data-table-row.tsxsrc/components/atoms/editable-page-name-cell.tsxsrc/components/atoms/editable-text-cell.tsxsrc/components/organisms/data-view-table.tsxsrc/components/molecules/column-form-modal.tsxsrc/app/layout-client.tsxsrc/components/organisms/data-view-render.tsxsrc/app/pages/[id]/page.tsx
src/app/api/**/route.@(ts|tsx)
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Implement API endpoints as Next.js route handlers under
src/app/api/**/route.ts(x)
Files:
src/app/api/v1/pages/[id]/values/route.tssrc/app/api/v1/data-sources/route.tssrc/app/api/v1/pages/[id]/route.tssrc/app/api/v1/data-sources/[id]/columns/route.tssrc/app/api/v1/data-sources/[id]/columns/[columnId]/route.tssrc/app/api/v1/pages/route.tssrc/app/api/v1/data-sources/[id]/route.ts
src/app/api/**/route.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/app/api/**/route.ts: Place Next.js API route handlers at src/app/api//route.ts and export functions named GET, POST, etc.
Use apiRoute<Resp, Query, Body>(options, handler) in all API route handlers to enforce Zod validation and typed session.
In routes, import and use expectedQuerySchema/expectedBodySchema from src/types/api when validating input.
Obtain repositories via getWorkspaceRepository()/getContainerRepository() from src/lib/database in API routes.
Always scope database queries with addUserIdToQuery(..., session.user.id) to prevent cross-user data leaks.
Routes must only live under src/app/api/*; do not place route handlers elsewhere.
Files:
src/app/api/v1/pages/[id]/values/route.tssrc/app/api/v1/data-sources/route.tssrc/app/api/v1/pages/[id]/route.tssrc/app/api/v1/data-sources/[id]/columns/route.tssrc/app/api/v1/data-sources/[id]/columns/[columnId]/route.tssrc/app/api/v1/pages/route.tssrc/app/api/v1/data-sources/[id]/route.ts
src/app/**/@(page|layout|loading|error).tsx
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Define routes using Next.js App Router conventions under
src/appwithpage.tsx,layout.tsx,loading.tsx, anderror.tsx
Files:
src/app/pages/[id]/page.tsx
src/app/!(api)/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
On the client, use useAuth() to access user/session state via the auth provider.
Files:
src/app/pages/[id]/page.tsx
src/app/!(api)/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Front-end code should call backend endpoints through src/lib/api/client.ts rather than raw fetch/axios calls.
Files:
src/app/pages/[id]/page.tsx
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: danships/thoth PR: 0
File: .cursor/rules/basics.mdc:0-0
Timestamp: 2025-10-23T17:57:41.144Z
Learning: Applies to src/{components,app}/**/*.tsx : Use Mantine UI 8 components for building React UI
📚 Learning: 2025-10-23T17:57:41.144Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .cursor/rules/basics.mdc:0-0
Timestamp: 2025-10-23T17:57:41.144Z
Learning: Applies to src/components/@(atoms|molecules|organisms|templates)/**/*.{ts,tsx} : Organize UI components following Atomic Design: atoms, molecules, organisms, templates
Applied to files:
.cursor/rules/basics.mdcsrc/components/molecules/new-page-row.tsxsrc/components/atoms/editable-page-name-cell.tsxsrc/components/molecules/column-form-modal.tsx
📚 Learning: 2025-10-23T17:57:41.144Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .cursor/rules/basics.mdc:0-0
Timestamp: 2025-10-23T17:57:41.144Z
Learning: Applies to src/{components,app}/**/*.tsx : Use Mantine UI 8 components for building React UI
Applied to files:
.cursor/rules/basics.mdcpackage.jsonsrc/app/layout-client.tsx
📚 Learning: 2025-10-23T17:57:41.144Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .cursor/rules/basics.mdc:0-0
Timestamp: 2025-10-23T17:57:41.144Z
Learning: Applies to src/app/**/@(page|layout|loading|error).tsx : Define routes using Next.js App Router conventions under `src/app` with `page.tsx`, `layout.tsx`, `loading.tsx`, and `error.tsx`
Applied to files:
.cursor/rules/basics.mdc
📚 Learning: 2025-10-23T17:57:41.144Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .cursor/rules/basics.mdc:0-0
Timestamp: 2025-10-23T17:57:41.144Z
Learning: Applies to src/app/api/**/route.@(ts|tsx) : Implement API endpoints as Next.js route handlers under `src/app/api/**/route.ts(x)`
Applied to files:
.cursor/rules/basics.mdcsrc/app/api/v1/pages/[id]/values/route.tssrc/app/api/v1/pages/[id]/route.tssrc/app/api/v1/data-sources/[id]/columns/route.tssrc/app/api/v1/data-sources/[id]/columns/[columnId]/route.tssrc/app/api/v1/pages/route.ts
📚 Learning: 2025-10-24T20:04:43.029Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-24T20:04:43.029Z
Learning: Applies to src/app/api/**/route.ts : Place Next.js API route handlers at src/app/api/<route>/route.ts and export functions named GET, POST, etc.
Applied to files:
.cursor/rules/basics.mdcsrc/app/api/v1/pages/[id]/values/route.tssrc/app/api/v1/pages/[id]/route.tssrc/app/api/v1/data-sources/[id]/columns/route.tssrc/app/api/v1/data-sources/[id]/columns/[columnId]/route.tssrc/app/api/v1/pages/route.ts
📚 Learning: 2025-10-24T20:04:43.029Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-24T20:04:43.029Z
Learning: Applies to src/app/!(api)/**/*.tsx : On the client, use useAuth() to access user/session state via the auth provider.
Applied to files:
.cursor/rules/basics.mdcsrc/app/layout-client.tsx
📚 Learning: 2025-10-24T20:04:43.029Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-24T20:04:43.029Z
Learning: Applies to src/types/api/endpoints/**/*.ts : Define Zod schemas and exported types for new endpoints under src/types/api/endpoints/.
Applied to files:
src/types/api/endpoints/get-page-blocks.tssrc/types/api/endpoints/index.tssrc/types/api/endpoints/create-data-source.tssrc/types/api/endpoints/delete-data-source-column.tssrc/types/api/entities.tssrc/types/api/endpoints/get-page-details.tssrc/types/api/endpoints/create-data-source-column.tssrc/types/schemas/entities/container.tssrc/types/api/endpoints/update-page-values.tssrc/types/api/endpoints/update-data-source-column.tssrc/types/api/endpoints/get-pages.tssrc/types/api/endpoints/update-page.ts
📚 Learning: 2025-10-24T20:04:43.029Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-24T20:04:43.029Z
Learning: Applies to src/app/api/**/route.ts : In routes, import and use expectedQuerySchema/expectedBodySchema from src/types/api when validating input.
Applied to files:
src/app/api/v1/pages/[id]/values/route.tssrc/app/api/v1/data-sources/route.tssrc/app/api/v1/pages/[id]/route.tssrc/app/api/v1/data-sources/[id]/columns/route.tssrc/app/api/v1/data-sources/[id]/columns/[columnId]/route.tssrc/app/api/v1/data-sources/[id]/route.ts
📚 Learning: 2025-10-24T20:04:43.029Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-24T20:04:43.029Z
Learning: Applies to src/app/api/**/route.ts : Use apiRoute<Resp, Query, Body>(options, handler) in all API route handlers to enforce Zod validation and typed session.
Applied to files:
src/app/api/v1/pages/[id]/values/route.tssrc/app/api/v1/pages/[id]/route.tssrc/app/api/v1/data-sources/[id]/columns/route.tssrc/app/api/v1/data-sources/[id]/columns/[columnId]/route.ts
📚 Learning: 2025-10-24T20:04:43.029Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-24T20:04:43.029Z
Learning: Applies to src/app/api/**/route.ts : Obtain repositories via getWorkspaceRepository()/getContainerRepository() from src/lib/database in API routes.
Applied to files:
src/app/api/v1/data-sources/route.tssrc/app/api/v1/pages/[id]/route.tssrc/app/api/v1/data-sources/[id]/columns/[columnId]/route.ts
📚 Learning: 2025-10-24T20:04:43.029Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-24T20:04:43.029Z
Learning: Applies to src/lib/api/client.ts : After implementing an endpoint, add a typed client helper in src/lib/api/client.ts (baseURL is /api/v1).
Applied to files:
src/types/api/endpoints/create-data-source-column.ts
📚 Learning: 2025-10-24T20:04:43.029Z
Learnt from: CR
Repo: danships/thoth PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-24T20:04:43.029Z
Learning: Applies to src/app/api/**/route.ts : Routes must only live under src/app/api/*; do not place route handlers elsewhere.
Applied to files:
src/app/api/v1/data-sources/[id]/columns/[columnId]/route.ts
🧬 Code graph analysis (33)
src/lib/hooks/api/use-delete-data-source-column.ts (2)
src/lib/hooks/use-cud-api.ts (1)
useCudApi(5-44)src/types/api/endpoints/delete-data-source-column.ts (1)
DELETE_DATA_SOURCE_COLUMN_ENDPOINT(3-3)
src/app/api/v1/pages/[id]/values/route.ts (6)
src/types/schemas/entities/container.ts (1)
pageValueSchema(16-16)src/lib/api/route-wrapper.ts (1)
apiRoute(15-128)src/lib/database/index.ts (1)
getContainerRepository(22-25)src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)src/lib/errors/not-found-error.ts (1)
NotFoundError(3-7)src/lib/errors/bad-request-error.ts (1)
BadRequestError(3-7)
src/types/api/endpoints/create-data-source.ts (1)
src/types/schemas/entities/container.ts (4)
dataSourceContainerSchema(52-57)stringColumnSchema(24-24)numberColumnSchema(25-25)booleanColumnSchema(26-26)
src/components/molecules/data-table-column-header.tsx (2)
src/types/schemas/entities/container.ts (1)
Column(30-30)src/components/atoms/column-header-actions.tsx (1)
ColumnHeaderActions(9-28)
src/lib/hooks/api/use-data-source.ts (2)
src/types/api/endpoints/get-data-source.ts (2)
GetDataSourceResponse(10-10)GET_DATA_SOURCE_ENDPOINT(6-6)src/lib/swr/fetcher.ts (1)
swrFetcher(3-3)
src/lib/hooks/api/use-update-page.ts (3)
src/types/api/endpoints/get-pages.ts (1)
GetPagesResponse(23-23)src/lib/hooks/use-cud-api.ts (1)
useCudApi(5-44)src/types/api/endpoints/update-page.ts (2)
UpdatePageBody(18-18)UPDATE_PAGE_ENDPOINT(7-7)
src/components/molecules/data-table-row.tsx (5)
src/types/api/entities.ts (1)
Page(13-13)src/types/schemas/entities/container.ts (2)
PageValue(17-17)Column(30-30)src/components/atoms/editable-page-name-cell.tsx (1)
EditablePageNameCell(14-86)src/components/atoms/editable-boolean-cell.tsx (1)
EditableBooleanCell(9-11)src/components/atoms/editable-text-cell.tsx (1)
EditableTextCell(8-62)
src/lib/hooks/api/use-data-view-columns.ts (3)
src/lib/hooks/api/use-create-data-source-column.ts (1)
useCreateDataSourceColumn(9-29)src/lib/hooks/api/use-update-data-source-column.ts (1)
useUpdateDataSourceColumn(9-26)src/lib/hooks/api/use-delete-data-source-column.ts (1)
useDeleteDataSourceColumn(5-21)
src/lib/hooks/api/use-page-value-update.ts (4)
src/types/api/endpoints/get-pages.ts (1)
GetPagesResponse(23-23)src/lib/hooks/use-cud-api.ts (1)
useCudApi(5-44)src/types/schemas/entities/container.ts (1)
PageValue(17-17)src/types/api/endpoints/update-page-values.ts (1)
UPDATE_PAGE_VALUES_ENDPOINT(4-4)
src/lib/hooks/api/use-update-data-source-column.ts (2)
src/lib/hooks/use-cud-api.ts (1)
useCudApi(5-44)src/types/api/endpoints/update-data-source-column.ts (3)
UpdateDataSourceColumnBody(14-14)UpdateDataSourceColumnResponse(17-17)UPDATE_DATA_SOURCE_COLUMN_ENDPOINT(5-5)
src/lib/hooks/api/use-create-data-source-column.ts (2)
src/lib/hooks/use-cud-api.ts (1)
useCudApi(5-44)src/types/api/endpoints/create-data-source-column.ts (3)
CreateDataSourceColumnResponse(14-14)CreateDataSourceColumnBody(11-11)CREATE_DATA_SOURCE_COLUMN_ENDPOINT(5-5)
src/lib/hooks/api/use-page-details.ts (2)
src/types/api/endpoints/get-page-details.ts (2)
GetPageDetailsResponse(16-16)GET_PAGE_DETAILS_ENDPOINT(7-7)src/lib/swr/fetcher.ts (1)
swrFetcher(3-3)
src/types/api/endpoints/get-page-details.ts (3)
src/types/api/entities.ts (1)
pageSchema(5-12)src/types/api/endpoints/get-page-blocks.ts (1)
getPageBlocksResponseSchema(13-15)src/types/schemas/entities/container.ts (1)
pageContainerSchema(41-50)
src/types/api/endpoints/create-data-source-column.ts (2)
src/types/schemas/entities/container.ts (1)
columnSchema(29-29)src/types/api/utilities.ts (1)
DataWrapper(2-4)
src/lib/hooks/api/use-data-view-pages.ts (3)
src/lib/hooks/api/use-pages.ts (1)
usePagesByDataSource(21-36)src/lib/hooks/use-cud-api.ts (1)
useCudApi(5-44)src/types/api/endpoints/create-page.ts (3)
CreatePageResponse(12-12)CreatePageBody(22-22)CREATE_PAGE_ENDPOINT(6-6)
src/app/api/v1/pages/[id]/route.ts (5)
src/lib/api/route-wrapper.ts (1)
apiRoute(15-128)src/types/api/endpoints/update-page.ts (5)
UpdatePageResponse(19-19)UpdatePageParameters(26-26)UpdatePageBody(18-18)updatePageBodySchema(10-15)updatePageParametersSchema(23-25)src/lib/database/index.ts (1)
getContainerRepository(22-25)src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)src/lib/errors/not-found-error.ts (1)
NotFoundError(3-7)
src/app/api/v1/data-sources/[id]/columns/route.ts (6)
src/lib/api/route-wrapper.ts (1)
apiRoute(15-128)src/types/schemas/entities/container.ts (2)
columnSchema(29-29)Column(30-30)src/lib/database/index.ts (1)
getContainerRepository(22-25)src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)src/lib/errors/not-found-error.ts (1)
NotFoundError(3-7)src/types/database/index.ts (1)
DataSourceContainer(13-13)
src/lib/hooks/api/use-pages.ts (1)
src/types/api/endpoints/get-pages.ts (2)
GetPagesResponse(23-23)GET_PAGES_ENDPOINT(6-6)
src/types/schemas/entities/container.ts (2)
src/types/schemas/utilities.ts (1)
withParentIdSchema(7-9)src/types/database/index.ts (2)
dataSourceContainerSchema(7-7)containerSchema(18-18)
src/app/api/v1/data-sources/[id]/columns/[columnId]/route.ts (5)
src/lib/api/route-wrapper.ts (1)
apiRoute(15-128)src/types/api/endpoints/update-data-source-column.ts (4)
UpdateDataSourceColumnResponse(17-17)UpdateDataSourceColumnBody(14-14)updateDataSourceColumnBodySchema(7-12)updateDataSourceColumnParametersSchema(20-23)src/lib/database/index.ts (1)
getContainerRepository(22-25)src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)src/lib/errors/not-found-error.ts (1)
NotFoundError(3-7)
src/app/api/v1/pages/route.ts (1)
src/types/api/endpoints/get-pages.ts (1)
GetPagesResponse(23-23)
src/types/api/endpoints/update-page-values.ts (1)
src/types/schemas/entities/container.ts (1)
pageValueSchema(16-16)
src/types/api/endpoints/update-data-source-column.ts (2)
src/types/schemas/entities/container.ts (1)
columnSchema(29-29)src/types/api/utilities.ts (1)
DataWrapper(2-4)
src/components/organisms/data-view-table.tsx (9)
src/types/schemas/entities/container.ts (1)
Column(30-30)src/types/api/endpoints/get-pages.ts (1)
GetPagesResponse(23-23)src/lib/hooks/api/use-data-view-columns.ts (1)
useDataViewColumns(9-20)src/lib/hooks/api/use-page-value-update.ts (1)
usePageValueUpdate(13-34)src/lib/hooks/api/use-update-page.ts (1)
useUpdatePage(12-43)src/components/molecules/data-table-column-header.tsx (1)
DataTableColumnHeader(12-23)src/components/molecules/data-table-row.tsx (1)
DataTableRow(19-79)src/components/molecules/new-page-row.tsx (1)
NewPageRow(11-28)src/components/molecules/column-form-modal.tsx (1)
ColumnFormModal(20-100)
src/app/api/v1/data-sources/[id]/route.ts (1)
src/types/api/endpoints/get-data-source.ts (1)
GetDataSourceResponse(10-10)
src/components/molecules/column-form-modal.tsx (1)
src/types/schemas/entities/container.ts (1)
Column(30-30)
src/lib/hooks/api/use-data-sources.ts (2)
src/types/api/endpoints/get-data-sources.ts (2)
GetDataSourcesResponse(10-10)GET_DATA_SOURCES_ENDPOINT(6-6)src/lib/swr/fetcher.ts (1)
swrFetcher(3-3)
src/app/layout-client.tsx (1)
src/lib/auth/provider.tsx (1)
AuthProvider(16-53)
src/types/api/endpoints/get-pages.ts (3)
src/types/api/entities.ts (1)
pageSchema(5-12)src/types/schemas/entities/container.ts (1)
pageContainerSchema(41-50)src/types/database/index.ts (1)
pageContainerSchema(7-7)
src/lib/hooks/api/use-page-blocks.ts (2)
src/types/api/endpoints/get-page-blocks.ts (2)
GetPageBlocksResponse(17-19)GET_PAGE_BLOCKS_ENDPOINT(4-4)src/lib/swr/fetcher.ts (1)
swrFetcher(3-3)
src/types/api/endpoints/update-page.ts (2)
src/types/api/entities.ts (1)
pageSchema(5-12)src/types/api/utilities.ts (1)
DataWrapper(2-4)
src/components/organisms/data-view-render.tsx (3)
src/lib/hooks/api/use-data-view-pages.ts (1)
useDataViewPages(6-34)src/lib/hooks/api/use-data-source.ts (1)
useDataSource(5-6)src/components/organisms/data-view-table.tsx (1)
DataViewTable(34-172)
src/app/pages/[id]/page.tsx (1)
src/components/organisms/page-detail-editor.tsx (1)
PageDetailEditor(16-42)
🪛 Biome (2.1.2)
src/components/organisms/data-view-render.tsx
[error] 7-7: Do not shadow the global "DataView" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ 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: ci
🔇 Additional comments (33)
commitlint.config.cjs (1)
14-14: Verify that disabling body line length limit is intentional.Setting
body-max-line-lengthto[0]disables commit message body line length checking entirely. This allows unlimited line length in commit bodies, which can reduce readability. Most teams enforce a limit (typically 72-100 characters) to ensure commit messages remain readable in various tools and terminal widths.Please confirm this change is intentional. If unlimited length is desired for flexibility, this is fine. Otherwise, consider setting a reasonable limit:
- 'body-max-line-length': [0], + 'body-max-line-length': [2, 'always', 100],src/lib/utils/find-index-and-value.ts (1)
1-10: Implementation is correct and efficient.The function correctly combines
findIndexwith value retrieval in a single pass. The non-null assertion on line 9 is safe because the preceding check ensuresindex !== -1.package.json (1)
25-25: LGTM! Mantine Modals integration.The
@mantine/modalspackage addition is consistent with the project's use of Mantine UI 8 and aligns with the ModalsProvider integration in the layout.src/types/api/endpoints/update-data-source.ts (1)
15-21: LGTM! Consistent extension of response schema.The addition of
columns: trueto the response schema correctly extends the update endpoint to include column information, aligning with the broader column management feature.src/components/atoms/editable-page-name-cell.module.css (1)
1-30: LGTM! Well-structured CSS module for editable cell.The CSS module provides appropriate styling for the editable page name cell with:
- Smooth hover-reveal transition for the action button (0.2s opacity)
- Flexible layout preventing text overflow (
min-width: 0)- Clear cursor states for disabled/enabled modes
src/types/schemas/entities/container.ts (3)
11-17: LGTM! Well-structured value schemas.The value schemas correctly use discriminated unions with
z.literalfor type safety. The structure is consistent across all value types (string, number, boolean) and properly exported.
19-30: LGTM! Consistent column schema pattern.The column schemas follow a clean pattern:
- Base schema with common fields (id, name)
- Extended schemas with type discrimination
- Discriminated union for type safety
This mirrors the value schema structure and maintains consistency.
48-48: LGTM! Appropriate values field addition.The optional
valuesfield usingz.recordcorrectly supports dynamic key-value storage for page values with proper type safety.src/components/molecules/data-table-column-header.module.css (1)
1-8: LGTM! Consistent hover-reveal pattern.The CSS module implements the same hover-reveal pattern used in
editable-page-name-cell.module.css, providing a consistent user experience across the data table interface..cursor/rules/basics.mdc (1)
14-17: LGTM! Clear styling guidelines.The addition of CSS module guidelines establishes a consistent styling approach that aligns with the new CSS modules introduced in this PR. The naming convention (component-name.module.css) promotes maintainability.
src/types/api/endpoints/index.ts (2)
9-9: LGTM! New endpoint export added.The
get-page-blocksendpoint is correctly exported, making it available through the central API barrel.
14-18: LGTM! Column and page update endpoints exported.The new endpoint exports for column management and page updates are correctly added:
- Data source column CRUD operations (create, update, delete)
- Page values and general page updates
These align with the comprehensive column management system introduced in this PR.
src/app/pages/[id]/page.tsx (1)
112-112: LGTM! Data binding updated to match API restructuring.The change from
pageDetails.page.blockstopageDetails.blocksaligns with the restructured API response where blocks are now exposed at the page-level rather than nested under the page object. The fallback to an empty array is preserved for defensive programming.src/types/api/entities.ts (1)
26-32: LGTM! DataSource schema extended to include columns.The addition of
columns: trueto thedataSourceSchemapick statement correctly extends the public DataSource type to include the columns property, supporting the new column management features introduced in this PR.src/lib/hooks/api/use-data-sources.ts (1)
1-5: LGTM! Refactored to use centralized endpoint constant.Replacing the hard-coded endpoint string with
GET_DATA_SOURCES_ENDPOINTfollows best practices by centralizing endpoint definitions and reducing magic strings. This improves maintainability and consistency across the codebase.src/lib/hooks/api/use-data-source.ts (1)
1-6: LGTM! Well-implemented hook following established patterns.The new
useDataSourcehook correctly implements conditional fetching with SWR, follows the same patterns as other hooks in the codebase (e.g.,usePageBlocks), and maintains type safety withGetDataSourceResponse.src/app/layout-client.tsx (1)
4-4: LGTM! ModalsProvider correctly integrated.The addition of
ModalsProviderfrom@mantine/modalsfollows Mantine UI 8 best practices and is correctly nested withinMantineProviderand wrappingAuthProvider. This enables modal functionality throughout the application. Based on learningsAlso applies to: 32-34
src/app/api/v1/data-sources/[id]/route.ts (2)
18-40: LGTM! GET endpoint correctly handles optional columns.The GET endpoint properly includes the
columnsproperty in the response using a defensive check with theinoperator and providing an empty array as a fallback. The use ofsatisfies GetDataSourceResponseensures type safety.
42-80: LGTM! PATCH endpoint correctly handles optional columns.The PATCH endpoint consistently includes the
columnsproperty using the same defensive pattern as the GET endpoint. ThelastUpdatedtimestamp is properly set, and the response satisfies theUpdateDataSourceResponsetype.src/lib/hooks/api/use-page-blocks.ts (1)
1-6: LGTM! Refactored to use centralized endpoint constant.The refactor replaces the hard-coded endpoint string with
GET_PAGE_BLOCKS_ENDPOINT, following the same pattern as other hooks in the codebase. The dynamic parameter substitution using.replace(':id', pageId)is correct and improves maintainability.src/lib/hooks/api/use-pages.ts (1)
19-22: LGTM! Options parameter added with backward compatibility.The addition of the optional
optionsparameter withincludeValuessupport is well-designed:
- Maintains backward compatibility (options is optional)
- Proper default values defined
- Type-safe implementation
src/components/molecules/data-table-column-header.tsx (1)
12-23: LGTM!The component structure is clean and follows best practices. It correctly uses Mantine UI 8 components, follows the Atomic Design pattern (molecules), and uses type aliases as per the coding guidelines.
src/components/atoms/editable-boolean-cell.tsx (1)
1-11: LGTM!The component is well-structured, follows Atomic Design principles (atoms), uses Mantine UI 8 components correctly, and properly transforms the checkbox event to a boolean value for the
onChangecallback.src/lib/hooks/api/use-page-details.ts (1)
2-9: LGTM!Centralizing the endpoint constant improves maintainability and consistency across the codebase. The implementation correctly replaces the path parameter and appends the query string.
src/lib/hooks/api/use-data-view-pages.ts (1)
6-34: LGTM!The hook is well-structured, follows React 19 best practices with proper use of
useCallback, and correctly manages dependencies. The composition of existing hooks is clean and the returned API is intuitive.src/components/atoms/column-header-actions.tsx (1)
1-28: LGTM!The component follows Atomic Design principles (atoms), uses Mantine UI 8 components with the correct API (
leftSectionfor icons), and properly uses type aliases. The implementation is clean and provides good UX with the red color for the Delete action.Based on learnings.
src/lib/hooks/api/use-update-page.ts (2)
15-37: LGTM!The
updatePageimplementation correctly handles the nested response structure from the updatedGetPagesResponsetype. The optimistic update pattern withrevalidate: falseprovides good UX by immediately reflecting changes in the UI while the server processes the request.
5-10: LGTM!The type definition correctly uses a type alias (per coding guidelines) and properly types the
mutatePagescallback to match the SWR mutate signature with the updatedGetPagesResponsetype.src/lib/hooks/api/use-delete-data-source-column.ts (2)
8-15: LGTM!The
deleteColumnimplementation correctly handles the two path parameter replacements for the endpoint. The function is properly memoized withuseCallbackand includes all necessary dependencies.
17-20: LGTM!The return object follows a consistent pattern with other CUD hooks in the codebase, providing both the deletion function and the progress state.
src/types/api/endpoints/get-pages.ts (1)
20-22: <!-- [scratchpad_start]
[task review_file_1/1 verify_breaking_change_complete]
Breaking change verification complete
[observations]
- API route (src/app/api/v1/pages/route.ts line 30): returns { page: {...}, values?: ... }
- Schema (src/types/api/endpoints/get-pages.ts lines 20-22): z.object({ page: pageSchema, values: optional })
- All consumers (hooks, components) already type responses as GetPagesResponse
- No test files found - likely not comprehensive test coverage in repo
- API logic: values included only if query.includeValues is true
[analysis]- The API implementation ALREADY returns the nested structure
- Schema change formalizes existing runtime behavior, NOT a breaking change
- All calling code already handles this structure correctly
- The consumers (usePagesByParent, usePagesByDataSource, data-view-table) receive data matching the schema
- No code updates needed anywhere in the codebase
- Original review comment incorrectly assumes this is a breaking change
[conclusion]- NOT a breaking change - this is a schema correction/formalization
- Code is internally consistent and already working with this structure
- Original review comment's concern is invalid
[done]- Verified API implementation matches schema
- Confirmed all consumers already handle nested structure
- No breaking changes detected
[scratchpad_start] -->The schema change is not a breaking change—the API already returns this structure.
The API implementation at
src/app/api/v1/pages/route.ts(line 30) already constructs responses with the nested object structure:{ page: {...}, values?: ... }. All existing consumers (usePagesByParent,usePagesByDataSource,data-view-table) are already typed withGetPagesResponseand correctly handle this structure. The schema update formalizes the actual runtime behavior rather than introducing a breaking change.Likely an incorrect or invalid review comment.
src/components/molecules/column-form-modal.tsx (1)
74-99: Modal wiring looks solid.The form integration with Mantine components and the submit/close handling read cleanly. Nice work keeping the UX tight.
src/lib/hooks/api/use-page-value-update.ts (1)
16-26: Nice optimistic cache update.The targeted
mutatePagescall keeps the SWR cache in sync without a round-trip—this should make the UI feel snappy.
| export const DELETE = apiRoute<void, undefined, { id: string; columnId: string }>({}, async ({ params }, session) => { | ||
| const containerRepository = await getContainerRepository(); | ||
| const dataSource = await containerRepository.getOneByQuery( | ||
| addUserIdToQuery(containerRepository.createQuery().eq('id', params.id), session.user.id).eq('type', 'data-source') | ||
| ); | ||
|
|
||
| if (!dataSource || dataSource.type !== 'data-source') { | ||
| throw new NotFoundError('Data source not found', true); | ||
| } | ||
|
|
||
| const nextColumns = (dataSource.columns ?? []).filter((c) => c.id !== params.columnId); | ||
| if (nextColumns.length === (dataSource.columns ?? []).length) { | ||
| throw new NotFoundError('Column not found', true); | ||
| } | ||
|
|
||
| await containerRepository.update({ ...dataSource, columns: nextColumns, lastUpdated: new Date().toISOString() }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate DELETE route params using the shared schema
The DELETE handler skips expectedParamsSchema, so we lose the guardrails that every other route gets from the typed Zod validation. Please reuse updateDataSourceColumnParametersSchema here so malformed requests fail fast and we stay consistent with the rest of the API surface.
-export const DELETE = apiRoute<void, undefined, { id: string; columnId: string }>({}, async ({ params }, session) => {
+export const DELETE = apiRoute<void, undefined, UpdateDataSourceColumnParameters>(
+ {
+ expectedParamsSchema: updateDataSourceColumnParametersSchema,
+ },
+ async ({ params }, session) => {
const containerRepository = await getContainerRepository();
const dataSource = await containerRepository.getOneByQuery(
addUserIdToQuery(containerRepository.createQuery().eq('id', params.id), session.user.id).eq('type', 'data-source')
);
if (!dataSource || dataSource.type !== 'data-source') {
throw new NotFoundError('Data source not found', true);
}
const nextColumns = (dataSource.columns ?? []).filter((c) => c.id !== params.columnId);
if (nextColumns.length === (dataSource.columns ?? []).length) {
throw new NotFoundError('Column not found', true);
}
await containerRepository.update({ ...dataSource, columns: nextColumns, lastUpdated: new Date().toISOString() });
-});
+ }
+);As per coding guidelines
📝 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 const DELETE = apiRoute<void, undefined, { id: string; columnId: string }>({}, async ({ params }, session) => { | |
| const containerRepository = await getContainerRepository(); | |
| const dataSource = await containerRepository.getOneByQuery( | |
| addUserIdToQuery(containerRepository.createQuery().eq('id', params.id), session.user.id).eq('type', 'data-source') | |
| ); | |
| if (!dataSource || dataSource.type !== 'data-source') { | |
| throw new NotFoundError('Data source not found', true); | |
| } | |
| const nextColumns = (dataSource.columns ?? []).filter((c) => c.id !== params.columnId); | |
| if (nextColumns.length === (dataSource.columns ?? []).length) { | |
| throw new NotFoundError('Column not found', true); | |
| } | |
| await containerRepository.update({ ...dataSource, columns: nextColumns, lastUpdated: new Date().toISOString() }); | |
| }); | |
| export const DELETE = apiRoute<void, undefined, UpdateDataSourceColumnParameters>( | |
| { | |
| expectedParamsSchema: updateDataSourceColumnParametersSchema, | |
| }, | |
| async ({ params }, session) => { | |
| const containerRepository = await getContainerRepository(); | |
| const dataSource = await containerRepository.getOneByQuery( | |
| addUserIdToQuery(containerRepository.createQuery().eq('id', params.id), session.user.id).eq('type', 'data-source') | |
| ); | |
| if (!dataSource || dataSource.type !== 'data-source') { | |
| throw new NotFoundError('Data source not found', true); | |
| } | |
| const nextColumns = (dataSource.columns ?? []).filter((c) => c.id !== params.columnId); | |
| if (nextColumns.length === (dataSource.columns ?? []).length) { | |
| throw new NotFoundError('Column not found', true); | |
| } | |
| await containerRepository.update({ ...dataSource, columns: nextColumns, lastUpdated: new Date().toISOString() }); | |
| } | |
| ); |
🤖 Prompt for AI Agents
In src/app/api/v1/data-sources/[id]/columns/[columnId]/route.ts around lines 50
to 66, the DELETE handler does not apply the shared Zod params schema so
malformed params bypass validation; update the apiRoute invocation to pass
expectedParamsSchema using the existing updateDataSourceColumnParametersSchema
(or its exported expectedParamsSchema) so the route validates params before
handler execution, adjust the apiRoute generic types if needed to match the
schema, and ensure any imports for the schema are added at the top.
| const createColumnBodySchema = z.object({ | ||
| name: z.string().min(1), | ||
| type: z.union([z.literal('string'), z.literal('number'), z.literal('boolean')]), | ||
| }); | ||
|
|
||
| export const POST = apiRoute< | ||
| z.infer<typeof columnSchema>, | ||
| undefined, | ||
| { id: string }, | ||
| z.infer<typeof createColumnBodySchema> | ||
| >({ expectedBodySchema: createColumnBodySchema }, async ({ body, params }, session) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reuse the shared request schema.
We already publish createDataSourceColumnBodySchema (and companions) under @/types/api. Duplicating the Zod definition here risks the server drifting from the typed client contracts. Please import the shared schema and hand it to apiRoute instead of redefining it locally. This keeps validation logic consistent across layers and matches the API route guidelines. Based on learnings
🤖 Prompt for AI Agents
In src/app/api/v1/data-sources/[id]/columns/route.ts around lines 10 to 20, the
handler redefines createColumnBodySchema locally instead of reusing the shared
createDataSourceColumnBodySchema from @/types/api; import the shared schema (and
its TypeScript inference type) from '@/types/api', remove the local Zod
definition, and pass the shared schema to apiRoute as expectedBodySchema and in
the generic type parameter so the route uses the canonical request shape used by
the client.
| if (query.includeBlocks) { | ||
| returnValue.page.blocks = page.blocks ?? []; | ||
| returnValue.blocks = page.blocks ?? []; | ||
| } | ||
| if (query.includeValues) { | ||
| returnValue.values = page.values ?? {}; | ||
| } |
There was a problem hiding this comment.
Parse boolean query switches explicitly.
Here too query.includeBlocks / query.includeValues come through as strings, so "false" still enters the branch. To honor the caller’s intent (and avoid leaking blocks/values unexpectedly), compare against 'true' or coerce once at the top of the handler.
- if (query.includeBlocks) {
+ if (query.includeBlocks === 'true') {
returnValue.blocks = page.blocks ?? [];
}
- if (query.includeValues) {
+ if (query.includeValues === 'true') {
returnValue.values = page.values ?? {};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (query.includeBlocks) { | |
| returnValue.page.blocks = page.blocks ?? []; | |
| returnValue.blocks = page.blocks ?? []; | |
| } | |
| if (query.includeValues) { | |
| returnValue.values = page.values ?? {}; | |
| } | |
| if (query.includeBlocks === 'true') { | |
| returnValue.blocks = page.blocks ?? []; | |
| } | |
| if (query.includeValues === 'true') { | |
| returnValue.values = page.values ?? {}; | |
| } |
🤖 Prompt for AI Agents
In src/app/api/v1/pages/[id]/route.ts around lines 64 to 69, the handler treats
query.includeBlocks and query.includeValues as truthy strings so values like
"false" still pass the check; explicitly parse them as booleans at the top of
the handler (e.g. const includeBlocks = query.includeBlocks === 'true'; const
includeValues = query.includeValues === 'true') and then use those boolean
variables in the if statements to ensure only an explicit 'true' includes
blocks/values.
| const bodySchema = z.record(z.string(), pageValueSchema); | ||
|
|
||
| export const PATCH = apiRoute<void, undefined, { id: string }, z.infer<typeof bodySchema>>( | ||
| { expectedBodySchema: bodySchema }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reuse shared endpoint schemas in this route.
We already export updatePageValuesBodySchema and updatePageValuesParametersSchema from src/types/api. Re‑declaring the body schema here drifts from the single source of truth and breaks the repo rule that routes should reuse the shared Zod schemas. Please import the shared schemas (and accompanying types) and wire them into apiRoute via expectedBodySchema/expectedParamsSchema instead of redefining them locally. This keeps validation consistent across layers.
-import { pageValueSchema } from '@/types/schemas/entities/container';
-import { z } from 'zod';
-
-const bodySchema = z.record(z.string(), pageValueSchema);
-
-export const PATCH = apiRoute<void, undefined, { id: string }, z.infer<typeof bodySchema>>(
- { expectedBodySchema: bodySchema },
+import {
+ updatePageValuesBodySchema,
+ updatePageValuesParametersSchema,
+ type UpdatePageValuesBody,
+ type UpdatePageValuesParameters,
+} from '@/types/api';
+
+export const PATCH = apiRoute<void, undefined, UpdatePageValuesParameters, UpdatePageValuesBody>(
+ {
+ expectedBodySchema: updatePageValuesBodySchema,
+ expectedParamsSchema: updatePageValuesParametersSchema,
+ },| const mergedValues = { ...page.values, ...body }; | ||
| await containerRepository.update({ | ||
| ...page, | ||
| values: mergedValues, | ||
| lastUpdated: new Date().toISOString(), |
There was a problem hiding this comment.
Handle pages with no existing values before spreading.
If a page predates this feature, page.values will be undefined. Spreading undefined triggers TypeError: Cannot convert undefined or null to object, so the first update for such a page will 500. Default to {} before merging.
- const mergedValues = { ...page.values, ...body };
+ const mergedValues = { ...(page.values ?? {}), ...body };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const mergedValues = { ...page.values, ...body }; | |
| await containerRepository.update({ | |
| ...page, | |
| values: mergedValues, | |
| lastUpdated: new Date().toISOString(), | |
| const mergedValues = { ...(page.values ?? {}), ...body }; | |
| await containerRepository.update({ | |
| ...page, | |
| values: mergedValues, | |
| lastUpdated: new Date().toISOString(), |
🤖 Prompt for AI Agents
In src/app/api/v1/pages/[id]/values/route.ts around lines 52–56, the code
spreads page.values directly which throws when page.values is undefined for
older pages; change the merge to default page.values to an empty object (e.g.,
use page.values || {} or nullish coalescing) before spreading so mergedValues is
always an object, then pass that mergedValues into the update payload and keep
lastUpdated as before.
| const text = event.currentTarget.textContent ?? ''; | ||
| if (type === 'number') { | ||
| const numberValue = Number(text); | ||
| if (Number.isNaN(numberValue)) { | ||
| return; | ||
| } | ||
| onBlur(numberValue); | ||
| } else { | ||
| onBlur(text); | ||
| } |
There was a problem hiding this comment.
Prevent numeric cells from saving blanks as zero and revert invalid text
When a user clears a numeric cell, text becomes '', Number('') evaluates to 0, and we persist 0 even though the intent was to erase the value. Likewise, if the user types something non-numeric (e.g. 12abc), we bail out without calling onBlur, but the DOM keeps showing the invalid text so the UI no longer reflects the stored value. Both cases result in confusing or incorrect data. Please guard against empty strings and reset the display whenever parsing fails, so we either commit a real number or restore the previous value.
- if (type === 'number') {
- const numberValue = Number(text);
- if (Number.isNaN(numberValue)) {
- return;
- }
- onBlur(numberValue);
+ if (type === 'number') {
+ const previousDisplay = value == null ? '' : String(value);
+ if (text.trim() === '') {
+ event.currentTarget.textContent = previousDisplay;
+ return;
+ }
+ const numberValue = Number(text);
+ if (Number.isNaN(numberValue)) {
+ event.currentTarget.textContent = previousDisplay;
+ return;
+ }
+ onBlur(numberValue);Committable suggestion skipped: line range outside the PR's diff.
| import type { DataView } from '@/types/api'; | ||
|
|
||
| type DataViewRenderProperties = { | ||
| view: DataView; |
There was a problem hiding this comment.
Rename the imported DataView type to avoid shadowing the global.
Biome raises lint/suspicious/noShadowRestrictedNames here, and the lint step will fail until we stop shadowing the built-in DataView (ArrayBuffer view). Alias the imported type and update its usages.
-import type { DataView } from '@/types/api';
+import type { DataView as DataViewModel } from '@/types/api';
type DataViewRenderProperties = {
- view: DataView;
+ view: DataViewModel;
};📝 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.
| import type { DataView } from '@/types/api'; | |
| type DataViewRenderProperties = { | |
| view: DataView; | |
| import type { DataView as DataViewModel } from '@/types/api'; | |
| type DataViewRenderProperties = { | |
| view: DataViewModel; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 7-7: Do not shadow the global "DataView" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
In src/components/organisms/data-view-render.tsx around lines 7 to 10, the
imported type DataView shadows the global DataView (ArrayBuffer view); rename
the imported symbol (e.g., import type { DataView as ApiDataView } from
'@/types/api') and update all local type usages (for example
DataViewRenderProperties.view: ApiDataView) accordingly to remove the name
collision and satisfy the linter.
| export function findIndexAndValue<T>( | ||
| array: T[], | ||
| predicate: (value: T) => boolean | ||
| ): { index: number; value: T } | undefined { | ||
| const index = array.findIndex((value) => predicate(value)); | ||
| if (index === -1) { | ||
| return undefined; | ||
| } | ||
| return { index, value: array[index]! }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the function is used and doesn't duplicate existing utilities.
The AI summary notes that no explicit usage was found in the PR. Please ensure this utility is actually needed and verify it doesn't duplicate existing functionality in the codebase.
Run the following script to check usage and potential duplication:
🏁 Script executed:
#!/bin/bash
# Check for usage of findIndexAndValue in the codebase
echo "=== Usage of findIndexAndValue ==="
rg -n --type=ts --type=tsx 'findIndexAndValue' -g '!src/lib/utils/find-index-and-value.ts'
echo -e "\n=== Check for similar utilities ==="
# Look for similar patterns that combine index + value finding
rg -n --type=ts --type=tsx -C3 'findIndex.*find\(' -g '!node_modules/**'Length of output: 127
---
🏁 Script executed:
```shell
#!/bin/bash
# Check for usage of findIndexAndValue in the codebase
echo "=== Usage of findIndexAndValue ==="
rg -n 'findIndexAndValue' -g '!src/lib/utils/find-index-and-value.ts'
echo -e "\n=== Check imports/exports in utils ==="
fd -e ts -e tsx src/lib/utils | head -20
echo -e "\n=== Check for similar patterns (findIndex combined with value retrieval) ==="
rg -n -C2 'findIndex\s*\(' src/lib/utils --type ts 2>/dev/null || rg -n -C2 'findIndex\s*\(' src/lib/utils -g '*.ts' -g '*.tsx'
Length of output: 896
Remove unused utility or clarify its purpose.
Verification confirms findIndexAndValue has zero usage in the codebase outside its definition file and doesn't duplicate existing utilities. However, this indicates the function is dead code. Either remove it or clarify if it's intended as a public API; unused code should not be added.
🤖 Prompt for AI Agents
In src/lib/utils/find-index-and-value.ts lines 1-10, the findIndexAndValue
utility is dead code (no usages); either delete the file and remove any exports
or barrel references (update any index.ts exports) or explicitly document and
publish it as part of the public API: add a JSDoc comment describing intent, add
unit tests that demonstrate expected behavior, and export it from the module
index so it’s discoverable; choose one path (remove OR document+export+test) and
apply the matching changes.
| import { Block } from '@blocknote/core'; | ||
|
|
||
| export const GET_PAGE_BLOCKS_ENDPOINT = '/api/v1/pages/:pageId/blocks'; | ||
| export const GET_PAGE_BLOCKS_ENDPOINT = '/pages/:id/blocks'; |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Simplified endpoint path.
The endpoint path has been simplified from /api/v1/pages/:pageId/blocks to /pages/:id/blocks, and the parameter naming now aligns with the schema (id instead of pageId).
🏁 Script executed:
#!/bin/bash
# Description: Verify endpoint path consistency across the codebase
# Check if other endpoints follow the simplified pattern (without /api/v1)
rg -n --type=ts "ENDPOINT = '/[^/]" src/types/api/endpoints/
# Check for any remaining references to the old path pattern
rg -n --type=ts "api/v1/pages.*blocks" Length of output: 1916
🏁 Script executed:
#!/bin/bash
# Description: Verify get-page-blocks.ts file exports Zod schemas and types
# View the full content of the endpoint file
cat src/types/api/endpoints/get-page-blocks.ts
# Search for any remaining references to old pageId parameter
rg -n "pageId" --type=ts -g "!node_modules" -g "!dist"
# Search for any remaining references to old api/v1/pages endpoint pattern
rg -n "api/v1/pages" --type=ts -g "!node_modules" -g "!dist"Length of output: 4057
🏁 Script executed:
#!/bin/bash
# Description: Find where GET_PAGE_BLOCKS_ENDPOINT is defined and verify file structure
# Search for GET_PAGE_BLOCKS_ENDPOINT definition
rg -n "GET_PAGE_BLOCKS_ENDPOINT" --type=ts -g "!node_modules" -g "!dist"
# List all files in the endpoints directory
fd . src/types/api/endpoints/ --type f
# Check current structure of endpoints directory
ls -la src/types/api/endpoints/ 2>&1 || echo "Directory does not exist"Length of output: 2739
🏁 Script executed:
#!/bin/bash
# Description: Verify get-page-blocks.ts content and check for consistency with set-page-blocks
# View complete get-page-blocks.ts content
cat src/types/api/endpoints/get-page-blocks.ts
# View set-page-blocks.ts to check parameter naming
cat src/types/api/endpoints/set-page-blocks.ts
# Search for any references to the old complete endpoint path
rg -n "pages/:pageId/blocks" --type=ts -g "!node_modules" -g "!dist"Length of output: 1143
Fix endpoint parameter naming inconsistency in set-page-blocks.ts
The get-page-blocks.ts change is correct and properly simplified. However, verification revealed a critical inconsistency: set-page-blocks.ts declares the endpoint path as /pages/:pageId/blocks but its parameter schema expects id. This mismatch must be resolved.
Update set-page-blocks.ts line 5:
- From:
export const SET_PAGE_BLOCKS = '/pages/:pageId/blocks'; - To:
export const SET_PAGE_BLOCKS = '/pages/:id/blocks';
This aligns the endpoint parameter naming with the schema definition and maintains consistency across all page-related endpoints.
🤖 Prompt for AI Agents
In src/types/api/endpoints/set-page-blocks.ts around line 5, fix the endpoint
parameter name to match the schema by changing the exported path from using
:pageId to :id (i.e., update the string so the path uses /pages/:id/blocks),
ensuring it aligns with the parameter schema and other page endpoints.
| export const dataSourceContainerSchema = containerSchema | ||
| .extend({ | ||
| type: z.literal('data-source'), | ||
| columns: z.array(columnSchema), | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify if columns should be optional.
The columns field is defined as required (not .optional()), meaning data sources must always include a columns array. Ensure this aligns with your API behavior—if data sources can exist without columns initially, consider making this field optional or allowing an empty array explicitly.
🏁 Script executed:
#!/bin/bash
# Description: Check if data sources are created without columns initially
# Search for data source creation patterns to see if columns are always provided
rg -n --type=ts -C3 'type.*data-source|data-source.*create'
# Search for any dataSourceContainerSchema usage to verify columns handling
ast-grep --pattern $'dataSourceContainerSchema'Length of output: 8829
Now I have enough context. The key insight is that using .default([]) with arrays is recommended over .optional() because optional is included within it. The current schema has columns as a required array, but the API handler provides a default empty array via JavaScript null-coalescing. Let me check the createDataSourceBodySchema to understand what the input schema expects.
Based on my analysis of the codebase, I can now provide the rewritten review comment.
Make columns field optional or provide a default value.
The columns field is currently required in the schema, but the data source creation handler at src/app/api/v1/data-sources/route.ts:52 defaults it to an empty array when not provided: body.columns?.map((column) => ({ id: randomUUID(), ...column })) ?? [].
To align the schema with the API's actual behavior, either add .optional() to the field or use .default([]). Zod's .default() method is recommended over .optional() for arrays, as it makes the field accept missing values without requiring an explicit .optional() modifier.
🤖 Prompt for AI Agents
In src/types/schemas/entities/container.ts around lines 52 to 56, the
dataSourceContainerSchema currently requires a columns array which contradicts
the API handler that defaults missing columns to an empty array; update the
schema so columns is not required by either adding .optional() or (preferred)
.default([]) to the columns field (e.g., columns:
z.array(columnSchema).default([])) so the schema matches runtime behavior and
accepts missing values.
1 participant
Summary by CodeRabbit
Release Notes