Product layout based on attribute_set (PR #36244)

[in-session]

follow up:
#36244 (comment)

Related:
#102 (comment)
https://github.com/Vendic/module-optimize-cache-size/tree/main
#189 (comment)

[m2-assistant]

Hi @in-session. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

[in-session]

@magento run all tests

[in-session]

Static test will be fixed after test run:

<!--
/**
 * Copyright 2014 Adobe
 * All Rights Reserved.
 */
-->

PHP Code Sniffer detected 1 violation(s): FILE: /var/www/html/app/code/Magento/Catalog/etc/adminhtml/system.xml -------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------- | WARNING | Copyright is missing or Copyright content/year is not valid -------------------------------------------------------------------------- FILE: /var/www/html/app/code/Magento/Catalog/etc/config.xml -------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------- | WARNING | Copyright is missing or Copyright content/year is not valid -------------------------------------------------------------------------- Failed asserting that 1 matches expected 0.

1
PHP Code Sniffer detected 2 violation(s): FILE: /var/www/html/app/code/Magento/Catalog/Helper/Product/View.php -------------------------------------------------------------------------- FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES -------------------------------------------------------------------------- 179 | WARNING | [x] Whitespace found at end of line 184 | WARNING | [x] Whitespace found at end of line 193 | WARNING | [ ] Line exceeds 120 characters; contains 122 characters 213 | WARNING | [ ] Line exceeds 120 characters; contains 122 characters -------------------------------------------------------------------------- PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------- Failed asserting that 2 matches expected 0.

Semantic Version Checker because of XML changes:

System changes (MINOR) | FailureDetailsLevelTarget/LocationCode/ReasonMINORdev/layout_settings/enable_id_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0M302 A field-node was addedMINORdev/layout_settings/enable_attribute_set_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0M302 A field-node was added | Level | Target/Location | Code/Reason | MINOR | dev/layout_settings/enable_id_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0 | M302 A field-node was added | MINOR | dev/layout_settings/enable_attribute_set_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0 | M302 A field-node was added -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | --

MINOR | dev/layout_settings/enable_id_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0 | M302 A field-node was added
MINOR | dev/layout_settings/enable_attribute_set_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0 | M302 A field-node was added

[in-session]

@magento run all tests

[in-session]

@magento run all tests

[in-session]

@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE

[wigman]

These copyrights are incorrect and were reverted previously. They are gone in 2.4.8-beta and 2.4.8-develop, but still present in 2.4.7-develop (on which this PR is branched).
Maybe @sidolov you can double check whether this propietary copyright notice does not end up in an actual release?

[in-session]

Yes, it would be nice if there was a clear statement about which copyright to use. My last status was the changes in the PR.
2.4.8-beta2: https://github.com/magento/magento2/blob/2.4.8-beta2/app/code/Magento/Catalog/etc/di.xml#L2

[hostep]

Correct format should be:

/**
 * Copyright [first year code created] Adobe
 * All rights reserved.
 */

Source: https://developer.adobe.com/commerce/contributor/guides/code-contributions/backward-compatibility-policy/#adding-a-constructor-parameter

So the year 2025 should be replaced with 2013 over here.

Same remark around the other files changed in this PR, please don't use the year 2025 unless it's a new file.

@wigman: is this what you were after? Or did I misunderstood?

[wigman]

@hostep no, that was not wat I was after :) the copyright notice is not supposed to change to "all rights reserved", unless the decision to roll that change back has been reversed, and I need to check with Adobe core team if that can be prevented.

[hostep]

So it changes from * Copyright © Magento, Inc. All rights reserved. to Copyright 2013 Adobe * All Rights Reserved.

All rights reserved has always been there as far as I know (example from 10 years ago). It's just that it was previously 'Magento', then 'Magento, Inc', which may no longer exist, and is now 'Adobe'.

[wigman]

Ah, you are completely right. I misread. It thought it was a partial return of these changes, which were quite severe 😅 24ce222 thanks!

[hostep]

No worries. You may be interested in magento/magento-coding-standard#492 then.

[in-session]

@magento run all tests

[in-session]

@engcom-Charlie PR is ready for review.

• Functional tests are not related to the PR
• Semantic Version Checker due to core XML changes.

[engcom-Hotel]

Hello @in-session,

Thank you for your contribution!

As this PR is marked as a feature request, we require PO approval to proceed. We have initiated an internal discussion regarding this PR and will be placing it on hold for the time being.

[in-session]

@magento run all tests

[engcom-Hotel]

@magento run all tests

[engcom-Hotel]

Hello @in-session,

We have received approval on this PR from the PO. Therefore, we will proceed with further activities related to it.

Thanks

[engcom-Hotel]

Hello @in-session,

Thank you for the contribution!

To ensure the new configuration options (enable_id_handle and enable_attribute_set_handle) work as intended and to prevent regressions, I suggest adding functional tests for the following scenarios:

1. When each flag is enabled and disabled.
2. Verifying that the correct layout handles are added or skipped based on the configuration.

These tests will enhance reliability, ensure expected behaviour across use cases, and align with Magento's high-quality standards. Let me know your thoughts on this.

Thanks