Clean up date_entity_metadata_struct_getter()

Thanks for the patch, we'll try to review it soon.

BTW when you upload a patch it helps to set the status to "needs review", that triggers the testbots and lets others know that there's something review.

Never hurt cleaning up stuff :)

When does this code actually get used? Did not test it. I tried to make date_entity_metadata_struct_getter() more verbose and hopefully clearer. DateObject::constructor do this horrible thing where the constructor actually can return NULL. That is the reason for some of it. We should remove that part when the Class constructor is fixed. Do we have a issue on that?

Ah Did not have entity enabled in my sandbox #2123039: Default timezone setting mismatch in metadata wrapper integration -

Testing patch date_getter.patch-2397997-5.patch - was getting Undefined index: field in date_entity_metadata_struct_getter() (line 659) , now getting Illegal offset type in date_entity_metadata_struct_getter() (line 665).

Thank your for testing
That is interesting. How did you test? Grepping through the code and we should have changes all use of them. Do you have code that call this function directly?

The error indicate to me that function date_entity_metadata_struct_getter($item, $name, $info) $name get a wrong data type. If we only had to support PHP 7 we could changed the parameter to function date_entity_metadata_struct_getter(array $item, string $name, array $info) { to enforce the datatypes and provide better error messages.

I'm testing on the test env of one of our production sites (on Pantheon) that makes heavy use of date and its related metadata. Just viewing existing event nodes that have date info. Site default time zone is set to America/Chicago.

ETA: no custom code that calls this directly.

PhP 7.1.23 and Drupal 7.60.

ETA: This error is being generated when these tokens are used in the metatag entry for event nodes. This content type includes the capability for multiple dates:

[node:field-event-date:0:value]
[node:field-event-date:0:value:custom:Y-m-d\TH:i:sO]
(See https://www.drupal.org/project/date/issues/1103032 for use pattern confusion discussion.)

This pattern generates incorrect date/day output, but generates output, along with the above error.

More info on metadata output for this:

Any node created with single date/time returns today's date using [node:field-event-date:0:value:custom:someformat].

More testing results. Here's what I get.

Meta-tags for multi-date field with one date entered (start date, no end date), using Drupal 7.60 with default time zone set to America/Chicago (-0600) and single date entered 01/08/2019 - 7:00pm (that's 08 Jan 2019):

7.x-2.10
[node:field_event_date] works, returning date as formatted by display mode format specification for token
[node:field-event-date:0:value] works, returning 01/08/2019 - 7:00pm as Tue, 01/08/2019 - 7:00pm
[node:field-event-date:0:value:custom:Y-m-d\TH:i:sO] works, returning 01/08/2019 - 7:00pm as 2019-01-08T19:00:00-0600

7.x-2.11-beta1
[node:field_event_date] works, returning date as formatted by display mode format specification for token
[node:field-event-date:0:value] returns 01/08/2019 - 7:00pm as Wed, 01/09/2019 - 1:00am
[node:field-event-date:0:value:custom:Y-m-d\TH:i:sO] returns 01/08/2019 - 7:00pm as 2019-01-09T01:00:00-0600

7.x-2.11-beta1+patch
[node:field_event_date] works, returning date as formatted by display mode format specification for token
[node:field-event-date:0:value] returns nothing
[node:field-event-date:0:value:custom:Y-m-d\TH:i:sO] returns today’s date/time

The issues with 2.11-beta1 and the patch are the loss of the ability to accurately return only a portion of the entered date (eg year only), or return the entered date with a custom format, or return the second date in a multi-date entry.

You might like to review #2921146: Entity metadata getter/setter ignores granularity too.

Just updated to 7.x-2.11 on a site (without any patches), and I'm getting the notices in #6. Using PHP 7.3.

Notice: Undefined index: field in date_entity_metadata_struct_getter() (line 665 of .../date/date.module).

The following would fix it:

  if (!empty($info['field']['settings']['tz_handling'])) {
    $timezone_db = date_get_timezone_db($info['field']['settings']['tz_handling']);
  }
  else {
    $timezone_db = NULL;
  }

I think if we can finish #2074457 we can then look at finishing this.

It looks like #2074457: Error messages from date_entity_metadata_struct_getter() is already committed. The error reported here still persists after applying that patch. It seems like the proposed patch in #12 is a a good solution. Can we take a closer look at this?

I am using the code from #12 on a site except I changed the else to be

if (!empty($info['field']['settings']['tz_handling'])) {
    $timezone_db = date_get_timezone_db($info['field']['settings']['tz_handling']);
  }
  else {
    $timezone_db = 'UTC';
  }

Without it, my field times were off by the site's timezone offset... Not sure if this is right for all, but worked for me. Looking at would date_get_timezone_db() was doing, I always wanted it to return 'UTC'. In my case, when I debugged the error, I was seeing that the $info variable was not coming in with $info['field'] set to anything. I could not tell what was calling this function to begin with. This site has been around for a bit so maybe there is some bad data in the config in the field set up.

It might also be worth adding more test coverage to this so we can verify we don't accidentally break anything.