Skip to content

Commit d8fd815

Browse files
authored
refactor(dotcom): remove the groups_backend flag from production (tldraw#9418)
This is the next step in retiring the workspaces feature flags. It removes `groups_backend` from production entirely — the logic that read it, the value written to new users, and the now-dead migration tooling. `groups_backend` is fully rolled out (every user is migrated and born with a home workspace), so the legacy (`file_state`/`ownerId`-based) branches it gated are dead, and new users no longer need it. Follows tldraw#9405 and tldraw#9411 (the `groups_frontend` removal), both merged. Relates to tldraw#8859. **Preview deploy:** https://pr-9418-preview-deploy.tldraw.com/ — sign in and exercise file/workspace flows (create, pin, open, remove, move, accept invite) and the admin page. No flag setup needed; every account is migrated. ### What changed - Removed `isWorkspacesMigrated()` and collapsed every branch on it to the migrated path: `mutators.ts` (`createFile`, `pinFile`/`unpinFile`, `removeFileFromWorkspace`, `onEnterFile`) and `TldrawApp.ts` (`getMyFiles`, `canCreateNewFile`, `isPinned`, `getFile`). - Dropped the `assertUserHasFlag('groups_backend')` gates on the workspace mutators. - Removed the admin migrate-to-groups tooling (`/app/admin/user/migrate` route + the `admin_migrateToGroups` durable-object method) and the `acceptInvite` migration check. - Stopped writing `groups_backend` at the three user-creation sites; new users get `flags: ''`. - Removed the redundant e2e groups-migration helpers (`migrateUser*`, `isUserMigrated*`, `ensureGroupsReadyByEmail`) and their call sites. Test users are born migrated via signup, so these were no-ops; the `ensureGroupsReady` scenario helper keeps its navigation/sidebar setup, only its migration call was dropped. - Demoted `TlaFlags` to a `string` stub, keeping `parseFlags`/`userHasFlag`/`hasFlag` so the next feature flag has scaffolding. ### What's kept (deliberately) - The `flags` column, the `TlaFlags` string stub, and `parseFlags`/`userHasFlag`/`hasFlag` — retained so the next feature flag has scaffolding (no schema migration needed). - The `migrate_user_to_groups` SQL function. It lives in an applied (immutable) migration and is now uncalled, so it stays as frozen history. - Dropping the `flags` column itself (a Postgres + Zero schema change) is intentionally out of scope — we're keeping the column for future flags. ### Decisions worth a reviewer's eye - **Access control is intentionally untouched.** `assertUserCanAccessFile` / `canUpdateFile` branch on the *file's* shape (`ownerId` vs `owningGroupId`), not on the flag, so they're kept as defensive fallbacks for any legacy `ownerId` files. We stop *producing* legacy files but keep *tolerating* them. - **New users get `flags: ''`.** Existing users' stored `groups_backend` value is harmless — nothing reads it after this PR. - **`assertNotMaxFiles` removed.** This server-side `createFile` limit only ran on the legacy path and was already inert for migrated users; the client-side limit in `canCreateNewFile` remains. If a server-enforced per-workspace cap is wanted, that's a separate feature. ### Change type - [x] `other` (internal refactor, no user-facing change) ### Test plan 1. On the preview deploy, sign in and confirm normal file/workspace flows work: create, pin/unpin, open, remove, move between workspaces, accept an invite. 2. Confirm the admin page loads with the "migrate user" action gone (other sections intact). - [x] Unit tests - [x] End to end tests ### Code changes | Section | LOC change | | --------- | ---------- | | Core code | +45 / -171 | | Tests | +6 / -81 | | Apps | +22 / -174 |
1 parent 83f51f6 commit d8fd815

11 files changed

Lines changed: 126 additions & 479 deletions

File tree

apps/dotcom/client/e2e/fixtures/Database.ts

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import fs from 'fs'
22
import { Page } from '@playwright/test'
3-
import { DB, userHasFlag } from '@tldraw/dotcom-shared'
3+
import { DB } from '@tldraw/dotcom-shared'
44
import { Kysely, PostgresDialect, sql } from 'kysely'
55
import pg from 'pg'
66
import { OTHER_USERS, USERS } from '../consts'
@@ -45,46 +45,6 @@ export class Database {
4545
return dbUser.rows[0].id
4646
}
4747

48-
/**
49-
* Check if a user is migrated to the groups model
50-
*/
51-
async isUserMigrated(isOther: boolean = false): Promise<boolean> {
52-
return await this.isUserMigratedByEmail(this.getEmail(isOther))
53-
}
54-
55-
async isUserMigratedByEmail(email: string): Promise<boolean> {
56-
const id = await this.getUserIdByEmail(email)
57-
if (!id) return false
58-
59-
const result = await sql<{
60-
flags: string | null
61-
}>`SELECT flags FROM public.user WHERE id = ${id}`.execute(db)
62-
63-
return userHasFlag(result.rows[0]?.flags, 'groups_backend')
64-
}
65-
66-
/**
67-
* Migrate a user to the groups model
68-
*/
69-
async migrateUser(isOther: boolean = false): Promise<void> {
70-
await this.migrateUserByEmail(this.getEmail(isOther))
71-
}
72-
73-
async migrateUserByEmail(email: string): Promise<void> {
74-
const id = await this.getUserIdByEmail(email)
75-
if (!id) throw new Error(`User not found: ${email}`)
76-
if (await this.isUserMigratedByEmail(email)) return
77-
78-
const inviteSecret = 'test' + Math.random().toString(36).substring(2, 15)
79-
80-
// Call the migration function
81-
await sql`SELECT * FROM migrate_user_to_groups(${id}, ${inviteSecret})`.execute(db)
82-
}
83-
84-
async ensureGroupsReadyByEmail(email: string): Promise<void> {
85-
await this.migrateUserByEmail(email)
86-
}
87-
8848
private async cleanUpUser(isOther: boolean) {
8949
const fileName = getStorageStateFileName(this.parallelIndex, isOther ? 'suppy' : 'huppy')
9050
if (!fs.existsSync(fileName)) return

apps/dotcom/client/e2e/fixtures/scenario-test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -508,8 +508,8 @@ class DotcomScenario {
508508
.slice(0, MAX_WORKSPACE_NAME_LENGTH)
509509
const fileName = opts.fileName ?? this.name('workspace file')
510510

511-
await this.ensureGroupsReady(opts.owner)
512-
await this.ensureGroupsReady(opts.member)
511+
await this.goToAndOpenSidebar(opts.owner)
512+
await this.goToAndOpenSidebar(opts.member)
513513

514514
if (!opts.member.email) throw new Error('Workspace member actor is not signed in')
515515
const memberUserId = await this.database.getUserIdByEmail(opts.member.email)
@@ -596,10 +596,9 @@ class DotcomScenario {
596596
)
597597
}
598598

599-
async ensureGroupsReady(actor: DotcomActor) {
599+
async goToAndOpenSidebar(actor: DotcomActor) {
600600
if (!actor.email) throw new Error(`Actor ${actor.name} is not signed in`)
601601

602-
await this.database.ensureGroupsReadyByEmail(actor.email)
603602
await actor.goto()
604603
await actor.editor.ensureSidebarOpen()
605604
}

apps/dotcom/client/e2e/tests/smoke/workspaces.spec.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ import { expect, test } from '../../fixtures/tla-test'
1010
// otherwise most recent (creating one if it's empty) — and creating a
1111
// workspace switches to it.
1212
test.describe('workspaces', () => {
13-
test.beforeEach(async ({ database, editor }) => {
14-
await database.migrateUser()
13+
test.beforeEach(async ({ editor }) => {
1514
await editor.isLoaded()
1615
await editor.ensureSidebarOpen()
1716
})
@@ -423,7 +422,7 @@ test.describe('workspaces', () => {
423422
await context.grantPermissions(['clipboard-read', 'clipboard-write'])
424423
})
425424

426-
test('invite already member user to workspace', async ({ sidebar, browser, database }) => {
425+
test('invite already member user to workspace', async ({ sidebar, browser }) => {
427426
const workspaceName = getRandomName()
428427
const fileName = getRandomName()
429428
const homeFileName = getRandomName()
@@ -436,9 +435,6 @@ test.describe('workspaces', () => {
436435

437436
const inviteUrl = await sidebar.copyWorkspaceInviteLink(workspaceName)
438437

439-
// Migrate invitee to groups backend but not frontend (tests auto-enable)
440-
await database.migrateUser(true)
441-
442438
const parallelIndex = test.info().parallelIndex
443439
const { newSidebar, newEditor, newWorkspaceInviteDialog, newContext } = await openNewTab(
444440
browser,
@@ -475,7 +471,6 @@ test.describe('workspaces', () => {
475471
test('signing in through the invite dialog completes the workspace join', async ({
476472
sidebar,
477473
browser,
478-
database,
479474
}) => {
480475
const workspaceName = getRandomName()
481476
const fileName = getRandomName()
@@ -489,9 +484,6 @@ test.describe('workspaces', () => {
489484

490485
const inviteUrl = await sidebar.copyWorkspaceInviteLink(workspaceName)
491486

492-
// Migrate invitee to groups backend but not frontend (tests auto-enable)
493-
await database.migrateUser(true)
494-
495487
const parallelIndex = test.info().parallelIndex
496488
const { newPage, newSidebar, newEditor, newWorkspaceInviteDialog, newContext } =
497489
await openNewTab(browser, {

apps/dotcom/client/e2e/tests/ui.scenario.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ test.describe('UI scenarios', () => {
128128
const workspaceName = scenario.name('workspace nav')
129129
const fileName = scenario.name('workspace movable file')
130130

131-
await scenario.ensureGroupsReady(owner)
131+
await scenario.goToAndOpenSidebar(owner)
132132
await scenario.createPersonalFile(owner, fileName)
133133

134134
await owner.sidebar.createWorkspace(workspaceName)
@@ -304,7 +304,7 @@ test.describe('UI scenarios', () => {
304304
// Regression: the home workspace ("My workspace") used to render an empty dialog — the
305305
// component returned null for it while the sidebar still opened the dialog, so the page
306306
// just dimmed with no content. It's private: it can be renamed, but not shared or managed.
307-
await scenario.ensureGroupsReady(owner)
307+
await scenario.goToAndOpenSidebar(owner)
308308
await owner.sidebar.switchToHomeWorkspace()
309309
await owner.page.getByTestId('tla-sidebar-workspace-settings').click()
310310

@@ -332,7 +332,7 @@ test.describe('UI scenarios', () => {
332332
// Regression: the home workspace is renameable (the reason its settings dialog exists at
333333
// all), so the rename must apply. Restore the name afterwards so later serial tests still
334334
// see the original.
335-
await scenario.ensureGroupsReady(owner)
335+
await scenario.goToAndOpenSidebar(owner)
336336
await owner.sidebar.switchToHomeWorkspace()
337337
const originalName = await owner.page.getByTestId('tla-active-workspace-name').innerText()
338338
const newName = scenario.name('home renamed')
@@ -356,7 +356,7 @@ test.describe('UI scenarios', () => {
356356
const workspaceName = scenario.name('settings rename old')
357357
const newWorkspaceName = scenario.name('settings rename new')
358358

359-
await scenario.ensureGroupsReady(owner)
359+
await scenario.goToAndOpenSidebar(owner)
360360
await owner.sidebar.createWorkspace(workspaceName)
361361
await owner.sidebar.renameWorkspace(workspaceName, newWorkspaceName)
362362

apps/dotcom/client/src/tla/app/TldrawApp.ts

Lines changed: 19 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,6 @@ import {
5858
dataUrlToFile,
5959
defaultUserPreferences,
6060
getUserPreferences,
61-
objectMapFromEntries,
62-
objectMapKeys,
6361
parseTldrawJsonFile,
6462
react,
6563
transact,
@@ -413,18 +411,9 @@ export class TldrawApp {
413411
return this.getUserFlags().has(flag)
414412
}
415413

416-
/**
417-
* Check if the user has been migrated to the new workspaces-based data model.
418-
* Users with the 'groups_backend' flag use group_file for access control and pinning.
419-
* Users without the flag use the legacy file_state-based approach.
420-
*/
421-
isWorkspacesMigrated() {
422-
return this.hasFlag('groups_backend')
423-
}
424-
425414
/**
426415
* Get the user's home workspace ID.
427-
* For migrated users, this is used to store shared files and pinned files.
416+
* Used to store shared files and pinned files.
428417
* The home workspace ID is the same as the user ID.
429418
*/
430419
getHomeWorkspaceId() {
@@ -543,12 +532,6 @@ export class TldrawApp {
543532
return this.fileStates$.get()
544533
}
545534

546-
lastRecentFileOrdering = null as null | Array<{
547-
fileId: TlaFile['id']
548-
isPinned: boolean
549-
date: number
550-
}>
551-
552535
// Store stable workspace file ordering for each workspace to prevent jumping when files are edited
553536
lastWorkspaceFileOrderings = new Map<
554537
string,
@@ -560,86 +543,7 @@ export class TldrawApp {
560543

561544
@computed({ isEqual })
562545
getMyFiles() {
563-
if (this.isWorkspacesMigrated()) {
564-
return this.getWorkspaceFilesSorted(this.getHomeWorkspaceId())
565-
}
566-
const myFiles = objectMapFromEntries(this.getUserOwnFiles().map((f) => [f.id, f]))
567-
const myStates = objectMapFromEntries(this.getUserFileStates().map((f) => [f.fileId, f]))
568-
569-
const myFileIds = new Set<string>([...objectMapKeys(myFiles), ...objectMapKeys(myStates)])
570-
const myWorkspaceMemberships = this.getWorkspaceMemberships()
571-
572-
const nextRecentFileOrdering: {
573-
fileId: TlaFile['id']
574-
isPinned: boolean
575-
date: number
576-
}[] = []
577-
578-
for (const fileId of myFileIds) {
579-
const file = myFiles[fileId]
580-
let state: (typeof myStates)[string] | undefined = myStates[fileId]
581-
if (!file) continue
582-
if (file.isDeleted) continue
583-
584-
if (!state && !file.isDeleted && file.ownerId === this.userId) {
585-
state = this.fileStates$.get().find((fs) => fs.fileId === fileId)
586-
}
587-
if (!state) {
588-
// if the file is deleted, we don't want to show it in the recent files
589-
continue
590-
}
591-
592-
// if the file is in a workspace we have access to, we don't want to show it in my workspace
593-
if (myWorkspaceMemberships.some((g) => g.groupFiles.some((gf) => gf.fileId === fileId))) {
594-
continue
595-
}
596-
597-
const existing = this.lastRecentFileOrdering?.find((f) => f.fileId === fileId)
598-
const isPinned = state.isPinned ?? false
599-
600-
if (existing && existing.isPinned === isPinned) {
601-
// Preserve existing entry to maintain ordering
602-
nextRecentFileOrdering.push(existing)
603-
continue
604-
}
605-
606-
// For new entries or pinned status changes
607-
const newEntry = {
608-
fileId,
609-
isPinned,
610-
date: getFileRecencyDate(state, file),
611-
}
612-
613-
// If this was previously unpinned and we have existing ordering,
614-
// preserve its position in the unpinned section to avoid real-time reordering
615-
if (!isPinned && existing && !existing.isPinned) {
616-
// Keep the old date to preserve ordering in "My workspace"
617-
newEntry.date = existing.date
618-
}
619-
620-
nextRecentFileOrdering.push(newEntry)
621-
}
622-
623-
// separate pinned and unpinned files
624-
const pinnedFiles = nextRecentFileOrdering.filter((f) => f.isPinned)
625-
const unpinnedFiles = nextRecentFileOrdering.filter((f) => !f.isPinned)
626-
627-
// sort pinned files by their pinnedIndex
628-
pinnedFiles.sort((a, b) => {
629-
// If neither has an index, sort by date (fallback)
630-
return b.date - a.date
631-
})
632-
633-
// sort unpinned files by date with most recent first
634-
unpinnedFiles.sort((a, b) => b.date - a.date)
635-
636-
// combine: pinned files first, then unpinned
637-
const sortedFiles = [...pinnedFiles, ...unpinnedFiles]
638-
639-
// stash the ordering for next time
640-
this.lastRecentFileOrdering = sortedFiles
641-
642-
return sortedFiles
546+
return this.getWorkspaceFilesSorted(this.getHomeWorkspaceId())
643547
}
644548

645549
/**
@@ -672,23 +576,17 @@ export class TldrawApp {
672576
}
673577

674578
private canCreateNewFile(workspaceId: string) {
675-
if (this.isWorkspacesMigrated()) {
676-
// Count only files the workspace actually owns — not guest files (shared files the
677-
// user opened) or mislinked rows that getWorkspaceFilesSorted hides. Counting those
678-
// would let the limit fire on files the user can neither see nor remove. For migrated
679-
// users createFile runs no server-side file-limit check, so this is the only gate;
680-
// scoping it to owned files keeps it to what the user can actually manage.
681-
const membership = this.getWorkspaceMembership(workspaceId)
682-
if (!membership) return true
683-
const nonDeletedCount = membership.groupFiles.filter(
684-
(gf) => gf.file && !gf.file.isDeleted && gf.file.owningGroupId === workspaceId
685-
).length
686-
return nonDeletedCount < this.config.maxNumberOfFiles
687-
} else {
688-
// For unmigrated users, count non-deleted files owned by the user
689-
const nonDeletedCount = this.getUserOwnFiles().filter((f) => !f.isDeleted).length
690-
return nonDeletedCount < this.config.maxNumberOfFiles
691-
}
579+
// Count only files the workspace actually owns — not guest files (shared files the
580+
// user opened) or mislinked rows that getWorkspaceFilesSorted hides. Counting those
581+
// would let the limit fire on files the user can neither see nor remove. createFile
582+
// runs no server-side file-limit check, so this is the only gate; scoping it to owned
583+
// files keeps it to what the user can actually manage.
584+
const membership = this.getWorkspaceMembership(workspaceId)
585+
if (!membership) return true
586+
const nonDeletedCount = membership.groupFiles.filter(
587+
(gf) => gf.file && !gf.file.isDeleted && gf.file.owningGroupId === workspaceId
588+
).length
589+
return nonDeletedCount < this.config.maxNumberOfFiles
692590
}
693591

694592
private showMaxFilesToast() {
@@ -701,12 +599,7 @@ export class TldrawApp {
701599
}
702600

703601
isPinned(fileId: string, workspaceId: string) {
704-
if (this.isWorkspacesMigrated()) {
705-
return this.getWorkspaceFilesSorted(workspaceId).some(
706-
(f) => f.fileId === fileId && f.isPinned
707-
)
708-
}
709-
return this.getFileState(fileId)?.isPinned ?? false
602+
return this.getWorkspaceFilesSorted(workspaceId).some((f) => f.fileId === fileId && f.isPinned)
710603
}
711604

712605
// A workspace's welcome file is seeded once, when the workspace is created. Its
@@ -902,14 +795,11 @@ export class TldrawApp {
902795

903796
getFile(fileId?: string): TlaFile | null {
904797
if (!fileId) return null
905-
if (this.isWorkspacesMigrated()) {
906-
return (
907-
this.getWorkspaceMemberships()
908-
.find((g) => g.groupFiles.some((gf) => gf.fileId === fileId))
909-
?.groupFiles.find((gf) => gf.fileId === fileId)?.file ?? null
910-
)
911-
}
912-
return this.getUserOwnFiles().find((f) => f.id === fileId) ?? null
798+
return (
799+
this.getWorkspaceMemberships()
800+
.find((g) => g.groupFiles.some((gf) => gf.fileId === fileId))
801+
?.groupFiles.find((gf) => gf.fileId === fileId)?.file ?? null
802+
)
913803
}
914804

915805
canUpdateFile(fileId: string): boolean {

0 commit comments

Comments
 (0)