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

WIP: Msi stock ui aggregation #39

Merged
merged 82 commits into from
Jul 28, 2017
Merged
Show file tree
Hide file tree
Changes from 76 commits
Commits
Show all changes
82 commits
Select commit Hold shift + click to select a range
be1d21e
magento-engcom/magento#34: Source to Stock Mapping UI and API
vrann Jun 26, 2017
dfd6a47
agento-engcom/magento#34: Source to Stock Mapping UI and API
jonfres Jun 26, 2017
67775b7
magento-engcom/magento#34: Source to Stock Mapping UI and API
vrann Jun 26, 2017
fbb4515
magento-engcom/magento#34: Source to Stock Mapping UI and API
jonfres Jun 26, 2017
1755600
magento-engcom/magento#34 Inventory Stock table created
Jun 26, 2017
3514312
Merge branch 'msi-stock-ui-aggregation' of https://github.com/jonfres…
Jun 26, 2017
35a65fb
magento-engcom/magento#34 Source stock link table created
Jun 26, 2017
01e5081
magento-engcom/magento#34 upgrade moved to the install script
Jun 26, 2017
9cb1802
magento-engcom/magento#34: Source to Stock Mapping UI and API
vrann Jun 26, 2017
e712957
magento-engcom/magento#34: Source to Stock Mapping UI and API
jonfres Jun 26, 2017
d51900e
Merge branch 'msi-stock-ui-aggregation' of https://github.com/jonfres…
vrann Jun 26, 2017
04b31b5
Merge branch 'msi-stock-ui-aggregation' of https://github.com/jonfres…
jonfres Jun 26, 2017
b659579
magento-engcom/magento#34 stock interface stock id constant change
Jun 26, 2017
9788440
magento-engcom/magento#34: Source to Stock Mapping UI and API
jonfres Jun 26, 2017
2530ee2
magento-engcom/magento#34: Source to Stock Mapping UI and API
vrann Jun 26, 2017
9fab8e2
magento-engcom/magento#34 admin add/edit Stock form and action added
Jun 26, 2017
314a859
magento-engcom/magento#34 Stock for created
Jun 26, 2017
394aa38
Merge branch 'msi-stock-ui-aggregation' of https://github.com/jonfres…
Jun 26, 2017
81f5921
magento-engcom/magento#34 saving Stock action added
Jun 26, 2017
e600552
magento-engcom/magento#34: Source to Stock Mapping UI and API
jonfres Jun 26, 2017
d2d5028
Merge branch 'msi-stock-ui-aggregation' of https://github.com/jonfres…
jonfres Jun 26, 2017
4947eb3
source item management
kieronthomas Jun 26, 2017
b27c53b
Merge pull request #36 from jonfres/source-item-management
Jun 30, 2017
2ec7748
MSI:
Jun 30, 2017
32f80ff
some code fixes, remove class names, improve ternary operator
jonfres Jul 3, 2017
1e020f2
MSI:
Jul 3, 2017
78cec9f
MSI: - Code Review Changes
Jul 3, 2017
aad1755
MSI:
Jul 3, 2017
d0a7ede
MSI
Jul 4, 2017
ac93d41
Merge remote-tracking branch 'origin/source-item-management' into sou…
Jul 4, 2017
7630364
MSI
Jul 4, 2017
629bfa8
Merge pull request #35 from jonfres/msi-stock-ui-aggregation
Jul 4, 2017
7b5a1a1
MSI - Pr38
Jul 6, 2017
06ac206
MSI - Pr38
Jul 6, 2017
fc83584
MSI - Pr38
Jul 6, 2017
984ce8b
MSI - Pr38
Jul 6, 2017
5296b0d
MSI - Pr38
Jul 6, 2017
760cca9
Merge branch 'source-item-management' into msi-stock-ui-aggregation
Jul 8, 2017
5647da2
magento-engcom/magento#34: Add Interfaces and add review fixes
Jul 8, 2017
0acae24
magento-engcom/magento#34:
Jul 9, 2017
c995aee
MSI - Pr38
Jul 10, 2017
d18bf44
MSI - Pr38
Jul 10, 2017
7603f58
MSI - Pr38
Jul 10, 2017
10cd6bf
MSI - Pr38
Jul 10, 2017
f3de3dc
MSI - Pr38
Jul 10, 2017
7272f26
MSI - Pr38
Jul 12, 2017
1e39c8f
MSI - Pr38
Jul 13, 2017
e6b74fb
Merge branch 'source-item-management' into msi-stock-ui-aggregation
Jul 13, 2017
e170309
Merge remote-tracking branch 'origin/msi-stock-ui-aggregation' into m…
Jul 13, 2017
8dd48da
MSI - Msi stock ui aggregation #39
Jul 13, 2017
d9230f8
MSI - Msi stock ui aggregation #39
Jul 13, 2017
ae3dfef
MSI - Msi stock ui aggregation #39
Jul 13, 2017
be564cd
MSI - Msi stock ui aggregation #39
Jul 13, 2017
a0f2f49
MSI - Msi stock ui aggregation #39
Jul 13, 2017
a88fe92
MSI magento-engcom/magento#34:
Jul 15, 2017
67da5db
MSI magento-engcom/magento#34:
Jul 17, 2017
3c6bde2
MSI - Msi stock ui aggregation #39
Jul 18, 2017
2017309
MSI - Msi stock ui aggregation #39
Jul 18, 2017
e3061b7
Merge remote-tracking branch 'origin/msi-stock-ui-aggregation' into m…
Jul 18, 2017
46f4835
MSI - Msi stock ui aggregation #39
Jul 18, 2017
855d483
MSI - Msi stock ui aggregation #39
Jul 18, 2017
866b7e1
MSI - Msi stock ui aggregation #39
Jul 18, 2017
f6012a0
MSI - Msi stock ui aggregation #39
Jul 20, 2017
d2e743f
MSI - Msi stock ui aggregation #39
Jul 20, 2017
66cd1cd
MSI - Msi stock ui aggregation #39
Jul 20, 2017
1cac332
MSI - Msi stock ui aggregation #39
Jul 20, 2017
aa9eb08
MSI - Msi stock ui aggregation #39
Jul 21, 2017
725112d
MSI - Msi stock ui aggregation #39
Jul 21, 2017
dd8ef3a
MSI - Msi stock ui aggregation #39
Jul 24, 2017
4be0150
MSI - Msi stock ui aggregation #39
Jul 24, 2017
b0d34c1
MSI - Msi stock ui aggregation #39
Jul 24, 2017
1738b3a
MSI - Msi stock ui aggregation #39
Jul 26, 2017
1b93290
MSI - Msi stock ui aggregation #39
Jul 26, 2017
73de6f4
MSI - Msi stock ui aggregation #39
Jul 26, 2017
e1ced8d
MSI - Msi stock ui aggregation #39
Jul 26, 2017
268a279
MSI - Msi stock ui aggregation #39
Jul 26, 2017
009a2b1
MSI - Msi stock ui aggregation #39
Jul 27, 2017
9175a99
MSI - Msi stock ui aggregation #39
Jul 27, 2017
770bc26
MSI - Msi stock ui aggregation #39
Jul 28, 2017
90d0194
MSI - Msi stock ui aggregation #39
Jul 28, 2017
e1ea6e5
MSI - Msi stock ui aggregation #39
Jul 28, 2017
956747b
MSI - Msi stock ui aggregation #39
Jul 28, 2017
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use Magento\Shipping\Model\Config;

/**
* Class CarrierRequestDataHydrator
* Populate Source by carrier codes links
Copy link
Contributor

Choose a reason for hiding this comment

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

Populate Source by carrier code links

*/
class CarrierRequestDataHydrator
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use Magento\InventoryApi\Api\Data\SourceInterface;

/**
* Class Edit
* Edit Controller
Copy link
Contributor

Choose a reason for hiding this comment

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

it's better not to provide such comments, because developer undestands that it's
Edit Controller class,

it's better to provide description what the class does or responsible for , to make description valueable

*/
class Edit extends Action
{
Expand All @@ -42,7 +42,7 @@ public function __construct(
}

/**
* {@inheritdoc}
* @inheritdoc
*/
public function execute()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use Magento\Framework\Controller\ResultFactory;

/**
* Class Index
* Index Controller
Copy link
Contributor

Choose a reason for hiding this comment

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

if you give such description, you should specify Index Controller of which Entity.

This is Source Index Controller.

*/
class Index extends Action
{
Expand All @@ -20,7 +20,7 @@ class Index extends Action
const ADMIN_RESOURCE = 'Magento_Inventory::source';

/**
* {@inheritdoc}
* @inheritdoc
*/
public function execute()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use Magento\InventoryApi\Api\Data\SourceInterface;

/**
* Class InlineEdit
* InlineEdit Controller
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

*/
class InlineEdit extends Action
{
Expand Down Expand Up @@ -51,7 +51,7 @@ public function __construct(
}

/**
* {@inheritdoc}
* @inheritdoc
*/
public function execute()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use Magento\Framework\Controller\ResultFactory;

/**
* Class NewAction
* NewAction Controller
*/
class NewAction extends Action
{
Expand All @@ -21,7 +21,7 @@ class NewAction extends Action
const ADMIN_RESOURCE = 'Magento_Inventory::source';

/**
* {@inheritdoc}
* @inheritdoc
*/
public function execute()
{
Expand Down
37 changes: 9 additions & 28 deletions app/code/Magento/Inventory/Controller/Adminhtml/Source/Save.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@
use Magento\Framework\Api\DataObjectHelper;
use Magento\Framework\Controller\Result\Redirect;
use Magento\Framework\Exception\CouldNotSaveException;
use Magento\Framework\Exception\InputException;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Framework\Registry;
use Magento\InventoryApi\Api\Data\SourceInterface;
use Magento\InventoryApi\Api\Data\SourceInterfaceFactory;
use Magento\InventoryApi\Api\SourceRepositoryInterface;

/**
* Class Save
* Save Controller
Copy link
Contributor

Choose a reason for hiding this comment

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

please fix description

*/
class Save extends Action
{
Expand All @@ -28,11 +27,6 @@ class Save extends Action
*/
const ADMIN_RESOURCE = 'Magento_Inventory::source';

/**
* Registry source_id key
*/
const REGISTRY_SOURCE_ID_KEY = 'source_id';

/**
* @var SourceInterfaceFactory
*/
Expand All @@ -48,11 +42,6 @@ class Save extends Action
*/
private $dataObjectHelper;

/**
* @var Registry
*/
private $registry;

/**
* @var CarrierRequestDataHydrator
*/
Expand All @@ -64,39 +53,34 @@ class Save extends Action
* @param SourceRepositoryInterface $sourceRepository
* @param DataObjectHelper $dataObjectHelper
* @param CarrierRequestDataHydrator $carrierRequestDataHydrator
* @param Registry $registry
*/
public function __construct(
Context $context,
SourceInterfaceFactory $sourceFactory,
SourceRepositoryInterface $sourceRepository,
DataObjectHelper $dataObjectHelper,
Registry $registry,
CarrierRequestDataHydrator $carrierRequestDataHydrator
) {
parent::__construct($context);
$this->sourceFactory = $sourceFactory;
$this->sourceRepository = $sourceRepository;
$this->dataObjectHelper = $dataObjectHelper;
$this->registry = $registry;
$this->carrierRequestDataHydrator = $carrierRequestDataHydrator;
}

/**
* {@inheritdoc}
* @inheritdoc
*/
public function execute()
{
$resultRedirect = $this->resultRedirectFactory->create();
$requestData = $this->getRequest()->getParam('general');
if ($this->getRequest()->isPost() && $requestData) {
$requestData = $this->getRequest()->getParams();
if ($this->getRequest()->isPost() && !empty($requestData['general'])) {
try {
$sourceId = !empty($requestData[SourceInterface::SOURCE_ID])
? $requestData[SourceInterface::SOURCE_ID] : null;
$sourceId = !empty($requestData['general'][SourceInterface::SOURCE_ID])
? $requestData['general'][SourceInterface::SOURCE_ID] : null;

$sourceId = $this->processSave($sourceId, $requestData);
// Keep data for plugins on Save controller. Now we can not call separate services from one form.
$this->registry->register(self::REGISTRY_SOURCE_ID_KEY, $sourceId);

$this->messageManager->addSuccessMessage(__('The Source has been saved.'));
$this->processRedirectAfterSuccessSave($resultRedirect, $sourceId);
Expand All @@ -107,11 +91,8 @@ public function execute()
} catch (CouldNotSaveException $e) {
$this->messageManager->addErrorMessage($e->getMessage());
$this->processRedirectAfterFailureSave($resultRedirect, $sourceId);
} catch (InputException $e) {
$this->messageManager->addErrorMessage($e->getMessage());
$this->processRedirectAfterFailureSave($resultRedirect, $sourceId);
} catch (Exception $e) {
$this->messageManager->addErrorMessage(__('Could not save source'));
$this->messageManager->addErrorMessage(__('Could not save source.'));
$this->processRedirectAfterFailureSave($resultRedirect, $sourceId);
}
} else {
Expand All @@ -134,8 +115,8 @@ private function processSave($sourceId, array $requestData)
/** @var SourceInterface $source */
$source = $this->sourceFactory->create();
}
$this->dataObjectHelper->populateWithArray($source, $requestData, SourceInterface::class);
$source = $this->carrierRequestDataHydrator->hydrate($source, $requestData);
$this->dataObjectHelper->populateWithArray($source, $requestData['general'], SourceInterface::class);
$source = $this->carrierRequestDataHydrator->hydrate($source, $requestData['general']);
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we have two separate hydration here in the controller code?

        $this->dataObjectHelper->populateWithArray($source, $requestData['general'], SourceInterface::class);
        $source = $this->carrierRequestDataHydrator->hydrate($source, $requestData['general']);

controlelr should accept the only Hydrator dependency .
And that hydrator should handle carrier links logic, if needed it could be done via separate class.
But now your controller is aware of the structure of Source object (that it contains Carrier Links and these links should be handled separately) which is wrong.

Copy link

Choose a reason for hiding this comment

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

Need to discuss how to do it better
This code related only to controller (for specific form)

Copy link

Choose a reason for hiding this comment

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

fixed


$sourceId = $this->sourceRepository->save($source);
return $sourceId;
Expand Down
70 changes: 70 additions & 0 deletions app/code/Magento/Inventory/Controller/Adminhtml/Stock/Edit.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Inventory\Controller\Adminhtml\Stock;

use Magento\Backend\App\Action;
use Magento\Backend\App\Action\Context;
use Magento\Backend\Model\View\Result\Page;
use Magento\Framework\Controller\Result\Redirect;
use Magento\Framework\Controller\ResultFactory;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\InventoryApi\Api\StockRepositoryInterface;
use Magento\InventoryApi\Api\Data\StockInterface;

/**
* Edit Controller
*/
class Edit extends Action
{
/**
* @see _isAllowed()
*/
const ADMIN_RESOURCE = 'Magento_Inventory::stock';

/**
* @var StockRepositoryInterface
*/
private $stockRepository;

/**
* @param Context $context
* @param StockRepositoryInterface $stockRepository
*/
public function __construct(
Context $context,
StockRepositoryInterface $stockRepository
) {
parent::__construct($context);
$this->stockRepository = $stockRepository;
}

/**
* @inheritdoc
*/
public function execute()
{
$stockId = $this->getRequest()->getParam(StockInterface::STOCK_ID);
try {
$stock = $this->stockRepository->get($stockId);

/** @var Page $result */
$result = $this->resultFactory->create(ResultFactory::TYPE_PAGE);
$result->setActiveMenu('Magento_Inventory::stock')
->addBreadcrumb(__('Edit Stock'), __('Edit Stock'));
$result->getConfig()
->getTitle()
->prepend(__('Edit Stock: %1', $stock->getName()));
} catch (NoSuchEntityException $e) {
/** @var Redirect $result */
$result = $this->resultRedirectFactory->create();
$this->messageManager->addErrorMessage(
__('Stock with id "%1" does not exist.', $stockId)
);
$result->setPath('*/*');
}
return $result;
}
}
34 changes: 34 additions & 0 deletions app/code/Magento/Inventory/Controller/Adminhtml/Stock/Index.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Inventory\Controller\Adminhtml\Stock;

use Magento\Backend\App\Action;
use Magento\Backend\Model\View\Result\Page;
use Magento\Framework\Controller\ResultFactory;

/**
* Index Controller
*/
class Index extends Action
{
/**
* @see _isAllowed()
*/
const ADMIN_RESOURCE = 'Magento_Inventory::stock';

/**
* @inheritdoc
*/
public function execute()
{
/** @var Page $resultPage */
$resultPage = $this->resultFactory->create(ResultFactory::TYPE_PAGE);
$resultPage->setActiveMenu('Magento_Inventory::stock')
->addBreadcrumb(__('Sources'), __('List'));
$resultPage->getConfig()->getTitle()->prepend(__('Manage Stock'));
return $resultPage;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Inventory\Controller\Adminhtml\Stock;

use Magento\Backend\App\Action;
use Magento\Backend\App\Action\Context;
use Magento\Framework\Controller\Result\Json;
use Magento\Framework\Controller\ResultFactory;
use Magento\Framework\EntityManager\HydratorInterface;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong dependency, it's better to avoid dependencies on EntityiManager component, because it's deprecated one.

In this particular case - you can make hydration with
Magento\Framework\Api\DataObjectHelper::populateWithArray

Copy link

Choose a reason for hiding this comment

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

Fixed

use Magento\Framework\Exception\CouldNotSaveException;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\InventoryApi\Api\StockRepositoryInterface;
use Magento\InventoryApi\Api\Data\StockInterface;

/**
* InlineEdit Controller
Copy link
Contributor

Choose a reason for hiding this comment

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

please fix description for all controller classes

*/
class InlineEdit extends Action
{
/**
* @see _isAllowed()
*/
const ADMIN_RESOURCE = 'Magento_Inventory::stock';

/**
* @var HydratorInterface
*/
private $hydrator;

/**
* @var StockRepositoryInterface
*/
private $stockRepository;

/**
* @param Context $context
* @param HydratorInterface $hydrator
* @param StockRepositoryInterface $stockRepository
*/
public function __construct(
Context $context,
HydratorInterface $hydrator,
StockRepositoryInterface $stockRepository
) {
parent::__construct($context);
$this->hydrator = $hydrator;
$this->stockRepository = $stockRepository;
}

/**
* @inheritdoc
*/
public function execute()
{
$errorMessages = [];
$request = $this->getRequest();
$requestData = $request->getParam('items', []);

if ($request->isXmlHttpRequest() && $request->isPost() && $requestData) {
foreach ($requestData as $itemData) {
try {
$stock = $this->stockRepository->get(
$itemData[StockInterface::STOCK_ID]
);
$stock = $this->hydrator->hydrate($stock, $itemData);
$this->stockRepository->save($stock);
} catch (NoSuchEntityException $e) {
$errorMessages[] = __(
'[ID: %1] The Stock does not exist.',
$itemData[StockInterface::STOCK_ID]
);
} catch (CouldNotSaveException $e) {
$errorMessages[] =
__('[ID: %1] ', $itemData[StockInterface::STOCK_ID])
. $e->getMessage();
}
}
} else {
$errorMessages[] = __('Please correct the data sent.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Please correct the sent data.

Copy link

Choose a reason for hiding this comment

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

Fixed

}

/** @var Json $resultJson */
$resultJson = $this->resultFactory->create(ResultFactory::TYPE_JSON);
$resultJson->setData([
'messages' => $errorMessages,
'error' => count($errorMessages),
]);
return $resultJson;
}
}
Loading