Refactor General Ledger #2462

jniles · 2018-01-24T09:40:37Z

This PR refactors the General Ledger to make it more useful from an accounting perspective.

Code Refactor

The server-side SQL query has been rewritten to be more performant and easier to read. In particular, the WHERE query filters on fiscal year instead of first looking up the periods in the fiscal year.
Removed the Account Slip code - it was essentially a duplication of effort from the Account Statement code.
Renamed all variables references general-ledger-accounts to general-ledger. We had folded the previous "General Ledger" into the Journal, so only this one remains.

Features

A <select> element to the grid footer to use to switch the fiscal year and removes the Fiscal Year modal. This was requested by @WayneNiles and improves the UX by allowing users to switch fiscal years on the fly. By removing the modal, we also alleviate the maintenance burden on developers.
A "check row" at the bottom of the General Ledger has been implemented, showing the sum of each column with period balances. These columns should always sum to zero. If not, it means that there is an imbalanced transaction somewhere.
Title accounts are now show and hold the sum of the balances of their children. This allows a really quick comparison of bulk amounts, such as Income versus Expense. This has been implemented using a Tree class, available to be used in other reports.
Added title account highlighting and indentation to both the client and server-side generated reports for the General Ledger. The indentation is set to 15px by default.
Add a link to the Account Statement module. This allows a user to directly inspect the values from the General Ledger in the Account Statement.

Screenshots

Fig 1: New General Ledger Client

Fig 2: New General Ledger Report w/ Indentation

Closes #1706. Closes #2459. Closes #2463. Closes #2373.

mbayopanda

LGTM

I just noticed that there is an error when click on action -> Extrait de compte to get the extrait de compte report.

mbayopanda · 2018-01-26T09:17:03Z

client/src/modules/general-ledger/general-ledger-accounts.ctrl.js

-    account.isTitleAccount = account.type_id === bhConstants.accounts.TITLE;
+  function removeZeroes(account) {
+    // remove zero values from the matrix to render as empty cells.
+    fields.forEach(function (field) {


mbayopanda · 2018-01-26T09:41:30Z

server/lib/Tree.js

@@ -0,0 +1,138 @@
+/**
+ * @class Tree


👍 awesome... Good job

mbayopanda · 2018-01-26T09:42:21Z

test/server-unit/Tree.spec.js

+const Tree = require('../../server/lib/Tree');
+const { expect } = require('chai');
+
+function TreeUnitTests() {


💃 Excellent

mbayopanda · 2018-01-26T09:47:48Z

@jniles this is the error thrown when we click on action -> Extrait de compte :

jniles · 2018-01-26T09:52:44Z

@mbayopanda, thanks for the bug report! Clearly undefined is not a currency... :(

I should probably address all the problems with the GL in this PR to make sure that we get it right. Here's my current plan of action:

Fix Improvements to the General Ledger #2463. This should take care of the select bug you mentioned.
Fix the bug you just found. Whoops!
Implement Add links from the General Ledger to the Account Statement #2459. This should give us a better way to validate the GL.

Thanks for the review! It's good to have you back 👍

jniles · 2018-01-26T19:08:19Z

@mbayopanda, I've completed all the work I said I would do. When you get a chance, could I have another review?

This commit optimizes the General Ledger matrix query by only doing a single SELECT with a GROUP BY. It also removes the need for pre-querying period ids and uses the raw fiscal year id instead. This is the first step to further optimizations of calculating the balances of title accounts by summing down the account tree.

This commit adds a footer to the General Ledger with the Fiscal Year in it. This allows rapid changes to the view without need to have multiple clicks.

This commit implements a Tree class on the server for constructing trees out of accounts, units, or other tree structures. The only requirement is that the objects have references pointing them to their parents.

This commit implements the sumOnProperty() function to sum tree nodes on a given property. Parent nodes are given the values of the sums of their children.

This commit implements basic summing on the general ledger's title accounts. The summing is performed on the server in JavaScript, but could be done on the client just as easily.

This commit makes sure empty accounts are pruned from the General Ledger report.

This commit removes the old Account Slip report link and replaces it with the account statement report link from the General Ledger. Closes Third-Culture-Software#1706.

This commit removes the account slip which is no longer used. It also adds an indent to the account list so that nested items are clearly below their parent. Finally, title accounts are rendered in bold.

This commit renames the general ledger files to remove the "accounts" portion of the name from all identifiers. This is the real General Ledger, since the previous version was folded into the Journal.

This commit indents the accounts in the tree to emphasize their relationships.

This commit implements a check footer at the bottom of the grid that sums the values in the columns. As a rule of accounting, summing the chart of accounts should give a zero. Thus, any columns which do not sum to 0 denote a problem in the dataset. Closes Third-Culture-Software#2463.

This commit links the General Ledger to the Account Statement via the dropdown menu in the General Ledger. Closes Third-Culture-Software#2459.

mbayopanda

It works very well. :+1

mbayopanda · 2018-01-29T13:34:32Z

bors r+

@WayneNiles

2462: Refactor General Ledger r=mbayopanda a=jniles This PR refactors the General Ledger to make it more useful from an accounting perspective. **Code Refactor** 1. The server-side SQL query has been rewritten to be more performant and easier to read. In particular, the WHERE query filters on fiscal year instead of first looking up the periods in the fiscal year. 2. Removed the Account Slip code - it was essentially a duplication of effort from the Account Statement code. 3. Renamed all variables references `general-ledger-accounts` to `general-ledger`. We had folded the previous "General Ledger" into the Journal, so only this one remains. **Features** 1. A `<select>` element to the grid footer to use to switch the fiscal year and removes the Fiscal Year modal. This was requested by @WayneNiles and improves the UX by allowing users to switch fiscal years on the fly. By removing the modal, we also alleviate the maintenance burden on developers. 2. A "check row" at the bottom of the General Ledger has been implemented, showing the sum of each column with period balances. _These columns should always sum to zero_. If not, it means that there is an imbalanced transaction somewhere. 3. Title accounts are now show and hold the sum of the balances of their children. This allows a really quick comparison of bulk amounts, such as Income versus Expense. This has been implemented using a `Tree` class, available to be used in other reports. 4. Added title account highlighting and indentation to both the client and server-side generated reports for the General Ledger. The indentation is set to `15px` by default. 5. Add a link to the Account Statement module. This allows a user to directly inspect the values from the General Ledger in the Account Statement. **Screenshots** ![newgl](https://user-images.githubusercontent.com/896472/35456000-5a04432a-02d4-11e8-812d-760446e3a6ae.PNG) _Fig 1: New General Ledger Client_ ![newglreport](https://user-images.githubusercontent.com/896472/35456057-8e33d76e-02d4-11e8-8418-db15de5c8195.PNG) _Fig 2: New General Ledger Report w/ Indentation_ Closes #1706. Closes #2459. Closes #2463. Closes #2373.

bors · 2018-01-29T13:52:27Z

Build succeeded

continuous-integration/travis-ci/push

jniles added the Refactor label Jan 24, 2018

jniles mentioned this pull request Jan 24, 2018

Improvements to the General Ledger #2463

Closed

jniles force-pushed the feat-general-ledger-refactor branch from 02cb7c7 to 4660c1b Compare January 24, 2018 09:51

jniles requested a review from mbayopanda January 24, 2018 12:41

mbayopanda reviewed Jan 26, 2018

View reviewed changes

jniles mentioned this pull request Jan 26, 2018

refactor: Restructure account slip structure #1706

Closed

4 tasks

jniles added 12 commits January 26, 2018 20:13

refactor(general ledger): <select> FY in footer

1ef175c

This commit adds a footer to the General Ledger with the Fiscal Year in it. This allows rapid changes to the view without need to have multiple clicks.

feat(Tree): implement tree structure w/ tests

85892b6

This commit implements a Tree class on the server for constructing trees out of accounts, units, or other tree structures. The only requirement is that the objects have references pointing them to their parents.

feat(Tree): implement sumOnProperty()

663165a

This commit implements the sumOnProperty() function to sum tree nodes on a given property. Parent nodes are given the values of the sums of their children.

feat(General Ledger): implement title summing

22f4cc1

This commit implements basic summing on the general ledger's title accounts. The summing is performed on the server in JavaScript, but could be done on the client just as easily.

feat(General Ledger): prune empty accounts

ceec405

This commit makes sure empty accounts are pruned from the General Ledger report.

feat(General Ledger): use Account Statement Report

a2096d1

This commit removes the old Account Slip report link and replaces it with the account statement report link from the General Ledger. Closes Third-Culture-Software#1706.

chore(General Ledger): remove accountSlip

c980b86

This commit removes the account slip which is no longer used. It also adds an indent to the account list so that nested items are clearly below their parent. Finally, title accounts are rendered in bold.

chore(General Ledger): remove accounts from name

9cc1161

This commit renames the general ledger files to remove the "accounts" portion of the name from all identifiers. This is the real General Ledger, since the previous version was folded into the Journal.

chore(General Ledger): indent accounts

1d309aa

This commit indents the accounts in the tree to emphasize their relationships.

feat(General Ledger): link Account Statement

ae25a1d

This commit links the General Ledger to the Account Statement via the dropdown menu in the General Ledger. Closes Third-Culture-Software#2459.

jniles force-pushed the feat-general-ledger-refactor branch from 5c91185 to ae25a1d Compare January 26, 2018 19:13

mbayopanda approved these changes Jan 29, 2018

View reviewed changes

bors bot merged commit ae25a1d into Third-Culture-Software:master Jan 29, 2018

jniles deleted the feat-general-ledger-refactor branch January 29, 2018 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor General Ledger #2462

Refactor General Ledger #2462

jniles commented Jan 24, 2018 •

edited

Loading

mbayopanda left a comment

mbayopanda Jan 26, 2018

mbayopanda Jan 26, 2018

mbayopanda Jan 26, 2018

mbayopanda commented Jan 26, 2018

jniles commented Jan 26, 2018

jniles commented Jan 26, 2018

mbayopanda left a comment

mbayopanda commented Jan 29, 2018

bors bot commented Jan 29, 2018

Refactor General Ledger #2462

Refactor General Ledger #2462

Conversation

jniles commented Jan 24, 2018 • edited Loading

mbayopanda left a comment

Choose a reason for hiding this comment

mbayopanda Jan 26, 2018

Choose a reason for hiding this comment

mbayopanda Jan 26, 2018

Choose a reason for hiding this comment

mbayopanda Jan 26, 2018

Choose a reason for hiding this comment

mbayopanda commented Jan 26, 2018

jniles commented Jan 26, 2018

jniles commented Jan 26, 2018

mbayopanda left a comment

Choose a reason for hiding this comment

mbayopanda commented Jan 29, 2018

bors bot commented Jan 29, 2018

Build succeeded

jniles commented Jan 24, 2018 •

edited

Loading