craftcms / commerce

Fully integrated ecommerce for Craft CMS.
https://craftcms.com/commerce
Other
215 stars 170 forks source link

[4.2.11]: Command `commerce/upgrade` fails on Addresses with Cyrillic Characters #3249

Open david-olson opened 1 year ago

david-olson commented 1 year ago

What happened?

Description

During a Craft 3 -> 4 upgrade involving Commerce, running the commerce/upgrade command causes an error in theiconic/name-parser when the upgrade script encounters addresses that have names with Cyrillic Characters. The following error is thrown:

'TypeError' with message 'TheIconic \NameParser\Part \AbstractPart:: camelcase(): Return value must be of type string, null returned'
in /code/vendor/theiconic/name-parser/src/Part/AbstractPart.php:73

Steps to reproduce

  1. Have a running instance of Craft 3 and Craft Commerce 3
  2. Add an address with a first or last name field with a Cyrillic character (like й)
  3. Perform the Craft 3 -> 4 upgrade steps
  4. Run the command craft commerce/upgrade

Expected behavior

The upgrade script migrates the data in the database.

Actual behavior

An error is thrown when the address with the Cyrillic character is reached.

Additional Notes

A related issue in the package causing the error can be seen here. For our current purposes, a workaround has been to add an event listener to the craft\elements\Address class, listen for EVENT_BEFORE_SAVE, and check the sending element's fullName property for Cyrillic characters. utf8_encode is run on the value if the characters are found.

Craft CMS version

4.4.15

Craft Commerce version

4.2.11

PHP version

8.1

Operating system and version

Ubuntu 20.04.6 LTS

Database type and version

MySQL 8.0.32

Image driver and version

No response

Installed plugins and versions

Anubarak commented 10 months ago

Nearly same issue for me, my migration failed after 74 hours due to an invalid user input, now I have to start it again. At least in the migration process there should be something that catches those exceptions since the entire very long running migration will fail only because one address name cannot be parsed. Especially because it is not even required at this point.

david-olson commented 10 months ago

@Anubarak I added the following to a module to help me get through the migration. The behavior referenced just provides a boolean value. Hopefully this helps you or any others seeing the issue until this gets fixed.

I included a full list of characters that could potentially cause errors, but the main one I saw issues with was И (uppercase I).

class MigrationModule extends Module
{
    public const CYRILLIC_CHARS = ['Љ', 'Њ', 'Џ', 'џ', 'ш', 'ђ', 'ч', 'ћ', 'ж', 'љ', 'њ', 'Ш', 'Ђ', 'Ч', 'Ћ', 'Ж', 'Ц', 'ц', 'а', 'б', 'в', 'г', 'д', 'е', 'ё', 'ж', 'з', 'и', 'й', 'к', 'л', 'м', 'н', 'о', 'п', 'р', 'с', 'т', 'у', 'ф', 'х', 'ц', 'ч', 'ш', 'щ', 'ъ', 'ы', 'ь', 'э', 'ю', 'я', 'А', 'Б', 'В', 'Г', 'Д', 'Е', 'Ё', 'Ж', 'З', 'И', 'Й', 'К', 'Л', 'М', 'Н', 'О', 'П', 'Р', 'С', 'Т', 'У', 'Ф', 'Х', 'Ц', 'Ч', 'Ш', 'Щ', 'Ъ', 'Ы', 'Ь', 'Э', 'Ю', 'Я'];

// ...
    public function init()
    {
        parent::init();

        Event::on(
            Address::class,
            Element::EVENT_BEFORE_SAVE,
            function (Event $event) {
                $element = $event->sender;

                $foundCount = 0;
                str_ireplace(self::CYRILLIC_CHARS, '', $element->fullName, $foundCount);
                if ($foundCount > 0) {
                    $element->fullName = utf8_encode($element->fullName);
                    // insert a row to a db
                    (new Db)->insert('{{%bugged_commerce_migration}}', ['address_uid' => $element->uid]);
                }
            }
        );

        Event::on(
            Address::class,
            Model::EVENT_DEFINE_BEHAVIORS,
            static function (DefineBehaviorsEvent $event) {
                $event->sender->attachBehaviors([
                    Utf8EncodedNameBehavior::class,
                ]);
            }
        );

        /**
         * @property craft\db\Query $query
         */
        Event::on(
            AddressQuery::class,
            ElementQuery::EVENT_BEFORE_PREPARE,
            static function (Event $event) {
                $query = $event->sender->query;
                $query->select['nameNeedsDecoding'] = 'IF(bugged_commerce_migration.id IS NOT NULL, TRUE, FALSE)';
                $query->join[] = [
                    "LEFT JOIN",
                    "{{%bugged_commerce_migration}}",
                    "[[elements.uid]] = [[bugged_commerce_migration.address_uid]]"
                ];
            }
        );

        Event::on(
            AddressQuery::class,
            ElementQuery::EVENT_AFTER_POPULATE_ELEMENT,
            static function (PopulateElementEvent $event) {
                if ($event->element->nameNeedsDecoding) {
                    $event->element->firstName = utf8_decode($event->element->firstName);
                    $event->element->lastName = utf8_decode($event->element->lastName);
                    $event->element->fullName = utf8_decode($event->element->fullName);
                }
            }
        );

    }
}
nfourtythree commented 10 months ago

Hi @david-olson

Thank you for reporting, do you have the full stack trace from the original error?

I am thinking that if this error exists during migration then this same issue could exist in the system under normal operation.

I have tried to see if I can simply recreate the error by using those characters and saving an address in the control panel in the latest version of Craft but everything seems to save as expected. Is this the same for you (if you remove your module)?

Thanks