feat(journal): edit cost centers #5938
Conversation
jniles
force-pushed
the
5934-feat-allow-users-to-edit-cost-centers-in-posting-journal
branch
from
70f9194
to
e63697d
Compare
|
It appears that editing the accounts is broken with these changes. Working on tracking that down. |
jniles
force-pushed
the
5934-feat-allow-users-to-edit-cost-centers-in-posting-journal
branch
3 times, most recently
from
c643728
to
f8bcf2f
Compare
Implements the ability to edit cost centers directly in the posting
journal with the following rules:
1. Only income/expense accounts can have cost centers attached to
them.
2. If the user changes the account, the cost center is cleared.
3. An error is shown on the account edit modal if the first rule is
violated.
The user can also remove cost centers by simply selecting the blank
"null" cost center in the top of the list of cost centers.
Closes IMA-WorldHealth#5934.
jniles
force-pushed
the
5934-feat-allow-users-to-edit-cost-centers-in-posting-journal
branch
from
f8bcf2f
to
c5c4034
Compare
|
@mbayopanda this is finally ready for a review. |
There was a problem hiding this comment.
I tested this PR. I like the ability to change the cost center, etc, directly on the line instead of popping up a second level of editor.
However, I had a couple of comments about the GUI: When I select a transaction (check mark on the left), then the edit button on the top right becomes green indicating that you can edit that transaction. This is different than most of our user interface where the editor is invoked via the action menu in the last column. I suggest changing to use that approach. Second (and related to the edit button): If I enable "show posted transactions", select a posted transaction, and then delete the filter for posted transaction, the edit button on the top is still active (green) and clicking on it will edit the previously selected item. So changes in filtering that make a transaction line disappear should deselect any selected transactions that have been hidden. Note that switching to the action menu approach would eliminate this problem also.
Finally, it would be helpful (from a user interface level) if the Cost center cells which are now editable showed a placeholder title ("Select Cost Center" or something similar) so it is obvious to users that it is editable.
|
@jmcameron thanks for the review. The posting journal has always had a degree of complexity above the other "registries", so it's UI is quite different on purpose. You need super-user privileges to edit transaction, same with deleting them. You have found a very cool bug! You should still be blocked from saving the edits to a posted transaction, but it shouldn't let you to make any edits at all. Is that what you found? |
You could still hide the action column (with edit link) if the user does not have adequate privileges.
The bug is not just for posted transactions, but anytime the transaction that you have selected is hidden by changing the search filters, the edit button should be deactivated. For instance, select a transaction and note that the edit button is activated for it. Then search for another transacation so only it shows up. The edit button will still be active for the transaction you can no longer see! That is counter-intuitive and might cause errors. Also, back to the action menu approach: With the current approach, if you select two transactions, the edit button has to go into a "no-op" state and disallows editing -- but this issue could be avoided entirely if you use the action menu approach (since you can only do an action menu on one transaction at a time). So I still think the action menu is a better user interface. |
| `, | ||
| headerCellFilter : 'translate', | ||
| editableCellTemplate : 'ui-grid/dropdownEditor', | ||
| editDropdownOptionsFunction : getCostCentersDropdown, |
| // prevent confusion and accidentally associated cost centers with non-income/expense accounts | ||
| if (colDef.field === 'account_number') { | ||
| delete rowEntity.cost_center_id; | ||
| delete rowEntity.principal_center_id; |
| .then(costCenters => { | ||
| // the top should be an empty cost center to serve as a "delete" cost center | ||
| // sets the ID to null | ||
| vm.costCenters = [{ id : null, label : '' }, ...costCenters]; |
|
@jmcameron thanks for your review. I'm not sure this PR is the appropriate place to change this behavior, as we'll need to do a lot of user retraining to make it. We've had this UI since the first version of BHIMA. The logic is that we have the following functionality supported in the posting journal:
These functionalities are grouped in two areas - the left side to select the transactions to operate on, and the top to make the operation. Like this:
This is different from all our registries, but very few of our other registries support this level of complexity. If we moved some of that functionality into the right side, I believe it would increase the complexity of trying to train a user, since some functionality would be located in some places, and others in other places. The user base of the posting journal and the other registries are not generally the same people. For the filtering, I'll make an issue about it. |
|
bors r+ |
|
Build succeeded: |
jniles
deleted the
5934-feat-allow-users-to-edit-cost-centers-in-posting-journal
branch
Implements the ability to edit cost centers directly in the posting journal with the following rules:
The user can also remove cost centers by simply selecting the blank "null" cost center in the top of the list of cost centers.
Here is what it looks like:
Closes #5934.