AmpersandHQ / ampersand-magento2-upgrade-patch-helper

Helper script to aid upgrading magento 2 websites by detecting overrides. Now supports third party module detections
GNU Lesser General Public License v3.0
322 stars 39 forks source link

Report redundant overrides, and ignore non-meaningful changes in `vendor` #110

Closed convenient closed 1 year ago

convenient commented 1 year ago

This PR attempts to solve the following, both of these combined should allow you to do less work, and the work you do have to do should be more targeted

Both of those issues require some understanding of what is "meaningful", so the start of that is stripping of whitespace/comments/multi newlines.

With the ability to understand what the actual change made in vendor looks like, we can hopefully ignore a lot of redundant checks. And in the event that the overridden file is now redundant itself, report that.

In this PR I have scaffolded the rough setup of how I think this can work, as well as some test cases for .html template overrides. I will link subsequent PRs pointing to this branch as I implement each check type just to try and keep it easier to review and grok what i'm doing. For example of each type and what can be ignored or flagged as redundant click through

I have added a new type called IGNR which is visible with --show-ignore, this is because I don't want to risk binning off anything that was previously highlighted and this way we can still investigate just in case there is a bug or something.

I see three types of cases

Case 1 - Current base functionality

| WARN  | Override (phtml/js/html) | vendor/magento/module-catalog/view/frontend/layout/catalog_category_view.xml                                                       | app/design/frontend/Ampersand/theme/Magento_Catalog/layout/catalog_category_view.xml                                               |

Case 2 - A NOOP change (comment/whitespace/etc)

| IGNR  | Override (phtml/js/html) | vendor/magento/module-catalog/view/frontend/layout/catalog_category_view_type_default.xml                                          | app/design/frontend/Ampersand/theme/Magento_Catalog/layout/catalog_category_view_type_default.xml                                  

Case 3 - Changed to match the override, maybe the override was backported but no longer necessary

| WARN  | Redundant Override       | vendor/magento/theme-frontend-blank/etc/view.xml                                                                                   | vendor/ampersand/upgrade-patch-helper-test-hyva-fallback-theme/theme/etc/view.xml                                                  |

Updated documentation can be seen

There's a lot of moving pieces and a bunch of test code required here which makes this PR quite chunky at 93, but looking at results may be the best way to get a sense of what is achieved here.

diff --git a/dev/phpunit/functional/expected_output/magentom24-show-info.out.txt b/dev/phpunit/functional/expected_output/magentom24-show-info.out.txt
index 3714f5dc..94db0e1f 100644
--- a/dev/phpunit/functional/expected_output/magentom24-show-info.out.txt
+++ b/dev/phpunit/functional/expected_output/magentom24-show-info.out.txt
@@ -32,13 +32,10 @@
 | WARN  | Override (phtml/js/html) | vendor/magento/module-sales/view/frontend/layout/sales_order_print.xml                                                             | app/design/frontend/Ampersand/theme/Magento_Sales/layout/sales_order_print.xml                                                     |
 | WARN  | Override (phtml/js/html) | vendor/magento/module-ui/view/base/web/templates/block-loader.html                                                                 | app/design/frontend/Ampersand/theme/Magento_Ui/web/templates/block-loader.html                                                     |
 | WARN  | Override (phtml/js/html) | vendor/magento/module-ui/view/base/web/templates/grid/masonry.html                                                                 | app/design/frontend/Ampersand/theme/Magento_Ui/web/templates/grid/masonry.html                                                     |
-| WARN  | Override (phtml/js/html) | vendor/magento/module-vault/view/frontend/web/js/view/payment/method-renderer/vault.js                                             | app/design/frontend/Ampersand/theme/Magento_Vault/web/js/view/payment/method-renderer/vault.js                                     |
-| WARN  | Override (phtml/js/html) | vendor/magento/theme-frontend-blank/etc/view.xml                                                                                   | vendor/ampersand/upgrade-patch-helper-test-hyva-fallback-theme/theme/etc/view.xml                                                  |
 | WARN  | Override (phtml/js/html) | vendor/magento/theme-frontend-blank/etc/view.xml                                                                                   | vendor/ampersand/upgrade-patch-helper-test-module/src/theme/etc/view.xml                                                           |
 | WARN  | Override (phtml/js/html) | vendor/magento/theme-frontend-luma/Magento_Sales/email/order_new.html                                                              | vendor/ampersand/upgrade-patch-helper-test-module/src/theme/Magento_Sales/email/order_new.html                                     |
 | WARN  | Override (phtml/js/html) | vendor/magento/theme-frontend-luma/etc/view.xml                                                                                    | vendor/ampersand/upgrade-patch-helper-test-hyva-fallback-theme/theme/etc/view.xml                                                  |
 | WARN  | Override (phtml/js/html) | vendor/magento/theme-frontend-luma/etc/view.xml                                                                                    | vendor/ampersand/upgrade-patch-helper-test-module/src/theme/etc/view.xml                                                           |
-| WARN  | Override (phtml/js/html) | vendor/paypal/module-braintree-core/view/base/web/js/form-builder.js                                                               | vendor/paypal/module-braintree-core/view/frontend/web/js/form-builder.js                                                           |
 | WARN  | Plugin                   | vendor/magento/framework/Stdlib/Cookie/PhpCookieManager.php                                                                        | Ampersand\Test\Plugin\PhpCookieManager::beforeSetPublicCookie                                                                      |
 | WARN  | Plugin                   | vendor/magento/module-adobe-ims/Model/UserProfile.php                                                                              | Ampersand\Test\Plugin\AdobeImsUserProfile::afterGetUpdatedAt                                                                       |
 | WARN  | Plugin                   | vendor/magento/module-adobe-ims/Model/UserProfile.php                                                                              | Ampersand\Test\Plugin\AdobeImsUserProfile::aroundGetUpdatedAt                                                                      |
@@ -59,6 +56,12 @@
 | WARN  | Preference               | vendor/magento/module-weee/Model/Total/Quote/Weee.php                                                                              | Ampersand\Test\Model\WebApiSoap\Total\Quote\Weee                                                                                   |
 | WARN  | Preference Removed       | vendor/ampersand/upgrade-patch-helper-test-module/src/module/Model/ToPreferenceAndDelete.php                                       | Ampersand\Test\Model\ToPreferenceAndDelete                                                                                         |
 | WARN  | Preference Removed       | vendor/ampersand/upgrade-patch-helper-test-module/src/module/Model/ToPreferenceAndExtendAndDelete.php                              | Ampersand\Test\Model\ToPreferenceAndExtendAndDelete                                                                                |
+| WARN  | Redundant Override       | vendor/ampersand/upgrade-patch-helper-test-hyva-theme-stub/theme/Magento_Checkout/templates/cart/redundant.phtml                   | vendor/ampersand/upgrade-patch-helper-test-hyva-theme-extended/theme/Magento_Checkout/templates/cart/redundant.phtml               |
+| WARN  | Redundant Override       | vendor/ampersand/upgrade-patch-helper-test-module/src/module/view/frontend/email/redundant.html                                    | app/design/frontend/Ampersand/theme/Ampersand_TestVendor/email/redundant.html                                                      |
+| WARN  | Redundant Override       | vendor/ampersand/upgrade-patch-helper-test-module/src/theme/Magento_Checkout/web/js/redundant.js                                   | app/design/frontend/Ampersand/theme/Magento_Checkout/web/js/redundant.js                                                           |
+| WARN  | Redundant Override       | vendor/ampersand/upgrade-patch-helper-test-module/src/theme/Magento_Ui/web/templates/redundant.html                                | app/design/frontend/Ampersand/theme/Magento_Ui/web/templates/redundant.html                                                        |
+| WARN  | Redundant Override       | vendor/magento/theme-frontend-blank/etc/view.xml                                                                                   | vendor/ampersand/upgrade-patch-helper-test-hyva-fallback-theme/theme/etc/view.xml                                                  |
+| WARN  | Redundant Override       | vendor/paypal/module-braintree-core/view/base/web/js/form-builder.js                                                               | vendor/paypal/module-braintree-core/view/frontend/web/js/form-builder.js                                                           |
 | INFO  | DB schema added          | vendor/magento/module-admin-adobe-ims/etc/db_schema.xml                                                                            | admin_adobe_ims_webapi                                                                                                             |
 | INFO  | DB schema added          | vendor/magento/module-catalog/etc/db_schema.xml                                                                                    | catalog_compare_list                                                                                                               |
 | INFO  | DB schema added          | vendor/magento/module-customer/etc/db_schema.xml                                                                                   | customer_group_excluded_website                                                                                                    |
@@ -454,7 +457,15 @@
 | INFO  | Setup Patch Data         | vendor/ampersand/upgrade-patch-helper-test-module/src/module/Setup/Patch/Data/SomeDataChanges.php                                  | Ampersand\TestVendor\Setup\Patch\Data\SomeDataChanges                                                                              |
 | INFO  | Setup Patch Schema       | vendor/ampersand/upgrade-patch-helper-test-module/src/module/Setup/Patch/Schema/SomeSchemaChanges.php                              | Ampersand\TestVendor\Setup\Patch\Schema\SomeSchemaChanges                                                                          |
 | INFO  | Setup Script             | vendor/ampersand/upgrade-patch-helper-test-module/src/module/Setup/InstallSchema.php                                               | Ampersand\TestVendor\Setup\InstallSchema                                                                                           |
+| IGNR  | Override (phtml/js/html) | vendor/ampersand/upgrade-patch-helper-test-hyva-theme-stub/theme/Magento_Checkout/templates/cart/ignored.phtml                     | vendor/ampersand/upgrade-patch-helper-test-hyva-theme-extended/theme/Magento_Checkout/templates/cart/ignored.phtml                 |
+| IGNR  | Override (phtml/js/html) | vendor/ampersand/upgrade-patch-helper-test-module/src/module/view/frontend/email/ignore.html                                       | app/design/frontend/Ampersand/theme/Ampersand_TestVendor/email/ignore.html                                                         |
+| IGNR  | Override (phtml/js/html) | vendor/ampersand/upgrade-patch-helper-test-module/src/theme/Magento_Checkout/web/js/ignore.js                                      | app/design/frontend/Ampersand/theme/Magento_Checkout/web/js/ignore.js                                                              |
+| IGNR  | Override (phtml/js/html) | vendor/ampersand/upgrade-patch-helper-test-module/src/theme/Magento_Ui/web/templates/ignore.html                                   | app/design/frontend/Ampersand/theme/Magento_Ui/web/templates/ignore.html                                                           |
+| IGNR  | Override (phtml/js/html) | vendor/magento/module-catalog/view/frontend/layout/catalog_category_view_type_default.xml                                          | app/design/frontend/Ampersand/theme/Magento_Catalog/layout/catalog_category_view_type_default.xml                                  |
+| IGNR  | Override (phtml/js/html) | vendor/magento/module-vault/view/frontend/web/js/view/payment/method-renderer/vault.js                                             | app/design/frontend/Ampersand/theme/Magento_Vault/web/js/view/payment/method-renderer/vault.js                                     |
+| IGNR  | Preference               | vendor/ampersand/upgrade-patch-helper-test-module/src/module/Model/ToPreferenceAndIgnore.php                                       | Ampersand\Test\Model\ToPreferenceAndIgnore                                                                                         |
 +-------+--------------------------+------------------------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------+
-WARN count: 58
+WARN count: 61
 INFO count: 395
+IGNORE count: 7
 For docs on each check see https://github.com/AmpersandHQ/ampersand-magento2-upgrade-patch-helper/blob/master/docs/CHECKS_AVAILABLE.md

Checklist

convenient commented 1 year ago

@hostep @sprankhub I think this is it, there's probably room for improvement as we get some real world experience of how these checks come through but would be curious to see if you have any rough thoughts.

@peterjaap FYI new WARN type, and the introduction of IGNR which is hidden by default and probably does not need reported in your tool.

hostep commented 1 year ago

Great job! Awesome work!!

Just gave it a try on a extremely customised project that we're currently upgrading from 2.3.6 to 2.4.6. And it only warns about 312 files instead of 328 files before. The 16 files it marks to be ignored all seem to be correct and are:

Examples ```diff diff -ur -N vendor_orig/magento/module-bundle/view/frontend/templates/catalog/product/view/summary.phtml vendor/magento/module-bundle/view/frontend/templates/catalog/product/view/summary.phtml --- vendor_orig/magento/module-bundle/view/frontend/templates/catalog/product/view/summary.phtml 2023-11-08 18:34:17 +++ vendor/magento/module-bundle/view/frontend/templates/catalog/product/view/summary.phtml 2023-07-21 09:34:42 @@ -3,10 +3,9 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ -?> -getProduct(); +// phpcs:disable PHPCompatibility.Miscellaneous.RemovedAlternativePHPTags.MaybeASPOpenTagFound +$_product = $block->getProduct(); ?> isSaleable() && $block->hasOptions()) : ?>
getData('wishlistDataViewModel'); ?> isAllow()): ?> diff -ur -N vendor_orig/magento/module-wishlist/view/frontend/templates/view.phtml vendor/magento/module-wishlist/view/frontend/templates/view.phtml --- vendor_orig/magento/module-wishlist/view/frontend/templates/view.phtml 2023-11-08 18:34:13 +++ vendor/magento/module-wishlist/view/frontend/templates/view.phtml 2023-02-23 14:11:02 @@ -4,6 +4,7 @@ * See COPYING.txt for license details. */ +// phpcs:disable PHPCompatibility.Miscellaneous.RemovedAlternativePHPTags.MaybeASPOpenTagFound /** @var \Magento\Wishlist\Block\Customer\Wishlist $block */ ?> diff -ur -N vendor_orig/magento/framework/Pricing/PriceCurrencyInterface.php vendor/magento/framework/Pricing/PriceCurrencyInterface.php --- vendor_orig/magento/framework/Pricing/PriceCurrencyInterface.php 2023-11-08 18:34:12 +++ vendor/magento/framework/Pricing/PriceCurrencyInterface.php 2023-09-22 08:57:04 @@ -14,9 +14,6 @@ */ interface PriceCurrencyInterface { - /** - * Default precision - */ const DEFAULT_PRECISION = 2; /** @@ -48,7 +45,7 @@ * @param int $precision * @param null|string|bool|int|\Magento\Framework\App\ScopeInterface $scope * @param \Magento\Framework\Model\AbstractModel|string|null $currency - * @return float + * @return string */ public function format( $amount, diff -ur -N vendor_orig/magento/module-catalog-inventory/Block/Stockqty/DefaultStockqty.php vendor/magento/module-catalog-inventory/Block/Stockqty/DefaultStockqty.php --- vendor_orig/magento/module-catalog-inventory/Block/Stockqty/DefaultStockqty.php 2023-11-08 18:34:14 +++ vendor/magento/module-catalog-inventory/Block/Stockqty/DefaultStockqty.php 2023-07-21 09:34:44 @@ -13,8 +13,8 @@ * @since 100.0.2 * * @deprecated 100.3.0 Replaced with Multi Source Inventory - * @link https://devdocs.magento.com/guides/v2.3/inventory/index.html - * @link https://devdocs.magento.com/guides/v2.3/inventory/catalog-inventory-replacements.html + * @link https://devdocs.magento.com/guides/v2.4/inventory/index.html + * @link https://devdocs.magento.com/guides/v2.4/inventory/inventory-api-reference.html */ class DefaultStockqty extends AbstractStockqty implements \Magento\Framework\DataObject\IdentityInterface { diff -ur -N vendor_orig/magento/module-catalog/Api/Data/ProductAttributeInterface.php vendor/magento/module-catalog/Api/Data/ProductAttributeInterface.php --- vendor_orig/magento/module-catalog/Api/Data/ProductAttributeInterface.php 2023-11-08 18:34:19 +++ vendor/magento/module-catalog/Api/Data/ProductAttributeInterface.php 2023-09-22 08:56:56 @@ -34,8 +34,6 @@ const CODE_WEIGHT = 'weight'; /** - * Get extension attributes - * * @return \Magento\Eav\Api\Data\AttributeExtensionInterface|null * @since 103.0.0 */ diff -ur -N vendor_orig/magento/module-catalog/Block/Product/ReviewRendererInterface.php vendor/magento/module-catalog/Block/Product/ReviewRendererInterface.php --- vendor_orig/magento/module-catalog/Block/Product/ReviewRendererInterface.php 2023-11-08 18:34:18 +++ vendor/magento/module-catalog/Block/Product/ReviewRendererInterface.php 2023-09-22 08:56:56 @@ -9,6 +9,7 @@ /** * Interface \Magento\Catalog\Block\Product\ReviewRendererInterface * + * @api */ interface ReviewRendererInterface { ```

Just one thing I'm missing is the mention of this new --show-ignore flag in the README file, I see it's documented in that CHECKS_AVAILABLE.md file, but a mention on the main README file can't harm here I think.

Again, awesome job! 👏

convenient commented 1 year ago

Good catch on the README.md I have updated that now.

Thanks for the real world example @hostep that looks good to me :)

Sorry for taking ~5 years to solve #13 but at least it's here now 😆