From 70788d42b7a3d688992dfa8958e65c347c25f455 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 20 Jan 2022 16:01:09 +0000 Subject: [PATCH 01/16] Share validation between app modals, add yup based validation framework, add url to app modals --- packages/builder/cypress/support/commands.js | 9 +- .../support/queryLevelTransformerFunction.js | 13 +- .../queryLevelTransformerFunctionWithData.js | 21 +- .../components/start/CreateAppModal.svelte | 194 ++++++------------ .../components/start/UpdateAppModal.svelte | 137 ++++--------- packages/builder/src/constants/index.js | 3 + .../builder/src/helpers/validation/yup/app.js | 79 +++++++ .../src/helpers/validation/yup/index.js | 58 ++++++ .../pages/builder/portal/apps/index.svelte | 6 +- packages/server/src/threads/query.js | 5 + 10 files changed, 282 insertions(+), 243 deletions(-) create mode 100644 packages/builder/src/helpers/validation/yup/app.js create mode 100644 packages/builder/src/helpers/validation/yup/index.js diff --git a/packages/builder/cypress/support/commands.js b/packages/builder/cypress/support/commands.js index 686d0ee9bf..e69bc8badc 100644 --- a/packages/builder/cypress/support/commands.js +++ b/packages/builder/cypress/support/commands.js @@ -430,13 +430,14 @@ Cypress.Commands.add("addDatasourceConfig", (datasource, skipFetch) => { // Click to fetch tables if (skipFetch) { cy.get(".spectrum-Dialog-grid").within(() => { - cy.get(".spectrum-Button").contains("Skip table fetch") + cy.get(".spectrum-Button") + .contains("Skip table fetch") .click({ force: true }) }) - } - else { + } else { cy.get(".spectrum-Dialog-grid").within(() => { - cy.get(".spectrum-Button").contains("Save and fetch tables") + cy.get(".spectrum-Button") + .contains("Save and fetch tables") .click({ force: true }) cy.wait(1000) }) diff --git a/packages/builder/cypress/support/queryLevelTransformerFunction.js b/packages/builder/cypress/support/queryLevelTransformerFunction.js index 0c099df1a0..8ad3e573d8 100644 --- a/packages/builder/cypress/support/queryLevelTransformerFunction.js +++ b/packages/builder/cypress/support/queryLevelTransformerFunction.js @@ -1,12 +1,13 @@ +// eslint-disable-next-line no-undef const breweries = data const totals = {} -for (let brewery of breweries) - {const state = brewery.state - if (totals[state] == null) - {totals[state] = 1 - } else - {totals[state]++ +for (let brewery of breweries) { + const state = brewery.state + if (totals[state] == null) { + totals[state] = 1 + } else { + totals[state]++ } } const entries = Object.entries(totals) diff --git a/packages/builder/cypress/support/queryLevelTransformerFunctionWithData.js b/packages/builder/cypress/support/queryLevelTransformerFunctionWithData.js index f7d9631ef9..50f3da7099 100644 --- a/packages/builder/cypress/support/queryLevelTransformerFunctionWithData.js +++ b/packages/builder/cypress/support/queryLevelTransformerFunctionWithData.js @@ -1,15 +1,16 @@ +// eslint-disable-next-line no-undef const breweries = data const totals = {} -for (let brewery of breweries) - {const state = brewery.state - if (totals[state] == null) - {totals[state] = 1 - } else - {totals[state]++ +for (let brewery of breweries) { + const state = brewery.state + if (totals[state] == null) { + totals[state] = 1 + } else { + totals[state]++ } } -const stateCodes = - {texas: "tx", +const stateCodes = { + texas: "tx", colorado: "co", florida: "fl", iwoa: "ia", @@ -24,7 +25,7 @@ const stateCodes = ohio: "oh", } const entries = Object.entries(totals) -return entries.map(([state, count]) => - {stateCodes[state.toLowerCase()] +return entries.map(([state, count]) => { + stateCodes[state.toLowerCase()] return { state, count, flag: "http://flags.ox3.in/svg/us/${stateCode}.svg" } }) diff --git a/packages/builder/src/components/start/CreateAppModal.svelte b/packages/builder/src/components/start/CreateAppModal.svelte index 178588f608..ec56e15080 100644 --- a/packages/builder/src/components/start/CreateAppModal.svelte +++ b/packages/builder/src/components/start/CreateAppModal.svelte @@ -4,98 +4,45 @@ import { notifications, Input, ModalContent, Dropzone } from "@budibase/bbui" import { store, automationStore, hostingStore } from "builderStore" import { admin, auth } from "stores/portal" - import { string, mixed, object } from "yup" import api, { get, post } from "builderStore/api" import analytics, { Events } from "analytics" import { onMount } from "svelte" - import { capitalise } from "helpers" import { goto } from "@roxi/routify" - import { APP_NAME_REGEX } from "constants" - import TemplateList from "./TemplateList.svelte" + import { createValidationStore } from "helpers/validation/yup" + import * as appValidation from "helpers/validation/yup/app" export let template - export let inline - const values = writable({ name: null }) - const errors = writable({}) - const touched = writable({}) - const validator = { - name: string() - .trim() - .required("Your application must have a name") - .matches( - APP_NAME_REGEX, - "App name must be letters, numbers and spaces only" - ), - file: template?.fromFile - ? mixed().required("Please choose a file to import") - : null, - } - - let submitting = false - let valid = false - let initialTemplateInfo = template?.fromFile || template?.key - - $: checkValidity($values, validator) - $: showTemplateSelection = !template && !initialTemplateInfo + const values = writable({ name: "", url: null }) + const validation = createValidationStore() + $: validation.check($values) onMount(async () => { - await hostingStore.actions.fetchDeployedApps() - const existingAppNames = svelteGet(hostingStore).deployedAppNames - validator.name = string() - .trim() - .required("Your application must have a name") - .matches(APP_NAME_REGEX, "App name must be letters and numbers only") - .test( - "non-existing-app-name", - "Another app with the same name already exists", - value => { - return !existingAppNames.some( - appName => appName.toLowerCase() === value.toLowerCase() - ) - } - ) + await setupValidation() }) - const checkValidity = async (values, validator) => { - const obj = object().shape(validator) - Object.keys(validator).forEach(key => ($errors[key] = null)) - if (template?.fromFile && values.file == null) { - valid = false - return - } - - try { - await obj.validate(values, { abortEarly: false }) - } catch (validationErrors) { - validationErrors.inner.forEach(error => { - $errors[error.path] = capitalise(error.message) - }) - } - - valid = await obj.isValid(values) + const setupValidation = async () => { + await hostingStore.actions.fetchDeployedApps() + const apps = svelteGet(hostingStore).deployedApps + appValidation.name(validation, { apps }) + appValidation.url(validation, { apps }) + appValidation.file(validation, { template }) + // init validation + validation.check($values) } async function createNewApp() { - const templateToUse = Object.keys(template).length === 0 ? null : template - submitting = true - - // Check a template exists if we are important - if (templateToUse?.fromFile && !$values.file) { - $errors.file = "Please choose a file to import" - valid = false - submitting = false - return false - } - try { // Create form data to create app let data = new FormData() data.append("name", $values.name.trim()) - data.append("useTemplate", templateToUse != null) - if (templateToUse) { - data.append("templateName", templateToUse.name) - data.append("templateKey", templateToUse.key) + if ($values.url) { + data.append("url", $values.url.trim()) + } + data.append("useTemplate", template != null) + if (template) { + data.append("templateName", template.name) + data.append("templateKey", template.key) data.append("templateFile", $values.file) } @@ -109,7 +56,7 @@ analytics.captureEvent(Events.APP.CREATED, { name: $values.name, appId: appJson.instance._id, - templateToUse, + templateToUse: template, }) // Select Correct Application/DB in prep for creating user @@ -137,7 +84,6 @@ } catch (error) { console.error(error) notifications.error(error) - submitting = false } } @@ -145,60 +91,50 @@ template = null await auth.setInitInfo({}) } + + // auto add slash to url + $: { + if ($values.url && !$values.url.startsWith("/")) { + $values.url = `/${$values.url}` + } + } -{#if showTemplateSelection} - { - template = {} - return false - }} - showCancelButton={!inline} - showCloseIcon={!inline} - > - { - if (!selected) { - template = useImport ? { fromFile: true } : {} - return - } - template = selected + + {#if template?.fromFile} + { + $values.file = e.detail?.[0] + $validation.touched.file = true }} /> - -{:else} - - {#if template?.fromFile} - { - $values.file = e.detail?.[0] - $touched.file = true - }} - /> - {/if} - ($touched.name = true)} - label="Name" - placeholder={$auth.user.firstName - ? `${$auth.user.firstName}'s app` - : "My app"} - /> - -{/if} + {/if} + ($validation.touched.name = true)} + label="Name" + placeholder={$auth.user.firstName + ? `${$auth.user.firstName}s app` + : "My app"} + /> + ($validation.touched.url = true)} + label="URL" + placeholder={$values.name + ? "/" + encodeURIComponent($values.name).toLowerCase() + : "/"} + /> + diff --git a/packages/builder/src/components/start/UpdateAppModal.svelte b/packages/builder/src/components/start/UpdateAppModal.svelte index 432b13c7c3..8c76001740 100644 --- a/packages/builder/src/components/start/UpdateAppModal.svelte +++ b/packages/builder/src/components/start/UpdateAppModal.svelte @@ -1,120 +1,71 @@ - - - Update the name of your app. - ($touched.name = true)} - on:change={() => (dirty = true)} - label="Name" - /> - - + + Update the name of your app. + ($validation.touched.name = true)} + label="Name" + /> + ($validation.touched.url = true)} + label="URL" + placeholder={$values.name + ? "/" + encodeURIComponent($values.name).toLowerCase() + : "/"} + /> + diff --git a/packages/builder/src/constants/index.js b/packages/builder/src/constants/index.js index 04f12672e8..6b7aafdfa9 100644 --- a/packages/builder/src/constants/index.js +++ b/packages/builder/src/constants/index.js @@ -36,4 +36,7 @@ export const LAYOUT_NAMES = { export const BUDIBASE_INTERNAL_DB = "bb_internal" +// one or more word characters and whitespace export const APP_NAME_REGEX = /^[\w\s]+$/ +// zero or more non-whitespace characters +export const APP_URL_REGEX = /^\S*$/ diff --git a/packages/builder/src/helpers/validation/yup/app.js b/packages/builder/src/helpers/validation/yup/app.js new file mode 100644 index 0000000000..4be6e3cbb0 --- /dev/null +++ b/packages/builder/src/helpers/validation/yup/app.js @@ -0,0 +1,79 @@ +import { string, mixed } from "yup" +import { APP_NAME_REGEX, APP_URL_REGEX } from "constants" + +export const name = (validation, { apps, currentApp } = { apps: [] }) => { + let existingApps = Object.values(apps) + validation.addValidator( + "name", + string() + .required("Your application must have a name") + .matches(APP_NAME_REGEX, "App name must be letters and numbers only") + .test( + "non-existing-app-name", + "Another app with the same name already exists", + value => { + if (!value) { + return true + } + if (currentApp) { + // filter out the current app if present + existingApps = existingApps + // match the id format of the current app (remove 'app_') + .map(app => ({ ...app, appId: app.appId.substring(4) })) + .filter(app => app.appId !== currentApp.appId) + } + return !existingApps + .map(app => app.name) + .some(appName => appName.toLowerCase() === value.toLowerCase()) + } + ) + ) +} + +export const url = (validation, { apps, currentApp } = { apps: {} }) => { + let existingApps = Object.values(apps) + validation.addValidator( + "url", + string() + .nullable() + .matches(APP_URL_REGEX, "App URL must not contain spaces") + .test( + "non-existing-app-url", + "Another app with the same URL already exists", + value => { + // url is nullable + if (!value) { + return true + } + if (currentApp) { + existingApps = existingApps + // match the id format of the current app (remove 'app_') + .map(app => ({ ...app, appId: app.appId.substring(4) })) + // filter out the current app if present + .filter(app => app.appId !== currentApp.appId) + } + return !existingApps + .map(app => app.url) + .some(appUrl => appUrl.toLowerCase() === value.toLowerCase()) + } + ) + .test("start-with-slash", "Not a valid URL", value => { + // url is nullable + if (!value) { + return true + } + return value.length > 1 + }) + ) +} + +export const file = (validation, { template } = {}) => { + const templateToUse = + template && Object.keys(template).length === 0 ? null : template + validation.addValidator( + "file", + templateToUse?.fromFile + ? mixed().required("Please choose a file to import") + : null + ) +} diff --git a/packages/builder/src/helpers/validation/yup/index.js b/packages/builder/src/helpers/validation/yup/index.js new file mode 100644 index 0000000000..08b8f5a56d --- /dev/null +++ b/packages/builder/src/helpers/validation/yup/index.js @@ -0,0 +1,58 @@ +import { capitalise } from "helpers" +import { object } from "yup" +import { writable, get } from "svelte/store" + +export const createValidationStore = () => { + const DEFAULT = { + errors: {}, + touched: {}, + valid: false, + } + + const validator = {} + const validation = writable(DEFAULT) + + const addValidator = (propertyName, propertyValidator) => { + if (!propertyValidator || !propertyName) { + return + } + validator[propertyName] = propertyValidator + } + + const check = async values => { + const obj = object().shape(validator) + // clear the previous errors + const properties = Object.keys(validator) + properties.forEach(property => (get(validation).errors[property] = null)) + try { + await obj.validate(values, { abortEarly: false }) + } catch (error) { + error.inner.forEach(err => { + validation.update(store => { + store.errors[err.path] = capitalise(err.message) + return store + }) + }) + } + + let valid + if (properties.length) { + valid = await obj.isValid(values) + } else { + // don't say valid until validators have been loaded + valid = false + } + + validation.update(store => { + store.valid = valid + return store + }) + } + + return { + subscribe: validation.subscribe, + set: validation.set, + check, + addValidator, + } +} diff --git a/packages/builder/src/pages/builder/portal/apps/index.svelte b/packages/builder/src/pages/builder/portal/apps/index.svelte index ac10b5317f..c1aca535f8 100644 --- a/packages/builder/src/pages/builder/portal/apps/index.svelte +++ b/packages/builder/src/pages/builder/portal/apps/index.svelte @@ -412,6 +412,11 @@ > + + + + + {selectedApp?.name}? -