Improves how Saleor version is validated during installation (Invoices) (#390)

* Extract SaleorVersionCompatibilityValidator + tests

* Use SaleorVersionCompatibilityValidtor in register handler

* Replace coerce with includePrerelease in saleor version matching

* Changelogs

.changeset/loud-taxis-end.md

@@ -0,0 +1,5 @@
+---
+"saleor-app-invoices": patch
+---
+
+Changed how Saleor version is validated during installation, to use dedicated SaleorVersionCompatibilityValidator. It also doesnt "coerce" version anymore, but uses "includePrelease" flag instead. This should match actual Saleor versioning better

.changeset/rare-tigers-agree.md

@@ -0,0 +1,5 @@
+---
+"saleor-app-invoices": patch
+---
+
+Use REQUIRED_SALEOR_VERSION from manifest in app's own Saleor version validation

apps/invoices/package.json

@@ -69,7 +69,8 @@
     "rimraf": "^3.0.2",
     "typescript": "4.9.5",
     "vite": "^4.1.1",
-    "vitest": "^0.28.4"
+    "vitest": "^0.28.4",
+    "@types/semver": "^7.3.13"
   },
   "lint-staged": {
     "*.{js,ts,tsx}": "eslint --cache --fix",

apps/invoices/src/lib/saleor-version-compatibility-validator.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, it } from "vitest";
+import { SaleorVersionCompatibilityValidator } from "./saleor-version-compatibility-validator";
+
+describe("SaleorVersionCompatibilityValidator", () => {
+  it.each([
+    [">=3.10 <4", "3.12.0"],
+    [">=3.10 <4", "3.999.0"],
+    [">=3.10", "4.0.0"],
+    [">=3.10", "4.1.0"],
+    [">3.10", "3.11.0"],
+    /**
+     * -a suffix is Saleor staging version indicator
+     */
+    [">=3.10", "3.10.0-a"],
+    [">3.10", "3.11.0-a"],
+  ])('Passes for app requirement "%s" and saleor version "%s"', (appVersionReq, saleorVersion) => {
+    expect(() =>
+      new SaleorVersionCompatibilityValidator(appVersionReq).validateOrThrow(saleorVersion)
+    ).not.to.throw();
+  });
+
+  it.each([
+    [">=3.10 <4", "4.0.0"],
+    [">3.10 <4", "3.10.0"],
+    [">3.10", "3.10.0"],
+    [">=3.10", "2.0.0"],
+  ])('Throws for app requirement "%s" and saleor version "%s"', (appVersionReq, saleorVersion) => {
+    expect(() =>
+      new SaleorVersionCompatibilityValidator(appVersionReq).validateOrThrow(saleorVersion)
+    ).to.throw();
+  });
+});

apps/invoices/src/lib/saleor-version-compatibility-validator.ts

@@ -0,0 +1,18 @@
+const semver = require("semver");
+
+/**
+ * TODO Extract to shared or even SDK
+ */
+export class SaleorVersionCompatibilityValidator {
+  constructor(private appRequiredVersion: string) {}
+
+  validateOrThrow(saleorVersion: string) {
+    const versionIsValid = semver.satisfies(saleorVersion, this.appRequiredVersion, {
+      includePrerelease: true,
+    });
+
+    if (!versionIsValid) {
+      throw new Error(`App requires Saleor matching semver: ${this.appRequiredVersion}`);
+    }
+  }
+}

apps/invoices/src/modules/trpc/protected-client-procedure.ts

@@ -1,7 +1,7 @@
 import { createClient } from "../../lib/graphql";
 import { verifyJWT } from "@saleor/app-sdk/verify-jwt";
 import { middleware, procedure } from "./trpc-server";
-import { saleorApp } from "../../../saleor-app";
+import { saleorApp } from "../../saleor-app";
 import { TRPCError } from "@trpc/server";
 import { ProtectedHandlerError } from "@saleor/app-sdk/handlers/next";
 import { logger } from "../../lib/logger";

apps/invoices/src/pages/api/manifest.ts

@@ -3,6 +3,7 @@ import { AppManifest } from "@saleor/app-sdk/types";
 
 import packageJson from "../../../package.json";
 import { invoiceRequestedWebhook } from "./webhooks/invoice-requested";
+import { REQUIRED_SALEOR_VERSION } from "../../saleor-app";
 
 export default createManifestHandler({
   async manifestFactory(context) {
@@ -21,7 +22,7 @@ export default createManifestHandler({
       /**
        * Requires 3.10 due to invoices event payload - in previous versions, order reference was missing
        */
-      requiredSaleorVersion: ">=3.10 <4",
+      requiredSaleorVersion: REQUIRED_SALEOR_VERSION,
     };
 
     return manifest;

apps/invoices/src/pages/api/register.ts

@@ -1,12 +1,11 @@
 import { createAppRegisterHandler } from "@saleor/app-sdk/handlers/next";
-import { saleorApp } from "../../../saleor-app";
+import { REQUIRED_SALEOR_VERSION, saleorApp } from "../../saleor-app";
 import { gql } from "urql";
 import { createClient } from "../../lib/graphql";
 import { SaleorVersionQuery } from "../../../generated/graphql";
 
 import { createLogger } from "../../lib/logger";
-
+import { SaleorVersionCompatibilityValidator } from "../../lib/saleor-version-compatibility-validator";
-const semver = require("semver");
 
 const allowedUrlsPattern = process.env.ALLOWED_DOMAIN_PATTERN;
 
@@ -18,12 +17,6 @@ const SaleorVersion = gql`
   }
 `;
 
-/**
- * TODO: Move to Manifest, when implemented
- * @see https://github.com/saleor/saleor-app-sdk/pull/186
- */
-const APP_SEMVER_REQUIREMENTS = ">=3.10";
-
 /**
  * Required endpoint, called by Saleor to install app.
  * It will exchange tokens with app, so saleorApp.apl will contain token
@@ -78,19 +71,9 @@ export default createAppRegisterHandler({
         throw new Error("Saleor Version couldnt be fetched from the API");
       }
 
-      const versionIsValid = semver.satisfies(
+      new SaleorVersionCompatibilityValidator(REQUIRED_SALEOR_VERSION).validateOrThrow(
-        semver.coerce(saleorVersion),
+        saleorVersion
-        APP_SEMVER_REQUIREMENTS
       );
-
-      logger.debug(
-        { saleorVersion, APP_SEMVER_REQUIREMENTS, coerced: semver.coerce(saleorVersion) },
-        "Semver validation failed"
-      );
-
-      if (!versionIsValid) {
-        throw new Error(`App requires Saleor matching semver: ${APP_SEMVER_REQUIREMENTS}`);
-      }
     } catch (e: unknown) {
       const message = (e as Error)?.message ?? "Unknown error";
 

apps/invoices/src/pages/api/webhooks/invoice-requested.ts

@@ -1,6 +1,6 @@
 import { NextWebhookApiHandler, SaleorAsyncWebhook } from "@saleor/app-sdk/handlers/next";
 import { gql } from "urql";
-import { saleorApp } from "../../../../saleor-app";
+import { saleorApp } from "../../../saleor-app";
 import {
   InvoiceRequestedPayloadFragment,
   OrderPayloadFragment,

apps/invoices/src/saleor-app.ts

@@ -33,3 +33,5 @@ switch (aplType) {
 export const saleorApp = new SaleorApp({
   apl,
 });
+
+export const REQUIRED_SALEOR_VERSION = ">=3.10 <4";

apps/invoices/turbo.json

@@ -12,7 +12,9 @@
         "PORT",
         "VERCEL_URL",
         "ALLOWED_DOMAIN_PATTERN",
-        "NEXT_PUBLIC_VERCEL_ENV"
+        "NEXT_PUBLIC_VERCEL_ENV",
+        "REST_APL_ENDPOINT",
+        "REST_APL_TOKEN"
       ]
     }
   }

pnpm-lock.yaml

@@ -771,6 +771,9 @@ 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.1.1)