From f100ae42fd91fce40ce99ef09ee701ee5cfb7ca3 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Fri, 21 Jan 2022 09:10:59 +0000 Subject: [PATCH] Add more work on API refactor in builder --- packages/builder/src/builderStore/api.js | 11 ++- .../src/components/common/Dropzone.svelte | 10 ++- .../src/components/deploy/DeployModal.svelte | 2 +- .../src/components/deploy/RevertModal.svelte | 2 +- .../src/components/deploy/VersionModal.svelte | 33 ++----- .../components/start/CreateAppModal.svelte | 36 +++----- .../admin/_components/ImportAppsModal.svelte | 21 ++--- .../src/pages/builder/admin/index.svelte | 8 +- .../pages/builder/portal/apps/index.svelte | 63 ++++---------- .../builder/portal/manage/email/index.svelte | 77 ++++++++--------- packages/builder/src/stores/portal/admin.js | 45 +++++----- packages/builder/src/stores/portal/oidc.js | 21 ++--- packages/client/src/api.js | 16 +++- packages/frontend-core/src/api/app.js | 77 ++++++++++++++++- packages/frontend-core/src/api/attachments.js | 14 +++ packages/frontend-core/src/api/auth.js | 85 ++++++++++++++++++- packages/frontend-core/src/api/index.js | 33 ++++--- 17 files changed, 343 insertions(+), 211 deletions(-) diff --git a/packages/builder/src/builderStore/api.js b/packages/builder/src/builderStore/api.js index 085c176f88..bd7cccbc5a 100644 --- a/packages/builder/src/builderStore/api.js +++ b/packages/builder/src/builderStore/api.js @@ -14,16 +14,25 @@ export const API = createAPIClient({ }, onError: error => { - const { url, message, status } = error + const { url, message, status, method, handled } = error || {} // Log all API errors to Sentry // analytics.captureException(error) + // Log any errors that we haven't manually handled + if (!handled) { + console.error("Unhandled error from API client", error) + return + } + // Show a notification for any errors if (message) { notifications.error(`Error fetching ${url}: ${message}`) } + // Log all errors to console + console.error(`HTTP ${status} on ${method}:${url}:\n\t${message}`) + // Logout on 403's if (status === 403) { // Don't do anything if fetching templates. diff --git a/packages/builder/src/components/common/Dropzone.svelte b/packages/builder/src/components/common/Dropzone.svelte index 328cd31f23..9a86554b49 100644 --- a/packages/builder/src/components/common/Dropzone.svelte +++ b/packages/builder/src/components/common/Dropzone.svelte @@ -1,6 +1,6 @@ diff --git a/packages/builder/src/components/deploy/DeployModal.svelte b/packages/builder/src/components/deploy/DeployModal.svelte index 4a86394c72..9f2aae56c5 100644 --- a/packages/builder/src/components/deploy/DeployModal.svelte +++ b/packages/builder/src/components/deploy/DeployModal.svelte @@ -9,7 +9,7 @@ async function deployApp() { try { - await API.deployApp() + await API.deployAppChanges() analytics.captureEvent(Events.APP.PUBLISHED, { appId: $store.appId, }) diff --git a/packages/builder/src/components/deploy/RevertModal.svelte b/packages/builder/src/components/deploy/RevertModal.svelte index 4207fcad00..717c55f05e 100644 --- a/packages/builder/src/components/deploy/RevertModal.svelte +++ b/packages/builder/src/components/deploy/RevertModal.svelte @@ -16,7 +16,7 @@ const revert = async () => { try { - await API.revertApp(appId) + await API.revertAppChanges(appId) // Reset frontend state after revert const applicationPkg = await API.fetchAppPackage(appId) diff --git a/packages/builder/src/components/deploy/VersionModal.svelte b/packages/builder/src/components/deploy/VersionModal.svelte index 0fb061face..9707517c54 100644 --- a/packages/builder/src/components/deploy/VersionModal.svelte +++ b/packages/builder/src/components/deploy/VersionModal.svelte @@ -8,7 +8,7 @@ Button, } from "@budibase/bbui" import { store } from "builderStore" - import api from "builderStore/api" + import { API } from "api" import clientPackage from "@budibase/client/package.json" let updateModal @@ -18,26 +18,17 @@ $: revertAvailable = $store.revertableVersion != null const refreshAppPackage = async () => { - const applicationPkg = await api.get( - `/api/applications/${appId}/appPackage` - ) - const pkg = await applicationPkg.json() - if (applicationPkg.ok) { + try { + const pkg = await API.fetchAppPackage(appId) await store.actions.initialise(pkg) - } else { - throw new Error(pkg) + } catch (error) { + notifications.error("Error fetching app package") } } const update = async () => { try { - const response = await api.post( - `/api/applications/${appId}/client/update` - ) - const json = await response.json() - if (response.status !== 200) { - throw json.message - } + await API.updateAppClientVersion(appId) // Don't wait for the async refresh, since this causes modal flashing refreshAppPackage() @@ -47,23 +38,17 @@ } catch (err) { notifications.error(`Error updating app: ${err}`) } + updateModal.hide() } const revert = async () => { try { - const revertableVersion = $store.revertableVersion - const response = await api.post( - `/api/applications/${appId}/client/revert` - ) - const json = await response.json() - if (response.status !== 200) { - throw json.message - } + await API.revertAppClientVersion(appId) // Don't wait for the async refresh, since this causes modal flashing refreshAppPackage() notifications.success( - `App reverted successfully to version ${revertableVersion}` + `App reverted successfully to version ${$store.revertableVersion}` ) } catch (err) { notifications.error(`Error reverting app: ${err}`) diff --git a/packages/builder/src/components/start/CreateAppModal.svelte b/packages/builder/src/components/start/CreateAppModal.svelte index 60065b6eef..ebb24e9e2f 100644 --- a/packages/builder/src/components/start/CreateAppModal.svelte +++ b/packages/builder/src/components/start/CreateAppModal.svelte @@ -5,7 +5,7 @@ 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 { API } from "api" import analytics, { Events } from "analytics" import { onMount } from "svelte" import { capitalise } from "helpers" @@ -99,40 +99,24 @@ } // Create App - const appResp = await post("/api/applications", data, {}) - const appJson = await appResp.json() - if (!appResp.ok) { - throw new Error(appJson.message) - } - + const createdApp = await API.createApp(data) analytics.captureEvent(Events.APP.CREATED, { name: $values.name, - appId: appJson.instance._id, + appId: createdApp.instance._id, templateToUse, }) // Select Correct Application/DB in prep for creating user - const applicationPkg = await get( - `/api/applications/${appJson.instance._id}/appPackage` - ) - const pkg = await applicationPkg.json() - if (applicationPkg.ok) { - await store.actions.initialise(pkg) - await automationStore.actions.fetch() - // update checklist - incase first app - await admin.init() - } else { - throw new Error(pkg) - } + const pkg = await API.fetchAppPackage(createdApp.instance._id) + await store.actions.initialise(pkg) + await automationStore.actions.fetch() + // Update checklist - in case first app + await admin.init() // Create user - const user = { - roleId: $values.roleId, - } - const userResp = await api.post(`/api/users/metadata/self`, user) - await userResp.json() + await API.updateOwnMetadata({ roleId: $values.roleId }) await auth.setInitInfo({}) - $goto(`/builder/app/${appJson.instance._id}`) + $goto(`/builder/app/${createdApp.instance._id}`) } catch (error) { console.error(error) notifications.error(error) diff --git a/packages/builder/src/pages/builder/admin/_components/ImportAppsModal.svelte b/packages/builder/src/pages/builder/admin/_components/ImportAppsModal.svelte index de29e11301..182df63967 100644 --- a/packages/builder/src/pages/builder/admin/_components/ImportAppsModal.svelte +++ b/packages/builder/src/pages/builder/admin/_components/ImportAppsModal.svelte @@ -1,6 +1,6 @@ @@ -36,10 +31,10 @@ onConfirm={importApps} disabled={!value.file} > - Please upload the file that was exported from your Cloud environment to get - started + + Please upload the file that was exported from your Cloud environment to get + started + { try { - const response = await del(`/api/dev/${app.devId}/lock`) - if (response.status !== 200) { - const json = await response.json() - throw json.message - } + await API.releaseAppLock(app.devId) await apps.load() notifications.success("Lock released successfully") } catch (err) { - notifications.error(`Error releasing lock: ${err}`) + notifications.error("Error releasing lock") } } diff --git a/packages/builder/src/pages/builder/portal/manage/email/index.svelte b/packages/builder/src/pages/builder/portal/manage/email/index.svelte index 5a78623b81..4ef59d2daa 100644 --- a/packages/builder/src/pages/builder/portal/manage/email/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/email/index.svelte @@ -14,7 +14,7 @@ Checkbox, } from "@budibase/bbui" import { email } from "stores/portal" - import api from "builderStore/api" + import { API } from "api" import { cloneDeep } from "lodash/fp" import analytics, { Events } from "analytics" @@ -54,55 +54,48 @@ delete smtp.config.auth } // Save your SMTP config - const response = await api.post(`/api/global/configs`, smtp) - - if (response.status !== 200) { - const error = await response.text() - let message - try { - message = JSON.parse(error).message - } catch (err) { - message = error - } - notifications.error(`Failed to save email settings, reason: ${message}`) - } else { - const json = await response.json() - smtpConfig._rev = json._rev - smtpConfig._id = json._id - notifications.success(`Settings saved.`) + try { + const savedConfig = await API.saveConfig(smtp) + smtpConfig._rev = savedConfig._rev + smtpConfig._id = savedConfig._id + notifications.success(`Settings saved`) analytics.captureEvent(Events.SMTP.SAVED) + } catch (error) { + notifications.error( + `Failed to save email settings, reason: ${error?.message || "Unknown"}` + ) } } async function fetchSmtp() { loading = true - // fetch the configs for smtp - const smtpResponse = await api.get( - `/api/global/configs/${ConfigTypes.SMTP}` - ) - const smtpDoc = await smtpResponse.json() - - if (!smtpDoc._id) { - smtpConfig = { - type: ConfigTypes.SMTP, - config: { - secure: true, - }, + try { + // Fetch the configs for smtp + const smtpDoc = await API.getConfig(ConfigTypes.SMTP) + if (!smtpDoc._id) { + smtpConfig = { + type: ConfigTypes.SMTP, + config: { + secure: true, + }, + } + } else { + smtpConfig = smtpDoc } - } else { - smtpConfig = smtpDoc - } - loading = false - requireAuth = smtpConfig.config.auth != null - // always attach the auth for the forms purpose - - // this will be removed later if required - if (!smtpDoc.config) { - smtpDoc.config = {} - } - if (!smtpDoc.config.auth) { - smtpConfig.config.auth = { - type: "login", + loading = false + requireAuth = smtpConfig.config.auth != null + // Always attach the auth for the forms purpose - + // this will be removed later if required + if (!smtpDoc.config) { + smtpDoc.config = {} } + if (!smtpDoc.config.auth) { + smtpConfig.config.auth = { + type: "login", + } + } + } catch (error) { + notifications.error("Error fetching SMTP config") } } diff --git a/packages/builder/src/stores/portal/admin.js b/packages/builder/src/stores/portal/admin.js index d98eae8363..d97105fd46 100644 --- a/packages/builder/src/stores/portal/admin.js +++ b/packages/builder/src/stores/portal/admin.js @@ -1,5 +1,5 @@ import { writable, get } from "svelte/store" -import api from "builderStore/api" +import { API } from "api" import { auth } from "stores/portal" export function createAdminStore() { @@ -25,21 +25,19 @@ export function createAdminStore() { async function init() { try { const tenantId = get(auth).tenantId - const response = await api.get( - `/api/global/configs/checklist?tenantId=${tenantId}` - ) - const json = await response.json() - const totalSteps = Object.keys(json).length - const completedSteps = Object.values(json).filter(x => x?.checked).length - + const checklist = await API.getChecklist(tenantId) + const totalSteps = Object.keys(checklist).length + const completedSteps = Object.values(checklist).filter( + x => x?.checked + ).length await getEnvironment() admin.update(store => { store.loaded = true - store.checklist = json + store.checklist = checklist store.onboardingProgress = (completedSteps / totalSteps) * 100 return store }) - } catch (err) { + } catch (error) { admin.update(store => { store.checklist = null return store @@ -48,11 +46,15 @@ export function createAdminStore() { } async function checkImportComplete() { - const response = await api.get(`/api/cloud/import/complete`) - if (response.status === 200) { - const json = await response.json() + try { + const result = await API.checkImportComplete() admin.update(store => { - store.importComplete = json ? json.imported : false + store.importComplete = result ? result.imported : false + return store + }) + } catch (error) { + admin.update(store => { + store.importComplete = false return store }) } @@ -65,15 +67,14 @@ export function createAdminStore() { let accountPortalUrl = "" let isDev = false try { - const response = await api.get(`/api/system/environment`) - const json = await response.json() - multiTenancyEnabled = json.multiTenancy - cloud = json.cloud - disableAccountPortal = json.disableAccountPortal - accountPortalUrl = json.accountPortalUrl - isDev = json.isDev + const environment = await API.getEnvironment() + multiTenancyEnabled = environment.multiTenancy + cloud = environment.cloud + disableAccountPortal = environment.disableAccountPortal + accountPortalUrl = environment.accountPortalUrl + isDev = environment.isDev } catch (err) { - // just let it stay disabled + // Just let it stay disabled } admin.update(store => { store.multiTenancy = multiTenancyEnabled diff --git a/packages/builder/src/stores/portal/oidc.js b/packages/builder/src/stores/portal/oidc.js index 3e3a7048ca..751988bff6 100644 --- a/packages/builder/src/stores/portal/oidc.js +++ b/packages/builder/src/stores/portal/oidc.js @@ -1,5 +1,5 @@ import { writable, get } from "svelte/store" -import api from "builderStore/api" +import { API } from "api" import { auth } from "stores/portal" const OIDC_CONFIG = { @@ -14,16 +14,17 @@ export function createOidcStore() { async function init() { const tenantId = get(auth).tenantId - const res = await api.get( - `/api/global/configs/public/oidc?tenantId=${tenantId}` - ) - const json = await res.json() - - if (json.status === 400 || Object.keys(json).length === 0) { + try { + const config = await API.getOIDCConfig(tenantId) + if (Object.keys(config || {}).length) { + // Just use the first config for now. + // We will be support multiple logins buttons later on. + set(...config) + } else { + set(OIDC_CONFIG) + } + } catch (error) { set(OIDC_CONFIG) - } else { - // Just use the first config for now. We will be support multiple logins buttons later on. - set(...json) } } diff --git a/packages/client/src/api.js b/packages/client/src/api.js index e617dae00f..3ded21f473 100644 --- a/packages/client/src/api.js +++ b/packages/client/src/api.js @@ -18,9 +18,21 @@ export const API = createAPIClient({ // We could also log these to sentry. // Or we could check error.status and redirect to login on a 403 etc. onError: error => { - if (error.message) { - notificationStore.actions.error(error.message) + const { status, method, url, message, handled } = error || {} + + // Log any errors that we haven't manually handled + if (!handled) { + console.error("Unhandled error from API client", error) + return } + + // Notify all errors + if (message) { + notificationStore.actions.error(message) + } + + // Log all errors to console + console.error(`HTTP ${status} on ${method}:${url}:\n\t${message}`) }, // Patch certain endpoints with functionality specific to client apps diff --git a/packages/frontend-core/src/api/app.js b/packages/frontend-core/src/api/app.js index 4edd76d9b1..c1c4204c9d 100644 --- a/packages/frontend-core/src/api/app.js +++ b/packages/frontend-core/src/api/app.js @@ -25,7 +25,7 @@ export const buildAppEndpoints = API => ({ /** * Deploys the current app. */ - deployApp: async () => { + deployAppChanges: async () => { return await API.post({ url: "/api/deploy", }) @@ -35,12 +35,32 @@ export const buildAppEndpoints = API => ({ * Reverts an app to a previous version. * @param appId the app ID to revert */ - revertApp: async appId => { + revertAppChanges: async appId => { return await API.post({ url: `/api/dev/${appId}/revert`, }) }, + /** + * Updates an app's version of the client library. + * @param appId the app ID to update + */ + updateAppClientVersion: async appId => { + return await API.post({ + url: `/api/applications/${appId}/client/update`, + }) + }, + + /** + * Reverts an app's version of the client library to the previous version. + * @param appId the app ID to revert + */ + revertAppClientVersion: async appId => { + return await API.post({ + url: `/api/applications/${appId}/client/revert`, + }) + }, + /** * Gets a list of app deployments. */ @@ -49,4 +69,57 @@ export const buildAppEndpoints = API => ({ url: "/api/deployments", }) }, + + /** + * Creates an app. + * @param app the app to create + */ + createApp: async app => { + return await API.post({ + url: "/api/applications", + body: app, + }) + }, + + /** + * Imports an export of all apps. + * @param apps the FormData containing the apps to import + */ + importApps: async apps => { + return await API.post({ + url: "/api/cloud/import", + body: apps, + json: false, + }) + }, + + /** + * Unpublishes a published app. + * @param appId the production ID of the app to unpublish + */ + unpublishApp: async appId => { + return await API.delete({ + url: `/api/applications/${appId}?unpublish=1`, + }) + }, + + /** + * Deletes a dev app. + * @param appId the dev app ID to delete + */ + deleteApp: async appId => { + return await API.delete({ + url: `/api/applications/${appId}`, + }) + }, + + /** + * Releases the lock on a dev app. + * @param appId the dev app ID to unlock + */ + releaseAppLock: async appId => { + return await API.delete({ + url: `/api/dev/${appId}/lock`, + }) + }, }) diff --git a/packages/frontend-core/src/api/attachments.js b/packages/frontend-core/src/api/attachments.js index 5dd2582ea5..9a8325a433 100644 --- a/packages/frontend-core/src/api/attachments.js +++ b/packages/frontend-core/src/api/attachments.js @@ -1,6 +1,8 @@ export const buildAttachmentEndpoints = API => ({ /** * Uploads an attachment to the server. + * @param data the attachment to upload + * @param tableId the table ID to upload to */ uploadAttachment: async ({ data, tableId }) => { return await API.post({ @@ -9,4 +11,16 @@ export const buildAttachmentEndpoints = API => ({ json: false, }) }, + + /** + * Uploads an attachment to the server as a builder user from the builder. + * @param data the data to upload + */ + uploadBuilderAttachment: async data => { + return await API.post({ + url: "/api/attachments/process", + body: data, + json: false, + }) + }, }) diff --git a/packages/frontend-core/src/api/auth.js b/packages/frontend-core/src/api/auth.js index 55554df94a..4f61c155f0 100644 --- a/packages/frontend-core/src/api/auth.js +++ b/packages/frontend-core/src/api/auth.js @@ -31,6 +31,89 @@ export const buildAuthEndpoints = API => ({ * Fetches the currently logged in user object */ fetchSelf: async () => { - return await API.get({ url: "/api/self" }) + return await API.get({ + url: "/api/self", + }) + }, + + /** + * Updates the current user metadata. + * @param metadata the metadata to save + */ + updateOwnMetadata: async metadata => { + return await API.post({ + url: "/api/users/metadata/self", + body: metadata, + }) + }, + + /** + * Creates an admin user. + * @param adminUser the admin user to create + */ + createAdminUser: async adminUser => { + return await API.post({ + url: "/api/global/users/init", + body: adminUser, + }) + }, + + /** + * Saves a global config. + * @param config the config to save + */ + saveConfig: async config => { + return await API.post({ + url: "/api/global/configs", + body: config, + }) + }, + + /** + * Gets a global config of a certain type. + * @param type the type to fetch + */ + getConfig: async type => { + return await API.get({ + url: `/api/global/configs/${type}`, + }) + }, + + /** + * Gets the OIDC config for a certain tenant. + * @param tenantId the tenant ID to get the config for + */ + getOIDCConfig: async tenantId => { + return await API.get({ + url: `/api/global/configs/public/oidc?tenantId=${tenantId}`, + }) + }, + + /** + * Gets the checklist for a specific tenant. + * @param tenantId the tenant ID to get the checklist for + */ + getChecklist: async tenantId => { + return await API.get({ + url: `/api/global/configs/checklist?tenantId=${tenantId}`, + }) + }, + + /** + * TODO: find out what this is + */ + checkImportComplete: async () => { + return await API.get({ + url: "/api/cloud/import/complete", + }) + }, + + /** + * Gets the current environment details. + */ + getEnvironment: async () => { + return await API.get({ + url: "/api/system/environment", + }) }, }) diff --git a/packages/frontend-core/src/api/index.js b/packages/frontend-core/src/api/index.js index f1c12dc941..e1cf2f3619 100644 --- a/packages/frontend-core/src/api/index.js +++ b/packages/frontend-core/src/api/index.js @@ -30,7 +30,9 @@ export const createAPIClient = config => { } // Generates an error object from an API response - const makeErrorFromResponse = async response => { + const makeErrorFromResponse = async (response, method) => { + console.log("making error from", response) + // Try to read a message from the error let message = response.statusText try { @@ -47,6 +49,8 @@ export const createAPIClient = config => { message, status: response.status, url: response.url, + method, + handled: true, } } @@ -56,6 +60,8 @@ export const createAPIClient = config => { message, status: 400, url: "", + method: "", + handled: true, } } @@ -110,11 +116,7 @@ export const createAPIClient = config => { return null } } else { - const error = await makeErrorFromResponse(response) - if (config?.onError) { - config.onError(error) - } - throw error + throw await makeErrorFromResponse(response, method) } } @@ -134,14 +136,21 @@ export const createAPIClient = config => { return await cache[identifier] } - // Constructs an API call function for a particular HTTP method. + // Constructs an API call function for a particular HTTP method const requestApiCall = method => async params => { - let { url, cache = false, external = false } = params - if (!external) { - url = `/${url}`.replace("//", "/") + try { + let { url, cache = false, external = false } = params + if (!external) { + url = `/${url}`.replace("//", "/") + } + const enrichedParams = { ...params, method, url } + return await (cache ? makeCachedApiCall : makeApiCall)(enrichedParams) + } catch (error) { + if (config?.onError) { + config.onError(error) + } + throw error } - const enrichedParams = { ...params, method, url } - return await (cache ? makeCachedApiCall : makeApiCall)(enrichedParams) } // Build the underlying core API methods