Skip to content

[IT9PE1-33922] Accept backward-compatible timezone aliases - #99

Merged
fguimier merged 2 commits into
mainfrom
florian_33922_timezone_accept_bc_identifiers
Nov 5, 2026
Merged

fguimier merged 2 commits into
mainfrom
florian_33922_timezone_accept_bc_identifiers

Conversation

@fguimier

@fguimier fguimier commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Why

\DateTimeZone accepts backward-compatible aliases such as US/Hawaii, but TimezoneValidator only accepted \DateTimeZone::listIdentifiers(), which leaves them out. A value PHP builds without complaint could therefore be stored, then fail the constraint on every later validation of the entity holding it.

What

  • TimezoneValidator checks against \DateTimeZone::listIdentifiers(\DateTimeZone::ALL_WITH_BC).
  • New test: an alias taken from the platform's own timezone database is accepted. It picks the alias at runtime because system tzdata builds differ (US/Hawaii is missing from some), and it is skipped when the platform has none.
  • New invalid case US/Atlantis keeps the rejection path covered.

Test plan

  • phpunit, phpcs, phpstan green locally (PHP 8.4)
  • The new test fails without the fix (0 violation expected. Got 1.)
  • CI matrix (PHP 8.4 / 8.5, lowest / highest)

Shipping

Merging this PR ships nothing: it needs a semver tag / GitHub release, then a
composer require assoconnect/validator-bundle:^2.51 bump PR in the backend. Two reviews total.

Refs IT9PE1-33922

🤖 Generated with Claude Code

Florian Guimier and others added 2 commits November 5, 2026 14:22
\DateTimeZone accepts aliases such as US/Hawaii, so a value it built can
be stored and then fail the Timezone constraint on every later update.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nt (review gate finding)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fguimier

fguimier commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Review gate — 0 error / 0 open warning / 1 info

Human review needed: no — one-line widening of the allow-list to ALL_WITH_BC, covered by a test that fails on revert and ran on all 4 CI jobs; can be merged without a peer review.

🔍 Reviewed at eea6221

🔧 Fixed before this verdict (1) — in eea6221, checked by the delta review:

  • src/Validator/Constraints/TimezoneValidator.php:33 — The added comment gave the reason for the change; it now keeps only the lasting constraint.

🧪 Coverage: 1/1 behavior changes covered
🎯 Hotspot thresholds: p99 = 22 commits / none fixes (12 months)

⚠️ Self-review by the author is mandatory before merging, regardless of this verdict.

@sonarqubecloud

sonarqubecloud Bot commented Nov 5, 2026

Copy link
Copy Markdown

@fguimier
fguimier merged commit 20165d1 into main Nov 5, 2026
5 checks passed
@fguimier
fguimier deleted the florian_33922_timezone_accept_bc_identifiers branch November 5, 2026 13:45
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.

1 participant