[NAE-2425] Allowing using seconds in dateTime field#327
Open
machacjozef wants to merge 2 commits intorelease/6.4.3from
Open
[NAE-2425] Allowing using seconds in dateTime field#327machacjozef wants to merge 2 commits intorelease/6.4.3from
machacjozef wants to merge 2 commits intorelease/6.4.3from
Conversation
- Introduces `DATE_TIME_SECONDS_FORMAT` for finer-grained date-time representation. - Refactors `FieldConverterService` with a reusable `resolveDateTime` method. - Enhances `DateTimeField` with seconds display and configurable steps. - Updates datetime-picker to include seconds and toggle meridian mode.
|
Member
Author
|
@coderabbitai Hey, check out my PR :) |
|
Hey ✅ Actions performedReview triggered.
|
WalkthroughThis PR adds seconds-level precision support to datetime fields by introducing configurable property keys, parsing utilities, and a new date format. Changes span base component helpers, datetime field models, format definitions, service consolidation, and template/provider updates to enable dynamic seconds visibility and step configuration. ChangesDatetime Seconds Precision Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@projects/netgrif-components-core/src/lib/data-fields/date-time-field/models/date-time-field.ts`:
- Line 7: DateTimeField currently imports AbstractBaseDataFieldComponent for
static parsing helpers; remove that dependency by extracting the parsing logic
into a neutral utility (e.g., FieldPropertyParser or date-time specific parser
util) and have DateTimeField use the new util instead; update the static parser
helpers referenced on the DateTimeField model (and any usages in
AbstractBaseDataFieldComponent) to import from the new util so the model no
longer imports the UI component class.
- Around line 31-51: The getters stepHour, stepMinute, and stepSecond should
validate the resolved number to ensure it is a positive integer (>0) and
otherwise fall back to their defaults (1, 5, 1); change each getter to call
AbstractBaseDataFieldComponent.resolveNumberProperty as now, then
coerce/validate the result from DateTimeField.STEP_HOUR_PROPERTY /
STEP_MINUTE_PROPERTY / STEP_SECOND_PROPERTY into a positive integer (e.g.,
Math.floor or equivalent) and if the value is not a finite integer > 0 return
the respective default.
In
`@projects/netgrif-components-core/src/lib/task-content/services/field-converter.service.ts`:
- Around line 317-340: In resolveDateTime, avoid treating falsy-but-valid inputs
like 0 as empty and guard against invalid Moment results: replace the initial if
(!value) with a nullish/empty-string check (e.g. if (value == null || value ===
'') return undefined), keep the existing branches for moment, Date and Array,
but after creating a Moment from Array or generic moment(value) verify the
result is valid (use moment.isMoment(...) || result.isValid()) and return
undefined if invalid; reference the resolveDateTime function, the Array
destructuring branch, and the final return moment(value) conversion when adding
these validity checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f6025c83-2cfa-49ad-9a2b-b4da0adcc245
📒 Files selected for processing (7)
projects/netgrif-components-core/src/lib/data-fields/base-component/abstract-base-data-field.component.tsprojects/netgrif-components-core/src/lib/data-fields/date-time-field/date-time-default-field/abstract-date-time-default-field.component.tsprojects/netgrif-components-core/src/lib/data-fields/date-time-field/models/date-time-field.tsprojects/netgrif-components-core/src/lib/moment/time-formats.tsprojects/netgrif-components-core/src/lib/task-content/services/field-converter.service.tsprojects/netgrif-components/src/lib/data-fields/date-time-field/date-time-default-field/date-time-default-field.component.htmlprojects/netgrif-components/src/lib/data-fields/date-time-field/date-time-default-field/date-time-default-field.component.ts
| import {Layout} from '../../models/layout'; | ||
| import {Validation} from '../../models/validation'; | ||
| import {Component, ComponentPrefixes} from '../../models/component'; | ||
| import {AbstractBaseDataFieldComponent} from '../../base-component/abstract-base-data-field.component'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Decouple DateTimeField model from AbstractBaseDataFieldComponent.
Importing a UI component class into a model for static parsing helpers couples layers unnecessarily. Move these parsers into a neutral utility (e.g., field-property parser util) and reuse from both model/component layers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@projects/netgrif-components-core/src/lib/data-fields/date-time-field/models/date-time-field.ts`
at line 7, DateTimeField currently imports AbstractBaseDataFieldComponent for
static parsing helpers; remove that dependency by extracting the parsing logic
into a neutral utility (e.g., FieldPropertyParser or date-time specific parser
util) and have DateTimeField use the new util instead; update the static parser
helpers referenced on the DateTimeField model (and any usages in
AbstractBaseDataFieldComponent) to import from the new util so the model no
longer imports the UI component class.
Comment on lines
+31
to
+51
| public get stepHour(): number { | ||
| return AbstractBaseDataFieldComponent.resolveNumberProperty( | ||
| this.component?.properties?.[DateTimeField.STEP_HOUR_PROPERTY], | ||
| 1 | ||
| ); | ||
| } | ||
|
|
||
| public get stepMinute(): number { | ||
| return AbstractBaseDataFieldComponent.resolveNumberProperty( | ||
| this.component?.properties?.[DateTimeField.STEP_MINUTE_PROPERTY], | ||
| 5 | ||
| ); | ||
| } | ||
|
|
||
|
|
||
| public get stepSecond(): number { | ||
| return AbstractBaseDataFieldComponent.resolveNumberProperty( | ||
| this.component?.properties?.[DateTimeField.STEP_SECOND_PROPERTY], | ||
| 1 | ||
| ); | ||
| } |
There was a problem hiding this comment.
Constrain step properties to positive integers before exposing them.
stepHour, stepMinute, and stepSecond currently pass through any finite number (including 0, negative, or decimal). Add a > 0 integer guard and fallback defaults.
Proposed fix
public get stepHour(): number {
- return AbstractBaseDataFieldComponent.resolveNumberProperty(
+ const value = AbstractBaseDataFieldComponent.resolveNumberProperty(
this.component?.properties?.[DateTimeField.STEP_HOUR_PROPERTY],
1
);
+ return Number.isInteger(value) && value > 0 ? value : 1;
}
public get stepMinute(): number {
- return AbstractBaseDataFieldComponent.resolveNumberProperty(
+ const value = AbstractBaseDataFieldComponent.resolveNumberProperty(
this.component?.properties?.[DateTimeField.STEP_MINUTE_PROPERTY],
5
);
+ return Number.isInteger(value) && value > 0 ? value : 5;
}
public get stepSecond(): number {
- return AbstractBaseDataFieldComponent.resolveNumberProperty(
+ const value = AbstractBaseDataFieldComponent.resolveNumberProperty(
this.component?.properties?.[DateTimeField.STEP_SECOND_PROPERTY],
1
);
+ return Number.isInteger(value) && value > 0 ? value : 1;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public get stepHour(): number { | |
| return AbstractBaseDataFieldComponent.resolveNumberProperty( | |
| this.component?.properties?.[DateTimeField.STEP_HOUR_PROPERTY], | |
| 1 | |
| ); | |
| } | |
| public get stepMinute(): number { | |
| return AbstractBaseDataFieldComponent.resolveNumberProperty( | |
| this.component?.properties?.[DateTimeField.STEP_MINUTE_PROPERTY], | |
| 5 | |
| ); | |
| } | |
| public get stepSecond(): number { | |
| return AbstractBaseDataFieldComponent.resolveNumberProperty( | |
| this.component?.properties?.[DateTimeField.STEP_SECOND_PROPERTY], | |
| 1 | |
| ); | |
| } | |
| public get stepHour(): number { | |
| const value = AbstractBaseDataFieldComponent.resolveNumberProperty( | |
| this.component?.properties?.[DateTimeField.STEP_HOUR_PROPERTY], | |
| 1 | |
| ); | |
| return Number.isInteger(value) && value > 0 ? value : 1; | |
| } | |
| public get stepMinute(): number { | |
| const value = AbstractBaseDataFieldComponent.resolveNumberProperty( | |
| this.component?.properties?.[DateTimeField.STEP_MINUTE_PROPERTY], | |
| 5 | |
| ); | |
| return Number.isInteger(value) && value > 0 ? value : 5; | |
| } | |
| public get stepSecond(): number { | |
| const value = AbstractBaseDataFieldComponent.resolveNumberProperty( | |
| this.component?.properties?.[DateTimeField.STEP_SECOND_PROPERTY], | |
| 1 | |
| ); | |
| return Number.isInteger(value) && value > 0 ? value : 1; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@projects/netgrif-components-core/src/lib/data-fields/date-time-field/models/date-time-field.ts`
around lines 31 - 51, The getters stepHour, stepMinute, and stepSecond should
validate the resolved number to ensure it is a positive integer (>0) and
otherwise fall back to their defaults (1, 5, 1); change each getter to call
AbstractBaseDataFieldComponent.resolveNumberProperty as now, then
coerce/validate the result from DateTimeField.STEP_HOUR_PROPERTY /
STEP_MINUTE_PROPERTY / STEP_SECOND_PROPERTY into a positive integer (e.g.,
Math.floor or equivalent) and if the value is not a finite integer > 0 return
the respective default.
Comment on lines
+317
to
+340
| protected resolveDateTime(value: any): Moment | undefined { | ||
| if (!value) { | ||
| return undefined; | ||
| } | ||
| if (moment.isMoment(value)) { | ||
| return value; | ||
| } | ||
| if (value instanceof Date) { | ||
| return moment(value); | ||
| } | ||
| if (Array.isArray(value)) { | ||
| const [year, month, day, hour = 0, minute = 0, second = 0, millisecond = 0] = value; | ||
| return moment({ | ||
| year, | ||
| month: month - 1, | ||
| date: day, | ||
| hour, | ||
| minute, | ||
| second, | ||
| millisecond | ||
| }); | ||
| } | ||
| return moment(value); | ||
| } |
There was a problem hiding this comment.
Handle falsy-but-valid input and invalid parse output in resolveDateTime.
Line 318 currently treats 0 as empty, and invalid parsed values are returned unguarded. Use a nullish/empty-string check and return undefined for invalid parse results.
Proposed fix
protected resolveDateTime(value: any): Moment | undefined {
- if (!value) {
+ if (value === undefined || value === null || value === '') {
return undefined;
}
+ let parsed: Moment;
if (moment.isMoment(value)) {
- return value;
+ parsed = value;
+ } else if (value instanceof Date) {
+ parsed = moment(value);
+ } else if (Array.isArray(value)) {
+ const [year, month, day, hour = 0, minute = 0, second = 0, millisecond = 0] = value;
+ parsed = moment({
+ year,
+ month: month - 1,
+ date: day,
+ hour,
+ minute,
+ second,
+ millisecond
+ });
+ } else {
+ parsed = moment(value);
}
- if (value instanceof Date) {
- return moment(value);
- }
- if (Array.isArray(value)) {
- const [year, month, day, hour = 0, minute = 0, second = 0, millisecond = 0] = value;
- return moment({
- year,
- month: month - 1,
- date: day,
- hour,
- minute,
- second,
- millisecond
- });
- }
- return moment(value);
+ return parsed.isValid() ? parsed : undefined;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@projects/netgrif-components-core/src/lib/task-content/services/field-converter.service.ts`
around lines 317 - 340, In resolveDateTime, avoid treating falsy-but-valid
inputs like 0 as empty and guard against invalid Moment results: replace the
initial if (!value) with a nullish/empty-string check (e.g. if (value == null ||
value === '') return undefined), keep the existing branches for moment, Date and
Array, but after creating a Moment from Array or generic moment(value) verify
the result is valid (use moment.isMoment(...) || result.isValid()) and return
undefined if invalid; reference the resolveDateTime function, the Array
destructuring branch, and the final return moment(value) conversion when adding
these validity checks.
Retoocs
approved these changes
May 6, 2026
Kovy95
approved these changes
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
DATE_TIME_SECONDS_FORMATfor finer-grained date-time representation.FieldConverterServicewith a reusableresolveDateTimemethod.DateTimeFieldwith seconds display and configurable steps.new property:
Implements NAE-2425
Dependencies
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc. You can use >
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Enhancements