From 8694b8d772d0d6ac855a2167080400bc16dda3fb Mon Sep 17 00:00:00 2001
From: melohagan <101575380+melohagan@users.noreply.github.com>
Date: Tue, 5 Mar 2024 09:03:19 +0000
Subject: [PATCH] Relationship picker incorrectly renders selections (#13175)

* Ensure _id is decoded for external search

* Fetch initial value for 'Update' type forms

* test didn't run locally - might run on github workflow

* Tested and appears to be as before

* Null-pointer fix

* undo type change

* update modules

* add test

* update modules
---
 .../app/forms/RelationshipField.svelte        |  27 +++-
 .../src/sdk/app/rows/search/external.ts       |  18 ++-
 .../app/rows/search/tests/external.spec.ts    | 115 ++++++++++--------
 .../server/src/sdk/app/rows/search/utils.ts   |   1 -
 4 files changed, 102 insertions(+), 59 deletions(-)

diff --git a/packages/client/src/components/app/forms/RelationshipField.svelte b/packages/client/src/components/app/forms/RelationshipField.svelte
index 7cd571f6d9..1fbd0df522 100644
--- a/packages/client/src/components/app/forms/RelationshipField.svelte
+++ b/packages/client/src/components/app/forms/RelationshipField.svelte
@@ -1,7 +1,7 @@
 <script>
   import { CoreSelect, CoreMultiselect } from "@budibase/bbui"
   import { fetchData, Utils } from "@budibase/frontend-core"
-  import { getContext } from "svelte"
+  import { getContext, onMount } from "svelte"
   import Field from "./Field.svelte"
   import { FieldTypes } from "../../../constants"
 
@@ -28,6 +28,7 @@
   let tableDefinition
   let searchTerm
   let open
+  let initialValue
 
   $: type =
     datasourceType === "table" ? FieldTypes.LINK : FieldTypes.BB_REFERENCE
@@ -109,7 +110,11 @@
   }
 
   $: forceFetchRows(filter)
-  $: debouncedFetchRows(searchTerm, primaryDisplay, defaultValue)
+  $: debouncedFetchRows(
+    searchTerm,
+    primaryDisplay,
+    initialValue || defaultValue
+  )
 
   const forceFetchRows = async () => {
     // if the filter has changed, then we need to reset the options, clear the selection, and re-fetch
@@ -127,9 +132,13 @@
     if (allRowsFetched || !primaryDisplay) {
       return
     }
-    if (defaultVal && !optionsObj[defaultVal]) {
+    // must be an array
+    if (defaultVal && !Array.isArray(defaultVal)) {
+      defaultVal = defaultVal.split(",")
+    }
+    if (defaultVal && defaultVal.some(val => !optionsObj[val])) {
       await fetch.update({
-        query: { equal: { _id: defaultVal } },
+        query: { oneOf: { _id: defaultVal } },
       })
     }
 
@@ -202,6 +211,16 @@
       fetch.nextPage()
     }
   }
+
+  onMount(() => {
+    // if the form is in 'Update' mode, then we need to fetch the matching row so that the value is correctly set
+    if (fieldState?.value) {
+      initialValue =
+        fieldSchema?.relationshipType !== "one-to-many"
+          ? flatten(fieldState?.value) ?? []
+          : flatten(fieldState?.value)?.[0]
+    }
+  })
 </script>
 
 <Field
diff --git a/packages/server/src/sdk/app/rows/search/external.ts b/packages/server/src/sdk/app/rows/search/external.ts
index e2d1a1b32c..e95b904767 100644
--- a/packages/server/src/sdk/app/rows/search/external.ts
+++ b/packages/server/src/sdk/app/rows/search/external.ts
@@ -11,7 +11,10 @@ import {
 import * as exporters from "../../../../api/controllers/view/exporters"
 import sdk from "../../../../sdk"
 import { handleRequest } from "../../../../api/controllers/row/external"
-import { breakExternalTableId } from "../../../../integrations/utils"
+import {
+  breakExternalTableId,
+  breakRowIdField,
+} from "../../../../integrations/utils"
 import { cleanExportRows } from "../utils"
 import { utils } from "@budibase/shared-core"
 import { ExportRowsParams, ExportRowsResult } from "../search"
@@ -52,6 +55,15 @@ export async function search(options: SearchParams) {
     }
   }
 
+  // Make sure oneOf _id queries decode the Row IDs
+  if (query?.oneOf?._id) {
+    const rowIds = query.oneOf._id
+    query.oneOf._id = rowIds.map((row: string) => {
+      const ids = breakRowIdField(row)
+      return ids[0]
+    })
+  }
+
   try {
     const table = await sdk.tables.getTable(tableId)
     options = searchInputMapping(table, options)
@@ -119,9 +131,7 @@ export async function exportRows(
     requestQuery = {
       oneOf: {
         _id: rowIds.map((row: string) => {
-          const ids = JSON.parse(
-            decodeURI(row).replace(/'/g, `"`).replace(/%2C/g, ",")
-          )
+          const ids = breakRowIdField(row)
           if (ids.length > 1) {
             throw new HTTPError(
               "Export data does not support composite keys.",
diff --git a/packages/server/src/sdk/app/rows/search/tests/external.spec.ts b/packages/server/src/sdk/app/rows/search/tests/external.spec.ts
index 1aaea8e258..bae84592ca 100644
--- a/packages/server/src/sdk/app/rows/search/tests/external.spec.ts
+++ b/packages/server/src/sdk/app/rows/search/tests/external.spec.ts
@@ -21,10 +21,11 @@ jest.unmock("mysql2/promise")
 
 jest.setTimeout(30000)
 
-describe.skip("external", () => {
+describe("external search", () => {
   const config = new TestConfiguration()
 
   let externalDatasource: Datasource, tableData: Table
+  const rows: Row[] = []
 
   beforeAll(async () => {
     const container = await new GenericContainer("mysql")
@@ -89,67 +90,81 @@ describe.skip("external", () => {
         },
       },
     }
+
+    const table = await config.createExternalTable({
+      ...tableData,
+      sourceId: externalDatasource._id,
+    })
+    for (let i = 0; i < 10; i++) {
+      rows.push(
+        await config.createRow({
+          tableId: table._id,
+          name: generator.first(),
+          surname: generator.last(),
+          age: generator.age(),
+          address: generator.address(),
+        })
+      )
+    }
   })
 
-  describe("search", () => {
-    const rows: Row[] = []
-    beforeAll(async () => {
-      const table = await config.createExternalTable({
-        ...tableData,
-        sourceId: externalDatasource._id,
-      })
-      for (let i = 0; i < 10; i++) {
-        rows.push(
-          await config.createRow({
-            tableId: table._id,
-            name: generator.first(),
-            surname: generator.last(),
-            age: generator.age(),
-            address: generator.address(),
-          })
-        )
+  it("default search returns all the data", async () => {
+    await config.doInContext(config.appId, async () => {
+      const tableId = config.table!._id!
+
+      const searchParams: SearchParams = {
+        tableId,
+        query: {},
       }
+      const result = await search(searchParams)
+
+      expect(result.rows).toHaveLength(10)
+      expect(result.rows).toEqual(
+        expect.arrayContaining(rows.map(r => expect.objectContaining(r)))
+      )
     })
+  })
 
-    it("default search returns all the data", async () => {
-      await config.doInContext(config.appId, async () => {
-        const tableId = config.table!._id!
+  it("querying by fields will always return data attribute columns", async () => {
+    await config.doInContext(config.appId, async () => {
+      const tableId = config.table!._id!
 
-        const searchParams: SearchParams = {
-          tableId,
-          query: {},
-        }
-        const result = await search(searchParams)
+      const searchParams: SearchParams = {
+        tableId,
+        query: {},
+        fields: ["name", "age"],
+      }
+      const result = await search(searchParams)
 
-        expect(result.rows).toHaveLength(10)
-        expect(result.rows).toEqual(
-          expect.arrayContaining(rows.map(r => expect.objectContaining(r)))
+      expect(result.rows).toHaveLength(10)
+      expect(result.rows).toEqual(
+        expect.arrayContaining(
+          rows.map(r => ({
+            ...expectAnyExternalColsAttributes,
+            name: r.name,
+            age: r.age,
+          }))
         )
-      })
+      )
     })
+  })
 
-    it("querying by fields will always return data attribute columns", async () => {
-      await config.doInContext(config.appId, async () => {
-        const tableId = config.table!._id!
+  it("will decode _id in oneOf query", async () => {
+    await config.doInContext(config.appId, async () => {
+      const tableId = config.table!._id!
 
-        const searchParams: SearchParams = {
-          tableId,
-          query: {},
-          fields: ["name", "age"],
-        }
-        const result = await search(searchParams)
+      const searchParams: SearchParams = {
+        tableId,
+        query: {
+          oneOf: {
+            _id: ["%5B1%5D", "%5B4%5D", "%5B8%5D"],
+          },
+        },
+      }
+      const result = await search(searchParams)
 
-        expect(result.rows).toHaveLength(10)
-        expect(result.rows).toEqual(
-          expect.arrayContaining(
-            rows.map(r => ({
-              ...expectAnyExternalColsAttributes,
-              name: r.name,
-              age: r.age,
-            }))
-          )
-        )
-      })
+      expect(result.rows).toHaveLength(3)
+      expect(result.rows.map(row => row.id)).toEqual([1, 4, 8])
     })
   })
 })
diff --git a/packages/server/src/sdk/app/rows/search/utils.ts b/packages/server/src/sdk/app/rows/search/utils.ts
index 4eee3cea41..5d93dcaca2 100644
--- a/packages/server/src/sdk/app/rows/search/utils.ts
+++ b/packages/server/src/sdk/app/rows/search/utils.ts
@@ -1,6 +1,5 @@
 import {
   FieldType,
-  FieldTypeSubtypes,
   SearchParams,
   Table,
   DocumentType,