Product layout based on attribute_set (PR #36244) #39718

in-session · 2025-03-07T08:56:13Z

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

m2-assistant · 2025-03-07T08:56:16Z

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:

Database Compare
Functional Tests CE
Functional Tests EE
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
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 · 2025-03-07T08:59:37Z

@magento run all tests

in-session · 2025-03-07T09:58:59Z

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 · 2025-03-07T15:15:24Z

@magento run all tests

in-session · 2025-03-09T12:06:12Z

@magento run all tests

in-session · 2025-03-09T16:05:12Z

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

wigman · 2025-03-10T08:41:45Z

app/code/Magento/Catalog/etc/config.xml

- * Copyright © Magento, Inc. All rights reserved.
- * See COPYING.txt for license details.
+ * Copyright 2025 Adobe
+ * All Rights Reserved.


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?

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

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?

@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.

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'.

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

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

in-session · 2025-03-10T09:29:55Z

@magento run all tests

in-session · 2025-03-10T19:39:00Z

@magento run all tests

in-session · 2025-03-11T06:38:49Z

@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 · 2025-03-20T15:36:27Z

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.

Update View.php

3a7cbf3

in-session added 2 commits March 7, 2025 09:57

Update system.xml

6c4d9c4

Update config.xml

ec86aac

in-session mentioned this pull request Mar 7, 2025

Product layout based on attribute_set #36244

Merged

1 task

in-session added 4 commits March 7, 2025 12:51

Merge branch '2.4-develop' into patch-12

e1d6c84

Update system.xml

345f761

Update config.xml

d6cb605

Update View.php

e31e9f9

in-session added 2 commits March 9, 2025 13:05

Update View.php

3b704a2

Merge branch '2.4-develop' into patch-12

d984a9f

in-session marked this pull request as ready for review March 9, 2025 16:06

engcom-Charlie added feature request Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Triage: Need PO Confirmation Requirements should be clarified/approved/confirmed with Product Manager. Not ready for fix/delivery labels Mar 10, 2025

wigman reviewed Mar 10, 2025

View reviewed changes

in-session added 7 commits March 10, 2025 10:16

Update View.php

fa7bca7

Update system.xml

ee56fdf

Update config.xml

2948f2f

Update View.php

400050a

Update View.php

e9ca03a

Update system.xml

21416cb

Update config.xml

af52faf

Update config.xml

bf88c74

in-session added 3 commits March 10, 2025 20:35

Update system.xml

a70f9d9

Update View.php

61eb4e2

Update View.php

7e03737

engcom-Charlie self-assigned this Mar 13, 2025

Priyakshic added the Project: Community Picked PRs upvoted by the community label Mar 13, 2025

engcom-Hotel requested a review from hostep March 20, 2025 11:14

hostep approved these changes Mar 20, 2025

View reviewed changes

engcom-Hotel added the Progress: on hold label Mar 20, 2025

hostep mentioned this pull request Mar 21, 2025

Redis / AOF - Magento 2.4.6-p3 #38148

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Product layout based on attribute_set (PR #36244) #39718

Product layout based on attribute_set (PR #36244) #39718

in-session commented Mar 7, 2025 •

edited

Loading

m2-assistant bot commented Mar 7, 2025

in-session commented Mar 7, 2025

in-session commented Mar 7, 2025 •

edited

Loading

in-session commented Mar 7, 2025

in-session commented Mar 9, 2025

in-session commented Mar 9, 2025

wigman Mar 10, 2025

in-session Mar 10, 2025

hostep Mar 10, 2025

wigman Mar 11, 2025

hostep Mar 11, 2025 •

edited

Loading

wigman Mar 11, 2025

hostep Mar 11, 2025

in-session commented Mar 10, 2025

in-session commented Mar 10, 2025

in-session commented Mar 11, 2025

engcom-Hotel commented Mar 20, 2025

Product layout based on attribute_set (PR #36244) #39718

Are you sure you want to change the base?

Product layout based on attribute_set (PR #36244) #39718

Conversation

in-session commented Mar 7, 2025 • edited Loading

m2-assistant bot commented Mar 7, 2025

in-session commented Mar 7, 2025

in-session commented Mar 7, 2025 • edited Loading

in-session commented Mar 7, 2025

in-session commented Mar 9, 2025

in-session commented Mar 9, 2025

wigman Mar 10, 2025

Choose a reason for hiding this comment

in-session Mar 10, 2025

Choose a reason for hiding this comment

hostep Mar 10, 2025

Choose a reason for hiding this comment

wigman Mar 11, 2025

Choose a reason for hiding this comment

hostep Mar 11, 2025 • edited Loading

Choose a reason for hiding this comment

wigman Mar 11, 2025

Choose a reason for hiding this comment

hostep Mar 11, 2025

Choose a reason for hiding this comment

in-session commented Mar 10, 2025

in-session commented Mar 10, 2025

in-session commented Mar 11, 2025

engcom-Hotel commented Mar 20, 2025

in-session commented Mar 7, 2025 •

edited

Loading

in-session commented Mar 7, 2025 •

edited

Loading

hostep Mar 11, 2025 •

edited

Loading