-
Notifications
You must be signed in to change notification settings - Fork 102
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
Prevent Imbalanced transaction in Journal #1465
Prevent Imbalanced transaction in Journal #1465
Conversation
fe92da4
to
f39e919
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.
This is a good start. If we want to add more checks later, we will need to move all the logic into validateTransacation()
. Why don't you do that from the beginning?
credit += Number(journal.credit_equiv); | ||
}); | ||
|
||
return debit !== credit ? true : false; |
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.
In the future, you can just return debit !== credit
. It's probably easier to read.
@@ -532,6 +552,12 @@ function TransactionService($timeout, util, uiGridConstants, bhConstants, Notify | |||
* This function saves all transactions by | |||
*/ | |||
Transactions.prototype.save = function save() { | |||
var clientErrors = validateTransaction(this._entity); |
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.
A better way than returning (and rejecting) a boolean would be to return the error code. So, instead of returning true
or false
, return the string POSTING_JOURNAL.ERRORS.UNBALANCED_TRANSACTIONS
.
That way, anyone else who makes an error condition (like missing dates, missing values, etc) can simply add to the validate function.
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.
The best way to do this would be to add a validation key at the top of this service, like:
var ERR_IMBALANCED_TXN = 'POSTING_JOURNAL.ERRORS.UNBALANCED_TRANSACTIONS';
// later in validateTransaction()
var error;
if (debit !== credit) {
error = ERR_UNBALANCED_TXN;
}
// another condition
// another condition ..
return error;
* This method is used to check if a transaction is balanced by their record | ||
*/ | ||
function validateTransaction(entity) { | ||
var dataEntity = entity.data.data; |
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.
A better name for this is transaction
, since we are editing a transaction and the function is called validateTransaction()
.
var debit = 0, | ||
credit = 0; | ||
|
||
dataEntity.forEach(function (journal) { |
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.
A transaction is made of rows, not journals (a journal contains transactions), so a better name for this variable is row
. Or line
. Or item
. But journal
is confusing.
f39e919
to
0a3081f
Compare
* @description | ||
* This method is used to check if a transaction is balanced by their record | ||
*/ | ||
function transaction(entity) { |
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.
@lomamech, the name validateTransaction()
was fine. It's dataEntity
that should be called transaction
. Because it is a transaction, right?
0a3081f
to
036fc36
Compare
- Adding function validate transaction who check if debit is equal to credit - Restructured catching error in controller of journal
-Using the best name for function who check the balance -Using not confusing key word -Return the String for the function transaction
* This method is used to check if a transaction is balanced by their record | ||
*/ | ||
function validateTransaction(entity) { | ||
var transaction = entity.data.data; |
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.
👍 This is much clearer!
@lomamech, can you rebase this branch to master so that we can be sure everything will work properly? |
036fc36
to
4e18251
Compare
@lomamech, thanks! 👍 |
Prevent Imbalanced transaction in Journal …
closes #1256