magento2 icon indicating copy to clipboard operation
magento2 copied to clipboard

Product layout based on attribute_set (PR #36244)

Open in-session opened this issue 8 months ago • 16 comments

follow up: https://github.com/magento/magento2/pull/36244#issuecomment-2705671813

Related: https://github.com/magento/magento2/issues/102#issuecomment-826058850 https://github.com/Vendic/module-optimize-cache-size/tree/main https://github.com/magento/magento2/issues/189#issuecomment-698430804

in-session avatar Mar 07 '25 08:03 in-session

Hi @in-session. Thank you for your contribution! Here are some useful tips on how you can test your changes using Magento test environment. :exclamation: 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 :information_source: 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.

m2-assistant[bot] avatar Mar 07 '25 08:03 m2-assistant[bot]

@magento run all tests

in-session avatar Mar 07 '25 08:03 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 avatar Mar 07 '25 09:03 in-session

@magento run all tests

in-session avatar Mar 07 '25 15:03 in-session

@magento run all tests

in-session avatar Mar 09 '25 12:03 in-session

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

in-session avatar Mar 09 '25 16:03 in-session

@magento run all tests

in-session avatar Mar 10 '25 09:03 in-session

@magento run all tests

in-session avatar Mar 10 '25 19:03 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.

in-session avatar Mar 11 '25 06:03 in-session

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.

engcom-Hotel avatar Mar 20 '25 15:03 engcom-Hotel

@magento run all tests

in-session avatar Apr 17 '25 15:04 in-session

@magento run all tests

engcom-Hotel avatar Apr 22 '25 05:04 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 avatar Apr 22 '25 05:04 engcom-Hotel

@magento run all tests

engcom-Charlie avatar May 20 '25 12:05 engcom-Charlie

@magento run all tests

engcom-Charlie avatar May 23 '25 13:05 engcom-Charlie

@magento run all tests

in-session avatar Jun 12 '25 17:06 in-session

Hello @in-session,

Have you got a chance to look into the review comments here?

Thanks

engcom-Hotel avatar Jun 23 '25 07:06 engcom-Hotel

@engcom-Hotel

Unfortunately, I'm not really familiar with the tests. Do you mean something like: dev/tests/integration/testsuite/Magento/Catalog/Helper/Product/ViewLayoutHandleTest.php

in-session avatar Jun 23 '25 08:06 in-session

Hello @in-session,

Thank you for implementing these tests - they look good! To ensure complete test coverage, could you please add two additional test methods that verify the mixed configuration states:

  1. When only ID handle is enabled (enable_id_handle=1, enable_attribute_set_handle=0)
  2. When only attribute set handle is enabled (enable_id_handle=0, enable_attribute_set_handle=1)

This will ensure all possible configuration combinations are verified.

Thanks

engcom-Hotel avatar Jun 24 '25 12:06 engcom-Hotel

@engcom-Hotel I hope that's OK. Please confirm again.

in-session avatar Jun 24 '25 14:06 in-session

@magento run all tests

in-session avatar Jun 24 '25 14:06 in-session

@magento run all tests

engcom-Hotel avatar Jun 25 '25 06:06 engcom-Hotel

@magento run all tests

in-session avatar Jun 26 '25 04:06 in-session

@magento run all tests

engcom-Charlie avatar Jul 08 '25 05:07 engcom-Charlie

@magento run all tests

engcom-Charlie avatar Jul 09 '25 05:07 engcom-Charlie

@magento run Functional Tests B2B, Static Tests

engcom-Charlie avatar Jul 10 '25 04:07 engcom-Charlie

@magento run all tests

in-session avatar Jul 10 '25 06:07 in-session

@magento run all tests

in-session avatar Jul 10 '25 07:07 in-session

@magento run Functional Tests B2B, Functional Tests EE

engcom-Charlie avatar Jul 10 '25 10:07 engcom-Charlie

@magento run Functional Tests B2B

engcom-Charlie avatar Jul 11 '25 07:07 engcom-Charlie