Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Msi 2182 source entity expansion with pickup location attributes #2267

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
4da7b8b
MSI-2182 Source Entity expansion with Pickup Location attributes.
Apr 16, 2019
10401ca
Merge remote-tracking branch 'origin/store-pickup' into MSI-2182-Sour…
novikor May 18, 2019
f6d9bb5
Source Entity expansion with Pickup Location attributes #2182.
novikor May 18, 2019
2314c9f
Source Entity expansion with Pickup Location attributes #2182.
novikor May 18, 2019
748fd78
Source Entity expansion with Pickup Location attributes #2182.
novikor May 18, 2019
7426756
Source Entity expansion with Pickup Location attributes #2182.
novikor May 18, 2019
7559630
Source Entity expansion with Pickup Location attributes #2182.
novikor May 18, 2019
551568b
Msi 2182 source entity expansion with pickup location attributes.
novikor May 20, 2019
143b67f
Msi 2182 source entity expansion with pickup location attributes.
novikor May 20, 2019
f46fe29
Merge remote-tracking branch 'origin/store-pickup' into MSI-2182-Sour…
May 21, 2019
28c8dda
Msi 2182 source entity expansion with pickup location attributes.
May 22, 2019
f57691c
Msi 2182 source entity expansion with pickup location attributes.
May 23, 2019
3b64018
Msi 2182 source entity expansion with pickup location attributes.
May 23, 2019
e276996
Msi 2182 source entity expansion with pickup location attributes.
May 23, 2019
86f5f3a
Msi 2182 source entity expansion with pickup location attributes.
May 23, 2019
0a20315
Merge remote-tracking branch 'origin/store-pickup' into MSI-2182-Sour…
novikor May 24, 2019
ca323b2
Merge branch 'store-pickup' into MSI-2182-Source-Entity-expansion-wit…
ishakhsuvarov May 30, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions app/code/Magento/InventoryInStorePickup/Model/PickupLocation.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,6 @@ class PickupLocation implements PickupLocationInterface
*/
private $phone;

/**
* @var string[]|null
*/
private $openHours;

/**
* @var string|null
*/
Expand All @@ -117,7 +112,6 @@ class PickupLocation implements PickupLocationInterface
* @param string|null $street
* @param string|null $postcode
* @param string|null $phone
* @param string[]|null $openHours
* @param PickupLocationExtensionInterface|null $extensionAttributes
*/
public function __construct(
Expand All @@ -136,7 +130,6 @@ public function __construct(
?string $street = null,
?string $postcode = null,
?string $phone = null,
?array $openHours = null,
?PickupLocationExtensionInterface $extensionAttributes = null
) {
$this->sourceCode = $sourceCode;
Expand All @@ -154,7 +147,6 @@ public function __construct(
$this->street = $street;
$this->postcode = $postcode;
$this->phone = $phone;
$this->openHours = $openHours;
$this->extensionAttributes = $extensionAttributes;
}

Expand Down Expand Up @@ -278,14 +270,6 @@ public function getPhone(): ?string
return $this->phone;
}

/**
* @inheritdoc
*/
public function getOpenHours(): ?array
{
return $this->openHours;
}

/**
* @inheritdoc
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);

namespace Magento\InventoryInStorePickup\Model\Source;

use Magento\Framework\Api\ExtensionAttributesFactory;
use Magento\Framework\DataObject;
use Magento\InventoryApi\Api\Data\SourceInterface;
use Magento\InventoryInStorePickupApi\Api\Data\PickupLocationInterface;

/**
* Set store-pickup related source extension attributes
*/
class SetExtensionAttributes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Class name seems to abstract

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InitPickupLocationExtensionAttributes ?

{
/**
* @var ExtensionAttributesFactory
*/
private $extensionAttributesFactory;

/**
* @param ExtensionAttributesFactory $extensionAttributesFactory
*/
public function __construct(ExtensionAttributesFactory $extensionAttributesFactory)
{
$this->extensionAttributesFactory = $extensionAttributesFactory;
}
/**
* @param SourceInterface|DataObject $source
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep doc block and contract consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

*/
public function execute(SourceInterface $source): void
{
$pickupAvailable = $source->getData(PickupLocationInterface::IS_PICKUP_LOCATION_ACTIVE);
$frontendName = $source->getData(PickupLocationInterface::FRONTEND_NAME);
$frontendDescription = $source->getData(PickupLocationInterface::FRONTEND_DESCRIPTION);
ishakhsuvarov marked this conversation as resolved.
Show resolved Hide resolved

$extensionAttributes = $source->getExtensionAttributes();

if ($extensionAttributes === null) {
$extensionAttributes = $this->extensionAttributesFactory->create(SourceInterface::class);
/** @noinspection PhpParamsInspection */
$source->setExtensionAttributes($extensionAttributes);
}

$extensionAttributes
->setIsPickupLocationActive((bool)$pickupAvailable)
->setFrontendName($frontendName)
->setFrontendDescription($frontendDescription);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,24 @@

namespace Magento\InventoryInStorePickup\Plugin\InventoryApi\SourceRepository;

use Magento\Framework\Api\ExtensionAttributesFactory;
use Magento\InventoryApi\Api\Data\SourceInterface;
use Magento\InventoryApi\Api\Data\SourceSearchResultsInterface;
use Magento\InventoryApi\Api\SourceRepositoryInterface;
use Magento\InventoryInStorePickupApi\Api\Data\PickupLocationInterface;
use Magento\InventoryInStorePickup\Model\Source\SetExtensionAttributes;

class LoadInStorePickupOnGetListPlugin
{
/**
* @var ExtensionAttributesFactory
* @var SetExtensionAttributes
*/
private $extensionAttributesFactory;
private $setExtensionAttributes;

/**
* @param ExtensionAttributesFactory $extensionAttributesFactory
* @param SetExtensionAttributes $setExtensionAttributes
*/
public function __construct(ExtensionAttributesFactory $extensionAttributesFactory)
{
$this->extensionAttributesFactory = $extensionAttributesFactory;
public function __construct(
SetExtensionAttributes $setExtensionAttributes
) {
$this->setExtensionAttributes = $setExtensionAttributes;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this seems to be an excessive extraction, it seems to be the bet solution if we don't figure out to load data in a better way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, as a variant, it probably would be nice to extract this logic into a chain of responsibility.

}

/**
Expand All @@ -40,18 +39,12 @@ public function __construct(ExtensionAttributesFactory $extensionAttributesFacto
public function afterGetList(
SourceRepositoryInterface $subject,
SourceSearchResultsInterface $sourceSearchResults
):SourceSearchResultsInterface {
foreach ($sourceSearchResults->getItems() as $source) {
$extensionAttributes = $source->getExtensionAttributes();

if ($extensionAttributes === null) {
$extensionAttributes = $this->extensionAttributesFactory->create(SourceInterface::class);
$source->setExtensionAttributes($extensionAttributes);
}

$pickupAvailable = $source->getData(PickupLocationInterface::IS_PICKUP_LOCATION_ACTIVE);
$extensionAttributes->setIsPickupLocationActive((bool)$pickupAvailable);
}
): SourceSearchResultsInterface {
$items = $sourceSearchResults->getItems();
array_walk(
$items,
[$this->setExtensionAttributes, 'execute']
);

return $sourceSearchResults;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,49 +7,41 @@

namespace Magento\InventoryInStorePickup\Plugin\InventoryApi\SourceRepository;

use Magento\Framework\Api\ExtensionAttributesFactory;
use Magento\Framework\DataObject;
use Magento\InventoryApi\Api\Data\SourceInterface;
use Magento\InventoryApi\Api\SourceRepositoryInterface;
use Magento\InventoryInStorePickupApi\Api\Data\PickupLocationInterface;
use Magento\InventoryInStorePickup\Model\Source\SetExtensionAttributes;

class LoadInStorePickupOnGetPlugin
{
/**
* @var \Magento\Framework\Api\ExtensionAttributesFactory
* @var SetExtensionAttributes
*/
private $extensionAttributesFactory;
private $setExtensionAttributes;

/**
* @param \Magento\Framework\Api\ExtensionAttributesFactory $extensionAttributesFactory
* @param SetExtensionAttributes $setExtensionAttributes
*/
public function __construct(ExtensionAttributesFactory $extensionAttributesFactory)
{
$this->extensionAttributesFactory = $extensionAttributesFactory;
public function __construct(
SetExtensionAttributes $setExtensionAttributes
) {
$this->setExtensionAttributes = $setExtensionAttributes;
}

/**
* Enrich the given Source Objects with the In-Store pickup attribute
*
* @param SourceRepositoryInterface $subject
* @param SourceInterface $source
* @param SourceInterface|DataObject $source
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's cheating. We can't typehint for SourceInterface and rely on Dataobject in reality

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

*
* @return SourceInterface
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function afterGet(
SourceRepositoryInterface $subject,
SourceInterface $source
):SourceInterface {
$pickupAvailable = $source->getData(PickupLocationInterface::IS_PICKUP_LOCATION_ACTIVE);

$extensionAttributes = $source->getExtensionAttributes();

if ($extensionAttributes === null) {
$extensionAttributes = $this->extensionAttributesFactory->create(SourceInterface::class);
$source->setExtensionAttributes($extensionAttributes);
}

$extensionAttributes->setIsPickupLocationActive((bool)$pickupAvailable);
): SourceInterface {
$this->setExtensionAttributes->execute($source);

return $source;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,32 @@

namespace Magento\InventoryInStorePickup\Plugin\InventoryApi\SourceRepository;

use Magento\Framework\DataObject;
use Magento\InventoryApi\Api\Data\SourceInterface;
use Magento\InventoryApi\Api\SourceRepositoryInterface;
use Magento\InventoryInStorePickupApi\Api\Data\PickupLocationInterface as Location;

class SaveInStorePickupPlugin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add description while we are here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

{
/**
* Persist the In-Store pickup attribute on Source save
*
* @param SourceRepositoryInterface $subject
* @param SourceInterface $source
* @param SourceInterface|DataObject $source
*
* @return array
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function beforeSave(
SourceRepositoryInterface $subject,
SourceInterface $source
):array {
): array {
$extensionAttributes = $source->getExtensionAttributes();

if ($extensionAttributes !== null && $extensionAttributes->getIsPickupLocationActive() !== null) {
$source->setIsPickupLocationActive($extensionAttributes->getIsPickupLocationActive());
if ($extensionAttributes !== null) {
$source->setData(Location::IS_PICKUP_LOCATION_ACTIVE, $extensionAttributes->getIsPickupLocationActive());
$source->setData(Location::FRONTEND_NAME, $extensionAttributes->getFrontendName());
$source->setData(Location::FRONTEND_DESCRIPTION, $extensionAttributes->getFrontendDescription());
}

return [$source];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,22 @@ protected function setUp()
public function testGetListOfSourcesWithPickupLocationExtensionAfterSave()
{
$pickupLocationConfig = [
'default' => false,
'eu-1' => true,
'eu-2' => true,
'eu-3' => false,
'eu-disabled' => false,
'us-1' => true,
'default' => ['active' => false, 'name' => 'default', 'desc' => 'default'],
'eu-1' => ['active' => true, 'name' => '', 'desc' => ''],
'eu-2' => ['active' => true, 'name' => 'zzz', 'desc' => ''],
'eu-3' => ['active' => false, 'name' => '', 'desc' => 'zzz1'],
'eu-disabled' => ['active' => false, 'name' => '', 'desc' => ''],
'us-1' => ['active' => true, 'name' => '666', 'desc' => ''],
];

$searchResult = $this->sourceRepository->getList();

/** @var SourceInterface $item */
foreach ($searchResult->getItems() as $item) {
$item->getExtensionAttributes()->setIsPickupLocationActive(
$pickupLocationConfig[$item->getSourceCode()]
);
$item->getExtensionAttributes()
->setIsPickupLocationActive($pickupLocationConfig[$item->getSourceCode()]['active'])
->setFrontendDescription($pickupLocationConfig[$item->getSourceCode()]['desc'])
->setFrontendName($pickupLocationConfig[$item->getSourceCode()]['name']);
$this->sourceRepository->save($item);
}

Expand All @@ -57,7 +58,10 @@ public function testGetListOfSourcesWithPickupLocationExtensionAfterSave()
$pickupLocationsStatus = [];

foreach ($searchResult->getItems() as $item) {
$pickupLocationsStatus[$item->getSourceCode()] = $item->getExtensionAttributes()->getIsPickupLocationActive();
$extension = $item->getExtensionAttributes();
$pickupLocationsStatus[$item->getSourceCode()]['active'] = $extension->getIsPickupLocationActive();
$pickupLocationsStatus[$item->getSourceCode()]['name'] = $extension->getFrontendName();
$pickupLocationsStatus[$item->getSourceCode()]['desc'] = $extension->getFrontendDescription();
}

$this->assertEquals($pickupLocationConfig, $pickupLocationsStatus);
Expand All @@ -71,10 +75,15 @@ public function testGetSourceWithPickupLocationExtensionAfterSave()
$sourceCode = 'source-code-1';

$source = $this->sourceRepository->get($sourceCode);
$source->getExtensionAttributes()->setIsPickupLocationActive(true);
$source->getExtensionAttributes()
->setIsPickupLocationActive(true)
->setFrontendName('zzz')
->setFrontendDescription('666');
$this->sourceRepository->save($source);

$source = $this->sourceRepository->get($sourceCode);
$this->assertEquals(true, $source->getExtensionAttributes()->getIsPickupLocationActive());
$this->assertEquals('zzz', $source->getExtensionAttributes()->getFrontendName());
$this->assertEquals('666', $source->getExtensionAttributes()->getFrontendDescription());
}
}
Loading