Add ready for pickup button on the order page in admin panel which notifies customer that order could be picked up 2034 #2127

novikor · 2019-04-01T20:30:02Z

Description (*)

New API:
- \Magento\InventoryInStorePickupApi\Api\IsOrderReadyForPickupInterface;
- \Magento\InventoryInStorePickupApi\Api\NotifyOrderIsReadyAndShipInterface;
Added 'Notify Order is Ready for Pickup' button;
Notify customer using custom email template
Ship the order after the notification to reserve QTY

Fixed Issues (if relevant)

Add "Ready for Pickup" button on the Order page in Admin Panel which notifies Customer that Order could be picked up #2034: Add "Ready for Pickup" button on the Order page in Admin Panel which notifies Customer that Order could be picked up

Manual testing scenarios (*)

Life is too short for that

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)

…notifies Customer that Order could be picked up #2034. Added the button itself

…notifies Customer that Order could be picked up #2034. Added check if order is ready for pickup

…notifies Customer that Order could be picked up #2034. Moved IsOrderReadyForPickup to API level. Created an init point for NotifyPickup controller.

…notifies Customer that Order could be picked up #2034. NotifyOrderIsReadyAndShipInterface API; Implemented NotifyPickup controller; Added some TODOs

…notifies Customer that Order could be picked up #2034. Added canShip() check to IsOrderReadyForPickup; Added TODO

…notifies Customer that Order could be picked up #2034. Implemented synchronous email sending with custom email template for storepickup. Added even more TODOs

…notifies Customer that Order could be picked up #2034. Do not show button is order is not placed using storepickup or it can not be shipped

app/code/Magento/InventoryInStorePickup/Model/NotifyOrderIsReadyAndShip.php

app/code/Magento/InventoryInStorePickup/Model/Order/IsFulfilled.php

app/code/Magento/InventoryInStorePickup/Model/Order/Email/ReadyForPickupNotifier.php

app/code/Magento/InventoryInStorePickup/Model/NotifyOrderIsReadyAndShip.php

app/code/Magento/InventoryInStorePickup/Model/IsOrderReadyForPickup.php

app/code/Magento/InventoryInStorePickup/Model/Order/IsFulfilled.php

app/code/Magento/InventoryInStorePickup/Model/Order/Email/ReadyForPickupSender.php

app/code/Magento/InventoryInStorePickup/Model/Order/Email/Container/ReadyForPickupIdentity.php

app/code/Magento/InventoryInStorePickup/Model/Order/IsFulfilled.php

app/code/Magento/InventoryInStorePickupAdminUi/Controller/Adminhtml/Order/NotifyPickup.php

maghamed · 2019-04-02T14:10:41Z

app/code/Magento/InventoryInStorePickupAdminUi/Model/IsDisplayReadyForPickupButton.php

+
+namespace Magento\InventoryInStorePickupAdminUi\Model;
+
+class IsDisplayReadyForPickupButton


Display. May replace by Render

app/code/Magento/InventoryInStorePickupApi/Api/NotifyOrderIsReadyAndShipInterface.php

novikor · 2019-04-05T13:09:41Z

All the points seem to be fixed.
Please recheck.

I suggest to implement asynchronous email and tests coverage in separate Issues/PRs.

ishakhsuvarov · 2019-04-05T13:49:32Z

@novikor I agree with implementing those items separately. This PR is already growing out of control

ishakhsuvarov · 2019-04-05T13:56:46Z

app/code/Magento/InventoryInStorePickup/Model/IsOrderReadyForPickup.php

+use Magento\Sales\Api\OrderRepositoryInterface;
+use Magento\Sales\Model\Order;
+
+class IsOrderReadyForPickup implements IsOrderReadyForPickupInterface


Please indicate clear purpose of each class in docblock.

While class name looks like it speaks for itself, as a someone who is new to Store Pickup I wouldn't know the conditions for that.

PS: This applies to every class, interface and method hereafter

Done for all the classes and most of the methods (except execute).

ishakhsuvarov · 2019-04-05T13:57:38Z

app/code/Magento/InventoryInStorePickup/Model/NotifyOrderIsReadyForPickup.php

+    private $orderRepository;
+
+    /**
+     * NotifyOrderIsReadyAndShip constructor.


Eliminated.

ishakhsuvarov · 2019-04-05T13:59:19Z

app/code/Magento/InventoryInStorePickup/Model/NotifyOrderIsReadyForPickup.php

+        /** @noinspection PhpParamsInspection */
+        $this->emailNotifier->notify($this->orderRepository->get($orderId));
+
+        /* TODO: add order comment? */


Yes, good idea. I think it was discussed on one of the meetings

I`ll create an issue

ishakhsuvarov · 2019-04-05T13:59:36Z

app/code/Magento/InventoryInStorePickup/Model/NotifyOrderIsReadyForPickup.php

+            throw new OrderIsNotReadyForPickupException();
+        }
+
+        /** @noinspection PhpParamsInspection */


Of course, I could use instanceof to do not send the email or throw an exception instead of a fatal error.
But anyway more likely I will kill the notifier class in the nearest future, so it is OK

ishakhsuvarov · 2019-04-05T14:00:29Z

app/code/Magento/InventoryInStorePickup/Model/NotifyOrderIsReadyForPickup.php

+    }
+
+    /**
+     * {@inheritdoc}


Curly braces aren't needed here.

Reference: https://devdocs.magento.com/guides/v2.3/coding-standards/docblock-standard-general.html#other-DocBlock-tags

Eliminated.

ishakhsuvarov · 2019-04-05T14:01:34Z

app/code/Magento/InventoryInStorePickup/Model/Order/CanBeFulfilled.php

+    public function execute(OrderInterface $order): bool
+    {
+        if ($order->getExtensionAttributes()
+            && $sourceCode = $order->getExtensionAttributes()->getPickupLocationCode()


Is it only me or assignments inside conditional statements are not that good for readability?

Well, I really like doing that and like to save the code lines and avoid the code duplicating.

However, I`ve created a poll and realized that nobody likes that.
So, - fixed.

ishakhsuvarov · 2019-04-05T14:05:34Z

app/code/Magento/InventoryInStorePickup/Model/Order/Email/ReadyForPickupSender.php

+    private $eventManager;
+
+    /**
+     * ReadyForPickupSender constructor.


Eliminated.

ishakhsuvarov · 2019-04-05T14:16:22Z

app/code/Magento/InventoryInStorePickupAdminUi/Block/Adminhtml/Order/View/ReadyForPickup.php

+    private $isDisplayButton;
+
+    /**
+     * ReadyForPickup constructor.


Eliminated.

ishakhsuvarov · 2019-04-05T14:16:44Z

app/code/Magento/InventoryInStorePickupAdminUi/Controller/Adminhtml/Order/NotifyPickup.php

+    private $logger;
+
+    /**
+     * NotifyPickup constructor.


Eliminated.

…tifies customer that order could be picked up 2034. Simplified Fully Qualified names

…tifies customer that order could be picked up 2034. CanBeFulfilled -> IsFulfillable

…tifies customer that order could be picked up 2034. Killed OrderIsNotReadyForPickupException

…tifies customer that order could be picked up 2034. Just another PHPDocs improvement

…tifies customer that order could be picked up 2034. Stopped declaring variables inside if condition

…rder-page-in-Admin-Panel-which-notifies-Customer-that-Order-could-be-picked-up-2034

ishakhsuvarov · 2019-04-10T15:51:26Z

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

@@ -25,4 +25,14 @@
            </argument>
        </arguments>
    </type>
+
+    <preference for="\Magento\InventoryInStorePickupApi\Api\NotifyOrderIsReadyForPickupInterface"


Please avoid leading slashes in di configuration

ishakhsuvarov · 2019-04-10T16:01:47Z

app/code/Magento/InventoryInStorePickupAdminUi/Controller/Adminhtml/Order/NotifyPickup.php

+ *
+ * @package Magento\InventoryInStorePickupAdminUi\Controller\Adminhtml\Order
+ */
+class NotifyPickup extends Action


Name is slightly misleading. It's notify and ship in reality

- Minor coding style improvements

- Static testing results applied

ghost · 2019-04-11T14:52:07Z

Hi @novikor, 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.

MSI-2156 Adjust prs #2127, #2201.

- Minor coding style improvements

- Static testing results applied

novikor added 7 commits March 28, 2019 21:00

Add "Ready for Pickup" button on the Order page in Admin Panel which …

3c78648

…notifies Customer that Order could be picked up #2034. Added the button itself

Add "Ready for Pickup" button on the Order page in Admin Panel which …

1f04467

…notifies Customer that Order could be picked up #2034. Added check if order is ready for pickup

Add "Ready for Pickup" button on the Order page in Admin Panel which …

ab9e1ba

…notifies Customer that Order could be picked up #2034. Moved IsOrderReadyForPickup to API level. Created an init point for NotifyPickup controller.

Add "Ready for Pickup" button on the Order page in Admin Panel which …

166410d

…notifies Customer that Order could be picked up #2034. NotifyOrderIsReadyAndShipInterface API; Implemented NotifyPickup controller; Added some TODOs

Add "Ready for Pickup" button on the Order page in Admin Panel which …

5fc0018

…notifies Customer that Order could be picked up #2034. Added canShip() check to IsOrderReadyForPickup; Added TODO

Add "Ready for Pickup" button on the Order page in Admin Panel which …

b695a48

…notifies Customer that Order could be picked up #2034. Implemented synchronous email sending with custom email template for storepickup. Added even more TODOs

Add "Ready for Pickup" button on the Order page in Admin Panel which …

3269674

…notifies Customer that Order could be picked up #2034. Do not show button is order is not placed using storepickup or it can not be shipped

novikor added the Store Pickup label Apr 1, 2019

novikor assigned maghamed and ishakhsuvarov Apr 1, 2019

novikor requested a review from ishakhsuvarov April 1, 2019 20:30

magento-engcom-team added Partner: ISM eCompany partners-contribution labels Apr 1, 2019

novikor commented Apr 1, 2019

View reviewed changes

app/code/Magento/InventoryInStorePickup/Model/NotifyOrderIsReadyAndShip.php Outdated Show resolved Hide resolved

novikor commented Apr 1, 2019

View reviewed changes

app/code/Magento/InventoryInStorePickup/Model/Order/IsFulfilled.php Outdated Show resolved Hide resolved

novikor commented Apr 1, 2019

View reviewed changes

app/code/Magento/InventoryInStorePickup/Model/Order/Email/ReadyForPickupNotifier.php Show resolved Hide resolved