[AlexSkrypnyk]

Closes #1109

Summary

DateRecurHandler stores a date_recur record's value and end_value exactly as the .feature table gives them again, the way 3.14 did through DrupalDriver's DateRecurHandler. date_recur reads those columns as UTC and computes occurrences in the record's timezone, so a table row gives the stored UTC values plus the timezone, the same columns the database holds.

#1056 made the handler read value and end_value as local times in the record's timezone and convert them to UTC. It was matching a docblock inherited from DrupalDriver, which claimed the stored values were local times, when the code had always stored them as written. Any 3.14 row with a non-UTC timezone, such as 2024-01-01T20:00:00 with Australia/Melbourne, saved fine on 4.x but landed in field_rec_schedule_value as 2024-01-01T09:00:00, so every occurrence moved by the timezone offset and nothing failed.

After this merges, a 3.14 .feature file saves the same database values on 4.x. Instead of a silent shift, the handler now rejects 2 mistakes at the step: a record with a date but no timezone fails with Date recur field record has a date but no "timezone"., and a date outside the storage format, such as 2025-01-01T09:00:00Z or relative:+1 day, fails with a message naming the format. A record with no dates still clears the field, rrule and infinite are still relayed, and no other field handler changes.

Before / After

.feature row:  value 2024-01-01T20:00:00 | timezone Australia/Melbourne
Site timezone: Australia/Perth

                 field_rec_schedule_value   first occurrence
                 ────────────────────────   ──────────────────────────
3.14             2024-01-01T20:00:00        2 Jan, 07:00 Melbourne
4.x before PR    2024-01-01T09:00:00        1 Jan, 20:00 Melbourne
4.x after PR     2024-01-01T20:00:00        2 Jan, 07:00 Melbourne

Edge cases

┌──────────────────────────────┬─────────────────────────────┬─────────────────────────────────────┐
│ .feature row                 │ BEFORE (4.x)                │ AFTER                               │
├──────────────────────────────┼─────────────────────────────┼─────────────────────────────────────┤
│ date, no timezone column     │ takes the site timezone,    │ fails: Date recur field record has  │
│                              │ value read in site time     │ a date but no "timezone".           │
├──────────────────────────────┼─────────────────────────────┼─────────────────────────────────────┤
│ 2025-01-01T09:00:00Z         │ offset honored, stored      │ fails: ... is not in the storage    │
│                              │ 09:00                       │ format "Y-m-d\TH:i:s".              │
├──────────────────────────────┼─────────────────────────────┼─────────────────────────────────────┤
│ relative:+1 day              │ resolved in record timezone │ fails: same storage format message  │
├──────────────────────────────┼─────────────────────────────┼─────────────────────────────────────┤
│ value and end_value empty    │ relayed, field cleared      │ relayed, field cleared              │
└──────────────────────────────┴─────────────────────────────┴─────────────────────────────────────┘

Changes

date_recur handler

• src/Backend/Drupal/Core/Field/DateRecurHandler.php extends AbstractHandler again instead of DatetimeHandler, and doExpand() relays value and end_value as given, turning an empty one into NULL.
• assertRecordTimezone() rejects a record with a date and an empty or missing timezone, and keeps the invalid timezone "..." message for a name PHP doesn't recognize.
• assertStorageFormat() parses each date with the storage format of the field's datetime_type (Y-m-d\TH:i:s, or Y-m-d for a date-only field) and rejects a value that doesn't parse or doesn't format back identically, which covers offsets, Z, a space separator, relative: and out-of-range dates such as 2025-02-30.

Tests

• DateRecurHandlerTest drops the site timezone container setup and covers relayed records, empty dates, every rejection and the date-only format (31 cases).
• DateRecurHandlerKernelTest saves stored UTC values and asserts date_recur computes the occurrences at 09:00 and 17:00 Sydney time, and asserts that a record without a timezone fails at createEntity().

Docs

• MIGRATION.md replaces the date_recur conversion entry with the 3 failure messages a 3.14 project can now hit.
• Row H13 of src/Backend/Drupal/Core/Field/README.md describes the relay and the 2 checks.
• docs/content/architecture/class-inheritance.drawio and its 2 SVGs move DateRecurHandler from under DatetimeHandler into the "extend AbstractHandler directly" box.

Summary

• DateRecurHandler now extends AbstractHandler and relays value and end_value without timezone conversion. Empty date values become NULL.
• Records with dates must provide a valid timezone and values in the field’s storage format. Records without dates remain unchanged. rrule and infinite are still relayed.
• Updated tests, migration guidance, the handler README, and the inheritance diagram.

Linked issue

Issue #1109 requests an opt-in storage-value mode while keeping the existing conversion behavior as the default. This change makes relaying values the default, so it does not preserve the issue’s requested default behavior.

Contribution rules

No step definitions changed. The step definition rules in CONTRIBUTING.md do not apply to these changes.

Validation

Test results were not provided.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: ASSERTIVE
• Plan: Team
• Run ID: 4e3b3916-5c06-4d67-84f8-228afff2e85d

📥 Commits

Reviewing files that changed from the base of the PR and between ecd9336 and 8416a80.

⛔ Files ignored due to path filters: 2

• docs/content/architecture/class-inheritance.dark.svg is excluded by !**/*.svg
• docs/content/architecture/class-inheritance.svg is excluded by !**/*.svg

📒 Files selected for processing: 6

• MIGRATION.md
• docs/content/architecture/class-inheritance.drawio
• src/Backend/Drupal/Core/Field/DateRecurHandler.php
• src/Backend/Drupal/Core/Field/README.md
• tests/phpunit/src/Kernel/Backend/Drupal/Core/Field/DateRecurHandlerKernelTest.php
• tests/phpunit/src/Unit/Backend/Drupal/Core/Field/DateRecurHandlerTest.php

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

Walkthrough

DateRecurHandler now extends AbstractHandler and relays value and end_value as written. Dated records require a valid timezone and values that match the field’s storage format. The migration guide, handler documentation, inheritance diagram, and tests reflect these changes.

Changes

Date recurrence handling

Layer / File(s)	Summary
Handler validation and storage behavior src/Backend/Drupal/Core/Field/DateRecurHandler.php, src/Backend/Drupal/Core/Field/README.md, MIGRATION.md, docs/content/architecture/class-inheritance.drawio	DateRecurHandler now extends AbstractHandler. It validates the timezone and present date values against the field’s storage format, normalizes empty date columns to NULL, and relays values without timezone conversion. The documentation and diagram describe the updated behavior.
Handler behavior tests tests/phpunit/src/Unit/Backend/Drupal/Core/Field/DateRecurHandlerTest.php, tests/phpunit/src/Kernel/Backend/Drupal/Core/Field/DateRecurHandlerKernelTest.php	Unit tests cover unchanged stored values and rejection of missing or invalid timezones and values outside the storage format. Kernel tests cover UTC values read in the record timezone and rejection of a missing timezone.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 8416a

The date_recur handler now stores value and end_value as written, as in 3.14, and rejects dated records that lack a valid timezone or use the wrong date format. The migration guide and tests cover this. No merge-blocking risk was found. The maintainers should know it departs from the opt-in approach suggested in issue #1109.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		Issue #1109 requests an opt-in that writes date_recur value and end_value as stored UTC values. It states that the default stays as #1056 decided. #1056 makes DateRecurHandler read both values as loca…	Keep the #1056 behavior as the default. The default DateRecurHandler path must read value and end_value as local times in the record timezone and convert them to UTC, and the missing-timezone site fallback must remain. Add one opt-in path t…
Docstring Coverage		Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (3 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the two main behaviors in the changeset: stored 'date_recur' values are relayed as written, and records with no 'timezone' or a non-storage date are rejected. The changeset does both (…
Out of Scope Changes check		All reviewable changed files relate to the date_recur handler that #1109 concerns. These files are src/Backend/Drupal/Core/Field/DateRecurHandler.php, DateRecurHandlerTest.php and DateRecurHandlerKern…

Full details: Title check

Explanation

The title names the two main behaviors in the changeset: stored 'date_recur' values are relayed as written, and records with no 'timezone' or a non-storage date are rejected. The changeset does both (DateRecurHandler.php, README, MIGRATION.md, tests). The title does not mention the change from timezone conversion to relay as the default, and the linked issue #1109 asked for an opt-in. That gap concerns the objectives, not the title, so the title itself is accurate.

Full details: Out of Scope Changes check

Explanation

All reviewable changed files relate to the date_recur handler that #1109 concerns. These files are src/Backend/Drupal/Core/Field/DateRecurHandler.php, DateRecurHandlerTest.php and DateRecurHandlerKernelTest.php, the date_recur row in src/Backend/Drupal/Core/Field/README.md, MIGRATION.md, and docs/content/architecture/class-inheritance.drawio. The diagram change follows the parent-class change in DateRecurHandler. No change touches another field handler, according to the PR description and summary. The change of default behavior is related to #1109 and is not an unrelated change. Its conflict with the issue requirement is assessed under linkedIssuesAssessment. The class-inheritance.svg and class-inheritance.dark.svg files are listed in ignored_files. They were not inspected and do not affect this result.

Full details: Linked Issues check

Explanation

Issue #1109 requests an opt-in that writes date_recur value and end_value as stored UTC values. It states that the default stays as #1056 decided. #1056 makes DateRecurHandler read both values as local times in the record timezone and convert them to UTC. The PR does not add an opt-in. Its summary shows no handler option, no second handler class registered through drupal.field_handlers, and no per-record marker such as an @utc suffix or a field:value_utc column. Instead, DateRecurHandler extends AbstractHandler, and doExpand() relays value and end_value unchanged for every record. The #1056 conversion is removed from the default path. The missing-timezone site fallback is also removed, and records with a date and no timezone now fail. The default behavior therefore changes, which #1109 says must not happen. The PR description says it closes #1109, but it implements the reverse of the requested opt-in and default-preserving design. The kernel test testStoredUtcValuesAreReadInTheRecordTimezone covers only the relay path. No test covers a preserved #1056 default path or an opt-in path.

Resolution

Keep the #1056 behavior as the default. The default DateRecurHandler path must read value and end_value as local times in the record timezone and convert them to UTC, and the missing-timezone site fallback must remain. Add one opt-in path that relays stored UTC values unchanged. Use one of the mechanisms named in #1109: a handler option such as drupal.field_handler_options.date_recur.values_in_storage_timezone, a second shipped handler class such as DateRecurStorageHandler, or a per-record marker. Add unit and kernel tests for the default conversion path and for the opt-in relay path. Update MIGRATION.md, the README row H13, and the PR description so they describe the opt-in and the unchanged default.

Full details: Docstring Coverage

Explanation

Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks the dates in line,
And keeps each stored time as written.
A named zone must be supplied,
Wrong-shaped dates are turned aside.
UTC bounds meet schedules bright,
Then hop along into the night.}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🚀 Deployed on https://6adafd692660c350f9247edf--behat-steps-docs.netlify.app

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.23%. Comparing base (ecd9336) to head (8416a80).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1111   +/-   ##
=======================================
  Coverage   99.23%   99.23%           
=======================================
  Files         180      180           
  Lines        9592     9596    +4     
=======================================
+ Hits         9519     9523    +4     
  Misses         73       73           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.