From e26cda4b6afb7fb25f0b0a7a7ce20b7f914d35db Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Tue, 30 Apr 2024 18:45:17 +0200 Subject: [PATCH] feat(medusa, core-flows, types): Allow to update the rules from a shipping options (#7175) **What** Add support for the following operations - update rules from the update shipping options end point - update rules from the batch update end point Also added some improvements, that can be revisited later - Add a rule value normalizer, jsonb will transform the input value to a [primitive](https://www.postgresql.org/docs/current/datatype-json.html#JSON-TYPE-MAPPING-TABLE) when possible meaning that passing `"true"` will result in storing `true` and not the string. The normalizer takes care of that --- .changeset/mighty-jokes-wave.md | 8 + .../admin/shipping-options.spec.ts | 15 + .../workflows/batch-shipping-options-rules.ts | 326 ++++++++++++++++++ .../steps/create-shipping-option-rules.ts | 8 +- .../steps/delete-shipping-option-rules.ts | 7 +- .../steps/update-shipping-option-rules.ts | 51 +++ .../workflows/batch-shipping-option-rules.ts | 33 +- .../services/fulfillment-module-service.ts | 24 +- packages/fulfillment/src/utils/utils.ts | 29 +- .../admin/shipping-options/validators.ts | 1 + packages/types/src/fulfillment/workflows.ts | 9 +- 11 files changed, 470 insertions(+), 41 deletions(-) create mode 100644 .changeset/mighty-jokes-wave.md create mode 100644 integration-tests/modules/__tests__/shipping-options/workflows/batch-shipping-options-rules.ts create mode 100644 packages/core-flows/src/fulfillment/steps/update-shipping-option-rules.ts diff --git a/.changeset/mighty-jokes-wave.md b/.changeset/mighty-jokes-wave.md new file mode 100644 index 0000000000..c965b812cd --- /dev/null +++ b/.changeset/mighty-jokes-wave.md @@ -0,0 +1,8 @@ +--- +"@medusajs/medusa": patch +"@medusajs/core-flows": patch +"@medusajs/fulfillment": patch +"@medusajs/types": patch +--- + +feat(medusa, core-flows, types): Allow to update the rules from a shipping options diff --git a/integration-tests/modules/__tests__/shipping-options/admin/shipping-options.spec.ts b/integration-tests/modules/__tests__/shipping-options/admin/shipping-options.spec.ts index a99ca05383..b65f111d25 100644 --- a/integration-tests/modules/__tests__/shipping-options/admin/shipping-options.spec.ts +++ b/integration-tests/modules/__tests__/shipping-options/admin/shipping-options.spec.ts @@ -254,6 +254,14 @@ medusaIntegrationTestRunner({ amount: 10000, }, ], + rules: [ + shippingOptionRule, + { + operator: RuleOperator.EQ, + attribute: "new_attr", + value: "true", + }, + ], } const updateResponse = await api.post( @@ -264,6 +272,7 @@ medusaIntegrationTestRunner({ expect(updateResponse.status).toEqual(200) expect(updateResponse.data.shipping_option.prices).toHaveLength(2) + expect(updateResponse.data.shipping_option.rules).toHaveLength(2) expect(updateResponse.data.shipping_option).toEqual( expect.objectContaining({ id: expect.any(String), @@ -301,6 +310,12 @@ medusaIntegrationTestRunner({ attribute: "old_attr", value: "old value", }), + expect.objectContaining({ + id: expect.any(String), + operator: "eq", + attribute: "new_attr", + value: "true", + }), ]), }) ) diff --git a/integration-tests/modules/__tests__/shipping-options/workflows/batch-shipping-options-rules.ts b/integration-tests/modules/__tests__/shipping-options/workflows/batch-shipping-options-rules.ts new file mode 100644 index 0000000000..a743d7bf3e --- /dev/null +++ b/integration-tests/modules/__tests__/shipping-options/workflows/batch-shipping-options-rules.ts @@ -0,0 +1,326 @@ +import { ModuleRegistrationName } from "@medusajs/modules-sdk" +import { + BatchWorkflowInput, + CreateShippingOptionRuleDTO, + FulfillmentSetDTO, + FulfillmentWorkflow, + IFulfillmentModuleService, + IRegionModuleService, + ServiceZoneDTO, + ShippingProfileDTO, + UpdateShippingOptionRuleDTO, +} from "@medusajs/types" +import { medusaIntegrationTestRunner } from "medusa-test-utils/dist" +import { + batchShippingOptionRulesWorkflow, + createShippingOptionsWorkflow, +} from "@medusajs/core-flows" +import { + ContainerRegistrationKeys, + remoteQueryObjectFromString, + RuleOperator, +} from "@medusajs/utils" + +jest.setTimeout(100000) + +const env = { MEDUSA_FF_MEDUSA_V2: true } +const provider_id = "manual_test-provider" + +async function createShippingOptionFixture({ + container, + serviceZone, + shippingProfile, +}) { + const regionService = container.resolve( + ModuleRegistrationName.REGION + ) as IRegionModuleService + + const [region] = await regionService.create([ + { + name: "Test region", + currency_code: "eur", + countries: ["fr"], + }, + ]) + + const shippingOptionData: FulfillmentWorkflow.CreateShippingOptionsWorkflowInput = + { + name: "Test shipping option", + price_type: "flat", + service_zone_id: serviceZone.id, + shipping_profile_id: shippingProfile.id, + provider_id, + type: { + code: "manual-type", + label: "Manual Type", + description: "Manual Type Description", + }, + prices: [ + { + currency_code: "usd", + amount: 10, + }, + { + region_id: region.id, + amount: 100, + }, + ], + rules: [ + { + attribute: "total", + operator: RuleOperator.EQ, + value: "100", + }, + { + attribute: "is_store", + operator: RuleOperator.EQ, + value: "true", + }, + ], + } + + const { result } = await createShippingOptionsWorkflow(container).run({ + input: [shippingOptionData], + }) + + const remoteQuery = container.resolve(ContainerRegistrationKeys.REMOTE_QUERY) + + const remoteQueryObject = remoteQueryObjectFromString({ + entryPoint: "shipping_option", + variables: { + id: result[0].id, + }, + fields: [ + "id", + "name", + "price_type", + "service_zone_id", + "shipping_profile_id", + "provider_id", + "data", + "metadata", + "type.*", + "created_at", + "updated_at", + "deleted_at", + "shipping_option_type_id", + "prices.*", + "rules.*", + ], + }) + + const [createdShippingOption] = await remoteQuery(remoteQueryObject) + + return createdShippingOption +} + +medusaIntegrationTestRunner({ + env, + testSuite: ({ getContainer }) => { + let service: IFulfillmentModuleService + let container + + beforeAll(() => { + container = getContainer() + service = container.resolve(ModuleRegistrationName.FULFILLMENT) + }) + + describe("Fulfillment workflows", () => { + let fulfillmentSet: FulfillmentSetDTO + let serviceZone: ServiceZoneDTO + let shippingProfile: ShippingProfileDTO + + beforeEach(async () => { + shippingProfile = await service.createShippingProfiles({ + name: "test", + type: "default", + }) + + fulfillmentSet = await service.create({ + name: "Test fulfillment set", + type: "manual_test", + }) + + serviceZone = await service.createServiceZones({ + name: "Test service zone", + fulfillment_set_id: fulfillmentSet.id, + geo_zones: [ + { + type: "country", + country_code: "US", + }, + ], + }) + }) + + it("should create, update and delete rules in batch", async () => { + const shippingOption = await createShippingOptionFixture({ + container, + serviceZone, + shippingProfile, + }) + + expect(shippingOption.rules).toHaveLength(2) + + const ruleToUpdate = shippingOption.rules.find((rule) => { + return rule.attribute === "is_store" + }) + const ruleToDelete = shippingOption.rules.find((rule) => { + return rule.attribute === "total" + }) + + const workflowInput: BatchWorkflowInput< + CreateShippingOptionRuleDTO, + UpdateShippingOptionRuleDTO + > = { + create: [ + { + shipping_option_id: shippingOption.id, + attribute: "new_attribute", + operator: RuleOperator.EQ, + value: "100", + }, + ], + update: [ + { + ...ruleToUpdate, + value: "false", + }, + ], + delete: [ruleToDelete.id], + } + + await batchShippingOptionRulesWorkflow(container).run({ + input: workflowInput, + }) + + const remoteQuery = container.resolve( + ContainerRegistrationKeys.REMOTE_QUERY + ) + + const remoteQueryObject = remoteQueryObjectFromString({ + entryPoint: "shipping_option", + variables: { + id: shippingOption.id, + }, + fields: ["id", "rules.*"], + }) + + const [updatedShippingOption] = await remoteQuery(remoteQueryObject) + + const newAttrRule = updatedShippingOption.rules.find((rule) => { + return rule.attribute === "new_attribute" + }) + const updatedRule = updatedShippingOption.rules.find((rule) => { + return rule.attribute === "is_store" + }) + + expect(updatedShippingOption.rules).toHaveLength(2) + expect(newAttrRule).toEqual( + expect.objectContaining({ + attribute: "new_attribute", + operator: "eq", + value: 100, + }) + ) + expect(updatedRule).toEqual( + expect.objectContaining({ + value: "false", + }) + ) + }) + + it("should revert the shipping options rules batch actions", async () => { + const shippingOption = await createShippingOptionFixture({ + container, + serviceZone, + shippingProfile, + }) + + expect(shippingOption.rules).toHaveLength(2) + + const ruleToUpdate = shippingOption.rules.find((rule) => { + return rule.attribute === "is_store" + }) + const ruleToDelete = shippingOption.rules.find((rule) => { + return rule.attribute === "total" + }) + + const workflowInput: BatchWorkflowInput< + CreateShippingOptionRuleDTO, + UpdateShippingOptionRuleDTO + > = { + create: [ + { + shipping_option_id: shippingOption.id, + attribute: "new_attribute", + operator: RuleOperator.EQ, + value: "100", + }, + ], + update: [ + { + ...ruleToUpdate, + value: "false", + }, + ], + delete: [ruleToDelete.id], + } + + const workflow = batchShippingOptionRulesWorkflow(container) + + workflow.addAction( + "throw", + { + invoke: async function failStep() { + throw new Error(`Failed to update shipping option rules`) + }, + }, + { + noCompensation: true, + } + ) + + const { errors } = await workflow.run({ + input: workflowInput, + throwOnError: false, + }) + + expect(errors).toHaveLength(1) + expect(errors[0].error.message).toEqual( + `Failed to update shipping option rules` + ) + + const remoteQuery = container.resolve( + ContainerRegistrationKeys.REMOTE_QUERY + ) + + const remoteQueryObject = remoteQueryObjectFromString({ + entryPoint: "shipping_option", + variables: { + id: shippingOption.id, + }, + fields: ["id", "rules.*"], + }) + + const [updatedShippingOption] = await remoteQuery(remoteQueryObject) + + expect(updatedShippingOption.rules).toHaveLength(2) + expect(updatedShippingOption.rules).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + attribute: "is_store", + operator: "eq", + value: "true", + }), + expect.objectContaining({ + attribute: "total", + operator: "eq", + value: 100, + }), + ]) + ) + }) + }) + }, +}) diff --git a/packages/core-flows/src/fulfillment/steps/create-shipping-option-rules.ts b/packages/core-flows/src/fulfillment/steps/create-shipping-option-rules.ts index c012a97f16..3dacd02d5b 100644 --- a/packages/core-flows/src/fulfillment/steps/create-shipping-option-rules.ts +++ b/packages/core-flows/src/fulfillment/steps/create-shipping-option-rules.ts @@ -3,7 +3,7 @@ import { AddFulfillmentShippingOptionRulesWorkflowDTO, IFulfillmentModuleService, } from "@medusajs/types" -import { StepResponse, createStep } from "@medusajs/workflows-sdk" +import { createStep, StepResponse } from "@medusajs/workflows-sdk" export const createShippingOptionRulesStepId = "create-shipping-option-rules" export const createShippingOptionRulesStep = createStep( @@ -18,12 +18,12 @@ export const createShippingOptionRulesStep = createStep( ModuleRegistrationName.FULFILLMENT ) - const createdPromotionRules = + const createdShippingOptionRules = await fulfillmentModule.createShippingOptionRules(data) return new StepResponse( - createdPromotionRules, - createdPromotionRules.map((pr) => pr.id) + createdShippingOptionRules, + createdShippingOptionRules.map((pr) => pr.id) ) }, async (ruleIds, { container }) => { diff --git a/packages/core-flows/src/fulfillment/steps/delete-shipping-option-rules.ts b/packages/core-flows/src/fulfillment/steps/delete-shipping-option-rules.ts index 252bcbc273..1a1ce36ddd 100644 --- a/packages/core-flows/src/fulfillment/steps/delete-shipping-option-rules.ts +++ b/packages/core-flows/src/fulfillment/steps/delete-shipping-option-rules.ts @@ -4,7 +4,7 @@ import { RemoveFulfillmentShippingOptionRulesWorkflowDTO, RuleOperatorType, } from "@medusajs/types" -import { StepResponse, createStep } from "@medusajs/workflows-sdk" +import { createStep, StepResponse } from "@medusajs/workflows-sdk" export const deleteShippingOptionRulesStepId = "delete-shipping-option-rules" export const deleteShippingOptionRulesStep = createStep( @@ -13,6 +13,10 @@ export const deleteShippingOptionRulesStep = createStep( input: RemoveFulfillmentShippingOptionRulesWorkflowDTO, { container } ) => { + if (!input.ids?.length) { + return + } + const { ids } = input const fulfillmentModule = container.resolve( @@ -39,6 +43,7 @@ export const deleteShippingOptionRulesStep = createStep( await fulfillmentModule.createShippingOptionRules( shippingOptionRules.map((rule) => ({ + id: rule.id, attribute: rule.attribute, operator: rule.operator as RuleOperatorType, value: rule.value as unknown as string | string[], diff --git a/packages/core-flows/src/fulfillment/steps/update-shipping-option-rules.ts b/packages/core-flows/src/fulfillment/steps/update-shipping-option-rules.ts new file mode 100644 index 0000000000..c15e69da64 --- /dev/null +++ b/packages/core-flows/src/fulfillment/steps/update-shipping-option-rules.ts @@ -0,0 +1,51 @@ +import { ModuleRegistrationName } from "@medusajs/modules-sdk" +import { + IFulfillmentModuleService, + UpdateFulfillmentShippingOptionRulesWorkflowDTO, + UpdateShippingOptionRuleDTO, +} from "@medusajs/types" +import { createStep, StepResponse } from "@medusajs/workflows-sdk" + +export const updateShippingOptionRulesStepId = "update-shipping-option-rules" +export const updateShippingOptionRulesStep = createStep( + updateShippingOptionRulesStepId, + async ( + input: UpdateFulfillmentShippingOptionRulesWorkflowDTO, + { container } + ) => { + if (!input.data?.length) { + return + } + + const { data } = input + + const fulfillmentModule = container.resolve( + ModuleRegistrationName.FULFILLMENT + ) + + const ids = data.map((d) => d.id) + const shippingOptionRules = await fulfillmentModule.listShippingOptionRules( + { id: ids }, + { select: ["id", "attribute", "operator", "value", "shipping_option_id"] } + ) + + const updatedPromotionRules = + await fulfillmentModule.updateShippingOptionRules(data) + + return new StepResponse( + updatedPromotionRules, + shippingOptionRules as unknown as UpdateShippingOptionRuleDTO[] + ) + }, + async (previousRulesData, { container }) => { + if (!previousRulesData?.length) { + return + } + + const fulfillmentModule = container.resolve( + ModuleRegistrationName.FULFILLMENT + ) + + await fulfillmentModule.updateShippingOptionRules(previousRulesData) + } +) diff --git a/packages/core-flows/src/fulfillment/workflows/batch-shipping-option-rules.ts b/packages/core-flows/src/fulfillment/workflows/batch-shipping-option-rules.ts index 711419b81e..d1d231e806 100644 --- a/packages/core-flows/src/fulfillment/workflows/batch-shipping-option-rules.ts +++ b/packages/core-flows/src/fulfillment/workflows/batch-shipping-option-rules.ts @@ -6,15 +6,16 @@ import { UpdateShippingOptionRuleDTO, } from "@medusajs/types" import { - WorkflowData, createWorkflow, parallelize, transform, + WorkflowData, } from "@medusajs/workflows-sdk" import { createShippingOptionRulesStep, deleteShippingOptionRulesStep, } from "../steps" +import { updateShippingOptionRulesStep } from "../steps/update-shipping-option-rules" export const batchShippingOptionRulesWorkflowId = "batch-shipping-option-rules" export const batchShippingOptionRulesWorkflow = createWorkflow( @@ -27,25 +28,21 @@ export const batchShippingOptionRulesWorkflow = createWorkflow( > > ): WorkflowData> => { - const createInput = transform({ input }, (data) => ({ - data: data.input.create ?? [], - })) + const actionInputs = transform({ input }, (data) => { + const { create, update, delete: del } = data.input + return { + createInput: { data: create ?? [] }, + updateInput: { data: update ?? [] }, + deleteInput: { ids: del ?? [] }, + } + }) - const updateInput = transform({ input }, (data) => ({ - data: data.input.update ?? [], - })) - - const deleteInput = transform({ input }, (data) => ({ - ids: data.input.delete ?? [], - })) - - // TODO: Currently we don't support edits, add support for this. - // We just call the steps directly here since there are no independent workflows, switch to CRUD workflows if they get added. - const [created, deleted] = parallelize( - createShippingOptionRulesStep(createInput), - deleteShippingOptionRulesStep(deleteInput) + const [created, updated, deleted] = parallelize( + createShippingOptionRulesStep(actionInputs.createInput), + updateShippingOptionRulesStep(actionInputs.updateInput), + deleteShippingOptionRulesStep(actionInputs.deleteInput) ) - return transform({ created, deleted }, (data) => ({ ...data, updated: [] })) + return transform({ created, deleted, updated }, (data) => data) } ) diff --git a/packages/fulfillment/src/services/fulfillment-module-service.ts b/packages/fulfillment/src/services/fulfillment-module-service.ts index 086a61d4a5..192f2d93aa 100644 --- a/packages/fulfillment/src/services/fulfillment-module-service.ts +++ b/packages/fulfillment/src/services/fulfillment-module-service.ts @@ -14,17 +14,17 @@ import { UpdateServiceZoneDTO, } from "@medusajs/types" import { + arrayDifference, EmitEvents, FulfillmentUtils, + getSetDifference, InjectManager, InjectTransactionManager, + isString, MedusaContext, MedusaError, Modules, ModulesSdkUtils, - arrayDifference, - getSetDifference, - isString, promiseAll, } from "@medusajs/utils" import { @@ -38,9 +38,9 @@ import { ShippingOptionType, ShippingProfile, } from "@models" -import { isContextValid, validateRules } from "@utils" -import { entityNameToLinkableKeysMap, joinerConfig } from "../joiner-config" -import { UpdateShippingOptionsInput } from "../types/service" +import {isContextValid, validateAndNormalizeRules} from "@utils" +import {entityNameToLinkableKeysMap, joinerConfig} from "../joiner-config" +import {UpdateShippingOptionsInput} from "../types/service" import FulfillmentProviderService from "./fulfillment-provider" const generateMethodForModels = [ @@ -343,7 +343,7 @@ export default class FulfillmentModuleService< | FulfillmentTypes.CreateServiceZoneDTO, @MedusaContext() sharedContext: Context = {} ): Promise { - let data_ = Array.isArray(data) ? data : [data] + const data_ = Array.isArray(data) ? data : [data] if (!data_.length) { return [] @@ -402,7 +402,7 @@ export default class FulfillmentModuleService< | FulfillmentTypes.CreateShippingOptionDTO, @MedusaContext() sharedContext: Context = {} ): Promise { - let data_ = Array.isArray(data) ? data : [data] + const data_ = Array.isArray(data) ? data : [data] if (!data_.length) { return [] @@ -410,7 +410,7 @@ export default class FulfillmentModuleService< const rules = data_.flatMap((d) => d.rules).filter(Boolean) if (rules.length) { - validateRules(rules as Record[]) + validateAndNormalizeRules(rules as Record[]) } const createdShippingOptions = await this.shippingOptionService_.create( @@ -555,7 +555,7 @@ export default class FulfillmentModuleService< return [] } - validateRules(data_ as unknown as Record[]) + validateAndNormalizeRules(data_ as unknown as Record[]) const createdShippingOptionRules = await this.shippingOptionRuleService_.create(data_, sharedContext) @@ -1175,7 +1175,7 @@ export default class FulfillmentModuleService< }) .filter(Boolean) - validateRules(newRules as Record[]) + validateAndNormalizeRules(newRules as Record[]) shippingOption.rules = shippingOption.rules.map((rule) => { if (!("id" in rule)) { @@ -1382,7 +1382,7 @@ export default class FulfillmentModuleService< return [] } - validateRules(data_ as unknown as Record[]) + validateAndNormalizeRules(data_ as unknown as Record[]) const updatedShippingOptionRules = await this.shippingOptionRuleService_.update(data_, sharedContext) diff --git a/packages/fulfillment/src/utils/utils.ts b/packages/fulfillment/src/utils/utils.ts index ad0fb1bbc8..76147bd397 100644 --- a/packages/fulfillment/src/utils/utils.ts +++ b/packages/fulfillment/src/utils/utils.ts @@ -1,4 +1,5 @@ import { + isObject, isString, MedusaError, pickValueFromObject, @@ -97,21 +98,21 @@ export function validateRule(rule: Record): boolean { if (!rule.attribute || !rule.operator || !rule.value) { throw new MedusaError( MedusaError.Types.INVALID_DATA, - "Rule must have an attribute, an operator and contextValue value" + "Rule must have an attribute, an operator and a value" ) } if (!isString(rule.attribute)) { throw new MedusaError( MedusaError.Types.INVALID_DATA, - "Rule attribute must be contextValue string" + "Rule attribute must be a string" ) } if (!isString(rule.operator)) { throw new MedusaError( MedusaError.Types.INVALID_DATA, - "Rule operator must be contextValue string" + "Rule operator must be a string" ) } @@ -132,10 +133,10 @@ export function validateRule(rule: Record): boolean { ) } } else { - if (!isString(rule.value)) { + if (Array.isArray(rule.value) || isObject(rule.value)) { throw new MedusaError( MedusaError.Types.INVALID_DATA, - `Rule value must be a string for the selected operator ${rule.operator}` + `Rule value must be a string, bool, number value for the selected operator ${rule.operator}` ) } } @@ -143,6 +144,24 @@ export function validateRule(rule: Record): boolean { return true } +export function normalizeRulesValue>(rules: T[]): void { + rules.forEach((rule) => { + /** + * If a string is provided, then we don't want jsonb to convert to the primitive value based on the RFC + */ + if (rule.value === "true" || rule.value === "false") { + rule.value = rule.value === "true" ? '"true"' : '"false"' + } + + return rule + }) +} + +export function validateAndNormalizeRules>(rules: T[]) { + rules.forEach(validateRule) + normalizeRulesValue(rules) +} + /** * Validate contextValue set of rules * @param rules diff --git a/packages/medusa/src/api-v2/admin/shipping-options/validators.ts b/packages/medusa/src/api-v2/admin/shipping-options/validators.ts index 8fc8919d73..a9c0e2b41c 100644 --- a/packages/medusa/src/api-v2/admin/shipping-options/validators.ts +++ b/packages/medusa/src/api-v2/admin/shipping-options/validators.ts @@ -126,5 +126,6 @@ export const AdminUpdateShippingOption = z ) .array() .optional(), + rules: AdminUpdateShippingOptionRule.or(AdminCreateShippingOptionRule).array().optional(), }) .strict() diff --git a/packages/types/src/fulfillment/workflows.ts b/packages/types/src/fulfillment/workflows.ts index 7116a42377..33e3bbb072 100644 --- a/packages/types/src/fulfillment/workflows.ts +++ b/packages/types/src/fulfillment/workflows.ts @@ -1,4 +1,7 @@ -import { CreateShippingOptionRuleDTO } from "./mutations" +import { + CreateShippingOptionRuleDTO, + UpdateShippingOptionRuleDTO, +} from "./mutations" export type AddFulfillmentShippingOptionRulesWorkflowDTO = { data: CreateShippingOptionRuleDTO[] @@ -7,3 +10,7 @@ export type AddFulfillmentShippingOptionRulesWorkflowDTO = { export type RemoveFulfillmentShippingOptionRulesWorkflowDTO = { ids: string[] } + +export type UpdateFulfillmentShippingOptionRulesWorkflowDTO = { + data: UpdateShippingOptionRuleDTO[] +}