Skip to content

Commit

Permalink
fix(medusa,dashboard): don't send price updates for deleted regions/c…
Browse files Browse the repository at this point in the history
…urrencies (#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
  • Loading branch information
srindom authored Sep 30, 2024
1 parent 7484cef commit 852df3f
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
34 changes: 17 additions & 17 deletions packages/admin/dashboard/src/hooks/api/shipping-options.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any>,
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
Expand Down
15 changes: 15 additions & 0 deletions packages/core/js-sdk/src/admin/shipping-option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,21 @@ export class ShippingOption {
)
}

async retrieve(
id: string,
query?: HttpTypes.SelectParams,
headers?: ClientHeaders
) {
return await this.client.fetch<HttpTypes.AdminShippingOptionResponse>(
`/admin/shipping-options/${id}`,
{
method: "GET",
headers,
query,
}
)
}

async update(
id: string,
body: HttpTypes.AdminUpdateShippingOption,
Expand Down
26 changes: 25 additions & 1 deletion packages/medusa/src/api/admin/shipping-options/[id]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AdminGetShippingOptionParamsType>,
res: MedusaResponse<HttpTypes.AdminShippingOptionResponse>
) => {
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<AdminUpdateShippingOptionType>,
Expand Down
44 changes: 18 additions & 26 deletions packages/medusa/src/api/admin/shipping-options/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,58 +6,50 @@ import {
import {
ContainerRegistrationKeys,
promiseAll,
remoteQueryObjectFromString,
} from "@medusajs/framework/utils"

export const refetchShippingOption = async (
shippingOptionId: string,
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 (
batchResult: BatchMethodResponse<ShippingOptionRuleDTO>,
scope: MedusaContainer,
fields: string[]
) => {
const remoteQuery = scope.resolve(ContainerRegistrationKeys.REMOTE_QUERY)
const query = scope.resolve(ContainerRegistrationKeys.QUERY)
let created = Promise.resolve<ShippingOptionRuleDTO[]>([])
let updated = Promise.resolve<ShippingOptionRuleDTO[]>([])

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])
Expand Down
10 changes: 10 additions & 0 deletions packages/medusa/src/api/admin/shipping-options/middlewares.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 852df3f

Please sign in to comment.