reactioncommerce · sujithvn · May 25, 2023 · Nov 1, 2022 · Oct 17, 2022 · Oct 18, 2022
diff --git a/...s/api-plugin-fulfillment/src/mutations/__snapshots__/createFulfillmentMethod.test.js.snap b/...s/api-plugin-fulfillment/src/mutations/__snapshots__/createFulfillmentMethod.test.js.snap
diff --git a/...ges/api-plugin-fulfillment/src/mutations/__snapshots__/createFulfillmentType.test.js.snap b/...ges/api-plugin-fulfillment/src/mutations/__snapshots__/createFulfillmentType.test.js.snap
diff --git a/...ugin-fulfillment/src/mutations/__snapshots__/selectFulfillmentOptionForGroup.test.js.snap b/...ugin-fulfillment/src/mutations/__snapshots__/selectFulfillmentOptionForGroup.test.js.snap
diff --git a/...s/api-plugin-fulfillment/src/mutations/__snapshots__/updateFulfillmentMethod.test.js.snap b/...s/api-plugin-fulfillment/src/mutations/__snapshots__/updateFulfillmentMethod.test.js.snap
diff --git a/...gin-fulfillment/src/mutations/__snapshots__/updateFulfillmentOptionsForGroup.test.js.snap b/...gin-fulfillment/src/mutations/__snapshots__/updateFulfillmentOptionsForGroup.test.js.snap
diff --git a/...ges/api-plugin-fulfillment/src/mutations/__snapshots__/updateFulfillmentType.test.js.snap b/...ges/api-plugin-fulfillment/src/mutations/__snapshots__/updateFulfillmentType.test.js.snap
diff --git a/packages/api-plugin-fulfillment/src/mutations/createFulfillmentMethod.js b/packages/api-plugin-fulfillment/src/mutations/createFulfillmentMethod.js
@@ -14,15 +14,15 @@ const inputSchema = new SimpleSchema({
  * @summary Creates a fulfillment method
  * @param {Object} context - an object containing the per-request state
  * @param {Object} input - Input
- * @param {Number} cost - optional cost
- * @param {String[]} fulfillmentTypes - fulfillment type retained for backward compatibility
- * @param {String} group - Group to which fulfillment method belong
- * @param {Number} handling - handling charges
- * @param {Boolean} enabled - status of fulfillment method
- * @param {String} label - displayed on the UI
- * @param {String} fulfillmentMethod - used by application, not user editable
- * @param {String} displayMessageMethod - used to display additional message on UI specific to ff-method
- * @param {Number} rate - ratefor themethod
+ * @param {Number} input.cost - optional cost
+ * @param {String[]} input.fulfillmentTypes - fulfillment type retained for backward compatibility
+ * @param {String} input.group - Group to which fulfillment method belong
+ * @param {Number} input.handling - handling charges
+ * @param {Boolean} input.enabled - status of fulfillment method
+ * @param {String} input.label - displayed on the UI
+ * @param {String} input.fulfillmentMethod - used by application, not user editable
+ * @param {String} input.displayMessageMethod - used to display additional message on UI specific to ff-method
+ * @param {Number} input.rate - rate for the method
  * @returns {Promise<Object>} An object with a `method` property containing the created method
  */
 export default async function createFulfillmentMethodMutation(context, input) {
@@ -35,18 +35,14 @@ export default async function createFulfillmentMethodMutation(context, input) {
 
   await context.validatePermissions("reaction:legacy:fulfillmentMethods", "create", { shopId });
 
-  const ffTypeRecord = await Fulfillment.findOne({ _id: fulfillmentTypeId, shopId });
-  if (!ffTypeRecord) throw new ReactionError("server-error", "Unable to create fulfillment method without defined type");
+  const fulfillmentType = await Fulfillment.findOne({ _id: fulfillmentTypeId, shopId });
+  if (!fulfillmentType) throw new ReactionError("server-error", "Unable to create fulfillment method without defined type");
 
-  const ffTypeMethodRecord = await Fulfillment.findOne({
-    _id: fulfillmentTypeId,
-    shopId,
-    methods: { $elemMatch: { fulfillmentMethod: method.fulfillmentMethod } }
-  });
-  if (ffTypeMethodRecord) throw new ReactionError("server-error", "Fulfillment Method already exists");
+  const fulfillmentMethod = fulfillmentType.methods?.find((currMethod) => currMethod.fulfillmentMethod === method.fulfillmentMethod);
+  if (fulfillmentMethod) throw new ReactionError("server-error", "Fulfillment Method already exists");
 
   method._id = Random.id();
-  method.fulfillmentTypes = [ffTypeRecord.fulfillmentType];
+  method.fulfillmentTypes = [fulfillmentType.fulfillmentType];
 
   const { matchedCount } = await Fulfillment.updateOne({
     shopId,
@@ -59,5 +55,5 @@ export default async function createFulfillmentMethodMutation(context, input) {
 
   if (matchedCount === 0) throw new ReactionError("server-error", "Unable to create fulfillment method");
 
-  return { group: method };
+  return { method };
 }
diff --git a/packages/api-plugin-fulfillment/src/mutations/createFulfillmentMethod.test.js b/packages/api-plugin-fulfillment/src/mutations/createFulfillmentMethod.test.js
@@ -18,7 +18,8 @@ test("throws if required fields are not supplied", async () => {
     shopId: "SHOP_ID"
   };
 
-  await expect(createFulfillmentMethodMutation(mockContext, fulfillmentMethodInput)).rejects.toThrowErrorMatchingSnapshot();
+  const expectedError = "Fulfillment types is required";
+  await expect(createFulfillmentMethodMutation(mockContext, fulfillmentMethodInput)).rejects.toThrow(expectedError);
 });
 
 test("add a new fulfillment method", async () => {
@@ -55,7 +56,7 @@ test("add a new fulfillment method", async () => {
   });
 
   expect(result).toEqual({
-    group: {
+    method: {
       _id: expect.any(String),
       cost: 99,
       handling: 99,

diff --git a/packages/api-plugin-fulfillment/src/mutations/createFulfillmentType.js b/packages/api-plugin-fulfillment/src/mutations/createFulfillmentType.js
@@ -12,25 +12,28 @@ import { FulfillmentTypeSchema } from "../simpleSchemas.js";
  * @param {ProviderSchema} input.provider - Provider details
  * @param {String} input.fulfillmentType - fulfillment type
  * @param {String} input.displayMessageType - displayMessage for fulfillment type
- * @param {FulfillmentMethodSchema[]} input.methods - ff-method array
  * @returns {Promise<Object>} An object with the updated type
  */
 export default async function createFulfillmentType(context, input) {
   const cleanedInput = FulfillmentTypeSchema.clean(input);
-  if (!cleanedInput.provider) cleanedInput.provider = {};
-  cleanedInput.provider.name = cleanedInput.name;
+  if (cleanedInput.provider) cleanedInput.provider.name = cleanedInput.name;
 
-  if (cleanedInput.method) {
-    cleanedInput.method._id = Random.id();
-    cleanedInput.method.fulfillmentTypes = [cleanedInput.fulfillmentType];
+  // Although allowed by schema, we do not add the ff-method while creating a new ff-type
+  // FulfillmentMethods are expected to be added using the mutation createFulfillmentMethod
+  // (as it makes sense to add a new ff-method only by a new plugin implementation).
+  if (cleanedInput.methods) {
+    delete cleanedInput.methods;
   }
+  const createdAt = new Date();
+  cleanedInput.createdAt = createdAt;
+  cleanedInput.updatedAt = createdAt;
   FulfillmentTypeSchema.validate(cleanedInput);
 
   const { collections: { Fulfillment } } = context;
   const { shopId, fulfillmentType } = cleanedInput;
 
-  const ffTypeRecord = await Fulfillment.findOne({ shopId, fulfillmentType });
-  if (ffTypeRecord) throw new ReactionError("invalid-parameter", "Fulfillment Type already exists");
+  const existingFulfillmentType = await Fulfillment.findOne({ shopId, fulfillmentType });
+  if (existingFulfillmentType) throw new ReactionError("invalid-parameter", "Fulfillment Type already exists");
 
   await context.validatePermissions("reaction:legacy:fulfillmentTypes", "create", { shopId });
 
@@ -40,5 +43,5 @@ export default async function createFulfillmentType(context, input) {
   });
   if (insertedCount === 0) throw new ReactionError("server-error", "Unable to create fulfillment type");
 
-  return { group: { name: cleanedInput.name, fulfillmentType: cleanedInput.fulfillmentType } };
+  return { fulfillmentType: { name: cleanedInput.name, fulfillmentType: cleanedInput.fulfillmentType } };
 }
diff --git a/packages/api-plugin-fulfillment/src/mutations/createFulfillmentType.test.js b/packages/api-plugin-fulfillment/src/mutations/createFulfillmentType.test.js
@@ -9,11 +9,12 @@ mockContext.validatePermissions.mockReturnValueOnce(Promise.resolve(null));
 
 test("throws if required fields are not supplied", async () => {
   const fulfillmentTypeInput = {
-    shopId: "SHOP_ID",
-    name: "fulfillmentType123"
+    shopId: "SHOP_ID"
+    // name: "fulfillmentType123"
   };
 
-  await expect(createFulfillmentTypeMutation(mockContext, fulfillmentTypeInput)).rejects.toThrowErrorMatchingSnapshot();
+  const expectedError = "Name is required";
+  await expect(createFulfillmentTypeMutation(mockContext, fulfillmentTypeInput)).rejects.toThrow(expectedError);
 });
 
 test("throws if the fulfillmentType added already exists", async () => {
@@ -61,7 +62,7 @@ test("add a new fulfillment type", async () => {
   const result = await createFulfillmentTypeMutation(mockContext, fulfillmentTypeInput);
 
   expect(result).toEqual({
-    group: {
+    fulfillmentType: {
       name: "fulfillmentType123",
       fulfillmentType: "shipping"
     }

diff --git a/packages/api-plugin-fulfillment/src/mutations/selectFulfillmentOptionForGroup.test.js b/packages/api-plugin-fulfillment/src/mutations/selectFulfillmentOptionForGroup.test.js
@@ -53,17 +53,19 @@ test("selects an existing shipping method", async () => {
 });
 
 test("throws if there is no fulfillment group with the given ID", async () => {
+  const expectedError = "Fulfillment group with ID group2 not found in cart with ID cartId";
   await expect(selectFulfillmentOptionForGroup(mockContext, {
     cartId: "cartId",
     fulfillmentGroupId: "group2",
     fulfillmentMethodId: "valid-method"
-  })).rejects.toThrowErrorMatchingSnapshot();
+  })).rejects.toThrow(expectedError);
 });
 
 test("throws if there is no fulfillment method with the given ID among the options", async () => {
+  const expectedError = "Fulfillment option with method ID invalid-method not found in cart with ID cartId";
   await expect(selectFulfillmentOptionForGroup(mockContext, {
     cartId: "cartId",
     fulfillmentGroupId: "group1",
     fulfillmentMethodId: "invalid-method"
-  })).rejects.toThrowErrorMatchingSnapshot();
+  })).rejects.toThrow(expectedError);
 });
diff --git a/packages/api-plugin-fulfillment/src/mutations/updateFulfillmentMethod.js b/packages/api-plugin-fulfillment/src/mutations/updateFulfillmentMethod.js
@@ -1,9 +1,10 @@
 import SimpleSchema from "simpl-schema";
 import ReactionError from "@reactioncommerce/reaction-error";
-import { FulfillmentMethodSchema } from "../simpleSchemas.js";
+import { UserEditableFulfillmentMethodSchema } from "../simpleSchemas.js";
+
 
 const inputSchema = new SimpleSchema({
-  method: FulfillmentMethodSchema,
+  method: UserEditableFulfillmentMethodSchema,
   fulfillmentTypeId: String,
   methodId: String,
   shopId: String
@@ -14,7 +15,7 @@ const inputSchema = new SimpleSchema({
  * @summary updates Fulfillment method
  * @param {Object} context - an object containing the per-request state
  * @param {Object} input - Input object
- * @param {FulfillmentMethodSchema} input.method - fulfillment method object
+ * @param {UserEditableFulfillmentMethodSchema} input.method - fulfillment method object with only user editable fields
  * @param {String} input.fulfillmentTypeId - id of fulfillment type
  * @param {String} input.methodId - ff-method Id
  * @param {String} input.shopId - Shop Id
@@ -28,30 +29,18 @@ export default async function updateFulfillmentMethodMutation(context, input) {
   const { collections: { Fulfillment } } = context;
   const method = { ...inputMethod };
 
-  if (!fulfillmentTypeId) throw new ReactionError("invalid-parameter", "Fulfillment Type ID to be updated not provided");
-  if (!methodId) throw new ReactionError("invalid-parameter", "Method ID to be updated not provided");
-
-  await context.validatePermissions(`reaction:legacy:fulfillmentMethods:${methodId}`, "update", { shopId });
+  await context.validatePermissions("reaction:legacy:fulfillmentMethods", "update", { shopId });
 
-  const ffTypeMethodRecord = await Fulfillment.findOne({
+  const fulfillmentType = await Fulfillment.findOne({
     "_id": fulfillmentTypeId,
     shopId,
     "methods._id": methodId
   });
-  if (!ffTypeMethodRecord) throw new ReactionError("server-error", "Fulfillment Method does not exist");
+  if (!fulfillmentType) throw new ReactionError("server-error", "Fulfillment Type / Method does not exist");
 
-  // Do not update the fulfillmentType, fulfillmentMethod, group, name & _id fields
-  // Find the matching fulfillmentMethod object and use those values to over-write
-  const currentFulfillmentMethod = (ffTypeMethodRecord.methods || []).find((meth) => meth._id === methodId);
+  const currentFulfillmentMethod = (fulfillmentType.methods || []).find((meth) => meth._id === methodId);
   if (!currentFulfillmentMethod) throw new ReactionError("server-error", "Fulfillment Method does not exist");
-  const updatedMethod = {
-    ...method,
-    _id: methodId,
-    name: currentFulfillmentMethod.name,
-    group: currentFulfillmentMethod.group,
-    fulfillmentMethod: currentFulfillmentMethod.fulfillmentMethod,
-    fulfillmentType: [ffTypeMethodRecord.fulfillmentType]
-  };
+  const updatedMethod = { ...currentFulfillmentMethod, ...method }; // update only provided user editable fields
 
   const { matchedCount } = await Fulfillment.updateOne({
     "_id": fulfillmentTypeId,
@@ -64,5 +53,5 @@ export default async function updateFulfillmentMethodMutation(context, input) {
   });
   if (matchedCount === 0) throw new ReactionError("not-found", "Fulfillment type to be updated not found");
 
-  return { group: updatedMethod };
+  return { method: updatedMethod };
 }
diff --git a/packages/api-plugin-fulfillment/src/mutations/updateFulfillmentMethod.test.js b/packages/api-plugin-fulfillment/src/mutations/updateFulfillmentMethod.test.js
@@ -22,7 +22,8 @@ test("throws if required fields are not supplied", async () => {
     }
   };
 
-  await expect(updateFulfillmentMethodMutation(mockContext, fulfillmentMethodInput)).rejects.toThrowErrorMatchingSnapshot();
+  const expectedError = "Method ID is required";
+  await expect(updateFulfillmentMethodMutation(mockContext, fulfillmentMethodInput)).rejects.toThrow(expectedError);
 });
 
 test("throws if the fulfillmentType does not exists", async () => {
@@ -42,7 +43,7 @@ test("throws if the fulfillmentType does not exists", async () => {
       rate: 99
     }
   };
-  const expectedError = new ReactionError("server-error", "Fulfillment Method does not exist");
+  const expectedError = new ReactionError("server-error", "Fulfillment Type / Method does not exist");
   await expect(updateFulfillmentMethodMutation(mockContext, fulfillmentMethodInput)).rejects.toThrow(expectedError);
 });
 
@@ -116,11 +117,10 @@ test("should update an existing fulfillment method", async () => {
   };
 
   const expectedOutput = {
-    group: {
+    method: {
       _id: "fulfillmentMethodId01",
       enabled: true,
       fulfillmentMethod: "ups",
-      fulfillmentType: ["Shipping"],
       fulfillmentTypes: ["Shipping"],
       group: "Ground",
       handling: 99,

diff --git a/packages/api-plugin-fulfillment/src/mutations/updateFulfillmentOptionsForGroup.js b/packages/api-plugin-fulfillment/src/mutations/updateFulfillmentOptionsForGroup.js
@@ -77,7 +77,7 @@ export default async function updateFulfillmentOptionsForGroup(context, input) {
 
   const commonOrder = await context.queries.getCommonOrderForCartGroup(context, { cart, fulfillmentGroupId: fulfillmentGroup._id });
   // In the future we want to do this async and subscribe to the results
-  const rates = await context.queries.getFulfillmentMethodsWithQuotes(commonOrder, context);
+  const rates = await context.queries.fulfillmentMethodsWithQuotes(commonOrder, context);
 
   const { shipmentQuotes, shipmentQuotesQueryStatus } = getShipmentQuotesQueryStatus(rates);
 

diff --git a/packages/api-plugin-fulfillment/src/mutations/updateFulfillmentOptionsForGroup.test.js b/packages/api-plugin-fulfillment/src/mutations/updateFulfillmentOptionsForGroup.test.js
@@ -5,12 +5,12 @@ import updateFulfillmentOptionsForGroup from "./updateFulfillmentOptionsForGroup
 
 const fakeCart = Factory.Cart.makeOne();
 const fakeQuote = Factory.ShipmentQuote.makeOne();
-const mockGetFulfillmentMethodsWithQuotes = jest.fn().mockName("getFulfillmentMethodsWithQuotes");
+const mockFulfillmentMethodsWithQuotes = jest.fn().mockName("fulfillmentMethodsWithQuotes");
 const mockGetCommonOrderForCartGroup = jest.fn().mockName("getCommonOrderForCartGroup");
 
 beforeAll(() => {
   mockContext.queries = {
-    getFulfillmentMethodsWithQuotes: mockGetFulfillmentMethodsWithQuotes,
+    fulfillmentMethodsWithQuotes: mockFulfillmentMethodsWithQuotes,
     getCommonOrderForCartGroup: mockGetCommonOrderForCartGroup
   };
   if (!mockContext.mutations.saveCart) {
@@ -22,7 +22,7 @@ beforeAll(() => {
 });
 
 beforeEach(() => {
-  mockGetFulfillmentMethodsWithQuotes.mockClear();
+  mockFulfillmentMethodsWithQuotes.mockClear();
 
   mockContext.queries.getCartById = jest.fn().mockName("getCartById").mockReturnValueOnce(Promise.resolve({
     _id: "cartId",
@@ -47,7 +47,7 @@ beforeEach(() => {
 });
 
 test("updates cart properly for empty rates", async () => {
-  mockGetFulfillmentMethodsWithQuotes.mockReturnValueOnce(Promise.resolve([]));
+  mockFulfillmentMethodsWithQuotes.mockReturnValueOnce(Promise.resolve([]));
   mockContext.collections.Cart.findOne.mockReturnValueOnce(Promise.resolve(fakeCart));
 
   const result = await updateFulfillmentOptionsForGroup(mockContext, {
@@ -83,7 +83,7 @@ test("updates cart properly for empty rates", async () => {
 });
 
 test("updates cart properly for error rates", async () => {
-  mockGetFulfillmentMethodsWithQuotes.mockReturnValueOnce(Promise.resolve([{
+  mockFulfillmentMethodsWithQuotes.mockReturnValueOnce(Promise.resolve([{
     requestStatus: "error",
     shippingProvider: "all",
     message: "All requests for shipping methods failed."
@@ -126,7 +126,7 @@ test("updates cart properly for error rates", async () => {
 });
 
 test("updates cart properly for success rates", async () => {
-  mockGetFulfillmentMethodsWithQuotes.mockReturnValueOnce(Promise.resolve([fakeQuote]));
+  mockFulfillmentMethodsWithQuotes.mockReturnValueOnce(Promise.resolve([fakeQuote]));
   mockContext.collections.Cart.findOne.mockReturnValueOnce(Promise.resolve(fakeCart));
 
   const result = await updateFulfillmentOptionsForGroup(mockContext, {
@@ -165,8 +165,9 @@ test("updates cart properly for success rates", async () => {
 });
 
 test("throws if there is no fulfillment group with the given ID", async () => {
+  const expectedError = "Fulfillment group with ID group2 not found in cart with ID cartId";
   await expect(updateFulfillmentOptionsForGroup(mockContext, {
     cartId: "cartId",
     fulfillmentGroupId: "group2"
-  })).rejects.toThrowErrorMatchingSnapshot();
+  })).rejects.toThrow(expectedError);
 });