Skip to content

Commit

Permalink
Extract part of Semver compatibility logic to shared package and impl…
Browse files Browse the repository at this point in the history
…ement 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 <[email protected]>

* Improve error message

* Fix lockfile

---------

Co-authored-by: Adrian Pilarczyk <[email protected]>
  • Loading branch information
lkostrowski and peelar authored May 23, 2023
1 parent b36502d commit 23b5c70
Show file tree
Hide file tree
Showing 15 changed files with 140 additions and 31 deletions.
5 changes: 5 additions & 0 deletions .changeset/chatty-melons-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@saleor/apps-shared": minor
---

Add SaleorCompatibilityValidator util that compares semver version of the app and Saleor's
9 changes: 9 additions & 0 deletions .changeset/famous-onions-jam.md
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions .changeset/purple-pots-sparkle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"saleor-app-invoices": patch
---

Moved Semver compatibility checking to shared package, removed semver library
2 changes: 0 additions & 2 deletions apps/invoices/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
3 changes: 1 addition & 2 deletions apps/invoices/src/pages/api/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 2 additions & 0 deletions apps/taxes/saleor-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,5 @@ switch (process.env.APL) {
export const saleorApp = new SaleorApp({
apl,
});

export const REQUIRED_SALEOR_VERSION = ">=3.9 <4";
2 changes: 2 additions & 0 deletions apps/taxes/src/pages/api/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
Expand Down
56 changes: 55 additions & 1 deletion apps/taxes/src/pages/api/register.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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,
});
}
},
});
1 change: 1 addition & 0 deletions packages/shared/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
8 changes: 6 additions & 2 deletions packages/shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
const semver = require("semver");

/**
* TODO Extract to shared or even SDK
*/
export class SaleorVersionCompatibilityValidator {
constructor(private appRequiredVersion: string) {}

Expand All @@ -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})`
);
}
}
}
6 changes: 6 additions & 0 deletions packages/shared/src/setup-tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* Add test setup logic here
*
* https://vitest.dev/config/#setupfiles
*/
export {};
12 changes: 12 additions & 0 deletions packages/shared/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -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,
},
});
53 changes: 33 additions & 20 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 23b5c70

Please sign in to comment.