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

Bug fix recorddate inconsistency #831

Merged
merged 2 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
**/types.ts
**/dist_electron
**/dummy/*.json
**/.github/ISSUE_TEMPLATE/*.yml
**/.github/ISSUE_TEMPLATE/*.yml
**/patches/v0_21_0/*
6 changes: 6 additions & 0 deletions backend/patches/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import fixRoundOffAccount from './fixRoundOffAccount';
import testPatch from './testPatch';
import updateSchemas from './updateSchemas';
import setPaymentReferenceType from './setPaymentReferenceType';
import fixLedgerDateTime from './v0_21_0/fixLedgerDateTime';

export default [
{ name: 'testPatch', version: '0.5.0-beta.0', patch: testPatch },
Expand Down Expand Up @@ -34,4 +35,9 @@ export default [
version: '0.20.1',
patch: setPaymentReferenceType,
},
{
name: 'fixLedgerDateTime',
version: '0.21.1',
patch: fixLedgerDateTime,
},
] as Patch[];
34 changes: 34 additions & 0 deletions backend/patches/v0_21_0/fixLedgerDateTime.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { DatabaseManager } from '../../database/manager';

/* eslint-disable */
async function execute(dm: DatabaseManager) {
await dm.db!.knex!('AccountingLedgerEntry')
.select('name', 'date')
.then((trx: Array<{name: string; date: Date;}> ) => {
trx.forEach(async entry => {
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);
Comment on lines +9 to +22
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the offsetMinutes and offsetHours to set the values aren't necessary but are there to mirror with the code fix in LedgerPosting.ts

entryDate.setSeconds(0);
entryDate.setMilliseconds(0);

await dm.db!.knex!('AccountingLedgerEntry')
.where({ name: entry['name'] })
.update({ date: entryDate.toISOString() });
});
});
}

export default { execute, beforeMigrate: true };
/* eslint-enable */
21 changes: 20 additions & 1 deletion models/Transactional/LedgerPosting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,31 @@ export class LedgerPosting {
return map[account];
}

// 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that equivalent?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

dateTimeValue = new Date(entryDateTime);
} else {
dateTimeValue = entryDateTime;
}
const dtFixedValue = dateTimeValue;
const dtMinutes = dtFixedValue.getTimezoneOffset() % 60;
const dtHours = (dtFixedValue.getTimezoneOffset() - dtMinutes) / 60;
// Forcing the time to always be set to 00:00.000 for locale time
dtFixedValue.setHours(0 - dtHours);
dtFixedValue.setMinutes(0 - dtMinutes);
dtFixedValue.setSeconds(0);
dtFixedValue.setMilliseconds(0);

// end ugly timezone fix code

const ledgerEntry = this.fyo.doc.getNewDoc(
ModelNameEnum.AccountingLedgerEntry,
{
account: account,
party: (this.refDoc.party as string) ?? '',
date: this.refDoc.date as string | Date,
date: dtFixedValue,
referenceType: this.refDoc.schemaName,
referenceName: this.refDoc.name!,
reverted: this.reverted,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "frappe-books",
"version": "0.21.0",
"version": "0.21.1",
"description": "Simple book-keeping app for everyone",
"author": {
"name": "Frappe Technologies Pvt. Ltd.",
Expand Down
Loading