Avoid search exception for calendar events when using localized time formats
Needs ReviewPublic
Actions

Authored by aklapper on May 4 2024, 12:21.

Details

Reviewers

None

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15811: Search exception when AM/PM string is localized in Calendar application

Summary

Do not call formatTime which will return a localized time format (based on the translation setting of the user) which then fails to match the (English language) $translatable characters defined in PhutilTranslator.php, throwing a search exception when calendar items match the search query string.

Also update DateTime exception handling comment per changes in PHP 8.3:
https://www.php.net/manual/en/class.datemalformedstringexception.php

EXCEPTION: (PhutilAggregateException) All of the configured Fulltext Search services failed.
    - DateMalformedStringException: Failed to parse time string (2024-05-04 12:00 epp.) at position 17 (e): The timezone could not be found in the database at [<phorge>/src/infrastructure/cluster/search/PhabricatorSearchService.php:276]

Closes T15811

Test Plan

Please follow the verbose steps in T15811. In short: Install the Translations extension, set your language to a language which translated AM/PM in time strings, create a calendar event, use the global search to search for the calendar event title.

Diff Detail

Repository

rP Phorge

Branch

amPmCalExplode (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1221
Build 1221: arc lint + arc unit

Event Timeline

aklapper created this revision.May 4 2024, 12:21

Owners added a reviewer: O1: Blessed Committers.May 4 2024, 12:21

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptMay 4 2024, 12:21

aklapper requested review of this revision.May 4 2024, 12:21

Harbormaster completed remote builds in B1221: Diff 1933.May 4 2024, 12:21

pppery subscribed.May 4 2024, 15:42

aklapper mentioned this in T15815: Make i18n string extraction ignore strings in test cases.May 7 2024, 20:36

valerio.bozzolan added inline comments.Fri, Jun 7, 15:42

src/view/viewutils.php

128–140

Part of this function is useful. For example, this historical try-catch with human error is useful.

We should probably keep that.

To re-use this code, maybe it's nice to create a new function that just does the basic thing we need:

function phabricator_datetime_from_epoch($epoch) {

  // NOTE: Although DateTime takes a second DateTimeZone parameter to its
  // constructor, it ignores it if the date string includes timezone
  // information. Further, it treats epoch timestamps ("@946684800") as having
  // a UTC timezone. Set the timezone explicitly after constructing the object.
  try {
    $date = new DateTime('@'.$epoch);
  } catch (Exception $ex) {
    ...
  }

  $date->setTimezone($zone);

  return $date;
}