code review

This commit is contained in:
Martin McKeaveney 2022-01-26 17:14:47 +01:00
commit efde072e70
26 changed files with 167 additions and 106 deletions

View File

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

View File

@ -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",

View File

@ -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

View File

@ -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 = {
auth: {
type: "google",
...tokens,
},
if (!datasource.config) {
datasource.config = {}
}
datasource.config.auth = { type: "google", ...tokens }
await db.put(datasource)
ctx.redirect(
`/builder/app/${authStateCookie.appId}/data/datasource/${authStateCookie.datasourceId}`

View File

@ -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
}

View File

@ -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",

View File

@ -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/client": "^1.0.41",
"@budibase/bbui": "^1.0.47",
"@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",

View File

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

View File

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

View File

@ -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)
bindings = bindings
queryBindings.splice(idx, 1)
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(
bindableOptions,
valueToParse
)
customParams[param] = readableToRuntimeBinding(bindings, 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)} />

View File

@ -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}

View File

@ -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": {

View File

@ -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"

View File

@ -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

View File

@ -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)

View File

@ -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/client": "^1.0.41",
"@budibase/string-templates": "^1.0.41",
"@budibase/backend-core": "^1.0.47",
"@budibase/client": "^1.0.47",
"@budibase/string-templates": "^1.0.47",
"@bull-board/api": "^3.7.0",
"@bull-board/koa": "^3.7.0",
"@elastic/elasticsearch": "7.10.0",

View File

@ -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 = newVariables
datasource.config.dynamicVariables = dynamicVariables.filter(
dv => dv.queryId !== queryId
)
await db.put(datasource)
// invalidate the deleted variables

View File

@ -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 (
(type === FieldTypes.OPTIONS || type === FieldTypes.ARRAY) &&
constraints.inclusion
) {
if (type === FieldTypes.OPTIONS && 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 {

View File

@ -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) {

View File

@ -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

View File

@ -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",

View File

@ -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
return runJS(js, sandboxContext)
// Create a sandbox with our context and run the JS
const res = { data: runJS(js, sandboxContext) }
return `{{${LITERAL_MARKER} js_result-${JSON.stringify(res)}}}`
} catch (error) {
return "Error while executing JS"
}

View File

@ -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,
})
)

View File

@ -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
}),

View File

@ -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: [

View File

@ -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/string-templates": "^1.0.41",
"@budibase/backend-core": "^1.0.47",
"@budibase/string-templates": "^1.0.47",
"@koa/router": "^8.0.0",
"@sentry/node": "^6.0.0",
"@techpass/passport-openidconnect": "^0.3.0",