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

@@ -0,0 +1,5 @@
+---
+"@medusajs/fulfillment": patch
+---
+
+fix(fulfillment): Update shipping options rules

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",

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,
+                    attribute: updateData.rules[1].attribute,
-                    operator: updateData.rules[0].operator,
+                    operator: updateData.rules[1].operator,
-                    value: updateData.rules[0].value,
+                    value: updateData.rules[1].value,
                   }),
                 ]),
               })
             )
 
             const rules = await service.listShippingOptionRules()
-            expect(rules).toHaveLength(1)
+            expect(rules).toHaveLength(2)
-            expect(rules[0]).toEqual(
+            expect(rules).toEqual(
-              expect.objectContaining({
+              expect.arrayContaining([
-                id: updatedShippingOption.rules[0].id,
+                expect.objectContaining({
-              })
+                  id: updatedShippingOption.rules[0].id,
+                }),
+                expect.objectContaining({
+                  id: updatedShippingOption.rules[1].id,
+                }),
+              ])
             )
 
             const types = await service.listShippingOptionTypes()

packages/fulfillment/src/services/fulfillment-module-service.ts

@@ -38,9 +38,9 @@ import {
   ShippingOptionType,
   ShippingProfile,
 } from "@models"
-import {isContextValid, validateAndNormalizeRules} from "@utils"
+import { isContextValid, validateAndNormalizeRules } from "@utils"
-import {entityNameToLinkableKeysMap, joinerConfig} from "../joiner-config"
+import { entityNameToLinkableKeysMap, joinerConfig } from "../joiner-config"
-import {UpdateShippingOptionsInput} from "../types/service"
+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(
+      const existingRulesMap: Map<
-        existingRules.map((rule) => [rule.id, rule])
+        string,
-      )
+        FulfillmentTypes.UpdateShippingOptionRuleDTO | ShippingOptionRule
+      > = new Map(existingRules.map((rule) => [rule.id, rule]))
 
       const updatedRules = shippingOption.rules
-      const updatedRuleIds = updatedRules
+        .map((rule) => {
-        .map((r) => "id" in r && r.id)
+          if ("id" in rule) {
-        .filter((id): id is string => !!id)
+            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<string, unknown>[])
-
       shippingOption.rules = shippingOption.rules.map((rule) => {
         if (!("id" in rule)) {
+          validateAndNormalizeRules([rule])
           return rule
         }
         return existingRulesMap.get(rule.id)!