Skip to content

Fix Week formatting issue when date range spans across two years #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lbeauvisage
Copy link

This PR addresses a bug occurring when:
• The selected interval is "week",
• The date range spans across two years (e.g. from December 23, 2024 to January 12, 2025).

Problem

In such a case, PHP’s $date->format('Y-W') can return “2024-01” for the week starting on Monday, December 30, 2024 and ending on Sunday, January 5, 2025.
Although the date is still in 2024, the ISO week number is 1, which typically refers to the first week of the following year. This leads to inconsistent keys when compared to those returned by MySQL.

Example returned by mapValuesToDates():

[
  "2024-01",  // Incorrect value from $placeholders
  "2024-52",
  "2024-53",  // Week properly returned by MySQL
  "2025-01",
  "2025-02"
]

Quick test :

collect(CarbonPeriod::between(
                '2024-12-23',
                '2025-01-12',
            )->interval("1 week"))
                ->map(function ($date) {
                    return $date->format('Y-m-d') . ' - ' . $date->format('Y-W');
                })

Fix

This PR ensures $placeholders uses consistent week-year formatting that aligns with ISO 8601 and MySQL outputs.
Now, for a week that starts on Dec 30, 2024, the returned key will be “2024-53”, not “2024-01”.

⚠️ Note on MySQL behavior

It is worth noting that MySQL may return two distinct keys for the same week: "2024-53" and "2025-01".
As a result, we end up with two items in the collection for what is essentially the same week.

This is particularly problematic when rendering data in line charts, as it leads to data duplication and visual inconsistencies.

➡️ A separate pull request will be created to address this specific issue.

@benjamindavid
Copy link

Thanks for the fix, hoping for it to be merged 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants