verbb / formie

The most user-friendly forms plugin for Craft CMS.
Other
95 stars 72 forks source link

Element integration saves incorrect timezone for Time subfield #1792

Closed cdccairns closed 6 months ago

cdccairns commented 6 months ago

Describe the bug

This bug is more or less a recurrence of a previously-reported bug.

Our website contains a form with two Table fields. Each Table field contains seven Time subfields, one for each day of the week. The entered values are mapped to equivalent entry fields (a Table field with seven Time subfields) via an Element integration.

Testing shows that the entered values are correctly saved in the submission. 10:00 is 10:00, 16:00 is 16:00, etc. However, there is a discrepancy in the way that the values are saved in the CMS, and strangely enough one which appears to be linked to the user's system settings and/or the global timezone in the CMS (in our case CET).

Values entered in a browser on MacOS are incremented by one hour (i.e. to the difference between GMT and CET) on each save, or every time the Element integration is triggered. 10:00 becomes 11:00, 16:00 becomes 17:00, etc. However, this issue does not occur in Windows browsers nor when the global timezone in Craft is set to GMT.

Steps to reproduce

  1. Set the global timezone in Craft to CET.
  2. Create a Section with a Table field and a Time subfield.
  3. Create a Form with a Table field and a Time subfield.
  4. Set up an Element integration and map the form fields to the Entry fields.
  5. Make a submission (first in a Windows browser, then in a MacOS browser).

Form settings

Craft CMS version

4.5.5

Plugin version

2.1.3

Multi-site?

No response

Additional context

No response

engram-design commented 6 months ago

There was an issue when editing a submission, which is fixed for the next release. To get this early, run composer require verbb/formie:"dev-craft-4 as 2.1.7".

As for the element side of things, Formie should be taking the raw value you provide (there's no timezone data saved) and converting that to UTC from the system time. This is mostly because Formie stores dates as strings, in the exact format the user provides when filling out the form. This is because we don't want the dates being localised to whatever the user's control panel locale settings are.

Just wanted to confirm if there's still an issue on the entry side after this fix?

cdccairns commented 6 months ago

This appears to have resolved the issue with the Time subfields. However, other fields in the form (specifically a Single Line Text field, an Email Address field, and a Phone Number field) which were previously succesfully populated using populateFormValues are now empty. When I revert the change, the values are present.

engram-design commented 6 months ago

That doesn't make much sense! Can you shoot me through your populateFormValues() call? This change shouldn't have effected any other fields outside of the Table field.

And just to be clear, by populated do you mean when you load the form and the fields are populated, or the resulting submission, or the Craft fields in the resulting entry element?

cdccairns commented 6 months ago

This is the relevant code, which I have tested in version dev-craft-4 as 2.1.7 (based on this discussion) to populate fields in the aforementioned form.

{% do craft.formie.populateFormValues(form, {
    locationAccessibility: {
        locationHasDisabledParking: location.locationHasDisabledParking,
        locationHasDisabledToilet: location.locationHasDisabledToilet,
        locationHasParkingBicycles: location.locationHasParkingBicycles,
        locationIsWheelchairAccessible: location.locationIsWheelchairAccessible,
    },
    locationEmail: location.locationEmail,
    locationName: location.title,
    locationTel: location.locationTel,
    locationOpeningHours: [
        {
            maandag: location.locationOpeningHours[0]['maandag'],
            dinsdag: location.locationOpeningHours[0]['dinsdag'],
            woensdag: location.locationOpeningHours[0]['woensdag'],
            donderdag: location.locationOpeningHours[0]['donderdag'],
            vrijdag: location.locationOpeningHours[0]['vrijdag'],
            zaterdag: location.locationOpeningHours[0]['zaterdag'],
            zondag: location.locationOpeningHours[0]['zondag'],
        },
    ],
    locationClosingHours: [
        {
            maandag: location.locationClosingHours[0]['maandag'],
            dinsdag: location.locationClosingHours[0]['dinsdag'],
            woensdag: location.locationClosingHours[0]['woensdag'],
            donderdag: location.locationClosingHours[0]['donderdag'],
            vrijdag: location.locationClosingHours[0]['vrijdag'],
            zaterdag: location.locationClosingHours[0]['zaterdag'],
            zondag: location.locationClosingHours[0]['zondag'],
        },
    ],
}, false) %}

{{ craft.formie.renderForm(form) }}

I can confirm that the submitted values (which I enter manually) are saved correctly in the submission and map correctly to the associated entry fields via the element integration. However, the values in the form are not correctly pre-populated. The fields locationEmail, locationName and locationTel are empty, as is the first Time subfield maandag in the Table field. Formie also fails to pre-populate checked Agree subfields in the Group locationAccessibility.

When I instead use integer-based indexes, all of the Time subfields in the Table field are correctly pre-populated:

locationOpeningHours: [
    {
        0: location.locationOpeningHours[0]['maandag'],
        1: location.locationOpeningHours[0]['dinsdag'],
        2: location.locationOpeningHours[0]['woensdag'],
        3: location.locationOpeningHours[0]['donderdag'],
        4: location.locationOpeningHours[0]['vrijdag'],
        5: location.locationOpeningHours[0]['zaterdag'],
        6: location.locationOpeningHours[0]['zondag']
    }
]

I can also confirm that the cache is cleared, that the storage folder is empty, and that the Time subfields in the entry location return a valid datetime, for example:

DateTime @1711526400 {#2085 ▼
  date: 2024-03-27 09:00:00.0 Europe/Brussels (+01:00)
}
cdccairns commented 6 months ago

Formie reports the following errors:

2024-03-29 10:36:59 [ERROR] Error populating form values for “locationEmail”. Template error: “Failed to instantiate component or class "verbb\base\services\Templates".” /Uvendor/yiisoft/yii2/di/Container.php:509
2024-03-29 10:36:59 [ERROR] Error populating form values for “locationName”. Template error: “Failed to instantiate component or class "verbb\base\services\Templates".” /vendor/yiisoft/yii2/di/Container.php:509
2024-03-29 10:36:59 [ERROR] Error populating form values for “locationTel”. Template error: “Failed to instantiate component or class "verbb\base\services\Templates".” /vendor/yiisoft/yii2/di/Container.php:509
2024-03-29 10:36:59 [ERROR] Error populating form values for “locationOpeningHours”. Template error: “Calling unknown method: verbb\formie\fields\formfields\Table::populateValue()” /vendor/yiisoft/yii2/base/Component.php:300
2024-03-29 10:36:59 [ERROR] Error populating form values for “locationClosingHours”. Template error: “Calling unknown method: verbb\formie\fields\formfields\Table::populateValue()” /vendor/yiisoft/yii2/base/Component.php:300
engram-design commented 6 months ago

Now that's an interesting error. Those last should be fixed in the next release (to get this early, run composer require verbb/formie:"dev-craft-4 as 2.1.8".)

As for the first few, that seems like it hasn't pulled in the latest verbb/base package. Can you run composer show verbb/base and let me know the output?

engram-design commented 6 months ago

Regardless, those errors should be fixed in 2.1.9

cdccairns commented 6 months ago

Thank you for addressing these issues. There are however two errors which are unresolved, see my earlier comment:

...the first Time subfield maandag in the Table field. Formie also fails to pre-populate checked Agree subfields in the Group locationAccessibility.

engram-design commented 6 months ago

I can't seem to replicate the first table column issue, but I can confirm there was an issue with a Group + Agree field. That should be fixed for the next release. To get this early, run composer require verbb/formie:"dev-craft-4 as 2.1.9".

image image
{% do craft.formie.populateFormValues(form, {
    locationName: 'Test Name',
    locationEmail: 'test@test.com',
    locationOpeningHours: [
        {
            monday: '2022-03-03 09:00:00',
            tuesday: '2022-03-03 10:00:00',
            wednesday: '2022-03-03 11:00:00',
            thursday: '2022-03-03 12:00:00',
            friday: '2022-03-03 13:00:00',
        },
    ],
    locationAccessibility: {
        locationHasDisabledParking: true,
    },
}) %}
image
cdccairns commented 6 months ago

I can't seem to replicate the first table column issue

This is a persistent bug locally, even with the upgrade to dev-craft-4 as 2.1.9. However, as you are unable to replicate it and we are able to pre-fill the values using integer-based indexes, I am glad to close the issue.

engram-design commented 6 months ago

I'll certainly keep at it. I will say that index-based columns shouldn't actually be a thing, so that might be the cause of that. I'll report back here with anything.