diff --git a/src/app/core/services/edit/edit.service.spec.ts b/src/app/core/services/edit/edit.service.spec.ts index cdd25d81e..07444dfc6 100644 --- a/src/app/core/services/edit/edit.service.spec.ts +++ b/src/app/core/services/edit/edit.service.spec.ts @@ -445,6 +445,72 @@ describe('EditService', () => { expect(apiService.folder.getStelaFolderVOs).not.toHaveBeenCalled(); }); + it('should reconcile a folder when the Stela response has a string folderId and no folder_linkId', async () => { + // The in-memory folder holds a numeric folderId and a folder_linkId, while + // the Stela response VO has a string folderId and no folder_linkId — the + // mismatch that previously left the folder unmatched and threw on update. + const folder = new FolderVO({ + folderId: 1, + folder_linkId: 100, + displayName: 'Test Folder', + }); + const updatedFolderVO = new FolderVO({ + folderId: '1' as unknown as number, + updatedDT: '2024-03-03', + displayTime: '1990-07', + }); + const mockFolderResponse = { + getFolderVOs: jasmine + .createSpy('getFolderVOs') + .and.returnValue([updatedFolderVO]), + }; + + (apiService.folder.updateStelaFolder as jasmine.Spy).and.returnValue( + Promise.resolve(mockFolderResponse), + ); + (apiService.folder.getStelaFolderVOs as jasmine.Spy).and.returnValue( + Promise.resolve(mockFolderResponse), + ); + folder.update = jasmine.createSpy('update'); + + await service.updateItems([folder], ['displayTime']); + + expect(folder.update).toHaveBeenCalledWith( + jasmine.objectContaining({ + updatedDT: '2024-03-03', + displayTime: '1990-07', + }), + ); + }); + + it('should not throw when a response folder matches no in-memory folder', async () => { + const folder = new FolderVO({ + folderId: 1, + folder_linkId: 100, + displayName: 'Test Folder', + }); + const unmatchedVO = new FolderVO({ + folderId: '999' as unknown as number, + updatedDT: '2024-03-03', + }); + const mockFolderResponse = { + getFolderVOs: jasmine + .createSpy('getFolderVOs') + .and.returnValue([unmatchedVO]), + }; + + (apiService.folder.updateStelaFolder as jasmine.Spy).and.returnValue( + Promise.resolve(mockFolderResponse), + ); + (apiService.folder.getStelaFolderVOs as jasmine.Spy).and.returnValue( + Promise.resolve(mockFolderResponse), + ); + + await expectAsync( + service.updateItems([folder], ['displayTime']), + ).toBeResolved(); + }); + it('should revert property and show a translatable generic error when updateStelaRecord fails', async () => { const messageService = TestBed.inject(MessageService); spyOn(messageService, 'showError'); diff --git a/src/app/core/services/edit/edit.service.ts b/src/app/core/services/edit/edit.service.ts index 4e3e87e72..04356e670 100644 --- a/src/app/core/services/edit/edit.service.ts +++ b/src/app/core/services/edit/edit.service.ts @@ -386,7 +386,10 @@ export class EditService { const itemsByLinkId: { [key: number]: ItemVO } = {}; const recordsByRecordId: Map = new Map(); - const foldersByFolderId: Map = new Map(); + // Keyed by a stringified folderId: the Stela update response returns + // folderId as a string while the in-memory folder holds a number, so we + // normalize both sides to match them reliably. + const foldersByFolderId: Map = new Map(); items.forEach((item) => { item.isFolder ? folders.push(item) : records.push(item); @@ -396,7 +399,7 @@ export class EditService { recordsByRecordId.set(item.recordId, item); } } else if (item.folderId) { - foldersByFolderId.set(item.folderId, item); + foldersByFolderId.set(String(item.folderId), item); } }); @@ -435,8 +438,10 @@ export class EditService { const folder = (itemsByLinkId[updatedItem.folder_linkId] as FolderVO) || - foldersByFolderId.get(updatedItem.folderId); - folder.update(newData); + foldersByFolderId.get(String(updatedItem.folderId)); + // Guard against a response VO that maps to no in-memory folder so a + // lookup miss can never crash an otherwise-successful save. + folder?.update(newData); }); } diff --git a/src/app/file-browser/components/file-list-item/file-list-item.component.spec.ts b/src/app/file-browser/components/file-list-item/file-list-item.component.spec.ts index 4140b020b..d731a8232 100644 --- a/src/app/file-browser/components/file-list-item/file-list-item.component.spec.ts +++ b/src/app/file-browser/components/file-list-item/file-list-item.component.spec.ts @@ -10,6 +10,7 @@ import { MessageService } from '@shared/services/message/message.service'; import { AccountService } from '@shared/services/account/account.service'; import { DragService } from '@shared/services/drag/drag.service'; import { ShareLinksService } from '@root/app/share-links/services/share-links.service'; +import { FeatureFlagService } from '@root/app/feature-flag/services/feature-flag.service'; import { EditService } from '@core/services/edit/edit.service'; import { DeviceService } from '@shared/services/device/device.service'; import { provideNoopAnimations } from '@angular/platform-browser/animations'; @@ -66,7 +67,13 @@ describe('FileListItemComponent', () => { isMobileWidth: jasmine.createSpy().and.returnValue(false), }; + const mockFeatureFlagService = { + isEnabled: jasmine.createSpy().and.returnValue(false), + }; + beforeEach(async () => { + mockFeatureFlagService.isEnabled.and.returnValue(false); + await TestBed.configureTestingModule({ imports: [MockItemTypeIconPipe, MockPrDatePipe, MockPrConstantsPipe], declarations: [FileListItemComponent, GetThumbnailPipe], @@ -131,6 +138,7 @@ describe('FileListItemComponent', () => { }, { provide: EditService, useValue: mockEditService }, { provide: DeviceService, useValue: mockDeviceService }, + { provide: FeatureFlagService, useValue: mockFeatureFlagService }, ], }).compileComponents(); @@ -419,4 +427,36 @@ describe('FileListItemComponent', () => { expect(component.date).not.toContain('2023'); expect(component.date).not.toContain('2026'); }); + + describe('with the edtf-date feature flag enabled', () => { + beforeEach(() => { + mockFeatureFlagService.isEnabled.and.callFake( + (flag: string) => flag === 'edtf-date', + ); + }); + + it('should not fall back to displayDT when displayTime is missing', () => { + component.item.displayTime = undefined; + component.item.displayDT = '2023-01-01T00:00:00.000Z'; + fixture.detectChanges(); + + expect(component.startDisplayTime).toBe(''); + }); + + it('should show nothing when displayTime was explicitly cleared', () => { + component.item.displayTime = null; + component.item.displayDT = '2023-01-01T00:00:00.000Z'; + fixture.detectChanges(); + + expect(component.startDisplayTime).toBe(''); + }); + + it('should still show the displayTime start date', () => { + component.item.displayTime = '2020-06-10/2026-06-15'; + component.item.displayDT = '2023-01-01T00:00:00.000Z'; + fixture.detectChanges(); + + expect(component.startDisplayTime).toBe('2020-06-10'); + }); + }); }); diff --git a/src/app/file-browser/components/file-list-item/file-list-item.component.ts b/src/app/file-browser/components/file-list-item/file-list-item.component.ts index d67939338..cd9fb57ec 100644 --- a/src/app/file-browser/components/file-list-item/file-list-item.component.ts +++ b/src/app/file-browser/components/file-list-item/file-list-item.component.ts @@ -34,6 +34,7 @@ import { import { DataStatus } from '@models/data-status.enum'; import { EditService } from '@core/services/edit/edit.service'; import { EdtfService } from '@shared/services/edtf-service/edtf.service'; +import { FeatureFlagService } from '@root/app/feature-flag/services/feature-flag.service'; import { RecordResponse, FolderResponse, @@ -249,13 +250,22 @@ export class FileListItemComponent @Inject(DOCUMENT) private document: Document, private shareLinksService: ShareLinksService, private edtfService: EdtfService, + private featureFlagService: FeatureFlagService, ) {} get startDisplayTime(): string { - return ( - this.edtfService.getEdtfIntervalStartDate(this.item.displayTime) || - this.item.displayDT + const edtfStartDate = this.edtfService.getEdtfIntervalStartDate( + this.item.displayTime, ); + + // Once the edtf-date UI ships, displayTime is authoritative (a null + // value means the user cleared the date, so nothing is shown). Until + // then, items may only have displayDT populated, so keep the fallback. + if (this.featureFlagService.isEnabled('edtf-date')) { + return edtfStartDate; + } + + return edtfStartDate || this.item.displayDT; } async ngOnInit() { diff --git a/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.spec.ts b/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.spec.ts index ace6c2130..2f3176f6e 100644 --- a/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.spec.ts +++ b/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.spec.ts @@ -292,10 +292,10 @@ describe('SidebarDatePickerComponent', () => { expect(component.formattedStartDate()).toBe('1985-XX-20'); }); - it('should fall back to ISO style "1985-1X-20" when month is partial', () => { + it('should render a single-digit month as the complete month, matching serialization ("1" -> January)', () => { setDate('1985', '1', '20'); - expect(component.formattedStartDate()).toBe('1985-1X-20'); + expect(component.formattedStartDate()).toBe('January 20, 1985'); }); it('should fall back to ISO style "XXXX-XX-20" when only day is set', () => { diff --git a/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.ts b/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.ts index 6274fbcb1..193b0c913 100644 --- a/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.ts +++ b/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.ts @@ -13,7 +13,6 @@ import { ElementRef, } from '@angular/core'; import { CommonModule } from '@angular/common'; -import { format } from 'date-fns'; import { EdtfService, TIME_FORMAT_LABEL, @@ -360,40 +359,8 @@ export class SidebarDatePickerComponent implements OnInit, OnChanges { ); } - private padDigitsWithX(value: string, width: number): string { - const v = value ?? ''; - return v.length >= width ? v : v + 'X'.repeat(width - v.length); - } - private formatDate(date: DateModel): string { - const yearRaw = date.year ?? ''; - const monthRaw = date.month ?? ''; - const dayRaw = date.day ?? ''; - - const hasYear = !!yearRaw; - const hasMonth = !!monthRaw; - const hasDay = !!dayRaw && parseInt(dayRaw, 10) !== 0; - - if (!hasYear && !hasMonth && !hasDay) return ''; - - const yearDisplay = this.padDigitsWithX(yearRaw, 4); - const monthComplete = /^\d{2}$/.test(monthRaw); - const monthName = monthComplete - ? format(new Date(2000, parseInt(monthRaw, 10) - 1), 'MMMM') - : null; - // A day is a discrete value, not a range, so a single digit is - // zero-padded on the left ("2" -> "02") rather than X-padded. - const dayDisplay = hasDay ? dayRaw.padStart(2, '0') : ''; - - if (monthName && hasDay) - return `${monthName} ${dayDisplay}, ${yearDisplay}`; - if (monthName) return `${monthName} ${yearDisplay}`; - if (!hasMonth && !hasDay) return yearDisplay; - - const monthDisplay = hasMonth ? this.padDigitsWithX(monthRaw, 2) : 'XX'; - const parts: string[] = [yearDisplay, monthDisplay]; - if (hasDay) parts.push(dayDisplay); - return parts.join('-'); + return this.edtfService.formatDateForDisplay(date); } private formatTime(time: TimeModel): string { diff --git a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts index a3364b71f..837b7866e 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts @@ -527,6 +527,72 @@ describe('SidebarComponent', () => { }); }); + describe('saveDisplayTime null handling', () => { + let saveItemVoPropertySpy: jasmine.Spy; + + beforeEach(() => { + saveItemVoPropertySpy = spyOn( + mockEditService, + 'saveItemVoProperty', + ).and.callFake(async (item: any, prop: any, value: any) => { + item[prop] = value; + }); + }); + + it('should save null when the date is cleared to empty', async () => { + component.selectedItem = new RecordVO({ displayTime: '1985-05-20' }); + + await component.onDateSaved({ + date: { year: '', month: '', day: '' }, + time: { format: 'am' }, + }); + + expect(saveItemVoPropertySpy).toHaveBeenCalledWith( + component.selectedItem, + 'displayTime', + null, + ); + }); + + it('should save the EDTF string unchanged for a non-empty date', async () => { + component.selectedItem = new RecordVO({ displayTime: '1985-05-20' }); + + await component.onDateSaved({ + date: { year: '1990', month: '06', day: '15' }, + time: { format: 'am' }, + }); + + expect(saveItemVoPropertySpy).toHaveBeenCalledWith( + component.selectedItem, + 'displayTime', + '1990-06-15', + ); + }); + }); + + describe('updateDisplayTimeObject', () => { + it('should show an empty date when displayTime is explicitly null, ignoring displayDT', () => { + component.selectedItem = new RecordVO({ + displayTime: null, + displayDT: '1985-05-20T00:00:00Z', + }); + + (component as any).updateDisplayTimeObject(); + + expect(component.displayTimeObject).toBeNull(); + }); + + it('should not fall back to displayDT when displayTime is undefined', () => { + component.selectedItem = new RecordVO({ + displayDT: '1985-05-20T00:00:00Z', + }); + + (component as any).updateDisplayTimeObject(); + + expect(component.displayTimeObject).toBeNull(); + }); + }); + describe('onDateMoreOptions', () => { it('should open the edit date time modal with provided data', () => { const openSpy = spyOn(mockModalService, 'open').and.callThrough(); diff --git a/src/app/file-browser/components/sidebar/sidebar.component.ts b/src/app/file-browser/components/sidebar/sidebar.component.ts index 70e731109..f3ea6e32f 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.ts @@ -52,9 +52,10 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { displayTimeObject: DateTimeModel | null = null; + // No displayDT fallback needed here because this feeds the edtf-date picker only, + // which is already hidden behind a flag private updateDisplayTimeObject(): void { - const timeSource = - this.selectedItem?.displayTime || this.selectedItem?.displayDT; + const timeSource = this.selectedItem?.displayTime; try { this.displayTimeObject = timeSource ? this.edtfService.toDateTimeModel(timeSource) @@ -267,7 +268,8 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { private async saveDisplayTime(result: DateTimeModel): Promise { try { - const newDisplayTime = this.edtfService.toEdtfDate(result); + const edtfDate = this.edtfService.toEdtfDate(result); + const newDisplayTime = edtfDate === '' ? null : edtfDate; await this.onFinishEditing('displayTime', newDisplayTime); } catch (err) { this.message.showError({ message: err?.message }); diff --git a/src/app/models/folder-vo.ts b/src/app/models/folder-vo.ts index c039d917a..90a6ace7d 100644 --- a/src/app/models/folder-vo.ts +++ b/src/app/models/folder-vo.ts @@ -49,7 +49,7 @@ export class FolderVO public displayName; public displayDT; public displayEndDT; - public displayTime?: string; + public displayTime?: string | null; public derivedDT; public derivedEndDT; public altText; @@ -182,7 +182,7 @@ export interface FolderVOData extends BaseVOData { displayName?: any; displayDT?: any; displayEndDT?: any; - displayTime?: string; + displayTime?: string | null; derivedDT?: any; derivedEndDT?: any; note?: any; diff --git a/src/app/models/record-vo.ts b/src/app/models/record-vo.ts index 8792cc147..aec8eb891 100644 --- a/src/app/models/record-vo.ts +++ b/src/app/models/record-vo.ts @@ -54,7 +54,7 @@ export class RecordVO public description; public displayDT; public displayEndDT; - public displayTime?: string; + public displayTime?: string | null; public derivedDT; public derivedEndDT; public altText; @@ -186,7 +186,7 @@ export interface RecordVOData extends BaseVOData { description?: any; displayDT?: any; displayEndDT?: any; - displayTime?: string; + displayTime?: string | null; derivedDT?: any; derivedEndDT?: any; derivedCreatedDT?: any; diff --git a/src/app/shared/components/datepicker-input/datepicker-input.component.html b/src/app/shared/components/datepicker-input/datepicker-input.component.html index 01c69a8d7..c3bbb0ded 100644 --- a/src/app/shared/components/datepicker-input/datepicker-input.component.html +++ b/src/app/shared/components/datepicker-input/datepicker-input.component.html @@ -1,51 +1,61 @@ -
- - / - - / - -
- - calendar_today - +
+
+ + / + + / + +
+ + calendar_today + +
+ + @if (currentError()) { + {{ currentError() }} + }
@if (showDatepicker()) { diff --git a/src/app/shared/components/datepicker-input/datepicker-input.component.scss b/src/app/shared/components/datepicker-input/datepicker-input.component.scss index c8dde7bb3..7c7bbbea1 100644 --- a/src/app/shared/components/datepicker-input/datepicker-input.component.scss +++ b/src/app/shared/components/datepicker-input/datepicker-input.component.scss @@ -4,7 +4,13 @@ :host { position: relative; display: block; - height: 40px; + min-height: 40px; +} + +.pr-date-input-wrap { + display: flex; + flex-direction: column; + gap: 6px; } .pr-date-input-group { @@ -15,6 +21,14 @@ &.active { @include input-focus-state; } + + &.has-error { + @include input-error-state; + } +} + +.pr-input-error { + @include input-error-message; } .pr-date-segment { diff --git a/src/app/shared/components/datepicker-input/datepicker-input.component.spec.ts b/src/app/shared/components/datepicker-input/datepicker-input.component.spec.ts index 946195d53..0685aca4f 100644 --- a/src/app/shared/components/datepicker-input/datepicker-input.component.spec.ts +++ b/src/app/shared/components/datepicker-input/datepicker-input.component.spec.ts @@ -1,7 +1,15 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { Component } from '@angular/core'; -import { DateModel } from '@shared/services/edtf-service/edtf.service'; -import { DatepickerInputComponent } from './datepicker-input.component'; +import { + DateModel, + DAY_RANGE_ERROR, + INVALID_DAY_FOR_MONTH_ERROR, + MONTH_RANGE_ERROR, +} from '@shared/services/edtf-service/edtf.service'; +import { + DatepickerInputComponent, + INVALID_CHARS_ERROR, +} from './datepicker-input.component'; @Component({ standalone: true, @@ -46,22 +54,20 @@ describe('DatepickerInputComponent', () => { const mockEvent = (value: string): Event => ({ target: { value } }) as unknown as Event; + // --- Valid input --- + it('should accept valid 4-digit year and emit', () => { component.updateYear(mockEvent('2026')); expect(hostComponent.lastEmittedDate?.year).toBe('2026'); + expect(component.fieldErrors.year()).toBeNull(); }); it('should emit incomplete year as raw digits (no X in input)', () => { component.updateYear(mockEvent('202')); expect(hostComponent.lastEmittedDate?.year).toBe('202'); - }); - - it('should reject non-numeric year', () => { - component.updateYear(mockEvent('20ab')); - - expect(hostComponent.lastEmittedDate).toBeNull(); + expect(component.fieldErrors.year()).toBeNull(); }); it('should accept year padded with leading zeros (ISO 8601)', () => { @@ -74,40 +80,81 @@ describe('DatepickerInputComponent', () => { component.updateMonth(mockEvent('06')); expect(hostComponent.lastEmittedDate?.month).toBe('06'); + expect(component.fieldErrors.month()).toBeNull(); }); - it('should emit single digit month', () => { + it('should accept single digit month', () => { component.updateMonth(mockEvent('1')); expect(hostComponent.lastEmittedDate?.month).toBe('1'); + expect(component.fieldErrors.month()).toBeNull(); }); - it('should not emit or auto-focus for invalid month', () => { + it('should accept day 29 for February in leap year', () => { + hostComponent.date = { year: '2024', month: '02', day: '' }; + fixture.detectChanges(); + component.updateDay(mockEvent('29')); + + expect(hostComponent.lastEmittedDate?.day).toBe('29'); + expect(component.fieldErrors.day()).toBeNull(); + }); + + // --- Invalid input now emits AND surfaces an error --- + + it('should emit invalid characters in the year and surface the invalid-characters error', () => { + component.updateYear(mockEvent('20ab')); + + expect(hostComponent.lastEmittedDate?.year).toBe('20ab'); + expect(component.fieldErrors.year()).toBe(INVALID_CHARS_ERROR); + expect(component.currentError()).toBe(INVALID_CHARS_ERROR); + }); + + it('should emit out-of-range month and surface the month error', () => { component.updateMonth(mockEvent('13')); - expect(hostComponent.lastEmittedDate).toBeNull(); + expect(hostComponent.lastEmittedDate?.month).toBe('13'); + expect(component.fieldErrors.month()).toBe(MONTH_RANGE_ERROR); + }); + + it('should NOT surface the month range error while only one digit has been typed', () => { + component.updateMonth(mockEvent('5')); + + expect(hostComponent.lastEmittedDate?.month).toBe('5'); + expect(component.fieldErrors.month()).toBeNull(); }); - it('should reject non-numeric month', () => { + it('should NOT surface the day range error while only one digit has been typed', () => { + hostComponent.date = { year: '2026', month: '02', day: '' }; + fixture.detectChanges(); + component.updateDay(mockEvent('9')); + + expect(hostComponent.lastEmittedDate?.day).toBe('9'); + expect(component.fieldErrors.day()).toBeNull(); + }); + + it('should emit invalid characters in the month and surface the invalid-characters error', () => { component.updateMonth(mockEvent('ab')); - expect(hostComponent.lastEmittedDate).toBeNull(); + expect(hostComponent.lastEmittedDate?.month).toBe('ab'); + expect(component.fieldErrors.month()).toBe(INVALID_CHARS_ERROR); }); - it('should accept valid 2-digit day for month and emit', () => { - hostComponent.date = { year: '2026', month: '01', day: '' }; + it('should emit day 31 in April and surface the day-for-month error', () => { + hostComponent.date = { year: '2026', month: '04', day: '' }; fixture.detectChanges(); component.updateDay(mockEvent('31')); expect(hostComponent.lastEmittedDate?.day).toBe('31'); + expect(component.fieldErrors.day()).toBe(INVALID_DAY_FOR_MONTH_ERROR); }); - it('should emit single digit day', () => { + it('should surface the range error for a day greater than 31', () => { hostComponent.date = { year: '2026', month: '01', day: '' }; fixture.detectChanges(); - component.updateDay(mockEvent('3')); + component.updateDay(mockEvent('32')); - expect(hostComponent.lastEmittedDate?.day).toBe('3'); + expect(hostComponent.lastEmittedDate?.day).toBe('32'); + expect(component.fieldErrors.day()).toBe(DAY_RANGE_ERROR); }); it('should allow backspacing a left-padded day down to a single "0" without reverting', () => { @@ -121,55 +168,118 @@ describe('DatepickerInputComponent', () => { expect(hostComponent.lastEmittedDate?.day).toBe('0'); }); - it('should not emit day greater than max for month', () => { + it('should emit day 30 in February and surface the day-for-month error', () => { hostComponent.date = { year: '2026', month: '02', day: '' }; fixture.detectChanges(); component.updateDay(mockEvent('30')); - expect(hostComponent.lastEmittedDate).toBeNull(); + expect(hostComponent.lastEmittedDate?.day).toBe('30'); + expect(component.fieldErrors.day()).toBe(INVALID_DAY_FOR_MONTH_ERROR); }); - it('should accept day 29 for February in leap year', () => { - hostComponent.date = { year: '2024', month: '02', day: '' }; + it('should emit day 29 in February of a non-leap year and surface the day-for-month error', () => { + hostComponent.date = { year: '2025', month: '02', day: '' }; fixture.detectChanges(); component.updateDay(mockEvent('29')); expect(hostComponent.lastEmittedDate?.day).toBe('29'); + expect(component.fieldErrors.day()).toBe(INVALID_DAY_FOR_MONTH_ERROR); }); - it('should not emit day 29 for February in non-leap year', () => { - hostComponent.date = { year: '2025', month: '02', day: '' }; + it('should surface the day-for-month error for Feb 29 given a single-digit month', () => { + hostComponent.date = { year: '2021', month: '2', day: '' }; fixture.detectChanges(); component.updateDay(mockEvent('29')); - expect(hostComponent.lastEmittedDate).toBeNull(); + expect(hostComponent.lastEmittedDate?.day).toBe('29'); + expect(component.fieldErrors.day()).toBe(INVALID_DAY_FOR_MONTH_ERROR); }); - it('should not emit day 31 for 30-day months', () => { - hostComponent.date = { year: '2026', month: '04', day: '' }; + it('should re-validate the day when the month changes', () => { + hostComponent.date = { year: '2026', month: '01', day: '31' }; fixture.detectChanges(); - component.updateDay(mockEvent('31')); - expect(hostComponent.lastEmittedDate).toBeNull(); + expect(component.fieldErrors.day()).toBeNull(); + + component.updateMonth(mockEvent('04')); + + expect(component.fieldErrors.day()).toBe(INVALID_DAY_FOR_MONTH_ERROR); }); - it('should allow typing first digit 0 or 1 for month', () => { - const input = { value: '0' } as HTMLInputElement; - component.updateMonth({ target: input } as unknown as Event); + it('should surface the range error inline for a lone "0" month', () => { + component.updateMonth(mockEvent('0')); - expect(input.value).toBe('0'); + expect(hostComponent.lastEmittedDate?.month).toBe('0'); + expect(component.fieldErrors.month()).toBe(MONTH_RANGE_ERROR); + }); + + it('should surface the range error inline for a lone "0" day', () => { + hostComponent.date = { year: '2026', month: '01', day: '' }; + fixture.detectChanges(); + component.updateDay(mockEvent('0')); + + expect(hostComponent.lastEmittedDate?.day).toBe('0'); + expect(component.fieldErrors.day()).toBe(DAY_RANGE_ERROR); }); - it('should reject first digit > 1 for month', () => { - hostComponent.date = { year: '', month: '', day: '' }; + it('should attribute a lone "0" month to the month field, not the day', () => { + hostComponent.date = { year: '2024', month: '', day: '31' }; fixture.detectChanges(); - const input = { value: '5' } as HTMLInputElement; - component.updateMonth({ target: input } as unknown as Event); + component.updateMonth(mockEvent('0')); + + expect(component.fieldErrors.month()).toBe(MONTH_RANGE_ERROR); + expect(component.fieldErrors.day()).toBeNull(); + }); + + it('should clear the year error when the field is cleared', () => { + component.updateYear(mockEvent('20ab')); + + expect(component.fieldErrors.year()).not.toBeNull(); + + component.updateYear(mockEvent('')); + + expect(component.fieldErrors.year()).toBeNull(); + }); + + // --- Auto-focus behavior --- + + it('should auto-focus the month input after a valid 4-digit year', () => { + const focusSpy = spyOn( + component.monthInput.nativeElement, + 'focus', + ).and.callThrough(); + component.updateYear(mockEvent('2026')); + + expect(focusSpy).toHaveBeenCalled(); + }); + + it('should NOT auto-focus the month input when the year has invalid characters', () => { + const focusSpy = spyOn(component.monthInput.nativeElement, 'focus'); + component.updateYear(mockEvent('20ab')); + + expect(focusSpy).not.toHaveBeenCalled(); + }); - expect(input.value).toBe(''); + it('should auto-focus the day input after a valid 2-digit month', () => { + const focusSpy = spyOn( + component.dayInput.nativeElement, + 'focus', + ).and.callThrough(); + component.updateMonth(mockEvent('06')); + + expect(focusSpy).toHaveBeenCalled(); + }); + + it('should NOT auto-focus the day input when the month is out of range', () => { + const focusSpy = spyOn(component.dayInput.nativeElement, 'focus'); + component.updateMonth(mockEvent('13')); + + expect(focusSpy).not.toHaveBeenCalled(); }); - it('should emit when clearing year field', () => { + // --- Misc --- + + it('should emit when clearing the year field', () => { hostComponent.date = { year: '2026', month: '02', day: '18' }; fixture.detectChanges(); component.updateYear(mockEvent('')); @@ -201,8 +311,24 @@ describe('DatepickerInputComponent', () => { expect(component.showDatepicker()).toBeFalse(); }); - it('should emit date and close datepicker on date select', () => { + it('should toggle datepicker via keyboard (Enter and Space)', () => { + const iconButton: HTMLElement = + fixture.nativeElement.querySelector('.pr-icon-button'); + + iconButton.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); + fixture.detectChanges(); + + expect(component.showDatepicker()).toBeTrue(); + + iconButton.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); + fixture.detectChanges(); + + expect(component.showDatepicker()).toBeFalse(); + }); + + it('should emit date and clear errors on date select', () => { component.showDatepicker.set(true); + component.fieldErrors.month.set(MONTH_RANGE_ERROR); component.onDateSelect({ year: 2026, month: 3, day: 15 }); expect(hostComponent.lastEmittedDate).toEqual({ @@ -212,6 +338,7 @@ describe('DatepickerInputComponent', () => { }); expect(component.showDatepicker()).toBeFalse(); + expect(component.currentError()).toBeNull(); }); it('should return null datepickerModel for incomplete date', () => { @@ -229,4 +356,11 @@ describe('DatepickerInputComponent', () => { expect(component.showDatepicker()).toBeFalse(); }); + + it('should refresh errors from incoming @Input date on changes', () => { + hostComponent.date = { year: '2026', month: '13', day: '' }; + fixture.detectChanges(); + + expect(component.fieldErrors.month()).toBe(MONTH_RANGE_ERROR); + }); }); diff --git a/src/app/shared/components/datepicker-input/datepicker-input.component.ts b/src/app/shared/components/datepicker-input/datepicker-input.component.ts index 5cdb954fe..629e471bf 100644 --- a/src/app/shared/components/datepicker-input/datepicker-input.component.ts +++ b/src/app/shared/components/datepicker-input/datepicker-input.component.ts @@ -4,12 +4,14 @@ import { Output, EventEmitter, signal, + computed, HostListener, ElementRef, ViewChild, OnChanges, SimpleChanges, OnInit, + WritableSignal, } from '@angular/core'; import { CommonModule } from '@angular/common'; import { NgbDatepicker, NgbDateStruct } from '@ng-bootstrap/ng-bootstrap'; @@ -18,6 +20,10 @@ import { EdtfService, } from '@shared/services/edtf-service/edtf.service'; +export const INVALID_CHARS_ERROR = 'The date contains invalid characters.'; + +type DateFieldKey = keyof DateModel; + @Component({ selector: 'pr-datepicker-input', standalone: true, @@ -38,6 +44,18 @@ export class DatepickerInputComponent implements OnInit, OnChanges { showDatepicker = signal(false); datepickerModel = signal(null); + readonly fieldErrors: Record> = { + year: signal(null), + month: signal(null), + day: signal(null), + }; + currentError = computed( + () => + this.fieldErrors.year() ?? + this.fieldErrors.month() ?? + this.fieldErrors.day(), + ); + constructor( private elementRef: ElementRef, private edtfService: EdtfService, @@ -45,11 +63,13 @@ export class DatepickerInputComponent implements OnInit, OnChanges { ngOnInit(): void { this.updateDatepickerModel(this.date); + this.refreshErrorsFromInput(); } ngOnChanges(changes: SimpleChanges): void { if (changes.date) { this.updateDatepickerModel(this.date); + this.refreshErrorsFromInput(); } } @@ -64,6 +84,41 @@ export class DatepickerInputComponent implements OnInit, OnChanges { } } + private refreshErrorsFromInput(): void { + (Object.keys(this.fieldErrors) as DateFieldKey[]).forEach((datePropKey) => + this.fieldErrors[datePropKey].set( + this.getFieldError(datePropKey, this.date[datePropKey] ?? ''), + ), + ); + } + + private getFieldError( + datePropKey: DateFieldKey, + value: string, + ): string | null { + if (datePropKey === 'year') return this.getYearError(value); + if (datePropKey === 'month') return this.getMonthError(value); + return this.getDayError(value, this.date.month ?? ''); + } + + private getYearError(value: string): string | null { + return this.edtfService.getSegmentError(value, { + invalidCharsMessage: INVALID_CHARS_ERROR, + }); + } + + private getMonthError(value: string): string | null { + return this.edtfService.getMonthError(value, { + invalidCharsMessage: INVALID_CHARS_ERROR, + }); + } + + private getDayError(value: string, month: string): string | null { + return this.edtfService.getDayError(value, this.date.year, month, { + invalidCharsMessage: INVALID_CHARS_ERROR, + }); + } + @HostListener('document:click', ['$event']) onDocumentClick(event: MouseEvent): void { if (!this.elementRef.nativeElement.contains(event.target)) { @@ -77,45 +132,34 @@ export class DatepickerInputComponent implements OnInit, OnChanges { } updateYear(event: Event): void { - const input = event.target as HTMLInputElement; - const value = input.value; - - if (!this.edtfService.isValidYear(value)) { - input.value = this.date.year; - return; - } + const value = (event.target as HTMLInputElement).value; + const error = this.getFieldError('year', value); + this.fieldErrors.year.set(error); this.dateChange.emit({ ...this.date, year: value }); - if (value.length === 4) { + if (!error && value.length === 4) { this.monthInput.nativeElement.focus(); } } updateMonth(event: Event): void { - const input = event.target as HTMLInputElement; - const value = input.value; - - if (!this.edtfService.isValidMonth(value)) { - input.value = this.date.month ?? ''; - return; - } + const value = (event.target as HTMLInputElement).value; + const error = this.getFieldError('month', value); + this.fieldErrors.month.set(error); this.dateChange.emit({ ...this.date, month: value }); - if (value.length === 2) { + + // Re-validate day because its bounds depend on the month. + this.fieldErrors.day.set(this.getDayError(this.date.day ?? '', value)); + + if (!error && value.length === 2) { this.dayInput.nativeElement.focus(); } } updateDay(event: Event): void { - const input = event.target as HTMLInputElement; - const value = input.value; - - if ( - !this.edtfService.isValidDay(value, this.date.year, this.date.month ?? '') - ) { - input.value = this.date.day ?? ''; - return; - } + const value = (event.target as HTMLInputElement).value; + this.fieldErrors.day.set(this.getFieldError('day', value)); this.dateChange.emit({ ...this.date, day: value }); } @@ -126,6 +170,7 @@ export class DatepickerInputComponent implements OnInit, OnChanges { if (target.value !== '') return; event.preventDefault(); const newYear = (this.date.year ?? '').slice(0, -1); + this.fieldErrors.year.set(this.getFieldError('year', newYear)); this.dateChange.emit({ ...this.date, year: newYear }); this.yearInput.nativeElement.focus(); } @@ -136,6 +181,8 @@ export class DatepickerInputComponent implements OnInit, OnChanges { if (target.value !== '') return; event.preventDefault(); const newMonth = (this.date.month ?? '').slice(0, -1); + this.fieldErrors.month.set(this.getFieldError('month', newMonth)); + this.fieldErrors.day.set(this.getDayError(this.date.day ?? '', newMonth)); this.dateChange.emit({ ...this.date, month: newMonth }); this.monthInput.nativeElement.focus(); } @@ -148,6 +195,9 @@ export class DatepickerInputComponent implements OnInit, OnChanges { day: String(newDate.day).padStart(2, '0'), }; this.date = updatedDate; + Object.values(this.fieldErrors).forEach((fieldError) => + fieldError.set(null), + ); this.dateChange.emit(updatedDate); this.showDatepicker.set(false); } diff --git a/src/app/shared/components/timepicker-input/timepicker-input.component.html b/src/app/shared/components/timepicker-input/timepicker-input.component.html index d23a855a1..0cca19580 100644 --- a/src/app/shared/components/timepicker-input/timepicker-input.component.html +++ b/src/app/shared/components/timepicker-input/timepicker-input.component.html @@ -1,54 +1,68 @@ -
- - : - - : - - -
- +
+ + : + + : + + +
+ + access_time + +
+ + @if (currentError()) { + {{ currentError() }} + }
@if (showTimepicker()) { diff --git a/src/app/shared/components/timepicker-input/timepicker-input.component.scss b/src/app/shared/components/timepicker-input/timepicker-input.component.scss index 1c961c2a5..b78c52c0c 100644 --- a/src/app/shared/components/timepicker-input/timepicker-input.component.scss +++ b/src/app/shared/components/timepicker-input/timepicker-input.component.scss @@ -4,7 +4,13 @@ :host { position: relative; display: block; - height: 40px; + min-height: 40px; +} + +.pr-time-input-wrap { + display: flex; + flex-direction: column; + gap: 6px; } .pr-time-input-group { @@ -15,6 +21,14 @@ &.active { @include input-focus-state; } + + &.has-error { + @include input-error-state; + } +} + +.pr-input-error { + @include input-error-message; } .pr-time-segment { diff --git a/src/app/shared/components/timepicker-input/timepicker-input.component.spec.ts b/src/app/shared/components/timepicker-input/timepicker-input.component.spec.ts index a172e3a31..f7a9f4f72 100644 --- a/src/app/shared/components/timepicker-input/timepicker-input.component.spec.ts +++ b/src/app/shared/components/timepicker-input/timepicker-input.component.spec.ts @@ -1,7 +1,14 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { Component } from '@angular/core'; import { TimeModel } from '@shared/services/edtf-service/edtf.service'; -import { TimepickerInputComponent } from './timepicker-input.component'; +import { + HOUR_12_RANGE_ERROR, + HOUR_24_RANGE_ERROR, + INVALID_CHARS_ERROR, + MINUTES_RANGE_ERROR, + SECONDS_RANGE_ERROR, + TimepickerInputComponent, +} from './timepicker-input.component'; @Component({ template: ` { const mockEvent = (value: string): Event => ({ target: { value } }) as unknown as Event; + const switchToH24 = (): void => { + hostComponent.time = { + hours: '', + minutes: '', + seconds: '', + format: 'h24', + }; + fixture.detectChanges(); + }; + // --- Basic rendering --- it('should create', () => { @@ -80,107 +97,135 @@ describe('TimepickerInputComponent', () => { expect(component.showTimepicker()).toBeFalse(); }); - // --- Hour validation (12-hour) --- + it('should toggle timepicker via keyboard (Enter and Space)', () => { + const iconButton: HTMLElement = + fixture.nativeElement.querySelector('.pr-icon-button'); + + iconButton.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); + fixture.detectChanges(); - it('should accept valid hour', () => { + expect(component.showTimepicker()).toBeTrue(); + + iconButton.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); + fixture.detectChanges(); + + expect(component.showTimepicker()).toBeFalse(); + }); + + // --- Hour validation (12-hour mode) --- + + it('should accept valid hour and clear hours error', () => { component.updateTime(mockEvent('10'), 'hours'); expect(hostComponent.lastEmittedTime?.hours).toBe('10'); + expect(component.fieldErrors.hours()).toBeNull(); }); - it('should reject hour greater than 12', () => { + it('should emit hour > 12 in 12-hour mode AND surface 1-12 error', () => { component.updateTime(mockEvent('13'), 'hours'); - expect(hostComponent.lastEmittedTime).toBeNull(); + expect(hostComponent.lastEmittedTime?.hours).toBe('13'); + expect(component.fieldErrors.hours()).toBe(HOUR_12_RANGE_ERROR); }); - it('should reject non-numeric hour', () => { + it('should emit non-numeric hour AND surface the invalid-characters error', () => { component.updateTime(mockEvent('ab'), 'hours'); - expect(hostComponent.lastEmittedTime).toBeNull(); + expect(hostComponent.lastEmittedTime?.hours).toBe('ab'); + expect(component.fieldErrors.hours()).toBe(INVALID_CHARS_ERROR); }); - it('should accept single digit 0 or 1 for hours', () => { + it('should accept single digit 0 or 1 for hours in 12-hour mode', () => { component.updateTime(mockEvent('1'), 'hours'); expect(hostComponent.lastEmittedTime?.hours).toBe('1'); + expect(component.fieldErrors.hours()).toBeNull(); }); - it('should reject single digit greater than 1 for hours', () => { + it('should NOT surface the hours range error while only one digit has been typed', () => { component.updateTime(mockEvent('2'), 'hours'); - expect(hostComponent.lastEmittedTime).toBeNull(); + expect(hostComponent.lastEmittedTime?.hours).toBe('2'); + expect(component.fieldErrors.hours()).toBeNull(); + }); + + it('should NOT surface the minutes range error while only one digit has been typed', () => { + component.updateTime(mockEvent('9'), 'minutes'); + + expect(hostComponent.lastEmittedTime?.minutes).toBe('9'); + expect(component.fieldErrors.minutes()).toBeNull(); }); - // --- Hour validation (24-hour) --- + it('should NOT surface the seconds range error while only one digit has been typed', () => { + component.updateTime(mockEvent('9'), 'seconds'); + + expect(hostComponent.lastEmittedTime?.seconds).toBe('9'); + expect(component.fieldErrors.seconds()).toBeNull(); + }); + + // --- Hour validation (24-hour mode) --- it('should accept hours 00-23 in h24 mode', () => { - hostComponent.time = { - hours: '', - minutes: '', - seconds: '', - format: 'h24', - }; - fixture.detectChanges(); + switchToH24(); component.updateTime(mockEvent('00'), 'hours'); expect(hostComponent.lastEmittedTime?.hours).toBe('00'); - - component.updateTime(mockEvent('13'), 'hours'); - - expect(hostComponent.lastEmittedTime?.hours).toBe('13'); + expect(component.fieldErrors.hours()).toBeNull(); component.updateTime(mockEvent('23'), 'hours'); expect(hostComponent.lastEmittedTime?.hours).toBe('23'); + expect(component.fieldErrors.hours()).toBeNull(); }); - it('should reject hours 24 and above in h24 mode', () => { - hostComponent.time = { - hours: '12', - minutes: '', - seconds: '', - format: 'h24', - }; - fixture.detectChanges(); - hostComponent.lastEmittedTime = null; + it('should emit hour 24 in h24 mode AND surface 0-23 error', () => { + switchToH24(); component.updateTime(mockEvent('24'), 'hours'); - expect(hostComponent.lastEmittedTime).toBeNull(); - - component.updateTime(mockEvent('30'), 'hours'); - - expect(hostComponent.lastEmittedTime).toBeNull(); + expect(hostComponent.lastEmittedTime?.hours).toBe('24'); + expect(component.fieldErrors.hours()).toBe(HOUR_24_RANGE_ERROR); }); - it('should accept single digit 0-2 for hours in h24 mode', () => { + it('should re-validate the hour when the format toggles (15 valid in h24, invalid in 12-hour)', () => { hostComponent.time = { - hours: '', + hours: '15', minutes: '', seconds: '', format: 'h24', }; fixture.detectChanges(); - component.updateTime(mockEvent('2'), 'hours'); + expect(component.fieldErrors.hours()).toBeNull(); - expect(hostComponent.lastEmittedTime?.hours).toBe('2'); + component.cycleFormat(); + + expect(component.fieldErrors.hours()).toBe(HOUR_12_RANGE_ERROR); }); - it('should reject single digit greater than 2 for hours in h24 mode', () => { + it('should clear the hour error when the format toggles to one where the value is valid', () => { hostComponent.time = { - hours: '', + hours: '15', minutes: '', seconds: '', - format: 'h24', + format: 'am', }; fixture.detectChanges(); - component.updateTime(mockEvent('3'), 'hours'); + expect(component.fieldErrors.hours()).toBe(HOUR_12_RANGE_ERROR); - expect(hostComponent.lastEmittedTime).toBeNull(); + // am -> pm: still 12-hour, still invalid + component.cycleFormat(); + fixture.detectChanges(); + + expect(component.fieldErrors.hours()).toBe(HOUR_12_RANGE_ERROR); + + // pm -> h24: now valid + component.cycleFormat(); + fixture.detectChanges(); + + expect(component.fieldErrors.hours()).toBeNull(); }); // --- Minute validation --- @@ -189,24 +234,21 @@ describe('TimepickerInputComponent', () => { component.updateTime(mockEvent('30'), 'minutes'); expect(hostComponent.lastEmittedTime?.minutes).toBe('30'); + expect(component.fieldErrors.minutes()).toBeNull(); }); - it('should reject minutes greater than 59', () => { + it('should emit minutes 60 AND surface the minutes error', () => { component.updateTime(mockEvent('60'), 'minutes'); - expect(hostComponent.lastEmittedTime).toBeNull(); + expect(hostComponent.lastEmittedTime?.minutes).toBe('60'); + expect(component.fieldErrors.minutes()).toBe(MINUTES_RANGE_ERROR); }); - it('should accept single digit 0-5 for minutes', () => { - component.updateTime(mockEvent('5'), 'minutes'); + it('should emit non-numeric minutes AND surface the invalid-characters error', () => { + component.updateTime(mockEvent('a5'), 'minutes'); - expect(hostComponent.lastEmittedTime?.minutes).toBe('5'); - }); - - it('should reject single digit greater than 5 for minutes', () => { - component.updateTime(mockEvent('6'), 'minutes'); - - expect(hostComponent.lastEmittedTime).toBeNull(); + expect(hostComponent.lastEmittedTime?.minutes).toBe('a5'); + expect(component.fieldErrors.minutes()).toBe(INVALID_CHARS_ERROR); }); // --- Second validation --- @@ -215,17 +257,19 @@ describe('TimepickerInputComponent', () => { component.updateTime(mockEvent('45'), 'seconds'); expect(hostComponent.lastEmittedTime?.seconds).toBe('45'); + expect(component.fieldErrors.seconds()).toBeNull(); }); - it('should reject seconds greater than 59', () => { + it('should emit seconds 60 AND surface the seconds error', () => { component.updateTime(mockEvent('60'), 'seconds'); - expect(hostComponent.lastEmittedTime).toBeNull(); + expect(hostComponent.lastEmittedTime?.seconds).toBe('60'); + expect(component.fieldErrors.seconds()).toBe(SECONDS_RANGE_ERROR); }); - // --- Empty values --- + // --- Empty values clear errors --- - it('should allow clearing fields', () => { + it('should allow clearing fields and clear errors', () => { hostComponent.time = { hours: '10', minutes: '30', @@ -236,6 +280,69 @@ describe('TimepickerInputComponent', () => { component.updateTime(mockEvent(''), 'hours'); expect(hostComponent.lastEmittedTime?.hours).toBe(''); + expect(component.fieldErrors.hours()).toBeNull(); + }); + + // --- currentError priority --- + + it('should surface hours error first when both hours and minutes are invalid', () => { + component.updateTime(mockEvent('13'), 'hours'); + component.updateTime(mockEvent('60'), 'minutes'); + + expect(component.currentError()).toBe(HOUR_12_RANGE_ERROR); + }); + + // --- Auto-focus --- + + it('should auto-focus the next field after a valid 2-digit hour', () => { + const focusSpy = spyOn( + component.minutesInput.nativeElement, + 'focus', + ).and.callThrough(); + component.updateTime( + mockEvent('10'), + 'hours', + component.minutesInput.nativeElement, + ); + + expect(focusSpy).toHaveBeenCalled(); + }); + + it('should NOT auto-focus the next field when the value is out of range', () => { + const focusSpy = spyOn(component.minutesInput.nativeElement, 'focus'); + component.updateTime( + mockEvent('13'), + 'hours', + component.minutesInput.nativeElement, + ); + + expect(focusSpy).not.toHaveBeenCalled(); + }); + + // Dispatches real DOM input events so the template bindings themselves are + // exercised — a regression test for the minutes -> seconds focus jump. + it('should auto-focus the seconds input after a valid 2-digit minutes value via the template', () => { + const [, minutesElement, secondsElement] = Array.from( + fixture.nativeElement.querySelectorAll('.pr-time-segment'), + ); + const focusSpy = spyOn(secondsElement, 'focus'); + + minutesElement.value = '30'; + minutesElement.dispatchEvent(new Event('input')); + + expect(focusSpy).toHaveBeenCalled(); + }); + + it('should auto-focus the minutes input after a valid 2-digit hour via the template', () => { + const [hoursElement, minutesElement] = Array.from( + fixture.nativeElement.querySelectorAll('.pr-time-segment'), + ); + const focusSpy = spyOn(minutesElement, 'focus'); + + hoursElement.value = '10'; + hoursElement.dispatchEvent(new Event('input')); + + expect(focusSpy).toHaveBeenCalled(); }); // --- Format cycle --- @@ -504,4 +611,17 @@ describe('TimepickerInputComponent', () => { expect(hostComponent.lastEmittedTime?.timezoneOffset).toBe('+05:30'); }); + // --- Initial / @Input-driven error sync --- + + it('should surface errors derived from incoming @Input time', () => { + hostComponent.time = { + hours: '25', + minutes: '', + seconds: '', + format: 'h24', + }; + fixture.detectChanges(); + + expect(component.fieldErrors.hours()).toBe(HOUR_24_RANGE_ERROR); + }); }); diff --git a/src/app/shared/components/timepicker-input/timepicker-input.component.ts b/src/app/shared/components/timepicker-input/timepicker-input.component.ts index 6ce01d446..b986f50b1 100644 --- a/src/app/shared/components/timepicker-input/timepicker-input.component.ts +++ b/src/app/shared/components/timepicker-input/timepicker-input.component.ts @@ -12,6 +12,7 @@ import { OnDestroy, ViewChild, ElementRef, + WritableSignal, } from '@angular/core'; import { CommonModule } from '@angular/common'; import { ReactiveFormsModule, FormControl } from '@angular/forms'; @@ -25,6 +26,14 @@ import { EdtfService, } from '@shared/services/edtf-service/edtf.service'; +export const INVALID_CHARS_ERROR = 'The time contains invalid characters.'; +export const HOUR_24_RANGE_ERROR = 'Hour must be between 0 and 23.'; +export const HOUR_12_RANGE_ERROR = 'Hour must be between 1 and 12.'; +export const MINUTES_RANGE_ERROR = 'Minutes must be between 0 and 59.'; +export const SECONDS_RANGE_ERROR = 'Seconds must be between 0 and 59.'; + +type TimeFieldKey = keyof Pick; + @Component({ selector: 'pr-timepicker-input', standalone: true, @@ -51,6 +60,18 @@ export class TimepickerInputComponent implements OnInit, OnChanges, OnDestroy { formatLabel = computed(() => TIME_FORMAT_LABEL[this.timeSignal().format]); is24Hour = computed(() => this.timeSignal().format === 'h24'); + readonly fieldErrors: Record> = { + hours: signal(null), + minutes: signal(null), + seconds: signal(null), + }; + currentError = computed( + () => + this.fieldErrors.hours() ?? + this.fieldErrors.minutes() ?? + this.fieldErrors.seconds(), + ); + private destroy$ = new Subject(); constructor( @@ -62,6 +83,7 @@ export class TimepickerInputComponent implements OnInit, OnChanges, OnDestroy { this.timepickerControl.valueChanges .pipe(takeUntil(this.destroy$)) .subscribe((ngbTime) => this.onTimeSelect(ngbTime)); + this.refreshErrorsFromInput(); } ngOnChanges(changes: SimpleChanges): void { @@ -72,6 +94,7 @@ export class TimepickerInputComponent implements OnInit, OnChanges, OnDestroy { if (!this.ngbTimeEquals(model, current)) { this.timepickerControl.setValue(model, { emitEvent: false }); } + this.refreshErrorsFromInput(); } } @@ -80,6 +103,51 @@ export class TimepickerInputComponent implements OnInit, OnChanges, OnDestroy { this.destroy$.complete(); } + private refreshErrorsFromInput(): void { + (Object.keys(this.fieldErrors) as TimeFieldKey[]).forEach((timePropKey) => + this.fieldErrors[timePropKey].set( + this.getFieldError(timePropKey, this.time[timePropKey] ?? ''), + ), + ); + } + + private getFieldError( + timePropKey: TimeFieldKey, + value: string, + ): string | null { + if (timePropKey === 'hours') { + return this.getHoursError(value, this.is24Hour()); + } + if (timePropKey === 'minutes') return this.getMinutesError(value); + return this.getSecondsError(value); + } + + private getHoursError(value: string, is24Hour: boolean): string | null { + return this.edtfService.getSegmentError(value, { + invalidCharsMessage: INVALID_CHARS_ERROR, + isWithinRange: (hours) => this.edtfService.isValidHour(hours, is24Hour), + rangeMessage: is24Hour ? HOUR_24_RANGE_ERROR : HOUR_12_RANGE_ERROR, + }); + } + + private getMinutesError(value: string): string | null { + return this.edtfService.getSegmentError(value, { + invalidCharsMessage: INVALID_CHARS_ERROR, + isWithinRange: (minutes) => + this.edtfService.isValidMinutesSeconds(minutes), + rangeMessage: MINUTES_RANGE_ERROR, + }); + } + + private getSecondsError(value: string): string | null { + return this.edtfService.getSegmentError(value, { + invalidCharsMessage: INVALID_CHARS_ERROR, + isWithinRange: (seconds) => + this.edtfService.isValidMinutesSeconds(seconds), + rangeMessage: SECONDS_RANGE_ERROR, + }); + } + @HostListener('document:click', ['$event']) onDocumentClick(event: MouseEvent): void { if (!this.elementRef.nativeElement.contains(event.target)) { @@ -126,6 +194,10 @@ export class TimepickerInputComponent implements OnInit, OnChanges, OnDestroy { const nextFormat = this.FORMAT_CYCLE[(currentIndex + 1) % this.FORMAT_CYCLE.length]; this.timeChange.emit({ ...this.time, format: nextFormat }); + // Hour validity depends on the format — re-check against the new format. + this.fieldErrors.hours.set( + this.getHoursError(this.time.hours ?? '', nextFormat === 'h24'), + ); } onMinutesKeydown(event: KeyboardEvent): void { @@ -134,6 +206,7 @@ export class TimepickerInputComponent implements OnInit, OnChanges, OnDestroy { if (target.value !== '') return; event.preventDefault(); const newHours = (this.time.hours ?? '').slice(0, -1); + this.fieldErrors.hours.set(this.getFieldError('hours', newHours)); this.timeChange.emit({ ...this.time, hours: newHours }); this.hoursInput.nativeElement.focus(); } @@ -144,37 +217,24 @@ export class TimepickerInputComponent implements OnInit, OnChanges, OnDestroy { if (target.value !== '') return; event.preventDefault(); const newMinutes = (this.time.minutes ?? '').slice(0, -1); + this.fieldErrors.minutes.set(this.getFieldError('minutes', newMinutes)); this.timeChange.emit({ ...this.time, minutes: newMinutes }); this.minutesInput.nativeElement.focus(); } updateTime( event: Event, - timePropKey: keyof Pick, + timePropKey: TimeFieldKey, nextField?: HTMLInputElement, ): void { - const input = event.target as HTMLInputElement; - const value = input.value; - - if (value !== '') { - const isValid = - timePropKey === 'hours' - ? this.edtfService.isValidHour(value, this.is24Hour()) - : this.edtfService.isValidMinutesSeconds(value); - if (!isValid) { - input.value = this.time[timePropKey] ?? ''; - return; - } - } + const value = (event.target as HTMLInputElement).value; + const error = this.getFieldError(timePropKey, value); + this.fieldErrors[timePropKey].set(error); this.timeChange.emit({ ...this.time, [timePropKey]: value }); - if (nextField && value.length === 2) { - const isComplete = - timePropKey === 'hours' - ? this.edtfService.isValidHour(value, this.is24Hour()) - : this.edtfService.isValidMinutesSeconds(value); - if (isComplete) nextField.focus(); + if (!error && nextField && value.length === 2) { + nextField.focus(); } } diff --git a/src/app/shared/services/edtf-service/edtf.service.spec.ts b/src/app/shared/services/edtf-service/edtf.service.spec.ts index 400dc5e1c..bbc57b9b2 100644 --- a/src/app/shared/services/edtf-service/edtf.service.spec.ts +++ b/src/app/shared/services/edtf-service/edtf.service.spec.ts @@ -1,4 +1,10 @@ -import { EdtfService, DateTimeModel } from './edtf.service'; +import { + EdtfService, + DateTimeModel, + MONTH_RANGE_ERROR, + DAY_RANGE_ERROR, + INVALID_DAY_FOR_MONTH_ERROR, +} from './edtf.service'; // Mirrors the service's local-offset stamping so the expectations stay // green in any timezone the tests run in. @@ -262,6 +268,26 @@ describe('EdtfService', () => { expect(result.qualifiers.uncertain).toBe(false); expect(result.qualifiers.unknown).toBe(false); }); + + it('should detect approximate qualifier combined with an unspecified month', () => { + const result = service.toDateTimeModel('2026-XX-11~'); + + expect(result.qualifiers.approximate).toBe(true); + expect(result.qualifiers.uncertain).toBe(false); + expect(result.date.year).toBe('2026'); + expect(result.date.month).toBe(''); + expect(result.date.day).toBe('11'); + }); + + it('should detect combined qualifier alongside an unspecified year digit', () => { + const result = service.toDateTimeModel('198X-05-20%'); + + expect(result.qualifiers.approximate).toBe(true); + expect(result.qualifiers.uncertain).toBe(true); + expect(result.date.year).toBe('198'); + expect(result.date.month).toBe('05'); + expect(result.date.day).toBe('20'); + }); }); describe('interval (range)', () => { @@ -335,6 +361,51 @@ describe('EdtfService', () => { expect(service.toEdtfDate(model)).toBe('1985-05-20'); }); + + it('should left-pad a single-digit month with zero', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '5' }, + time: { format: 'am' }, + }; + + expect(service.toEdtfDate(model)).toBe('1985-05'); + }); + + it('should left-pad a single-digit month with zero when a day is present', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '5', day: '20' }, + time: { format: 'am' }, + }; + + expect(service.toEdtfDate(model)).toBe('1985-05-20'); + }); + + it('should left-pad single-digit month 1 with zero (January)', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '1' }, + time: { format: 'am' }, + }; + + expect(service.toEdtfDate(model)).toBe('1985-01'); + }); + + it('should left-pad a single-digit day with zero', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '05', day: '2' }, + time: { format: 'am' }, + }; + + expect(service.toEdtfDate(model)).toBe('1985-05-02'); + }); + + it('should left-pad single-digit day 1 with zero', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '05', day: '1' }, + time: { format: 'am' }, + }; + + expect(service.toEdtfDate(model)).toBe('1985-05-01'); + }); }); describe('unspecified-digit (X-padding)', () => { @@ -383,44 +454,117 @@ describe('EdtfService', () => { expect(service.toEdtfDate(model)).toBe('1985-XX-20'); }); - it('should zero-pad a single-digit day on the left', () => { + it('should combine partial year, full month, and full day', () => { const model: DateTimeModel = { - date: { year: '1985', month: '05', day: '2' }, + date: { year: '198', month: '05', day: '20' }, time: { format: 'am' }, }; - expect(service.toEdtfDate(model)).toBe('1985-05-02'); + expect(service.toEdtfDate(model)).toBe('198X-05-20'); }); + }); - it('should zero-pad a single-digit day that would be an invalid X-range', () => { + describe('rejecting a lone zero', () => { + // The specific message is surfaced inline by getMonthError/getDayError; + // serialization only needs to reject with the generic footer message. + it('should reject a month of "0" instead of serializing it to "0X"', () => { const model: DateTimeModel = { - date: { year: '1985', month: '05', day: '9' }, + date: { year: '1985', month: '0', day: '20' }, time: { format: 'am' }, }; - expect(service.toEdtfDate(model)).toBe('1985-05-09'); + expect(() => service.toEdtfDate(model)).toThrowError( + /Please check the values/, + ); }); - it('should pad single-digit month with one X', () => { + it('should reject a day of "0" instead of serializing it to "0X"', () => { const model: DateTimeModel = { - date: { year: '1985', month: '1' }, + date: { year: '1985', month: '05', day: '0' }, time: { format: 'am' }, }; - expect(service.toEdtfDate(model)).toBe('1985-1X'); + expect(() => service.toEdtfDate(model)).toThrowError( + /Please check the values/, + ); }); + }); - it('should combine partial year, full month, and full day', () => { + describe('rejecting an impossible calendar day', () => { + // Serialization rejects with the generic footer message; the specific + // day-for-month message is surfaced inline by getDayError. + it('should reject serializing Feb 29 in a non-leap year instead of rolling it forward', () => { const model: DateTimeModel = { - date: { year: '198', month: '05', day: '20' }, + date: { year: '2021', month: '02', day: '29' }, time: { format: 'am' }, }; - expect(service.toEdtfDate(model)).toBe('198X-05-20'); + expect(() => service.toEdtfDate(model)).toThrowError( + /Please check the values/, + ); + }); + + it('should reject serializing Feb 29 in a non-leap year given a single-digit month', () => { + const model: DateTimeModel = { + date: { year: '2021', month: '2', day: '29' }, + time: { format: 'am' }, + }; + + expect(() => service.toEdtfDate(model)).toThrowError( + /Please check the values/, + ); + }); + + it('should reject serializing day 31 in a 30-day month', () => { + const model: DateTimeModel = { + date: { year: '2021', month: '04', day: '31' }, + time: { format: 'am' }, + }; + + expect(() => service.toEdtfDate(model)).toThrowError( + /Please check the values/, + ); + }); + + it('should accept serializing Feb 29 in a leap year', () => { + const model: DateTimeModel = { + date: { year: '2024', month: '02', day: '29' }, + time: { format: 'am' }, + }; + + expect(service.toEdtfDate(model)).toBe('2024-02-29'); }); }); describe('time building', () => { + it('should left-pad single-digit hour, minute and second with zero', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '05', day: '20' }, + time: { + hours: '5', + minutes: '7', + seconds: '9', + format: 'am', + }, + }; + + expect(service.toEdtfDate(model)).toContain('T05:07:09'); + }); + + it('should left-pad a single-digit hour in 24-hour mode', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '05', day: '20' }, + time: { + hours: '5', + minutes: '30', + seconds: '00', + format: 'h24', + }, + }; + + expect(service.toEdtfDate(model)).toContain('T05:30:00'); + }); + it('should build date with PM time', () => { const model: DateTimeModel = { date: { year: '1985', month: '05', day: '20' }, @@ -596,6 +740,36 @@ describe('EdtfService', () => { expect(service.toEdtfDate(model)).toBe('XXXX-XX-XX'); }); + + it('should add approximate qualifier with an unspecified month', () => { + const model: DateTimeModel = { + date: { year: '2026', month: '', day: '11' }, + time: { format: 'am' }, + qualifiers: { approximate: true, uncertain: false, unknown: false }, + }; + + expect(service.toEdtfDate(model)).toBe('2026-XX-11~'); + }); + + it('should add uncertain qualifier on a month with an unspecified year', () => { + const model: DateTimeModel = { + date: { year: '', month: '05', day: '' }, + time: { format: 'am' }, + qualifiers: { approximate: false, uncertain: true, unknown: false }, + }; + + expect(service.toEdtfDate(model)).toBe('XXXX-05?'); + }); + + it('should add combined qualifier alongside an unspecified year digit', () => { + const model: DateTimeModel = { + date: { year: '198', month: '05', day: '20' }, + time: { format: 'am' }, + qualifiers: { approximate: true, uncertain: true, unknown: false }, + }; + + expect(service.toEdtfDate(model)).toBe('198X-05-20%'); + }); }); describe('interval output (range)', () => { @@ -766,6 +940,12 @@ describe('EdtfService', () => { expect(result).toBeNull(); }); + + it('should throw for a qualifier combined with a time (unsupported combination)', () => { + expect(() => + service.toDateTimeModel('2026-01-01T10:00:00~'), + ).toThrowError(); + }); }); describe('formatting errors', () => { @@ -779,6 +959,60 @@ describe('EdtfService', () => { /Please check the values/, ); }); + + it('should throw for a complete date with a time and a qualifier (unsupported combination)', () => { + const model: DateTimeModel = { + date: { year: '2026', month: '01', day: '01' }, + time: { hours: '10', minutes: '30', seconds: '00', format: 'am' }, + qualifiers: { approximate: true, uncertain: false, unknown: false }, + }; + + expect(() => service.toEdtfDate(model)).toThrowError( + /Please check the values/, + ); + }); + + it('should throw when time is filled but the date is missing the day', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '05', day: '' }, + time: { hours: '10', minutes: '30', seconds: '00', format: 'am' }, + }; + + expect(() => service.toEdtfDate(model)).toThrowError( + /complete date is required/i, + ); + }); + + it('should throw when time is filled but the date is missing the month', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '', day: '20' }, + time: { hours: '10', minutes: '30', seconds: '00', format: 'am' }, + }; + + expect(() => service.toEdtfDate(model)).toThrowError( + /complete date is required/i, + ); + }); + + it('should throw when time is filled but the date has only a year', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '', day: '' }, + time: { hours: '10', minutes: '30', seconds: '00', format: 'am' }, + }; + + expect(() => service.toEdtfDate(model)).toThrowError( + /complete date is required/i, + ); + }); + + it('should still emit a partial date when no time is provided', () => { + const model: DateTimeModel = { + date: { year: '1985', month: '05', day: '' }, + time: { format: 'am' }, + }; + + expect(service.toEdtfDate(model)).toBe('1985-05'); + }); }); }); @@ -1159,6 +1393,138 @@ describe('EdtfService', () => { it('should fall back to month 01 (31 days) when month is missing', () => { expect(service.isValidDay('31', '1985', '')).toBe(true); }); + + it('should reject Feb 29 in a non-leap year given a single-digit month', () => { + expect(service.isValidDay('29', '2021', '2')).toBe(false); + }); + + it('should accept Feb 29 in a leap year given a single-digit month', () => { + expect(service.isValidDay('29', '2024', '2')).toBe(true); + }); + }); + + describe('getSegmentError', () => { + const INVALID_MESSAGE = 'invalid characters'; + const RANGE_MESSAGE = 'out of range'; + const rangeOptions = { + invalidCharsMessage: INVALID_MESSAGE, + isWithinRange: (value: string): boolean => parseInt(value, 10) <= 12, + rangeMessage: RANGE_MESSAGE, + }; + + it('should return null for an empty value', () => { + expect(service.getSegmentError('', rangeOptions)).toBeNull(); + }); + + it('should flag non-numeric characters with the provided message', () => { + expect(service.getSegmentError('a5', rangeOptions)).toBe(INVALID_MESSAGE); + }); + + it('should NOT range-check a partially-typed single digit', () => { + expect(service.getSegmentError('9', rangeOptions)).toBeNull(); + }); + + it('should flag a complete out-of-range value with the provided message', () => { + expect(service.getSegmentError('13', rangeOptions)).toBe(RANGE_MESSAGE); + }); + + it('should return null for a complete in-range value', () => { + expect(service.getSegmentError('12', rangeOptions)).toBeNull(); + }); + + it('should skip the range check when no range options are provided', () => { + expect( + service.getSegmentError('9999', { + invalidCharsMessage: INVALID_MESSAGE, + }), + ).toBeNull(); + }); + }); + + describe('getDayError', () => { + const options = { invalidCharsMessage: 'invalid characters' }; + + it('should return null for a valid day in the month', () => { + expect(service.getDayError('20', '1985', '05', options)).toBeNull(); + }); + + it('should return null while only a single digit has been typed', () => { + expect(service.getDayError('9', '2021', '02', options)).toBeNull(); + }); + + it('should flag non-numeric day characters with the provided message', () => { + expect(service.getDayError('a5', '1985', '05', options)).toBe( + options.invalidCharsMessage, + ); + }); + + it('should return the range error for a day greater than 31', () => { + expect(service.getDayError('32', '1985', '01', options)).toBe( + DAY_RANGE_ERROR, + ); + }); + + it('should return the range error for day 00', () => { + expect(service.getDayError('00', '1985', '05', options)).toBe( + DAY_RANGE_ERROR, + ); + }); + + it('should return the day-for-month error for Feb 29 in a non-leap year', () => { + expect(service.getDayError('29', '2021', '02', options)).toBe( + INVALID_DAY_FOR_MONTH_ERROR, + ); + }); + + it('should return the day-for-month error given a single-digit month', () => { + expect(service.getDayError('29', '2021', '2', options)).toBe( + INVALID_DAY_FOR_MONTH_ERROR, + ); + }); + + it('should return null for Feb 29 in a leap year', () => { + expect(service.getDayError('29', '2024', '02', options)).toBeNull(); + }); + + it('should return the range error for a lone "0" day', () => { + expect(service.getDayError('0', '2024', '01', options)).toBe( + DAY_RANGE_ERROR, + ); + }); + + it('should not report a day-for-month error when the month is a lone "0"', () => { + expect(service.getDayError('31', '2024', '0', options)).toBeNull(); + }); + + it('should not report a day-for-month error when the month is out of range', () => { + expect(service.getDayError('31', '2024', '13', options)).toBeNull(); + }); + }); + + describe('getMonthError', () => { + const options = { invalidCharsMessage: 'invalid characters' }; + + it('should return null for a valid two-digit month', () => { + expect(service.getMonthError('12', options)).toBeNull(); + }); + + it('should return null while only a valid single digit has been typed', () => { + expect(service.getMonthError('1', options)).toBeNull(); + }); + + it('should flag non-numeric month characters with the provided message', () => { + expect(service.getMonthError('a5', options)).toBe( + options.invalidCharsMessage, + ); + }); + + it('should return the range error for a month greater than 12', () => { + expect(service.getMonthError('13', options)).toBe(MONTH_RANGE_ERROR); + }); + + it('should return the range error for a lone "0" month', () => { + expect(service.getMonthError('0', options)).toBe(MONTH_RANGE_ERROR); + }); }); describe('roundtrip', () => { @@ -1266,17 +1632,49 @@ describe('EdtfService', () => { expect(result).toBe(edtfString); }); - it('should normalize a partial day (1985-05-2X) to a discrete zero-padded day', () => { - // A day is treated as a discrete value, not an unspecified-digit - // range, so 2X collapses to the 2nd rather than round-tripping. + it('should roundtrip partial year with full month and day (198X-05-20)', () => { + const edtfString = '198X-05-20'; + const model = service.toDateTimeModel(edtfString); + const result = service.toEdtfDate(model); + + expect(result).toBe(edtfString); + }); + + it('should canonicalize a partial day from "1985-05-2X" to "1985-05-02"', () => { + // The parser strips the trailing X, leaving day "2"; the serializer now + // treats a single 1–9 digit as a complete day and left-pads with zero. const model = service.toDateTimeModel('1985-05-2X'); const result = service.toEdtfDate(model); expect(result).toBe('1985-05-02'); }); - it('should roundtrip partial year with full month and day (198X-05-20)', () => { - const edtfString = '198X-05-20'; + it('should roundtrip approximate qualifier with an unspecified month (2026-XX-11~)', () => { + const edtfString = '2026-XX-11~'; + const model = service.toDateTimeModel(edtfString); + const result = service.toEdtfDate(model); + + expect(result).toBe(edtfString); + }); + + it('should roundtrip approximate qualifier with unknown year and month (XXXX-XX-20~)', () => { + const edtfString = 'XXXX-XX-20~'; + const model = service.toDateTimeModel(edtfString); + const result = service.toEdtfDate(model); + + expect(result).toBe(edtfString); + }); + + it('should roundtrip combined qualifier with a partial year (198X-05-20%)', () => { + const edtfString = '198X-05-20%'; + const model = service.toDateTimeModel(edtfString); + const result = service.toEdtfDate(model); + + expect(result).toBe(edtfString); + }); + + it('should roundtrip an interval with per-side qualifiers and an unspecified month', () => { + const edtfString = '2026-XX-11~/2027-01?'; const model = service.toDateTimeModel(edtfString); const result = service.toEdtfDate(model); @@ -1320,5 +1718,60 @@ describe('EdtfService', () => { '1985-05-20', ); }); + + it('should return an empty string for a null input', () => { + expect(service.getEdtfIntervalStartDate(null)).toBe(''); + }); + }); + + describe('formatDateForDisplay', () => { + it('should return an empty string when no field has a value', () => { + expect( + service.formatDateForDisplay({ year: '', month: '', day: '' }), + ).toBe(''); + }); + + it('should render a complete date with the month name', () => { + expect( + service.formatDateForDisplay({ year: '1985', month: '05', day: '20' }), + ).toBe('May 20, 1985'); + }); + + it('should X-pad a partial year like serialization does', () => { + expect( + service.formatDateForDisplay({ year: '198', month: '', day: '' }), + ).toBe('198X'); + }); + + it('should interpret a single-digit month the same way serialization does ("1" -> January)', () => { + expect( + service.formatDateForDisplay({ year: '1985', month: '1', day: '20' }), + ).toBe('January 20, 1985'); + + expect( + service.toEdtfDate({ + date: { year: '1985', month: '1', day: '20' }, + time: { format: 'am' }, + }), + ).toBe('1985-01-20'); + }); + + it('should zero-pad a single-digit day like serialization does', () => { + expect( + service.formatDateForDisplay({ year: '1985', month: '05', day: '2' }), + ).toBe('May 02, 1985'); + }); + + it('should show XX for a missing month when a day is present', () => { + expect( + service.formatDateForDisplay({ year: '1985', month: '', day: '20' }), + ).toBe('1985-XX-20'); + }); + + it('should treat an in-progress "0" day as absent instead of guessing', () => { + expect( + service.formatDateForDisplay({ year: '1985', month: '05', day: '0' }), + ).toBe('May 1985'); + }); }); }); diff --git a/src/app/shared/services/edtf-service/edtf.service.ts b/src/app/shared/services/edtf-service/edtf.service.ts index 14ed0f385..300743f44 100644 --- a/src/app/shared/services/edtf-service/edtf.service.ts +++ b/src/app/shared/services/edtf-service/edtf.service.ts @@ -1,6 +1,13 @@ import { Injectable } from '@angular/core'; import edtf, { Date as EdtfDate, Interval as EdtfInterval } from 'edtf'; -import { getHours, getMinutes, getSeconds, isValid, parse } from 'date-fns'; +import { + format, + getHours, + getMinutes, + getSeconds, + isValid, + parse, +} from 'date-fns'; export enum DateQualifier { Approximate = 'approximate', @@ -25,6 +32,13 @@ export enum EdtfPrecision { export const UNKNOWN_VALUE = 'XXXX-XX-XX'; +export const MONTH_RANGE_ERROR = 'Month must be between 1 and 12.'; +export const DAY_RANGE_ERROR = 'Day must be between 1 and 31.'; +export const INVALID_DAY_FOR_MONTH_ERROR = + 'That day does not exist in the selected month and year.'; + +const DIGITS_ONLY = /^\d*$/; + export const DEFAULT_TIME: TimeModel = { hours: '', minutes: '', @@ -77,7 +91,19 @@ export class EdtfService { return null; } - if (/^X{4}-X{2}-X{2}$/i.test(edtfString)) { + if (edtfString.includes('/')) { + return this.parseInterval(edtfString); + } + + // The library can't parse a qualifier alongside unspecified (X) digits, + // so for those strings we strip the trailing qualifier, parse the base, + // and reattach the flags below. + const hasUnspecified = this.hasUnspecifiedDigits(edtfString); + const { base, approximate, uncertain } = hasUnspecified + ? this.parseTrailingQualifier(edtfString) + : { base: edtfString, approximate: false, uncertain: false }; + + if (/^X{4}-X{2}-X{2}$/i.test(base)) { return { qualifiers: { ...DEFAULT_DATE_QUALIFIERS, unknown: true }, date: { year: '', month: '', day: '' }, @@ -85,24 +111,25 @@ export class EdtfService { }; } - if (edtfString.includes('/')) { - return this.parseInterval(edtfString); - } - - const normalizedString = this.normalizeForParsing(edtfString); + const normalizedString = this.normalizeForParsing(base); const edtfObject = edtf(normalizedString); if (!(edtfObject instanceof EdtfDate)) { return null; } - return this.extDateToDateTimeModel(edtfObject, edtfString); + const model = this.extDateToDateTimeModel(edtfObject, base); + if (hasUnspecified && model.qualifiers && !model.qualifiers.unknown) { + model.qualifiers.approximate = approximate; + model.qualifiers.uncertain = uncertain; + } + return model; } catch (error) { throw new Error(this.toHumanReadableError(error)); } } - getEdtfIntervalStartDate(edtfString: string | undefined): string { + getEdtfIntervalStartDate(edtfString: string | null | undefined): string { if (!edtfString) { return ''; } @@ -116,16 +143,27 @@ export class EdtfService { } private parseInterval(edtfString: string): DateTimeModel | null { - const [startPart, endPart] = edtfString.split('/'); + const [rawStart, rawEnd] = edtfString.split('/'); + + // Only the unspecified-digit (X) case needs the strip-and-reattach + // workaround; without X the library parses per-side qualifiers natively + // (and rejects a qualifier combined with a time, which we keep blocking). + const hasUnspecified = this.hasUnspecifiedDigits(edtfString); + const start = hasUnspecified + ? this.parseTrailingQualifier(rawStart ?? '') + : { base: rawStart ?? '', approximate: false, uncertain: false }; + const end = hasUnspecified + ? this.parseTrailingQualifier(rawEnd ?? '') + : { base: rawEnd ?? '', approximate: false, uncertain: false }; const normalizedStart = - startPart === '..' || startPart === '' - ? startPart - : this.normalizeForParsing(startPart); + start.base === '..' || start.base === '' + ? start.base + : this.normalizeForParsing(start.base); const normalizedEnd = - endPart === '..' || endPart === '' - ? endPart - : this.normalizeForParsing(endPart); + end.base === '..' || end.base === '' + ? end.base + : this.normalizeForParsing(end.base); try { const edtfObject = edtf(`${normalizedStart}/${normalizedEnd}`); @@ -134,7 +172,24 @@ export class EdtfService { return null; } - return this.intervalToDateTimeModel(edtfObject, startPart, endPart); + const model = this.intervalToDateTimeModel( + edtfObject, + start.base, + end.base, + ); + + if (hasUnspecified) { + if (model.qualifiers && !model.qualifiers.unknown) { + model.qualifiers.approximate = start.approximate; + model.qualifiers.uncertain = start.uncertain; + } + if (model.endQualifiers && !model.endQualifiers.unknown) { + model.endQualifiers.approximate = end.approximate; + model.endQualifiers.uncertain = end.uncertain; + } + } + + return model; } catch { return null; } @@ -185,7 +240,14 @@ export class EdtfService { const stringDate = endPart === null ? startPart : `${startPart}/${endPart}`; - edtf(stringDate); + // The edtf library's grammar rejects a qualifier (~/?/%) combined with + // unspecified (X) digits, but that combination is valid EDTF the backend + // accepts, so for those we validate the qualifier-stripped base instead. + edtf( + this.hasUnspecifiedDigits(stringDate) + ? this.stripGroupQualifiers(stringDate) + : stringDate, + ); return stringDate; } catch (error) { throw new Error(this.toHumanReadableError(error)); @@ -197,30 +259,66 @@ export class EdtfService { time: TimeModel, qualifiers?: DateQualifierFlags, ): string { + const hasCompleteDate = !!(date.year && date.month && date.day); + const hasTime = !!time?.hours; + + if (hasTime && !hasCompleteDate) { + throw new Error('A complete date is required when time is provided.'); + } + const dateStr = this.buildDateString(date); const edtfObject = edtf(dateStr); // Strip any time/timezone the library may append (e.g. T00:00:00.000Z) let result = edtfObject.toEDTF().replace(/T.*$/, ''); - const hasCompleteDate = !!(date.year && date.month && date.day); - const timeStr = hasCompleteDate ? this.buildTimeString(time) : ''; + const timeStr = hasTime ? this.buildTimeString(time) : ''; if (timeStr) { result = `${result}${timeStr}`; } // Add qualifiers after the complete date-time string - if (qualifiers?.approximate && qualifiers?.uncertain) { - result += '%'; - } else if (qualifiers?.approximate) { - result += '~'; - } else if (qualifiers?.uncertain) { - result += '?'; - } + result += this.buildQualifierSuffix(qualifiers); return result; } + private hasUnspecifiedDigits(edtfString: string): boolean { + return /X/i.test(edtfString); + } + + private buildQualifierSuffix(qualifiers?: DateQualifierFlags): string { + if (qualifiers?.approximate && qualifiers?.uncertain) return '%'; + if (qualifiers?.approximate) return '~'; + if (qualifiers?.uncertain) return '?'; + return ''; + } + + // Inverse of buildQualifierSuffix: split a trailing group qualifier off a + // single date + private parseTrailingQualifier(part: string): { + base: string; + approximate: boolean; + uncertain: boolean; + } { + const suffix = part.slice(-1); + if (suffix === '%') + return { base: part.slice(0, -1), approximate: true, uncertain: true }; + if (suffix === '~') + return { base: part.slice(0, -1), approximate: true, uncertain: false }; + if (suffix === '?') + return { base: part.slice(0, -1), approximate: false, uncertain: true }; + return { base: part, approximate: false, uncertain: false }; + } + + // Remove group qualifiers (~/?/%) that sit at the end of the whole string or + // at an interval boundary, leaving a base string the edtf library can parse + // even when it contains unspecified (X) digits. Used for serialize-time + // validation only. + private stripGroupQualifiers(edtfString: string): string { + return edtfString.replace(/[%~?](?=\/|$)/g, ''); + } + private buildDateString(date: DateModel): string { const hasYear = !!date.year; const hasMonth = !!date.month; @@ -228,16 +326,49 @@ export class EdtfService { if (!hasYear && !hasMonth && !hasDay) return ''; + // A lone '0' is an unfinished value ('05' minus a keystroke), never a + // month or day on its own — reject it instead of guessing ('0X'). + if (date.month === '0') throw new Error(MONTH_RANGE_ERROR); + if (date.day === '0') throw new Error(DAY_RANGE_ERROR); + const year = this.padWithX(date.year, 4); if (!hasMonth && !hasDay) return year; - const month = hasMonth ? this.padWithX(date.month, 2) : 'XX'; + const month = hasMonth ? this.padMonthOrDay(date.month) : 'XX'; if (!hasDay) return `${year}-${month}`; - // A day is a discrete value, so a single digit is zero-padded on the - // left ("9" -> "09"); X-padding would produce an invalid day (90-99). - const day = date.day.padStart(2, '0'); - return `${year}-${month}-${day}`; + const day = this.padMonthOrDay(date.day); + const dateStr = `${year}-${month}-${day}`; + + // A fully-numeric date with an in-range month and day can still be an + // impossible calendar day (e.g. Feb 29 in a non-leap year, or Apr 31). The + // edtf library silently rolls those forward (2021-02-29 becomes 2021-03-01), + // so reject them here instead of storing the shift + const monthNumber = parseInt(month, 10); + const dayNumber = parseInt(day, 10); + if ( + /^\d{4}-\d{2}-\d{2}$/.test(dateStr) && + monthNumber >= 1 && + monthNumber <= 12 && + dayNumber >= 1 && + dayNumber <= 31 && + !isValid(parse(dateStr, 'yyyy-MM-dd', new Date())) + ) { + throw new Error(INVALID_DAY_FOR_MONTH_ERROR); + } + + return dateStr; + } + + // Shared by serialization (toEdtfDate) and display (formatDateForDisplay) + // so a saved value always reads back the way the preview rendered it. + private padMonthOrDay(value: string): string { + // A single digit is zero-padded ('1' → '01', i.e. January / the 1st). + // '1' could in principle be the start of '10'–'12', but this runs on a + // finished value, not mid-keystroke, and the digits-only inputs give no + // way to type an unspecified digit — so '1X' is not expressible intent. + if (/^[1-9]$/.test(value)) return `0${value}`; + return this.padWithX(value, 2); } private padWithX(value: string, width: number): string { @@ -245,6 +376,40 @@ export class EdtfService { return v.length >= width ? v : v + 'X'.repeat(width - v.length); } + // Human-readable counterpart of buildDateString: both share padWithX and + // padMonthOrDay, so the preview always shows what serialization will write. + // Unlike serialization it must tolerate in-progress values (it renders + // live while the user types), so it never throws. + formatDateForDisplay(date: DateModel): string { + const yearRaw = date.year ?? ''; + const monthRaw = date.month ?? ''; + const dayRaw = date.day ?? ''; + + const hasYear = !!yearRaw; + const hasMonth = !!monthRaw; + // A lone '0' day is an unfinished value ('05' minus a keystroke), so + // the preview treats it as absent rather than guessing. + const hasDay = !!dayRaw && parseInt(dayRaw, 10) !== 0; + + if (!hasYear && !hasMonth && !hasDay) return ''; + + const yearDisplay = this.padWithX(yearRaw, 4); + const monthPadded = hasMonth ? this.padMonthOrDay(monthRaw) : 'XX'; + const monthName = /^\d{2}$/.test(monthPadded) + ? format(new Date(2000, parseInt(monthPadded, 10) - 1), 'MMMM') + : null; + const dayDisplay = hasDay ? this.padMonthOrDay(dayRaw) : ''; + + if (monthName && hasDay) + return `${monthName} ${dayDisplay}, ${yearDisplay}`; + if (monthName) return `${monthName} ${yearDisplay}`; + if (!hasMonth && !hasDay) return yearDisplay; + + const parts: string[] = [yearDisplay, monthPadded]; + if (hasDay) parts.push(dayDisplay); + return parts.join('-'); + } + private buildTimeString(time: TimeModel): string { if (!time?.hours) return ''; @@ -362,6 +527,14 @@ export class EdtfService { return 'The date range is not valid. Please make sure the start date is before the end date.'; } + if (message.includes('complete date is required')) { + return 'A complete date is required when time is provided.'; + } + + // Segment errors (month/day range, day-for-month) are shown inline under + // the offending field, so the footer/toast only needs the generic message — + // no need to duplicate the specific one here. Errors without an inline + // counterpart (the date-range and complete-date cases above) keep theirs. return 'The date entered is not valid. Please check the values and try again.'; } @@ -528,11 +701,97 @@ export class EdtfService { if (/^\d$/.test(value)) return true; // Defaults let day be typed before year/month are filled in. // 2000 is a leap year (allows Feb 29); 01 has 31 days (most permissive). + // A single-digit month is a complete value (e.g. '2' is February), so pad + // it rather than discarding it — otherwise a bad day like Feb 29 would be + // validated against January and wrongly pass. const yearStr = year.length === 4 ? year : '2000'; - const monthStr = month.length === 2 ? month : '01'; + const monthStr = month ? month.padStart(2, '0') : '01'; const dayStr = value.padStart(2, '0'); return isValid( parse(`${yearStr}-${monthStr}-${dayStr}`, 'yyyy-MM-dd', new Date()), ); } + + getSegmentError( + value: string, + options: { + invalidCharsMessage: string; + isWithinRange?: (completeValue: string) => boolean; + rangeMessage?: string; + }, + ): string | null { + if (value === '') return null; + if (!DIGITS_ONLY.test(value)) return options.invalidCharsMessage; + if (value.length < 2) return null; + if (options.isWithinRange && !options.isWithinRange(value)) { + return options.rangeMessage ?? null; + } + return null; + } + + getMonthError( + value: string, + options: { invalidCharsMessage: string }, + ): string | null { + // A lone "0" is an unfinished value ("05" minus a keystroke), not a valid + // month — flag it inline instead of leaving it to serialization. + if (value === '0') return MONTH_RANGE_ERROR; + + return this.getSegmentError(value, { + invalidCharsMessage: options.invalidCharsMessage, + isWithinRange: (month) => this.isValidMonth(month), + rangeMessage: MONTH_RANGE_ERROR, + }); + } + + // Validates a day segment in tiers so each failure gets a specific message: + // a lone "0", then the plain 1–31 range, then whether that day actually + // exists in the selected month/year (e.g. Feb 29 in a non-leap year). + getDayError( + value: string, + year: string, + month: string, + options: { invalidCharsMessage: string }, + ): string | null { + // A lone "0" is an unfinished value ("05" minus a keystroke), not a valid + // day — flag it inline instead of leaving it to serialization. + if (value === '0') return DAY_RANGE_ERROR; + + const rangeError = this.getSegmentError(value, { + invalidCharsMessage: options.invalidCharsMessage, + isWithinRange: (day) => this.isDayInRange(day), + rangeMessage: DAY_RANGE_ERROR, + }); + if (rangeError) return rangeError; + + // Only cross-check the day against the month when the month is itself a + // valid month; when it is not (e.g. "0" or "13"), the error belongs to the + // month field, so we must not mislabel it as a day-for-month problem. + if ( + value.length === 2 && + this.isResolvableMonth(month) && + !this.isValidDay(value, year, month) + ) { + return INVALID_DAY_FOR_MONTH_ERROR; + } + + return null; + } + + // The plain 1–31 numeric range, independent of month/year — the first tier of + // getDayError, kept separate from the calendar check so an out-of-range day + // and a day that does not exist in the month get different messages. + private isDayInRange(value: string): boolean { + const dayNumber = parseInt(value, 10); + return dayNumber >= 1 && dayNumber <= 31; + } + + // A month that resolves to a real 1–12 value (single digit or two digits). + // Used to gate the day-for-month check so an invalid month does not surface + // as a day error. + private isResolvableMonth(month: string): boolean { + if (!/^\d{1,2}$/.test(month)) return false; + const monthNumber = parseInt(month, 10); + return monthNumber >= 1 && monthNumber <= 12; + } } diff --git a/src/styles/_mixins.scss b/src/styles/_mixins.scss index 51d51de94..564686cfb 100644 --- a/src/styles/_mixins.scss +++ b/src/styles/_mixins.scss @@ -44,6 +44,17 @@ box-shadow: 0px 0px 0px 4px #131b4a08; } +@mixin input-error-state { + border-color: $red; + background: rgba($red, 0.06); +} + +@mixin input-error-message { + display: block; + font-size: 12px; + color: $red; +} + @mixin icon-wrapper { background: $PR-blue-25; border-radius: 0 8px 8px 0;