Skip to content

Commit

Permalink
feat(journal): add transaction validation
Browse files Browse the repository at this point in the history
This commit adds "offline" validation to the transaction modal, which
was the no 1 requested item on the feedback from developers.  The intent
is not to be comprehensive, but to prevent users from making silly
mistakes.
  • Loading branch information
jniles committed Sep 25, 2017
1 parent 60eba0d commit 3872263
Show file tree
Hide file tree
Showing 3 changed files with 619 additions and 362 deletions.
103 changes: 95 additions & 8 deletions client/src/modules/journal/modals/editTransaction.modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ angular.module('bhima.controllers')

JournalEditTransactionController.$inject = [
'JournalService', 'Store', 'TransactionTypeService', '$uibModalInstance',
'transactionUuid', 'readOnly', 'uiGridConstants', 'uuid',
'transactionUuid', 'readOnly', 'uiGridConstants', 'uuid', 'util',
];

function JournalEditTransactionController(
Journal, Store, TransactionType, Modal,
transactionUuid, readOnly, uiGridConstants, uuid
Journal, Store, TransactionType, Modal, transactionUuid, readOnly, uiGridConstants, uuid, util
) {
var gridApi = {};
var vm = this;
Expand All @@ -19,6 +18,14 @@ function JournalEditTransactionController(
var addedRows = [];
var changes = {};

// must have transaction_type for certain cases
var ERROR_MISSING_TRANSACTION_TYPE = 'TRANSACTIONS.MISSING_TRANSACTION_TYPE';
var ERROR_IMBALANCED_TRANSACTION = 'TRANSACTIONS.IMBALANCED_TRANSACTION';
var ERROR_SINGLE_ACCOUNT_TRANSACTION = 'TRANSACTIONS.SINGLE_ACCOUNT_TRANSACTION';
var ERROR_SINGLE_ROW_TRANSACTION = 'TRANSACTIONS.SINGLE_ROW_TRANSACTION';
var ERROR_NEGATIVE_VALUES = 'VOUCHERS.COMPLEX.ERRORS_NEGATIVE_VALUES'
var ERROR_INVALID_DEBITS_AND_CREDITS = 'VOUCHERS.COMPLEX.ERROR_AMOUNT';

var footerTemplate =
'<div class="ui-grid-cell-contents"><span translate>POSTING_JOURNAL.ROWS</span> <span>{{grid.rows.length}}</span></div>';

Expand All @@ -29,7 +36,6 @@ function JournalEditTransactionController(
// @TODO(sfount) apply read only logic to save buttons and grid editing logic
vm.readOnly = readOnly || false;


vm.validation = {
errored : false,
blockedPostedTransactionEdit : false,
Expand Down Expand Up @@ -137,6 +143,77 @@ function JournalEditTransactionController(
vm.loadingTransaction = false;
});

/**
* @function offlineTransactionValidation
*
* @description
* This function validates transactions without doing a round-trip to the server. It implements some simple checks
* such as:
* 1. Making sure a transaction has multiple lines
* 2. Make sure a transaction is balanced
* 3. Making sure a transaction involves at least two accounts
* 4. Making sure a transaction has a transaction_type associated with it.
* 5. Make sure both the debits and credits are defined and not equal to each other.
*
* If any of these checks fail, the transaction submission is aborted until the user corrects those mistakes.
*/
function offlineTransactionValidation(rows) {
var hasSingleLine = rows.length < 2;
if (hasSingleLine) {
return ERROR_SINGLE_ROW_TRANSACTION;
}

var debits = 0;
var credits = 0;

var i = rows.length;
var row;
while (i--) {
row = rows[i];

var hasTransactionType = typeof row.origin_id === 'number';
if (!hasTransactionType) {
return ERROR_MISSING_TRANSACTION_TYPE;
}

var hasNegativeNumbers = (row.debit_equiv < 0 || row.credit_equiv < 0);
if (hasNegativeNumbers) {
return ERROR_NEGATIVE_NUMBERS;
}

window.util = util;

var hasSingleNumericValue = !util.xor(Boolean(row.debit_equiv), Boolean(row.credit_equiv));
if (hasSingleNumericValue) {
return ERROR_INVALID_DEBITS_AND_CREDITS;
}

credits += row.credit_equiv;
debits += row.debit_equiv;
}

var uniqueAccountsArray = rows
.map(function (row) {
return row.account_id;
})
.filter(function (accountId, index, array) {
return array.indexOf(accountId) === index;
});

var hasSingleAccount = uniqueAccountsArray.length === 1;
if (hasSingleAccount) {
return ERROR_SINGLE_ACCOUNT_TRANSACTION;
}

var hasImbalancedTransaction = Number(debits.toFixed('2')) !== Number(credits.toFixed('2'));
if (hasImbalancedTransaction) {
return ERROR_IMBALANCED_TRANSACTION;
}

return false;
}


function verifyEditableTransaction(transaction) {
var posted = transaction[0].posted;

Expand Down Expand Up @@ -184,10 +261,13 @@ function JournalEditTransactionController(
return;
}

// reset error validation
vm.validation.errored = false;
vm.validation.message = null;
vm.saving = true;
// run local validation before submission
var offlineErrors = offlineTransactionValidation(vm.rows.data);
if (offlineErrors) {
vm.validation.errored = true;
vm.validation.message = offlineErrors;
return;
}

// building object to conform to legacy API
// @FIXME(sfount) update journal service API for human readable interface
Expand All @@ -197,6 +277,13 @@ function JournalEditTransactionController(
removedRows : removedRows.map(mapRowUuids),
};


// reset error validation
vm.validation.errored = false;
vm.validation.message = null;
vm.saving = true;


Journal.saveChanges(transactionRequest, changes)
.then(function (resultUpdatedTransaction) {
var transaction = new Store({ identifier : 'uuid' });
Expand Down
30 changes: 16 additions & 14 deletions test/end-to-end/edit_journal/edit_journal.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,27 @@ describe('Edit Posting Journal', () => {
GU.selectRow(gridId, 0);
FU.buttons.edit();

const debitCell = GU.getCell(editingGridId, 0, 2);
const creditCell = GU.getCell(editingGridId, 1, 3);
const debitCellA = GU.getCell(editingGridId, 0, 2);
const creditCellA = GU.getCell(editingGridId, 0, 3);
const debitCellB = GU.getCell(editingGridId, 1, 2);
const creditCellB = GU.getCell(editingGridId, 1, 3);

doubleClick(debitCell);
debitCell.element(by.css('input')).sendKeys(150);
doubleClick(debitCellA);
debitCellA.element(by.css('input')).sendKeys(150);
doubleClick(creditCellA);
creditCellA.element(by.css('input')).sendKeys(0);

doubleClick(creditCell);
creditCell.element(by.css('input')).sendKeys(150);
doubleClick(debitCellB);
debitCellB.element(by.css('input')).sendKeys(0);
doubleClick(creditCellB);
creditCellB.element(by.css('input')).sendKeys(150);

FU.buttons.submit();
components.notification.hasSuccess();
});

// Test for validation
// @TODO(sfount) this logic is no longer validated by the client or the server - if this validation is reimplemented the test can be added
it.skip('Preventing a single-line transaction', () => {
it('Preventing a single-line transaction', () => {
GU.selectRow(gridId, 1);
FU.buttons.edit();

Expand All @@ -63,8 +68,7 @@ describe('Edit Posting Journal', () => {
FU.buttons.cancel();
});

// @TODO(sfount) this logic is no longer validated by the client or the server - if this validation is reimplemented the test can be added
it.skip('Preventing unbalanced transaction', () => {
it('Preventing unbalanced transaction', () => {
GU.selectRow(gridId, 1);
FU.buttons.edit();

Expand All @@ -82,8 +86,7 @@ describe('Edit Posting Journal', () => {
FU.buttons.cancel();
});

// @TODO(sfount) this logic is no longer validated by the client or the server - if this validation is reimplemented the test can be added
it.skip('Preventing transaction who have debit and Credit null', () => {
it('Preventing transaction who have debit and Credit null', () => {
GU.selectRow(gridId, 1);
FU.buttons.edit();

Expand All @@ -101,8 +104,7 @@ describe('Edit Posting Journal', () => {
FU.buttons.cancel();
});

// @TODO(sfount) this logic is no longer validated by the client or the server - if this validation is reimplemented the test can be added
it.skip('Preventing transaction who was debited and Credited in a same line', () => {
it('Preventing transaction who was debited and Credited in a same line', () => {
GU.selectRow(gridId, 1);
FU.buttons.edit();

Expand Down
Loading

0 comments on commit 3872263

Please sign in to comment.