Skip to content

Respect Category Top Navigation Max Depth setting #4

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

Merged
merged 1 commit into from
Dec 11, 2017
Merged

Respect Category Top Navigation Max Depth setting #4

merged 1 commit into from
Dec 11, 2017

Conversation

arnoudhgz
Copy link

Description

The setting Stores > Configuration > Catalog > Catalog > Category Top Navigation > Maximal Depth was actually never being used to limit the top menu maximum depth.

Now the setting will be used as a condition for the category collection when it is fetched.

Fixed Issues

  1. Category Top Navigation / Maximal Depth configuration not working magento/magento2#7543: Category Top Navigation / Maximal Depth configuration not working

Manual testing scenarios

  1. Change the setting in the backend
  2. Refresh the invalidated cache
  3. See that the setting actually is being used

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)

@arnoudhgz
Copy link
Author

arnoudhgz commented Dec 10, 2017

@ksangers and @lenlorijn the original PR to Magento was this one: magento#12528

It received feedback and now I want to create a PR again, but now on the 2.3-develop branch, because it has a new public method and a new (not backwards compatible) dependency injection.

The setting `Stores > Configuration > Catalog > Catalog > Category Top Navigation > Maximal Depth` was actually never being used to limit the topmenu.

Now the setting will be used as a condition for the category collection when it is fetched.

/**
* Category resource collection
*
* @api
* @author Magento Core Team <[email protected]>
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)

Choose a reason for hiding this comment

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

How come SuppressWarnings wasn't necessary before, but is now?

Copy link
Author

Choose a reason for hiding this comment

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

Because this class had no DI on its own before, it relied on its parent __construct. Since I added one dependency that is not in its parent I had to add the complete __construct what triggered the PHPMD to fail on Travis-CI.

@lenlorijn lenlorijn merged commit 0a6ac4c into mediact:bugfix/MAGETWO-61422-category-max-depth-configuration Dec 11, 2017
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.

3 participants