Msi 2030 add ability to mark orders placed with store pickup by swnsma · Pull Request #2082 · magento/inventory

swnsma · 2019-03-14T14:45:33Z

Description (*)

Add possibility to mark orders, created with 'In Store Pickup' Shipping method.
Add extension attribute to order entity with information about related pickup point.

Fixed Issues

Add an ability to Mark Orders placed with Store Pickup #2030: Add an ability to Mark Orders placed with Store Pickup

Points to discuss

Probably, it will be nice to add this extension attribute to collection join:

    <extension_attributes for="Magento\Sales\Api\Data\OrderInterface">
        <attribute code="pickup_point_id" type="string">
            <join reference_table="inventory_pickup_point_order" join_on_field="entity_id" reference_field="order_id">
                <field column="pickup_point_id">pickup_point_id</field>
            </join>
        </attribute>

But for this we need to add join processor for Order Repository 'getList' method in Magento 2 Core Repository (https://github.com/magento-engcom/msi/blob/475dc7c32e54621087b9669ca76ab1eacecde12f/app/code/Magento/Sales/Model/OrderRepository.php#L197)
E.g.

        /** @var \Magento\Framework\Api\ExtensionAttribute\JoinProcessor $extensionJoinProcessor */
        $extensionJoinProcessor = ObjectManager::getInstance()->get('\Magento\Framework\Api\ExtensionAttribute\JoinProcessor');
        $extensionJoinProcessor->process($searchResult, OrderInterface::class);

UPD: issue already reported magento/magento2#8035

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

Declare db schema.

Add new extension attribute.

Decrease schema dependency on inventory_source.

magento-cicd2 · 2019-03-14T14:45:36Z

All committers have signed the CLA.

ishakhsuvarov

Pickup Point term has to be discussed and aligned with business domain
Please add integration tests as well
Please update with the latest changes from store-pickup branch

ishakhsuvarov · 2019-03-14T15:38:42Z

...ento/InventoryInStorePickup/Model/ResourceModel/OrderPickupPoint/GetPickupPointByOrderId.php

+use Magento\Framework\App\ResourceConnection;
+
+/**
+ * Class GetPickupPointByOrderId.


Repeating class name in the comment is redundant. Clear responsibility description is sufficient.

ishakhsuvarov · 2019-03-14T15:39:05Z

...ento/InventoryInStorePickup/Model/ResourceModel/OrderPickupPoint/GetPickupPointByOrderId.php

+ */
+class GetPickupPointByOrderId
+{
+    const ORDER_ID        = 'order_id';


Should these constants be private?

ishakhsuvarov · 2019-03-14T15:41:07Z

...ento/InventoryInStorePickup/Model/ResourceModel/OrderPickupPoint/GetPickupPointByOrderId.php

+     *
+     * @return string|null
+     */
+    public function execute(int $orderId):?string


Why should pickup point id be a string? Maybe it's a point code?

ishakhsuvarov · 2019-03-14T15:45:11Z

app/code/Magento/InventoryInStorePickup/etc/db_schema.xml

+-->
+<schema xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:noNamespaceSchemaLocation="urn:magento:framework:Setup/Declaration/Schema/etc/schema.xsd">
+    <table name="inventory_pickup_point_order" resource="default" engine="innodb">


We have to discuss Pickup Point naming. It's best to align it with business domain

Changed to 'Pickup Location'.

ishakhsuvarov · 2019-03-14T15:46:15Z

app/code/Magento/InventoryInStorePickup/etc/module.xml

+    <module name="Magento_InventoryInStorePickup" setup_version="1.0.0">
+        <sequence>
+            <module name="Magento_Sales" />
+            <module name="Magento_Inventory" />


Should it be Magento_Inventory or InventoryApi?

Also sales should be added to composer dependencies

For now, dependency on Magento_Sales only.
Added to composer.json.

…ability-to-mark-orders-placed-with-store-pickup

Fix tests and apply code review corrections.

…ability-to-mark-orders-placed-with-store-pickup

Add integration test for order placement. MSI-2030-Add an ability to Mark Orders placed with Store Pickup.

Pickup Point -> Pickup Location Pickup Point Id -> Pickup Location Code

Add join directive for extension attribute.

ishakhsuvarov · 2019-03-22T21:29:09Z

...de/Magento/InventoryInStorePickup/Test/_files/create_in_store_pickup_quote_on_eu_website.php

+/** @var AddressInterface $address */
+$address = $addressFactory->create(
+    [
+        'data' => [


I love how you save a US address to the Quote in fixture designed for EU Website :)

…placed-with-store-pickup

ghost · 2019-03-23T14:14:44Z

Hi @swnsma, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

al.kravchuk added 3 commits March 14, 2019 16:03

MSI-2030-Add an ability to Mark Orders placed with Store Pickup.

f82908b

Declare db schema.

MSI-2030-Add an ability to Mark Orders placed with Store Pickup.

95e7878

Add new extension attribute.

MSI-2030-Add an ability to Mark Orders placed with Store Pickup.

ec60951

Decrease schema dependency on inventory_source.

magento-engcom-team added partners-contribution Partner: ISM eCompany labels Mar 14, 2019

ishakhsuvarov suggested changes Mar 14, 2019

View reviewed changes

ishakhsuvarov added Store Pickup Award: special achievement labels Mar 14, 2019

al.kravchuk added 4 commits March 14, 2019 23:17

Merge remote-tracking branch 'origin/store-pickup' into MSI-2030-add-…

5ada1ee

…ability-to-mark-orders-placed-with-store-pickup

MSI-2030-Add an ability to Mark Orders placed with Store Pickup.

4b5296a

Fix tests and apply code review corrections.

Merge remote-tracking branch 'origin/store-pickup' into MSI-2030-add-…

a9e1a88

…ability-to-mark-orders-placed-with-store-pickup

MSI-2030-Add an ability to Mark Orders placed with Store Pickup.

3548061

Add integration test for order placement. MSI-2030-Add an ability to Mark Orders placed with Store Pickup.

swnsma force-pushed the MSI-2030-add-ability-to-mark-orders-placed-with-store-pickup branch from 744249d to 3548061 Compare March 18, 2019 08:17

ishakhsuvarov mentioned this pull request Mar 18, 2019

magento/magento2#8035: Join extension attributes are not added to Order results (REST api) magento/magento2#21797

Merged

4 tasks

al.kravchuk added 2 commits March 22, 2019 13:46

MSI-2030-Add an ability to Mark Orders placed with Store Pickup.

b600990

Pickup Point -> Pickup Location Pickup Point Id -> Pickup Location Code

MSI-2030-Add an ability to Mark Orders placed with Store Pickup.

c06e37c

Add join directive for extension attribute.

swnsma requested a review from ishakhsuvarov March 22, 2019 12:08

ishakhsuvarov self-assigned this Mar 22, 2019

ishakhsuvarov reviewed Mar 22, 2019

View reviewed changes

ishakhsuvarov approved these changes Mar 22, 2019

View reviewed changes

ishakhsuvarov added 2 commits March 22, 2019 16:39

Merge branch 'store-pickup' into MSI-2030-add-ability-to-mark-orders-…

27554a7

…placed-with-store-pickup

#2082: Minor coding style fixes

97556b5

ishakhsuvarov merged commit d3ead30 into store-pickup Mar 23, 2019

magento-engcom-team added Award: complex Award: test coverage and removed Award: special achievement labels Mar 23, 2019

ishakhsuvarov mentioned this pull request Mar 25, 2019

Add an ability to Mark Orders placed with Store Pickup #2030

Closed

magento-engcom-team removed the Award: complex label Mar 31, 2019

magento-engcom-team added the Award: advanced label Mar 31, 2019

ishakhsuvarov added this to the Store Pickup Support milestone May 30, 2019

ishakhsuvarov deleted the MSI-2030-add-ability-to-mark-orders-placed-with-store-pickup branch June 19, 2019 16:19

ishakhsuvarov removed this from the Store Pickup Support milestone Sep 10, 2019

ishakhsuvarov added a commit that referenced this pull request Nov 22, 2019

#2082: Minor coding style fixes

3641423

Conversation

swnsma commented Mar 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues

Points to discuss

Contribution checklist (*)

Uh oh!

magento-cicd2 commented Mar 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ishakhsuvarov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Mar 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

swnsma commented Mar 14, 2019 •

edited

Loading

magento-cicd2 commented Mar 14, 2019 •

edited

Loading

ishakhsuvarov left a comment •

edited

Loading