Skip to content
This repository has been archived by the owner on Apr 29, 2019. It is now read-only.

Add missed $searchCriteriaSearch property #147

Conversation

thicolares
Copy link

@thicolares thicolares commented Oct 28, 2018

Description (*)

  • Add missed $searchCriteriaSearch property
  • Analyse against phpstan returned ""[OK] No errors"

Fixed Issues (if relevant)

  1. Clean up Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/TierPriceTest #127: Clean up Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/TierPriceTest

Manual testing scenarios (*)

  1. Run ./vendor/bin/phpstan analyse -l 0 app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/TierPriceTest.php

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)

- Analyse against phpstan returned ""[OK] No errors"
@dmanners
Copy link
Contributor

Hello, thank you for your pull request. I will start to process this PR and get back to you if I need any more information.

@thicolares
Copy link
Author

thicolares commented Oct 29, 2018

Ok! I am glad to help

@@ -19,6 +19,9 @@ class TierPriceTest extends \PHPUnit\Framework\TestCase
/** @var \Magento\Customer\Api\GroupRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject */
protected $groupRepositoryInterface;

/** @var \Magento\Framework\Api\SearchCriteria|\PHPUnit_Framework_MockObject_MockObject */
protected $searchCriteriaSearch;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think looking at it's usage we do not need this as a call variable but could make it local to the method only. Would you be up for making this change and then validating the phpstan and phpunit test again?

Copy link
Author

@thicolares thicolares Nov 1, 2018

Choose a reason for hiding this comment

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

Sure! Besides $searchCriteriaSearch, I believe I should do the same for $objectManagerHelper and $searchCriteriaBuilder -- both are also only local and not instantiated via dependency injection.

@magento-engcom-team
Copy link
Contributor

@thicolares thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@dmanners
Copy link
Contributor

dmanners commented Apr 5, 2019

Thank you for your effort here. As we are focusing import work on the async-import project we will be closing this project for now.

@dmanners dmanners closed this Apr 5, 2019
@ghost
Copy link

ghost commented Apr 5, 2019

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants