From 4fc10627d3aa9bbca126ac062d9c1f95a69f5c7f Mon Sep 17 00:00:00 2001 From: Lukasz Ostrowski Date: Tue, 1 Oct 2024 09:13:33 +0200 Subject: [PATCH] refactor order confirmed event (#1601) * SaleorOrderConfirmedEvent remove storing zod schema instead of payload * Use event in transformed * wip * fix tests * imprvoe types --- .../modules/avatax/avatax-webhook.service.ts | 5 +- .../avatax-order-confirmed-adapter.ts | 11 +--- ...rder-confirmed-payload-transformer.test.ts | 4 -- ...tax-order-confirmed-payload-transformer.ts | 48 +++++++------- .../avatax-order-confirmed-payload.service.ts | 7 +- apps/avatax/src/modules/saleor/index.ts | 1 - .../saleor/order-confirmed/event.test.ts | 40 ++++++++++++ .../modules/saleor/order-confirmed/event.ts | 64 +++++++++++++++---- .../modules/saleor/order-confirmed/mocks.ts | 28 +++++--- apps/avatax/src/modules/saleor/types.ts | 6 -- .../src/pages/api/webhooks/order-confirmed.ts | 2 - 11 files changed, 141 insertions(+), 75 deletions(-) delete mode 100644 apps/avatax/src/modules/saleor/types.ts diff --git a/apps/avatax/src/modules/avatax/avatax-webhook.service.ts b/apps/avatax/src/modules/avatax/avatax-webhook.service.ts index 543ddda03..553e57128 100644 --- a/apps/avatax/src/modules/avatax/avatax-webhook.service.ts +++ b/apps/avatax/src/modules/avatax/avatax-webhook.service.ts @@ -4,7 +4,7 @@ import { AvataxCalculateTaxesPayloadService } from "@/modules/avatax/calculate-t import { AvataxCalculateTaxesPayloadTransformer } from "@/modules/avatax/calculate-taxes/avatax-calculate-taxes-payload-transformer"; import { AvataxTaxCodeMatchesService } from "@/modules/avatax/tax-code/avatax-tax-code-matches.service"; -import { DeprecatedOrderConfirmedSubscriptionFragment, SaleorOrderConfirmedEvent } from "../saleor"; +import { SaleorOrderConfirmedEvent } from "../saleor"; import { CancelOrderPayload } from "../taxes/tax-provider-webhook"; import { CalculateTaxesPayload } from "../webhooks/payloads/calculate-taxes-payload"; import { AvataxConfig } from "./avatax-connection-schema"; @@ -43,14 +43,13 @@ export class AvataxWebhookService { } async confirmOrder( - order: DeprecatedOrderConfirmedSubscriptionFragment, confirmedOrderEvent: SaleorOrderConfirmedEvent, avataxConfig: AvataxConfig, authData: AuthData, discountStrategy: PriceReductionDiscountsStrategy, ) { const response = await this.avataxOrderConfirmedAdapter.send( - { order, confirmedOrderEvent }, + { confirmedOrderEvent }, avataxConfig, authData, discountStrategy, diff --git a/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-adapter.ts b/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-adapter.ts index e2f3cd3a3..20f79ba84 100644 --- a/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-adapter.ts +++ b/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-adapter.ts @@ -4,10 +4,7 @@ import { ObservabilityAttributes } from "@saleor/apps-otel/src/lib/observability import { loggerContext } from "@/logger-context"; import { createLogger } from "../../../logger"; -import { - DeprecatedOrderConfirmedSubscriptionFragment, - SaleorOrderConfirmedEvent, -} from "../../saleor"; +import { SaleorOrderConfirmedEvent } from "../../saleor"; import { CreateOrderResponse } from "../../taxes/tax-provider-webhook"; import { WebhookAdapter } from "../../taxes/tax-webhook-adapter"; import { AvataxClient } from "../avatax-client"; @@ -19,12 +16,9 @@ import { AvataxOrderConfirmedPayloadService } from "./avatax-order-confirmed-pay import { AvataxOrderConfirmedResponseTransformer } from "./avatax-order-confirmed-response-transformer"; type AvataxOrderConfirmedPayload = { - /** - * @deprecated use `SaleorOrderConfirmedEvent` instead - */ - order: DeprecatedOrderConfirmedSubscriptionFragment; confirmedOrderEvent: SaleorOrderConfirmedEvent; }; + type AvataxOrderConfirmedResponse = CreateOrderResponse; export class AvataxOrderConfirmedAdapter @@ -53,7 +47,6 @@ export class AvataxOrderConfirmedAdapter this.logger.debug("Transforming the Saleor payload for creating order with AvaTax..."); const target = await this.avataxOrderConfirmedPayloadService.getPayload( - payload.order, payload.confirmedOrderEvent, config, authData, diff --git a/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload-transformer.test.ts b/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload-transformer.test.ts index 7481a1a49..e614298a6 100644 --- a/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload-transformer.test.ts +++ b/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload-transformer.test.ts @@ -17,8 +17,6 @@ const mockGenerator = new AvataxOrderConfirmedMockGenerator(); const saleorOrderConfirmedEventMock = SaleorOrderConfirmedEventMockFactory.create(); const discountsStrategy = new PriceReductionDiscountsStrategy(); -const orderMock = mockGenerator.generateOrder(); - /** * TODO: Dont export this, extract to shared code */ @@ -36,7 +34,6 @@ const transformer = new AvataxOrderConfirmedPayloadTransformer( describe("AvataxOrderConfirmedPayloadTransformer", () => { it("returns document type of SalesInvoice when isDocumentRecordingEnabled is true", async () => { const payload = await transformer.transform({ - order: orderMock, confirmedOrderEvent: saleorOrderConfirmedEventMock, avataxConfig: avataxConfigMock, matches: [], @@ -47,7 +44,6 @@ describe("AvataxOrderConfirmedPayloadTransformer", () => { }); it("returns document type of SalesOrder when isDocumentRecordingEnabled is false", async () => { const payload = await transformer.transform({ - order: orderMock, confirmedOrderEvent: saleorOrderConfirmedEventMock, avataxConfig: { ...avataxConfigMock, diff --git a/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload-transformer.ts b/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload-transformer.ts index da5c3023f..744bf2be2 100644 --- a/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload-transformer.ts +++ b/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload-transformer.ts @@ -3,10 +3,7 @@ import { DocumentType } from "avatax/lib/enums/DocumentType"; import { err, ok } from "neverthrow"; import { createLogger } from "../../../logger"; -import { - DeprecatedOrderConfirmedSubscriptionFragment, - SaleorOrderConfirmedEvent, -} from "../../saleor"; +import { SaleorOrderConfirmedEvent } from "../../saleor"; import { TaxBadPayloadError } from "../../taxes/tax-error"; import { avataxAddressFactory } from "../address-factory"; import { AvataxCalculationDateResolver } from "../avatax-calculation-date-resolver"; @@ -37,50 +34,57 @@ export class AvataxOrderConfirmedPayloadTransformer { return DocumentType.SalesInvoice; } - private getSaleorAddress(order: DeprecatedOrderConfirmedSubscriptionFragment) { - if (order.shippingAddress) { - return ok(order.shippingAddress); + private getSaleorAddress(confirmedOrderEvent: SaleorOrderConfirmedEvent) { + const shippingAddress = confirmedOrderEvent.getOrderShippingAddress(); + const billingAddress = confirmedOrderEvent.getOrderBillingAddress(); + + if (shippingAddress) { + return ok(shippingAddress); } - if (order.billingAddress) { + if (billingAddress) { this.logger.warn( "OrderConfirmedPayload has no shipping address, falling back to billing address", ); - return ok(order.billingAddress); + return ok(billingAddress); } return err(new TaxBadPayloadError("OrderConfirmedPayload has no shipping or billing address")); } async transform({ - order, confirmedOrderEvent, avataxConfig, matches, discountsStrategy, }: { - order: DeprecatedOrderConfirmedSubscriptionFragment; confirmedOrderEvent: SaleorOrderConfirmedEvent; avataxConfig: AvataxConfig; matches: AvataxTaxCodeMatches; discountsStrategy: PriceReductionDiscountsStrategy; }): Promise { - const entityUseCode = await this.avataxEntityTypeMatcher.match(order.avataxEntityCode); + const entityUseCode = await this.avataxEntityTypeMatcher.match( + confirmedOrderEvent.getAvaTaxEntityCode(), + ); + const date = this.avataxCalculationDateResolver.resolve( - order.avataxTaxCalculationDate, - order.created, + confirmedOrderEvent.getAvaTaxTaxCalculationDate(), + confirmedOrderEvent.getOrderCreationDate(), ); + const code = this.avataxDocumentCodeResolver.resolve({ - avataxDocumentCode: order.avataxDocumentCode, - orderId: order.id, + avataxDocumentCode: confirmedOrderEvent.getAvaTaxDocumentCode(), + orderId: confirmedOrderEvent.getOrderId(), }); + const customerCode = avataxCustomerCode.resolve({ - avataxCustomerCode: order.avataxCustomerCode, - legacyAvataxCustomerCode: order.user?.avataxCustomerCode, - legacyUserId: order.user?.id, + avataxCustomerCode: confirmedOrderEvent.getAvaTaxCustomerCode(), + legacyAvataxCustomerCode: confirmedOrderEvent.getLegacyAvaTaxCustomerCode(), + legacyUserId: confirmedOrderEvent.getUserId(), source: "Order", }); - const addressPayload = this.getSaleorAddress(order); + + const addressPayload = this.getSaleorAddress(confirmedOrderEvent); if (addressPayload.isErr()) { Sentry.captureException(addressPayload.error); @@ -105,9 +109,9 @@ export class AvataxOrderConfirmedPayloadTransformer { shipFrom: avataxAddressFactory.fromChannelAddress(avataxConfig.address), shipTo: avataxAddressFactory.fromSaleorAddress(addressPayload.value), }, - currencyCode: order.total.currency, + currencyCode: confirmedOrderEvent.getOrderCurrency(), // we can fall back to empty string because email is not a required field - email: order.user?.email ?? order.userEmail ?? "", + email: confirmedOrderEvent.resolveUserEmailOrEmpty(), lines: this.saleorOrderToAvataxLinesTransformer.transform({ confirmedOrderEvent, matches, diff --git a/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload.service.ts b/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload.service.ts index 43ca91d93..f72078562 100644 --- a/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload.service.ts +++ b/apps/avatax/src/modules/avatax/order-confirmed/avatax-order-confirmed-payload.service.ts @@ -1,9 +1,6 @@ import { AuthData } from "@saleor/app-sdk/APL"; -import { - DeprecatedOrderConfirmedSubscriptionFragment, - SaleorOrderConfirmedEvent, -} from "../../saleor"; +import { SaleorOrderConfirmedEvent } from "../../saleor"; import { CreateTransactionArgs } from "../avatax-client"; import { AvataxConfig } from "../avatax-connection-schema"; import { PriceReductionDiscountsStrategy } from "../discounts"; @@ -22,7 +19,6 @@ export class AvataxOrderConfirmedPayloadService { } async getPayload( - order: DeprecatedOrderConfirmedSubscriptionFragment, confirmedOrderEvent: SaleorOrderConfirmedEvent, avataxConfig: AvataxConfig, authData: AuthData, @@ -31,7 +27,6 @@ export class AvataxOrderConfirmedPayloadService { const matches = await this.getMatches(authData); return this.avataxOrderConfirmedPayloadTransformer.transform({ - order, confirmedOrderEvent, avataxConfig, matches, diff --git a/apps/avatax/src/modules/saleor/index.ts b/apps/avatax/src/modules/saleor/index.ts index 61172b9c9..a37f9c595 100644 --- a/apps/avatax/src/modules/saleor/index.ts +++ b/apps/avatax/src/modules/saleor/index.ts @@ -1,4 +1,3 @@ export * from "./order-cancelled"; export * from "./order-confirmed"; export * from "./order-line"; -export * from "./types"; diff --git a/apps/avatax/src/modules/saleor/order-confirmed/event.test.ts b/apps/avatax/src/modules/saleor/order-confirmed/event.test.ts index fc7f27a21..ec240d7ca 100644 --- a/apps/avatax/src/modules/saleor/order-confirmed/event.test.ts +++ b/apps/avatax/src/modules/saleor/order-confirmed/event.test.ts @@ -135,4 +135,44 @@ describe("SaleorOrderConfirmedEvent", () => { expect(event.getShippingAmount()).toEqual(payload.order.shippingPrice.net.amount); }); }); + + describe("resolveUserEmailOrEmpty", () => { + it("Returns order.user.email if exists", () => { + const payload = SaleorOrderConfirmedEventMockFactory.getGraphqlPayload(); + + payload.order.user = { + email: "a@b.com", + id: "1", + }; + + payload.order.userEmail = "another@another.com"; + + const result = SaleorOrderConfirmedEvent.createFromGraphQL(payload); + + expect(result._unsafeUnwrap().resolveUserEmailOrEmpty()).toBe("a@b.com"); + }); + + it("Returns order.userEmail.email if exists and user.email doesnt", () => { + const payload = SaleorOrderConfirmedEventMockFactory.getGraphqlPayload(); + + payload.order.user = undefined; + + payload.order.userEmail = "another@another.com"; + + const result = SaleorOrderConfirmedEvent.createFromGraphQL(payload); + + expect(result._unsafeUnwrap().resolveUserEmailOrEmpty()).toBe("another@another.com"); + }); + + it("Returns empty string if neither user.email or userEmail exist", () => { + const payload = SaleorOrderConfirmedEventMockFactory.getGraphqlPayload(); + + payload.order.user = undefined; + payload.order.userEmail = undefined; + + const result = SaleorOrderConfirmedEvent.createFromGraphQL(payload); + + expect(result._unsafeUnwrap().resolveUserEmailOrEmpty()).toBe(""); + }); + }); }); diff --git a/apps/avatax/src/modules/saleor/order-confirmed/event.ts b/apps/avatax/src/modules/saleor/order-confirmed/event.ts index 29b21f67f..75b1e1ab1 100644 --- a/apps/avatax/src/modules/saleor/order-confirmed/event.ts +++ b/apps/avatax/src/modules/saleor/order-confirmed/event.ts @@ -5,8 +5,18 @@ import { BaseError } from "../../../error"; import { OrderConfirmedPayload } from "../../webhooks/payloads/order-confirmed-payload"; import { SaleorOrderLine } from "../order-line"; +type EventWithOrder = Omit & { + order: NonNullable; +}; + export class SaleorOrderConfirmedEvent { - private static schema = z.object({ + /** + * While GraphQL provides types contract, not everything can be consumed by the app. + * For example App requires lines or shipping to calculate taxes. + * + * Schema here is additional validation - if these fields don't exist, app must handle gracefully lack of data in payload + */ + private static requiredFieldsSchema = z.object({ order: z.object({ channel: z.object({ taxConfiguration: z.object({ @@ -29,8 +39,8 @@ export class SaleorOrderConfirmedEvent { }); private constructor( - private data: z.infer, - private lines: SaleorOrderLine[], + private rawPayload: EventWithOrder, + private parsedLines: SaleorOrderLine[], ) {} static ParsingError = BaseError.subclass("SaleorOrderConfirmedEventParsingError"); @@ -45,7 +55,7 @@ export class SaleorOrderConfirmedEvent { } const parser = Result.fromThrowable( - SaleorOrderConfirmedEvent.schema.parse, + SaleorOrderConfirmedEvent.requiredFieldsSchema.parse, SaleorOrderConfirmedEvent.ParsingError.normalize, ); @@ -63,26 +73,52 @@ export class SaleorOrderConfirmedEvent { return err(parsedLinePayload.error); } - return ok(new SaleorOrderConfirmedEvent(parsedPayload.value, parsedLinePayload.value)); + return ok(new SaleorOrderConfirmedEvent(payload as EventWithOrder, parsedLinePayload.value)); }; - getChannelSlug = () => this.data.order.channel.slug; + getChannelSlug = () => this.rawPayload.order.channel.slug; - getOrderId = () => this.data.order.id; + getOrderId = () => this.rawPayload.order.id; - isFulfilled = () => this.data.order.status === "FULFILLED"; + isFulfilled = () => this.rawPayload.order.status === "FULFILLED"; isStrategyFlatRates = () => - this.data.order.channel.taxConfiguration.taxCalculationStrategy === "FLAT_RATES"; + this.rawPayload.order.channel.taxConfiguration.taxCalculationStrategy === "FLAT_RATES"; - getIsTaxIncluded = () => this.data.order.channel.taxConfiguration.pricesEnteredWithTax; + getIsTaxIncluded = () => this.rawPayload.order.channel.taxConfiguration.pricesEnteredWithTax; - getLines = () => this.lines; + getLines = () => this.parsedLines; - hasShipping = () => this.data.order.shippingPrice.net.amount !== 0; + hasShipping = () => this.rawPayload.order.shippingPrice.net.amount !== 0; getShippingAmount = () => this.getIsTaxIncluded() - ? this.data.order.shippingPrice.gross.amount - : this.data.order.shippingPrice.net.amount; + ? this.rawPayload.order.shippingPrice.gross.amount + : this.rawPayload.order.shippingPrice.net.amount; + + resolveUserEmailOrEmpty = () => + this.rawPayload.order.user?.email ?? this.rawPayload.order?.userEmail ?? ""; + + getOrderCurrency = () => this.rawPayload.order.total.currency; + + getUserId = () => this.rawPayload.order.user?.id; + + /** + * @deprecated - We should use customer code from order + */ + getLegacyAvaTaxCustomerCode = () => this.rawPayload.order.user?.avataxCustomerCode; + + getAvaTaxCustomerCode = () => this.rawPayload.order.avataxCustomerCode; + + getAvaTaxDocumentCode = () => this.rawPayload.order.avataxDocumentCode; + + getAvaTaxEntityCode = () => this.rawPayload.order.avataxEntityCode; + + getAvaTaxTaxCalculationDate = () => this.rawPayload.order.avataxTaxCalculationDate; + + getOrderCreationDate = () => this.rawPayload.order.created; + + getOrderShippingAddress = () => this.rawPayload.order.shippingAddress; + + getOrderBillingAddress = () => this.rawPayload.order.shippingAddress; } diff --git a/apps/avatax/src/modules/saleor/order-confirmed/mocks.ts b/apps/avatax/src/modules/saleor/order-confirmed/mocks.ts index 248b3130b..4be437c15 100644 --- a/apps/avatax/src/modules/saleor/order-confirmed/mocks.ts +++ b/apps/avatax/src/modules/saleor/order-confirmed/mocks.ts @@ -3,13 +3,19 @@ import { SaleorOrderLineMockFactory } from "../order-line-mocks"; import { SaleorOrderConfirmedEvent } from "./event"; export class SaleorOrderConfirmedEventMockFactory { - private static graphqlPayload = { + private static _getGraphqlPayload = (): { + order: NonNullable; + __typename: "OrderConfirmed"; + } => ({ order: { + user: { + id: "id", + email: "email@example.com", + }, id: "order-id", number: "order-number", created: "2021-01-01T00:00:00Z", status: "FULFILLED" as const, - discounts: [], channel: { id: "channel-id", slug: "channel-slug", @@ -26,10 +32,16 @@ export class SaleorOrderConfirmedEventMockFactory { amount: 10, }, }, + shippingAddress: { + __typename: "Address", + city: "Krakow", + country: { code: "PL", __typename: "CountryDisplay" }, + countryArea: "Malopolskie", + postalCode: "12345", + streetAddress1: "Jana Pawla 2", + streetAddress2: "2137", + }, total: { - gross: { - amount: 10, - }, net: { amount: 10, }, @@ -42,10 +54,10 @@ export class SaleorOrderConfirmedEventMockFactory { __typename: "Order" as const, }, __typename: "OrderConfirmed" as const, - }; + }); static create( - graphqlPayload: OrderConfirmedPayload = SaleorOrderConfirmedEventMockFactory.graphqlPayload, + graphqlPayload: OrderConfirmedPayload = SaleorOrderConfirmedEventMockFactory._getGraphqlPayload(), ) { const possibleOrderLine = SaleorOrderConfirmedEvent.createFromGraphQL(graphqlPayload); @@ -56,5 +68,5 @@ export class SaleorOrderConfirmedEventMockFactory { return possibleOrderLine.value; } - static getGraphqlPayload = () => SaleorOrderConfirmedEventMockFactory.graphqlPayload; + static getGraphqlPayload = () => SaleorOrderConfirmedEventMockFactory._getGraphqlPayload(); } diff --git a/apps/avatax/src/modules/saleor/types.ts b/apps/avatax/src/modules/saleor/types.ts deleted file mode 100644 index 8f19e2926..000000000 --- a/apps/avatax/src/modules/saleor/types.ts +++ /dev/null @@ -1,6 +0,0 @@ -import { OrderConfirmedSubscriptionFragment } from "../../../generated/graphql"; - -/** - * @deprecated use `SaleorOrderConfirmedEvent` instead - */ -export type DeprecatedOrderConfirmedSubscriptionFragment = OrderConfirmedSubscriptionFragment; diff --git a/apps/avatax/src/pages/api/webhooks/order-confirmed.ts b/apps/avatax/src/pages/api/webhooks/order-confirmed.ts index fde9e7512..f4d18a486 100644 --- a/apps/avatax/src/pages/api/webhooks/order-confirmed.ts +++ b/apps/avatax/src/pages/api/webhooks/order-confirmed.ts @@ -200,8 +200,6 @@ const handler = orderConfirmedAsyncWebhook.createHandler(async (req, res, ctx) = try { const confirmedOrder = await taxProvider.confirmOrder( - // @ts-expect-error: OrderConfirmedSubscriptionFragment is deprecated - payload.order, confirmedOrderEvent, providerConfig.value.avataxConfig.config, ctx.authData,