[PER-10643] Add edtf date and time validation by aasandei-vsp · Pull Request #1056 · PermanentOrg/web-app

aasandei-vsp · 2026-06-11T12:47:43Z

Manual test cases — Date/time input inline validation

Setup: log in, upload a file, click it.
EXPECTED: sidebar with a Date section. Open the edit modal via "More options" for each test.

Invalid characters

Type 19ab in the year, then ab in the hours.
- EXPECTED: The values stay visible (not silently reverted), with "The date contains invalid characters." / "The time contains invalid characters." below the inputs and red styling.
Clear the inputs.
- EXPECTED: Errors disappear.

Out-of-range month

Type 13 in the month input.
- EXPECTED: The value stays visible and "Month must be between 1 and 12." appears below the input.
Change the month to 12.
- EXPECTED: The error disappears.

Out-of-range day

Enter year 2026, month 04, then type 31 in the day input.
- EXPECTED: "Day must be between 1 and 31." appears below the input.
Change the day to 30.
- EXPECTED: The error disappears.

Out-of-range hours (AM/PM mode)

With the format toggle on AM or PM, type 13 in the hours input.
- EXPECTED: The value stays visible and "Hour must be between 1 and 12." appears below the time input.
Change the hours to 10.
- EXPECTED: The error disappears.

Out-of-range minutes

Type 60 in the minutes input.
- EXPECTED: "Minutes must be between 0 and 59." appears below the time input.
Change the minutes to 59.
- EXPECTED: The error disappears.

Out-of-range seconds

Type 60 in the seconds input.
- EXPECTED: "Seconds must be between 0 and 59." appears below the time input.
Change the seconds to 59.
- EXPECTED: The error disappears.

No error while typing a single digit

Type a single 5 in the month or 9 in the day.
- EXPECTED: No error, and the EDTF preview shows it zero-padded (2026-05, not 2026-5X).

Day re-validated when the month changes

Enter 2026 / 01 / 31 (no error), then change the month to 04.
- EXPECTED: The day error appears without touching the day input.
Similarly, Feb 29 errors in 2025 but not in 2024.

Hour re-validated when the format changes

In 24H mode type hours 15 (no error), then toggle to AM.
- EXPECTED: "Hour must be between 1 and 12." appears; toggling back to 24H clears it.
In 24H mode type 24.
- EXPECTED: "Hour must be between 0 and 23."

Calendar selection clears errors

With a month error showing, pick a date from the calendar dropdown.
- EXPECTED: Inputs are filled and the error clears.

Time without a complete date

Enter year 1985, month 05, no day, and a time of 10:30 AM.
- EXPECTED: "A complete date is required when time is provided." appears and Save is disabled.
Fill in the day.
- EXPECTED: Error clears, preview reads 1985-05-20T10:30:00.

codecov · 2026-06-11T12:49:23Z

Codecov Report

❌ Patch coverage is 88.03419% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.98%. Comparing base (44e1116) to head (67edbff).

Files with missing lines	Patch %	Lines
...nts/datepicker-input/datepicker-input.component.ts	81.81%	3 Missing and 3 partials ⚠️
...nts/timepicker-input/timepicker-input.component.ts	87.87%	2 Missing and 2 partials ⚠️
...c/app/shared/services/edtf-service/edtf.service.ts	90.47%	0 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1056      +/-   ##
==========================================
+ Coverage   51.60%   51.98%   +0.37%     
==========================================
  Files         354      354              
  Lines       12067    12127      +60     
  Branches     2172     2183      +11     
==========================================
+ Hits         6227     6304      +77     
+ Misses       5613     5599      -14     
+ Partials      227      224       -3

☔ 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.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

aasandei-vsp · 2026-06-15T12:28:05Z

@cecilia-donnelly @slifty I have managed to rebase this one on the branch that's been checked out from too :D

cecilia-donnelly · 2026-06-16T18:47:51Z

Thanks for the branch management! For others reading this (QA), this needs a feature flag to be turned on: edtf-date.

cecilia-donnelly

I love the way this is looking! The "sometime before"/"sometime after" language for an open ended range is very cool.

I did see that if I enter February 29 in a non-leap-year, the error is "Day must be between 1 and 31" which is not exactly what we want to express there. I'm not sure what this should be, but maybe @omnignorant knows.
If I leave both start and end dates empty and indicate that this is a range then I get "sometime before .." which isn't exactly right either
If I have an open range (no start or end dates), then remove the "range" indicator, the new value is an empty string which gets an error from the backend. Maybe that's fine, but maybe we should instead send "Unknown"? QA/design question.
the sidebar behaves slightly differently from the modal for invalid data. The sidebar allows you to click "save" with only a time filled (no date) while the modal blocks that path. The time-only value doesn't save, but in the sidebar the user gets a "We could not update" error in a toast instead of the nicer inline "A complete date is required when time is provided."

None of these are showstoppers, but if possible would be nice to fix!

aasandei-vsp · 2026-06-29T09:45:53Z

When the date and time fields are undefined, we send null to the BE PATCH for the displayTime property and then we show the date field empty in the sidebar.
The problem is, the displayTime is set to null even if it has never been set by the user, so we cannot rely on the displayDT fallback.
@cecilia-donnelly How should we deal with this situation? Right now, if we just remove the displayDT fallback for the sidebar, the list shows the displayDT and it looks inconsistent.

slifty

I might be coming at this with the wrong design philosophy, so please forgive me for possibly questioning defined specification!

I think right now we have code that is silently attempting to make inferences about user intent which may or may not actually be correct.

I'd encourage we reconsider this approach, and instead simply convey "this isn't correct, you need to fix it by X" (or possibly offering a suggested correction).

All that said, if we do decide to keep attempted inference we should make sure that the logic for save / serialization matches the logic for formatting / rendering.

cecilia-donnelly · 2026-07-10T15:14:39Z

@aasandei-vsp and I talked about the null problem in displayTime during a meeting. What we will do is remove the fallback on these branches and complete the migration where we copy displayDt values to null displayTime values before we flip the feature flag. In the old UI, there is no way for a user to clear a date, so the fallback will work as intended. This requires that these intermediate PRs also consider the feature flag in the file list, though, because we should keep the fallback in deployed code until the new UI is released (i.e., the feature flag is turned on).

More succinctly:

Behind the edtf-date feature flag, remove the fallback in the file list
Do the same on other branches working on this feature

cecilia-donnelly

Officially requesting changes so this doesn't show up in my "review requested" list anymore. The specific change requested is to remove the fallback in the file list behind the feature flag.

aasandei-vsp · 2026-07-14T12:24:20Z

Officially requesting changes so this doesn't show up in my "review requested" list anymore. The specific change requested is to remove the fallback in the file list behind the feature flag.

I have created a commit specifically for this situation, making it easier to review.

aasandei-vsp · 2026-07-14T12:27:46Z

I might be coming at this with the wrong design philosophy, so please forgive me for possibly questioning defined specification!

I think right now we have code that is silently attempting to make inferences about user intent which may or may not actually be correct.

I'd encourage we reconsider this approach, and instead simply convey "this isn't correct, you need to fix it by X" (or possibly offering a suggested correction).

All that said, if we do decide to keep attempted inference we should make sure that the logic for save / serialization matches the logic for formatting / rendering.

There are indeed a lot of scenarios and dealing with each one of them proved to be slightly complex. Your suggestions are great and correct, but at some point it was decided that we need to silently make some decisions for the user, as not everyone is savvy when it comes to edtf dates. I'll be addressing the rest of the comments one by one.

cecilia-donnelly

The code makes sense but there are a couple things I found while testing it out:

Clear date doesn't work because somewhere we're sending an empty string instead of explicit null
I can't enter a month value like "3" -- the sidebar simply doesn't show anything if I try. "03" works, but I should be able to enter a plain "3" (or any other single digit) and have it be padded with a zero afterward
It looks like the same is happening for hours
It lets me enter a value like "2026-XX-10" but then I can't set it to "uncertain" or "approximate." That might be an EDTF restriction, do you know?

I would go back through and do some testing again -- I know this has been waiting a while so maybe there are some odd interactions?

Also I see that you put the keyboard interactions back, disregard my inline comments! Thank you!

cecilia-donnelly · 2026-07-21T15:44:25Z

 		}

+		if (message.includes('complete date is required')) {
+			return 'A complete date is required when time is provided.';


This is smart.

cecilia-donnelly · 2026-07-21T15:45:12Z

-			[class.disabled]="disabled"
-			(click)="toggleDatepicker()"
-			(keydown.enter)="toggleDatepicker()"
-			(keydown.space)="toggleDatepicker()"


Why remove these?

It's true that at some point in the past Claude removed them for some reason and I didn't notice and I'm glad you did! Now it's all in there, thank you for having such a thorough look!

cecilia-donnelly · 2026-07-21T15:48:35Z

-			[class.disabled]="disabled"
-			(click)="toggleTimepicker()"
-			(keydown.enter)="toggleTimepicker()"
-			(keydown.space)="toggleTimepicker()"


Same question - why remove this? (Maybe this isn't acting as a button anymore?)

It's true that at some point in the past Claude removed them for some reason and I didn't notice and I'm glad you did! Now it's all in there, thank you for having such a thorough look!

aasandei-vsp · 2026-07-22T10:38:44Z

The code makes sense but there are a couple things I found while testing it out:

Clear date doesn't work because somewhere we're sending an empty string instead of explicit null

I can't enter a month value like "3" -- the sidebar simply doesn't show anything if I try. "03" works, but I should be able to enter a plain "3" (or any other single digit) and have it be padded with a zero afterward

It looks like the same is happening for hours

It lets me enter a value like "2026-XX-10" but then I can't set it to "uncertain" or "approximate." That might be an EDTF restriction, do you know?

I would go back through and do some testing again -- I know this has been waiting a while so maybe there are some odd interactions?

Also I see that you put the keyboard interactions back, disregard my inline comments! Thank you!

Clear date+only one digit
I have checked the mentioned behaviors and the first three I couldn't reproduce. I have added a small video, from the branch deployed on dev, where things seem to be working as expected.
The only thing worth mentioning, when you only insert one digit, the next input won't be focused, but that makes sense.
Please let me know if I'm missing something:
https://www.loom.com/share/c0639bb8f5ba4275993d71afa584b61b

Unspecified digits+qualifiers
TL;DR Specs unclear to me(no example to actually confirm this behavior). BE supports it. Made workaround in FE.

The library's grammar treats "unspecified digits" (X) and "qualification" ( ~ / ? / % ) as separate, non-combinable features, so it throws a syntax error on anything like 2026-XX-10~ — regardless of where the qualifier is placed. That error was being caught and surfaced as the generic "date is not valid" message, which is why you could enter 2026-XX-10 but not mark it approximate/uncertain.

I verified the backend happily accepts and stores these strings (2026-XX-10~ and friends), so the restriction was purely FE. The fix works around the library by stripping the trailing qualifier before parsing/validating, then reattaching the flags to the model ourselves — so 2026-XX-10~ now serializes, saves, and parses back into the picker correctly, in both directions.

One intentional boundary: a qualifier combined with a time (e.g. 2026-01-01T10:00:00~) is still rejected — the library doesn't support that combination either, and since an unspecified date and a time can't coexist anyway, we kept that blocked rather than silently allowing it.

Left-pad single-digit month and day values with zero instead of X-padding them, since a single 1-9 digit can only be a complete value ('5' -> '05', no longer '5X'). Require a complete date whenever a time is provided and surface a dedicated friendly error message for that case. Add a generic getSegmentError helper that the datepicker and timepicker inputs use to build their inline field error messages (invalid characters and out-of-range checks, skipped while the value is still being typed). Issue: PER-10643

Instead of silently rejecting invalid input, the year, month and day segments now emit whatever the user typed and surface a per-field error message below the input (invalid characters, out-of-range month or day). The day is re-validated when the month changes, and picking a date from the calendar clears all errors. The error styling lives in new shared input-error-state and input-error-message mixins. Issue: PER-10643

Instead of silently rejecting invalid input, the hours, minutes and seconds segments now emit whatever the user typed and surface a per-field error message below the input. The hour is re-validated when the AM/PM/24H format changes. Issue: PER-10643

The inline-error restructure dropped role="button", tabindex, and the keydown.enter/space handlers from the calendar and clock toggle buttons, leaving them mouse-only. Restore them and add keyboard-toggle tests. Issue: PER-10643

Persist null instead of '' when a date is cleared, and render an explicitly-null displayTime as empty ("Click to add date"). Widen displayTime to string | null and add specs. Issue: PER-10643

With the flag on, displayTime is authoritative (the migration backfills it before the flip), so a missing value means the date was cleared and nothing is shown. With the flag off, the file list keeps its fallback so deployed code is unchanged. Issue: PER-10643

Move the picker's display formatting into EdtfService so the preview and the serialized EDTF are built from the same padding helpers — a single-digit month now reads as the month itself ("1" -> January / 1985-01) in both. A lone "0" month or day is rejected with a clear range error instead of being silently serialized to "0X". Issue: PER-10643

The edtf library rejects a qualifier (~/?/%) combined with unspecified (X) digits, so approximate/uncertain dates with a blank month or day (e.g. 2026-XX-10~) failed validation, even though the backend accepts them. Strip the qualifier before parsing/validating and reattach the flags ourselves so the combination round-trips both ways. Issue: PER-10643

cecilia-donnelly

Thanks for this, @aasandei-vsp! I tested on dev this time to avoid any local environment oddities. I did find one edge case. When I put in "2021-2-29" (i.e., a leap day in a non-leap year) instead of erroring, the UI autocorrected to "2021-03-01" (no doubt a Date library fix). I think we want to give an error there. Final little things!

aasandei-vsp requested review from cecilia-donnelly and slifty June 11, 2026 13:03

aasandei-vsp force-pushed the PER-10642-create-edtf-date-modal-component branch from 2a3db6f to 215f0c0 Compare June 12, 2026 14:33

aasandei-vsp force-pushed the PER-10643-edtf-validation branch from f3c2c87 to 374ded6 Compare June 15, 2026 12:26

aasandei-vsp self-assigned this Jun 15, 2026

aasandei-vsp force-pushed the PER-10643-edtf-validation branch 2 times, most recently from 44786c5 to 6de3071 Compare June 15, 2026 12:57

aasandei-vsp force-pushed the PER-10642-create-edtf-date-modal-component branch from 215f0c0 to a71e024 Compare June 16, 2026 13:40

aasandei-vsp force-pushed the PER-10643-edtf-validation branch from 6de3071 to 1c74479 Compare June 16, 2026 14:04

cecilia-donnelly reviewed Jun 16, 2026

View reviewed changes

Comment thread src/app/shared/components/datepicker-input/datepicker-input.component.html

Comment thread src/app/shared/components/timepicker-input/timepicker-input.component.html

aasandei-vsp force-pushed the PER-10642-create-edtf-date-modal-component branch from a71e024 to 3876198 Compare June 17, 2026 12:02

aasandei-vsp force-pushed the PER-10643-edtf-validation branch from 1c74479 to fe7dd07 Compare June 17, 2026 12:36

aasandei-vsp requested a review from cecilia-donnelly June 18, 2026 06:29

aasandei-vsp force-pushed the PER-10642-create-edtf-date-modal-component branch from 3876198 to b6b99b9 Compare June 18, 2026 08:50

Base automatically changed from PER-10642-create-edtf-date-modal-component to main June 19, 2026 13:30

aasandei-vsp force-pushed the PER-10643-edtf-validation branch from fe7dd07 to 57c3de4 Compare June 29, 2026 10:27

slifty reviewed Jul 2, 2026

View reviewed changes

Comment thread src/app/shared/services/edtf-service/edtf.service.ts

Comment thread src/app/shared/services/edtf-service/edtf.service.ts

Comment thread src/app/shared/services/edtf-service/edtf.service.ts

cecilia-donnelly requested changes Jul 10, 2026

View reviewed changes

aasandei-vsp force-pushed the PER-10643-edtf-validation branch from 57c3de4 to 67edbff Compare July 14, 2026 12:45

aasandei-vsp requested review from cecilia-donnelly and slifty July 14, 2026 12:45

cecilia-donnelly requested changes Jul 21, 2026

View reviewed changes

aasandei-vsp added 7 commits July 22, 2026 15:03

Restore keyboard toggle for the date and time pickers

ba8fa30

The inline-error restructure dropped role="button", tabindex, and the keydown.enter/space handlers from the calendar and clock toggle buttons, leaving them mouse-only. Restore them and add keyboard-toggle tests. Issue: PER-10643

Send null for cleared EDTF dates

c8a0683

Persist null instead of '' when a date is cleared, and render an explicitly-null displayTime as empty ("Click to add date"). Widen displayTime to string | null and add specs. Issue: PER-10643

aasandei-vsp force-pushed the PER-10643-edtf-validation branch from bad07a3 to a3a7e5a Compare July 22, 2026 12:11

aasandei-vsp requested a review from cecilia-donnelly July 22, 2026 12:23

aasandei-vsp force-pushed the PER-10643-edtf-validation branch from a3a7e5a to 452da47 Compare July 22, 2026 13:33

cecilia-donnelly reviewed Jul 23, 2026

View reviewed changes

Uh oh!

Conversation

aasandei-vsp commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Manual test cases — Date/time input inline validation

Invalid characters

Out-of-range month

Out-of-range day

Out-of-range hours (AM/PM mode)

Out-of-range minutes

Out-of-range seconds

No error while typing a single digit

Day re-validated when the month changes

Hour re-validated when the format changes

Calendar selection clears errors

Time without a complete date

Uh oh!

codecov Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aasandei-vsp commented Jun 15, 2026

Uh oh!

cecilia-donnelly commented Jun 16, 2026

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aasandei-vsp commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slifty left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cecilia-donnelly commented Jul 10, 2026

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

aasandei-vsp commented Jul 14, 2026

Uh oh!

aasandei-vsp commented Jul 14, 2026

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

aasandei-vsp Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

aasandei-vsp Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

aasandei-vsp commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aasandei-vsp commented Jun 11, 2026 •

edited

Loading

codecov Bot commented Jun 11, 2026 •

edited

Loading

aasandei-vsp commented Jun 29, 2026 •

edited

Loading

aasandei-vsp commented Jul 22, 2026 •

edited

Loading