#9113 fix url_path attribute value is not populated #12012
Conversation
| class ProductUrlPathAutogeneratorObserver implements ObserverInterface | ||
| { | ||
| /** @var ProductUrlPathGenerator */ | ||
| protected $productUrlPathGenerator; |
There was a problem hiding this comment.
According to Magento technical guidelines
http://devdocs.magento.com/guides/v2.1/coding-standards/technical-guidelines/technical-guidelines.html
2.7. All non-public properties and methods SHOULD be private.
Could you change protected visibility to private?
|
@ihor-sviziev I have removed the observer that I created. I would say that is better to use the same observer ProductUrlKeyAutogeneratorObserver and just add a new line instead |
There was a problem hiding this comment.
@thiagolima-bm not sure that setting url_path should be added to existing class because of class name, but in general looks good.
now integration tests failed. Could you adjust them jf needed?
|
@ihor-sviziev I will take a look why the tests are failing |
| @@ -32,5 +32,6 @@ public function execute(\Magento\Framework\Event\Observer $observer) | |||
| /** @var Product $product */ | |||
| $product = $observer->getEvent()->getProduct(); | |||
| $product->setUrlKey($this->productUrlPathGenerator->getUrlKey($product)); | |||
| $product->setUrlPath($this->productUrlPathGenerator->getUrlPath($product)); | |||
There was a problem hiding this comment.
The issue mentioned using [product-suffix-from-setting] to build the path. Should this not be getUrlPathWithSuffix?
Sorry for the drive-by comment, I'm in need of a populated path, that includes the suffix.
|
@ihor-sviziev any clue where this test could be failing? |
| { | ||
| /** @var Product $product */ | ||
| $product = $observer->getEvent()->getProduct(); | ||
| $product->setUrlKey($this->productUrlPathGenerator->getUrlPath($product)); |
There was a problem hiding this comment.
- Looks like you was thinking to us $product->setUrlPath() method instead
- Would be great to add some unit or integration test there. Could you add it?
If you need any help - feel free to ask
There was a problem hiding this comment.
For sure! I need more coffee :P
|
Hi @thiagolima-bm, are you interested in finishing your PR or maybe you need some help? |
|
Hi @thiagolima-bm |
Fix url_path attribute value is not populated for any product #9113