Skip to content

Commit 6b761e6

Browse files
XingYXingY
committed
Issue 52820: Sample Manager: editing datetime/time fields in app with display format could result in time precision loss
1 parent 669ac50 commit 6b761e6

11 files changed

Lines changed: 120 additions & 120 deletions

File tree

‎packages/components/package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/components/src/internal/components/EditInlineField.tsx‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
getColDateFormat,
77
getDateFNSDateFormat,
88
getJsonDateTimeFormatString,
9-
getJsonDateFormatString,
9+
getJsonDateFormatString, getJsonTimeFormatString,
1010
} from '../util/Date';
1111
import { Key, useEnterEscape } from '../../public/useEnterEscape';
1212

@@ -69,7 +69,6 @@ export const EditInlineField: FC<Props> = memo(props => {
6969
const inputRef = useRef(null);
7070
const _value = typeof value === 'object' ? value?.value : value;
7171
const [dateValue, setDateValue] = useState<Date>(() => (isDate && _value ? new Date(_value) : undefined));
72-
const [timeJsonValue, setTimeJsonValue] = useState<string>(undefined);
7372
const [columnBasedValue, setColumnBasedValue] = useState();
7473

7574
// Utilizing useReducer here so multiple state attributes can be updated at once
@@ -107,7 +106,7 @@ export const EditInlineField: FC<Props> = memo(props => {
107106
}, [dateFormat, emptyText, isDate, value, _value]);
108107

109108
const getInputValue = useCallback((): any => {
110-
if (isTime) return timeJsonValue;
109+
if (isTime) return getJsonTimeFormatString(dateValue);
111110
if (isDate) {
112111
if (useJsonDateFormat) {
113112
return isDateOnly ? getJsonDateFormatString(dateValue) : getJsonDateTimeFormatString(dateValue);
@@ -116,7 +115,7 @@ export const EditInlineField: FC<Props> = memo(props => {
116115
}
117116
if (column) return columnBasedValue;
118117
return inputRef.current?.value;
119-
}, [dateValue, timeJsonValue, isDate, isTime, columnBasedValue, column, useJsonDateFormat, isDateOnly]);
118+
}, [dateValue, isDate, isTime, columnBasedValue, column, useJsonDateFormat, isDateOnly]);
120119

121120
const onCancel = useCallback((): void => {
122121
setState({ editing: false, ignoreBlur: true });
@@ -147,23 +146,11 @@ export const EditInlineField: FC<Props> = memo(props => {
147146
}, [allowBlank, getInputValue, isDate, onCancel, saveEdit, state.ignoreBlur]);
148147

149148
const onDateChange = useCallback(
150-
(date: Date | string) => {
149+
(date: Date) => {
151150
if (date instanceof Array) throw new Error('Unsupported date/time type');
152-
153-
if (!date) {
154-
if (isDate) setDateValue(undefined);
155-
else setTimeJsonValue(undefined);
156-
}
157-
158-
if (typeof date === 'string') {
159-
if (!isDate) setTimeJsonValue(date);
160-
}
161-
else {
162-
if (isDate)
163-
setDateValue(date);
164-
}
151+
setDateValue(date);
165152
},
166-
[isDate]
153+
[]
167154
);
168155

169156
const onFormsyColumnChange = useCallback(

‎packages/components/src/internal/components/editable/Cell.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ export interface CellProps extends SharedProps {
185185
row?: any;
186186
rowIdx: number;
187187
values?: List<ValueDescriptor>;
188+
getDisplayValue?: (vd: ValueDescriptor) => string;
188189
}
189190

190191
interface State {
@@ -541,6 +542,7 @@ export class Cell extends React.PureComponent<CellProps, State> {
541542
selection,
542543
values,
543544
containerPath,
545+
getDisplayValue
544546
} = this.props;
545547

546548
const alignRight = col.align === 'right';
@@ -549,7 +551,8 @@ export class Cell extends React.PureComponent<CellProps, State> {
549551
if (!focused) {
550552
const displayValue = values
551553
.filter(vd => vd && vd.display !== undefined)
552-
.reduce((v, vd, i) => v + (i > 0 ? ', ' : '') + vd.display, '');
554+
.reduce((v, vd, i) =>
555+
v + (i > 0 ? ', ' : '') + (getDisplayValue?.(vd) ?? vd.display), '');
553556

554557
return (
555558
<>

‎packages/components/src/internal/components/editable/DateInputCell.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import React, { FC, memo, useCallback } from 'react';
33
import { QueryColumn } from '../../../public/QueryColumn';
44

55
import { DatePickerInput } from '../forms/input/DatePickerInput';
6-
import { formatDate, formatDateTime, isDateTimeCol } from '../../util/Date';
6+
import { formatDate, formatDateTime, formatTime, isDateTimeCol } from '../../util/Date';
77

88
import { MODIFICATION_TYPES, SELECTION_TYPES } from './constants';
99
import { ValueDescriptor } from './models';
@@ -39,7 +39,7 @@ export const DateInputCell: FC<DateInputCellProps> = memo(props => {
3939
if (!display) {
4040
if (newDate && typeof newDate === 'string') display = newDate;
4141
else if (newDate && newDate instanceof Date) {
42-
display = isDateTimeCol(col) ? formatDateTime(newDate) : formatDate(newDate);
42+
display = col.isTimeColumn ? formatTime(newDate): (isDateTimeCol(col) ? formatDateTime(newDate) : formatDate(newDate));
4343
}
4444
}
4545

‎packages/components/src/internal/components/editable/EditableGrid.tsx‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import { AddRowsControl, AddRowsControlProps, PlacementType } from './Controls';
7373
import { CellMessage, EditableColumnMetadata, EditorModel, EditorModelProps, ValueDescriptor } from './models';
7474
import { computeRangeChange, genCellKey, getValidatedEditableGridValue, parseCellKey } from './utils';
7575
import { RemoveColumnMenuItem } from './RemoveColumnMenuItem';
76+
import { formatDateTimeDisplayValueForUpdate } from '../../util/Date';
7677

7778
function anyCell(values: List<ValueDescriptor>): boolean {
7879
return true;
@@ -168,7 +169,8 @@ function inputCellFactory(
168169
containerFilter: Query.ContainerFilter,
169170
forUpdate: boolean,
170171
initialSelection: string[],
171-
containerPath?: string
172+
containerPath?: string,
173+
getDisplayValue?: (vd: ValueDescriptor) => string,
172174
): GridColumnCellRenderer {
173175
// Note: We ignore the incoming value (_) and rowNumber (__) because they come from the underlying QueryModel that
174176
// backs the Grid component, but we need to reference the data that is in the EditorModel.
@@ -258,6 +260,7 @@ function inputCellFactory(
258260
values={editorModel.getValue(fieldKey, rowIdx)}
259261
linkedValues={linkedValues}
260262
containerPath={containerPath}
263+
getDisplayValue={getDisplayValue}
261264
/>
262265
</td>
263266
);
@@ -872,6 +875,12 @@ export class EditableGrid extends PureComponent<EditableGridProps, EditableGridS
872875
}
873876
}
874877
const hideTooltip = metadata?.hideTitleTooltip ?? qCol.hasHelpTipData;
878+
let getDisplayValue = null;
879+
if (qCol.isTimeColumn || qCol.jsonType === 'date') {
880+
getDisplayValue = (vd) => {
881+
return formatDateTimeDisplayValueForUpdate(vd, qCol);
882+
}
883+
}
875884
gridColumns = gridColumns.push(
876885
new GridColumn({
877886
align: qCol.align,
@@ -885,7 +894,8 @@ export class EditableGrid extends PureComponent<EditableGridProps, EditableGridS
885894
metadata?.containerFilter ?? containerFilter,
886895
forUpdate,
887896
this.state.initialSelection,
888-
containerPath
897+
containerPath,
898+
getDisplayValue
889899
),
890900
index: qCol.fieldKey,
891901
fixedWidth,

‎packages/components/src/internal/components/editable/utils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ import { Operation, QueryColumn } from '../../../public/QueryColumn';
44

55
import {
66
getColDateFormat,
7-
getFormattedStringFromDate,
87
getJsonDateFormatString,
98
getJsonDateTimeFormatString,
9+
getJsonTimeFormatString,
1010
parseDate,
1111
parseTime,
1212
} from '../../util/Date';
@@ -55,7 +55,7 @@ export const getValidatedEditableGridValue = (origValue: any, col: QueryColumn):
5555
if (validValues.indexOf(trimmed) === -1) message = `'${trimmed}' is not a valid choice`;
5656
} else if (jsonType === 'time') {
5757
const time = parseTime(value);
58-
if (time) value = getFormattedStringFromDate(time, col, false);
58+
if (time) value = getJsonTimeFormatString(time);
5959
else message = 'Invalid time';
6060
} else if (jsonType === 'boolean' && !isBoolean(value)) {
6161
message = 'Invalid boolean';

‎packages/components/src/internal/components/forms/QueryInfoForm.tsx‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,8 @@ export const getUpdatedFields = (queryInfo: QueryInfo, data: any, submitForEdit?
5252
for (const key in data) {
5353
if (data.hasOwnProperty(key)) {
5454
if (fieldsToUpdate.has(key.toLowerCase()) || additionalFields.indexOf(key) !== -1) {
55-
// Date values are Dates not strings. We convert them to strings in the desired format here.
56-
// They are converted back to Dates when saving to the server.
5755
const col = queryInfo?.getColumn(key);
58-
if (submitForEdit && col?.jsonType === 'date') {
59-
if (col.isDateOnlyColumn)
60-
filteredData = filteredData.set(key, formatDate(data[key], null, col.format));
61-
else filteredData = filteredData.set(key, formatDateTime(data[key], null, col.format));
62-
} else if (submitForEdit && col?.jsonType === 'time') {
63-
filteredData = filteredData.set(key, formatTime(data[key], col.format));
64-
} else if (col?.jsonType === 'string' && typeof data[key] === 'string') {
56+
if (col?.jsonType === 'string' && typeof data[key] === 'string') {
6557
filteredData = filteredData.set(key, data[key]?.trim());
6658
} else {
6759
filteredData = filteredData.set(key, data[key]);

‎packages/components/src/internal/components/forms/detail/DetailDisplay.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,6 @@ export function resolveDetailEditRenderer(
408408
showLabel={showLabel}
409409
value={value}
410410
wrapperClassName={DETAIL_INPUT_WRAPPER_CLASS_NAME}
411-
initValueFormatted
412411
/>
413412
);
414413
}

‎packages/components/src/internal/components/forms/input/DatePickerInput.tsx‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import DatePicker from 'react-datepicker';
1919
import { FormsyInjectedProps, withFormsy } from '../formsy';
2020
import { FieldLabel } from '../FieldLabel';
2121
import {
22-
getFormattedStringFromDate,
22+
getDateFromISO,
2323
getJsonDateFormatString,
2424
getJsonDateTimeFormatString,
2525
getJsonTimeFormatString,
@@ -43,7 +43,6 @@ export interface DatePickerInputProps extends DisableableInputProps {
4343
disabled?: boolean;
4444
formsy?: boolean;
4545
hideTime?: boolean;
46-
initValueFormatted?: boolean;
4746
inlineEdit?: boolean;
4847
inputClassName?: string;
4948
inputWrapperClassName?: string;
@@ -79,7 +78,6 @@ export class DatePickerInputImpl extends DisableableInput<DatePickerInputImplPro
7978
allowDisable: false,
8079
containerClassName: INPUT_CONTAINER_CLASS_NAME,
8180
initiallyDisabled: false,
82-
initValueFormatted: false,
8381
inputClassName: 'form-control',
8482
inputWrapperClassName: 'block',
8583
isClearable: true,
@@ -156,22 +154,12 @@ export class DatePickerInputImpl extends DisableableInput<DatePickerInputImplPro
156154
};
157155

158156
getInitDate(props: DatePickerInputProps, minDate?: Date): Date {
159-
const { allowRelativeInput, initValueFormatted, queryColumn, value } = props;
160-
161-
if (!value || (allowRelativeInput && isRelativeDateFilterValue(value))) return undefined;
162-
163-
if (queryColumn.isTimeColumn) {
164-
return parseTime(value);
165-
}
166-
167-
// Issue 45140: props.value is the original formatted date, so pass the date format
168-
// to parseDate when getting the initial value.
169-
const dateFormat = initValueFormatted ? this.getDateFormat() : undefined;
170-
return parseDate(value, dateFormat, minDate, false, queryColumn.isDateOnlyColumn);
157+
const { allowRelativeInput, queryColumn, value } = props;
158+
return getDateFromISO(value, queryColumn, allowRelativeInput, minDate)
171159
}
172160

173161
onChange = (date: Date, event?: any, raw?: boolean): void => {
174-
const { formsy, hideTime, inlineEdit, queryColumn } = this.props;
162+
const { onChange, formsy, hideTime, inlineEdit, queryColumn } = this.props;
175163

176164
if (!event && !raw && date?.getMilliseconds() > 0) {
177165
date.setMilliseconds(0); // react-datepicker milliseconds are not 0 when selecting time
@@ -180,11 +168,9 @@ export class DatePickerInputImpl extends DisableableInput<DatePickerInputImplPro
180168
this.setState({ selectedDate: date, invalid: false, invalidStart: false });
181169

182170
if (this.state.relativeInputValue) {
183-
this.props.onChange?.(this.state.relativeInputValue, this.state.relativeInputValue);
171+
this.props.onChange?.(this.state.relativeInputValue);
184172
} else {
185-
const formatted = getFormattedStringFromDate(date, queryColumn, hideTime);
186-
187-
this.props.onChange?.(queryColumn.isTimeColumn ? formatted : date, formatted);
173+
onChange?.(date);
188174

189175
if (formsy) {
190176
this.props.setValue?.(this.getFormsyValue(date));
@@ -251,7 +237,7 @@ export class DatePickerInputImpl extends DisableableInput<DatePickerInputImplPro
251237
inlineEdit,
252238
} = this.props;
253239
const { isDisabled, selectedDate, invalid, invalidStart } = this.state;
254-
const { dateFormat, timeFormat } = getPickerDateAndTimeFormat(queryColumn, hideTime);
240+
const { dateFormat, timeFormat } = getPickerDateAndTimeFormat(queryColumn, hideTime, selectedDate);
255241
const validValueInvalidStart = !invalid && invalidStart;
256242
const isTimeOnly = queryColumn.isTimeColumn;
257243
const picker = (

‎packages/components/src/internal/util/Date.test.ts‎

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -359,40 +359,6 @@ describe('Date Utilities', () => {
359359
});
360360
});
361361

362-
describe('getFormattedStringFromDate', () => {
363-
const datePOSIX = 1596750283812; // Aug 6, 2020 21:44 UTC
364-
const testDate = new Date(datePOSIX);
365-
const invalidDate = new Date(NaN);
366-
367-
const dateOnlyColumn = new QueryColumn({ rangeURI: DATE_TYPE.rangeURI });
368-
const timeColumn = new QueryColumn({ rangeURI: TIME_TYPE.rangeURI });
369-
370-
test('preconditions', () => {
371-
expect(dateOnlyColumn.isDateOnlyColumn).toBe(true);
372-
expect(timeColumn.isTimeColumn).toBe(true);
373-
});
374-
375-
test('invalid date', () => {
376-
expect(getFormattedStringFromDate(undefined, timeColumn)).toBeUndefined();
377-
expect(getFormattedStringFromDate(null, timeColumn)).toBeUndefined();
378-
expect(getFormattedStringFromDate(invalidDate, timeColumn)).toBeUndefined();
379-
});
380-
381-
test('uses column format', () => {
382-
const columnFormat = 'yyyy-dd-MM-dd-yyyy';
383-
const dateOnlyColumnWithFormat = dateOnlyColumn.mutate({ format: columnFormat });
384-
const timeColumnWithFormat = timeColumn.mutate({ format: columnFormat });
385-
386-
expect(getFormattedStringFromDate(testDate, dateOnlyColumnWithFormat)).toEqual('2020-06-08-06-2020');
387-
expect(getFormattedStringFromDate(testDate, timeColumnWithFormat)).toEqual('2020-06-08-06-2020');
388-
});
389-
390-
test('resolved format matches column configuration', () => {
391-
expect(getFormattedStringFromDate(testDate, timeColumn)).toEqual('21:44');
392-
expect(getFormattedStringFromDate(testDate, dateOnlyColumn)).toEqual('2020-08-06');
393-
});
394-
});
395-
396362
describe('getJsonDateTimeFormatString', () => {
397363
test('without date', () => {
398364
expect(getJsonDateTimeFormatString(undefined)).toBeUndefined();

0 commit comments

Comments
 (0)