From 852df3f76473a61118d0665471272a74fc6405d7 Mon Sep 17 00:00:00 2001 From: Sebastian Rindom Date: Mon, 30 Sep 2024 13:51:55 +0200 Subject: [PATCH] fix(medusa,dashboard): don't send price updates for deleted regions/currencies (#9361) **What** - Fixes an issue where the admin dashboard would send region prices for deleted regions. - Also, includes the implementation for `GET /admin/shipping-options/:id` and its corresponding SDK function. **Why** - When a region price for a deleted region was sent to the backend it would result in the insert hitting a not null constraint on the currency_code for prices. To avoid this the dashboard should not send region prices for deleted regions. **Additional context** - Prices for deleted regions should ideally not be returned when fetching shipping option prices. However, we don't yet have a mechanism for cleaning up region prices after a region is deleted. Fixes CC-540 --- .../admin/shipping-option.spec.ts | 99 +++++++++++++++++++ .../src/hooks/api/shipping-options.tsx | 34 +++---- .../edit-shipping-options-pricing-form.tsx | 13 +++ ...n-service-zone-shipping-option-pricing.tsx | 21 ++-- .../core/js-sdk/src/admin/shipping-option.ts | 15 +++ .../api/admin/shipping-options/[id]/route.ts | 26 ++++- .../src/api/admin/shipping-options/helpers.ts | 44 ++++----- .../api/admin/shipping-options/middlewares.ts | 10 ++ 8 files changed, 207 insertions(+), 55 deletions(-) diff --git a/integration-tests/http/__tests__/shipping-option/admin/shipping-option.spec.ts b/integration-tests/http/__tests__/shipping-option/admin/shipping-option.spec.ts index 6857435e20872..94d8ab71ecc8a 100644 --- a/integration-tests/http/__tests__/shipping-option/admin/shipping-option.spec.ts +++ b/integration-tests/http/__tests__/shipping-option/admin/shipping-option.spec.ts @@ -134,6 +134,105 @@ medusaIntegrationTestRunner({ }) }) + describe("GET /admin/shipping-options/:id", () => { + it("should filters options by stock_location_id", async () => { + const shippingOptionPayload = { + name: "Test shipping option", + service_zone_id: fulfillmentSet.service_zones[0].id, + shipping_profile_id: shippingProfile.id, + provider_id: "manual_test-provider", + price_type: "flat", + type: { + label: "Test type", + description: "Test description", + code: "test-code", + }, + prices: [{ currency_code: "usd", amount: 1000 }], + } + + const { + data: { shipping_option: shippingOption }, + } = await api.post( + `/admin/shipping-options`, + shippingOptionPayload, + adminHeaders + ) + + const shippingOptionRes = await api.get( + `/admin/shipping-options/${shippingOption.id}`, + adminHeaders + ) + + expect(shippingOptionRes.data.shipping_option).toEqual({ + id: expect.any(String), + name: "Test shipping option", + price_type: "flat", + prices: [ + { + id: expect.any(String), + amount: 1000, + currency_code: "usd", + max_quantity: null, + min_quantity: null, + price_list: null, + price_set_id: expect.any(String), + raw_amount: { + precision: 20, + value: "1000", + }, + rules_count: 0, + title: null, + created_at: expect.any(String), + updated_at: expect.any(String), + deleted_at: null, + }, + ], + provider_id: "manual_test-provider", + provider: { + id: "manual_test-provider", + is_enabled: true, + }, + rules: [], + service_zone_id: expect.any(String), + service_zone: { + id: expect.any(String), + name: "Test", + fulfillment_set: { + id: expect.any(String), + }, + fulfillment_set_id: expect.any(String), + metadata: null, + created_at: expect.any(String), + updated_at: expect.any(String), + deleted_at: null, + }, + shipping_profile_id: expect.any(String), + shipping_profile: { + id: expect.any(String), + metadata: null, + name: "Test", + type: "default", + created_at: expect.any(String), + updated_at: expect.any(String), + deleted_at: null, + }, + type: { + code: "test-code", + description: "Test description", + id: expect.any(String), + label: "Test type", + created_at: expect.any(String), + updated_at: expect.any(String), + deleted_at: null, + }, + created_at: expect.any(String), + updated_at: expect.any(String), + data: null, + metadata: null, + }) + }) + }) + describe("POST /admin/shipping-options", () => { it("should throw error when required params are missing", async () => { const shippingOptionPayload = { diff --git a/packages/admin/dashboard/src/hooks/api/shipping-options.tsx b/packages/admin/dashboard/src/hooks/api/shipping-options.tsx index 4297b2109db1c..fad303d5fd8ef 100644 --- a/packages/admin/dashboard/src/hooks/api/shipping-options.tsx +++ b/packages/admin/dashboard/src/hooks/api/shipping-options.tsx @@ -18,24 +18,24 @@ export const shippingOptionsQueryKeys = queryKeysFactory( SHIPPING_OPTIONS_QUERY_KEY ) -// TODO: Endpoint is not implemented yet -// export const useShippingOption = ( -// id: string, -// options?: UseQueryOptions< -// HttpTypes.AdminShippingOptionResponse, -// Error, -// HttpTypes.AdminShippingOptionResponse, -// QueryKey -// > -// ) => { -// const { data, ...rest } = useQuery({ -// queryFn: () => sdk.admin.shippingOption.retrieve(id), -// queryKey: shippingOptionsQueryKeys.retrieve(id), -// ...options, -// }) +export const useShippingOption = ( + id: string, + query?: Record, + options?: UseQueryOptions< + HttpTypes.AdminShippingOptionResponse, + Error, + HttpTypes.AdminShippingOptionResponse, + QueryKey + > +) => { + const { data, ...rest } = useQuery({ + queryFn: () => sdk.admin.shippingOption.retrieve(id, query), + queryKey: shippingOptionsQueryKeys.detail(id), + ...options, + }) -// return { ...data, ...rest } -// } + return { ...data, ...rest } +} export const useShippingOptions = ( query?: HttpTypes.AdminShippingOptionListParams, diff --git a/packages/admin/dashboard/src/routes/locations/location-service-zone-shipping-option-pricing/components/create-shipping-options-form/edit-shipping-options-pricing-form.tsx b/packages/admin/dashboard/src/routes/locations/location-service-zone-shipping-option-pricing/components/create-shipping-options-form/edit-shipping-options-pricing-form.tsx index 4e81d0e4c5b17..7703ad3229a3f 100644 --- a/packages/admin/dashboard/src/routes/locations/location-service-zone-shipping-option-pricing/components/create-shipping-options-form/edit-shipping-options-pricing-form.tsx +++ b/packages/admin/dashboard/src/routes/locations/location-service-zone-shipping-option-pricing/components/create-shipping-options-form/edit-shipping-options-pricing-form.tsx @@ -129,6 +129,11 @@ export function EditShippingOptionsPricingForm({ return undefined } + const currencyExists = currencies.some(currencyCode => currencyCode.toLowerCase() == code.toLowerCase()) + if (!currencyExists) { + return undefined + } + const amount = castNumber(value) const priceRecord: PriceRecord = { @@ -155,6 +160,14 @@ export function EditShippingOptionsPricingForm({ return undefined } + // Check if the region_id exists in the regions array to avoid + // sending updates of region prices where the region has been + // deleted + const regionExists = regions?.some(region => region.id === region_id) + if (!regionExists) { + return undefined + } + const amount = castNumber(value) const priceRecord: PriceRecord = { diff --git a/packages/admin/dashboard/src/routes/locations/location-service-zone-shipping-option-pricing/location-service-zone-shipping-option-pricing.tsx b/packages/admin/dashboard/src/routes/locations/location-service-zone-shipping-option-pricing/location-service-zone-shipping-option-pricing.tsx index 7763bbc1f78f6..9cd94ca5687cd 100644 --- a/packages/admin/dashboard/src/routes/locations/location-service-zone-shipping-option-pricing/location-service-zone-shipping-option-pricing.tsx +++ b/packages/admin/dashboard/src/routes/locations/location-service-zone-shipping-option-pricing/location-service-zone-shipping-option-pricing.tsx @@ -1,25 +1,24 @@ import { json, useParams } from "react-router-dom" import { RouteFocusModal } from "../../../components/modals" -import { useShippingOptions } from "../../../hooks/api/shipping-options" +import { useShippingOption } from "../../../hooks/api/shipping-options" import { EditShippingOptionsPricingForm } from "./components/create-shipping-options-form" export function LocationServiceZoneShippingOptionPricing() { const { so_id, location_id } = useParams() - const { shipping_options, isPending, isFetching, isError, error } = - useShippingOptions({ - // TODO: change this when GET option by id endpoint is implemented - id: [so_id!], - fields: "*prices,*prices.price_rules", + if (!so_id) { + throw json({ + message: "Shipping Option ID paramater is missing", + status: 404, }) - - const shippingOption = shipping_options?.find((so) => so.id === so_id) - - if (!isPending && !isFetching && !shippingOption) { - throw json(`Shipping option with id: ${so_id} not found`, { status: 404 }) } + const { shipping_option: shippingOption, isError, error } = + useShippingOption(so_id, { + fields: "*prices,*prices.price_rules", + }) + if (isError) { throw error } diff --git a/packages/core/js-sdk/src/admin/shipping-option.ts b/packages/core/js-sdk/src/admin/shipping-option.ts index b6928c8037d93..c01114ed1477e 100644 --- a/packages/core/js-sdk/src/admin/shipping-option.ts +++ b/packages/core/js-sdk/src/admin/shipping-option.ts @@ -24,6 +24,21 @@ export class ShippingOption { ) } + async retrieve( + id: string, + query?: HttpTypes.SelectParams, + headers?: ClientHeaders + ) { + return await this.client.fetch( + `/admin/shipping-options/${id}`, + { + method: "GET", + headers, + query, + } + ) + } + async update( id: string, body: HttpTypes.AdminUpdateShippingOption, diff --git a/packages/medusa/src/api/admin/shipping-options/[id]/route.ts b/packages/medusa/src/api/admin/shipping-options/[id]/route.ts index 3397d75089a2f..9d8c7f1f87682 100644 --- a/packages/medusa/src/api/admin/shipping-options/[id]/route.ts +++ b/packages/medusa/src/api/admin/shipping-options/[id]/route.ts @@ -8,7 +8,31 @@ import { MedusaResponse, } from "../../../../types/routing" import { refetchShippingOption } from "../helpers" -import { AdminUpdateShippingOptionType } from "../validators" +import { + AdminGetShippingOptionParamsType, + AdminUpdateShippingOptionType, +} from "../validators" +import { MedusaError } from "@medusajs/framework/utils" + +export const GET = async ( + req: AuthenticatedMedusaRequest, + res: MedusaResponse +) => { + const shippingOption = await refetchShippingOption( + req.params.id, + req.scope, + req.remoteQueryConfig.fields + ) + + if (!shippingOption) { + throw new MedusaError( + MedusaError.Types.NOT_FOUND, + `Shipping Option with id: ${req.params.id} not found` + ) + } + + res.json({ shipping_option: shippingOption }) +} export const POST = async ( req: AuthenticatedMedusaRequest, diff --git a/packages/medusa/src/api/admin/shipping-options/helpers.ts b/packages/medusa/src/api/admin/shipping-options/helpers.ts index 8c2c2df4b27a3..2966b4b8b2c9d 100644 --- a/packages/medusa/src/api/admin/shipping-options/helpers.ts +++ b/packages/medusa/src/api/admin/shipping-options/helpers.ts @@ -6,7 +6,6 @@ import { import { ContainerRegistrationKeys, promiseAll, - remoteQueryObjectFromString, } from "@medusajs/framework/utils" export const refetchShippingOption = async ( @@ -14,17 +13,14 @@ export const refetchShippingOption = async ( scope: MedusaContainer, fields: string[] ) => { - const remoteQuery = scope.resolve(ContainerRegistrationKeys.REMOTE_QUERY) - const queryObject = remoteQueryObjectFromString({ - entryPoint: "shipping_option", - variables: { - filters: { id: shippingOptionId }, - }, + const query = scope.resolve(ContainerRegistrationKeys.QUERY) + const { data } = await query.graph({ + entity: "shipping_option", + filters: { id: shippingOptionId }, fields: fields, }) - const shippingOptions = await remoteQuery(queryObject) - return shippingOptions[0] + return data[0] } export const refetchBatchRules = async ( @@ -32,32 +28,28 @@ export const refetchBatchRules = async ( scope: MedusaContainer, fields: string[] ) => { - const remoteQuery = scope.resolve(ContainerRegistrationKeys.REMOTE_QUERY) + const query = scope.resolve(ContainerRegistrationKeys.QUERY) let created = Promise.resolve([]) let updated = Promise.resolve([]) if (batchResult.created.length) { - const createdQuery = remoteQueryObjectFromString({ - entryPoint: "shipping_option_rule", - variables: { + created = query + .graph({ + entity: "shipping_option_rule", filters: { id: batchResult.created.map((p) => p.id) }, - }, - fields: fields, - }) - - created = remoteQuery(createdQuery) + fields: fields, + }) + .then(({ data }) => data) } if (batchResult.updated.length) { - const updatedQuery = remoteQueryObjectFromString({ - entryPoint: "shipping_option_rule", - variables: { + updated = query + .graph({ + entity: "shipping_option_rule", filters: { id: batchResult.updated.map((p) => p.id) }, - }, - fields: fields, - }) - - updated = remoteQuery(updatedQuery) + fields: fields, + }) + .then(({ data }) => data) } const [createdRes, updatedRes] = await promiseAll([created, updated]) diff --git a/packages/medusa/src/api/admin/shipping-options/middlewares.ts b/packages/medusa/src/api/admin/shipping-options/middlewares.ts index 507201351d694..3ed040e4e1c36 100644 --- a/packages/medusa/src/api/admin/shipping-options/middlewares.ts +++ b/packages/medusa/src/api/admin/shipping-options/middlewares.ts @@ -35,6 +35,16 @@ export const adminShippingOptionRoutesMiddlewares: MiddlewareRoute[] = [ }), ], }, + { + method: ["GET"], + matcher: "/admin/shipping-options/:id", + middlewares: [ + validateAndTransformQuery( + AdminGetShippingOptionParams, + retrieveTransformQueryConfig + ), + ], + }, { method: ["POST"], matcher: "/admin/shipping-options",