Bug fix recorddate inconsistency #831
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
This was referenced Jan 31, 2024
mildred
approved these changes
Feb 1, 2024
| // Timezone inconsistency fix (very ugly code for now) | ||
| const entryDateTime = this.refDoc.date as string | Date; | ||
| let dateTimeValue: Date; | ||
| if (typeof entryDateTime === 'string' || entryDateTime instanceof String) { |
There was a problem hiding this comment.
It should be, yet as we saw with this DateTime fun, there are some weird errors that pop up. By forcing a check I want to make sure that it is as expected as I really don't want to revisit this error later.
|
The plan is to break it out in the future, but as you mentioned we should check the scope of the entirety of the issue. I included it in the function to ensure its scope is only applied to that section and children that use it. For the scope of the issue, if you look at the db patch, it shows all of the models affected |
Isaac-GC
force-pushed
the
bug_fix_recorddate_inconsistency
branch
from
f048880
to
ca2a316
Compare
Isaac-GC
commented
Feb 1, 2024
Comment on lines
+9
to
+22
| const entryDate = new Date(entry['date']); | ||
| const timeZoneOffset = entryDate.getTimezoneOffset(); | ||
| const offsetMinutes = timeZoneOffset % 60; | ||
| const offsetHours = (timeZoneOffset - offsetMinutes) / 60; | ||
|
|
||
| let daysToAdd = 0; // If behind or at GMT/Zulu time, don't need to add a day | ||
| if (timeZoneOffset < 0) { | ||
| // If ahead of GMT/Zulu time, need to advance a day forward first | ||
| daysToAdd = 1; | ||
| } | ||
|
|
||
| entryDate.setDate(entryDate.getDate() + daysToAdd); | ||
| entryDate.setHours(0 - offsetHours); | ||
| entryDate.setMinutes(0 - offsetMinutes); |
There was a problem hiding this comment.
Using the offsetMinutes and offsetHours to set the values aren't necessary but are there to mirror with the code fix in LedgerPosting.ts
Isaac-GC
force-pushed
the
bug_fix_recorddate_inconsistency
branch
from
0f36a16
to
64ad73b
Compare
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.
After doing some testing in various timezones it looks like this (very ugly) patch successfully fixes the various timestamp inconsistency issues, which resulted in incorrect ledgers, profit/loss, etc
What was happening is that depending on one's timezone, the Date that was saved in the JournalEntry/Ledger would convert the Datetime to a timestamp of
00:00.000for the local timezone and then calculates the GMT timestamp based on the local timezone. This creates an instance where what is saved may not properly reflect what was entered and expected in the related invoices (or other items).Here are two test cases showing the issue of what was happening:
GMT +6.5)The time is converted to GMT which is 6.5 hours behind Myanmar at that point of time, resulting in saving the datetime as previous date (in this test case,
2022-12-31T17:30.000Z)GMT -8)In this case, when the timestamp for the local timezone is
00:00.000, GMT time will be 8 hours ahead (2023-01-01T08:00.000Z)