Skip to content

#9768: Admin dashboard Most Viewed Products Tab only gives default attribute set's products #11725

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
Nov 8, 2017

Conversation

RomaKis
Copy link
Contributor

@RomaKis RomaKis commented Oct 25, 2017

Admin dashboard Most Viewed Products Tab only gives default attribute set's products

Fixed Issues (if relevant)

  1. Admin dashboard Most Viewed Products Tab only gives default attribute set's products #9768: Admin dashboard Most Viewed Products Tab only gives default attribute set's products

Manual testing scenarios

  1. Login to Customer Account
  2. Visited Product "Minerva LumaTech™ V-Tee"
  3. Login to magento admin panel
  4. Check magento dashboard "Most Viewed Product" grid

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)

@orlangur
Copy link
Contributor

@miguelbalparda please process this PR together with #11597. Implementation is identical, but this PR has a test 💪

@miguelbalparda miguelbalparda self-assigned this Oct 25, 2017
@miguelbalparda
Copy link
Contributor

@RomaKis any chance you can take a look at the failing tests? If you need any help just let us know. Thanks!

@miguelbalparda miguelbalparda added this to the October 2017 milestone Oct 26, 2017
@miguelbalparda miguelbalparda added Release Line: 2.3 2.2.x bug report Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Oct 26, 2017
@RomaKis
Copy link
Contributor Author

RomaKis commented Oct 26, 2017

@miguelbalparda, yes of course. I fixed it. Hope Jenkins won't fail again.

* Test for Magento\Reports\Model\ResourceModel\Product\Collection.
*
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
Copy link
Contributor

Choose a reason for hiding this comment

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

While coupling is unavoidable you cannot suppress ExcessiveMethodLength in new code, please refactor test accordingly.

Use imports actively (Alt+Enter in PhpStorm) to fit method calls in one line, getMockForAbstractClass for interface could be replaced with createMock.

After that please squash changes into single commit and force push into the same branch.

@RomaKis
Copy link
Contributor Author

RomaKis commented Oct 27, 2017

@orlangur, fixed "SuppressWarnings(PHPMD.ExcessiveMethodLength)" and squashed commits.
I cann't use createMock()(1) or createPartialMock()(2) instead of getMockForAbstractClass() for interfaces:

  1. I haven't ability to setMethods().
  2. I need to configure all methods which exists in interface, but i don't need all of them.

@orlangur
Copy link
Contributor

@RomaKis ok, I remember I was using getMock for interfaces but these new methods seem to be different.

@miguelbalparda
Copy link
Contributor

This is good to go and going to be merged soon, can you please backport these changes to 2.1 and 2.0 please?

@okorshenko okorshenko merged commit cd821b4 into magento:2.3-develop Nov 8, 2017
okorshenko pushed a commit that referenced this pull request Nov 8, 2017
@RomaKis
Copy link
Contributor Author

RomaKis commented Nov 9, 2017

@miguelbalparda, yes, I'll do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report bugfix Progress: accept Release Line: 2.3 Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants