code review

lerna.json

@@ -1,5 +1,5 @@
 {
-  "version": "1.0.41",
+  "version": "1.0.47",
   "npmClient": "yarn",
   "packages": [
     "packages/*"

packages/backend-core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@budibase/backend-core",
-  "version": "1.0.41",
+  "version": "1.0.47",
   "description": "Budibase backend core libraries used in server and worker",
   "main": "src/index.js",
   "author": "Budibase",

packages/backend-core/src/environment.js

@@ -6,13 +6,6 @@ function isTest() {
   )
 }
 
-function isDev() {
-  return (
-    process.env.NODE_ENV !== "production" &&
-    process.env.BUDIBASE_ENVIRONMENT !== "production"
-  )
-}
-
 module.exports = {
   JWT_SECRET: process.env.JWT_SECRET,
   COUCH_DB_URL: process.env.COUCH_DB_URL,
@@ -34,7 +27,6 @@ module.exports = {
   COOKIE_DOMAIN: process.env.COOKIE_DOMAIN,
   PLATFORM_URL: process.env.PLATFORM_URL,
   isTest,
-  isDev,
   _set(key, value) {
     process.env[key] = value
     module.exports[key] = value

packages/backend-core/src/middleware/passport/datasource/google.js

@@ -24,6 +24,8 @@ async function preAuth(passport, ctx, next) {
 
   return passport.authenticate(strategy, {
     scope: ["profile", "email", "https://www.googleapis.com/auth/spreadsheets"],
+    accessType: "offline",
+    prompt: "consent",
   })(ctx, next)
 }
 
@@ -58,12 +60,10 @@ async function postAuth(passport, ctx, next) {
       // update the DB for the datasource with all the user info
       const db = getDB(authStateCookie.appId)
       const datasource = await db.get(authStateCookie.datasourceId)
-      datasource.config = {
+      if (!datasource.config) {
-        auth: {
+        datasource.config = {}
-          type: "google",
-          ...tokens,
-        },
       }
+      datasource.config.auth = { type: "google", ...tokens }
       await db.put(datasource)
       ctx.redirect(
         `/builder/app/${authStateCookie.appId}/data/datasource/${authStateCookie.datasourceId}`

packages/backend-core/src/utils.js

@@ -23,7 +23,6 @@ const { getUserSessions, invalidateSessions } = require("./security/sessions")
 const { migrateIfRequired } = require("./migrations")
 const { USER_EMAIL_VIEW_CASING } = require("./migrations").MIGRATIONS
 const { GLOBAL_DB } = require("./migrations").MIGRATION_DBS
-const { isDev, isTest } = require("./environment")
 
 const APP_PREFIX = DocumentTypes.APP + SEPARATOR
 
@@ -109,11 +108,6 @@ exports.setCookie = (ctx, value, name = "builder", opts = { sign: true }) => {
     overwrite: true,
   }
 
-  if (!isDev() && !isTest()) {
-    config.sameSite = "none"
-    config.secure = true
-  }
-
   if (environment.COOKIE_DOMAIN) {
     config.domain = environment.COOKIE_DOMAIN
   }

packages/bbui/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@budibase/bbui",
   "description": "A UI solution used in the different Budibase projects.",
-  "version": "1.0.41",
+  "version": "1.0.47",
   "license": "MPL-2.0",
   "svelte": "src/index.js",
   "module": "dist/bbui.es.js",

packages/builder/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@budibase/builder",
-  "version": "1.0.41",
+  "version": "1.0.47",
   "license": "GPL-3.0",
   "private": true,
   "scripts": {
@@ -65,10 +65,10 @@
     }
   },
   "dependencies": {
-    "@budibase/bbui": "^1.0.41",
+    "@budibase/bbui": "^1.0.47",
-    "@budibase/client": "^1.0.41",
+    "@budibase/client": "^1.0.47",
     "@budibase/colorpicker": "1.1.2",
-    "@budibase/string-templates": "^1.0.41",
+    "@budibase/string-templates": "^1.0.47",
     "@sentry/browser": "5.19.1",
     "@spectrum-css/page": "^3.0.1",
     "@spectrum-css/vars": "^3.0.1",

packages/builder/src/components/design/PropertiesPanel/PropertyControls/ButtonActionEditor/actions/ExecuteQuery.svelte

@@ -55,8 +55,8 @@
         <div>
           <BindingBuilder
             bind:customParams={parameters.queryParams}
-            bindings={query.parameters}
+            queryBindings={query.parameters}
-            bind:bindableOptions={bindings}
+            bind:bindings
           />
           <IntegrationQueryEditor
             height={200}

packages/builder/src/components/design/PropertiesPanel/PropertyControls/DataSourceSelect.svelte

@@ -169,8 +169,8 @@
           {#if getQueryParams(value).length > 0}
             <BindingBuilder
               bind:customParams={tmpQueryParams}
-              bindings={getQueryParams(value)}
+              queryBindings={getQueryParams(value)}
-              bind:bindableOptions={bindings}
+              bind:bindings
             />
           {/if}
           <IntegrationQueryEditor

packages/builder/src/components/integration/QueryBindingBuilder.svelte

@@ -7,27 +7,24 @@
   import DrawerBindableInput from "components/common/bindings/DrawerBindableInput.svelte"
 
   export let bindable = true
+  export let queryBindings = []
   export let bindings = []
-  export let bindableOptions = []
   export let customParams = {}
 
   function newQueryBinding() {
-    bindings = [...bindings, {}]
+    queryBindings = [...queryBindings, {}]
   }
 
   function deleteQueryBinding(idx) {
-    bindings.splice(idx, 1)
+    queryBindings.splice(idx, 1)
-    bindings = bindings
+    queryBindings = queryBindings
   }
 
   // This is necessary due to the way readable and writable bindings are stored.
   // The readable binding in the UI gets converted to a UUID value that the client understands
   // for parsing, then converted back so we can display it the readable form in the UI
   function onBindingChange(param, valueToParse) {
-    customParams[param] = readableToRuntimeBinding(
+    customParams[param] = readableToRuntimeBinding(bindings, valueToParse)
-      bindableOptions,
-      valueToParse
-    )
   }
 </script>
 
@@ -49,7 +46,7 @@
     {/if}
   </Body>
   <div class="bindings" class:bindable>
-    {#each bindings as binding, idx}
+    {#each queryBindings as binding, idx}
       <Input
         placeholder="Binding Name"
         thin
@@ -69,10 +66,10 @@
           thin
           on:change={evt => onBindingChange(binding.name, evt.detail)}
           value={runtimeToReadableBinding(
-            bindableOptions,
+            bindings,
             customParams?.[binding.name]
           )}
-          {bindableOptions}
+          {bindings}
         />
       {:else}
         <Icon hoverable name="Close" on:click={() => deleteQueryBinding(idx)} />

packages/builder/src/components/integration/QueryViewer.svelte

@@ -120,7 +120,7 @@
           config={integrationInfo.extra}
         />
       {/if}
-      <BindingBuilder bind:bindings={query.parameters} bindable={false} />
+      <BindingBuilder bind:queryBindings={query.parameters} bindable={false} />
     {/if}
   </div>
   {#if shouldShowQueryConfig}

packages/cli/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@budibase/cli",
-  "version": "1.0.41",
+  "version": "1.0.47",
   "description": "Budibase CLI, for developers, self hosting and migrations.",
   "main": "src/index.js",
   "bin": {

packages/client/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@budibase/client",
-  "version": "1.0.41",
+  "version": "1.0.47",
   "license": "MPL-2.0",
   "module": "dist/budibase-client.js",
   "main": "dist/budibase-client.js",
@@ -19,9 +19,9 @@
     "dev:builder": "rollup -cw"
   },
   "dependencies": {
-    "@budibase/bbui": "^1.0.41",
+    "@budibase/bbui": "^1.0.47",
     "@budibase/standard-components": "^0.9.139",
-    "@budibase/string-templates": "^1.0.41",
+    "@budibase/string-templates": "^1.0.47",
     "regexparam": "^1.3.0",
     "shortid": "^2.2.15",
     "svelte-spa-router": "^3.0.5"

packages/client/src/components/Component.svelte

@@ -276,27 +276,29 @@
   // reactive statements as much as possible.
   const cacheSettings = (enriched, nested, conditional) => {
     const allSettings = { ...enriched, ...nested, ...conditional }
-    if (!cachedSettings) {
+    const mounted = ref?.$$set != null
+    if (!cachedSettings || !mounted) {
       cachedSettings = { ...allSettings }
       initialSettings = cachedSettings
     } else {
       Object.keys(allSettings).forEach(key => {
         const same = propsAreSame(allSettings[key], cachedSettings[key])
         if (!same) {
+          // Updated cachedSettings (which is assigned by reference to
+          // initialSettings) so that if we remount the component then the
+          // initial props are up to date. By setting it this way rather than
+          // setting it on initialSettings directly, we avoid a double render.
           cachedSettings[key] = allSettings[key]
-          assignSetting(key, allSettings[key])
+
+          // Programmatically set the prop to avoid svelte reactive statements
+          // firing inside components. This circumvents the problems caused by
+          // spreading a props object.
+          ref.$$set({ [key]: allSettings[key] })
         }
       })
     }
   }
 
-  // Assigns a certain setting to this component.
-  // We manually use the svelte $set function to avoid triggering additional
-  // reactive statements.
-  const assignSetting = (key, value) => {
-    ref?.$$set?.({ [key]: value })
-  }
-
   // Generates a key used to determine when components need to fully remount.
   // Currently only toggling editing requires remounting.
   const getRenderKey = (id, editing) => {
@@ -305,7 +307,7 @@
 </script>
 
 {#key renderKey}
-  {#if constructor && cachedSettings && (visible || inSelectedPath)}
+  {#if constructor && initialSettings && (visible || inSelectedPath)}
     <!-- The ID is used as a class because getElementsByClassName is O(1) -->
     <!-- and the performance matters for the selection indicators -->
     <div

packages/client/src/utils/fetch/DataFetch.js

@@ -111,12 +111,6 @@ export default class DataFetch {
    */
   async getInitialData() {
     const { datasource, filter, sortColumn, paginate } = this.options
-    const tableId = datasource?.tableId
-
-    // Ensure table ID exists
-    if (!tableId) {
-      return
-    }
 
     // Fetch datasource definition and determine feature flags
     const definition = await this.constructor.getDefinition(datasource)

packages/server/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@budibase/server",
   "email": "hi@budibase.com",
-  "version": "1.0.41",
+  "version": "1.0.47",
   "description": "Budibase Web Server",
   "main": "src/index.ts",
   "repository": {
@@ -70,9 +70,9 @@
   "license": "GPL-3.0",
   "dependencies": {
     "@apidevtools/swagger-parser": "^10.0.3",
-    "@budibase/backend-core": "^1.0.41",
+    "@budibase/backend-core": "^1.0.47",
-    "@budibase/client": "^1.0.41",
+    "@budibase/client": "^1.0.47",
-    "@budibase/string-templates": "^1.0.41",
+    "@budibase/string-templates": "^1.0.47",
     "@bull-board/api": "^3.7.0",
     "@bull-board/koa": "^3.7.0",
     "@elastic/elasticsearch": "7.10.0",

packages/server/src/api/controllers/query/index.js

@@ -12,7 +12,7 @@ const { invalidateDynamicVariables } = require("../../../threads/utils")
 const environment = require("../../../environment")
 
 const Runner = new Thread(ThreadType.QUERY, {
-  timeoutMs: 10000 || environment.QUERY_THREAD_TIMEOUT,
+  timeoutMs: environment.QUERY_THREAD_TIMEOUT || 10000,
 })
 
 // simple function to append "readable" to all read queries
@@ -141,6 +141,16 @@ async function execute(ctx, opts = { rowsOnly: false }) {
   const query = await db.get(ctx.params.queryId)
   const datasource = await db.get(query.datasourceId)
 
+  const enrichedParameters = ctx.request.body.parameters || {}
+  // make sure parameters are fully enriched with defaults
+  if (query && query.parameters) {
+    for (let parameter of query.parameters) {
+      if (!enrichedParameters[parameter.name]) {
+        enrichedParameters[parameter.name] = parameter.default
+      }
+    }
+  }
+
   // call the relevant CRUD method on the integration class
   try {
     const { rows, pagination, extra } = await Runner.run({
@@ -149,7 +159,7 @@ async function execute(ctx, opts = { rowsOnly: false }) {
       queryVerb: query.queryVerb,
       fields: query.fields,
       pagination: ctx.request.body.pagination,
-      parameters: ctx.request.body.parameters,
+      parameters: enrichedParameters,
       transformer: query.transformer,
       queryId: ctx.params.queryId,
     })
@@ -178,8 +188,9 @@ const removeDynamicVariables = async (db, queryId) => {
 
   if (dynamicVariables) {
     // delete dynamic variables from the datasource
-    const newVariables = dynamicVariables.filter(dv => dv.queryId !== queryId)
+    datasource.config.dynamicVariables = dynamicVariables.filter(
-    datasource.config.dynamicVariables = newVariables
+      dv => dv.queryId !== queryId
+    )
     await db.put(datasource)
 
     // invalidate the deleted variables

packages/server/src/api/controllers/row/utils.js

@@ -52,21 +52,29 @@ exports.validate = async ({ appId, tableId, row, table }) => {
     const constraints = cloneDeep(table.schema[fieldName].constraints)
     const type = table.schema[fieldName].type
     // special case for options, need to always allow unselected (null)
-    if (
+    if (type === FieldTypes.OPTIONS && constraints.inclusion) {
-      (type === FieldTypes.OPTIONS || type === FieldTypes.ARRAY) &&
-      constraints.inclusion
-    ) {
       constraints.inclusion.push(null)
     }
     let res
 
     // Validate.js doesn't seem to handle array
-    if (type === FieldTypes.ARRAY && row[fieldName] && row[fieldName].length) {
+    if (type === FieldTypes.ARRAY) {
+      const hasValues =
+        Array.isArray(row[fieldName]) && row[fieldName].length > 0
+
+      // Check values are valid if values are specified
+      if (hasValues) {
         row[fieldName].map(val => {
           if (!constraints.inclusion.includes(val)) {
-          errors[fieldName] = "Field not in list"
+            errors[fieldName] = "Value not in list"
           }
         })
+      }
+
+      // Check for required constraint
+      if (constraints.presence === true && !hasValues) {
+        errors[fieldName] = "Required field"
+      }
     } else if (type === FieldTypes.JSON && typeof row[fieldName] === "string") {
       // this should only happen if there is an error
       try {

packages/server/src/integrations/googlesheets.ts

@@ -118,7 +118,9 @@ module GoogleSheetsModule {
      */
     cleanSpreadsheetUrl(spreadsheetId: string) {
       if (!spreadsheetId) {
-        throw new Error("You must set a spreadsheet ID in your configuration to fetch tables.")
+        throw new Error(
+          "You must set a spreadsheet ID in your configuration to fetch tables."
+        )
       }
       const parts = spreadsheetId.split("/")
       return parts.length > 5 ? parts[5] : spreadsheetId
@@ -179,22 +181,27 @@ module GoogleSheetsModule {
       const sheet = json.endpoint.entityId
 
       const handlers = {
-        [DataSourceOperation.CREATE]: () => this.create({ sheet, row: json.body }),
+        [DataSourceOperation.CREATE]: () =>
+          this.create({ sheet, row: json.body }),
         [DataSourceOperation.READ]: () => this.read({ sheet }),
-        [DataSourceOperation.UPDATE]: () => this.update({ 
+        [DataSourceOperation.UPDATE]: () =>
+          this.update({
             // exclude the header row and zero index
             rowIndex: json.extra?.idFilter?.equal?.rowNumber - 2,
             sheet,
             row: json.body,
           }),
-        [DataSourceOperation.DELETE]: () => this.delete({
+        [DataSourceOperation.DELETE]: () =>
+          this.delete({
             // exclude the header row and zero index
             rowIndex: json.extra?.idFilter?.equal?.rowNumber - 2,
             sheet,
           }),
-        [DataSourceOperation.CREATE_TABLE]: () => this.createTable(json?.table?.name),
+        [DataSourceOperation.CREATE_TABLE]: () =>
+          this.createTable(json?.table?.name),
         [DataSourceOperation.UPDATE_TABLE]: () => this.updateTable(json.table),
-        [DataSourceOperation.DELETE_TABLE]: () => this.deleteTable(json?.table?.name),
+        [DataSourceOperation.DELETE_TABLE]: () =>
+          this.deleteTable(json?.table?.name),
       }
 
       const internalQueryMethod = handlers[json.endpoint.operation]
@@ -214,7 +221,7 @@ module GoogleSheetsModule {
     async createTable(name?: string) {
       try {
         await this.connect()
-        const sheet = await this.client.addSheet({ title: name });
+        const sheet = await this.client.addSheet({ title: name })
         return sheet
       } catch (err) {
         console.error("Error creating new table in google sheets", err)
@@ -239,7 +246,9 @@ module GoogleSheetsModule {
           }
           await sheet.setHeaderRow(headers)
         } else {
-          let newField = Object.keys(table.schema).find(key => !sheet.headerValues.includes(key))
+          let newField = Object.keys(table.schema).find(
+            key => !sheet.headerValues.includes(key)
+          )
           await sheet.setHeaderRow([...sheet.headerValues, newField])
         }
       } catch (err) {

packages/server/src/utilities/workerRequests.js

@@ -71,7 +71,7 @@ exports.getDeployedApps = async () => {
     for (let [key, value] of Object.entries(json)) {
       if (value.url) {
         value.url = value.url.toLowerCase()
-        apps[key] = value
+        apps[key.toLowerCase()] = value
       }
     }
     return apps

packages/string-templates/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@budibase/string-templates",
-  "version": "1.0.41",
+  "version": "1.0.47",
   "description": "Handlebars wrapper for Budibase templating.",
   "main": "src/index.cjs",
   "module": "dist/bundle.mjs",

packages/string-templates/src/helpers/javascript.js

@@ -1,5 +1,6 @@
 const { atob } = require("../utilities")
 const { cloneDeep } = require("lodash/fp")
+const { LITERAL_MARKER } = require("../helpers/constants")
 
 // The method of executing JS scripts depends on the bundle being built.
 // This setter is used in the entrypoint (either index.cjs or index.mjs).
@@ -46,8 +47,9 @@ module.exports.processJS = (handlebars, context) => {
       $: path => getContextValue(path, cloneDeep(context)),
     }
 
-    // Create a sandbox with out context and run the JS
+    // Create a sandbox with our context and run the JS
-    return runJS(js, sandboxContext)
+    const res = { data: runJS(js, sandboxContext) }
+    return `{{${LITERAL_MARKER} js_result-${JSON.stringify(res)}}}`
   } catch (error) {
     return "Error while executing JS"
   }

packages/string-templates/src/index.js

@@ -112,9 +112,10 @@ module.exports.processStringSync = (string, context, opts) => {
     const template = instance.compile(string, {
       strict: false,
     })
+    const now = Math.floor(Date.now() / 1000) * 1000
     return processors.postprocess(
       template({
-        now: new Date().toISOString(),
+        now: new Date(now).toISOString(),
         ...context,
       })
     )

packages/string-templates/src/processors/postprocessor.js

@@ -36,6 +36,11 @@ module.exports.processors = [
         return value === "true"
       case "object":
         return JSON.parse(value)
+      case "js_result":
+        // We use the literal helper to process the result of JS expressions
+        // as we want to be able to return any types.
+        // We wrap the value in an abject to be able to use undefined properly.
+        return JSON.parse(value).data
     }
     return value
   }),

packages/string-templates/test/javascript.spec.js

@@ -7,7 +7,7 @@ const processJS = (js, context) => {
 describe("Test the JavaScript helper", () => {
   it("should execute a simple expression", () => {
     const output = processJS(`return 1 + 2`)
-    expect(output).toBe("3")
+    expect(output).toBe(3)
   })
 
   it("should be able to use primitive bindings", () => {
@@ -50,6 +50,52 @@ describe("Test the JavaScript helper", () => {
     expect(output).toBe("shazbat")
   })
 
+  it("should be able to return an object", () => {
+    const output = processJS(`return $("foo")`, {
+      foo: {
+        bar: {
+          baz: "shazbat",
+        },
+      },
+    })
+    expect(output.bar.baz).toBe("shazbat")
+  })
+
+  it("should be able to return an array", () => {
+    const output = processJS(`return $("foo")`, {
+      foo: ["a", "b", "c"],
+    })
+    expect(output[2]).toBe("c")
+  })
+
+  it("should be able to return null", () => {
+    const output = processJS(`return $("foo")`, {
+      foo: null,
+    })
+    expect(output).toBe(null)
+  })
+
+  it("should be able to return undefined", () => {
+    const output = processJS(`return $("foo")`, {
+      foo: undefined,
+    })
+    expect(output).toBe(undefined)
+  })
+
+  it("should be able to return 0", () => {
+    const output = processJS(`return $("foo")`, {
+      foo: 0,
+    })
+    expect(output).toBe(0)
+  })
+
+  it("should be able to return an empty string", () => {
+    const output = processJS(`return $("foo")`, {
+      foo: "",
+    })
+    expect(output).toBe("")
+  })
+
   it("should be able to use a deep array binding", () => {
     const output = processJS(`return $("foo.0.bar")`, {
       foo: [

packages/worker/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@budibase/worker",
   "email": "hi@budibase.com",
-  "version": "1.0.41",
+  "version": "1.0.47",
   "description": "Budibase background service",
   "main": "src/index.js",
   "repository": {
@@ -29,8 +29,8 @@
   "author": "Budibase",
   "license": "GPL-3.0",
   "dependencies": {
-    "@budibase/backend-core": "^1.0.41",
+    "@budibase/backend-core": "^1.0.47",
-    "@budibase/string-templates": "^1.0.41",
+    "@budibase/string-templates": "^1.0.47",
     "@koa/router": "^8.0.0",
     "@sentry/node": "^6.0.0",
     "@techpass/passport-openidconnect": "^0.3.0",