Skip to content
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

Do not hardcode product link types #9600

Merged
merged 9 commits into from
Jun 1, 2017
Merged

Conversation

kassner
Copy link
Contributor

@kassner kassner commented May 11, 2017

When creating a new relation type, it is not possible to save it from the product edit form on admin, since Magento\Catalog\Controller\Adminhtml\Product\Initialization\Helper only iterates over the original link types (related, crosssell, upsell) because they are hardcoded.

@fooman fooman self-assigned this May 12, 2017
@fooman fooman added this to the May 2017 milestone May 12, 2017
@fooman fooman added the develop label May 12, 2017
@fooman
Copy link
Contributor

fooman commented May 12, 2017

Hey @kassner - thanks for the pull request. Makes sense.

Can you please take a look at the Travis failures along with my comment on the constructor.

@@ -90,14 +95,16 @@ public function __construct(
StockDataFilter $stockFilter,
\Magento\Catalog\Model\Product\Initialization\Helper\ProductLinks $productLinks,
\Magento\Backend\Helper\Js $jsHelper,
\Magento\Framework\Stdlib\DateTime\Filter\Date $dateFilter
\Magento\Framework\Stdlib\DateTime\Filter\Date $dateFilter,
\Magento\Catalog\Model\Product\LinkTypeProvider $linkTypeProvider
Copy link
Contributor

Choose a reason for hiding this comment

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

To introduce a new constructor argument as backwards compatible as possible can you please follow this pattern used here: https://github.com/magento/magento2/pull/9261/files#diff-e4882753f27f1777c744e441e2b4f9e2R103

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to be a bit counter intuitive to use ObjectManager here when "the good practices" tell you to avoid touching it, specially for a class that is not part of the @api.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed - but I guess it's currently better to be able to move forward without breaking things until a deprecation strategy is adopted.

@fooman
Copy link
Contributor

fooman commented May 12, 2017

If you could apply the changes you added to #9601 to here that be great, as develop gets processed before the backport.

kassner added a commit to kassner/magento2 that referenced this pull request May 16, 2017
kassner added a commit to kassner/magento2 that referenced this pull request May 29, 2017
kassner added a commit to kassner/magento2 that referenced this pull request May 29, 2017
@okorshenko okorshenko modified the milestones: May 2017, June 2017 Jun 1, 2017
@magento-team magento-team merged commit f3a855c into magento:develop Jun 1, 2017
magento-team pushed a commit that referenced this pull request Jun 1, 2017
[EngCom] Public Pull Requests
 - MAGETWO-69573: Adding logo in media folder #9797
 - MAGETWO-69555: Allow for referenceBlock to include template argument #9772
 - MAGETWO-69540: Fix for #5897: getIdentities relies on uninitialized collection #9777
 - MAGETWO-69533: [BUGFIX][6244] Fix Issue with code label display in cart checkout. #9721
 - MAGETWO-69499: Update select.js #9475
 - MAGETWO-69451: Replace Zend_Json in the configurable product block test #9753
 - MAGETWO-69373: Customer with unique attribute can't be saved #7844 #9712
 - MAGETWO-69369: Replace the direct usage of Zend_Json with a call to the Json Help class #9344
 - MAGETWO-69085: Do not hardcode product link types #9600
 - MAGETWO-69554: Patch to allow multiple filter_url_params to function #9723
@magento-team
Copy link
Contributor

@kassner thank you for your contribution to Magento 2 project

@kassner kassner deleted the patch-5 branch June 1, 2023 15:01
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.

4 participants