[Fixed: #31396] Data/Schema Patches getAliases() not working as expected #37682

MatthijsBreed · 2023-06-28T10:57:20Z

Description (*)

This PR fixed next issues #31396 #23031

This PR is a continuation of work in PR 31635 (#31635) to account for every patch alias that might be already applied and to persist any new patch aliases

Fixed Issues (if relevant)

#31396 #23031

Testing Scenario

1. Create new Data Patch and apply via bin/magento setup:upgrade

2. See that patch is added to patch_list in the database

3. Rename Data Patch file and add old name to aliases

4. Re-run bin/magento setup:upgrade

5. See that the new patch name is added to path_list but the apply() function has not been called

…ta-patch-fix

…date patch aliases in db regardless of patch application

m2-assistant · 2023-06-28T10:57:23Z

Hi @MatthijsBreed. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

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

MatthijsBreed · 2023-06-28T10:57:36Z

@magento run all tests

magento-automated-testing · 2023-06-28T10:57:45Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

sippsolutions · 2023-07-20T13:51:04Z

lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php

@@ -24,6 +24,7 @@
 use Magento\Framework\Setup\SchemaSetupInterface;
 use Magento\Framework\Setup\SetupInterface;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use phpDocumentor\Reflection\Types\True_;


this can be removed, or not?

Suggested change

use phpDocumentor\Reflection\Types\True_;

sippsolutions · 2023-07-20T13:51:21Z

lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php

@@ -296,4 +292,38 @@ public function revertDataPatches($moduleName = null)
            }
        }
    }
+
+    /**
+     * Checks if patch should be applied by already applied patch alias names


Suggested change

* Checks if patch should be applied by already applied patch alias names

* Check if patch should be applied by already applied patch alias names

sippsolutions · 2023-07-20T13:52:08Z

lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php

+        if ($patch = $patch->getAliases()) {
+            foreach ($patch as $patchAlias) {
+                if ($this->patchHistory->isApplied($patchAlias)) {
+                    $shouldApply = false;


variable assigning is not necessary, you can use return directly

MatthijsBreed · 2023-07-20T16:24:46Z

@magento run all tests

magento-automated-testing · 2023-07-20T16:24:57Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

MatthijsBreed · 2023-07-20T16:35:54Z

@magento run Unit Tests

magento-automated-testing · 2023-07-20T16:36:00Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

MatthijsBreed · 2023-07-20T20:48:00Z

@sippsolutions , thank you for your feedback.

I have checked all failing tests, but after the last commit, the only tests that are failing are urelated functionalities, are these blocking for this PR or is there another way to resolve them?

Unit Tests fail on 'Magento\CatalogGraphQl\Test\Unit\DataProvider\Product\SearchCriteriaBuilderTest::testBuild'

Functional tests fail on "Magento\AcceptanceTest_InventorySingleModeSuite_0_G\Backend\StorefrontValidateVisualSwatchAbsenceOfAttributeWhenProductStocksOutTestCest::StorefrontValidateVisualSwatchAbsenceOfAttributeWhenProductStocksOutTest" and "Magento\AcceptanceTest_default\Backend\AdminApplyChangePriceForConfigurableProductWithAssignedSimpleProductsTestCest::AdminApplyChangePriceForConfigurableProductWithAssignedSimpleProductsTest"

Webapi tests fail on 'Magento\GraphQl\Catalog\ProductSearchTest::testFilterProductsBySingleCategoryId'

engcom-Hotel · 2023-09-18T05:46:38Z

@magento run all tests

magento-automated-testing · 2023-09-18T05:46:48Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

engcom-Hotel · 2023-09-18T05:51:17Z

@magento run all tests

magento-automated-testing · 2023-09-18T05:51:21Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

engcom-Hotel · 2023-09-18T09:35:44Z

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

magento-automated-testing · 2023-09-18T09:35:50Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

engcom-Hotel

Hello @MatthijsBreed,

Thanks for the contribution!

Code changes look good to me and failed tests seem flaky.

Hence approving this PR.

engcom-Echo · 2023-09-26T06:10:44Z

@magento run all tests

magento-automated-testing · 2023-09-26T06:10:54Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

engcom-Echo · 2023-09-27T06:20:28Z

@MatthijsBreed,

We are not able to reproduce the issue.
Can you please confirm the issue still exist on 2.4-develop?

To Reproduce the issue we have followed below steps.

Create new Data Patch and apply via bin/magento setup:upgrade
See that patch is added to patch_list in the database
Rename Data Patch file and add old name to aliases
Re-run bin/magento setup:upgrade
Patch gets applied successfully.

Also please refer to this abc8e696.

Lets us know if you are still facing the issue on 2.4-develop with detailed steps.

Moving it to on hold

MatthijsBreed · 2023-09-27T08:30:41Z

@engcom-Echo

The reproduction steps you described you described are the problem this PR aims to solve.
The commit you linked is only a check for possibly writing duplicate aliases to the database AFTER the patch has been applied.

Current (incorrect) situation:

Create a patch called 'A' with no aliases
Run bin/magento setup:upgrade, patch A gets applied and 'A' is added to the patch_list table
Rename patch A to 'B'
Add 'A' to the aliases
Run bin/magento setup:upgrade again, patch A gets applied again and 'B' is added to the patch_list table

Desired situation:

Create a patch called 'A' with no aliases
Run bin/magento setup:upgrade, patch A gets applied and 'A' is added to the patch_list table
Rename patch A to 'B'
Add 'A' to the aliases
Run bin/magento setup:upgrade again, patch A is not applied and 'B' is added to the patch_list table

Please see the PR description and the issues linked therein

engcom-Echo · 2023-09-27T11:50:18Z

@MatthijsBreed

I have followed the step mentioned here

Before PR

Create a patch called 'ExampleDataPatch' with no aliases

class ExampleDataPatch implements DataPatchInterface
{
    /**
     * @var ModuleDataSetupInterface
     */
    private $moduleDataSetup;


    /**
     * @var LoggerInterface
     */
    private $logger;

    public function __construct(
        ModuleDataSetupInterface $moduleDataSetup,
        LoggerInterface $logger
    ) {
        $this->moduleDataSetup = $moduleDataSetup;
        $this->logger = $logger;
    }
    
    public function apply()
    {
        $this->moduleDataSetup->startSetup();
        $this->logger->info('ExampleSchemaPatch: Apply method executed.');
        $this->moduleDataSetup->endSetup();
    }
    
    public static function getDependencies()
    {
        return [];
    }
    
    public function getAliases()
    {
        return [];
    }
}

Run bin/magento setup:upgrade, patch ExampleDataPatch gets applied and 'ExampleDataPatch' is added to the patch_list table
Rename patch ExampleDataPatch file and class name to 'ChangedExampleDataPatch'
Add 'ExampleDataPatch' to the aliases

public function getAliases()
    {
        return [
            ExampleDataPatch::Class
        ];
    }

Run bin/magento setup:upgrade again, patch ExampleDataPatch gets applied again verfied from log file and 'ChangedExampleDataPatch' is added to the patch_list table

After PR Changes

Follow step 1-6 as above case
Run bin/magento setup:upgrade again, patch ExampleDataPatch not applied again verfied from log file. But ChangedExampleDataPatch is not added to the patch_list table

Let us know if we have missed anything here.

engcom-Echo · 2023-10-04T13:53:53Z

@MatthijsBreed can you please have a look at comment and suggest if we are missing anything here?

engcom-Echo · 2023-10-10T10:23:05Z

@MatthijsBreed did you get time to have a look at comment ?

engcom-Charlie · 2023-10-16T05:59:13Z

Hi @MatthijsBreed,

Thank you for your contribution!

As mentioned here, please have a look and let us know if we are missing anything.

Till then closing this PR. Please feel free to reopen it as you want to update further.

Thank you!

Usik2203 and others added 9 commits January 13, 2021 14:52

Fixed issue with Data/Schema patch applying

f8bed9b

minor fix

015ac91

Merge branch '2.4-develop' into data-patch-fix

05aba68

Merge branch '2.4-develop' into data-patch-fix

fbca506

Fixed failed tests

d88bd43

Fixed failed tests

13e4a2d

Merge branch '2.4-develop' into data-patch-fix

391d60e

Merge branch 'data-patch-fix' of github.com:Usik2203/magento2 into da…

3cf9514

…ta-patch-fix

Update to Usik2203's PR branch, account for applied patch aliases, up…

31f3498

…date patch aliases in db regardless of patch application

engcom-Bravo added the Priority: P3 May be fixed according to the position in the backlog. label Jun 28, 2023

m2-community-project bot added the Progress: pending review label Jun 28, 2023

Fix testcases

6881afe

sippsolutions suggested changes Jul 20, 2023

View reviewed changes

m2-community-project bot assigned sippsolutions Jul 20, 2023

m2-community-project bot added Progress: needs update and removed Progress: pending review labels Jul 20, 2023

sippsolutions removed their assignment Jul 20, 2023

m2-community-project bot added Progress: pending review and removed Progress: needs update labels Jul 20, 2023

MatthijsBreed and others added 2 commits July 20, 2023 16:18

Merge branch 'magento:2.4-develop' into data-patch-fix

9a69f52

Applied feedback, fixed unit tests

8ddedc8

Fix 1 more test

76e06f1

m2-community-project bot added Progress: review and removed Progress: needs update labels Sep 15, 2023

Merge branch '2.4-develop' into data-patch-fix

3f43bcf

engcom-Hotel approved these changes Sep 18, 2023

View reviewed changes

m2-community-project bot added Progress: ready for testing and removed Progress: review labels Sep 18, 2023

Merge branch '2.4-develop' into data-patch-fix

a13676e

engcom-Lima added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Sep 25, 2023

engcom-Echo self-assigned this Sep 26, 2023

m2-community-project bot added Progress: testing in progress and removed Progress: ready for testing labels Sep 26, 2023

m2-community-project bot added Progress: on hold and removed Progress: testing in progress labels Sep 27, 2023

engcom-Charlie closed this Oct 16, 2023

m2-community-project bot removed the Progress: on hold label Oct 16, 2023

thomas-kl1 mentioned this pull request Dec 1, 2023

Fix Patch aliases usage (issue #31396) #38239

Open

5 tasks

	* Checks if patch should be applied by already applied patch alias names
	* Check if patch should be applied by already applied patch alias names

[Fixed: #31396] Data/Schema Patches getAliases() not working as expected #37682

[Fixed: #31396] Data/Schema Patches getAliases() not working as expected #37682

Uh oh!

Conversation

MatthijsBreed commented Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues (if relevant)

Testing Scenario

Uh oh!

m2-assistant bot commented Jun 28, 2023

Uh oh!

MatthijsBreed commented Jun 28, 2023

Uh oh!

magento-automated-testing bot commented Jun 28, 2023

Uh oh!

sippsolutions Jul 20, 2023

Choose a reason for hiding this comment

Uh oh!

sippsolutions Jul 20, 2023

Choose a reason for hiding this comment

Uh oh!

sippsolutions Jul 20, 2023

Choose a reason for hiding this comment

Uh oh!

MatthijsBreed commented Jul 20, 2023

Uh oh!

magento-automated-testing bot commented Jul 20, 2023

Uh oh!

MatthijsBreed commented Jul 20, 2023

Uh oh!

magento-automated-testing bot commented Jul 20, 2023

Uh oh!

MatthijsBreed commented Jul 20, 2023

Uh oh!

engcom-Hotel commented Sep 18, 2023

Uh oh!

magento-automated-testing bot commented Sep 18, 2023

Uh oh!

engcom-Hotel commented Sep 18, 2023

Uh oh!

magento-automated-testing bot commented Sep 18, 2023

Uh oh!

engcom-Hotel commented Sep 18, 2023

Uh oh!

magento-automated-testing bot commented Sep 18, 2023

Uh oh!

engcom-Hotel left a comment

Choose a reason for hiding this comment

Uh oh!

engcom-Echo commented Sep 26, 2023

Uh oh!

magento-automated-testing bot commented Sep 26, 2023

Uh oh!

engcom-Echo commented Sep 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MatthijsBreed commented Sep 27, 2023

Uh oh!

engcom-Echo commented Sep 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

engcom-Echo commented Oct 4, 2023

Uh oh!

engcom-Echo commented Oct 10, 2023

Uh oh!

engcom-Charlie commented Oct 16, 2023

Uh oh!

Uh oh!

MatthijsBreed commented Jun 28, 2023 •

edited

Loading

engcom-Echo commented Sep 27, 2023 •

edited

Loading

engcom-Echo commented Sep 27, 2023 •

edited

Loading