Skip to content

Commit

Permalink
Modify update to compare dates/timestamps better (#1006)
Browse files Browse the repository at this point in the history
* check type when comparing values so we properly convert the old/new values to the same representative value for date, timestamp, and timestamptz
* update code to not assume format and use isSame from moment
* add test cases to 10.update spec to verify date, timestamp, timestamptz comparisons are occurring properly
  • Loading branch information
jrchudy authored Feb 29, 2024
1 parent a42ec41 commit 7949f4f
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 7 deletions.
31 changes: 24 additions & 7 deletions js/reference.js
Original file line number Diff line number Diff line change
Expand Up @@ -1669,12 +1669,29 @@
var column, keyColumns, referenceColumn;

// add column name into list of column projections if not in column projections set and data has changed
var addProjection = function(colName) {
var addProjection = function(colName, colType) {
// don't add a column name in if it's already there
// this can be the case for multi-edit
// and if the data is unchanged, no need to add the column name to the projections list
// NOTE: This doesn't properly verify if date/timestamp/timestamptz values were changed
if ( (columnProjections.indexOf(colName) === -1) && (oldData[colName] != newData[colName]) ) {
if (columnProjections.indexOf(colName) !== -1) return;

var oldVal = oldData[colName]
var newVal = newData[colName]

var typename = colType.rootName;
var compareWithMoment = typename === 'date' || typename === 'timestamp' || typename === 'timestamptz';
// test with moment if datetime column type and one of the 2 values are defined
// NOTE: moment will test 2 null values as different even though they are both null
if (compareWithMoment && (oldVal || newVal)) {
var moment = module._moment;

var oldMoment = moment(oldData[colName])
var newMoment = moment(newData[colName])

if (!oldMoment.isSame(newMoment)) {
columnProjections.push(colName);
}
} else if (oldData[colName] != newData[colName]) {
columnProjections.push(colName);
}
};
Expand All @@ -1694,11 +1711,11 @@
if (assetColumns[colIndex]) {
// If asset url is null then set the metadata also null
if (isNull) newData[assetColumns[colIndex].name] = null;
addProjection(assetColumns[colIndex].name);
addProjection(assetColumns[colIndex].name, assetColumns[colIndex].type);
}
}

addProjection(column.name);
addProjection(column.name, column.type);

} else {
keyColumns = [];
Expand All @@ -1712,11 +1729,11 @@
for (n = 0; n < keyColumns.length; n++) {
keyColumnName = keyColumns[n].name;

addProjection(keyColumnName);
addProjection(keyColumnName, keyColumns[n].type);
}
}
} else {
addProjection(column.name);
addProjection(column.name, column.type);
}
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"key": 1, "date_col": null, "timestamp_col": "2024-02-27T14:14:12", "timestamptz_col": "2024-02-28T20:39:25+00:00"}]
45 changes: 45 additions & 0 deletions test/specs/reference/conf/update_schema/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,50 @@
],
"annotations": {}
},
"update_table_with_date_and_timestamps": {
"comment": "Table to represent a reference object with date and timestamp columns.",
"kind": "table",
"keys": [
{
"comment": null,
"annotations": {},
"unique_columns": [
"key"
]
}
],
"foreign_keys": [],
"table_name": "update_table_with_date_and_timestamps",
"schema_name": "update_schema",
"column_definitions": [
{
"name": "key",
"nullok": false,
"type": {
"typename": "text"
}
}, {
"name": "date_col",
"nullok": true,
"type": {
"typename": "date"
}
}, {
"name": "timestamp_col",
"nullok": true,
"type": {
"typename": "timestamp"
}
}, {
"name": "timestamptz_col",
"nullok": true,
"type": {
"typename": "timestamptz"
}
}
],
"annotations": {}
},
"table_w_composite_key": {
"comment": "Table with composite key",
"kind": "table",
Expand Down Expand Up @@ -383,6 +427,7 @@
"alias_table",
"update_table",
"update_table_with_foreign_key",
"update_table_with_date_and_timestamps",
"table_w_composite_key",
"table_w_composite_key_as_shortest_key",
"table_w_independent_key"
Expand Down
139 changes: 139 additions & 0 deletions test/specs/reference/tests/10.update.js
Original file line number Diff line number Diff line change
Expand Up @@ -1820,6 +1820,145 @@ exports.execute = function (options) {
});
});

describe("for updating entities with date and timestamp columns ", function () {
var tableName = "update_table_with_date_and_timestamps",
sortBy = "key", // column used to sort the data
uriSingle = options.url + "/catalog/" + catalogId + "/entity/" + schemaName + ':' + tableName + "/key=1";

var reference;

beforeAll(function (done) {
options.ermRest.resolve(uriSingle, {cid: "test"}).then(function (response) {
reference = response;

done();
}).catch(function (error) {
console.dir(error);
done.fail();
});
});

it("a single entity should should throw an error when no data was updated.", function (done) {
// makes sure case added to validate 2 null date values are properly compared still and recognized as unchanged
reference = reference.contextualize.entryEdit;

reference.read(1).then(function (response) {
return reference.update(response.tuples);
}).then(function (response) {
throw new Error("Did not return any errors");
}).catch(function (err) {
expect(err instanceof options.ermRest.NoDataChangedError).toBe(true);
expect(err.message).toEqual("No data was changed in the update request. Please check the form content and resubmit the data.", "Wrong error message was returned");

done();
});
});

it("should throw an error when timestamp value is sent to be updated but is unchanged.", function (done) {
var updateData = {
"timestamp_col": "2024-02-27T14:14:12"
};

reference = reference.contextualize.entryEdit;

reference.read(1).then(function (response) {
tuples = response.tuples;
tuple = tuples[0];
var data = tuple.data;

for (var key in updateData) {
data[key] = updateData[key];
}

return reference.update(response.tuples);
}).then(function (response) {
throw new Error("Did not return any errors");
}).catch(function (err) {
expect(err instanceof options.ermRest.NoDataChangedError).toBe(true);
expect(err.message).toEqual("No data was changed in the update request. Please check the form content and resubmit the data.", "Wrong error message was returned");

done();
});
});

it("should throw an error when timestamptz value is sent to be updated with a different timezone but value is unchanged.", function (done) {
var updateData = {
"timestamptz_col": "2024-02-28T12:39:25-08:00"
};

reference = reference.contextualize.entryEdit;

reference.read(1).then(function (response) {
tuples = response.tuples;
tuple = tuples[0];
var data = tuple.data;

for (var key in updateData) {
data[key] = updateData[key];
}

return reference.update(response.tuples);
}).then(function (response) {
throw new Error("Did not return any errors");
}).catch(function (err) {
expect(err instanceof options.ermRest.NoDataChangedError).toBe(true);
expect(err.message).toEqual("No data was changed in the update request. Please check the form content and resubmit the data.", "Wrong error message was returned");

done();
});
});

it("should update the date, timestamp, and timestamptz columns when all are changed.", function (done) {
var tuples;

var updateData = {
"date_col": "2024-02-28",
"timestamp_col": "2024-02-28T12:56:12",
"timestamptz_col": "2024-02-28T12:52:21+00:00"
};

reference = reference.contextualize.entryEdit;

reference.read(1).then(function (response) {
tuples = response.tuples;
tuple = tuples[0];
var data = tuple.data;

for (var key in updateData) {
data[key] = updateData[key];
}

return reference.update(response.tuples);
}).then(function (response) {
response = response.successful;
expect(response._data.length).toBe(1, "response data is not the same size as given.");
expect(response.reference._context).toEqual("compact", "page reference is not in the correct context.");

utils.checkPageValues(response._data, tuples, sortBy);

return options.ermRest.resolve(uriSingle, {cid: "test"});
}).then(function (response) {
return response.read(1);
}).then(function (response) {
var pageData = response._data[0];

expect(pageData['date_col']).toBe(updateData['date_col'], "updated date is not correct.");
expect(pageData['date_col']).not.toBe(tuple._oldData['date_col'], "date has not been updated.");

expect(pageData['timestamp_col']).toBe(updateData['timestamp_col'], "updated timestamp is not correct.");
expect(pageData['timestamp_col']).not.toBe(tuple._oldData['timestamp_col'], "timestamp has not been updated.");

expect(pageData['timestamptz_col']).toBe(updateData['timestamptz_col'], "updated timestamptz is not correct.");
expect(pageData['timestamptz_col']).not.toBe(tuple._oldData['timestamptz_col'], "timestamptz has not been updated.");

done();
}).catch(function (error) {
console.dir(error);
done.fail();
});
});
})

describe("for updating multiple entities should return a page object, when", function () {
var tableName = "table_w_composite_key_as_shortest_key",
sortBy = "id_1",
Expand Down

0 comments on commit 7949f4f

Please sign in to comment.