search

magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.56k stars 9.32k forks source link

Cannot update NULL value in custom module #26582

Open textarea opened 4 years ago

textarea commented 4 years ago

Preconditions (*)

  1. Magento 2.3.4 & 2.4-develop
  2. PHP 7.2.27

Steps to reproduce (*)

  1. Create a custom module with a field of any data type, for example "varchar", nullable.
  2. Insert one record with a (not null) value into the varchar field
  3. Update programmatically with null value, i.e.: 
    $testRepository = $obj->get('\Acme\Test\Api\TestRepositoryInterface');
$test = $testRepository->get(1);
$test->setField(NULL);
$testRepository->save($test);

Expected result (*)

  1. Field value is NULL

Actual result (*)

  1. Field value still contains the old value
m2-assistant[bot] commented 4 years ago

Hi @textarea. Thank you for your report. To help us process this issue please make sure that you provided the following information:

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, please, review the Magento Contributor Assistant documentation.

@textarea do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

m2-assistant[bot] commented 4 years ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

magento-engcom-team commented 4 years ago

:white_check_mark: Confirmed by @engcom-Hotel Thank you for verifying the issue. Based on the provided information internal tickets MC-30919 were created

Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

willtran commented 4 years ago

FYI can't reproduct on 2.3.3

textarea commented 4 years ago

@willtran I can reproduce this issue on 2.3.3 also.

engcom-Hotel commented 4 years ago

@willtran this issue is reproducible on 2.4-develop

m2-assistant[bot] commented 4 years ago

Hi @sanjay-wagento. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

sanjay-wagento commented 4 years ago

MM20IN

sanjay-wagento commented 4 years ago

@magento-engcom-team I have checked in Magento 2.4-develop branch with custom module and I can not reproduce issue.

textarea commented 4 years ago

@sanjay-wagento can you share your custom module?

textarea commented 4 years ago

@magento give me 2.4-develop instance

magento-engcom-team commented 4 years ago

Hi @textarea. Thank you for your request. I'm working on Magento 2.4-develop instance for you

magento-engcom-team commented 4 years ago

Hi @textarea, here is your Magento instance. Admin access: https://i-26582-2-4-develop.instances.magento-community.engineering/admin_a2e3 Login: 527ca76b Password: 813708a3759c Instance will be terminated in up to 3 hours.

rendyep commented 4 years ago

I'm unable to reproduce above issue in Magento 2.4-develop branch with custom module.

Below is the code of the custom module: http://www.mediafire.com/file/ev813ldhnhsolpl/rendyep.tar.gz/file

Extract the compressed file, put it on app/code/, enable the module (Rendyep_Test), run the upgrade script, and execute bin/magento rendyep:test:test from terminal to see the result. The result can also be checked from database on table rendyep_test_test after executing the console command from terminal.

textarea commented 4 years ago

Hi @rendyep , your module works, but I replicated it using Data\Interface instead of Model and it doesn't works.

Please give it a try with these sources: https://www.mediafire.com/file/qmgjqadpn5ixbbr/Acme.zip/file

Same instructions as yours, run bin/magento acme:test:test then check db table acme_test_test, detail column will remain filled.

rendyep commented 4 years ago

@textarea the issue is in your repository class:

        $testData = $this->extensibleDataObjectConverter->toNestedArray(
            $test,
            [],
            \Acme\Test\Api\Data\TestInterface::class
        );

        $testModel = $this->testFactory->create()->setData($testData);

        try {
            $this->resource->save($testModel);
        } catch (\Exception $exception) {
            throw new CouldNotSaveException(__(
                'Could not save the test: %1',
                $exception->getMessage()
            ));
        }
        return $testModel->getDataModel();

The value $testData of the first line when you call toNestedArray would only get test_id, no detail, so when you call $testModel = $this->testFactory->create()->setData($testData);, only test_id is filled. To be clear, this is the output of the model values just before you save.

value of $testModel->getData(): 
array(1) {
  ["detail"]=>
  string(9) "765321651"
}

Data before set detail to null: 
string(9) "765321651"

value of $testModel->getData(): 
array(1) {
  ["test_id"]=>
  string(2) "15"
}

That is why, when you save that model, the data other than test_id would be saved to database, and the others remain as they are in current values.

The null value is not converted as null to $testData because you don't mark the getDetail method in Acme\Test\Api\Data\TestInterface as required, therefore the converter skips the null value. https://github.com/magento/magento2/blob/c43d76343807540acf71b630fd2916570d79f0ff/lib/internal/Magento/Framework/Reflection/DataObjectProcessor.php#L90-L97

textarea commented 4 years ago

Thanks @rendyep, that code is generated by mage2gen, however, how to mark the getDetail method in Acme\Test\Api\Data\TestInterface as required? Thanks

textarea commented 4 years ago

Ok @rendyep, I removed @return string|null and left @return string in getDetail() method and now NULL value is saved. However I don't think this is the right solution, it is still a bug.

rendyep commented 4 years ago

@textarea could you please make sure that the value is NULL, not empty string?

textarea commented 4 years ago

@rendyep yes I'm sure

shinesoftware commented 4 years ago

We can confirm that saving a field to NULL doesn't set the custom field.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

textarea commented 3 years ago

Keep it live please

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

textarea commented 3 years ago

anyone can take a look into this?

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 28 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!