Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
The granularity option in DateFormatter::formatDiff() is not respected in some cases. If we have weeks as final part of the formatted string (granularity goes from 1 to 0), then days is not added anymore. However, the granularity option is decremented anyway, making it -1.
The problem lies in the fact that the granularity check in the loop only checks for equal to 0.
Proposed resolution
Fix the bug by making sure the loop is terminated if granularity <= 0 instead of granularity == 0.
Remaining tasks
None.
User interface changes
None.
API changes
None.
Original report by @mpdonadio
#2456521: Add DateFormatter::formatDiff() as a non-buggy alternative to DateFormatter::formatInterval() when the start and end of the interval are known introduced DateFormatter::formatDiff(). The logic in it can skip the $granularity parameter in some instances. Demonstration to follow.
Beta phase evaluation
| Issue category | Bug because the granularity option should be checked properly. |
|---|---|
| Issue priority | Major because this bug blocks another major: #2399211: Support all options from views fields in DateTime formatters |
| Prioritized changes | The main goal of this issue is fixing a bug, see issue above. |
| Disruption | None |
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 2504211-01.patch | 1.26 KB | mpdonadio |
| #1 | 2504211-test-only.patch | 825 bytes | mpdonadio |
Comments
Comment #1
mpdonadioPatch with test. Run the test-only patch as
to clearly see the problem. Because of the $granularity decrement in the day portion of DateFormatter::formatDiff(), $granularity can become -1, so the loop continues.
This is currently blocking #2399211: Support all options from views fields in DateTime formatters, hence Major status.
Comment #3
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedSo that is what it was :)
The patch looks good and we have coverage.
Updated IS and added beta eval to summary.
Comment #4
jhodgdonGood one! :) Does this also affect formatInterval() by the way?
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedNo, it does not affect formatInterval(), because weeks are supported out of the box there. So you don't have to do fancy logic to calculate both weeks and days in the same switch case.
Comment #6
jhodgdonAh yes, it's the "PHP only does days and we want weeks" logic that was the problem.
Good then, let's get this one in. Good patch, one line of code changed, with a one-line test that has been demonstrated to fail/pass appropriately. What's not to like? :)
Oh good, you've even got the Beta eval, Blocker tag, and Quick Fix tag.
Comment #7
alexpottCommitted d5d7809 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation to the issue summary.