Skip to content

Commit

Permalink
feat: decouple transaction checks
Browse files Browse the repository at this point in the history
This commit decouples the transaction checks from transaction controller
so that they can be shared between multiple controllers.  This ensures
safety in case a developers accidentally writes code that uses the
delete interface to delete transactions w/o ensuring the transactions
are properly dereferenced.
  • Loading branch information
jniles committed Oct 15, 2017
1 parent 0b38588 commit 8ddf37d
Show file tree
Hide file tree
Showing 10 changed files with 193 additions and 132 deletions.
4 changes: 2 additions & 2 deletions client/src/i18n/fr/form.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
},
"DIALOGS": {
"CONFIRM_ACTION": "Confirmez vous cette action ?",
"CONFIRMATION_NEEDED" : "Confirmation Nécessaire",
"CONFIRMATION_NEEDED" : "Confirmation Requise",
"CONFIRM_ACTIVATION": "Voulez vous vraiment activer les droits d'accès de cet utilisateur",
"CONFIRM_CREDIT_NOTE": "La facture dont vous voulez annuler a déjà été payée, voulez-vous vraiment annuler cette facture ",
"CONFIRM_DEACTIVATION": "Voulez vous vraiment blocker les droits d'accès de cet utilisateur",
Expand Down Expand Up @@ -157,7 +157,7 @@
"UNPOSTED_TRANSACTION": "Transactions non-posté ne sont pas affichées",
"TRANSACTIONS": "Transactions",
"ROWS": "enregistrements",
"DELETE_RECORD_SUCCESS": "Supprimèe la transaction avec succès."
"DELETE_RECORD_SUCCESS": "Suppression de la transaction avec succès."
},
"LABELS": {
"ABBREVIATION": "Abbreviation",
Expand Down
4 changes: 2 additions & 2 deletions client/src/modules/cash/payments/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function CashPaymentRegistryController(
vm.cancelCash = cancelCash;
vm.openColumnConfigModal = openColumnConfigModal;
vm.clearGridState = clearGridState;
vm.deleteCashPayment = deleteCashPayment;
vm.deleteCashPayment = deleteCashPaymentWithConfirmation;
vm.download = Cash.download;

columnDefs = [{
Expand Down Expand Up @@ -217,7 +217,7 @@ function CashPaymentRegistryController(

// this function deletes the cash payment and associated transactions from
// the database
function deleteCashPayment(entity) {
function deleteCashPaymentWithConfirmation(entity) {
Modals.confirm('FORM.DIALOGS.CONFIRM_DELETE')
.then(function (isOk) {
if (isOk) { remove(entity); }
Expand Down
8 changes: 4 additions & 4 deletions client/src/modules/invoices/registry/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function InvoiceRegistryController(
vm.creditNote = creditNote;
vm.bhConstants = bhConstants;
vm.download = Invoices.download;
vm.deleteInvoice = deleteInvoice;
vm.deleteInvoice = deleteInvoiceWithConfirmation;

// track if module is making a HTTP request for invoices
vm.loading = false;
Expand Down Expand Up @@ -190,13 +190,13 @@ function InvoiceRegistryController(
$state.reload();
};

// Call the opening of Modal
// Call the opening of Modal
function openModal(invoice) {
Invoices.openCreditNoteModal(invoice)
.then(function (success) {
if (success) {
Notify.success('FORM.INFO.TRANSACTION_REVER_SUCCESS');
return load(vm.filters);
load(vm.filters);
}
})
.catch(Notify.handleError);
Expand Down Expand Up @@ -234,7 +234,7 @@ function InvoiceRegistryController(
}

// check if it is okay to remove the entity.
function deleteInvoice(entity) {
function deleteInvoiceWithConfirmation(entity) {
Modals.confirm('FORM.DIALOGS.CONFIRM_DELETE')
.then(function (isOk) {
if (isOk) { remove(entity); }
Expand Down
4 changes: 2 additions & 2 deletions client/src/modules/vouchers/voucher-registry.ctrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function VoucherController(
vm.openColumnConfigModal = openColumnConfigModal;
vm.clearGridState = clearGridState;
vm.download = Vouchers.download;
vm.deleteVoucher = deleteVoucher;
vm.deleteVoucher = deleteVoucherWithConfirmation;

vm.loading = false;

Expand Down Expand Up @@ -248,7 +248,7 @@ function VoucherController(
}

// this function deletes the voucher from the database
function deleteVoucher(entity) {
function deleteVoucherWithConfirmation(entity) {
Modals.confirm('FORM.DIALOGS.CONFIRM_DELETE')
.then(function (isOk) {
if (isOk) { remove(entity); }
Expand Down
23 changes: 15 additions & 8 deletions server/controllers/finance/cash.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ const { BadRequest, NotFound } = require('../../lib/errors');
const identifiers = require('../../config/identifiers');
const cashCreate = require('./cash.create');

// shared transaction methods
// TODO(@jniles) - find a better name
const shared = require('./shared');

exports.detail = detail;
exports.read = read;
exports.create = cashCreate;
Expand Down Expand Up @@ -282,14 +286,17 @@ function safelyDeleteCashPayment(uuid) {
DELETE FROM document_map WHERE uuid = ?;
`;

const binaryUuid = db.bid(uuid);
const transaction = db.transaction();
return shared.isRemovableTransaction(uuid)
.then(() => {
const binaryUuid = db.bid(uuid);
const transaction = db.transaction();

transaction
.addQuery(DELETE_TRANSACTION, binaryUuid)
.addQuery(DELETE_TRANSACTION_HISTORY, binaryUuid)
.addQuery(DELETE_CASH_PAYMENT, binaryUuid)
.addQuery(DELETE_DOCUMENT_MAP, binaryUuid);
transaction
.addQuery(DELETE_TRANSACTION, binaryUuid)
.addQuery(DELETE_TRANSACTION_HISTORY, binaryUuid)
.addQuery(DELETE_CASH_PAYMENT, binaryUuid)
.addQuery(DELETE_DOCUMENT_MAP, binaryUuid);

return transaction.execute();
return transaction.execute();
});
}
21 changes: 13 additions & 8 deletions server/controllers/finance/patientInvoice.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ const BadRequest = require('../../lib/errors/BadRequest');
const createInvoice = require('./invoice/patientInvoice.create');
const Debtors = require('./debtors');

const shared = require('./shared');

const entityIdentifier = identifiers.INVOICE.key;
const CREDIT_NOTE_ID = 10;

Expand Down Expand Up @@ -303,14 +305,17 @@ function safelyDeleteInvoice(guid) {
DELETE FROM document_map WHERE uuid = ?;
`;

const binaryUuid = db.bid(guid);
const transaction = db.transaction();
return shared.isRemovableTransaction(guid)
.then(() => {
const binaryUuid = db.bid(guid);
const transaction = db.transaction();

transaction
.addQuery(DELETE_TRANSACTION, binaryUuid)
.addQuery(DELETE_TRANSACTION_HISTORY, binaryUuid)
.addQuery(DELETE_INVOICE, binaryUuid)
.addQuery(DELETE_DOCUMENT_MAP, binaryUuid);
transaction
.addQuery(DELETE_TRANSACTION, binaryUuid)
.addQuery(DELETE_TRANSACTION_HISTORY, binaryUuid)
.addQuery(DELETE_INVOICE, binaryUuid)
.addQuery(DELETE_DOCUMENT_MAP, binaryUuid);

return transaction.execute();
return transaction.execute();
});
}
134 changes: 134 additions & 0 deletions server/controllers/finance/shared.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/**
* @overview shared.js
*
* @description
* This module contains helper functions for operating on transactions. These
* helper functions do things like like
*
* @requires lib/db
* @requires lib/errors/BadRequest
*/

const db = require('../../lib/db');
const BadRequest = require('../../lib/errors/BadRequest');

exports.getTransactionReferences = getTransactionReferences;
exports.getTransactionRecords = getTransactionRecords;
exports.isRemovableTransaction = isRemovableTransaction;

exports.getRecordTextByUuid = getRecordTextByUuid;
exports.getEntityTextByUuid = getEntityTextByUuid;


/**
* @function getRecordTextByUuid
*
* @description
* This function returns a record's human readable text string by its uuid.
*/
function getRecordTextByUuid(uuid) {
const sql = `
SELECT text FROM document_map WHERE uuid = ?;
`;

return db.one(sql, db.bid(uuid));
}

/**
* @function getEntityTextByUuid
*
* @description
* This function returns an entity's human readable text from the entity_map
* table.
*/
function getEntityTextByUuid(uuid) {
const sql = `
SELECT text FROM entity_map WHERE uuid = ?;
`;

return db.one(sql, db.bid(uuid));
}

/**
* @function getTransactionReferences
*
* @description
* This function will find the uuids of any transactions that reference the
* provided transaction's uuid.
*
* @param {String} transactionUuid - the record_uuid of the transaction
*/
function getTransactionReferences(transactionUuid) {
const sql = `
SELECT DISTINCT uuid, text FROM (
SELECT dm.uuid, dm.text
FROM posting_journal AS j JOIN document_map AS dm ON
j.reference_uuid = dm.uuid
WHERE j.reference_uuid = ?
UNION ALL
SELECT dm.uuid, dm.text
FROM general_ledger AS g JOIN document_map AS dm ON
g.reference_uuid = dm.uuid
WHERE g.reference_uuid = ?
)c;
`;

const buid = db.bid(transactionUuid);

return db.exec(sql, [buid, buid]);
}

/**
* @function getTransactionRecords
*
* @description
* Returns the transaction from the posting journal and general_ledger.
*/
function getTransactionRecords(uuid) {
const sql = `
SELECT BUID(j.uuid) AS uuid, trans_id, BUID(record_uuid) AS record_uuid,
trans_date, debit_equiv, credit_equiv, currency_id,
BUID(reference_uuid) AS reference_uuid,
BUID(entity_uuid) AS entity_uuid, 0 AS posted,
document_map.text AS identifier
FROM posting_journal AS j JOIN document_map ON
j.record_uuid = document_map.uuid
WHERE record_uuid = ?
UNION ALL
SELECT BUID(j.uuid) AS uuid, trans_id, BUID(record_uuid) AS record_uuid,
trans_date, debit_equiv, credit_equiv, currency_id,
BUID(reference_uuid) AS reference_uuid,
BUID(entity_uuid) AS entity_uuid, 1 AS posted,
document_map.text AS identifier
FROM general_ledger AS j JOIN document_map ON
j.record_uuid = document_map.uuid
WHERE record_uuid = ?
`;

return db.exec(sql, [db.bid(uuid), db.bid(uuid)]);
}


function isRemovableTransaction(uuid) {
// get all the rows of the transaction
return getTransactionRecords(uuid)
.then(rows => {
const isPosted = rows[0].posted;
if (isPosted) {
throw new BadRequest('This transaction is already posted.', 'TRANSACTIONS.ERRORS.TRANSACTION_POSTED');
}

// check if the transaction has references elsewhere
return getTransactionReferences(uuid);
})
.then(references => {
const isReferenced = references.length > 0;
if (isReferenced) {
throw new BadRequest('This transaction is referenced.', 'TRANSACTIONS.ERRORS.TRANSACTION_REFERENCED');
}
});
}
Loading

0 comments on commit 8ddf37d

Please sign in to comment.