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

Creating an account with "0" account number breaks account management. #2433

Closed
jniles opened this issue Jan 2, 2018 · 0 comments · Fixed by #2440 or #2446
Closed

Creating an account with "0" account number breaks account management. #2433

jniles opened this issue Jan 2, 2018 · 0 comments · Fixed by #2440 or #2446
Assignees
@jeremielodi
Labels
Accounts Bug

Comments

@jniles
Copy link
Collaborator

jniles commented Jan 2, 2018

If a user makes an account with the account number "0", account management breaks. Specifically, grouping breaks and collapsing at the root does not work anymore. See below:

weirdaccounts
Fig 1: All of these are parent accounts except 100.

It also makes for a confusing "parent account" dropdown. See below:
createaccountparentaccount
Fig 1: Two "0" Accounts?

There are two workarounds to this - either allow the parent account number to be NULL or simply disallow "0" as an account number the user can select. I'm in favor of the second option personally, since it requires the least work and allows the database to enforce its NOT NULL constraint. But I am open to any suggestion.

You also cannot delete an account with account number "0".
jniles added a commit to jniles/bhima that referenced this issue Jan 9, 2018
@jniles
fix(accounts): properly compute $$treeLevel
41ef757 
This commit ensures the $$treeLevel is properly computed by checking if
the account depth is undefined and assigning it -1 if so.  When the tree
level is not properly defined, the grouping in the accounts management
module breaks.

Closes Third-Culture-Software#2433.
@jniles jniles mentioned this issue Jan 9, 2018
Merged
jniles added a commit to jniles/bhima that referenced this issue Jan 9, 2018
@jniles
fix(accounts): properly compute $$treeLevel
799efa9 
This commit ensures the $$treeLevel is properly computed by checking if
the account depth is undefined and assigning it -1 if so.  When the tree
level is not properly defined, the grouping in the accounts management
module breaks.

Closes Third-Culture-Software#2433.
@jeremielodi jeremielodi self-assigned this Jan 12, 2018
@jeremielodi jeremielodi mentioned this issue Jan 12, 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
bors bot added a commit that referenced this issue Jan 23, 2018
@bors
Merge #2446
d386f37 
2446: feat(account) prevent the user to set 0 as account number r=jniles a=jeremielodi

This PR  oblige the user to enter an account number different to 0.
closes #2433
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jeremielodi jeremielodi

Labels
Accounts Bug
Projects
None yet
Milestone
No milestone
2 participants
@jniles @jeremielodi