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.

Sign up for GitHub

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

Jump to bottom

Exporting the Chart of Accounts to Excel gives confusing columns. #2436

Closed
jniles opened this issue Jan 2, 2018 · 1 comment · Fixed by #2440
Closed

Exporting the Chart of Accounts to Excel gives confusing columns. #2436

jniles opened this issue Jan 2, 2018 · 1 comment · Fixed by #2440
Assignees
@jniles
Labels
Refactor

Comments

@jniles
Copy link
Collaborator

jniles commented Jan 2, 2018

Exporting the chart of accounts to Excel from the Account Management module gives a really confusing export. I created a custom chart of accounts for fun, and have uploaded it here to demonstrate:
Chart of Accounts 2018-1-2.xlsx

Observe that we have multiple columns discussing title accounts:

  1. type (comes from an SQL join on type_id)
  2. is_title (a legacy column in the account table)
  3. is_title_account (computed ... somewhere. It looks like it is hardcoded to be TRUE when type_id === 3).

None of these columns correspond! This is very confusing. I propose we remove all hard-coded references to the title accounts as 3 and remove the is_title account until we actually use it.
@jniles jniles added the Refactor label Jan 2, 2018
@jniles jniles self-assigned this Jan 9, 2018
@jniles
Copy link
Collaborator Author

jniles commented Jan 9, 2018

I'll tackle this one.

jniles added a commit to jniles/bhima that referenced this issue Jan 9, 2018
@jniles
fix(accounts): unify account types
483b3f3 
This commit removes all the `is_` account prefixes to replace them with
account_type checks instead.  The only one actually used was `is_title`
which has been replaced with an account type check.

Closes IMA-WorldHealth#2436.
@jniles jniles mentioned this issue Jan 9, 2018
Merged
bors bot added a commit that referenced this issue Jan 14, 2018
@bors
Merge #2440
c2d886a 
2440: Fix: Chart of Accounts r=jniles a=jniles

This PR fixes two bugs in the accounts management:
 1. It removes all `is_*` columns from the account table.  These should typically be represented as `type_id`s. 
 2. Properly computes the `$$treeLevel` on the Accounts Management grid.  The previous version of the code did not factor in an undefined "depth" resulting in broken grouping behavior.

Closes #2436.
Closes #2433.
@bors bors bot closed this as completed in #2440 Jan 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jniles jniles

Labels
Refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix: Chart of Accounts jniles/bhima
1 participant
@jniles