Merge pull request #12158 from Budibase/feature/budi-7607-migrate-user-relationship-columns-to-the-new-user-column-4

Frontend changes for the user column migration work.

packages/frontend-core/src/api/tables.js

@@ -140,4 +140,13 @@ export const buildTableEndpoints = API => ({
       },
     })
   },
+  migrateColumn: async ({ tableId, oldColumn, newColumn }) => {
+    return await API.post({
+      url: `/api/tables/${tableId}/migrate`,
+      body: {
+        oldColumn,
+        newColumn,
+      },
+    })
+  },
 })

packages/frontend-core/src/components/grid/cells/HeaderCell.svelte

@@ -1,11 +1,20 @@
 <script>
   import { getContext, onMount, tick } from "svelte"
   import { canBeDisplayColumn, canBeSortColumn } from "@budibase/shared-core"
-  import { Icon, Popover, Menu, MenuItem, clickOutside } from "@budibase/bbui"
+  import {
+    Icon,
+    Popover,
+    Menu,
+    MenuItem,
+    clickOutside,
+    Modal,
+  } from "@budibase/bbui"
   import GridCell from "./GridCell.svelte"
   import { getColumnIcon } from "../lib/utils"
+  import MigrationModal from "../controls/MigrationModal.svelte"
   import { debounce } from "../../../utils/utils"
   import { FieldType, FormulaTypes } from "@budibase/types"
+  import { TableNames } from "../../../constants"
 
   export let column
   export let idx
@@ -45,6 +54,7 @@
   let editIsOpen = false
   let timeout
   let popover
+  let migrationModal
   let searchValue
   let input
 
@@ -189,6 +199,11 @@
     })
   }
 
+  const openMigrationModal = () => {
+    migrationModal.show()
+    open = false
+  }
+
   const startSearching = async () => {
     $focusedCellId = null
     searchValue = ""
@@ -224,6 +239,10 @@
   onMount(() => subscribe("close-edit-column", cancelEdit))
 </script>
 
+<Modal bind:this={migrationModal}>
+  <MigrationModal {column} />
+</Modal>
+
 <div
   class="header-cell"
   class:open
@@ -363,6 +382,11 @@
       >
         Hide column
       </MenuItem>
+      {#if $config.canEditColumns && column.schema.type === "link" && column.schema.tableId === TableNames.USERS}
+        <MenuItem icon="User" on:click={openMigrationModal}>
+          Migrate to user column
+        </MenuItem>
+      {/if}
     </Menu>
   {/if}
 </Popover>

packages/frontend-core/src/components/grid/controls/MigrationModal.svelte

@@ -0,0 +1,70 @@
+<script>
+  import {
+    ModalContent,
+    notifications,
+    Input,
+    InlineAlert,
+  } from "@budibase/bbui"
+  import { getContext } from "svelte"
+  import { ValidColumnNameRegex } from "@budibase/shared-core"
+  import { FieldSubtype, FieldType, RelationshipType } from "@budibase/types"
+
+  const { API, definition, rows } = getContext("grid")
+
+  export let column
+
+  let newColumnName = `${column.schema.name} migrated`
+  $: error = checkNewColumnName(newColumnName)
+
+  const checkNewColumnName = newColumnName => {
+    if (newColumnName in $definition.schema) {
+      return "New column name can't be the same as an existing column name."
+    }
+    if (newColumnName.match(ValidColumnNameRegex) === null) {
+      return "Illegal character; must be alpha-numeric."
+    }
+  }
+
+  const migrateUserColumn = async () => {
+    let subtype = FieldSubtype.USERS
+    if (column.schema.relationshipType === RelationshipType.ONE_TO_MANY) {
+      subtype = FieldSubtype.USER
+    }
+
+    try {
+      await API.migrateColumn({
+        tableId: $definition._id,
+        oldColumn: column.schema,
+        newColumn: {
+          name: newColumnName,
+          type: FieldType.BB_REFERENCE,
+          subtype,
+        },
+      })
+      notifications.success("Column migrated")
+    } catch (e) {
+      notifications.error(`Failed to migrate: ${e.message}`)
+    }
+    await rows.actions.refreshData()
+  }
+</script>
+
+<ModalContent
+  title="Migrate column"
+  confirmText="Continue"
+  cancelText="Cancel"
+  onConfirm={migrateUserColumn}
+  disabled={error !== undefined}
+  size="M"
+>
+  This operation will kick off a migration of the column "{column.schema.name}"
+  to a new column, with the name provided - this operation may take a moment to
+  complete.
+
+  <InlineAlert
+    type="error"
+    header="Are you sure?"
+    message="This will leave bindings which utilised the user relationship column in a state where they will need to be updated to use the new column instead."
+  />
+  <Input bind:value={newColumnName} label="New column name" {error} />
+</ModalContent>

packages/server/src/api/controllers/table/index.ts

@@ -24,6 +24,7 @@ import sdk from "../../../sdk"
 import { jsonFromCsvString } from "../../../utilities/csv"
 import { builderSocket } from "../../../websockets"
 import { cloneDeep, isEqual } from "lodash"
+import { processInternalTable } from "../../../sdk/app/tables/getters"
 
 function pickApi({ tableId, table }: { tableId?: string; table?: Table }) {
   if (table && !tableId) {
@@ -165,7 +166,13 @@ export async function migrate(ctx: UserCtx<MigrateRequest, MigrateResponse>) {
   const { oldColumn, newColumn } = ctx.request.body
   let tableId = ctx.params.tableId as string
   const table = await sdk.tables.getTable(tableId)
-  await sdk.tables.migrate(table, oldColumn, newColumn)
+  let result = await sdk.tables.migrate(table, oldColumn, newColumn)
+
+  for (let table of result.tablesUpdated) {
+    builderSocket?.emitTableUpdate(ctx, table, {
+      includeOriginator: true,
+    })
+  }
 
   ctx.status = 200
   ctx.body = { message: `Column ${oldColumn.name} migrated.` }

packages/server/src/integrations/utils.ts

@@ -87,6 +87,10 @@ export function isExternalTable(tableId: string) {
   return tableId.includes(DocumentType.DATASOURCE)
 }
 
+export function isInternalTable(tableId: string) {
+  return !isExternalTable(tableId)
+}
+
 export function buildExternalTableId(datasourceId: string, tableName: string) {
   // encode spaces
   if (tableName.includes(" ")) {

packages/server/src/sdk/app/tables/getters.ts

@@ -19,12 +19,16 @@ import {
 import datasources from "../datasources"
 import sdk from "../../../sdk"
 
-function processInternalTables(docs: AllDocsResponse<Table[]>): Table[] {
-  return docs.rows.map((tableDoc: any) => ({
-    ...tableDoc.doc,
+function processInternalTables(tables: Table[]): Table[] {
+  return tables.map(processInternalTable)
+}
+
+export function processInternalTable(table: Table): Table {
+  return {
+    ...table,
     type: "internal",
-    sourceId: tableDoc.doc.sourceId || BudibaseInternalDB._id,
-  }))
+    sourceId: table.sourceId || BudibaseInternalDB._id,
+  }
 }
 
 export async function getAllInternalTables(db?: Database): Promise<Table[]> {
@@ -36,7 +40,7 @@ export async function getAllInternalTables(db?: Database): Promise<Table[]> {
       include_docs: true,
     })
   )
-  return processInternalTables(internalTables)
+  return processInternalTables(internalTables.rows.map(row => row.doc!))
 }
 
 async function getAllExternalTables(): Promise<Table[]> {
@@ -106,7 +110,9 @@ export async function getTables(tableIds: string[]): Promise<Table[]> {
     const internalTableDocs = await db.allDocs<Table[]>(
       getMultiIDParams(internalTableIds)
     )
-    tables = tables.concat(processInternalTables(internalTableDocs))
+    tables = tables.concat(
+      processInternalTables(internalTableDocs.rows.map(row => row.doc!))
+    )
   }
   return tables
 }

packages/server/src/sdk/app/tables/migration.ts

@@ -1,44 +1,52 @@
-import { BadRequestError, context } from "@budibase/backend-core"
+import { BadRequestError, context, db as dbCore } from "@budibase/backend-core"
 import {
   BBReferenceFieldMetadata,
   FieldSchema,
   FieldSubtype,
   InternalTable,
-  ManyToManyRelationshipFieldMetadata,
-  ManyToOneRelationshipFieldMetadata,
-  OneToManyRelationshipFieldMetadata,
+  isBBReferenceField,
+  isRelationshipField,
+  LinkDocument,
+  RelationshipFieldMetadata,
   RelationshipType,
   Row,
   Table,
-  isBBReferenceField,
-  isRelationshipField,
 } from "@budibase/types"
 import sdk from "../../../sdk"
-import { isExternalTable } from "../../../../src/integrations/utils"
-import { db as dbCore } from "@budibase/backend-core"
-import { EventType, updateLinks } from "../../../../src/db/linkedRows"
+import { isExternalTable } from "../../../integrations/utils"
+import { EventType, updateLinks } from "../../../db/linkedRows"
 import { cloneDeep } from "lodash"
 
+export interface MigrationResult {
+  tablesUpdated: Table[]
+}
+
 export async function migrate(
   table: Table,
   oldColumn: FieldSchema,
   newColumn: FieldSchema
-) {
-  let migrator = getColumnMigrator(table, oldColumn, newColumn)
-  let oldTable = cloneDeep(table)
+): Promise<MigrationResult> {
+  if (newColumn.name in table.schema) {
+    throw new BadRequestError(`Column "${newColumn.name}" already exists`)
+  }
 
   table.schema[newColumn.name] = newColumn
   table = await sdk.tables.saveTable(table)
 
-  await migrator.doMigration()
-
-  delete table.schema[oldColumn.name]
-  table = await sdk.tables.saveTable(table)
-  await updateLinks({ eventType: EventType.TABLE_UPDATED, table, oldTable })
+  let migrator = getColumnMigrator(table, oldColumn, newColumn)
+  try {
+    return await migrator.doMigration()
+  } catch (e) {
+    // If the migration fails then we need to roll back the table schema
+    // change.
+    delete table.schema[newColumn.name]
+    await sdk.tables.saveTable(table)
+    throw e
+  }
 }
 
 interface ColumnMigrator {
-  doMigration(): Promise<void>
+  doMigration(): Promise<MigrationResult>
 }
 
 function getColumnMigrator(
@@ -46,8 +54,8 @@ function getColumnMigrator(
   oldColumn: FieldSchema,
   newColumn: FieldSchema
 ): ColumnMigrator {
-  // For now we're only supporting migrations of user relationships to user
-  // columns in internal tables. In future we may want to support other
+  // For now, we're only supporting migrations of user relationships to user
+  // columns in internal tables. In the future, we may want to support other
   // migrations but for now return an error if we aren't migrating a user
   // relationship.
   if (isExternalTable(table._id!)) {
@@ -58,10 +66,6 @@ function getColumnMigrator(
     throw new BadRequestError(`Column "${oldColumn.name}" does not exist`)
   }
 
-  if (newColumn.name in table.schema) {
-    throw new BadRequestError(`Column "${newColumn.name}" already exists`)
-  }
-
   if (!isBBReferenceField(newColumn)) {
     throw new BadRequestError(`Column "${newColumn.name}" is not a user column`)
   }
@@ -105,14 +109,17 @@ function getColumnMigrator(
   throw new BadRequestError(`Unknown migration type`)
 }
 
-class SingleUserColumnMigrator implements ColumnMigrator {
+abstract class UserColumnMigrator implements ColumnMigrator {
   constructor(
-    private table: Table,
-    private oldColumn: OneToManyRelationshipFieldMetadata,
-    private newColumn: BBReferenceFieldMetadata
+    protected table: Table,
+    protected oldColumn: RelationshipFieldMetadata,
+    protected newColumn: BBReferenceFieldMetadata
   ) {}
 
-  async doMigration() {
+  abstract updateRow(row: Row, link: LinkDocument): void
+
+  async doMigration(): Promise<MigrationResult> {
+    let oldTable = cloneDeep(this.table)
     let rows = await sdk.rows.fetchRaw(this.table._id!)
     let rowsById = rows.reduce((acc, row) => {
       acc[row._id!] = row
@@ -121,63 +128,58 @@ class SingleUserColumnMigrator implements ColumnMigrator {
 
     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) {
+      if (
+        link.doc1.tableId !== this.table._id ||
+        link.doc1.fieldName !== this.oldColumn.name ||
+        link.doc2.tableId !== InternalTable.USER_METADATA
+      ) {
         continue
       }
 
-      let userId = dbCore.getGlobalIDFromUserMetadataID(link.doc2.rowId)
       let row = rowsById[link.doc1.rowId]
-      row[this.newColumn.name] = userId
+      if (!row) {
+        // This can happen if the row has been deleted but the link hasn't,
+        // which was a state that was found during the initial testing of this
+        // feature. Not sure exactly what can cause it, but best to be safe.
+        continue
+      }
+
+      this.updateRow(row, link)
     }
 
     let db = context.getAppDB()
     await db.bulkDocs(rows)
-  }
-}
 
-class MultiUserColumnMigrator implements ColumnMigrator {
-  constructor(
-    private table: Table,
-    private oldColumn:
-      | ManyToManyRelationshipFieldMetadata
-      | ManyToOneRelationshipFieldMetadata,
-    private newColumn: BBReferenceFieldMetadata
-  ) {}
+    delete this.table.schema[this.oldColumn.name]
+    this.table = await sdk.tables.saveTable(this.table)
+    await updateLinks({
+      eventType: EventType.TABLE_UPDATED,
+      table: this.table,
+      oldTable,
+    })
 
-  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<string, Row>)
-
-    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 otherTable = await sdk.tables.getTable(this.oldColumn.tableId)
+    return {
+      tablesUpdated: [this.table, otherTable],
     }
-
-    let db = context.getAppDB()
-    await db.bulkDocs(rows)
+  }
+}
+
+class SingleUserColumnMigrator extends UserColumnMigrator {
+  updateRow(row: Row, link: LinkDocument): void {
+    row[this.newColumn.name] = dbCore.getGlobalIDFromUserMetadataID(
+      link.doc2.rowId
+    )
+  }
+}
+
+class MultiUserColumnMigrator extends UserColumnMigrator {
+  updateRow(row: Row, link: LinkDocument): void {
+    if (!row[this.newColumn.name]) {
+      row[this.newColumn.name] = []
+    }
+    row[this.newColumn.name].push(
+      dbCore.getGlobalIDFromUserMetadataID(link.doc2.rowId)
+    )
   }
 }

packages/server/src/tests/utilities/api/table.ts

@@ -58,8 +58,13 @@ export class TableAPI extends TestAPI {
       .post(`/api/tables/${tableId}/migrate`)
       .send(data)
       .set(this.config.defaultHeaders())
-      .expect("Content-Type", /json/)
-      .expect(expectStatus)
+    if (res.status !== expectStatus) {
+      throw new Error(
+        `Expected status ${expectStatus} but got ${
+          res.status
+        } with body ${JSON.stringify(res.body)}`
+      )
+    }
     return res.body
   }
 }

packages/server/src/websockets/builder.ts

@@ -1,5 +1,5 @@
 import authorized from "../middleware/authorized"
-import { BaseSocket } from "./websocket"
+import { BaseSocket, EmitOptions } from "./websocket"
 import { permissions, events, context } from "@budibase/backend-core"
 import http from "http"
 import Koa from "koa"
@@ -16,6 +16,8 @@ import { gridSocket } from "./index"
 import { clearLock, updateLock } from "../utilities/redis"
 import { Socket } from "socket.io"
 import { BuilderSocketEvent } from "@budibase/shared-core"
+import { processInternalTable } from "../sdk/app/tables/getters"
+import { isExternalTable, isInternalTable } from "../integrations/utils"
 
 export default class BuilderSocket extends BaseSocket {
   constructor(app: Koa, server: http.Server) {
@@ -100,11 +102,24 @@ export default class BuilderSocket extends BaseSocket {
     })
   }
 
-  emitTableUpdate(ctx: any, table: Table) {
-    this.emitToRoom(ctx, ctx.appId, BuilderSocketEvent.TableChange, {
-      id: table._id,
-      table,
-    })
+  emitTableUpdate(ctx: any, table: Table, options?: EmitOptions) {
+    // This was added to make sure that sourceId is always present when
+    // sending this message to clients. Without this, tables without a
+    // sourceId (e.g. ta_users) won't get correctly updated client-side.
+    if (isInternalTable(table._id!)) {
+      table = processInternalTable(table)
+    }
+
+    this.emitToRoom(
+      ctx,
+      ctx.appId,
+      BuilderSocketEvent.TableChange,
+      {
+        id: table._id,
+        table,
+      },
+      options
+    )
     gridSocket?.emitTableUpdate(ctx, table)
   }
 

packages/server/src/websockets/websocket.ts

@@ -11,6 +11,14 @@ import { SocketSession } from "@budibase/types"
 import { v4 as uuid } from "uuid"
 import { createContext, runMiddlewares } from "./middleware"
 
+export interface EmitOptions {
+  // Whether to include the originator of the request from the broadcast,
+  // defaults to false because it is assumed that the user who triggered
+  // an action will already have the changes of that action reflected in their
+  // own UI, so there is no need to send them again.
+  includeOriginator?: boolean
+}
+
 const anonUser = () => ({
   _id: uuid(),
   email: "user@mail.com",
@@ -270,10 +278,17 @@ export class BaseSocket {
 
   // Emit an event to everyone in a room, including metadata of whom
   // the originator of the request was
-  emitToRoom(ctx: any, room: string | string[], event: string, payload: any) {
-    this.io.in(room).emit(event, {
-      ...payload,
-      apiSessionId: ctx.headers?.[Header.SESSION_ID],
-    })
+  emitToRoom(
+    ctx: any,
+    room: string | string[],
+    event: string,
+    payload: any,
+    options?: EmitOptions
+  ) {
+    let emitPayload = { ...payload }
+    if (!options?.includeOriginator) {
+      emitPayload.apiSessionId = ctx.headers?.[Header.SESSION_ID]
+    }
+    this.io.in(room).emit(event, emitPayload)
   }
 }