From a3ad8780deb6089bc96d1a66d21b0ed9ebf30c2e Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 19 Oct 2023 17:28:55 +0100 Subject: [PATCH 1/5] Implement many-to-many user column migrations. --- .../server/src/api/routes/tests/table.spec.ts | 95 +++++++++++++++++-- packages/server/src/sdk/app/rows/search.ts | 2 +- .../server/src/sdk/app/tables/migration.ts | 66 ++++++++++++- .../src/tests/utilities/TestConfiguration.ts | 3 +- .../server/src/tests/utilities/api/index.ts | 1 + 5 files changed, 154 insertions(+), 13 deletions(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index dcf1704ed0..907b020246 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -8,6 +8,7 @@ import { AutoFieldSubTypes, InternalTable, FieldSubtype, + Row, } from "@budibase/types" import { checkBuilderEndpoint } from "./utilities/TestFunctions" import * as setup from "./utilities" @@ -421,8 +422,13 @@ describe("/tables", () => { }) describe("migrate", () => { - it("should successfully migrate a user relationship to a user column", async () => { - const users = await config.api.row.fetch(InternalTable.USER_METADATA) + it("should successfully migrate a one-to-many user relationship to a user column", async () => { + const users = await Promise.all([ + config.createUser({ email: "1@example.com" }), + config.createUser({ email: "2@example.com" }), + config.createUser({ email: "3@example.com" }), + ]) + const table = await config.api.table.create({ name: "table", type: "table", @@ -441,9 +447,11 @@ describe("/tables", () => { }, }) - await config.api.row.save(table._id!, { - "user relationship": users, - }) + const rows = await Promise.all( + users.map(u => + config.api.row.save(table._id!, { "user relationship": [u] }) + ) + ) await config.api.table.migrate(table._id!, { oldColumn: table.schema["user relationship"], @@ -458,9 +466,80 @@ describe("/tables", () => { expect(migratedTable.schema["user column"]).toBeDefined() expect(migratedTable.schema["user relationship"]).not.toBeDefined() - const rows = await config.api.row.fetch(table._id!) - expect(rows[0]["user column"]).toBeDefined() - expect(rows[0]["user relationship"]).not.toBeDefined() + const migratedRows = await config.api.row.fetch(table._id!) + + rows.sort((a, b) => a._id!.localeCompare(b._id!)) + migratedRows.sort((a, b) => a._id!.localeCompare(b._id!)) + + for (const [i, row] of rows.entries()) { + const migratedRow = migratedRows[i] + expect(migratedRow["user column"]).toBeDefined() + expect(migratedRow["user relationship"]).not.toBeDefined() + expect(row["user relationship"][0]._id).toEqual( + migratedRow["user column"][0]._id + ) + } + }) + + it("should successfully migrate a many-to-many user relationship to a users column", async () => { + const users = await Promise.all([ + config.createUser({ email: "1@example.com" }), + config.createUser({ email: "2@example.com" }), + config.createUser({ email: "3@example.com" }), + ]) + + const table = await config.api.table.create({ + name: "table", + type: "table", + schema: { + "user relationship": { + type: FieldType.LINK, + fieldName: "test", + name: "user relationship", + constraints: { + type: "array", + presence: false, + }, + relationshipType: RelationshipType.MANY_TO_MANY, + tableId: InternalTable.USER_METADATA, + }, + }, + }) + + const row1 = await config.api.row.save(table._id!, { + "user relationship": [users[0], users[1]], + }) + + const row2 = await config.api.row.save(table._id!, { + "user relationship": [users[2]], + }) + + await config.api.table.migrate(table._id!, { + oldColumn: table.schema["user relationship"], + newColumn: { + name: "user column", + type: FieldType.BB_REFERENCE, + subtype: FieldSubtype.USERS, + }, + }) + + const migratedTable = await config.api.table.get(table._id!) + expect(migratedTable.schema["user column"]).toBeDefined() + expect(migratedTable.schema["user relationship"]).not.toBeDefined() + + const row1Migrated = (await config.api.row.get(table._id!, row1._id!)) + .body as Row + expect(row1Migrated["user relationship"]).not.toBeDefined() + expect(row1Migrated["user column"].map((r: Row) => r._id)).toEqual( + expect.arrayContaining([users[0]._id, users[1]._id]) + ) + + const row2Migrated = (await config.api.row.get(table._id!, row2._id!)) + .body as Row + expect(row2Migrated["user relationship"]).not.toBeDefined() + expect(row2Migrated["user column"].map((r: Row) => r._id)).toEqual([ + users[2]._id, + ]) }) }) }) diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 221aa6486c..caf9ffea02 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -57,6 +57,6 @@ export async function fetchView( tableId: string, viewName: string, params: ViewParams -) { +): Promise { return pickApi(tableId).fetchView(viewName, params) } diff --git a/packages/server/src/sdk/app/tables/migration.ts b/packages/server/src/sdk/app/tables/migration.ts index cea15114a1..e3f817f893 100644 --- a/packages/server/src/sdk/app/tables/migration.ts +++ b/packages/server/src/sdk/app/tables/migration.ts @@ -2,8 +2,12 @@ import { BadRequestError, context } from "@budibase/backend-core" import { BBReferenceFieldMetadata, FieldSchema, + FieldSubtype, InternalTable, + ManyToManyRelationshipFieldMetadata, + OneToManyRelationshipFieldMetadata, RelationshipFieldMetadata, + RelationshipType, Row, Table, isBBReferenceField, @@ -77,13 +81,30 @@ function getColumnMigrator( ) } - return new UserColumnMigrator(table, oldColumn, newColumn) + if (oldColumn.relationshipType === RelationshipType.ONE_TO_MANY) { + if (newColumn.subtype !== FieldSubtype.USER) { + throw new BadRequestError( + `Column "${oldColumn.name}" is a one-to-many column but "${newColumn.name}" is not a single user column` + ) + } + return new OneToManyUserColumnMigrator(table, oldColumn, newColumn) + } + if (oldColumn.relationshipType === RelationshipType.MANY_TO_MANY) { + if (newColumn.subtype !== FieldSubtype.USERS) { + throw new BadRequestError( + `Column "${oldColumn.name}" is a many-to-many column but "${newColumn.name}" is not a multi user column` + ) + } + return new ManyToManyUserColumnMigrator(table, oldColumn, newColumn) + } + + throw new BadRequestError(`Unknown migration type`) } -class UserColumnMigrator implements ColumnMigrator { +class OneToManyUserColumnMigrator implements ColumnMigrator { constructor( private table: Table, - private oldColumn: RelationshipFieldMetadata, + private oldColumn: OneToManyRelationshipFieldMetadata, private newColumn: BBReferenceFieldMetadata ) {} @@ -115,3 +136,42 @@ class UserColumnMigrator implements ColumnMigrator { await db.bulkDocs(rows) } } + +class ManyToManyUserColumnMigrator implements ColumnMigrator { + constructor( + private table: Table, + private oldColumn: ManyToManyRelationshipFieldMetadata, + private newColumn: BBReferenceFieldMetadata + ) {} + + async doMigration() { + let rows = await sdk.rows.fetchRaw(this.table._id!) + let rowsById = rows.reduce((acc, row) => { + acc[row._id!] = row + return acc + }, {} as Record) + + let links = await sdk.links.fetchWithDocument(this.table._id!) + for (let link of links) { + if (link.doc1.tableId !== this.table._id) { + continue + } + if (link.doc1.fieldName !== this.oldColumn.name) { + continue + } + if (link.doc2.tableId !== InternalTable.USER_METADATA) { + continue + } + + let userId = dbCore.getGlobalIDFromUserMetadataID(link.doc2.rowId) + let row = rowsById[link.doc1.rowId] + if (!row[this.newColumn.name]) { + row[this.newColumn.name] = [] + } + row[this.newColumn.name].push(userId) + } + + let db = context.getAppDB() + await db.bulkDocs(rows) + } +} diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index cec8c8aa12..c00772ef63 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -55,6 +55,7 @@ import { RelationshipType, CreateViewRequest, RelationshipFieldMetadata, + User, } from "@budibase/types" import API from "./api" @@ -254,7 +255,7 @@ class TestConfiguration { } catch (err) { existing = { email } } - const user = { + const user: User = { _id: id, ...existing, roles: roles || {}, diff --git a/packages/server/src/tests/utilities/api/index.ts b/packages/server/src/tests/utilities/api/index.ts index fce8237760..1352874914 100644 --- a/packages/server/src/tests/utilities/api/index.ts +++ b/packages/server/src/tests/utilities/api/index.ts @@ -27,5 +27,6 @@ export default class API { this.datasource = new DatasourceAPI(config) this.screen = new ScreenAPI(config) this.application = new ApplicationAPI(config) + this.global = new GlobalAPI(config) } } From 9dd16381a7e1c83eb3339f1790289af14b7b8774 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 23 Oct 2023 09:52:17 +0100 Subject: [PATCH 2/5] Merge base branch. --- packages/server/src/tests/utilities/api/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/tests/utilities/api/index.ts b/packages/server/src/tests/utilities/api/index.ts index 1352874914..fce8237760 100644 --- a/packages/server/src/tests/utilities/api/index.ts +++ b/packages/server/src/tests/utilities/api/index.ts @@ -27,6 +27,5 @@ export default class API { this.datasource = new DatasourceAPI(config) this.screen = new ScreenAPI(config) this.application = new ApplicationAPI(config) - this.global = new GlobalAPI(config) } } From 06f0a8da1ae5ee15f4e583a526e81f8c93eb6fef Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 23 Oct 2023 09:59:11 +0100 Subject: [PATCH 3/5] Update pro submodule. --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 570d14aa44..56d968bfe6 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 570d14aa44aa88f4d053856322210f0008ba5c76 +Subproject commit 56d968bfe6998e1077d3fce4eb1c9e483d1d6fc9 From 9e279be4c334c5e613066a59fe0ab8b42a842e0c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 23 Oct 2023 10:35:41 +0100 Subject: [PATCH 4/5] Put pro back to where it was. --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 56d968bfe6..570d14aa44 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 56d968bfe6998e1077d3fce4eb1c9e483d1d6fc9 +Subproject commit 570d14aa44aa88f4d053856322210f0008ba5c76 From febfab092717f106ef13f9827221e67d74c244b8 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 23 Oct 2023 10:48:10 +0100 Subject: [PATCH 5/5] Fix tests/types. --- packages/server/src/sdk/users/tests/utils.spec.ts | 14 +++++++------- .../src/tests/utilities/TestConfiguration.ts | 7 ++----- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/server/src/sdk/users/tests/utils.spec.ts b/packages/server/src/sdk/users/tests/utils.spec.ts index 5c6777df59..f7c9413ebd 100644 --- a/packages/server/src/sdk/users/tests/utils.spec.ts +++ b/packages/server/src/sdk/users/tests/utils.spec.ts @@ -39,12 +39,12 @@ describe("syncGlobalUsers", () => { expect(metadata).toHaveLength(3) expect(metadata).toContainEqual( expect.objectContaining({ - _id: db.generateUserMetadataID(user1._id), + _id: db.generateUserMetadataID(user1._id!), }) ) expect(metadata).toContainEqual( expect.objectContaining({ - _id: db.generateUserMetadataID(user2._id), + _id: db.generateUserMetadataID(user2._id!), }) ) }) @@ -59,7 +59,7 @@ describe("syncGlobalUsers", () => { expect(metadata).toHaveLength(1) expect(metadata).not.toContainEqual( expect.objectContaining({ - _id: db.generateUserMetadataID(user._id), + _id: db.generateUserMetadataID(user._id!), }) ) }) @@ -70,7 +70,7 @@ describe("syncGlobalUsers", () => { const group = await proSdk.groups.save(structures.userGroups.userGroup()) const user1 = await config.createUser({ admin: false, builder: false }) const user2 = await config.createUser({ admin: false, builder: false }) - await proSdk.groups.addUsers(group.id, [user1._id, user2._id]) + await proSdk.groups.addUsers(group.id, [user1._id!, user2._id!]) await config.doInContext(config.appId, async () => { await syncGlobalUsers() @@ -87,12 +87,12 @@ describe("syncGlobalUsers", () => { expect(metadata).toHaveLength(3) expect(metadata).toContainEqual( expect.objectContaining({ - _id: db.generateUserMetadataID(user1._id), + _id: db.generateUserMetadataID(user1._id!), }) ) expect(metadata).toContainEqual( expect.objectContaining({ - _id: db.generateUserMetadataID(user2._id), + _id: db.generateUserMetadataID(user2._id!), }) ) }) @@ -109,7 +109,7 @@ describe("syncGlobalUsers", () => { { appId: config.prodAppId!, roleId: roles.BUILTIN_ROLE_IDS.BASIC }, ], }) - await proSdk.groups.addUsers(group.id, [user1._id, user2._id]) + await proSdk.groups.addUsers(group.id, [user1._id!, user2._id!]) await config.doInContext(config.appId, async () => { await syncGlobalUsers() diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index c00772ef63..5f5b211975 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -295,7 +295,7 @@ class TestConfiguration { admin?: boolean roles?: UserRoles } = {} - ) { + ): Promise { let { id, firstName, lastName, email, builder, admin, roles } = user firstName = firstName || this.defaultUserValues.firstName lastName = lastName || this.defaultUserValues.lastName @@ -315,10 +315,7 @@ class TestConfiguration { roles, }) await cache.user.invalidateUser(globalId) - return { - ...resp, - globalId, - } + return resp } async createGroup(roleId: string = roles.BUILTIN_ROLE_IDS.BASIC) {