Fix date-only searching. #15337
Fix date-only searching. #15337
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
…er' of github.com:budibase/budibase into budi-8957-date-filter-equals-does-not-work
| let rootErr = err | ||
| while (rootErr.cause) { | ||
| rootErr = rootErr.cause | ||
| } | ||
| // @ts-ignore | ||
| error.stack = err.stack | ||
| error.stack = rootErr.stack |
There was a problem hiding this comment.
This was bothering me when tests failed. I only got shown the outermost error, which was never where the real problem was. This makes the error stack traces substantially more useful.
| for (const value of values) { | ||
| if (value != null) { | ||
| q = q.or.whereLike(key, `${value.toISOString().slice(0, 10)}%`) | ||
| } else { | ||
| q = q.or.whereNull(key) | ||
| } | ||
| } |
There was a problem hiding this comment.
It's a shame we can't easily use whereIn here. We could, I suppose, if we expanded out, say, 2020-01-01 to ["2020-01-01", "2020-01-01T00:00:00Z"] but that felt more fraught than a bunch of LIKE queries.
| return false | ||
| } | ||
| return d.toISOString() === trimmedValue | ||
| return ISO_DATE_REGEX.test(str.trim()) |
There was a problem hiding this comment.
This function is used in a single place, and it's where we parse incoming data to be searched on. I want both dates and date time to get parsed into Date objects, so I've modified this function to include anything that looks like 2020-01-01 or 2020-01-01T00:00:00Z.
It stood out to me that the regex we use does not consider any other timezones, not sure how big a deal that is in practice.
| @@ -2341,7 +2341,7 @@ if (descriptions.length) { | |||
| [FieldType.ARRAY]: ["options 2", "options 4"], | |||
| [FieldType.NUMBER]: generator.natural(), | |||
| [FieldType.BOOLEAN]: generator.bool(), | |||
| [FieldType.DATETIME]: generator.date().toISOString(), | |||
| [FieldType.DATETIME]: generator.date().toISOString().slice(0, 10), | |||
There was a problem hiding this comment.
For whatever reason in this test we use a datetime field but set dateOnly=true.
| describe("datetime - date only", () => { | ||
| describe.each([true, false])( | ||
| "saved with timestamp: %s", | ||
| saveWithTimestamp => { | ||
| describe.each([true, false])( | ||
| "search with timestamp: %s", | ||
| searchWithTimestamp => { |
There was a problem hiding this comment.
Yep I hate it too, but we're in a position where dates could have been saved into SQS with or without time components, and you can also search with or without a time component. For dateOnly fields, I think the best trade-off is to handle the cross product of all eventualities in the same way: ignore time.
| } else if (column.type === FieldType.DATETIME && column.dateOnly) { | ||
| for (const row of rows) { | ||
| if (typeof row[property] === "string") { | ||
| row[property] = new Date(row[property]) | ||
| } | ||
| if (row[property] instanceof Date) { | ||
| row[property] = row[property].toISOString().slice(0, 10) | ||
| } | ||
| } |
There was a problem hiding this comment.
We used to return date only fields with a timestamp sometimes, in a format that wasn't consistent (depended how each database returned the data, e.g. postgres would do 2020-01-01 +00:00:00). This unifies them all to return 2020-01-01.
| const leftDate = dayjs(docValue) | ||
| if (leftDate.isValid()) { | ||
| const rightDate = dayjs(testValue) | ||
| if (rightDate.isValid()) { | ||
| return leftDate.isSame(rightDate) | ||
| } | ||
| } |
There was a problem hiding this comment.
This is to help in-memory search match date-only fields correctly when either side is a date/datetime.
Description
Bit of a thorny one, this. Mostly affected SQS.
The problem boils down to how we store dates as strings in SQS. The problem is that we haven't been consistent in how we do it: it's possible to save a date as either
2020-01-01or2020-01-01T00:00:00Z. It's also possible to search for a date with either format. Because they're strings, despite being semantically identical (if you assume UTC both sides), they do not match.What I've done is make sure that regardless of whether you search or save the date in either format, you can always find them.
Addresses
Launchcontrol