fix(vizBuilder): RN-1054: picks wrong date #5447
Merged
Conversation
This file contains 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
avoids typing issues where return type could be `null`
alexd-bes
reviewed
Mar 6, 2024
Importing from `tsutils` to front-end packages is problematic
alexd-bes
approved these changes
Mar 7, 2024
packages/admin-panel/src/VizBuilderApp/components/PreviewOptions.jsx
Outdated
Show resolved
Hide resolved
packages/admin-panel/src/VizBuilderApp/components/PreviewOptions.jsx
Outdated
Show resolved
Hide resolved
merge latest dev before testing
merge latest dev before testing
merge latest dev
# Conflicts: # packages/admin-panel/src/VizBuilderApp/components/PreviewOptions.jsx
jaskfla
changed the title
jaskfla
changed the title
merge latest dev before testing
jaskfla
changed the title
jaskfla
commented
Apr 3, 2024
Comment on lines
44
to
61
| /** | ||
| * When you select a date in the date picker (either with the interactive calendar or by inserting | ||
| * a valid date string), the picker actually selects a specific moment in time on that date with | ||
| * the normal, millisecond granularity of a Date object. The time of day it selects is the current | ||
| * local time on the system where VizBuilder is running. | ||
| * | ||
| * When inputting the date, the date picker interprets it in the user’s time zone. Under the hood, | ||
| * this is converted to UTC. But since we only need day-level granularity, this timezone | ||
| * conversion can make the date picker select a date different from what the user input (±1 day) | ||
| * when {@link converDateToIsoString} discards the timestamp. | ||
| * | ||
| * This helper function accounts for that discrepancy. | ||
| * | ||
| * @param date A valid date object | ||
| * @returns {Date} | ||
| */ | ||
| const shiftEpoch = date => | ||
| new Date(date.setMinutes(date.getMinutes() - date.getTimezoneOffset())); |
There was a problem hiding this comment.
@alexd-bes This is the hacky fix I mentioned in stand-up this morning. Easy enough to understand, but you can imagine why I’m not happy about it 😅
Andrew went to the trouble of testing at multiple times throughout the day and it does seem to work, though; so it’s currently marked as test passed
There was a problem hiding this comment.
I think this is probably fine - as you say, it's a very specific case and it's an old install of datepicker which may get rejigged anyway, so if it works it's all good
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue RN-1054: Fix Viz Buidler Date Picker behaviour
Changes
Fixes behaviour in Viz Builder where date picker would select a date and time (with a deterministic-but-unpredictable time), which resulted in unexpected query results
The nitty-gritty
fetchReportPreviewDataendpoint with full ISO timestamps (date and time) in Zulu time, usually converted from whatever the user’s browser’s local time was (this behaviour seems to vary between browsers/runtime environments).Dateobject in back-end (which introduces timezone conversion issues), and back into string forDataTableService’s consumption.