Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core - DeepExtendArraySafe - fix bug with undefined(T1226946) + scheduler(T1228488) #28022

Merged
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions e2e/testcafe-devextreme/tests/dataGrid/editing/undefinedValues.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import DataGrid from 'devextreme-testcafe-models/dataGrid';
import { createWidget } from '../../../helpers/createWidget';
import url from '../../../helpers/getPageUrl';

fixture`Editing - undefined values`
.disablePageReloads
.page(url(__dirname, '../../container.html'));

test('Should properly set nested undefined values (T1226946)', async (t) => {
const dataGrid = new DataGrid('#container');
const firstCell = dataGrid.getDataCell(0, 0);
const secondCell = dataGrid.getDataCell(1, 0);

await t.expect(firstCell.element().textContent).eql('100');
await t.expect(secondCell.element().textContent).eql('undefined');

await dataGrid.apiCellValue(0, 0, { data: undefined });
await dataGrid.apiSaveEditData();

await t.expect(firstCell.element().textContent).eql('undefined');
await t.expect(secondCell.element().textContent).eql('undefined');
}).before(async () => createWidget('dxDataGrid', {
dataSource: [{
id: 0,
value: {
data: 100,
},
}, {
id: 1,
value: {
data: undefined,
},
}],
keyExpr: 'id',
columns: [{
dataField: 'value',
customizeText: (cellInfo) => String(cellInfo.value.data ?? 'undefined'),
}],
showBorders: true,
}));
8 changes: 4 additions & 4 deletions packages/devextreme/js/__internal/data/m_array_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import config from '@js/core/config';
import Guid from '@js/core/guid';
import { compileGetter } from '@js/core/utils/data';
import { extend } from '@js/core/utils/extend';
import { deepExtendArraySafe } from '@js/core/utils/object';
import { deepExtendArraySafe, newAssign } from '@js/core/utils/object';
import {
isDefined, isEmptyObject, isObject, isPlainObject,
} from '@js/core/utils/type';
Expand Down Expand Up @@ -94,7 +94,7 @@ function cloneInstanceWithChangedPaths(instance, changes, clonedInstances) {
}

const instanceWithoutPrototype = { ...instance };
deepExtendArraySafe(result, instanceWithoutPrototype, true, true);
deepExtendArraySafe(result, instanceWithoutPrototype, true, true, true);
// eslint-disable-next-line no-restricted-syntax, guard-for-in
for (const name in instanceWithoutPrototype) {
const value = instanceWithoutPrototype[name];
Expand All @@ -120,7 +120,7 @@ function createObjectWithChanges(target, changes) {
// @ts-expect-error
const result = cloneInstanceWithChangedPaths(target, changes);

return deepExtendArraySafe(result, changes, true, true);
return deepExtendArraySafe(result, changes, true, true, true);
}

function applyBatch({
Expand Down Expand Up @@ -195,7 +195,7 @@ function update(keyInfo, array, key, data, isBatch, immutable, logError) {
target = key;
}

deepExtendArraySafe(target, data, extendComplexObject);
deepExtendArraySafe(target, data, extendComplexObject, false, true, newAssign);
if (!isBatch) {
if (config().useLegacyStoreResult) {
return trivialPromise(key, data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ export const createColumn = function (that: ColumnsController, columnOptions, us
if (!columnOptions.type) {
result = { headerId: `dx-col-${globalColumnId++}` };
}
result = deepExtendArraySafe(result, DEFAULT_COLUMN_OPTIONS);
deepExtendArraySafe(result, commonColumnOptions);
deepExtendArraySafe(result, calculatedColumnOptions);
deepExtendArraySafe(result, columnOptions);
deepExtendArraySafe(result, { selector: null });
result = deepExtendArraySafe(result, DEFAULT_COLUMN_OPTIONS, false, true);
deepExtendArraySafe(result, commonColumnOptions, false, true);
deepExtendArraySafe(result, calculatedColumnOptions, false, true);
deepExtendArraySafe(result, columnOptions, false, true);
deepExtendArraySafe(result, { selector: null }, false, true);
}
if (columnOptions.filterOperations === columnOptions.defaultFilterOperations) {
setFilterOperationsAsDefaultValues(result);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { extend } from '@js/core/utils/extend';
import { deepExtendArraySafe } from '@js/core/utils/object';
import { deepExtendArraySafe, newAssign } from '@js/core/utils/object';
import errors from '@js/ui/widget/ui.errors';

import { ExpressionUtils } from './m_expression_utils';
Expand Down Expand Up @@ -147,7 +147,7 @@ class AppointmentAdapter {

clone(options: any = undefined) {
const result = new AppointmentAdapter(
deepExtendArraySafe({}, this.rawAppointment),
deepExtendArraySafe({}, this.rawAppointment, false, false, false, newAssign),
this.dataAccessors,
this.timeZoneCalculator,
options,
Expand Down
51 changes: 45 additions & 6 deletions packages/devextreme/js/core/utils/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,49 @@ const orderEach = function(map, func) {
}
};

const assignValueToProperty = function(target, property, value, assignByReference) {
const getDeepCopyTarget = (item) => {
if(isObject(item)) {
return Array.isArray(item) ? [] : {};
}
return item;
};

const legacyAssign = function(target, property, value, extendComplexObject, assignByReference, shouldCopyUndefined) {
if(!assignByReference && variableWrapper.isWrapped(target[property])) {
variableWrapper.assign(target[property], value);
} else {
target[property] = value;
}
};

const newAssign = function(target, property, value, extendComplexObject, assignByReference, shouldCopyUndefined) {
const goDeeper = extendComplexObject ? isObject(target) : isPlainObject(target);
if(!assignByReference && variableWrapper.isWrapped(target[property])) {
variableWrapper.assign(target[property], value);
} else if(!assignByReference && Array.isArray(value)) {
target[property] = value.map(item => deepExtendArraySafe(
getDeepCopyTarget(item),
item,
extendComplexObject,
assignByReference,
shouldCopyUndefined
));
} else if(!assignByReference && goDeeper) {
target[property] = deepExtendArraySafe(
getDeepCopyTarget(value),
value,
extendComplexObject,
assignByReference,
shouldCopyUndefined,
newAssign
);
} else {
target[property] = value;
}
};

// B239679, http://bugs.jquery.com/ticket/9477
const deepExtendArraySafe = function(target, changes, extendComplexObject, assignByReference) {
const deepExtendArraySafe = function(target, changes, extendComplexObject, assignByReference, shouldCopyUndefined, assignFunc = legacyAssign) {
let prevValue;
let newValue;

Expand All @@ -62,11 +95,15 @@ const deepExtendArraySafe = function(target, changes, extendComplexObject, assig

if(isPlainObject(newValue)) {
const goDeeper = extendComplexObject ? isObject(prevValue) : isPlainObject(prevValue);
newValue = deepExtendArraySafe(goDeeper ? prevValue : {}, newValue, extendComplexObject, assignByReference);
newValue = deepExtendArraySafe(goDeeper ? prevValue : {}, newValue, extendComplexObject, assignByReference, shouldCopyUndefined);
}

if(newValue !== undefined && prevValue !== newValue) {
assignValueToProperty(target, name, newValue, assignByReference);
const isDeepCopyArray = Array.isArray(newValue) && !assignByReference;
const hasDifferentNewValue = (shouldCopyUndefined || newValue !== undefined) && prevValue !== newValue ||
shouldCopyUndefined && prevValue === undefined;

if(isDeepCopyArray || hasDifferentNewValue) {
assignFunc(target, name, newValue, extendComplexObject, assignByReference, shouldCopyUndefined);
}
}

Expand All @@ -76,5 +113,7 @@ const deepExtendArraySafe = function(target, changes, extendComplexObject, assig
export {
clone,
orderEach,
deepExtendArraySafe
deepExtendArraySafe,
legacyAssign,
newAssign
};
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,44 @@ QUnit.test('deepExtendArraySafe utility does not throw an error with \'null\' de
assert.equal(result.deepProp.toChange, 'changed value');
});

QUnit.test('deepExtendArraySafe utility does not pollute object prototype', function(assert) {
objectUtils.deepExtendArraySafe({ }, JSON.parse('{ "__proto__": { "pollution": true }}'), true);
assert.ok(!('pollution' in { }), 'object prototype is not polluted');
QUnit.test('deepExtendArraySafe sets undefined', function(assert) {
const objWithValue = { time: { duration: 50 } };
const objNoValue = {};
objectUtils.deepExtendArraySafe(objWithValue, { time: { duration: undefined } }, true);
objectUtils.deepExtendArraySafe(objNoValue, { time: { duration: undefined } }, true);

assert.equal(objWithValue.time.duration, 50);
assert.notOk(Object.prototype.hasOwnProperty.call(objNoValue.time, 'duration'));

objectUtils.deepExtendArraySafe(objWithValue, { time: { duration: undefined } }, true, false, true);
objectUtils.deepExtendArraySafe(objNoValue, { time: { duration: undefined } }, true, false, true);

assert.equal(objWithValue.time.duration, undefined);
assert.ok(Object.prototype.hasOwnProperty.call(objWithValue.time, 'duration'));
assert.ok(Object.prototype.hasOwnProperty.call(objNoValue.time, 'duration'));

});

QUnit.test('deepExtendArraySafe doesn\'t set undefined if shouldCopyUndefined == false', function(assert) {
const objWithValue = { time: { duration: 50 } };
const objNoValue = {};
objectUtils.deepExtendArraySafe(objWithValue, { time: { duration: undefined } }, true, false, false);
objectUtils.deepExtendArraySafe(objNoValue, { time: { duration: undefined } }, true, false, false);

assert.equal(objWithValue.time.duration, 50);
assert.notOk(Object.prototype.hasOwnProperty.call(objNoValue.time, 'duration'));

});

QUnit.test('deepExtendArraySafe copies array into object property deeply', function(assert) {
const objWithValue = { time: undefined };
const complexTime = { complexTime: 2 };
const timeArray = [1, complexTime, 3];
objectUtils.deepExtendArraySafe(objWithValue, { time: timeArray }, true, false, false, objectUtils.newAssign);

assert.deepEqual(objWithValue.time, timeArray);
assert.notStrictEqual(objWithValue.time, timeArray);
assert.notStrictEqual(objWithValue.time[1], complexTime);
timeArray[0] = 5;
assert.deepEqual(objWithValue.time, [1, complexTime, 3]);
});
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,28 @@ QUnit.test('update with key', function(assert) {

});

QUnit.test('update with explicit undefined', function(assert) {
const done = assert.async();

const store = new ArrayStore({
key: 'id',
data: [{ id: 0, nested: { a: 1 } }],
});

store.update(0, { nested: { a: undefined } }).done(function(data, key) {
assert.equal(key, 0);

const expectedData = {
nested: { a: undefined },
id: key
};

assert.deepEqual(data, expectedData);
assert.ok(Object.prototype.hasOwnProperty.call(data.nested, 'a'));
done();
});
});

QUnit.test('insert duplicate key (simple)', function(assert) {
const done = assert.async();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ if(QUnit.urlParams['nocsp']) {

QUnit.test('deepExtendArraySafe works correctly with array contain observables', function(assert) {
const testObj = { id: 4, name: ko.observable('John') };
const resultObj = objectUtils.deepExtendArraySafe(testObj, { name: 'Sue' });
const resultObj = objectUtils.deepExtendArraySafe(testObj, { name: 'Sue' }, false, false, false, objectUtils.newAssign);

assert.equal(variableWrapper.isWrapped(resultObj.name), true, '\'name\' field is still observable');
assert.equal(resultObj.name(), 'Sue', 'New value accepted');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,52 @@ supportedScrollingModes.forEach(scrollingMode => {
assert.equal(updatedRecurringItem.recurrenceException, dateSerialization.serializeDate(exceptionDate, 'yyyyMMddTHHmmssZ'), 'Exception for recurrence appointment is correct');
});

test('Recurrent Task editing, single mode, should not reference copy recurrent data (T1228488)', function(assert) {
const updatedItem = {
text: 'Task 2',
customData: { texts: ['123', '456'] },
startDate: new Date(2015, 1, 11, 3),
endDate: new Date(2015, 1, 11, 4),
};

const scheduler = this.createInstance({
currentDate: new Date(2015, 1, 9),
dataSource: [{
text: 'Task 1',
startDate: new Date(2015, 1, 9, 1, 0),
endDate: new Date(2015, 1, 9, 2, 0),
customData: { texts: ['123'] },
recurrenceRule: 'FREQ=DAILY'
}],
currentView: 'week',
onAppointmentAdding: (e) => {
e.appointmentData.customData.texts.push('456');
},
firstDayOfWeek: 1
});

scheduler.appointments.click(2);
this.clock.tick(300);
scheduler.tooltip.clickOnItem();
$('.dx-dialog-buttons .dx-button').eq(1).trigger('dxclick');

const $title = $('.dx-textbox').eq(0);
const title = $title.dxTextBox('instance');
const $startDate = $('.dx-datebox').eq(0);
const startDate = $startDate.dxDateBox('instance');

title.option('value', 'Task 2');
startDate.option('value', new Date(2015, 1, 11, 3, 0));
$('.dx-button.dx-popup-done').eq(0).trigger('dxclick');
this.clock.tick(300);

const updatedSingleItem = scheduler.instance.option('dataSource')[1];
const updatedRecurringItem = scheduler.instance.option('dataSource')[0];

assert.deepEqual(updatedSingleItem, updatedItem, 'New data is correct');
assert.deepEqual(updatedRecurringItem.customData.texts, ['123'], 'Recurrence data is correct');
});

test('Recurrent Task edition canceling, single mode', function(assert) {
const data = new DataSource({
store: [
Expand Down
2 changes: 1 addition & 1 deletion packages/testcafe-models/dataGrid/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ export default class DataGrid extends Widget {
)();
}

apiCellValue(rowIndex: number, columnIndex: number, value: string): Promise<void> {
apiCellValue<T>(rowIndex: number, columnIndex: number, value: T): Promise<void> {
const { getInstance } = this;
return ClientFunction(
() => (getInstance() as any).cellValue(rowIndex, columnIndex, value),
Expand Down
Loading