fix(medusa, utils): fix the way selects are consumed alongside the relations (#4389)

**What**
There is actually an issue with using the `fields` query params with the way the repositories are using our custom query strategy. This pr aims to fix this issue by reworking the strategy.

What we had to do was to rework the way the selects are built for each subquery in order to follow the aliasing convention and to be taken into consideration. Alongside these changes, the join used to always select everything, this needed to be changed so that if there are any selects provided for a join, the join should not select everything and let the query select the fields that are requested.

Another notable change is that all the repositories are now using the repository util in order to centralize the customization and to have a single place to update when this kind of issue arises. This means that the eager relations when using the query builder are not necessarily taken into account. For that reason, I have removed the `shipping_option` eager option in favor of explicitly asking for the relations like we started to do it in some places.

FIXES CORE-1413
This commit is contained in:
Adrien de Peretti authored and GitHub committed 2023-06-29 13:26:41 +00:00
1 parent 9760d4a96c
commit 9dcdc0041a
38 files changed
+402 -166

No files matched your search

+12
View File
@@ -0,0 +1,12 @@
---
"@medusajs/medusa": patch
"@medusajs/admin-ui": patch
"@medusajs/admin": patch
"medusa-plugin-brightpearl": patch
"medusa-plugin-segment": patch
"medusa-plugin-sendgrid": patch
"medusa-plugin-slack-notification": patch
"@medusajs/utils": patch
---
fix(medusa, utils): fix the way selects are consumed alongside the relations
@@ -109,8 +109,6 @@ describe("Batchjob with type order-export", () => {
expect(batchJob.status).toBe("completed") expect(batchJob.status).toBe("completed")
expect(batchJob.status).toBe("completed")
exportFilePath = path.resolve(__dirname, batchJob.result.file_key) exportFilePath = path.resolve(__dirname, batchJob.result.file_key)
const isFileExists = (await fs.stat(exportFilePath)).isFile() const isFileExists = (await fs.stat(exportFilePath)).isFile()
@@ -60,6 +60,7 @@ describe("Batch job of product-export type", () => {
const db = useDb() const db = useDb()
await db.teardown() await db.teardown()
// @ts-ignore
try { try {
const isFileExists = (await fs.stat(exportFilePath))?.isFile() const isFileExists = (await fs.stat(exportFilePath))?.isFile()
@@ -74,8 +75,8 @@ describe("Batch job of product-export type", () => {
await fs.unlink(exportFilePath) await fs.unlink(exportFilePath)
} }
} catch (e) { } catch (err) {
console.log(e) // noop
} }
}) })
@@ -40,7 +40,7 @@ describe("Database options", () => {
// Idle time is 1000 ms so this should timeout // Idle time is 1000 ms so this should timeout
await new Promise((resolve) => await new Promise((resolve) =>
setTimeout(() => resolve(console.log("")), 2000) setTimeout(() => resolve(undefined), 2000)
) )
// This query should fail with a QueryRunnerAlreadyReleasedError // This query should fail with a QueryRunnerAlreadyReleasedError
@@ -293,7 +293,9 @@ describe("/store/customers", () => {
}) })
expect(response.status).toEqual(200) expect(response.status).toEqual(200)
expect(response.data.orders).toEqual([ expect(response.data.orders.length).toEqual(2)
expect(response.data.orders).toEqual(
expect.arrayContaining([
expect.objectContaining({ expect.objectContaining({
display_id: 3, display_id: 3,
status: "canceled", status: "canceled",
@@ -303,6 +305,7 @@ describe("/store/customers", () => {
status: "completed", status: "completed",
}), }),
]) ])
)
expect(response.data.orders.length).toEqual(2) expect(response.data.orders.length).toEqual(2)
}) })
}) })
@@ -10,6 +10,10 @@ const {
const productSeeder = require("../../helpers/store-product-seeder") const productSeeder = require("../../helpers/store-product-seeder")
const adminSeeder = require("../../helpers/admin-seeder") const adminSeeder = require("../../helpers/admin-seeder")
const {
allowedStoreProductsFields,
defaultStoreProductsRelations,
} = require("@medusajs/medusa/dist")
jest.setTimeout(30000) jest.setTimeout(30000)
@@ -988,23 +992,28 @@ describe("/store/products", () => {
it("response contains only fields defined with `fields` param", async () => { it("response contains only fields defined with `fields` param", async () => {
const api = useApi() const api = useApi()
const fields = allowedStoreProductsFields
const response = await api.get( const response = await api.get(
"/store/products/test-product?fields=handle" `/store/products/test-product?fields=${fields.join(",")}`
) )
expect(response.status).toEqual(200) expect(response.status).toEqual(200)
expect(Object.keys(response.data.product)).toEqual([ const expectedProperties = [...fields, ...defaultStoreProductsRelations]
// fields const actualProperties = [
"handle", ...Object.keys(response.data.product),
// relations ...Object.keys(response.data.product.variants[0]).map(
"variants", (key) => `variants.${key}`
"options", ),
"images", "variants.prices.amount",
"tags", "options.values",
"collection", ]
"type",
]) expect(Object.keys(response.data.product).length).toEqual(31)
expect(actualProperties).toEqual(
expect.arrayContaining(expectedProperties)
)
}) })
}) })
}) })
@@ -5,7 +5,6 @@ const { initDb, useDb } = require("../../../../helpers/use-db")
const { setPort, useApi } = require("../../../../helpers/use-api") const { setPort, useApi } = require("../../../../helpers/use-api")
const adminSeeder = require("../../../helpers/admin-seeder") const adminSeeder = require("../../../helpers/admin-seeder")
const cartSeeder = require("../../../helpers/cart-seeder")
const { const {
simpleProductFactory, simpleProductFactory,
simpleCustomerFactory, simpleCustomerFactory,
@@ -7,6 +7,7 @@ const orderRelations = [
"discounts", "discounts",
"discounts.rule", "discounts.rule",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"payments", "payments",
"items", "items",
"fulfillments", "fulfillments",
@@ -1,8 +1,5 @@
import { MedusaError, humanizeAmount } from "medusa-core-utils" import { humanizeAmount, MedusaError } from "medusa-core-utils"
import { import { updateInventoryAndReservations } from "@medusajs/medusa"
ReservationType,
updateInventoryAndReservations,
} from "@medusajs/medusa"
import { BaseService } from "medusa-interfaces" import { BaseService } from "medusa-interfaces"
import Brightpearl from "../utils/brightpearl" import Brightpearl from "../utils/brightpearl"
@@ -111,7 +108,7 @@ class BrightpearlService extends BaseService {
httpMethod: "POST", httpMethod: "POST",
uriTemplate: `${this.options.backend_url}/brightpearl/goods-out`, uriTemplate: `${this.options.backend_url}/brightpearl/goods-out`,
bodyTemplate: bodyTemplate:
"{\"account\": \"${account-code}\", \"lifecycle_event\": \"${lifecycle-event}\", \"resource_type\": \"${resource-type}\", \"id\": \"${resource-id}\" }", '{"account": "${account-code}", "lifecycle_event": "${lifecycle-event}", "resource_type": "${resource-type}", "id": "${resource-id}" }',
contentType: "application/json", contentType: "application/json",
idSetAccepted: false, idSetAccepted: false,
}, },
@@ -1008,6 +1005,7 @@ class BrightpearlService extends BaseService {
"shipping_address", "shipping_address",
"billing_address", "billing_address",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"payments", "payments",
"sales_channel", "sales_channel",
], ],
@@ -102,6 +102,7 @@ class OrderSubscriber {
"additional_items.tax_lines", "additional_items.tax_lines",
"shipping_address", "shipping_address",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"shipping_methods.tax_lines", "shipping_methods.tax_lines",
], ],
}) })
@@ -148,6 +149,7 @@ class OrderSubscriber {
"additional_items.variant.product", "additional_items.variant.product",
"additional_items.tax_lines", "additional_items.tax_lines",
"shipping_address", "shipping_address",
"shipping_methods.shipping_option",
"shipping_methods", "shipping_methods",
], ],
}) })
@@ -46,6 +46,7 @@ class OrderSubscriber {
"discounts", "discounts",
"discounts.rule", "discounts.rule",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"payments", "payments",
"fulfillments", "fulfillments",
"returns", "returns",
@@ -56,6 +57,7 @@ class OrderSubscriber {
"swaps.return_order", "swaps.return_order",
"swaps.payment", "swaps.payment",
"swaps.shipping_methods", "swaps.shipping_methods",
"swaps.shipping_methods.shipping_option",
"swaps.shipping_address", "swaps.shipping_address",
"swaps.additional_items", "swaps.additional_items",
"swaps.fulfillments", "swaps.fulfillments",
@@ -149,6 +151,7 @@ class OrderSubscriber {
"discounts", "discounts",
"discounts.rule", "discounts.rule",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"payments", "payments",
"fulfillments", "fulfillments",
"returns", "returns",
@@ -159,6 +162,7 @@ class OrderSubscriber {
"swaps.return_order", "swaps.return_order",
"swaps.payment", "swaps.payment",
"swaps.shipping_methods", "swaps.shipping_methods",
"swaps.shipping_methods.shipping_option",
"swaps.shipping_address", "swaps.shipping_address",
"swaps.additional_items", "swaps.additional_items",
"swaps.fulfillments", "swaps.fulfillments",
@@ -257,6 +261,7 @@ class OrderSubscriber {
"discounts", "discounts",
"discounts.rule", "discounts.rule",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"payments", "payments",
"fulfillments", "fulfillments",
"items", "items",
@@ -267,6 +272,7 @@ class OrderSubscriber {
"swaps.return_order", "swaps.return_order",
"swaps.payment", "swaps.payment",
"swaps.shipping_methods", "swaps.shipping_methods",
"swaps.shipping_methods.shipping_option",
"swaps.shipping_address", "swaps.shipping_address",
"swaps.additional_items", "swaps.additional_items",
"swaps.fulfillments", "swaps.fulfillments",
@@ -850,7 +850,9 @@ class SendGridService extends NotificationService {
} }
async swapCreatedData({ id }) { async swapCreatedData({ id }) {
const store = await this.storeService_.retrieve({ where: { id: Not(IsNull()) } }) const store = await this.storeService_.retrieve({
where: { id: Not(IsNull()) },
})
const swap = await this.swapService_.retrieve(id, { const swap = await this.swapService_.retrieve(id, {
relations: [ relations: [
"additional_items", "additional_items",
@@ -913,7 +915,7 @@ class SendGridService extends NotificationService {
"shipping_total", "shipping_total",
"subtotal", "subtotal",
], ],
relations: ["items", "items.variant", "items.variant.product"] relations: ["items", "items.variant", "items.variant.product"],
}) })
const currencyCode = order.currency_code.toUpperCase() const currencyCode = order.currency_code.toUpperCase()
@@ -993,6 +995,7 @@ class SendGridService extends NotificationService {
relations: [ relations: [
"shipping_address", "shipping_address",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"shipping_methods.tax_lines", "shipping_methods.tax_lines",
"additional_items", "additional_items",
"additional_items.variant", "additional_items.variant",
@@ -1028,11 +1031,7 @@ class SendGridService extends NotificationService {
"shipping_total", "shipping_total",
"subtotal", "subtotal",
], ],
relations: [ relations: ["items", "items.variant", "items.variant.product"],
"items",
"items.variant",
"items.variant.product",
]
}) })
const returnRequest = swap.return_order const returnRequest = swap.return_order
@@ -1152,7 +1151,7 @@ class SendGridService extends NotificationService {
"order.items", "order.items",
"order.items.variant", "order.items.variant",
"order.items.variant.product", "order.items.variant.product",
"order.shipping_address" "order.shipping_address",
], ],
}) })
@@ -42,6 +42,7 @@ class SlackService extends BaseService {
"discounts", "discounts",
"discounts.rule", "discounts.rule",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"payments", "payments",
"fulfillments", "fulfillments",
"returns", "returns",
@@ -51,6 +52,7 @@ class SlackService extends BaseService {
"swaps.return_order", "swaps.return_order",
"swaps.payment", "swaps.payment",
"swaps.shipping_methods", "swaps.shipping_methods",
"swaps.shipping_methods.shipping_option",
"swaps.shipping_address", "swaps.shipping_address",
"swaps.additional_items", "swaps.additional_items",
"swaps.fulfillments", "swaps.fulfillments",
@@ -15,6 +15,7 @@ const defaultRelations = [
"return_order", "return_order",
"shipping_address", "shipping_address",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
] ]
const defaultFields = [ const defaultFields = [
@@ -34,6 +34,7 @@ export const defaultAdminSwapRelations = [
"return_order", "return_order",
"shipping_address", "shipping_address",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
] ]
export const defaultAdminSwapFields = [ export const defaultAdminSwapFields = [
@@ -83,6 +83,7 @@ export const defaultStoreOrdersRelations = [
"items", "items",
"items.variant", "items.variant",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"discounts", "discounts",
"discounts.rule", "discounts.rule",
"customer", "customer",
@@ -104,9 +104,7 @@ export const allowedStoreProductsFields = [
export const allowedStoreProductsRelations = [ export const allowedStoreProductsRelations = [
...defaultStoreProductsRelations, ...defaultStoreProductsRelations,
"variants.title",
"variants.inventory_items", "variants.inventory_items",
"variants.prices.amount",
"sales_channels", "sales_channels",
] ]
@@ -24,10 +24,12 @@ export const defaultStoreSwapRelations = [
"additional_items.variant", "additional_items.variant",
"return_order", "return_order",
"return_order.shipping_method", "return_order.shipping_method",
"return_order.shipping_method.shipping_option",
"fulfillments", "fulfillments",
"payment", "payment",
"shipping_address", "shipping_address",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"cart", "cart",
] ]
export const defaultStoreSwapFields: FindConfig<Swap>["select"] = [ export const defaultStoreSwapFields: FindConfig<Swap>["select"] = [
@@ -76,7 +76,7 @@ export class ShippingMethod {
@JoinColumn({ name: "return_id" }) @JoinColumn({ name: "return_id" })
return_order: Return return_order: Return
@ManyToOne(() => ShippingOption, { eager: true }) @ManyToOne(() => ShippingOption)
@JoinColumn({ name: "shipping_option_id" }) @JoinColumn({ name: "shipping_option_id" })
shipping_option: ShippingOption shipping_option: ShippingOption
+13 -23
View File
@@ -1,8 +1,12 @@
import { objectToStringPath } from "@medusajs/utils" import { objectToStringPath } from "@medusajs/utils"
import { flatten, groupBy, map, merge } from "lodash" import { FindManyOptions, FindOptionsRelations } from "typeorm"
import { FindManyOptions, FindOptionsRelations, In } from "typeorm"
import { dataSource } from "../loaders/database" import { dataSource } from "../loaders/database"
import { Cart } from "../models" import { Cart } from "../models"
import {
getGroupedRelations,
mergeEntitiesWithRelations,
queryEntityWithIds,
} from "../utils/repository"
export const CartRepository = dataSource.getRepository(Cart).extend({ export const CartRepository = dataSource.getRepository(Cart).extend({
async findWithRelations( async findWithRelations(
@@ -12,31 +16,17 @@ export const CartRepository = dataSource.getRepository(Cart).extend({
const entities = await this.find(optionsWithoutRelations) const entities = await this.find(optionsWithoutRelations)
const entitiesIds = entities.map(({ id }) => id) const entitiesIds = entities.map(({ id }) => id)
const groupedRelations = {} const groupedRelations = getGroupedRelations(objectToStringPath(relations))
for (const rel of objectToStringPath(relations)) {
const [topLevel] = rel.split(".")
if (groupedRelations[topLevel]) {
groupedRelations[topLevel].push(rel)
} else {
groupedRelations[topLevel] = [rel]
}
}
const entitiesIdsWithRelations = await Promise.all( const entitiesIdsWithRelations = await queryEntityWithIds({
Object.entries(groupedRelations).map(async ([_, rels]) => { repository: this,
return this.find({ entityIds: entitiesIds,
where: { id: In(entitiesIds) }, groupedRelations,
select: ["id"], select: ["id"],
relations: rels as string[],
}) })
})
).then(flatten)
const entitiesAndRelations = entitiesIdsWithRelations.concat(entities)
const entitiesAndRelationsById = groupBy(entitiesAndRelations, "id") const entitiesAndRelations = entities.concat(entitiesIdsWithRelations)
return map(entitiesAndRelationsById, (entityAndRelations) => return mergeEntitiesWithRelations<Cart>(entitiesAndRelations)
merge({}, ...entityAndRelations)
)
}, },
async findOneWithRelations( async findOneWithRelations(
@@ -82,9 +82,9 @@ export const CustomerGroupRepository = dataSource
: { ...idsOrOptionsWithoutRelations.order } : { ...idsOrOptionsWithoutRelations.order }
const originalSelect = isOptionsArray const originalSelect = isOptionsArray
? undefined ? undefined
: (objectToStringPath( : (objectToStringPath(idsOrOptionsWithoutRelations.select, {
idsOrOptionsWithoutRelations.select includeParentPropertyFields: false,
) as (keyof CustomerGroup)[]) }) as (keyof CustomerGroup)[])
const clonedOptions = isOptionsArray const clonedOptions = isOptionsArray
? idsOrOptionsWithoutRelations ? idsOrOptionsWithoutRelations
: cloneDeep(idsOrOptionsWithoutRelations) : cloneDeep(idsOrOptionsWithoutRelations)
@@ -169,7 +169,7 @@ export const CustomerGroupRepository = dataSource
withDeleted, withDeleted,
}) })
const entitiesAndRelations = entitiesIdsWithRelations.concat(entities) const entitiesAndRelations = entities.concat(entitiesIdsWithRelations)
const entitiesToReturn = const entitiesToReturn =
mergeEntitiesWithRelations<CustomerGroup>(entitiesAndRelations) mergeEntitiesWithRelations<CustomerGroup>(entitiesAndRelations)
+8 -15
View File
@@ -1,8 +1,12 @@
import { objectToStringPath } from "@medusajs/utils" import { objectToStringPath } from "@medusajs/utils"
import { flatten, groupBy, map, merge } from "lodash" import { flatten } from "lodash"
import { FindManyOptions, FindOptionsRelations, In } from "typeorm" import { FindManyOptions, FindOptionsRelations, In } from "typeorm"
import { dataSource } from "../loaders/database" import { dataSource } from "../loaders/database"
import { Order } from "../models" import { Order } from "../models"
import {
getGroupedRelations,
mergeEntitiesWithRelations,
} from "../utils/repository"
const ITEMS_REL_NAME = "items" const ITEMS_REL_NAME = "items"
const REGION_REL_NAME = "region" const REGION_REL_NAME = "region"
@@ -15,15 +19,7 @@ export const OrderRepository = dataSource.getRepository(Order).extend({
const entities = await this.find(optionsWithoutRelations) const entities = await this.find(optionsWithoutRelations)
const entitiesIds = entities.map(({ id }) => id) const entitiesIds = entities.map(({ id }) => id)
const groupedRelations: { [topLevel: string]: string[] } = {} const groupedRelations = getGroupedRelations(objectToStringPath(relations))
for (const rel of objectToStringPath(relations)) {
const [topLevel] = rel.split(".")
if (groupedRelations[topLevel]) {
groupedRelations[topLevel].push(rel)
} else {
groupedRelations[topLevel] = [rel]
}
}
const entitiesIdsWithRelations = await Promise.all( const entitiesIdsWithRelations = await Promise.all(
Object.entries(groupedRelations).map(async ([topLevel, rels]) => { Object.entries(groupedRelations).map(async ([topLevel, rels]) => {
@@ -39,11 +35,8 @@ export const OrderRepository = dataSource.getRepository(Order).extend({
}) })
).then(flatten) ).then(flatten)
const entitiesAndRelations = entitiesIdsWithRelations.concat(entities) const entitiesAndRelations = entities.concat(entitiesIdsWithRelations)
return mergeEntitiesWithRelations<Order>(entitiesAndRelations)
const entitiesAndRelationsById = groupBy(entitiesAndRelations, "id")
return map(entities, (e) => merge({}, ...entitiesAndRelationsById[e.id]))
}, },
async findOneWithRelations( async findOneWithRelations(
@@ -1,10 +1,9 @@
import { import {
Brackets, DeleteResult,
FindOneOptions,
FindOptionsWhere, FindOptionsWhere,
ILike, ILike,
DeleteResult,
In, In,
FindOneOptions,
} from "typeorm" } from "typeorm"
import { ProductCategory } from "../models/product-category" import { ProductCategory } from "../models/product-category"
import { ExtendedFindConfig, QuerySelector } from "../types/common" import { ExtendedFindConfig, QuerySelector } from "../types/common"
@@ -44,7 +43,9 @@ export const ProductCategoryRepository = dataSource
const options_ = { ...options } const options_ = { ...options }
options_.where = options_.where as FindOptionsWhere<ProductCategory> options_.where = options_.where as FindOptionsWhere<ProductCategory>
const columnsSelected = objectToStringPath(options_.select) const columnsSelected = objectToStringPath(options_.select, {
includeParentPropertyFields: false,
})
const relationsSelected = objectToStringPath(options_.relations) const relationsSelected = objectToStringPath(options_.relations)
const fetchSelectColumns = (relationName: string): string[] => { const fetchSelectColumns = (relationName: string): string[] => {
+18 -15
View File
@@ -18,10 +18,11 @@ import { ExtendedFindConfig } from "@medusajs/types"
import { import {
applyOrdering, applyOrdering,
getGroupedRelations, getGroupedRelations,
mergeEntitiesWithRelations,
queryEntityWithIds, queryEntityWithIds,
queryEntityWithoutRelations, queryEntityWithoutRelations,
} from "../utils/repository" } from "../utils/repository"
import { cloneDeep, groupBy, map, merge } from "lodash" import { cloneDeep } from "lodash"
export type DefaultWithoutRelations = Omit< export type DefaultWithoutRelations = Omit<
ExtendedFindConfig<Product>, ExtendedFindConfig<Product>,
@@ -213,11 +214,13 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
customJoinBuilders: [ customJoinBuilders: [
(queryBuilder, alias, topLevel) => { (queryBuilder, alias, topLevel) => {
if (topLevel === "variants") { if (topLevel === "variants") {
queryBuilder.leftJoinAndSelect( const joinMethod = select.filter(
`${alias}.${topLevel}`, (key) => !!key.match(/^variants\.\w+$/i)
topLevel, ).length
`${topLevel}.deleted_at IS NULL` ? "leftJoin"
) : "leftJoinAndSelect"
queryBuilder[joinMethod](`${alias}.${topLevel}`, topLevel)
if ( if (
!Object.keys(order!).some((key) => key.startsWith("variants")) !Object.keys(order!).some((key) => key.startsWith("variants"))
@@ -228,7 +231,8 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
return false return false
} }
return true
return
}, },
(queryBuilder, alias, topLevel) => { (queryBuilder, alias, topLevel) => {
if (topLevel === "categories") { if (topLevel === "categories") {
@@ -245,7 +249,7 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
return false return false
} }
return true return
}, },
], ],
}) })
@@ -479,9 +483,9 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
: { ...idsOrOptionsWithoutRelations.order } : { ...idsOrOptionsWithoutRelations.order }
const originalSelect = isOptionsArray const originalSelect = isOptionsArray
? undefined ? undefined
: (objectToStringPath( : (objectToStringPath(idsOrOptionsWithoutRelations.select, {
idsOrOptionsWithoutRelations.select includeParentPropertyFields: false,
) as (keyof Product)[]) }) as (keyof Product)[])
const clonedOptions = isOptionsArray const clonedOptions = isOptionsArray
? idsOrOptionsWithoutRelations ? idsOrOptionsWithoutRelations
: cloneDeep(idsOrOptionsWithoutRelations) : cloneDeep(idsOrOptionsWithoutRelations)
@@ -543,10 +547,9 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
withDeleted, withDeleted,
}) })
const entitiesAndRelations = groupBy(entitiesIdsWithRelations, "id") const entitiesAndRelations = entities.concat(entitiesIdsWithRelations)
const entitiesToReturn = map(entitiesIds, (id) => const entitiesToReturn =
merge({}, ...entitiesAndRelations[id]) mergeEntitiesWithRelations<Product>(entitiesAndRelations)
)
return [entitiesToReturn, count] return [entitiesToReturn, count]
}, },
+2 -1
View File
@@ -34,7 +34,8 @@ export const TaxRateRepository = dataSource.getRepository(TaxRate).extend({
if (isDefined(findOptions.select)) { if (isDefined(findOptions.select)) {
const selectableCols: (keyof TaxRate)[] = [] const selectableCols: (keyof TaxRate)[] = []
const legacySelect = objectToStringPath( const legacySelect = objectToStringPath(
findOptions.select as FindOptionsSelect<TaxRate> findOptions.select as FindOptionsSelect<TaxRate>,
{ includeParentPropertyFields: false }
) as (keyof TaxRate)[] ) as (keyof TaxRate)[]
for (const k of legacySelect) { for (const k of legacySelect) {
if (!resolveableFields.includes(k)) { if (!resolveableFields.includes(k)) {
@@ -865,7 +865,9 @@ describe("CartService", () => {
payment_sessions: true, payment_sessions: true,
region: { countries: true }, region: { countries: true },
shipping_address: true, shipping_address: true,
shipping_methods: true, shipping_methods: {
shipping_option: true,
},
}), }),
expect.objectContaining({ expect.objectContaining({
select: undefined, select: undefined,
+27 -30
View File
@@ -12,16 +12,12 @@ import {
ProductVariantInventoryService, ProductVariantInventoryService,
ReturnService, ReturnService,
ShippingOptionService, ShippingOptionService,
TotalsService TotalsService,
} from "../index" } from "../index"
import LineItemAdjustmentService from "../line-item-adjustment" import LineItemAdjustmentService from "../line-item-adjustment"
import SwapService from "../swap" import SwapService from "../swap"
import { import { LineItemAdjustmentServiceMock } from "../__mocks__/line-item-adjustment"
LineItemAdjustmentServiceMock import { ProductVariantInventoryServiceMock } from "../__mocks__/product-variant-inventory"
} from "../__mocks__/line-item-adjustment"
import {
ProductVariantInventoryServiceMock
} from "../__mocks__/product-variant-inventory"
/* ******************** DEFAULT REPOSITORY MOCKS ******************** */ /* ******************** DEFAULT REPOSITORY MOCKS ******************** */
@@ -273,32 +269,33 @@ describe("SwapService", () => {
expect(swapRepo.findOne).toHaveBeenCalledWith({ expect(swapRepo.findOne).toHaveBeenCalledWith({
relations: { relations: {
additional_items: { additional_items: {
variant: true variant: true,
}, },
order: { order: {
claims: { claims: {
additional_items: true additional_items: true,
}, },
discounts: { discounts: {
rule: true rule: true,
}, },
items: { items: {
variant: { variant: {
product: true product: true,
} },
}, },
swaps: { swaps: {
additional_items: true additional_items: true,
} },
}, },
return_order: { return_order: {
items: true, items: true,
shipping_method: { shipping_method: {
tax_lines: true shipping_option: true,
} tax_lines: true,
}
}, },
where: { id: IdMap.getId("swap-1") } },
},
where: { id: IdMap.getId("swap-1") },
}) })
expect(lineItemService.createReturnLines).toHaveBeenCalledTimes(1) expect(lineItemService.createReturnLines).toHaveBeenCalledTimes(1)
@@ -413,9 +410,7 @@ describe("SwapService", () => {
describe("success", () => { describe("success", () => {
const lineItemService = { const lineItemService = {
generate: jest generate: jest.fn().mockImplementation(({ variantId, quantity }) => {
.fn()
.mockImplementation(({ variantId, quantity }) => {
return { return {
unit_price: 100, unit_price: 100,
variant_id: variantId, variant_id: variantId,
@@ -457,13 +452,16 @@ describe("SwapService", () => {
) )
expect(lineItemService.generate).toHaveBeenCalledTimes(1) expect(lineItemService.generate).toHaveBeenCalledTimes(1)
expect(lineItemService.generate).toHaveBeenCalledWith({ expect(lineItemService.generate).toHaveBeenCalledWith(
{
quantity: 1, quantity: 1,
variantId: IdMap.getId("new-variant") variantId: IdMap.getId("new-variant"),
}, { },
"cart": undefined, {
region_id: IdMap.getId("region") cart: undefined,
}) region_id: IdMap.getId("region"),
}
)
}) })
it("creates swap", async () => { it("creates swap", async () => {
@@ -605,7 +603,7 @@ describe("SwapService", () => {
shipping_methods: existing.shipping_methods, shipping_methods: existing.shipping_methods,
}, },
[{ item_id: "1234", quantity: 2 }], [{ item_id: "1234", quantity: 2 }],
{ swap_id: IdMap.getId("swap"), metadata: {} }, { swap_id: IdMap.getId("swap"), metadata: {} }
) )
}) })
}) })
@@ -798,8 +796,7 @@ describe("SwapService", () => {
describe("failure", () => { describe("failure", () => {
const swapRepo = MockRepository({ const swapRepo = MockRepository({
findOne: () => findOne: () => Promise.resolve({ canceled_at: new Date() }),
Promise.resolve({ canceled_at: new Date() }),
}) })
const swapService = new SwapService({ const swapService = new SwapService({
+12 -2
View File
@@ -1004,7 +1004,7 @@ class CartService extends TransactionBaseService {
* shipping discount * shipping discount
* If a free shipping is present, we set shipping methods price to 0 * If a free shipping is present, we set shipping methods price to 0
* if a free shipping was present, we set shipping methods to original amount * if a free shipping was present, we set shipping methods to original amount
* @param cart - the the cart to adjust free shipping for * @param cart - the cart to adjust free shipping for
* @param shouldAdd - flag to indicate, if we should add or remove * @param shouldAdd - flag to indicate, if we should add or remove
* @return void * @return void
*/ */
@@ -1058,6 +1058,7 @@ class CartService extends TransactionBaseService {
"items.variant", "items.variant",
"items.variant.product", "items.variant.product",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"shipping_address", "shipping_address",
"billing_address", "billing_address",
"gift_cards", "gift_cards",
@@ -1530,6 +1531,7 @@ class CartService extends TransactionBaseService {
"discounts.rule", "discounts.rule",
"payment_sessions", "payment_sessions",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
], ],
}) })
@@ -1829,6 +1831,7 @@ class CartService extends TransactionBaseService {
"discounts.rule", "discounts.rule",
"gift_cards", "gift_cards",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"billing_address", "billing_address",
"shipping_address", "shipping_address",
"region", "region",
@@ -2170,7 +2173,12 @@ class CartService extends TransactionBaseService {
} }
const updatedCart = await this.retrieve(cart.id, { const updatedCart = await this.retrieve(cart.id, {
relations: ["discounts", "discounts.rule", "shipping_methods"], relations: [
"discounts",
"discounts.rule",
"shipping_methods",
"shipping_methods.shipping_option",
],
}) })
// if cart has freeshipping, adjust price // if cart has freeshipping, adjust price
@@ -2537,6 +2545,7 @@ class CartService extends TransactionBaseService {
"region.tax_rates", "region.tax_rates",
"shipping_address", "shipping_address",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
], ],
}) })
@@ -2559,6 +2568,7 @@ class CartService extends TransactionBaseService {
"items", "items",
"items.tax_lines", "items.tax_lines",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"shipping_methods.tax_lines", "shipping_methods.tax_lines",
], ],
}) })
+1
View File
@@ -530,6 +530,7 @@ export default class ClaimService extends TransactionBaseService {
"additional_items.variant", "additional_items.variant",
"additional_items.variant.product", "additional_items.variant.product",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"shipping_methods.tax_lines", "shipping_methods.tax_lines",
"shipping_address", "shipping_address",
"order", "order",
@@ -514,6 +514,7 @@ export default class OrderEditService extends TransactionBaseService {
"items.variant", "items.variant",
"region.tax_rates", "region.tax_rates",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"shipping_methods.tax_lines", "shipping_methods.tax_lines",
], ],
}) })
+1
View File
@@ -596,6 +596,7 @@ class ReturnService extends TransactionBaseService {
"discounts.rule", "discounts.rule",
"refunds", "refunds",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"region", "region",
"swaps", "swaps",
"swaps.additional_items", "swaps.additional_items",
+3 -1
View File
@@ -29,7 +29,7 @@ import {
} from "./index" } from "./index"
import { EntityManager, In } from "typeorm" import { EntityManager, In } from "typeorm"
import { FindConfig, Selector, WithRequiredProperty } from "../types/common" import { FindConfig, Selector, WithRequiredProperty } from "../types/common"
import { MedusaError, isDefined } from "medusa-core-utils" import { isDefined, MedusaError } from "medusa-core-utils"
import { buildQuery, setMetadata, validateId } from "../utils" import { buildQuery, setMetadata, validateId } from "../utils"
import { CreateShipmentConfig } from "../types/fulfillment" import { CreateShipmentConfig } from "../types/fulfillment"
@@ -579,6 +579,7 @@ class SwapService extends TransactionBaseService {
"return_order", "return_order",
"return_order.items", "return_order.items",
"return_order.shipping_method", "return_order.shipping_method",
"return_order.shipping_method.shipping_option",
"return_order.shipping_method.tax_lines", "return_order.shipping_method.tax_lines",
], ],
}) })
@@ -918,6 +919,7 @@ class SwapService extends TransactionBaseService {
"additional_items.variant", "additional_items.variant",
"additional_items.variant.product", "additional_items.variant.product",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"shipping_methods.tax_lines", "shipping_methods.tax_lines",
"order", "order",
"order.region", "order.region",
@@ -197,6 +197,7 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy {
"region.tax_rates", "region.tax_rates",
"shipping_address", "shipping_address",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
], ],
}) })
+5
View File
@@ -133,9 +133,12 @@ export const defaultAdminOrdersRelations = [
"returns.items", "returns.items",
"returns.items.reason", "returns.items.reason",
"returns.shipping_method", "returns.shipping_method",
"returns.shipping_method.shipping_option",
"returns.shipping_method.tax_lines", "returns.shipping_method.tax_lines",
"shipping_address", "shipping_address",
"shipping_methods", "shipping_methods",
"shipping_methods.shipping_option",
"shipping_methods.tax_lines",
"swaps", "swaps",
"swaps.additional_items", "swaps.additional_items",
"swaps.additional_items.variant", "swaps.additional_items.variant",
@@ -144,9 +147,11 @@ export const defaultAdminOrdersRelations = [
"swaps.payment", "swaps.payment",
"swaps.return_order", "swaps.return_order",
"swaps.return_order.shipping_method", "swaps.return_order.shipping_method",
"swaps.return_order.shipping_method.shipping_option",
"swaps.return_order.shipping_method.tax_lines", "swaps.return_order.shipping_method.tax_lines",
"swaps.shipping_address", "swaps.shipping_address",
"swaps.shipping_methods", "swaps.shipping_methods",
"swaps.shipping_methods.shipping_option",
"swaps.shipping_methods.tax_lines", "swaps.shipping_methods.tax_lines",
// "claims.claim_items.tags", // "claims.claim_items.tags",
] ]
+55 -26
View File
@@ -7,6 +7,11 @@ import {
} from "typeorm" } from "typeorm"
import { ExtendedFindConfig } from "../types/common" import { ExtendedFindConfig } from "../types/common"
// Regex matches all '.' except the rightmost
export const positiveLookaheadDotReplacer = new RegExp(/\.(?=[^.]*\.)/, "g")
// Replace all '.' with '__' to avoid typeorm's automatic aliasing
export const dotReplacer = new RegExp(/\./, "g")
/** /**
* Custom query entity, it is part of the creation of a custom findWithRelationsAndCount needs. * Custom query entity, it is part of the creation of a custom findWithRelationsAndCount needs.
* Allow to query the relations for the specified entity ids * Allow to query the relations for the specified entity ids
@@ -35,62 +40,86 @@ export async function queryEntityWithIds<T extends ObjectLiteral>({
qb: SelectQueryBuilder<T>, qb: SelectQueryBuilder<T>,
alias: string, alias: string,
toplevel: string toplevel: string
) => boolean)[] ) => false | undefined)[]
}): Promise<T[]> { }): Promise<T[]> {
const alias = repository.metadata.name.toLowerCase() const alias = repository.metadata.name.toLowerCase()
return await Promise.all( return await Promise.all(
Object.entries(groupedRelations).map(async ([toplevel, rels]) => { Object.entries(groupedRelations).map(
async ([toplevel, topLevelRelations]) => {
let querybuilder = repository.createQueryBuilder(alias) let querybuilder = repository.createQueryBuilder(alias)
if (select && select.length) { if (select?.length) {
querybuilder.select(select.map((f) => `${alias}.${f as string}`)) querybuilder.select(
(select as string[])
.filter(function (s) {
return s.startsWith(toplevel) || !s.includes(".")
})
.map((column) => {
// In case the column is the toplevel relation, we need to replace the dot with a double underscore if it also contains top level relations
if (column.includes(toplevel)) {
return topLevelRelations.some((rel) => column.includes(rel))
? column.replace(positiveLookaheadDotReplacer, "__")
: column
}
return `${alias}.${column}`
})
)
} }
let shouldAttachDefault = true let shouldAttachDefault: boolean | undefined = true
for (const customJoinBuilder of customJoinBuilders) { for (const customJoinBuilder of customJoinBuilders) {
const result = customJoinBuilder(querybuilder, alias, toplevel) const result = customJoinBuilder(querybuilder, alias, toplevel)
if (result === undefined) {
continue
}
shouldAttachDefault = shouldAttachDefault && result shouldAttachDefault = shouldAttachDefault && result
} }
// If the toplevel relation has been attached with a customJoinBuilder and the function return false then
// do not attach the toplevel join bellow.
if (shouldAttachDefault) { if (shouldAttachDefault) {
querybuilder = querybuilder.leftJoinAndSelect( const regexp = new RegExp(`^${toplevel}\\.\\w+$`)
const joinMethod = (select as string[]).filter(
(key) => !!key.match(regexp)
).length
? "leftJoin"
: "leftJoinAndSelect"
querybuilder = querybuilder[joinMethod](
`${alias}.${toplevel}`, `${alias}.${toplevel}`,
toplevel toplevel
) )
} }
for (const rel of rels) { for (const rel of topLevelRelations) {
const [_, rest] = rel.split(".") const [_, rest] = rel.split(".")
if (!rest) { if (!rest) {
continue continue
} }
querybuilder = querybuilder.leftJoinAndSelect(
// Regex matches all '.' except the rightmost const regexp = new RegExp(`^${rel}\\.\\w+$`)
rel.replace(/\.(?=[^.]*\.)/g, "__"), const joinMethod = (select as string[]).filter(
// Replace all '.' with '__' to avoid typeorm's automatic aliasing (key) => !!key.match(regexp)
rel.replace(/\./g, "__") ).length
? "leftJoin"
: "leftJoinAndSelect"
querybuilder = querybuilder[joinMethod](
rel.replace(positiveLookaheadDotReplacer, "__"),
rel.replace(dotReplacer, "__")
) )
} }
if (withDeleted) { querybuilder = querybuilder.where(`${alias}.id IN (:...entitiesIds)`, {
querybuilder = querybuilder
.where(`${alias}.id IN (:...entitiesIds)`, {
entitiesIds: entityIds, entitiesIds: entityIds,
}) })
.withDeleted()
} else { if (withDeleted) {
querybuilder = querybuilder.where( querybuilder.withDeleted()
`${alias}.deleted_at IS NULL AND ${alias}.id IN (:...entitiesIds)`,
{
entitiesIds: entityIds,
}
)
} }
return querybuilder.getMany() return querybuilder.getMany()
}) }
)
).then(flatten) ).then(flatten)
} }
@@ -0,0 +1,91 @@
import { buildSelects } from "../build-query"
describe("buildSelects", () => {
it("successfully build back select object shape to list", () => {
const q = buildSelects([
"order",
"order.items",
"order.swaps",
"order.swaps.additional_items",
"order.discounts",
"order.discounts.rule",
"order.claims",
"order.claims.additional_items",
"additional_items",
"additional_items.variant",
"return_order",
"return_order.items",
"return_order.shipping_method",
"return_order.shipping_method.tax_lines",
])
expect(q).toEqual({
order: {
items: true,
swaps: {
additional_items: true,
},
discounts: {
rule: true,
},
claims: {
additional_items: true,
},
},
additional_items: {
variant: true,
},
return_order: {
items: true,
shipping_method: {
tax_lines: true,
},
},
})
})
})
describe("buildSelects", () => {
it("successfully build back select object shape to list", () => {
const q = buildSelects([
"order",
"order.items",
"order.swaps",
"order.swaps.additional_items",
"order.discounts",
"order.discounts.rule",
"order.claims",
"order.claims.additional_items",
"additional_items",
"additional_items.variant",
"return_order",
"return_order.items",
"return_order.shipping_method",
"return_order.shipping_method.tax_lines",
])
expect(q).toEqual({
order: {
items: true,
swaps: {
additional_items: true,
},
discounts: {
rule: true,
},
claims: {
additional_items: true,
},
},
additional_items: {
variant: true,
},
return_order: {
items: true,
shipping_method: {
tax_lines: true,
},
},
})
})
})
@@ -0,0 +1,42 @@
import { objectToStringPath } from "../object-to-string-path"
describe("objectToStringPath", function () {
it("should return only the properties path of the properties that are set to true", function () {
const res = objectToStringPath(
{
product: true,
variants: {
title: true,
prices: {
amount: true,
},
},
},
{
includeParentPropertyFields: false,
}
)
expect(res).toEqual(["product", "variants.title", "variants.prices.amount"])
})
it("should return a string path from an object including properties that are object and contains other properties set to true", function () {
const res = objectToStringPath({
product: true,
variants: {
title: true,
prices: {
amount: true,
},
},
})
expect(res).toEqual([
"product",
"variants",
"variants.title",
"variants.prices",
"variants.prices.amount",
])
})
})
@@ -4,7 +4,8 @@ import { isObject } from "./is-object"
* Converts a structure of find options to an * Converts a structure of find options to an
* array of string paths * array of string paths
* @example * @example
* input: { * // With `includeTruePropertiesOnly` default value set to false
* const result = objectToStringPath({
* test: { * test: {
* test1: true, * test1: true,
* test2: true, * test2: true,
@@ -13,20 +14,50 @@ import { isObject } from "./is-object"
* }, * },
* }, * },
* test2: true * test2: true
* } * })
* output: ['test.test1', 'test.test2', 'test.test3.test4', 'test2'] * console.log(result)
* @param input * // output: ['test', 'test.test1', 'test.test2', 'test.test3', 'test.test3.test4', 'test2']
*
* @example
* // With `includeTruePropertiesOnly` set to true
* const result = objectToStringPath({
* test: {
* test1: true,
* test2: true,
* test3: {
* test4: true
* },
* },
* test2: true
* }, {
* includeTruePropertiesOnly: true
* })
* console.log(result)
* // output: ['test.test1', 'test.test2', 'test.test3.test4', 'test2']
*
* @param {InputObject} input
* @param {boolean} includeParentPropertyFields If set to true (example 1), all properties will be included as well as the parents,
* otherwise (example 2) all properties path set to true will included, excluded the parents
*/ */
export function objectToStringPath(input: object = {}): string[] { export function objectToStringPath(
input: object = {},
{ includeParentPropertyFields }: { includeParentPropertyFields: boolean } = {
includeParentPropertyFields: true,
}
): string[] {
if (!isObject(input) || !Object.keys(input).length) { if (!isObject(input) || !Object.keys(input).length) {
return [] return []
} }
const output: Set<string> = new Set(Object.keys(input)) const output: Set<string> = includeParentPropertyFields
? new Set(Object.keys(input))
: new Set()
for (const key of Object.keys(input)) { for (const key of Object.keys(input)) {
if (input[key] != undefined && typeof input[key] === "object") { if (isObject(input[key])) {
const deepRes = objectToStringPath(input[key]) const deepRes = objectToStringPath(input[key], {
includeParentPropertyFields,
})
const items = deepRes.reduce((acc, val) => { const items = deepRes.reduce((acc, val) => {
acc.push(`${key}.${val}`) acc.push(`${key}.${val}`)
@@ -37,8 +68,10 @@ export function objectToStringPath(input: object = {}): string[] {
continue continue
} }
if (isObject(key) || input[key] === true) {
output.add(key) output.add(key)
} }
}
return Array.from(output) return Array.from(output)
} }