From 23b5c70f51c3d70db394fc29a62776c4c43add01 Mon Sep 17 00:00:00 2001 From: Lukasz Ostrowski Date: Tue, 23 May 2023 11:04:52 +0200 Subject: [PATCH] Extract part of Semver compatibility logic to shared package and implement in Invoices and Taxes (#488) * Extract semver compatibility logic to shared package and implement it in taxes * Move semver checking package to packages/shared * Update lock * Apply suggestions from code review Co-authored-by: Adrian Pilarczyk * Improve error message * Fix lockfile --------- Co-authored-by: Adrian Pilarczyk --- .changeset/chatty-melons-cover.md | 5 ++ .changeset/famous-onions-jam.md | 9 +++ .changeset/purple-pots-sparkle.md | 5 ++ apps/invoices/package.json | 2 - apps/invoices/src/pages/api/register.ts | 3 +- apps/taxes/saleor-app.ts | 2 + apps/taxes/src/pages/api/manifest.ts | 2 + apps/taxes/src/pages/api/register.ts | 56 ++++++++++++++++++- packages/shared/index.ts | 1 + packages/shared/package.json | 8 ++- ...or-version-compatibility-validator.test.ts | 0 .../saleor-version-compatibility-validator.ts | 7 +-- packages/shared/src/setup-tests.ts | 6 ++ packages/shared/vitest.config.ts | 12 ++++ pnpm-lock.yaml | 53 +++++++++++------- 15 files changed, 140 insertions(+), 31 deletions(-) create mode 100644 .changeset/chatty-melons-cover.md create mode 100644 .changeset/famous-onions-jam.md create mode 100644 .changeset/purple-pots-sparkle.md rename {apps/invoices/src/lib => packages/shared/src}/saleor-version-compatibility-validator.test.ts (100%) rename {apps/invoices/src/lib => packages/shared/src}/saleor-version-compatibility-validator.ts (69%) create mode 100644 packages/shared/src/setup-tests.ts create mode 100644 packages/shared/vitest.config.ts diff --git a/.changeset/chatty-melons-cover.md b/.changeset/chatty-melons-cover.md new file mode 100644 index 0000000..6a44551 --- /dev/null +++ b/.changeset/chatty-melons-cover.md @@ -0,0 +1,5 @@ +--- +"@saleor/apps-shared": minor +--- + +Add SaleorCompatibilityValidator util that compares semver version of the app and Saleor's diff --git a/.changeset/famous-onions-jam.md b/.changeset/famous-onions-jam.md new file mode 100644 index 0000000..8a837ed --- /dev/null +++ b/.changeset/famous-onions-jam.md @@ -0,0 +1,9 @@ +--- +"saleor-app-taxes": minor +--- + +Set minimum Saleor version where app can be installed (3.9). + +Previously, app could have been installed in any Saleor, but if required taxes APIs were missing, app would crash + +Now, Saleor will reject installation if possible. If Saleor can't do it, App will check Saleor version during installation and fail it if version doesn't match \ No newline at end of file diff --git a/.changeset/purple-pots-sparkle.md b/.changeset/purple-pots-sparkle.md new file mode 100644 index 0000000..af24956 --- /dev/null +++ b/.changeset/purple-pots-sparkle.md @@ -0,0 +1,5 @@ +--- +"saleor-app-invoices": patch +--- + +Moved Semver compatibility checking to shared package, removed semver library diff --git a/apps/invoices/package.json b/apps/invoices/package.json index 546ef91..4f1951d 100644 --- a/apps/invoices/package.json +++ b/apps/invoices/package.json @@ -39,7 +39,6 @@ "react": "18.2.0", "react-dom": "18.2.0", "react-hook-form": "^7.41.0", - "semver": "^7.3.8", "tiny-invariant": "^1.3.1", "urql": "^3.0.3", "usehooks-ts": "^2.9.1", @@ -59,7 +58,6 @@ "@types/react": "^18.0.27", "@types/react-dom": "^18.0.10", "@types/rimraf": "^3.0.2", - "@types/semver": "^7.3.13", "@vitejs/plugin-react": "^3.0.0", "@vitest/coverage-c8": "^0.28.4", "dotenv": "^16.0.3", diff --git a/apps/invoices/src/pages/api/register.ts b/apps/invoices/src/pages/api/register.ts index 618349c..448c3ad 100644 --- a/apps/invoices/src/pages/api/register.ts +++ b/apps/invoices/src/pages/api/register.ts @@ -4,8 +4,7 @@ import { gql } from "urql"; import { createClient } from "../../lib/graphql"; import { SaleorVersionQuery } from "../../../generated/graphql"; -import { createLogger } from "@saleor/apps-shared"; -import { SaleorVersionCompatibilityValidator } from "../../lib/saleor-version-compatibility-validator"; +import { createLogger, SaleorVersionCompatibilityValidator } from "@saleor/apps-shared"; const allowedUrlsPattern = process.env.ALLOWED_DOMAIN_PATTERN; diff --git a/apps/taxes/saleor-app.ts b/apps/taxes/saleor-app.ts index 1142a1e..56e7c83 100644 --- a/apps/taxes/saleor-app.ts +++ b/apps/taxes/saleor-app.ts @@ -34,3 +34,5 @@ switch (process.env.APL) { export const saleorApp = new SaleorApp({ apl, }); + +export const REQUIRED_SALEOR_VERSION = ">=3.9 <4"; diff --git a/apps/taxes/src/pages/api/manifest.ts b/apps/taxes/src/pages/api/manifest.ts index ffa92ae..264789d 100644 --- a/apps/taxes/src/pages/api/manifest.ts +++ b/apps/taxes/src/pages/api/manifest.ts @@ -6,6 +6,7 @@ import { checkoutCalculateTaxesSyncWebhook } from "./webhooks/checkout-calculate import { orderCalculateTaxesSyncWebhook } from "./webhooks/order-calculate-taxes"; import { orderCreatedAsyncWebhook } from "./webhooks/order-created"; import { orderFulfilledAsyncWebhook } from "./webhooks/order-fulfilled"; +import { REQUIRED_SALEOR_VERSION } from "../../../saleor-app"; export default createManifestHandler({ async manifestFactory(context) { @@ -27,6 +28,7 @@ export default createManifestHandler({ supportUrl: "https://github.com/saleor/apps/discussions", author: "Saleor Commerce", dataPrivacyUrl: "https://saleor.io/legal/privacy/", + requiredSaleorVersion: REQUIRED_SALEOR_VERSION, }; return manifest; diff --git a/apps/taxes/src/pages/api/register.ts b/apps/taxes/src/pages/api/register.ts index 4596d14..da6bfd2 100644 --- a/apps/taxes/src/pages/api/register.ts +++ b/apps/taxes/src/pages/api/register.ts @@ -1,9 +1,21 @@ import { createAppRegisterHandler } from "@saleor/app-sdk/handlers/next"; -import { saleorApp } from "../../../saleor-app"; +import { REQUIRED_SALEOR_VERSION, saleorApp } from "../../../saleor-app"; +import { createLogger, SaleorVersionCompatibilityValidator } from "@saleor/apps-shared"; +import { createClient } from "../../lib/graphql"; +import { gql } from "urql"; +import { SaleorVersionQuery } from "../../../generated/graphql"; const allowedUrlsPattern = process.env.ALLOWED_DOMAIN_PATTERN; +const SaleorVersion = gql` + query SaleorVersion { + shop { + version + } + } +`; + /** * Required endpoint, called by Saleor to install app. * It will exchange tokens with app, so saleorApp.apl will contain token @@ -25,4 +37,46 @@ export default createAppRegisterHandler({ return true; }, ], + /** + * TODO Unify with all apps - shared code. Consider moving to app-sdk + */ + async onRequestVerified(req, { authData: { token, saleorApiUrl }, respondWithError }) { + const logger = createLogger({ + context: "onRequestVerified", + }); + + try { + const client = createClient(saleorApiUrl, async () => { + return { + token, + }; + }); + + const saleorVersion = await client + .query(SaleorVersion, {}) + .toPromise() + .then((res) => { + return res.data?.shop.version; + }); + + logger.debug({ saleorVersion }, "Received saleor version from Shop query"); + + if (!saleorVersion) { + throw new Error("Saleor Version couldnt be fetched from the API"); + } + + new SaleorVersionCompatibilityValidator(REQUIRED_SALEOR_VERSION).validateOrThrow( + saleorVersion + ); + } catch (e: unknown) { + const message = (e as Error)?.message ?? "Unknown error"; + + logger.debug({ message }, "Failed validating semver, will respond with error and status 400"); + + throw respondWithError({ + message: message, + status: 400, + }); + } + }, }); diff --git a/packages/shared/index.ts b/packages/shared/index.ts index ece0520..36795f8 100644 --- a/packages/shared/index.ts +++ b/packages/shared/index.ts @@ -3,3 +3,4 @@ export * from "./src/macaw-theme-provider/macaw-theme-provider"; export * from "./src/no-ssr-wrapper"; export * from "./src/use-dashboard-notification"; export * from "./src/logger"; +export * from "./src/saleor-version-compatibility-validator"; diff --git a/packages/shared/package.json b/packages/shared/package.json index 217c502..d4967af 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -3,7 +3,8 @@ "version": "1.5.1", "dependencies": { "pino": "^8.14.1", - "pino-pretty": "^10.0.0" + "pino-pretty": "^10.0.0", + "semver": "^7.5.1" }, "devDependencies": { "@material-ui/core": "^4.12.4", @@ -17,7 +18,10 @@ "eslint-config-saleor": "workspace:*", "next": "^13.3.0", "react": "^18.2.0", - "react-dom": "^18.2.0" + "react-dom": "^18.2.0", + "vite": "^4.3.1", + "vitest": "^0.30.1", + "@types/semver": "^7.5.0" }, "peerDependencies": { "next": "^13.3.0", diff --git a/apps/invoices/src/lib/saleor-version-compatibility-validator.test.ts b/packages/shared/src/saleor-version-compatibility-validator.test.ts similarity index 100% rename from apps/invoices/src/lib/saleor-version-compatibility-validator.test.ts rename to packages/shared/src/saleor-version-compatibility-validator.test.ts diff --git a/apps/invoices/src/lib/saleor-version-compatibility-validator.ts b/packages/shared/src/saleor-version-compatibility-validator.ts similarity index 69% rename from apps/invoices/src/lib/saleor-version-compatibility-validator.ts rename to packages/shared/src/saleor-version-compatibility-validator.ts index af23456..18f7350 100644 --- a/apps/invoices/src/lib/saleor-version-compatibility-validator.ts +++ b/packages/shared/src/saleor-version-compatibility-validator.ts @@ -1,8 +1,5 @@ const semver = require("semver"); -/** - * TODO Extract to shared or even SDK - */ export class SaleorVersionCompatibilityValidator { constructor(private appRequiredVersion: string) {} @@ -12,7 +9,9 @@ export class SaleorVersionCompatibilityValidator { }); if (!versionIsValid) { - throw new Error(`App requires Saleor matching semver: ${this.appRequiredVersion}`); + throw new Error( + `Your Saleor version (${saleorVersion}) doesn't match App's required version (semver: ${this.appRequiredVersion})` + ); } } } diff --git a/packages/shared/src/setup-tests.ts b/packages/shared/src/setup-tests.ts new file mode 100644 index 0000000..5f1e198 --- /dev/null +++ b/packages/shared/src/setup-tests.ts @@ -0,0 +1,6 @@ +/** + * Add test setup logic here + * + * https://vitest.dev/config/#setupfiles + */ +export {}; diff --git a/packages/shared/vitest.config.ts b/packages/shared/vitest.config.ts new file mode 100644 index 0000000..841a596 --- /dev/null +++ b/packages/shared/vitest.config.ts @@ -0,0 +1,12 @@ +import { defineConfig } from "vitest/config"; + +// https://vitejs.dev/config/ +export default defineConfig({ + plugins: [], + test: { + passWithNoTests: true, + environment: "jsdom", + setupFiles: "./src/setup-tests.ts", + css: false, + }, +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cd6868b..994bb22 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -725,9 +725,6 @@ importers: react-hook-form: specifier: ^7.41.0 version: 7.43.1(react@18.2.0) - semver: - specifier: ^7.3.8 - version: 7.3.8 tiny-invariant: specifier: ^1.3.1 version: 1.3.1 @@ -780,9 +777,6 @@ importers: '@types/rimraf': specifier: ^3.0.2 version: 3.0.2 - '@types/semver': - specifier: ^7.3.13 - version: 7.3.13 '@vitejs/plugin-react': specifier: ^3.0.0 version: 3.1.0(vite@4.3.6) @@ -1780,6 +1774,9 @@ importers: pino-pretty: specifier: ^10.0.0 version: 10.0.0 + semver: + specifier: ^7.5.1 + version: 7.5.1 devDependencies: '@material-ui/core': specifier: ^4.12.4 @@ -1802,6 +1799,9 @@ importers: '@types/react-dom': specifier: ^18.0.10 version: 18.0.10 + '@types/semver': + specifier: ^7.5.0 + version: 7.5.0 clsx: specifier: ^1.2.1 version: 1.2.1 @@ -1817,6 +1817,12 @@ importers: react-dom: specifier: ^18.2.0 version: 18.2.0(react@18.2.0) + vite: + specifier: ^4.3.1 + version: 4.3.1(@types/node@18.13.0) + vitest: + specifier: ^0.30.1 + version: 0.30.1(jsdom@20.0.3) packages/ui: devDependencies: @@ -9051,7 +9057,7 @@ packages: '@storybook/node-logger': 7.0.12 '@storybook/telemetry': 7.0.12 '@storybook/types': 7.0.12 - '@types/semver': 7.3.13 + '@types/semver': 7.5.0 boxen: 5.1.2 chalk: 4.1.2 commander: 6.2.1 @@ -9188,7 +9194,7 @@ packages: '@types/node': 16.18.32 '@types/node-fetch': 2.6.3 '@types/pretty-hrtime': 1.0.1 - '@types/semver': 7.3.13 + '@types/semver': 7.5.0 better-opn: 2.1.1 boxen: 5.1.2 chalk: 4.1.2 @@ -10260,8 +10266,8 @@ packages: /@types/semver@6.2.3: resolution: {integrity: sha512-KQf+QAMWKMrtBMsB8/24w53tEsxllMj6TuA80TT/5igJalLI/zm0L3oXRbIAl4Ohfc85gyHX/jhMwsVkmhLU4A==} - /@types/semver@7.3.13: - resolution: {integrity: sha512-21cFJr9z3g5dW8B0CVI9g2O9beqaThGQ6ZFBqHfwhzLDKUxaqTIy3vnfah/UPkfOiF2pLq+tGz+W8RyCskuslw==} + /@types/semver@7.5.0: + resolution: {integrity: sha512-G8hZ6XJiHnuhQKR7ZmysCeJWE08o8T0AXtk5darsCaTVsYZhhgUrq53jizaR2FvsoeCwJhlmwTjkXBY5Pn/ZHw==} dev: true /@types/send@0.17.1: @@ -10499,7 +10505,7 @@ packages: debug: 4.3.4 globby: 11.1.0 is-glob: 4.0.3 - semver: 7.3.8 + semver: 7.5.1 tsutils: 3.21.0(typescript@4.8.3) typescript: 4.8.3 transitivePeerDependencies: @@ -10520,7 +10526,7 @@ packages: debug: 4.3.4 globby: 11.1.0 is-glob: 4.0.3 - semver: 7.3.8 + semver: 7.5.1 tsutils: 3.21.0(typescript@4.8.4) typescript: 4.8.4 transitivePeerDependencies: @@ -10541,7 +10547,7 @@ packages: debug: 4.3.4 globby: 11.1.0 is-glob: 4.0.3 - semver: 7.3.8 + semver: 7.5.1 tsutils: 3.21.0(typescript@4.9.4) typescript: 4.9.4 transitivePeerDependencies: @@ -10562,7 +10568,7 @@ packages: debug: 4.3.4 globby: 11.1.0 is-glob: 4.0.3 - semver: 7.3.8 + semver: 7.5.1 tsutils: 3.21.0(typescript@4.9.5) typescript: 4.9.5 transitivePeerDependencies: @@ -10582,7 +10588,7 @@ packages: debug: 4.3.4 globby: 11.1.0 is-glob: 4.0.3 - semver: 7.3.8 + semver: 7.5.1 tsutils: 3.21.0(typescript@5.0.4) typescript: 5.0.4 transitivePeerDependencies: @@ -10596,14 +10602,14 @@ packages: eslint: ^6.0.0 || ^7.0.0 || ^8.0.0 dependencies: '@types/json-schema': 7.0.11 - '@types/semver': 7.3.13 + '@types/semver': 7.5.0 '@typescript-eslint/scope-manager': 5.51.0 '@typescript-eslint/types': 5.51.0 '@typescript-eslint/typescript-estree': 5.51.0(typescript@4.8.3) eslint: 8.35.0 eslint-scope: 5.1.1 eslint-utils: 3.0.0(eslint@8.35.0) - semver: 7.3.8 + semver: 7.5.1 transitivePeerDependencies: - supports-color - typescript @@ -12542,7 +12548,7 @@ packages: js-string-escape: 1.0.1 lodash: 4.17.21 md5-hex: 3.0.1 - semver: 7.3.8 + semver: 7.5.1 well-known-symbols: 2.0.0 /config-chain@1.1.13: @@ -16562,7 +16568,7 @@ packages: jws: 3.2.2 lodash: 4.17.21 ms: 2.1.3 - semver: 7.3.8 + semver: 7.5.1 dev: true /jsprim@1.4.2: @@ -20212,6 +20218,14 @@ packages: hasBin: true dependencies: lru-cache: 6.0.0 + dev: true + + /semver@7.5.1: + resolution: {integrity: sha512-Wvss5ivl8TMRZXXESstBA4uR5iXgEN/VC5/sOcuXdVLzcdkz4HWetIoRfG5gb5X+ij/G9rw9YoGn3QoQ8OCSpw==} + engines: {node: '>=10'} + hasBin: true + dependencies: + lru-cache: 6.0.0 /send@0.18.0: resolution: {integrity: sha512-qqWzuOjSFOuqPjFe4NOsMLafToQQwBSOEpS+FwEt3A2V3vKubTquT3vmLTQpFgMXp8AlFWFuP1qKaJZOtPpVXg==} @@ -22046,7 +22060,6 @@ packages: rollup: 3.21.7 optionalDependencies: fsevents: 2.3.2 - dev: false /vite@4.3.6(@types/node@18.13.0): resolution: {integrity: sha512-cqIyLSbA6gornMS659AXTVKF7cvSHMdKmJJwQ9DXq3lwsT1uZSdktuBRlpHQ8VnOWx0QHtjDwxPpGtyo9Fh/Qg==}