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

Bugfix for _getProductCollection on a product page #7598

Merged
merged 2 commits into from
Mar 16, 2017

Conversation

evgk
Copy link
Member

@evgk evgk commented Nov 28, 2016

The following code will fail if $this->setCategoryId puts a category-instance instead of a category ID:

            if ($this->getCategoryId()) {
                try {
                    $category = $this->categoryRepository->get($this->getCategoryId());
                } catch (NoSuchEntityException $e) {
                    $category = null;
                }

Since category-repository object's get call can't accept the category-instance as parameter, it should be a category ID. Suggested patch fixes this issue(and method's comment).

@ihor-sviziev
Copy link
Contributor

@evgk would be great to cover this case with unit tests

@vrann
Copy link
Contributor

vrann commented Mar 15, 2017

@evgk can you please update the branch with the latest from the mainline? There is a merge conflict.

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Mar 15, 2017

CLA assistant check
All committers have signed the CLA.

@evgk evgk force-pushed the patch-1 branch 2 times, most recently from 6a259b2 to e5bfc33 Compare March 15, 2017 08:41
@vrann vrann self-assigned this Mar 15, 2017
@vrann vrann added this to the March 2017 milestone Mar 15, 2017
@magento-team magento-team merged commit 877c9c7 into magento:develop Mar 16, 2017
@okorshenko
Copy link
Contributor

@evgk thank you for your contribution. Your Pull Request successfully merged to develop branch

@orlangur orlangur mentioned this pull request Oct 18, 2017
4 tasks
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.

None yet

7 participants