Skip to content

AAE-43749 Added date formatting according to locale#11769

Open
tomaszhanaj wants to merge 2 commits intodevelopfrom
fix/AAE-43749-date-formatting
Open

AAE-43749 Added date formatting according to locale#11769
tomaszhanaj wants to merge 2 commits intodevelopfrom
fix/AAE-43749-date-formatting

Conversation

@tomaszhanaj
Copy link
Copy Markdown
Contributor

Please check if the PR fulfills these requirements

  • [x ] The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation
  • Other... Please describe:

What is the current behaviour? (You can also link to an open issue here)
https://hyland.atlassian.net/browse/AAE-43749

What is the new behaviour?
Date is formatted according to locale. In angular DatePipe 'short' is an equivalent of 'M/d/yy, h:mm a' which is similar to previous default format 'dd/MM/yyyy HH:mm'; and is same as an example showed on Admin Portal
https://angular.dev/api/common/DatePipe#usage-notes
image

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@tomaszhanaj tomaszhanaj requested a review from eromano as a code owner March 27, 2026 12:25
Copilot AI review requested due to automatic review settings March 27, 2026 12:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates TimeAgoPipe to format “older than 7 days” timestamps using Angular DatePipe’s locale-aware 'short' format instead of a fixed date-time pattern, aligning absolute date output with the active locale.

Changes:

  • Remove configurable defaultDateTimeFormat usage in TimeAgoPipe and switch absolute formatting to DatePipe(...).transform(..., 'short').
  • Expand unit tests to cover locale-aware absolute formatting and additional relative/locale behaviors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
lib/core/src/lib/pipes/time-ago.pipe.ts Switches absolute date formatting to locale-aware 'short' and removes app-config-based format selection.
lib/core/src/lib/pipes/time-ago.pipe.spec.ts Adds/updates tests for locale-aware formatting and relative/absolute behavior, including clock mocking and locale registration.

Comment on lines +37 to +42
if (diff > 7) {
const datePipe: DatePipe = new DatePipe(actualLocale);
return datePipe.transform(value, this.defaultDateTimeFormat);
return datePipe.transform(value, 'short', null, actualLocale);
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

This change removes support for the configurable dateValues.defaultDateTimeFormat (defined in app config schema) and hard-codes the output to DatePipe's 'short'. This is a behavioral/config compatibility change for anyone relying on dateValues.defaultDateTimeFormat to control the absolute (older-than-7-days) formatting. Consider keeping the app-config override as the primary format and falling back to 'short' (or the previous default) when not configured, while still passing the resolved locale to DatePipe.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

With this default format locale is not applied.

if (diff > 7) {
const datePipe: DatePipe = new DatePipe(actualLocale);
return datePipe.transform(value, this.defaultDateTimeFormat);
return datePipe.transform(value, 'short', null, actualLocale);
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

DatePipe.transform(...) can return null (e.g., if an invalid date slips through), but this pipe otherwise returns strings. To avoid propagating null into templates/consumers, coalesce the result to an empty string (or a defined fallback) before returning.

Suggested change
return datePipe.transform(value, 'short', null, actualLocale);
return datePipe.transform(value, 'short', null, actualLocale) ?? '';

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated

Comment on lines +51 to 56
jasmine.clock().uninstall();
jasmine.clock().install();
jasmine.clock().mockDate(NOW);
jasmine.clock().mockDate(new Date('2026-03-27T12:00:00.000Z'));
});

Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

The Jasmine clock is installed in beforeEach but never uninstalled in an afterEach, and uninstall() is called unconditionally (which can throw if the clock wasn't installed). This can leak the mocked clock into other specs and cause unrelated test failures. Prefer installing in beforeEach and uninstalling in afterEach, and remove the redundant second mockDate call.

Suggested change
jasmine.clock().uninstall();
jasmine.clock().install();
jasmine.clock().mockDate(NOW);
jasmine.clock().mockDate(new Date('2026-03-27T12:00:00.000Z'));
});
jasmine.clock().install();
jasmine.clock().mockDate(NOW);
});
afterEach(() => {
jasmine.clock().uninstall();
});

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

without jasmine.clock().uninstall(); in beforeEach it throws an error
Error: Jasmine Clock was unable to install over custom global timer functions. Is the clock already installed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

really weird... maybe worth wrapping with try...catch then

Comment on lines +62 to 72
it('should return exact date if given date is more than seven days for en locale ', () => {
const date = new Date('1990-11-03T15:25:42.749');
expect(pipe.transform(date)).toBe('03/11/1990 15:25');
expect(pipe.transform(date)).toBe('11/3/90, 3:25 PM');
});

it('should return exact date if given date is more than seven days for de locale ', () => {
localeSignalSpy.and.returnValue('de');
const date = new Date('1990-11-03T15:25:42.749');

expect(pipe.transform(date)).toBe('03.11.90, 15:25');
});
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

These assertions hard-code locale-specific formatted strings derived from a date created without an explicit timezone (1990-11-03T15:25:42.749). DatePipe formats in the environment's local timezone, so the expected output can change depending on CI machine timezone and lead to flaky tests. Make the test deterministic by using a UTC date (with Z) and/or asserting against new DatePipe(locale).transform(date, 'short', 'UTC', locale) rather than a hard-coded string.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would rather to leave it to show what is the expected string for related locale.


const result = pipe.transform(oldDate, 'de');

const expected = '01.03.26, 10:00';
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

This test hard-codes the expected DatePipe output ('01.03.26, 10:00') which is sensitive to runtime timezone/ICU differences. To avoid flakiness, compute the expected value via DatePipe using the same locale/format/timezone as the production code (ideally pinning a timezone like UTC for the assertion).

Suggested change
const expected = '01.03.26, 10:00';
const expected = new DatePipe('de').transform(oldDate, 'short', null, 'de');

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is to test specific case

@sonarqubecloud
Copy link
Copy Markdown

const userPreferences = TestBed.inject(UserPreferencesService);
localeSignalSpy = userPreferences.localeSignal as unknown as jasmine.Spy<() => string>;

jasmine.clock().uninstall();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

to provide better testability, maybe you can introduce a dedicated function you can easily mock without jasmine clock mocking? you can have something like "getCurrentDateTime()" on the pipe level, and then mock it as you need?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants