Skip to content

Commit f3ec1d4

Browse files
committed
refactor(tables): match TableViewProps to the shape the other three units use
Two deviations this migration introduced, found by diffing all four views' prop surfaces against each other rather than reading them one at a time. - The axes did not lead the interface. `source` had replaced `workspaceId` + `tableId` in the slot those occupied at the bottom, so the view declared `host, grants, … source`. Every other unit reads `source, grants, host` first. - Two loose optional feature booleans. The rule says collapse into one object and never add a loose prop, and the optional default was load-bearing in the wrong direction: nobody decided the embedded table has no lock settings — `tableLocksEnabled?: boolean` decided it by omission. `features: TableViewFeatures` is required, so the panel now states that answer in a comment instead of inheriting it from a default. Both fields disappear together when the flags GA, which is the other reason they travel as one object rather than spread. Behaviour is unchanged: the panel passes `locks: false`, which is what the absent prop already resolved to.
1 parent d8cb711 commit f3ec1d4

3 files changed

Lines changed: 62 additions & 47 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,13 @@ export const ResourceContent = memo(function ResourceContent({
323323
onNavigate={navigate}
324324
showExecutionInternals={!permissionConfig.hideTraceSpans}
325325
source={tableSource}
326-
viewsEnabled={tableViewsEnabled}
326+
features={{
327+
// Deliberately false: the home page resolves no `table-locks` flag,
328+
// so the panel has never offered lock settings. Giving it them means
329+
// resolving the flag server-side there — a product decision.
330+
locks: false,
331+
views: tableViewsEnabled ?? false,
332+
}}
327333
/>
328334
)
329335

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table-route.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@ export function TableRoute({ tableLocksEnabled, viewsEnabled }: TableRouteProps)
4747
onNavigate={navigate}
4848
showExecutionInternals={!permissionConfig.hideTraceSpans}
4949
source={source}
50-
tableLocksEnabled={tableLocksEnabled}
51-
viewsEnabled={viewsEnabled}
50+
features={{ locks: tableLocksEnabled, views: viewsEnabled }}
5251
/>
5352
)
5453
}

apps/sim/components/resources/table-view/table-view.tsx

Lines changed: 54 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -101,25 +101,56 @@ const logger = createLogger('Table')
101101
/** Blocked-action toasts carry a button, so they linger past the 5s default. */
102102
const BLOCKED_TOAST_MS = 8000
103103

104+
/**
105+
* The AppConfig gates a table renders behind.
106+
*
107+
* Resolved by each host's Server Component, because `isFeatureEnabled` is
108+
* server-only by construction and the org it keys on is the workspace's host
109+
* organization rather than the viewer's active one.
110+
*
111+
* Required, and required as an object. Two loose optional booleans is exactly
112+
* what the axis rule means by "never add a fourth spelling", and the optional
113+
* default was load-bearing in the wrong direction: nobody decided the embedded
114+
* table has no lock settings — `features.locks?: boolean` decided it by
115+
* omission. A host now states its answer. Both fields disappear together when
116+
* the flags GA, which is the other reason they travel as one object.
117+
*/
118+
export interface TableViewFeatures {
119+
/**
120+
* `table-locks` — whether an admin may *change* locks. Enforcement of locks
121+
* already stored is independent of this and lives on the server.
122+
*/
123+
readonly locks: boolean
124+
/** `table-views` — the saved-views bar (Views/Columns menus, the Save chip). */
125+
readonly views: boolean
126+
}
127+
104128
export interface TableViewProps {
105129
/**
106-
* Which surface this table is mounted on. `'page'` renders the full route
107-
* chrome — header, breadcrumbs, page-level options bar; every other host
108-
* renders the grid alone, as the mothership chat panel does.
130+
* Where the table comes from and by what address.
109131
*
110-
* This replaces an `embedded` boolean so the shell speaks the same vocabulary
111-
* as the canonical views it sits beside. `'public'` is not reachable here:
112-
* the editing shell holds a write path, so an anonymous surface would mount a
113-
* read-only view of this table's view layer rather than this component.
132+
* Built by each host's client shell, never by its Server Component: a source
133+
* carries closures (`hrefFor`, `unavailableCopy`) and cannot cross the RSC
134+
* boundary — the same reason the public share page hands over a plain seed and
135+
* lets the client mint the source.
114136
*/
115-
host: Extract<ResourceHost, 'page' | 'panel'>
137+
source: ResourceSource<'table'>
116138
/**
117139
* What this viewer may do. `write` is `canEdit` exactly; `manage` is the
118140
* admin-only governance capability that gates lock settings; `settled` says
119141
* whether those are final, which the one-shot lock notice must wait for or it
120142
* permanently loses its action.
121143
*/
122144
grants: ResourceGrants
145+
/**
146+
* Which surface this table is mounted on. `'page'` renders the full route
147+
* chrome — header, breadcrumbs, page-level options bar; every other host
148+
* renders the grid alone, as the mothership chat panel does.
149+
*
150+
* `'public'` is not reachable: `ResourceSeedMap['table']` is `never`, so a
151+
* table cannot be addressed by a share token at all.
152+
*/
153+
host: Extract<ResourceHost, 'page' | 'panel'>
123154
/**
124155
* How this host moves the viewer — the router half of `host`. Targets come
125156
* from `source.hrefFor`, so nothing here hand-builds a workspace path; a host
@@ -133,28 +164,8 @@ export interface TableViewProps {
133164
* compile when a host forgets it, not default to revealing payloads.
134165
*/
135166
showExecutionInternals: boolean
136-
/**
137-
* Where the table comes from and by what address.
138-
*
139-
* Built by each host's client shell, never by its Server Component: a source
140-
* carries closures (`hrefFor`, `unavailableCopy`) and cannot cross the RSC
141-
* boundary — the same reason the public share page hands over a plain seed and
142-
* lets the client mint the source.
143-
*/
144-
source: ResourceSource<'table'>
145-
/**
146-
* Whether an admin may CHANGE locks, resolved server-side by the page (the
147-
* flag's gating lives in AppConfig and has no client counterpart). Defaults
148-
* to false so embedded renders, which have no server resolution, fail closed
149-
* — enforcement of stored locks is unaffected either way.
150-
*/
151-
tableLocksEnabled?: boolean
152-
/**
153-
* Resolved `table-views` flag. Server-only to resolve for the same reason.
154-
* Defaults to `false` so the embedded mothership table — which has no server
155-
* context to resolve it — stays on today's Filter/Sort bar.
156-
*/
157-
viewsEnabled?: boolean
167+
/** Server-resolved AppConfig gates. See {@link TableViewFeatures}. */
168+
features: TableViewFeatures
158169
}
159170

160171
/**
@@ -252,13 +263,12 @@ function isSameViewConfig(a: TableViewConfig, b: TableViewConfig): boolean {
252263
* Embedded mode skips the page header but otherwise renders the same surface.
253264
*/
254265
export function TableView({
255-
host,
266+
source,
256267
grants,
268+
host,
257269
onNavigate,
258270
showExecutionInternals,
259-
source,
260-
tableLocksEnabled = false,
261-
viewsEnabled = false,
271+
features,
262272
}: TableViewProps) {
263273
/**
264274
* The subtree below takes plain ids — the grid, the sidebars and the mutation
@@ -430,7 +440,7 @@ export function TableView({
430440
const { data: viewsData, isError: viewsErrored } = useTableViews({
431441
workspaceId,
432442
tableId,
433-
enabled: viewsEnabled,
443+
enabled: features.views,
434444
})
435445
const views = viewsData ?? NO_VIEWS
436446
/** A views list exists — fresh or cached. A failed background refetch flips
@@ -571,7 +581,7 @@ export function TableView({
571581
* view even after someone changes which view is default.
572582
*/
573583
useEffect(() => {
574-
if (!viewsEnabled) return
584+
if (!features.views) return
575585
// Terminal only when the fetch failed WITHOUT ever producing a list — then
576586
// the table settles to All: mark the owner resolved so layout writes flow
577587
// to shared metadata, and flush what was touched during the load. It does
@@ -653,7 +663,7 @@ export function TableView({
653663
}
654664
applyViewConfig(activeView?.config ?? null)
655665
}, [
656-
viewsEnabled,
666+
features.views,
657667
viewsAvailable,
658668
viewsErrored,
659669
views,
@@ -1143,7 +1153,7 @@ export function TableView({
11431153
},
11441154
// Reachable with the flag off when something is locked, so an
11451155
// admin can always clear locks (the route allows clearing).
1146-
...(grants.manage && (tableLocksEnabled || lockedNouns(tableData.locks).length > 0)
1156+
...(grants.manage && (features.locks || lockedNouns(tableData.locks).length > 0)
11471157
? [
11481158
{
11491159
label: 'Lock settings',
@@ -1183,7 +1193,7 @@ export function TableView({
11831193
// a plain notice with no action.
11841194
const canOpenLockSettings =
11851195
grants.manage &&
1186-
(tableLocksEnabled || (tableData ? lockedNouns(tableData.locks).length > 0 : false))
1196+
(features.locks || (tableData ? lockedNouns(tableData.locks).length > 0 : false))
11871197

11881198
/**
11891199
* Explains why a table mutation is unavailable. A toast rather than a modal:
@@ -1350,7 +1360,7 @@ export function TableView({
13501360
) : null
13511361

13521362
const saveViewChip =
1353-
viewsEnabled && isViewDirty && grants.write ? (
1363+
features.views && isViewDirty && grants.write ? (
13541364
<Chip onClick={handleSaveView} disabled={updateViewMutation.isPending}>
13551365
{activeView ? 'Save' : 'Save as view'}
13561366
</Chip>
@@ -1408,7 +1418,7 @@ export function TableView({
14081418
sort={sortConfig}
14091419
filter={filterConfig}
14101420
aside={
1411-
viewsEnabled ? (
1421+
features.views ? (
14121422
<ViewsMenu
14131423
views={views}
14141424
activeViewId={activeView?.id ?? null}
@@ -1421,7 +1431,7 @@ export function TableView({
14211431
) : undefined
14221432
}
14231433
asideEnd={
1424-
viewsEnabled ? (
1434+
features.views ? (
14251435
<ColumnsMenu
14261436
columns={columns}
14271437
workflowGroups={tableWorkflowGroups}
@@ -1441,7 +1451,7 @@ export function TableView({
14411451
/>
14421452
)}
14431453
<SaveViewModal
1444-
open={viewsEnabled && (viewModal?.mode === 'create' || renamingView !== null)}
1454+
open={features.views && (viewModal?.mode === 'create' || renamingView !== null)}
14451455
onOpenChange={(open) => !open && setViewModal(null)}
14461456
mode={viewModal?.mode === 'rename' ? 'rename' : viewModal?.blank ? 'new' : 'create'}
14471457
initialName={renamingView?.name ?? ''}
@@ -1482,7 +1492,7 @@ export function TableView({
14821492
// Always bound while views are enabled: the router reads the owner at
14831493
// call time (buffer / view / All-metadata), so no binding gap can send a
14841494
// write to the wrong place between settle and adoption.
1485-
onPersistLayout={viewsEnabled ? handlePersistLayout : undefined}
1495+
onPersistLayout={features.views ? handlePersistLayout : undefined}
14861496
columnRenameSinkRef={columnRenameSinkRef}
14871497
layoutSnapshotSinkRef={layoutSnapshotRef}
14881498
afterDeleteRowsSinkRef={afterDeleteRowsSinkRef}

0 commit comments

Comments
 (0)