feat(core): Simplify API for creating Fulfillments

packages/common/src/generated-types.ts

@@ -449,8 +449,7 @@ export type CreateFacetValueWithFacetInput = {
 };
 
 export type CreateFulfillmentInput = {
-  orderItemIds?: Maybe<Array<Scalars['ID']>>,
-  orderId?: Maybe<Scalars['ID']>,
+  lines: Array<FulfillmentLineInput>,
   method: Scalars['String'],
   trackingCode?: Maybe<Scalars['String']>,
 };
@@ -1050,6 +1049,11 @@ export type Fulfillment = Node & {
   trackingCode?: Maybe<Scalars['String']>,
 };
 
+export type FulfillmentLineInput = {
+  orderLineId: Scalars['ID'],
+  quantity: Scalars['Int'],
+};
+
 export type GlobalSettings = {
   __typename?: 'GlobalSettings',
   id: Scalars['ID'],
@@ -1572,14 +1576,14 @@ export type Mutation = {
   createShippingMethod: ShippingMethod,
   /** Update an existing ShippingMethod */
   updateShippingMethod: ShippingMethod,
-  /** Create a new TaxRate */
-  createTaxRate: TaxRate,
-  /** Update an existing TaxRate */
-  updateTaxRate: TaxRate,
   /** Create a new TaxCategory */
   createTaxCategory: TaxCategory,
   /** Update an existing TaxCategory */
   updateTaxCategory: TaxCategory,
+  /** Create a new TaxRate */
+  createTaxRate: TaxRate,
+  /** Update an existing TaxRate */
+  updateTaxRate: TaxRate,
   /** Create a new Zone */
   createZone: Zone,
   /** Update an existing Zone */
@@ -1857,23 +1861,23 @@ export type MutationUpdateShippingMethodArgs = {
 };
 
 
-export type MutationCreateTaxRateArgs = {
-  input: CreateTaxRateInput
+export type MutationCreateTaxCategoryArgs = {
+  input: CreateTaxCategoryInput
 };
 
 
-export type MutationUpdateTaxRateArgs = {
-  input: UpdateTaxRateInput
+export type MutationUpdateTaxCategoryArgs = {
+  input: UpdateTaxCategoryInput
 };
 
 
-export type MutationCreateTaxCategoryArgs = {
-  input: CreateTaxCategoryInput
+export type MutationCreateTaxRateArgs = {
+  input: CreateTaxRateInput
 };
 
 
-export type MutationUpdateTaxCategoryArgs = {
-  input: UpdateTaxCategoryInput
+export type MutationUpdateTaxRateArgs = {
+  input: UpdateTaxRateInput
 };
 
 
@@ -2424,10 +2428,10 @@ export type Query = {
   shippingMethod?: Maybe<ShippingMethod>,
   shippingEligibilityCheckers: Array<ConfigurableOperation>,
   shippingCalculators: Array<ConfigurableOperation>,
-  taxRates: TaxRateList,
-  taxRate?: Maybe<TaxRate>,
   taxCategories: Array<TaxCategory>,
   taxCategory?: Maybe<TaxCategory>,
+  taxRates: TaxRateList,
+  taxRate?: Maybe<TaxRate>,
   zones: Array<Zone>,
   zone?: Maybe<Zone>,
 };
@@ -2597,17 +2601,17 @@ export type QueryShippingMethodArgs = {
 };
 
 
-export type QueryTaxRatesArgs = {
-  options?: Maybe<TaxRateListOptions>
+export type QueryTaxCategoryArgs = {
+  id: Scalars['ID']
 };
 
 
-export type QueryTaxRateArgs = {
-  id: Scalars['ID']
+export type QueryTaxRatesArgs = {
+  options?: Maybe<TaxRateListOptions>
 };
 
 
-export type QueryTaxCategoryArgs = {
+export type QueryTaxRateArgs = {
   id: Scalars['ID']
 };
 

packages/core/e2e/graphql/fragments.ts

@@ -329,6 +329,9 @@ export const ORDER_ITEM_FRAGMENT = gql`
         unitPriceIncludesTax
         unitPriceWithTax
         taxRate
+        fulfillment {
+            id
+        }
     }
 `;
 

packages/core/e2e/graphql/generated-e2e-admin-types.ts

@@ -449,8 +449,7 @@ export type CreateFacetValueWithFacetInput = {
 };
 
 export type CreateFulfillmentInput = {
-    orderItemIds?: Maybe<Array<Scalars['ID']>>;
-    orderId?: Maybe<Scalars['ID']>;
+    lines: Array<FulfillmentLineInput>;
     method: Scalars['String'];
     trackingCode?: Maybe<Scalars['String']>;
 };
@@ -1048,6 +1047,11 @@ export type Fulfillment = Node & {
     trackingCode?: Maybe<Scalars['String']>;
 };
 
+export type FulfillmentLineInput = {
+    orderLineId: Scalars['ID'];
+    quantity: Scalars['Int'];
+};
+
 export type GlobalSettings = {
     __typename?: 'GlobalSettings';
     id: Scalars['ID'];
@@ -1569,14 +1573,14 @@ export type Mutation = {
     createShippingMethod: ShippingMethod;
     /** Update an existing ShippingMethod */
     updateShippingMethod: ShippingMethod;
-    /** Create a new TaxRate */
-    createTaxRate: TaxRate;
-    /** Update an existing TaxRate */
-    updateTaxRate: TaxRate;
     /** Create a new TaxCategory */
     createTaxCategory: TaxCategory;
     /** Update an existing TaxCategory */
     updateTaxCategory: TaxCategory;
+    /** Create a new TaxRate */
+    createTaxRate: TaxRate;
+    /** Update an existing TaxRate */
+    updateTaxRate: TaxRate;
     /** Create a new Zone */
     createZone: Zone;
     /** Update an existing Zone */
@@ -1803,14 +1807,6 @@ export type MutationUpdateShippingMethodArgs = {
     input: UpdateShippingMethodInput;
 };
 
-export type MutationCreateTaxRateArgs = {
-    input: CreateTaxRateInput;
-};
-
-export type MutationUpdateTaxRateArgs = {
-    input: UpdateTaxRateInput;
-};
-
 export type MutationCreateTaxCategoryArgs = {
     input: CreateTaxCategoryInput;
 };
@@ -1819,6 +1815,14 @@ export type MutationUpdateTaxCategoryArgs = {
     input: UpdateTaxCategoryInput;
 };
 
+export type MutationCreateTaxRateArgs = {
+    input: CreateTaxRateInput;
+};
+
+export type MutationUpdateTaxRateArgs = {
+    input: UpdateTaxRateInput;
+};
+
 export type MutationCreateZoneArgs = {
     input: CreateZoneInput;
 };
@@ -2361,10 +2365,10 @@ export type Query = {
     shippingMethod?: Maybe<ShippingMethod>;
     shippingEligibilityCheckers: Array<ConfigurableOperation>;
     shippingCalculators: Array<ConfigurableOperation>;
-    taxRates: TaxRateList;
-    taxRate?: Maybe<TaxRate>;
     taxCategories: Array<TaxCategory>;
     taxCategory?: Maybe<TaxCategory>;
+    taxRates: TaxRateList;
+    taxRate?: Maybe<TaxRate>;
     zones: Array<Zone>;
     zone?: Maybe<Zone>;
 };
@@ -2502,6 +2506,10 @@ export type QueryShippingMethodArgs = {
     id: Scalars['ID'];
 };
 
+export type QueryTaxCategoryArgs = {
+    id: Scalars['ID'];
+};
+
 export type QueryTaxRatesArgs = {
     options?: Maybe<TaxRateListOptions>;
 };
@@ -2510,10 +2518,6 @@ export type QueryTaxRateArgs = {
     id: Scalars['ID'];
 };
 
-export type QueryTaxCategoryArgs = {
-    id: Scalars['ID'];
-};
-
 export type QueryZoneArgs = {
     id: Scalars['ID'];
 };
@@ -3553,7 +3557,7 @@ export type OrderFragment = { __typename?: 'Order' } & Pick<
 export type OrderItemFragment = { __typename?: 'OrderItem' } & Pick<
     OrderItem,
     'id' | 'unitPrice' | 'unitPriceIncludesTax' | 'unitPriceWithTax' | 'taxRate'
->;
+> & { fulfillment: Maybe<{ __typename?: 'Fulfillment' } & Pick<Fulfillment, 'id'>> };
 
 export type OrderWithLinesFragment = { __typename?: 'Order' } & Pick<
     Order,
@@ -4607,6 +4611,7 @@ export namespace Order {
 
 export namespace OrderItem {
     export type Fragment = OrderItemFragment;
+    export type Fulfillment = NonNullable<OrderItemFragment['fulfillment']>;
 }
 
 export namespace OrderWithLines {

packages/core/e2e/order.e2e-spec.ts

@@ -88,7 +88,7 @@ describe('Orders resolver', () => {
         });
         await shopClient.query<AddItemToOrder.Mutation, AddItemToOrder.Variables>(ADD_ITEM_TO_ORDER, {
             productVariantId: 'T_3',
-            quantity: 1,
+            quantity: 2,
         });
     }, TEST_SETUP_TIMEOUT_MS);
 
@@ -182,15 +182,12 @@ describe('Orders resolver', () => {
     describe('fulfillment', () => {
 
         it('throws if Order is not in "PaymentSettled" state', assertThrowsWithMessage(async () => {
-                const { orders } = await adminClient.query<GetOrderList.Query>(GET_ORDERS_LIST);
-                const nonSettledOrder = orders.items.find(o => o.state !== 'PaymentSettled');
-                if (!nonSettledOrder) {
-                    fail('Could not find an Order not in the PaymentSettled state');
-                    return;
-                }
+                const { order } = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, { id: 'T_1' });
+                expect(order!.state).toBe('PaymentAuthorized');
+
                 await adminClient.query<CreateFulfillment.Mutation, CreateFulfillment.Variables>(CREATE_FULFILLMENT, {
                     input: {
-                        orderId: nonSettledOrder.id,
+                        lines: order!.lines.map(l => ({ orderLineId: l.id, quantity: l.quantity })),
                         method: 'Test',
                     },
                 });
@@ -199,32 +196,42 @@ describe('Orders resolver', () => {
             ),
         );
 
-        it('throws if neither orderId not orderItemIds are specified', assertThrowsWithMessage(async () => {
-                const { orders } = await adminClient.query<GetOrderList.Query>(GET_ORDERS_LIST);
-                const nonSettledOrder = orders.items.find(o => o.state !== 'PaymentSettled');
-                if (!nonSettledOrder) {
-                    fail('Could not find an Order not in the PaymentSettled state');
-                    return;
-                }
+        it('throws if lines is empty', assertThrowsWithMessage(async () => {
+                const { order } = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, { id: 'T_2' });
+                expect(order!.state).toBe('PaymentSettled');
                 await adminClient.query<CreateFulfillment.Mutation, CreateFulfillment.Variables>(CREATE_FULFILLMENT, {
                     input: {
+                        lines: [],
                         method: 'Test',
                     },
                 });
             },
-            'Please specify either orderId or orderItemIds',
+            'Nothing to fulfill',
+            ),
+        );
+
+        it('throws if all quantities are zero', assertThrowsWithMessage(async () => {
+                const { order } = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, { id: 'T_2' });
+                expect(order!.state).toBe('PaymentSettled');
+                await adminClient.query<CreateFulfillment.Mutation, CreateFulfillment.Variables>(CREATE_FULFILLMENT, {
+                    input: {
+                        lines: order!.lines.map(l => ({ orderLineId: l.id, quantity: 0 })),
+                        method: 'Test',
+                    },
+                });
+            },
+            'Nothing to fulfill',
             ),
         );
 
         it('creates a partial fulfillment', async () => {
             const { order } = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, { id: 'T_2' });
             expect(order!.state).toBe('PaymentSettled');
-
-            const orderItems = order!.lines.reduce((items, line) => [...items, ...line.items], [] as OrderItemFragment[]);
+            const lines = order!.lines;
 
             const { createFulfillment } = await adminClient.query<CreateFulfillment.Mutation, CreateFulfillment.Variables>(CREATE_FULFILLMENT, {
                 input: {
-                    orderItemIds: [orderItems[0].id],
+                    lines: lines.map(l => ({ orderLineId: l.id, quantity: 1 })),
                     method: 'Test',
                     trackingCode: '123456',
                 },
@@ -232,17 +239,29 @@ describe('Orders resolver', () => {
 
             expect(createFulfillment!.method).toBe('Test');
             expect(createFulfillment!.trackingCode).toBe('123456');
-            expect(createFulfillment!.orderItems).toEqual([{ id: orderItems[0].id }]);
+            expect(createFulfillment!.orderItems).toEqual([
+                { id: lines[0].items[0].id },
+                { id: lines[1].items[0].id },
+            ]);
 
             const result = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, { id: 'T_2' });
+            // expect(result.order!.lines).toEqual({});
             expect(result.order!.state).toBe('PartiallyFulfilled');
+            expect(result.order!.lines[0].items[0].fulfillment!.id).toBe(createFulfillment!.id);
+            expect(result.order!.lines[1].items[1].fulfillment!.id).toBe(createFulfillment!.id);
+            expect(result.order!.lines[1].items[0].fulfillment).toBe(null);
         });
 
         it('throws if an OrderItem already part of a Fulfillment', assertThrowsWithMessage(async () => {
+                const { order } = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, { id: 'T_2' });
+                expect(order!.state).toBe('PartiallyFulfilled');
                 await adminClient.query<CreateFulfillment.Mutation, CreateFulfillment.Variables>(CREATE_FULFILLMENT, {
                     input: {
                         method: 'Test',
-                        orderId: 'T_2',
+                        lines: [{
+                            orderLineId: order!.lines[0].id,
+                            quantity: 1,
+                        }],
                     },
                 });
             },
@@ -258,7 +277,10 @@ describe('Orders resolver', () => {
 
             const { createFulfillment } = await adminClient.query<CreateFulfillment.Mutation, CreateFulfillment.Variables>(CREATE_FULFILLMENT, {
                 input: {
-                    orderItemIds: [orderItems[1].id],
+                    lines: [{
+                        orderLineId: order!.lines[1].id,
+                        quantity: 1,
+                    }],
                     method: 'Test2',
                     trackingCode: '56789',
                 },

packages/core/src/api/common/id-codec.spec.ts

@@ -157,6 +157,19 @@ describe('IdCodecService', () => {
             expect(result.items[0].friends[0].name.id).toBe(ENCODED);
         });
 
+        it('works with lists with a nullable object property', () => {
+            const input = {
+                items: [
+                    { user: null },
+                    { user: { id: 'id' }},
+                ],
+            };
+
+            const result = idCodec.encode(input);
+            expect(result.items[0].user).toBe(null);
+            expect(result.items[1].user).toEqual({ id: ENCODED });
+        });
+
         it('works with nested list of nested lists', () => {
             const input = {
                 items: [
@@ -264,6 +277,19 @@ describe('IdCodecService', () => {
             });
         });
 
+        it('works with lists with a nullable object property', () => {
+            const input = {
+                items: [
+                    { user: null },
+                    { user: { id: 'id' }},
+                ],
+            };
+
+            const result = idCodec.decode(input);
+            expect(result.items[0].user).toBe(null);
+            expect(result.items[1].user).toEqual({ id: DECODED });
+        });
+
         it('works with large and nested list', () => {
             const length = 100;
             const input = {

packages/core/src/api/common/id-codec.ts

@@ -112,7 +112,7 @@ export class IdCodec {
     private isSimpleObject(target: any): boolean {
         const values = Object.values(target);
         for (const value of values) {
-            if (this.isObject(value)) {
+            if (this.isObject(value) || value === null) {
                 return false;
             }
         }

packages/core/src/api/resolvers/admin/order.resolver.ts

@@ -39,7 +39,7 @@ export class OrderResolver {
     }
 
     @Mutation()
-    @Decode('orderId', 'orderItemIds')
+    @Decode('orderLineId')
     @Allow(Permission.UpdateOrder)
     async createFulfillment(@Ctx() ctx: RequestContext, @Args() args: MutationCreateFulfillmentArgs) {
         return this.orderService.createFulfillment(ctx, args.input);

packages/core/src/api/schema/admin-api/order.api.graphql

@@ -12,8 +12,13 @@ type Mutation {
 input OrderListOptions
 
 input CreateFulfillmentInput {
-    orderItemIds: [ID!]
-    orderId: ID
+    lines: [FulfillmentLineInput!]!
     method: String!
     trackingCode: String
 }
+
+input FulfillmentLineInput {
+    orderLineId: ID!
+    quantity: Int!
+}
+

packages/core/src/i18n/messages/en.json

@@ -10,7 +10,7 @@
     "country-code-not-valid":  "The countryCode \"{ countryCode }\" was not recognized",
     "create-fulfillment-items-already-fulfilled": "One or more OrderItems have already been fulfilled",
     "create-fulfillment-orders-must-be-settled": "One or more OrderItems belong to an Order which is in an invalid state",
-    "create-fulfillment-specify-order-id-or-order-item-ids": "Please specify either orderId or orderItemIds",
+    "create-fulfillment-nothing-to-fulfill": "Nothing to fulfill",
     "email-address-not-available": "This email address is not available",
     "email-address-not-verified": "Please verify this email address before logging in",
     "entity-has-no-translation-in-language": "Translatable entity '{ entityName }' has not been translated into the requested language ({ languageCode })",

packages/core/src/service/services/order.service.ts

@@ -319,33 +319,33 @@ export class OrderService {
     }
 
     async createFulfillment(ctx: RequestContext, input: CreateFulfillmentInput) {
-        if (!input.orderId && (!input.orderItemIds || input.orderItemIds.length === 0)) {
-            throw new UserInputError('error.create-fulfillment-specify-order-id-or-order-item-ids');
+        if (!input.lines || input.lines.length === 0 || input.lines.reduce((total, line) => total + line.quantity, 0) === 0) {
+            throw new UserInputError('error.create-fulfillment-nothing-to-fulfill');
         }
         const relatedOrders = new Map<ID, Order>();
         const orderItems = new Map<ID, OrderItem>();
 
-        if (input.orderItemIds && input.orderItemIds.length) {
-            const items = await this.connection.getRepository(OrderItem).findByIds(input.orderItemIds, {
-                relations: ['line', 'line.order', 'fulfillment'],
-            });
-            for (const item of items) {
-                const order = item.line.order;
-                if (!relatedOrders.has(order.id)) {
-                    relatedOrders.set(order.id, order);
-                }
-                orderItems.set(item.id, item);
+        const lines = await this.connection.getRepository(OrderLine).findByIds(input.lines.map(l => l.orderLineId), {
+            relations: ['order', 'items', 'items.fulfillment'],
+        });
+        for (const line of lines) {
+            const inputLine = input.lines.find(l => idsAreEqual(l.orderLineId, line.id));
+            if (!inputLine) {
+                continue;
             }
-        }
-        if (input.orderId) {
-            const order = await this.findOne(ctx, input.orderId);
-            if (order) {
+            const order = line.order;
+            if (!relatedOrders.has(order.id)) {
                 relatedOrders.set(order.id, order);
-                order.lines.reduce((items, line) => [...items, ...line.items], [] as OrderItem[]).forEach(item => {
-                    orderItems.set(item.id, item);
-                });
             }
+            const unfulfilledItems = line.items.filter(i => !i.fulfillment);
+            if (unfulfilledItems.length < inputLine.quantity) {
+                throw new IllegalOperationError('error.create-fulfillment-items-already-fulfilled');
+            }
+            unfulfilledItems.slice(0, inputLine.quantity).forEach(item => {
+                orderItems.set(item.id, item);
+            });
         }
+
         for (const item of Array.from(orderItems.values())) {
             if (!!item.fulfillment) {
                 throw new IllegalOperationError('error.create-fulfillment-items-already-fulfilled');