Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed null deprecation in Mage_Catalog_Model_Product_Option_Type_Text::validateUserValue() #4357

Merged
merged 7 commits into from
Nov 22, 2024

Conversation

kiatng
Copy link
Contributor

@kiatng kiatng commented Nov 15, 2024

    [type] => 8192:E_DEPRECATED
    [message] => trim(): Passing null to parameter #1 ($string) of type string is deprecated
    [file] => .../app/code/core/Mage/Catalog/Model/Product/Option/Type/Text.php
    [line] => 36

@kiatng kiatng added the PHP 8.1 Related to PHP 8.1 label Nov 15, 2024
@github-actions github-actions bot added the Component: Catalog Relates to Mage_Catalog label Nov 15, 2024
@sreichel
Copy link

Its an issue in core or 3rd-party?

@kiatng
Copy link
Contributor Author

kiatng commented Nov 15, 2024

It happened in custom code to create an order programmatically, I agree that null check for values can be placed here. But I feel the core should be more tolerant because it works before. It's a matter of opinion, I have no issue if others feel otherwise.

@sreichel sreichel added the 3rd-party Related to 3rd-party code or issues with customization label Nov 15, 2024
@sreichel
Copy link

sreichel commented Nov 15, 2024

Mhh, instead of adding null-checks everywhere (not only in this PR), why not to override the magic method calls? Add ... ?

    public function getUserValue(): string
    {
        return (string) $this->getDataByKey('user_value');
    }

Sven Reichel and others added 3 commits November 15, 2024 05:56
@kiatng
Copy link
Contributor Author

kiatng commented Nov 15, 2024

Commit 2 doesn't work: I get Warning: Array to string conversion in .../app/code/core/Mage/Catalog/Model/Product/Option/Type/Default.php on line 59 (in my version in production)

@kiatng kiatng marked this pull request as draft November 15, 2024 07:11
@kiatng
Copy link
Contributor Author

kiatng commented Nov 15, 2024

It's not that straight forward!

@sreichel
Copy link

you made the change in the wrong file. check https://github.com/kiatng/magento-lts/pull/2/files

kiatng and others added 2 commits November 15, 2024 15:19
@kiatng kiatng marked this pull request as ready for review November 15, 2024 07:29
@sreichel sreichel requested a review from addison74 November 22, 2024 20:12
@sreichel sreichel changed the title Fixed null deprecation in Mage/Catalog/Model/Product/Option/Type/Text Fixed null deprecation in Mage_Catalog_Model_Product_Option_Type_Text::validateUserValue() Nov 22, 2024
@addison74 addison74 merged commit 11db607 into OpenMage:main Nov 22, 2024
19 checks passed
fballiano added a commit to MahoCommerce/maho that referenced this pull request Nov 22, 2024
@kiatng kiatng deleted the null_deprecation_product_option_text branch November 25, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd-party Related to 3rd-party code or issues with customization Component: Catalog Relates to Mage_Catalog PHP 8.1 Related to PHP 8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants