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

#207 - Move Qty product import processing to new stock item importer #211

Merged
merged 14 commits into from
Nov 21, 2017

Conversation

josh-carter
Copy link
Contributor

@josh-carter josh-carter commented Nov 18, 2017

Description

Move Qty Importing to new Stock Item Importer, new extension point to allow for eventual move of product stock import logic to here.

Fixed Issues (if relevant)

#207

Outstanding Tasks

  • Add Extension Point for new Product Import functionality
  • Fix Current Integration Tests
  • Initial Integration Test for new Stock Item Importer

@josh-carter josh-carter changed the title Move Qty product import processing to new stock item importer #207 - Move Qty product import processing to new stock item importer Nov 19, 2017
@josh-carter josh-carter changed the title #207 - Move Qty product import processing to new stock item importer WIP: #207 - Move Qty product import processing to new stock item importer Nov 19, 2017
@josh-carter josh-carter changed the title WIP: #207 - Move Qty product import processing to new stock item importer #207 - Move Qty product import processing to new stock item importer Nov 21, 2017
@@ -778,6 +787,7 @@ public function __construct(
Product\TaxClassProcessor $taxClassProcessor,
\Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig,
\Magento\Catalog\Model\Product\Url $productUrl,
StockItemImporterInterface $stockItemImporter,
Copy link
Contributor

Choose a reason for hiding this comment

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

currently adding required dependency will introduce Backward Incompatible changes.

It's better to do like we usually do to preserve constructor BC :

 public function __construct(
        \\old arguments 
        StockItemImporterInterface $stockItemImporter = null //add new dependency at the end of the dependencies list
    ) {
        
        $this->stockItemImporter = $stockItemImporter ?: \Magento\Framework\App\ObjectManager::getInstance()->get(
            StockItemImporterInterface::class
        );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maghamed Understood - i've just pushed up a commit to fix this see here

- By not requiring stockItemInterface in construct
/**
* Source Item Importer
*
* @var
Copy link
Contributor

Choose a reason for hiding this comment

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

please specify data type for @var StockItemImporterInterface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot - fixed here

@maghamed maghamed merged commit 1ddb686 into develop Nov 21, 2017
@maghamed maghamed deleted the qty-processing-import-export branch December 11, 2018 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants