-
Notifications
You must be signed in to change notification settings - Fork 105
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
fix(Trial Balance): limit lookup to FY #2569
fix(Trial Balance): limit lookup to FY #2569
Conversation
f5d72d0
to
66edb9f
Compare
66edb9f
to
227097a
Compare
This commit limits the Trial Balance lookup to only oldest fiscal year. It also fixes a grouping bug that summed the before total an extra time. This lead to N * beginning_balance being reported for N transactions. Closes Third-Culture-Software#2568.
227097a
to
16ce3c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a test locally with the test dataset the trial balance shows correct values.
it looks good to me 👍
GROUP BY u.account_id; | ||
|
||
SELECT account_id, account.number AS number, account.label AS label, | ||
balance_before, debit_equiv, credit_equiv, | ||
balance_before + debit_equiv - credit_equiv AS balance_final | ||
FROM ( | ||
SELECT posting_journal.account_id, SUM(totals.balance_before) AS balance_before, SUM(debit_equiv) AS debit_equiv, | ||
SELECT posting_journal.account_id, MAX(totals.balance_before) AS balance_before, SUM(debit_equiv) AS debit_equiv, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you fix this MAX
if it is required, in my test i cannot see the error with MAX
bors r+ |
Build succeeded |
This commit limits the Trial Balance lookup to only oldest fiscal year.
Closes #2568.