Scheduled email handler ISO date/time field doesn't accept ISO8601-formatted date and time strings

Created on 28 March 2024, 3 months ago
Updated 22 April 2024, 2 months ago

Problem/Motivation

The scheduled email handler's ISO date/time field doesn't accept ISO8601-formatted date and time strings ([YYYY]-[MM]-[DD]T[hh]:[mm]:[ss]). The guidance in the field's help area and validation error message shows what appears to be a MYSQL DATETIME-formatted string (YYYY-MM-DD hh:mm:ss).

Illustration of the format difference using the example ISO 8601 date from PHP's date formatting manual page (see the "c" / "ISO 8601 date" format):

ISO8601: 2004-02-12T15:19:21+00:00
MYSQL DATETIME: 2004-02-12 15:19:21

I suspect this happened when time support was added to this field. Instead of expanding out to accept time information in ISO8601-format, which was probably intended and which the field still says it wants, MYSQL DATETIME format was used.

For my project, which has users across many timezones, entering an ISO8601-formatted date and time string here is preferable because I can include crucial timezone/offset information (instead of having to translate all send dates into the site's default timezone).

At a base level it also seems like we should accept an ISO date-time string in here if we're asking for an ISO date-time string.

Steps to reproduce

  1. Add a scheduled email handler to your webform.
  2. In the "Send email on" field, select "Custom date/time..."
  3. Enter a valid ISO8601-formatted date and time string and save the handler.
  4. See the form validation error message that your string is in the wrong format.

Proposed resolution

The scheduled email handler ultimately interprets the provided date string using strtotime() and therefore has no actual reason to require one specific format like ISO8601 or MYSQL DATETIME. Let's drop the "ISO date" stuff from the field description and instead ask for a date or date-time string in a format compatible with PHP's strtotime() function, keeping the current MYSQL DATETIME YYYY-MM-DD or YYYY-MM-DD HH:MM:SS as the suggested format. This opens the field up to actually accept ISO8601 date strings, preserves compatibility with MYSQL DATETIME strings, makes the field's description/help-text consistent with what is actually accepted, and additionally adds support for many other types of date and date-time strings without any downstream consequences.

Remaining tasks

Review.

User interface changes

The scheduled email handler's "Send on" field asks for and accepts any string compatible with PHP's strtotime() function. The MYSQL DATETIME format (YYYY-MM-DD or YYYY-MM-DD HH:MM:SS) is suggested/provided as an example, but no longer the only format accepted.

API changes

None.

Data model changes

None.

πŸ› Bug report
Status

Fixed

Version

6.2

Component

User interface

Created by

πŸ‡ΊπŸ‡ΈUnited States chrisolof

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @chrisolof
  • πŸ‡ΊπŸ‡ΈUnited States chrisolof

    After diving into the code a bit more I've realized, rather than broadening out to accepting just ISO 8601 and MYSQL DATETIME formats, it probably makes more sense to accept anything PHP's strtotime() function can consume. The downstream code just throws this value against strtotime(), which accepts many formats, including both MYSQL DATETIME and ISO 8601. As such, limiting entry to just MYSQL DATETIME and/or ISO 8601 seems unwarranted and adds what feels like unnecessary complexity to the validation method.

    The approach I'm taking with the MR currently is to drop the "ISO date" stuff and instead ask for a date or date-time string in a format compatible with PHP's strtotime() function, keeping the current MYSQL DATETIME YYYY-MM-DD or YYYY-MM-DD HH:MM:SS as the suggested format. This seems to fix the bug in the least disruptive way, bringing in actual ISO 8601 date format support, keeping MYSQL DATETIME support, slightly reducing validation complexity, and adding support for many other date formats.

    I'll post the MR for review after I've confirmed tests pass locally.

  • Open on Drupal.org β†’
    Core: 10.1.4 + Environment: PHP 8.2 & MySQL 8
    last update 3 months ago
    Waiting for branch to pass
  • Pipeline finished with Failed
    3 months ago
    Total: 2724s
    #132940
  • Status changed to Needs review 3 months ago
  • πŸ‡ΊπŸ‡ΈUnited States chrisolof

    MR 425 opened. Status changed to needs-review. Proposed resolution and UI changes sections updated to match the resolution proposed in MR 425. Immutable diff (patch) attached here capturing the current state of this MR.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.4 + Environment: PHP 8.2 & MySQL 8
    last update 3 months ago
    536 pass
  • πŸ‡ΊπŸ‡ΈUnited States chrisolof

    Merged latest 6.2.x changes into MR branch. Removed unused use statement. Added note about the default timezone that will be used to interpret the custom send-on date if not provided in the date string (so a source-code-dive isn't necessary to figure that out). Current state of the MR is attached, along with a screenshot of the changed field.

  • Pipeline finished with Success
    3 months ago
    Total: 2057s
    #139110
  • Pipeline finished with Skipped
    3 months ago
    #141139
  • First commit to issue fork.
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.4 + Environment: PHP 8.2 & MySQL 8
    last update 3 months ago
    536 pass
  • Pipeline finished with Skipped
    3 months ago
    #141140
  • Status changed to Fixed 3 months ago
  • πŸ‡ΊπŸ‡ΈUnited States jrockowitz Brooklyn, NY
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.69.0 2024