Skip to content

Conversation

Nazar65
Copy link
Member

@Nazar65 Nazar65 commented Sep 27, 2019

Description (*)

By default premium filter is set to All in \Magento\AdobeStockClient\Model\SearchParametersProvider\Premium::apply

Instead of setting the default filter in Client, it now sets by adding the filter to Magento\AdobeStockImage\Model\GetImageList in AdobeStockImage/etc/di.xml

Fixed Issues (if relevant)

  1. Move default premium filter to di.xml configuration #471: Move default premium filter to di.xml configuration
  2. ...

Manual testing scenarios (*)

  1. Configure Adobe Stock integration
  2. Open Adobe Stock search
  3. Verify premium filter all applied
  4. Chose in grid filters filter premium
  5. Verify filter applied.

Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @Nazar65 ! Please see my code review comments

<item name="premium_filter" xsi:type="array">
<item name="type" xsi:type="string">premium_price_filter</item>
<item name="condition" xsi:type="string">eq</item>
<item name="field" xsi:type="string">all</item>
Copy link
Member

Choose a reason for hiding this comment

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

I think it's worth to try to use Magento\AdobeStockClient\Model\SearchParametersProvider\SimpleFilters for Premium filter. It should be possible after accepting the following suggestion:

Suggested change
<item name="field" xsi:type="string">all</item>
<item name="field" xsi:type="string">ALL</item>

@Nazar65
Copy link
Member Author

Nazar65 commented Sep 27, 2019

@sivaschenko done ✔️

@sivaschenko sivaschenko merged commit 286af17 into magento:develop Oct 1, 2019
@ghost
Copy link

ghost commented Oct 1, 2019

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

2 participants