Skip to content

Add DateTime property type#685

Merged
JeroenDeDauw merged 15 commits intomasterfrom
datetime-property-type
Apr 16, 2026
Merged

Add DateTime property type#685
JeroenDeDauw merged 15 commits intomasterfrom
datetime-property-type

Conversation

@malberts
Copy link
Copy Markdown
Collaborator

For #678

Result of back and forth with @malberts.
Context: the NeoWiki codebase, existing property type implementations, and RedHerb test extension.
Written by Claude Code, Opus 4.6

Summary

  • Adds DateTimeType and DateTimeProperty to RedHerb test extension (PHP backend), proving the plugin system works for non-trivial property types with constraints
  • Adds TypeScript DateTimeType with ISO 8601 validation and min/max constraint checking
  • Adds Vue components: DateTimeDisplay, DateTimeInput (native datetime-local), DateTimeAttributesEditor
  • Registers in frontend property type and component registries
  • Integration tests via RedHerb hook + unit tests for DateTimeProperty serialization

Still to do

  • The frontend components are currently registered directly in NeoWiki core (Neo.ts, NeoWikiExtension.ts) rather than via RedHerb's plugin system. A frontend extension mechanism (mw.hook-based registration) is needed so external extensions can register property type components without rebuilding NeoWiki. This will be addressed in a follow-up.

Test plan

  • PHP unit tests pass (DateTimePropertyTest)
  • PHP integration tests pass (NeoWikiRegistrationHookTest — skipped when RedHerb not loaded)
  • TypeScript unit tests pass (DateTime.spec.ts — 11 tests)
  • Full make tsci passes (test + build + lint)
  • Browser verified: "Date & Time" appears in schema editor type dropdown with min/max datetime pickers

🤖 Generated with Claude Code

@JeroenDeDauw
Copy link
Copy Markdown
Member

JeroenDeDauw commented Apr 16, 2026

@malberts in which ways is this still draft? I'd be good to have an MVP version of the type soon. UIs and validation can be skipped for now, and ideally we do not block on figuring out the TS topic and do that via another task.

@malberts
Copy link
Copy Markdown
Collaborator Author

It is draft, because the UI part is still hardcoded in NeoWiki, not RedHerb. However, since the plan was to eventually move this back into core anyway, I'll rebase this to unblock. It just means we're skipping #678 (comment).

malberts and others added 12 commits April 16, 2026 15:34
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove trailing commas and fix self-closing HTML void elements.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Compare timestamps instead of raw strings for min/max validation,
making comparison robust across different ISO 8601 formats. Display
dates in UTC to match the UTC storage convention.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@malberts malberts force-pushed the datetime-property-type branch from d0ad125 to c40fc0b Compare April 16, 2026 13:42
malberts and others added 3 commits April 16, 2026 16:11
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Without this, the clock icon for the DateTime property type was not
loaded by MediaWiki's ResourceLoader, showing no icon in the dropdown.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@malberts
Copy link
Copy Markdown
Collaborator Author

datetime-icon-fixed datetime-attributes-editor-fixed datetime-subject-editor datetime-infobox-display

@malberts malberts marked this pull request as ready for review April 16, 2026 14:51
@alistair3149
Copy link
Copy Markdown
Member

alistair3149 commented Apr 16, 2026

Result of a code review pass with @alistair3149.
Context: the NeoWiki codebase, Number property type patterns (especially the recent min/max cross-validation in acccff5), ECHOLOT linked-data goals, and Codex design system conventions.
Written by Claude Code, Opus 4.7

The critical issues are blockers. All the rest can be done in a follow-up.

Critical

Timezone handling is silently broken

DateTimeAttributesEditor.vue:41-49 and DateTimeInput.vue:54-64 treat the datetime-local input as UTC:

function toLocalInputValue( isoString ) { return isoString.replace( /Z$/, '' ).slice( 0, 16 ); }
function fromLocalInputValue( localValue ) { return localValue ? localValue + ':00Z' : undefined; }

A user in Berlin entering "2025-06-15 14:00" has it stored as 2025-06-15T14:00:00Z, which is actually 16:00 Berlin time — data comes back two hours off. ISO strings with explicit offsets (...+02:00) fail the Z$ regex, get sliced, and silently re-encoded as UTC. The :min/:max bounds on the input use the same broken conversion.

Pick one story — UTC-throughout (with a visible "UTC" label) or user-local-with-UTC-storage — and apply it consistently across Input/Display/AttributesEditor. Add a test that runs under a non-UTC TZ to pin the behavior.

Date.parse is too lenient

DateTime.ts:32. Date.parse( '2025' ), Date.parse( '2025-06' ), Date.parse( '2025-02-30' ) all return valid timestamps in various engines. Timezone-less strings are interpreted as local time per spec, so the min/max comparison mixes local and UTC epochs depending on input. For ECHOLOT's xsd:dateTime round-tripping this matters — gate with a regex (or stricter parser) before Date.parse, and require stored values carry an explicit offset or Z.

DateTimeProperty (PHP) also accepts any ?string for default/minimum/maximum with no shape check.

Missing component tests

No DateTimeInput.spec.ts, DateTimeAttributesEditor.spec.ts, or DateTimeDisplay.spec.ts. Every other value type has them (NumberInput.spec.ts, TextInput.spec.ts, UrlInput.spec.ts, RelationInput.spec.ts, etc.). This is the component most likely to break on timezone edge cases and there's zero component-level coverage. Mirror NumberInput.spec.ts at minimum, including a TZ-pinned round-trip test.

Important

No min/max cross-validation

Number got minExceedsMax cross-validation in acccff5. DateTimeAttributesEditor.vue has no equivalent — a user can set minimum: '2030-01-01T00:00:00Z', maximum: '2020-01-01T00:00:00Z' without feedback and the backend accepts it. Refactor minExceedsMax to be value-agnostic (accept a comparator) and reuse across types. The existing neowiki-property-editor-min-exceeds-max message can be reused.

Use <time> for semantic rendering

DateTimeDisplay.vue:2-4 renders into a <div>. Switch to:

<time :datetime="dateString">{{ formattedValue }}</time>

Keeps the canonical ISO value machine-readable in the DOM while showing a formatted string to users. Aligns with ECHOLOT's linked-data goals (CIDOC-CRM, xsd:dateTime, RDF export) and lets assistive tech announce it as a time. Helps mitigate the "UTC invisible to user" issue too — scrapers and ATs get the precise value regardless of visible formatting. Only emit the datetime attribute when the string parses as valid ISO 8601; on the invalid fallback, render a plain <span>.

DateTimePropertyTest doesn't extend PropertyTestCase

DateTimePropertyTest.php:13 extends bare TestCase and hand-rolls a serialization round-trip. NumberPropertyTest and others extend PropertyTestCase, which exercises the PropertyDefinition::fromJson path production uses. Switch to PropertyTestCase (or move the helpers to a shared utility if the namespace blocks direct reuse).

Missing invalid-type tests

NumberPropertyTest::testExceptionOnInvalidMax exists; DateTimePropertyTest has no equivalent. Add testExceptionOnInvalidMinimum / testExceptionOnInvalidMaximum that pass numbers/booleans/objects and assert TypeError.

DateTimeDisplay.vue UTC indicator

DateTimeDisplay.vue:28: formatted via toLocaleString( undefined, { timeZone: 'UTC' } ), so "2025-06-15, 12:00:00 PM" looks like local time with no indication. Either add a visible UTC suffix, format as ISO, or use the user's timezone. Whatever the choice, make it visible. Partially resolved by the <time> suggestion above, but users still see misleading visible text.

Min/max inclusivity undocumented

Tests assert inclusive bounds (DateTime.spec.ts:105), but docs/qqq say nothing. For partners needing xsd:dateTime semantics this matters — document in qqq or a docblock.

Empty-string value handling

DateTime.ts:35-37: newStringValue('') yields { parts: [''] }, which hits Date.parse('') → NaN → invalid-datetime. Emitter is correct by accident. Consider treating empty as "no value" to match UrlType.validate, and add an explicit test.

Bounds tests too permissive

DateTime.spec.ts:61-67 tests only one value inside a wide range; :105 tests a value equal to both min and max. Both would pass a trivial return [] implementation. Add "1 ms above min" and "1 ms below max" cases.

Minor

Type name inconsistency

DateTimeType::NAME = 'dateTime' (camelCase) but i18n key is neowiki-property-type-datetime (lowercase), and existing types are single-word lowercase (number, url, relation, string, boolean). Might be more consistent if we normalize to datetime.

Use CdxTextInput input-type="datetime-local"

DateTimeInput.vue uses a raw <input> instead of Codex's wrapper. Loses focus ring, validation states, and the deliberately registered cdxIconClock start-icon never appears. Two eslint-disable vue/html-self-closing comments hint at the same friction. CdxTextInput supports datetime-local as an input-type. Same applies to DateTimeAttributesEditor.vue.

i18n/qqq ordering

New entries are appended to the end of extension.json (:265, :276) and i18n/qqq.json, rather than slotted into their alphabetically-clustered siblings.

PropertyDefinitionDeserializer.unit.spec.ts not updated

Every other property type is tested through the deserializer path there. Add DateTime.

@covers tag coverage

DateTimePropertyTest::testBuildPropertyDefinitionFromJsonViaType exercises DateTimeType too — add a second @covers \ProfessionalWiki\RedHerb\DateTimeType or split the test class.

modelValue default divergence

DateTimeInput.vue:34 uses modelValue: undefined while NumberInput.vue:42 uses modelValue: () => newNumberValue( NaN ). Pick one convention for the next property type to copy.


@JeroenDeDauw
Copy link
Copy Markdown
Member

All of that can be done in follow-ups

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.

3 participants