Skip to content

issue #13497 - Method getUrl in Magento\Catalog\Model\Product\Attribu… #13498

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
Feb 6, 2018
Merged

issue #13497 - Method getUrl in Magento\Catalog\Model\Product\Attribu… #13498

merged 1 commit into from
Feb 6, 2018

Conversation

igortregub
Copy link

@igortregub igortregub commented Feb 5, 2018

Preconditions

  1. Magento 2.2.2

Steps to reproduce

  1. Try to get product image via method getUrl from class Magento\Catalog\Model\Product\Attribute\Frontend

Fixed Issues

  1. Method getUrl in Magento\Catalog\Model\Product\Attribute\Frontend returns image url with double slash #13497 Method getUrl in Magento\Catalog\Model\Product\Attribute\Frontend returns image url with double slash

Expected result

  1. returns image url without double slash

Actual result

  1. will be returned image with double slash
    screen shot 2018-02-05 at 12 37 04 pm 2018-02-05 13-25-55

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Feb 5, 2018

CLA assistant check
All committers have signed the CLA.

->getBaseUrl(\Magento\Framework\UrlInterface::URL_TYPE_MEDIA)
. 'catalog/product/' . $image;
->getBaseUrl(UrlInterface::URL_TYPE_MEDIA)
. 'catalog/product' . $image;
Copy link
Contributor

Choose a reason for hiding this comment

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

As $image may come from various sources there is no guarantee it contains leading slash.

Please

  1. change logic to . 'catalog/product/' . ltrim($image, '/') instead
  2. create a separate test case to assure there is no slash duplication
  3. as always, do an amend commit and force push

Copy link
Author

Choose a reason for hiding this comment

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

ok, thank you

Copy link
Contributor

Choose a reason for hiding this comment

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

@igortregub changes look good now, please squash changes into single commit and force push.

Copy link
Author

Choose a reason for hiding this comment

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

@orlangur, done

…te\Frontend returns image url with double slash
@ihor-sviziev ihor-sviziev requested a review from orlangur February 5, 2018 16:11
@orlangur orlangur added this to the February 2018 milestone Feb 6, 2018
@magento-engcom-team
Copy link
Contributor

@igortregub thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

magento-team pushed a commit that referenced this pull request Feb 6, 2018
@magento-engcom-team magento-engcom-team merged commit 220ad97 into magento:2.2-develop Feb 6, 2018
thomasdom pushed a commit to thomasdom/magento2 that referenced this pull request Feb 7, 2018
Accepted Public Pull Requests:
 - magento#13498: issue magento#13497 - Method getUrl in Magento\Catalog\Model\Product\Attribu� (by @igortregub)
 - magento#13494: Fixing of Problem with updating stock item qty and stock status (by @nuzil)
 - magento#13132: Update the Emogrifier dependency to ^2.0.0 (by @oliverklee)

Fixed store view switcher issues when changing store view on product and category pages.
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.

5 participants