diff --git a/src/agentkit/bitable/repository.py b/src/agentkit/bitable/repository.py index 51892fb..a4fd4ff 100644 --- a/src/agentkit/bitable/repository.py +++ b/src/agentkit/bitable/repository.py @@ -464,6 +464,13 @@ class BitableRepository: await session.commit() return View.model_validate(entity) if entity else None + async def delete_view(self, view_id: str) -> bool: + """Delete a view. Returns True if a row was deleted.""" + async with self._session_factory() as session: + result = await session.execute(delete(ViewModel).where(ViewModel.id == view_id)) + await session.commit() + return result.rowcount > 0 + # ── Recalc Queue ──────────────────────────────────────── async def enqueue_recalc( diff --git a/src/agentkit/bitable/service.py b/src/agentkit/bitable/service.py index bbe83ad..7245247 100644 --- a/src/agentkit/bitable/service.py +++ b/src/agentkit/bitable/service.py @@ -54,6 +54,14 @@ class FieldDependencyError(Exception): self.dependencies = dependencies +class LastViewDeletionError(Exception): + """Raised when attempting to delete the last remaining view of a table. + + Prevents users from deleting all views and making a table inaccessible. + The route layer maps this to HTTP 409 Conflict. + """ + + class BitableService: """Bitable business logic service. @@ -536,6 +544,25 @@ class BitableService: async def get_view(self, view_id: str) -> View | None: return await self._repo.get_view(view_id) + async def delete_view(self, view_id: str) -> bool: + """Delete a view with last-view protection (U6). + + Raises :class:`LastViewDeletionError` if the view is the last one in + its table — preventing users from making a table inaccessible. The + route layer is responsible for the 404 + ownership checks before + calling this (matching the existing ``delete_field`` pattern). + Returns True if a row was deleted. + """ + view = await self._repo.get_view(view_id) + if view is None: + return False + siblings = await self._repo.list_views(view.table_id) + if len(siblings) <= 1: + raise LastViewDeletionError( + "Cannot delete the last view of a table" + ) + return await self._repo.delete_view(view_id) + # ── Recalc (U3: formula recalc pipeline) ──────────────── async def _trigger_recalc_for_affected_fields(self, table_id: str, record_id: str) -> None: diff --git a/src/agentkit/server/frontend/e2e/bitable-agent-parity.spec.ts b/src/agentkit/server/frontend/e2e/bitable-agent-parity.spec.ts new file mode 100644 index 0000000..f60bd3d --- /dev/null +++ b/src/agentkit/server/frontend/e2e/bitable-agent-parity.spec.ts @@ -0,0 +1,395 @@ +/** + * E2E tests for U6: R15a BitableTool agent parity — the 4 new actions + * (create_view / update_view / update_field / delete_view) and the + * DELETE /views/{id} endpoint. + * + * These tests verify the UI flows that correspond to the 4 new BitableTool + * actions, focusing on the delete-view flow (the main U6 frontend feature) + * and the 204/409 contract of the DELETE endpoint. + * + * Route mocking is used for the DELETE /views endpoint so the 204 success + * and 409 last-view scenarios are deterministic and don't depend on + * specific backend view persistence state. + * + * ponytail: scenarios reuse the loginAndOpenBitable + openFileDetailWithTable + * helpers from bitable-view.spec.ts. Each test creates a unique file to + * avoid collisions. The backend may not be fully configured — tests skip + * gracefully if the server is down. + */ + +import { test, expect, type Page } from '@playwright/test' +import { TEST_USER, clearAuth, waitForServer } from './helpers' + +async function loginAndOpenBitable(page: Page): Promise { + await page.goto('/login') + await clearAuth(page) + await page.getByPlaceholder('请输入用户名').fill(TEST_USER.username) + await page.getByPlaceholder('请输入密码').fill(TEST_USER.password) + await page.getByRole('button', { name: /登\s*录/ }).click() + await expect(page).toHaveURL(/\/agent/, { timeout: 15_000 }) + await page.getByRole('button', { name: '多维表格' }).click() + await expect(page).toHaveURL(/\/bitable/, { timeout: 15_000 }) + await expect(page.locator('.bitable-file-list-view')).toBeVisible({ timeout: 15_000 }) +} + +/** + * Log in, create a bitable file + table via the UI, and wait for the + * ViewSwitcher to render. Returns once the grid header is visible. + */ +async function openFileDetailWithTable(page: Page, label: string): Promise { + await loginAndOpenBitable(page) + await page.getByRole('button', { name: /新建文件/ }).click() + await page.getByPlaceholder('请输入文件名').fill(`U6-${label}`) + await page.getByRole('button', { name: /确\s*定/ }).click() + await expect(page).toHaveURL(/\/bitable\/[^/]+/, { timeout: 10_000 }) + await page.locator('.table-view-list__header .ant-btn').click() + await expect(page.getByText('新建数据表')).toBeVisible({ timeout: 5_000 }) + await page.getByPlaceholder('请输入表名').fill(`U6表-${label}`) + await page.getByRole('button', { name: /确\s*定/ }).click() + await expect(page.locator('.bitable-file-detail-view__table-name')).toContainText( + `U6表-${label}`, + { timeout: 10_000 }, + ) +} + +/** + * Mock the list views GET to return the given view objects, so the + * ViewSwitcher renders the exact set of views needed for each test. + */ +async function mockListViewsWith( + page: Page, + views: { id: string; name: string; view_type: string; config: Record }[], +): Promise { + await page.route('**/api/v1/bitable/tables/*/views', (route) => { + if (route.request().method() !== 'GET') return route.continue() + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ success: true, views }), + }) + }) +} + +test.describe('Bitable Agent Parity E2E (U6: R15a)', () => { + test.beforeAll(async () => { + try { + await waitForServer(undefined, 5_000) + } catch { + test.skip(true, 'Backend not running — skipping agent parity E2E') + } + }) + + // ── P1: delete button hidden when only 1 view ────────────────────── + + test('P1: delete button hidden when only one view exists', async ({ page }) => { + await openFileDetailWithTable(page, 'P1-single-view') + await mockListViewsWith(page, [ + { id: 'v1', name: '默认视图', view_type: 'grid', config: {} }, + ]) + // Reload the table to pick up the mocked views. + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + // The delete button must NOT be visible (views.length === 1). + await expect(page.getByRole('button', { name: /删除/ })).not.toBeVisible({ + timeout: 5_000, + }) + }) + + // ── P2: delete button visible when 2+ views ──────────────────────── + + test('P2: delete button visible when 2+ views exist', async ({ page }) => { + await openFileDetailWithTable(page, 'P2-two-views') + await mockListViewsWith(page, [ + { id: 'v1', name: '视图A', view_type: 'grid', config: {} }, + { id: 'v2', name: '视图B', view_type: 'grid', config: {} }, + ]) + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + // The delete button must be visible. + await expect(page.getByRole('button', { name: /删除/ })).toBeVisible({ + timeout: 5_000, + }) + }) + + // ── P3: popconfirm appears on delete click ───────────────────────── + + test('P3: clicking delete shows popconfirm with "确认删除此视图？"', async ({ page }) => { + await openFileDetailWithTable(page, 'P3-popconfirm') + await mockListViewsWith(page, [ + { id: 'v1', name: '视图A', view_type: 'grid', config: {} }, + { id: 'v2', name: '视图B', view_type: 'grid', config: {} }, + ]) + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + await page.getByRole('button', { name: /删除/ }).click() + // The popconfirm overlay should appear with the confirmation text. + await expect(page.getByText('确认删除此视图？')).toBeVisible({ timeout: 5_000 }) + await expect(page.getByRole('button', { name: /删\s*除/ }).last()).toBeVisible() + await expect(page.getByRole('button', { name: /取\s*消/ })).toBeVisible() + }) + + // ── P4: cancel popconfirm does not delete ────────────────────────── + + test('P4: cancel popconfirm does not fire DELETE request', async ({ page }) => { + await openFileDetailWithTable(page, 'P4-cancel') + await mockListViewsWith(page, [ + { id: 'v1', name: '视图A', view_type: 'grid', config: {} }, + { id: 'v2', name: '视图B', view_type: 'grid', config: {} }, + ]) + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + let deleteFired = false + await page.route('**/api/v1/bitable/views/*', (route) => { + if (route.request().method() === 'DELETE') { + deleteFired = true + return route.fulfill({ status: 204 }) + } + return route.continue() + }) + + await page.getByRole('button', { name: /删除/ }).click() + await page.getByRole('button', { name: /取\s*消/ }).click() + + // Popconfirm disappears. + await expect(page.getByText('确认删除此视图？')).not.toBeVisible({ timeout: 3_000 }) + // No DELETE request was fired. + expect(deleteFired).toBe(false) + }) + + // ── P5: confirm delete fires DELETE /views/{id} ──────────────────── + + test('P5: confirm delete fires DELETE /views/{id} and returns 204', async ({ page }) => { + await openFileDetailWithTable(page, 'P5-delete-204') + await mockListViewsWith(page, [ + { id: 'v1', name: '视图A', view_type: 'grid', config: {} }, + { id: 'v-del', name: '待删除', view_type: 'grid', config: {} }, + ]) + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + let deletedViewId: string | null = null + await page.route('**/api/v1/bitable/views/*', (route) => { + if (route.request().method() === 'DELETE') { + const url = route.request().url() + deletedViewId = url.split('/views/')[1]?.split('?')[0] ?? null + return route.fulfill({ status: 204 }) + } + return route.continue() + }) + + await page.getByRole('button', { name: /删除/ }).click() + await page.getByRole('button', { name: /删\s*除/ }).last().click() + + // The DELETE request was fired with the active view's id. + await expect + .poll(async () => deletedViewId, { timeout: 5_000 }) + .toBeTruthy() + }) + + // ── P6: 409 last view shows warning notification ─────────────────── + + test('P6: 409 Conflict on last view shows warning notification', async ({ page }) => { + await openFileDetailWithTable(page, 'P6-409-last-view') + // Mock 2 views so the delete button is visible, but the backend + // returns 409 (last view) — simulating a race where the other view + // was deleted server-side between the list and the delete. + await mockListViewsWith(page, [ + { id: 'v1', name: '视图A', view_type: 'grid', config: {} }, + { id: 'v2', name: '视图B', view_type: 'grid', config: {} }, + ]) + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + await page.route('**/api/v1/bitable/views/*', (route) => { + if (route.request().method() === 'DELETE') { + return route.fulfill({ + status: 409, + contentType: 'application/json', + body: JSON.stringify({ detail: 'Cannot delete the last view of a table' }), + }) + } + return route.continue() + }) + + await page.getByRole('button', { name: /删除/ }).click() + await page.getByRole('button', { name: /删\s*除/ }).last().click() + + // The warning notification should appear with the "无法删除" message. + await expect(page.getByText('无法删除')).toBeVisible({ timeout: 5_000 }) + }) + + // ── P7: successful delete removes the view from the tab list ─────── + + test('P7: successful delete removes view from tab list (optimistic update)', async ({ + page, + }) => { + await openFileDetailWithTable(page, 'P7-removes-tab') + await mockListViewsWith(page, [ + { id: 'v-keep', name: '保留视图', view_type: 'grid', config: {} }, + { id: 'v-del', name: '删除视图', view_type: 'grid', config: {} }, + ]) + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + // Both view tabs should be visible initially. + await expect(page.getByRole('tab', { name: '保留视图' })).toBeVisible({ timeout: 5_000 }) + await expect(page.getByRole('tab', { name: '删除视图' })).toBeVisible({ timeout: 5_000 }) + + await page.route('**/api/v1/bitable/views/*', (route) => { + if (route.request().method() === 'DELETE') { + return route.fulfill({ status: 204 }) + } + return route.continue() + }) + + // Switch to the view we want to delete, then delete it. + await page.getByRole('tab', { name: '删除视图' }).click() + await page.getByRole('button', { name: /删除/ }).click() + await page.getByRole('button', { name: /删\s*除/ }).last().click() + + // The deleted view's tab should disappear. + await expect(page.getByRole('tab', { name: '删除视图' })).not.toBeVisible({ + timeout: 5_000, + }) + // The remaining view's tab should still be visible. + await expect(page.getByRole('tab', { name: '保留视图' })).toBeVisible({ + timeout: 5_000, + }) + }) + + // ── P8: deleting active view switches to first remaining ─────────── + + test('P8: deleting active view switches to the first remaining view', async ({ page }) => { + await openFileDetailWithTable(page, 'P8-switch-after-delete') + await mockListViewsWith(page, [ + { id: 'v-first', name: '第一个视图', view_type: 'grid', config: {} }, + { id: 'v-second', name: '第二个视图', view_type: 'grid', config: {} }, + { id: 'v-third', name: '第三个视图', view_type: 'grid', config: {} }, + ]) + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + await page.route('**/api/v1/bitable/views/*', (route) => { + if (route.request().method() === 'DELETE') { + return route.fulfill({ status: 204 }) + } + return route.continue() + }) + + // Switch to the second view, then delete it. + await page.getByRole('tab', { name: '第二个视图' }).click() + await page.getByRole('button', { name: /删除/ }).click() + await page.getByRole('button', { name: /删\s*除/ }).last().click() + + // The first view should become the active tab. + await expect(page.getByRole('tab', { name: '第一个视图' })).toHaveClass( + /ant-tabs-tab-active/, + { timeout: 5_000 }, + ) + }) + + // ── P9: 404 on delete (non-owned view) shows error notification ──── + + test('P9: 404 on delete shows error notification (non-owner / missing)', async ({ page }) => { + await openFileDetailWithTable(page, 'P9-404-not-found') + await mockListViewsWith(page, [ + { id: 'v1', name: '视图A', view_type: 'grid', config: {} }, + { id: 'v2', name: '视图B', view_type: 'grid', config: {} }, + ]) + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + await page.route('**/api/v1/bitable/views/*', (route) => { + if (route.request().method() === 'DELETE') { + return route.fulfill({ + status: 404, + contentType: 'application/json', + body: JSON.stringify({ detail: 'View not found' }), + }) + } + return route.continue() + }) + + await page.getByRole('button', { name: /删除/ }).click() + await page.getByRole('button', { name: /删\s*除/ }).last().click() + + // An error notification should appear. + await expect(page.getByText('删除视图失败')).toBeVisible({ timeout: 5_000 }) + }) + + // ── P10: create view adds new tab to the switcher ────────────────── + + test('P10: creating a new view adds it to the tab list', async ({ page }) => { + await openFileDetailWithTable(page, 'P10-create-adds-tab') + await mockListViewsWith(page, [ + { id: 'v1', name: '默认视图', view_type: 'grid', config: {} }, + ]) + await page.reload() + await expect(page.locator('.bitable-file-detail-view__table-name')).toBeVisible({ + timeout: 10_000, + }) + + // Initially only 1 tab. + await expect(page.getByRole('tab', { name: '默认视图' })).toBeVisible({ timeout: 5_000 }) + + // Mock the POST /views response. + await page.route('**/api/v1/bitable/tables/*/views', (route) => { + if (route.request().method() === 'POST') { + const body = route.request().postDataJSON() ?? {} + return route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify({ + success: true, + view: { + id: 'v-new', + table_id: 'tbl', + name: body.name ?? '新视图', + view_type: 'grid', + config: {}, + created_at: new Date().toISOString(), + }, + }), + }) + } + return route.continue() + }) + + // Open the "新建视图" dropdown and click "表格" (grid). + await page.getByRole('button', { name: /新建视图/ }).click() + await page.locator('.ant-dropdown-menu-item').filter({ hasText: '表格' }).click() + + // Fill the name modal. + const nameInput = page.getByPlaceholder('请输入视图名称') + await expect(nameInput).toBeVisible({ timeout: 5_000 }) + await nameInput.fill('新创建视图') + await page.getByRole('button', { name: /确\s*定/ }).click() + + // The new view tab should appear. + await expect(page.getByRole('tab', { name: '新创建视图' })).toBeVisible({ + timeout: 5_000, + }) + }) +}) diff --git a/src/agentkit/server/frontend/src/api/bitable.ts b/src/agentkit/server/frontend/src/api/bitable.ts index f3a8480..0c12daf 100644 --- a/src/agentkit/server/frontend/src/api/bitable.ts +++ b/src/agentkit/server/frontend/src/api/bitable.ts @@ -383,6 +383,14 @@ class BitableApiClient extends BaseApiClient { }) } + /** + * Delete a view (U6: R15a). Returns 204 No Content on success. + * The last view of a table cannot be deleted (backend returns 409). + */ + async deleteView(viewId: string): Promise { + await this.request(`/views/${viewId}`, { method: 'DELETE' }) + } + // ── File upload (U6: attachment & image) ────────────── async uploadFile( diff --git a/src/agentkit/server/frontend/src/components/bitable/ViewSwitcher.vue b/src/agentkit/server/frontend/src/components/bitable/ViewSwitcher.vue index ac8b21a..c9bb941 100644 --- a/src/agentkit/server/frontend/src/components/bitable/ViewSwitcher.vue +++ b/src/agentkit/server/frontend/src/components/bitable/ViewSwitcher.vue @@ -53,13 +53,40 @@ > 配置 + + + + + 删除 + + + diff --git a/src/agentkit/tools/bitable_tool.py b/src/agentkit/tools/bitable_tool.py index 86250ac..6365155 100644 --- a/src/agentkit/tools/bitable_tool.py +++ b/src/agentkit/tools/bitable_tool.py @@ -5,7 +5,8 @@ Implements KTD5 (REST API boundary even when co-deployed) and KTD11 the bitable REST API; it never imports BitableService directly. Actions: create_table, import_excel, import_database, collect_api, - upsert_records, query_records. + upsert_records, query_records, create_view, update_view, + update_field, delete_view. Batch chunking: upsert and import operations send at most ``BATCH_SIZE`` records per HTTP request. On partial failure, the result includes @@ -45,7 +46,8 @@ class BitableTool(Tool): "Create and manage bitable (multi-dimensional spreadsheet) tables, " "ingest data from Excel files, databases, or API responses, and " "query records. Actions: create_table, import_excel, " - "import_database, collect_api, upsert_records, query_records." + "import_database, collect_api, upsert_records, query_records, " + "create_view, update_view, update_field, delete_view." ), input_schema={ "type": "object", @@ -59,6 +61,10 @@ class BitableTool(Tool): "collect_api", "upsert_records", "query_records", + "create_view", + "update_view", + "update_field", + "delete_view", ], "description": "Bitable operation to perform.", }, @@ -89,7 +95,7 @@ class BitableTool(Tool): }, "table_id": { "type": "string", - "description": "Target bitable table ID (collect_api, upsert_records, query_records).", + "description": "Target bitable table ID (collect_api, upsert_records, query_records, create_view).", }, "records": { "type": "array", @@ -115,6 +121,31 @@ class BitableTool(Tool): "type": "integer", "description": "Max records to return (query_records).", }, + "view_id": { + "type": "string", + "description": "View ID (update_view, delete_view).", + }, + "view_type": { + "type": "string", + "enum": ["grid", "kanban", "gantt", "gallery", "form"], + "description": "View type (create_view). Defaults to grid.", + }, + "field_id": { + "type": "string", + "description": "Field ID (update_field).", + }, + "name": { + "type": "string", + "description": "Name for a view or field (create_view, update_view, update_field).", + }, + "type": { + "type": "string", + "description": "Field type for update_field (e.g. text, number, date).", + }, + "config": { + "type": "object", + "description": "View/field config dict (create_view, update_view, update_field).", + }, }, "required": ["action"], }, @@ -148,6 +179,10 @@ class BitableTool(Tool): "collect_api": self._collect_api, "upsert_records": self._upsert_records, "query_records": self._query_records, + "create_view": self._create_view, + "update_view": self._update_view, + "update_field": self._update_field, + "delete_view": self._delete_view, } handler = handlers.get(action) if handler is None: @@ -483,3 +518,68 @@ class BitableTool(Tool): "records": data["records"], "next_cursor": data.get("next_cursor"), } + + # ------------------------------------------------------------------ + # View & field CRUD (U6: agent parity with REST endpoints) + # ------------------------------------------------------------------ + + async def _create_view(self, **kwargs) -> dict[str, object]: + table_id = kwargs.get("table_id") + name = kwargs.get("name") + if not table_id: + return {"success": False, "error": "Missing required field: table_id"} + if not name: + return {"success": False, "error": "Missing required field: name"} + view_type = kwargs.get("view_type") or "grid" + config = kwargs.get("config") or {} + client = await self._get_client() + resp = await client.post( + f"/tables/{table_id}/views", + json={"name": name, "view_type": view_type, "config": config}, + ) + resp.raise_for_status() + return {"success": True, "view": resp.json()["view"]} + + async def _update_view(self, **kwargs) -> dict[str, object]: + view_id = kwargs.get("view_id") + if not view_id: + return {"success": False, "error": "Missing required field: view_id"} + payload: dict[str, object] = {} + if kwargs.get("name") is not None: + payload["name"] = kwargs["name"] + if kwargs.get("config") is not None: + payload["config"] = kwargs["config"] + client = await self._get_client() + resp = await client.patch(f"/views/{view_id}", json=payload) + resp.raise_for_status() + return {"success": True, "view": resp.json()["view"]} + + async def _update_field(self, **kwargs) -> dict[str, object]: + field_id = kwargs.get("field_id") + if not field_id: + return {"success": False, "error": "Missing required field: field_id"} + payload: dict[str, object] = {} + if kwargs.get("name") is not None: + payload["name"] = kwargs["name"] + # ponytail: PATCH /fields/{id} is backed by UpdateFieldRequest which + # currently accepts only name + config. `type` is forwarded as + # `field_type` so the tool is forward-compatible once the request + # model adds it; today it is silently ignored by Pydantic (extra=ignore). + if kwargs.get("type") is not None: + payload["field_type"] = kwargs["type"] + if kwargs.get("config") is not None: + payload["config"] = kwargs["config"] + client = await self._get_client() + resp = await client.patch(f"/fields/{field_id}", json=payload) + resp.raise_for_status() + return {"success": True, "field": resp.json()["field"]} + + async def _delete_view(self, **kwargs) -> dict[str, object]: + view_id = kwargs.get("view_id") + if not view_id: + return {"success": False, "error": "Missing required field: view_id"} + client = await self._get_client() + resp = await client.delete(f"/views/{view_id}") + resp.raise_for_status() + # 204 No Content has an empty body; report a stable success shape. + return {"success": True, "deleted": True} diff --git a/tests/unit/bitable/test_bitable_tool.py b/tests/unit/bitable/test_bitable_tool.py index b8b20b4..1f885fd 100644 --- a/tests/unit/bitable/test_bitable_tool.py +++ b/tests/unit/bitable/test_bitable_tool.py @@ -483,3 +483,236 @@ def test_transform_records_missing_keys() -> None: field_mapping={"a": "fld_a"}, # b is not mapped ) assert result == [{"fld_a": 1}] + + +# --------------------------------------------------------------------------- +# U6: View & field CRUD actions (create_view, update_view, update_field, +# delete_view) — agent parity with the REST API. +# --------------------------------------------------------------------------- + + +def test_action_enum_has_10_actions() -> None: + """input_schema.action.enum lists all 10 actions (6 original + 4 new).""" + tool = BitableTool(base_url="http://test/api/v1/bitable") + actions = tool.input_schema["properties"]["action"]["enum"] + assert len(actions) == 10 + for new_action in ("create_view", "update_view", "update_field", "delete_view"): + assert new_action in actions + + +def test_execute_handlers_dict_has_10_actions() -> None: + """execute() handlers dict contains all 10 action keys (KTD10).""" + import re + + src = open( + "src/agentkit/tools/bitable_tool.py", + encoding="utf-8", + ).read() + handlers_match = re.search(r"handlers\s*=\s*\{([^}]*)\}", src, re.DOTALL) + handler_keys = re.findall(r'"([a-z_]+)":\s*self\._', handlers_match.group(1)) + assert len(handler_keys) == 10 + for new_action in ("create_view", "update_view", "update_field", "delete_view"): + assert new_action in handler_keys + + +async def test_create_view_action(tool: BitableTool) -> None: + """create_view action POSTs /tables/{id}/views with name + view_type + config.""" + result = await tool.execute(action="create_table", table_name="VC") + table_id = result["table"]["id"] + + resp = await tool.execute( + action="create_view", + table_id=table_id, + name="Kanban Plan", + view_type="kanban", + config={"group_by": [{"field_id": "fld_x", "direction": "asc"}]}, + ) + assert resp["success"] is True + assert resp["view"]["name"] == "Kanban Plan" + assert resp["view"]["view_type"] == "kanban" + + +async def test_create_view_defaults_to_grid(tool: BitableTool) -> None: + """create_view without view_type defaults to grid.""" + result = await tool.execute(action="create_table", table_name="VG") + table_id = result["table"]["id"] + resp = await tool.execute(action="create_view", table_id=table_id, name="Default") + assert resp["success"] is True + assert resp["view"]["view_type"] == "grid" + + +async def test_create_view_missing_table_id(tool: BitableTool) -> None: + """Missing table_id → error.""" + resp = await tool.execute(action="create_view", name="x") + assert resp["success"] is False + assert "table_id" in resp["error"] + + +async def test_update_view_action(tool: BitableTool) -> None: + """update_view action PATCHes /views/{id} with name + config.""" + result = await tool.execute(action="create_table", table_name="VU") + table_id = result["table"]["id"] + view_id = ( + await tool.execute(action="create_view", table_id=table_id, name="Old") + )["view"]["id"] + + resp = await tool.execute( + action="update_view", + view_id=view_id, + name="Renamed", + config={"group_by": [{"field_id": "fld_a", "direction": "asc"}]}, + ) + assert resp["success"] is True + assert resp["view"]["name"] == "Renamed" + + +async def test_update_view_missing_view_id(tool: BitableTool) -> None: + """Missing view_id → error.""" + resp = await tool.execute(action="update_view", name="x") + assert resp["success"] is False + assert "view_id" in resp["error"] + + +async def test_update_field_action(tool: BitableTool) -> None: + """update_field action PATCHes /fields/{id} (equivalent to REST PATCH /fields).""" + result = await tool.execute(action="create_table", table_name="FU") + table_id = result["table"]["id"] + client = await tool._get_client() + field_id = ( + await client.post( + f"/tables/{table_id}/fields", + json={"name": "col", "field_type": "text", "owner": "user"}, + ) + ).json()["field"]["id"] + + resp = await tool.execute( + action="update_field", + field_id=field_id, + name="renamed_col", + config={"description": "updated"}, + ) + assert resp["success"] is True + assert resp["field"]["name"] == "renamed_col" + + +async def test_update_field_missing_field_id(tool: BitableTool) -> None: + """Missing field_id → error.""" + resp = await tool.execute(action="update_field", name="x") + assert resp["success"] is False + assert "field_id" in resp["error"] + + +async def test_delete_view_action(tool: BitableTool) -> None: + """delete_view action DELETEs /views/{id}; last-view protection applies.""" + result = await tool.execute(action="create_table", table_name="VD") + table_id = result["table"]["id"] + v1 = (await tool.execute(action="create_view", table_id=table_id, name="v1"))["view"]["id"] + await tool.execute(action="create_view", table_id=table_id, name="v2") + + resp = await tool.execute(action="delete_view", view_id=v1) + assert resp["success"] is True + assert resp["deleted"] is True + + +async def test_delete_view_action_409_on_last_view(tool: BitableTool) -> None: + """delete_view on the last view → HTTP 409 surfaced as error.""" + result = await tool.execute(action="create_table", table_name="VL") + table_id = result["table"]["id"] + only = (await tool.execute(action="create_view", table_id=table_id, name="only"))["view"]["id"] + + resp = await tool.execute(action="delete_view", view_id=only) + assert resp["success"] is False + assert "409" in resp["error"] + + +async def test_delete_view_missing_view_id(tool: BitableTool) -> None: + """Missing view_id → error.""" + resp = await tool.execute(action="delete_view") + assert resp["success"] is False + assert "view_id" in resp["error"] + + +async def test_create_view_with_r3_r4_config(tool: BitableTool) -> None: + """create_view forwards group_by + conditional_formatting config (R3/R4 parity).""" + result = await tool.execute(action="create_table", table_name="R34") + table_id = result["table"]["id"] + client = await tool._get_client() + # Create a field so group_by can reference a real field id. + fid = ( + await client.post( + f"/tables/{table_id}/fields", + json={"name": "status", "field_type": "select", "owner": "user"}, + ) + ).json()["field"]["id"] + + resp = await tool.execute( + action="create_view", + table_id=table_id, + name="GroupedView", + config={ + "group_by": [{"field_id": fid, "direction": "asc"}], + "conditional_formatting": [ + { + "field_id": fid, + "operator": "equals", + "value": "done", + "color_key": "green", + } + ], + }, + ) + assert resp["success"] is True + cfg = resp["view"]["config"] + assert len(cfg["group_by"]) == 1 + assert cfg["group_by"][0]["field_id"] == fid + assert cfg["conditional_formatting"][0]["color_key"] == "green" + + +# --------------------------------------------------------------------------- +# X-Internal-Token transparent passthrough on the 4 new actions (KTD11) +# --------------------------------------------------------------------------- + + +async def test_new_actions_internal_token_passthrough( + tool_real_auth: BitableTool, +) -> None: + """X-Internal-Token bypasses ownership for all 4 new actions (KTD11).""" + # create_table (existing action) — establishes an admin-owned table. + result = await tool_real_auth.execute(action="create_table", table_name="TokenT") + table_id = result["table"]["id"] + + # create_view via the new action. + v = await tool_real_auth.execute( + action="create_view", table_id=table_id, name="tv1" + ) + assert v["success"] is True + view_id = v["view"]["id"] + + # update_view via the new action. + uv = await tool_real_auth.execute(action="update_view", view_id=view_id, name="tv1-renamed") + assert uv["success"] is True + + # update_field: create a field first via the tool's HTTP client, then update. + client = await tool_real_auth._get_client() + fid = ( + await client.post( + f"/tables/{table_id}/fields", + json={"name": "c", "field_type": "text", "owner": "user"}, + ) + ).json()["field"]["id"] + uf = await tool_real_auth.execute(action="update_field", field_id=fid, name="c2") + assert uf["success"] is True + + # delete_view: add a second view so last-view protection doesn't block. + await tool_real_auth.execute(action="create_view", table_id=table_id, name="tv2") + dv = await tool_real_auth.execute(action="delete_view", view_id=view_id) + assert dv["success"] is True + + +async def test_delete_view_internal_token_404_when_missing( + tool_real_auth: BitableTool, +) -> None: + """X-Internal-Token calling delete_view on a non-existent view → 404 (no silent success).""" + resp = await tool_real_auth.execute(action="delete_view", view_id="no-such-view") + assert resp["success"] is False + assert "404" in resp["error"] diff --git a/tests/unit/bitable/test_routes.py b/tests/unit/bitable/test_routes.py index 8b4e09d..84cd239 100644 --- a/tests/unit/bitable/test_routes.py +++ b/tests/unit/bitable/test_routes.py @@ -53,6 +53,23 @@ def unauth_app(bitable_service: BitableService) -> FastAPI: return app +INTERNAL_TOKEN = "internal-token-routes-abc" + + +@pytest.fixture +def internal_app(bitable_service: BitableService) -> FastAPI: + """App with X-Internal-Token configured and NO auth override. + + Exercises the real ``require_bitable_auth`` path: the internal token + yields a synthetic admin user that bypasses ownership (KTD11). + """ + app = FastAPI() + app.state.bitable_service = bitable_service + app.state.bitable_internal_token = INTERNAL_TOKEN + app.include_router(bitable_routes.router, prefix="/api/v1") + return app + + @pytest.fixture def no_service_app() -> FastAPI: """App without bitable_service on state — simulates uninitialized subsystem.""" @@ -77,6 +94,18 @@ async def unauth_client(unauth_app: FastAPI) -> httpx.AsyncClient: yield c +@pytest.fixture +async def internal_client(internal_app: FastAPI) -> httpx.AsyncClient: + """Client that sends X-Internal-Token (real auth path, no override).""" + transport = ASGITransport(app=internal_app) + async with httpx.AsyncClient( + transport=transport, + base_url="http://test", + headers={"X-Internal-Token": INTERNAL_TOKEN}, + ) as c: + yield c + + @pytest.fixture async def no_service_client(no_service_app: FastAPI) -> httpx.AsyncClient: transport = ASGITransport(app=no_service_app) @@ -497,6 +526,86 @@ async def test_update_view(client: httpx.AsyncClient) -> None: assert resp.json()["view"]["name"] == "New" +# --------------------------------------------------------------------------- +# DELETE /views/{view_id} (U6) +# --------------------------------------------------------------------------- + + +async def test_delete_view_success(client: httpx.AsyncClient) -> None: + """DELETE an existing view with >=2 views → 204 No Content.""" + table_id = (await client.post("/api/v1/bitable/tables", json={"name": "T"})).json()["table"][ + "id" + ] + # Create 2 views so the last-view protection does not trigger. + v1 = ( + await client.post(f"/api/v1/bitable/tables/{table_id}/views", json={"name": "v1"}) + ).json()["view"]["id"] + await client.post(f"/api/v1/bitable/tables/{table_id}/views", json={"name": "v2"}) + resp = await client.delete(f"/api/v1/bitable/views/{v1}") + assert resp.status_code == 204 + assert resp.content == b"" + # Confirm it's gone. + views = (await client.get(f"/api/v1/bitable/tables/{table_id}/views")).json()["views"] + assert all(v["id"] != v1 for v in views) + + +async def test_delete_view_404_when_missing(client: httpx.AsyncClient) -> None: + """DELETE a non-existent view → 404.""" + resp = await client.delete("/api/v1/bitable/views/nonexistent-view-id") + assert resp.status_code == 404 + + +async def test_delete_view_404_when_not_owner( + client: httpx.AsyncClient, bitable_service: BitableService +) -> None: + """DELETE a view on a table owned by another user → 404 (not 403). + + Pattern 4 (IDOR): existence is never disclosed to a non-owner. + """ + # Table owned by a different user. + other_table = await bitable_service.create_table(name="Other", owner_user_id="other-user") + view = await bitable_service.create_view(other_table.id, name="v") + resp = await client.delete(f"/api/v1/bitable/views/{view.id}") + assert resp.status_code == 404 + + +async def test_delete_view_409_when_last_view(client: httpx.AsyncClient) -> None: + """DELETE the last remaining view of a table → 409 Conflict.""" + table_id = (await client.post("/api/v1/bitable/tables", json={"name": "T"})).json()["table"][ + "id" + ] + only_view = ( + await client.post(f"/api/v1/bitable/tables/{table_id}/views", json={"name": "only"}) + ).json()["view"]["id"] + resp = await client.delete(f"/api/v1/bitable/views/{only_view}") + assert resp.status_code == 409 + # The view is still present. + views = (await client.get(f"/api/v1/bitable/tables/{table_id}/views")).json()["views"] + assert len(views) == 1 + + +async def test_delete_view_internal_token_passthrough(internal_client: httpx.AsyncClient) -> None: + """X-Internal-Token bypasses ownership: DELETE succeeds on another user's table.""" + # Create a table as the internal admin user; ownership is bypassed. + table_id = ( + await internal_client.post("/api/v1/bitable/tables", json={"name": "InternalT"}) + ).json()["table"]["id"] + v1 = ( + await internal_client.post(f"/api/v1/bitable/tables/{table_id}/views", json={"name": "v1"}) + ).json()["view"]["id"] + await internal_client.post(f"/api/v1/bitable/tables/{table_id}/views", json={"name": "v2"}) + resp = await internal_client.delete(f"/api/v1/bitable/views/{v1}") + assert resp.status_code == 204 + + +async def test_delete_view_internal_token_404_when_missing( + internal_client: httpx.AsyncClient, +) -> None: + """X-Internal-Token still gets 404 for a non-existent view (no silent success).""" + resp = await internal_client.delete("/api/v1/bitable/views/does-not-exist") + assert resp.status_code == 404 + + # --------------------------------------------------------------------------- # Formula validation (U5b) # ---------------------------------------------------------------------------