Order validation on multi-language sites breaks on Kirby 3.6.4+

jimkrutor commented 2 years ago

Reproduced on

Kirby site running on 3.6.4 and up
Merx 1.5.1
Multi-language settings on with one language ('cs' in my case, to have plugins translated)

Steps to reproduce issue

Put product in cart, fill checkout form and click submit
Field validation returns every required field as missing even when I did fill them correctly

Submit request response:

{
"exception": "Kirby\\Exception\\Exception",
"message": "Pole bylo nesprávně zadáno.", //Field validation failed.
"key": "error.merx.fieldsvalidation", 
"file": "site/plugins/merx/src/Merx.php",
"line": 233,
"details": {
    "email": {
        "label": "Email",
        "message": {
            "required": "Zadejte prosím jakoukoli hodnotu" //Please enter something
        }
    },
    "phone": {
        "label": "Phone",
        "message": {
            "required": "Zadejte prosím jakoukoli hodnotu" //Please enter something
        }
    },
    "name": {...},
    "street": {...},
    "postalcode": {...},
    "city": {...},
    "countrycode": {...},
    "paymentmethod": {...},
},
"code": 400
}

My investigation

I isolated the breaking change to this bugfix included in Kirby 3.6.4 that makes $page->blueprint() language specific

When OrderPage->errors() is run to check recieved form data, it runs inherited function from Kirby/Cms/ModelWithContent (line 240) that is using said blueprint:

/**
* Returns all content validation errors
*
* @return array
*/
public function errors(): array
{
$errors = [];

foreach ($this->blueprint()->sections() as $section) {
    $errors = array_merge($errors, $section->errors());
}

return $errors;
}

Because virtual OrderPage is created without translations, just with "content" field, the validator sees data for validated blueprint as empty:

{
    ...
    "content": { // <- this should be validated
        "deliveryMethod": "dpd",
        "paymentMethod": "invoice",
        "name": "John Doe",
        "street": "Street 123",
        "postalcode": "12345",
        "city": "City",
        "email": "john.doe@test.com",
        "countryCode": "CZ",
        "useDifferentShippingAddress": "off",
        "shippingaddressname": "",
        "shippingaddressphone": "",
        "shippingaddressstreet": "",
        "shippingaddresspostalcode": "",
        "shippingaddresscity": "",
        "shippingAddressCountryCode": "CZ",
        "additionalinfo": "",
        "legal": "on",
        "items": "..."
    },
        "translations": {
        "cs": {
            "code": "cs",
            "content": [], // <- this is actually validated
            "exists": false,
            "slug": null
        }
    }
    ...
}

Solutions I tried

Lock kirby version to 3.6.3, works but it's just workaround.
Set fields in blueprint as not translatable with translate: false, this did not help.
Try to generate the translation in OrderPage model itself, right before the check. I tried to do it in __construct and errors() function, but I didn't find a way how to make it work. Maybe it's possible.

Modify Merx.php on line 216 to generate default translation when creating OrderPage, this works and it looks like it is using same logic as kirby bugfix that introduced this. I am not really confident if this is the correct and future-proof way, not to mention actual multilingual site with more languages.

// create virtual order page
$virtualOrderPage = new OrderPage([
'slug' => Str::random(16),
'template' => 'order',
'model' => 'order',
'content' => $data,
// modification start
'translations' => [
    kirby()->languageCode() => [
        'code' => kirby()->languageCode(),
        'content' => $data
    ]
]
// modification end
]);

tobiasfabian commented 2 years ago

Hi @jimkrutor,

thank you so much for your detailed bug report and sorry for the late replay. I think there is already a fix for this problem in the develop branch (commit https://github.com/wagnerwagner/merx/commit/99b92e95214ba982a2dfc4745886cd81c73df1f8).

I will have take a closer look at this. If this is a solution for your problem I will merge it into the main branch and create a new release.

christinegotth commented 2 years ago

I have the exact same problem and would love a fix in the next release.

el-schneider commented 2 years ago

I ran into the same problem today. Any news?

jimkrutor commented 2 years ago

Hi all! I tried solution from commit https://github.com/wagnerwagner/merx/commit/99b92e95214ba982a2dfc4745886cd81c73df1f8 and here's my feedback:

Said fix is taking place in completePayment() method which is happening later in the process. In my case, saving order page (creating content file) works without problem.

Original issue starts earlier in initializePayment() method. In this step, $virtualOrderPage is created and validated. Validation fails because virtual page have its translation created empty by default. I tried to use same approach with $kirby->setCurrentLanguage() before creating $virtualOrderPage, but translation is still created empty.

I still see forcing of the default translation as a viable option. I think I found better way to do it, that can work even without multi-lang option and hopefully for multi-lang sites with more than one language. I didn't do any deep testing of different language configurations though.

Merx.php starting from line 215 (v1.6.1)

// create virtual order page
$virtualOrderPage = new OrderPage([
    'slug' => Str::random(16),
    'template' => 'order',
    'model' => 'order',
    'content' => $data
]);

//modification start
$kirby = kirby();

if ($kirby->multilang()) {
    $languageCode = $kirby->defaultLanguage()->code();
    $virtualOrderPage->translation($languageCode)->update($data);
}
//modification end

// check for validation errors
$errors = $virtualOrderPage->errors();
if (sizeof($errors) > 0) {
    throw new Exception([
        'key' => 'merx.fieldsvalidation',
        'httpCode' => 400,
        'details' => $errors,
    ]);
}

I also found another workaround in OrderPage model that fills empty translation on request. It works, but i think fix on merx level will be more effecient (not having to fetch and convert content object)

site/models/order.php

public function translation(?string $languageCode = null) {

    if($translation = parent::translation($languageCode)) { //language exists

        if($translation->content()) { 
            //this should happen most of the time
            return $translation; 
        } else { 
            //this should happen only once in initializePayment()
            parent::translation($languageCode)->update($this->content()->toArray());
            return parent::translation($languageCode);
        }

    } else { //language do not exist
        return null;
    }
}

tobiasfabian commented 2 years ago

Hello everyone!

thanks for your patience and sorry for the late fix of this issue.

This issue should be fixed with 1.7.0-beta.1. Please report back if you still have problems with validation on multilingual pages.

jimkrutor commented 1 year ago

Confirmed - this solved my original issue, thank you!

wagnerwagner / merx