search

magento / adobe-stock-integration

Magento Adobe Stock integration Community Project.
Open Software License 3.0
60 stars 92 forks source link

Search for 0 - doesn't work #100

Closed TomashKhamlai closed 5 years ago

TomashKhamlai commented 5 years ago

Preconditions:

  1. php bin/magento config:set adobe_stock/integration/enabled 1
  2. php bin/magento config:set adobe_stock/integration/api_key \<key>
  3. php bin/magento config:set cms/wysiwyg/enabled disabled
  4. php bin/magento config:set admin/security/admin_account_sharing 1
  5. php bin/magento config:set admin/security/use_form_key 0
  6. php bin/magento cache:flush

Steps to reproduce:

  1. Navigate to Content > Elements > Pages
  2. Click Add New Page button
  3. Fill Title with "Test CMS Page"
  4. Expand the "Content" section
  5. Fill Content Heading with "Test Content Heading"
  6. Click "Insert Image..." button
  7. Click "Search Adobe Stock" button
  8. Type 0 in the search input
  9. Submit search

Expected result:

:heavy_check_mark: 1. Images similar to 00, zero, O requests.

Actual result

:x: 1. Initial images.

jeysmook commented 5 years ago

Hi @TomashKhamlai @sivaschenko

I investigated this issue. I have bad news.

  1. PHP treats 0 as an empty string and as result, Magento 2 doesn't see search parameter. Look here
  2. adobe/stock-api-libphp throws an exception if you call method setWords from AdobeStock\Api\Models\SearchParameters with string '0' or 0. Look here.
sivaschenko commented 5 years ago

Hi @jaysmook thanks for investigation!

Considering the investigation results I think that the issue can be fixed by 2 pull requests introducing more strict value checks to:

  1. https://github.com/magento/magento2
  2. https://github.com/adobe/stock-api-libphp/
engcom-Golf commented 5 years ago

Related issue: magento/magento2:23424

sivaschenko commented 5 years ago

Waiting for https://github.com/adobe/stock-api-libphp/pull/7 to be merged

sidolov commented 5 years ago

@chfabbro , do you have any estimates when https://github.com/adobe/stock-api-libphp/pull/7 may be merged to the SDK? Thanks!

chfabbro commented 5 years ago

@sidolov I just added some comments. There is a typo, and I'm not sure if this is the correct fix.