Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

feat(calculations): dateDiff primitive (closes scholiq deps #5)#1476

Merged
1 commit merged into
developmentfrom
feat/scholiq-deps/date-diff
May 15, 2026
Merged

feat(calculations): dateDiff primitive (closes scholiq deps #5)#1476
1 commit merged into
developmentfrom
feat/scholiq-deps/date-diff

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

  • Adds dateDiff JSON-logic primitive to CalculationEvaluator per openregister#1470 §5
  • Extends CalculationAnnotationValidator with dateDiff-specific key and unit validation
  • 42 new unit tests covering all 7 units, edge cases, and validator integration

What was added

CalculationEvaluator::dateDiff()

  • Dict-based syntax: { "dateDiff": { "from": "...", "to": "...", "unit": "..." } }
  • Supported units: years, months, weeks, days, hours, minutes, seconds
  • from/to accept: literal "now" (current server time), ISO-8601 strings, @self.<field> references, or prop() expressions
  • Returns a signed integer (positive when to > from)
  • Calendar units (years/months) use DateInterval for leap-year / variable-month accuracy
  • Sub-day/week units use timestamp delta — consistent across DST transitions
  • Returns null (not an exception) when either operand cannot be parsed as a date

CalculationAnnotationValidator

  • dateDiff added to VALID_OPS
  • walkDateDiff() validates required keys (from, to, unit) and rejects unknown unit literals at schema-save time

Test coverage (42 new tests)

Area Tests
Positive/negative diff per unit (all 7 units) 14
Same-date zero 1
Leap year days (through Feb 29) 2
End-of-month months 3
Partial-week truncation 1
Leap year year boundaries 2
DST boundary (days/seconds) 2
@self field + prop() references 2
"now" sentinel (from and to) 2
Null propagation (null, unparseable, missing prop) 3
Error cases (invalid unit, missing keys) 5
Validator integration (valid, invalid unit, missing keys, unknown prop) 4
Cross-year month 1

Quality gates

  • PHPCS (lib/ only, --warning-severity=0): clean
  • Psalm: no errors
  • PHPUnit (64 tests, 86 assertions): all pass

Review notes

DO NOT admin-merge — openregister is production-tier. Awaiting code review.

Implements the `dateDiff` JSON-logic primitive in CalculationEvaluator
and CalculationAnnotationValidator per openregister#1470 §5.

- Supports 7 units: years, months, weeks, days, hours, minutes, seconds
- Dict-based syntax: { "dateDiff": { "from": "...", "to": "...", "unit": "..." } }
- "now" sentinel, @self.<field> references, and prop() refs are all accepted
- Calendar units (years/months) use DateInterval for leap-year accuracy
- Sub-day/week units use timestamp delta so DST transitions are consistent
- Returns null (not exception) when either operand is an unparseable date
- Validator checks required keys and rejects unknown unit literals at schema-save time
- 42 new unit tests covering positive/negative diffs for all 7 units,
  leap years, end-of-month months, DST boundaries, null propagation,
  @self refs, "now" sentinel, and validator integration
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 44d4f7f

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 153/153
npm ✅ 598/598
PHPUnit
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-05-11 19:33 UTC

Download the full PDF report from the workflow artifacts.


return match ($unit) {
'weeks' => (int) intdiv($deltaSecs, 604800),
'days' => (int) intdiv($deltaSecs, 86400),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CONCERN] DST spring-forward gives 0 days for a 23-hour local-timezone day

When inputs are date-only strings (e.g. '2026-03-29' / '2026-03-30') and the server is in a DST-observing timezone (e.g. Europe/Amsterdam), new DateTimeImmutable('2026-03-29') parses to midnight local time. The spring-forward day is only 82800 seconds long, so intdiv(82800, 86400) = 0 — the user gets 0 days instead of 1. Fix: normalise both operands to UTC before computing the timestamp delta for calendar-day units, or add a setTimezone(new DateTimeZone('UTC')) call inside toDateOrNull before the delta computation.

$toRaw = $this->evaluate(object: $object, expression: $args['to']);

// Treat the special sentinel "now" (literal string) as current time.
if ($fromRaw === 'now') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CONCERN] "now" sentinel creates a server-local-timezone object; no UTC normalisation documented

new DateTimeImmutable('now') uses the PHP process's default timezone. If the other operand is a UTC string, the two objects have different timezone contexts. For months/years (DateInterval path) diff() is timezone-aware and the result can be off by 1 month on a timezone boundary. Document that callers must supply explicit UTC offsets, or normalise both operands to UTC before calling diff().


// -----------------------------------------------------------------------
// Annotation validator integration
// -----------------------------------------------------------------------

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CONCERN] DST test uses UTC offsets — does not cover the real DST risk (server-tz date-only strings)

testDstSpringForwardDays uses +00:00 explicit-offset strings, which sidestep the DST issue entirely. The scenario that actually triggers the bug — dateDiff({from:'2026-03-29', to:'2026-03-30', unit:'days'}) on a Europe/Amsterdam server — is not tested. Add a test that passes date-only strings across the spring-forward boundary or document that date-only strings are UTC-assumed.

return $sign * $interval->y;
}

return $sign * ($interval->y * 12 + $interval->m);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CONCERN] months formula $interval->y * 12 + $interval->m truncates; Jan 31 → Feb 28 = 0 months

PHP DateInterval::diff('2026-01-31', '2026-02-28') yields y=0, m=0, d=28 — the user gets 0 months instead of 1. This is tested and documented in testEndOfMonthJanToFeb, but may surprise API consumers. Consider documenting this explicitly in the expression schema or accepting it as intentional PHP semantics.


// Validate unit when it's a plain string literal (not a nested expression).
$unit = $args['unit'];
if (is_string($unit) === true && in_array($unit, self::VALID_DATE_DIFF_UNITS, true) === false) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CONCERN] Validator skips unit validation when unit is a dynamic expression

In walkDateDiff, if (is_string($unit) === true …) only validates literal string units — a nested expression like {'prop': 'unitField'} passes validation entirely unchecked. At runtime, CalculationEvaluator casts to string and validates, which throws. Consider adding a warning-level error or a note in the docblock.

@WilcoLouwerse WilcoLouwerse left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

🟡 Concerns (5)

🟢 Minor (3)

  • Docblock claims epoch-delta is DST-safe for all sub-day units; this overstates the guarantee (lib/Service/Calculation/CalculationEvaluator.php:582)
    The docblock says 'elapsed-second delta so DST transitions are handled consistently'. This is only true when both inputs carry an explicit UTC offset. Date-only strings parsed in a local DST timezone produce a delta that differs from the wall-clock calendar day count. Revise to clarify that date-only strings are parsed in server local timezone.
  • testAtSelfFieldReference uses a non-standard @self object shape (tests/Unit/Service/Calculation/DateDiffTest.php:696)
    $object = ['@self' => ['dueDate' => '2026-06-01']] — verify this matches the real runtime shape that CalculationEvaluator::propValue expects for @self.* references; if the shape differs in production the test gives false confidence.
  • testNowSentinel will fail in year 2099 — use a relative assertion or freeze time (tests/Unit/Service/Calculation/DateDiffTest.php:737)
    $this->assertGreaterThan(0, $result) with a hardcoded '2099-12-31' target date will eventually become ≤ 0. Prefer injecting a clock stub or asserting relative to a controlled date.

Reviewed by WilcoLouwerse via automated batch review.

@WilcoLouwerse WilcoLouwerse closed this pull request by merging all changes into development in f86e83f May 15, 2026
@WilcoLouwerse WilcoLouwerse deleted the feat/scholiq-deps/date-diff branch May 15, 2026 09:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants