Skip to content

261 alert functionality#366

Closed
chrom wants to merge 5 commits intomagento:261_alert_functionalityfrom
chrom:261_alert_functionality
Closed

261 alert functionality#366
chrom wants to merge 5 commits intomagento:261_alert_functionalityfrom
chrom:261_alert_functionality

Conversation

@chrom
Copy link
Copy Markdown
Contributor

@chrom chrom commented Dec 31, 2017

No description provided.

* @var array
*/
protected $_websites;
private $websites;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is legacy class, we need to follow backward compatibility
So need to revert all changes in this class
We change the only if ($this->productIsSalable->isSalable condition

*/
declare(strict_types=1);

namespace Magento\InventorySalesAlert\Api;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

More related name is InventoryProductAlert (now we have ProductAlert module but not SalesAlert)
NAmespace should be Magento\ProductAlert\Model\...

*
* @api
*/
interface ProductIsSalableInterface
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This interface should be placed in current ProductAlert module and should be part of current implementation
ProductAlert module must work without new InventoryProductAlert

* @param \Magento\InventoryApi\Api\IsProductInStockInterface $stockItem
*/
public function __construct(
StockResolver $stockResolver = null,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It is new code, so parameters should be not null (without '= null')

StockResolver $stockResolver = null,
IsProductInStockInterface $stockItem = null
) {
$this->stockItem = $stockItem ?: ObjectManager::getInstance()->get(IsProductInStock::class);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This trick is needed only for backward compatibility, but i this case it is new code

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_InventorySalesAlert" setup_version="0.1.0">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

use 1.0.0 version

/**
* Is product salable
*
* Used fully qualified namespaces in annotations for proper work of WebApi request parser
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove this comments because this interface is not part of Web-API

{
/**
* @param ProductInterface $product
* @param string $websiteCode
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pls use Magento code style for formatting

*/
public function isSalable(
ProductInterface $product,
string $websiteCode,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The second parameter should be only $website
This is related to current implementation

*/
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
<preference for="Magento\InventorySalesAlert\Api\ProductIsSalableInterface" type="Magento\InventorySalesAlert\Model\ProductIsSalable"/>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Default implementation (related to current code with calling $product->isSalable()) of ProductIsSalableInterface is missed

@naydav naydav mentioned this pull request Jan 2, 2018
@naydav
Copy link
Copy Markdown

naydav commented Jan 11, 2018

We try to resolve this issue in more global way

Implementation will be finished in
#387
#388

@naydav naydav closed this Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants