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 <admin@peelar.dev>

* Improve error message

* Fix lockfile

---------

Co-authored-by: Adrian Pilarczyk <admin@peelar.dev>

.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

.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

.changeset/purple-pots-sparkle.md

@@ -0,0 +1,5 @@
+---
+"saleor-app-invoices": patch
+---
+
+Moved Semver compatibility checking to shared package, removed semver library

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

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 { createLogger, SaleorVersionCompatibilityValidator } from "@saleor/apps-shared";
-import { SaleorVersionCompatibilityValidator } from "../../lib/saleor-version-compatibility-validator";
 
 const allowedUrlsPattern = process.env.ALLOWED_DOMAIN_PATTERN;
 

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

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;

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<SaleorVersionQuery>(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,
+      });
+    }
+  },
 });

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

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

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})`
+      );
     }
   }
 }

packages/shared/src/setup-tests.ts

@@ -0,0 +1,6 @@
+/**
+ * Add test setup logic here
+ *
+ * https://vitest.dev/config/#setupfiles
+ */
+export {};

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,
+  },
+});

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:
+  /@types/semver@7.5.0:
-    resolution: {integrity: sha512-21cFJr9z3g5dW8B0CVI9g2O9beqaThGQ6ZFBqHfwhzLDKUxaqTIy3vnfah/UPkfOiF2pLq+tGz+W8RyCskuslw==}
+    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==}