From 25b0ccc60a979f1a0145277d06690a04850b2da7 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Thu, 2 May 2024 14:30:13 +0200 Subject: [PATCH] fix(fulfillment): Update shipping options rules (#7204) **What** Fix the update shipping options to apply the rules update properly and remove unused code --- .changeset/shiny-apples-serve.md | 5 ++ .../admin/shipping-options.spec.ts | 45 +++++++++++++++-- .../shipping-option.spec.ts | 31 +++++++++--- .../services/fulfillment-module-service.ts | 49 +++++++++++-------- 4 files changed, 99 insertions(+), 31 deletions(-) create mode 100644 .changeset/shiny-apples-serve.md diff --git a/.changeset/shiny-apples-serve.md b/.changeset/shiny-apples-serve.md new file mode 100644 index 0000000000..407fb64639 --- /dev/null +++ b/.changeset/shiny-apples-serve.md @@ -0,0 +1,5 @@ +--- +"@medusajs/fulfillment": patch +--- + +fix(fulfillment): Update shipping options rules 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 b65f111d25..0885377be1 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 @@ -226,7 +226,18 @@ medusaIntegrationTestRunner({ amount: 1000, }, ], - rules: [shippingOptionRule], + rules: [ + { + operator: RuleOperator.EQ, + attribute: "old_attr", + value: "old value", + }, + { + operator: RuleOperator.EQ, + attribute: "old_attr_2", + value: "true", + }, + ], } const response = await api.post( @@ -240,6 +251,14 @@ medusaIntegrationTestRunner({ const eurPrice = response.data.shipping_option.prices.find( (p) => p.currency_code === "eur" ) + + const oldAttrRule = response.data.shipping_option.rules.find( + (r) => r.attribute === "old_attr" + ) + const oldAttr2Rule = response.data.shipping_option.rules.find( + (r) => r.attribute === "old_attr_2" + ) + const updateShippingOptionPayload = { name: "Updated shipping option", provider_id: "manual_test-provider", @@ -255,8 +274,22 @@ medusaIntegrationTestRunner({ }, ], rules: [ - shippingOptionRule, { + // Un touched + id: oldAttrRule.id, + operator: RuleOperator.EQ, + attribute: "old_attr", + value: "old value", + }, + { + // Updated + id: oldAttr2Rule.id, + operator: RuleOperator.EQ, + attribute: "old_attr_2", + value: "false", + }, + { + // Created operator: RuleOperator.EQ, attribute: "new_attr", value: "true", @@ -272,7 +305,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.rules).toHaveLength(3) expect(updateResponse.data.shipping_option).toEqual( expect.objectContaining({ id: expect.any(String), @@ -310,6 +343,12 @@ medusaIntegrationTestRunner({ attribute: "old_attr", value: "old value", }), + expect.objectContaining({ + id: expect.any(String), + operator: "eq", + attribute: "old_attr_2", + value: "false", + }), expect.objectContaining({ id: expect.any(String), operator: "eq", diff --git a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts index 723ce665c5..fd8275d7d8 100644 --- a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts +++ b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts @@ -593,6 +593,8 @@ moduleIntegrationTestRunner({ shippingOptionData ) + const existingRule = shippingOption.rules[0]! + const updateData: UpdateShippingOptionDTO = { id: shippingOption.id, name: "updated-test", @@ -609,6 +611,10 @@ moduleIntegrationTestRunner({ amount: 2000, }, rules: [ + { + ...existingRule, + value: "false", + }, { attribute: "new-test", operator: "eq", @@ -639,22 +645,31 @@ moduleIntegrationTestRunner({ }), data: updateData.data, rules: expect.arrayContaining([ + expect.objectContaining({ + id: existingRule.id, + value: '"false"', + }), expect.objectContaining({ id: expect.any(String), - attribute: updateData.rules[0].attribute, - operator: updateData.rules[0].operator, - value: updateData.rules[0].value, + attribute: updateData.rules[1].attribute, + operator: updateData.rules[1].operator, + value: updateData.rules[1].value, }), ]), }) ) const rules = await service.listShippingOptionRules() - expect(rules).toHaveLength(1) - expect(rules[0]).toEqual( - expect.objectContaining({ - id: updatedShippingOption.rules[0].id, - }) + expect(rules).toHaveLength(2) + expect(rules).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: updatedShippingOption.rules[0].id, + }), + expect.objectContaining({ + id: updatedShippingOption.rules[1].id, + }), + ]) ) const types = await service.listShippingOptionTypes() diff --git a/packages/fulfillment/src/services/fulfillment-module-service.ts b/packages/fulfillment/src/services/fulfillment-module-service.ts index 192f2d93aa..04d3dc5659 100644 --- a/packages/fulfillment/src/services/fulfillment-module-service.ts +++ b/packages/fulfillment/src/services/fulfillment-module-service.ts @@ -38,9 +38,9 @@ import { ShippingOptionType, ShippingProfile, } from "@models" -import {isContextValid, validateAndNormalizeRules} 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 = [ @@ -1148,14 +1148,33 @@ export default class FulfillmentModuleService< shippingOption ) - const existingRulesMap = new Map( - existingRules.map((rule) => [rule.id, rule]) - ) + const existingRulesMap: Map< + string, + FulfillmentTypes.UpdateShippingOptionRuleDTO | ShippingOptionRule + > = new Map(existingRules.map((rule) => [rule.id, rule])) const updatedRules = shippingOption.rules - const updatedRuleIds = updatedRules - .map((r) => "id" in r && r.id) - .filter((id): id is string => !!id) + .map((rule) => { + if ("id" in rule) { + const existingRule = (existingRulesMap.get(rule.id) ?? + {}) as FulfillmentTypes.UpdateShippingOptionRuleDTO + + const ruleData: FulfillmentTypes.UpdateShippingOptionRuleDTO = { + ...existingRule, + ...rule, + } + + existingRulesMap.set(rule.id, ruleData) + return ruleData + } + + return + }) + .filter(Boolean) as FulfillmentTypes.UpdateShippingOptionRuleDTO[] + + validateAndNormalizeRules(updatedRules) + + const updatedRuleIds = updatedRules.map((r) => "id" in r && r.id) const toDeleteRuleIds = arrayDifference( updatedRuleIds, @@ -1166,19 +1185,9 @@ export default class FulfillmentModuleService< ruleIdsToDelete.push(...toDeleteRuleIds) } - const newRules = updatedRules - .map((rule) => { - if (!("id" in rule)) { - return rule - } - return - }) - .filter(Boolean) - - validateAndNormalizeRules(newRules as Record[]) - shippingOption.rules = shippingOption.rules.map((rule) => { if (!("id" in rule)) { + validateAndNormalizeRules([rule]) return rule } return existingRulesMap.get(rule.id)!