Problem/Motivation

I discovered this issue while upgrading from php7 to php8.

dump(5 > OfficeHoursDateHelper::format("00:00", 'G', TRUE)); gives different result on php7 compared to php8.

Steps to reproduce

use OfficeHoursDateHelper::format with the format parameter 'G'

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 office_hours_3266562_Add Endhours Test.patch1.21 KBjohnv

Issue fork office_hours-3266562

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikran created an issue. See original summary.

mikran opened merge request !10

mikran’s picture

mikran CreditAttribution: mikran at Factorial GmbH commented
Status: Active » Needs review

Please review, old behavior is kept as fallback if requested format is something else than 24 hour time format. It does not make any sense to return "24:00" for those but hopefully the tests bring some stability to it.

  • johnv committed 6786429 on 8.x-1.x 
    Issue #3266562 by mikran: OfficeHoursDateHelper::format does not apply...
johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented
Version: 8.x-1.x-dev » 8.x-1.5
Component: Code » Code - formatter
Status: Needs review » Fixed

Thanks, committed.
Not sure why/where this would go wrong. I have no PHP8, yet.
I did keep the (mb_strlen($time_format) == strspn($time_format, 'GH:i ')) part to avoid the fallback (only for 24:00 not 12AM).

mikran’s picture

mikran CreditAttribution: mikran at Factorial GmbH commented

I did keep the (mb_strlen($time_format) == strspn($time_format, 'GH:i ')) part to avoid the fallback (only for 24:00 not 12AM).

tests need to be updated accordinly, there are couple that are now failing

johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented
Status: Fixed » Needs work
johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented
Version: 8.x-1.5 » 8.x-1.6
FileSize
office_hours_3266562_Add Endhours Test.patch1.21 KB

Correcting the test.

  • johnv committed c6c5453 on 8.x-1.x 
    Issue #3266562 by mikran, johnv: Fix UnitTest for OfficeHoursDateHelper...
johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

johnv closed merge request !10