Skip to content

Commit

Permalink
Merge #2220
Browse files Browse the repository at this point in the history
2220: fix(vouchers): undo reversals on delete r=DedrickEnc a=jniles

This commit updates the voucher delete method to undo the "reversal"
flag on the reversed record if the voucher has been removed.  It is
currently a naive method - simply attempt to remove the value if it is
present.  This can be improved in the future by checking the transaction
types first.

Closes #2207.
  • Loading branch information
bors[bot] committed Oct 20, 2017
2 parents af66528 + 581ce95 commit 92ccb55
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 8 deletions.
47 changes: 41 additions & 6 deletions server/controllers/finance/vouchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@ function lookupVoucher(vUuid) {
});
}

// NOTE(@jniles) - this is used to find references for both vouchers and credit notes.
const REFERENCE_SQL = `
v.uuid IN (
SELECT DISTINCT voucher.uuid FROM voucher JOIN voucher_item
ON voucher.uuid = voucher_item.voucher_uuid
WHERE voucher_item.document_uuid = ? OR voucher.reference_uuid = ?
)`;

function find(options) {
db.convert(options, ['uuid', 'reference_uuid', 'entity_uuid', 'cash_uuid', 'invoice_uuid']);

Expand Down Expand Up @@ -151,9 +159,8 @@ function find(options) {
// @todo - could this be improved
filters.custom('account_id', 'v.uuid IN (SELECT DISTINCT voucher_uuid FROM voucher_item WHERE account_id = ?)');

filters.custom('invoice_uuid', 'v.uuid IN (SELECT DISTINCT voucher_uuid FROM voucher_item WHERE document_uuid = ?)');

filters.custom('cash_uuid', 'v.uuid IN (SELECT DISTINCT voucher_uuid FROM voucher_item WHERE document_uuid = ?)');
filters.custom('invoice_uuid', REFERENCE_SQL, [options.invoice_uuid, options.invoice_uuid]);
filters.custom('cash_uuid', REFERENCE_SQL, [options.cash_uuid, options.cash_uuid]);

// @TODO Support ordering query (reference support for limit)?
filters.setOrder('ORDER BY v.date DESC');
Expand Down Expand Up @@ -246,8 +253,11 @@ function create(req, res, next) {
* @function safelyDeleteVoucher
*
* @description
* This function deletes a voucher from the system. It assumes that
* checks have already been made for referencing transactions.
* This function deletes a voucher from the system. The method first checks
* that a transaction can be deleted using the shared transaction library.
* After removing the voucher, it also updates and "reversal" flags if necessary
* to ensure that cash payments and invoices do not maintain broken links to
* vouchers that have been deleted.
*/
function safelyDeleteVoucher(guid) {
const DELETE_TRANSACTION = `
Expand All @@ -266,15 +276,40 @@ function safelyDeleteVoucher(guid) {
DELETE FROM document_map WHERE uuid = ?;
`;

// NOTE(@jniles) - this is a naive way of undoing reversals. If no value is
// matched, nothing happens. This can be improved in the future by first
// checking if the voucher's transaction_type is a reversal type, and then
// performing or skipping this step based on that result.

const TOGGLE_INVOICE_REVERSAL = `
UPDATE invoice
JOIN voucher ON invoice.uuid = voucher.reference_uuid
SET invoice.reversed = 0
WHERE voucher.uuid = ?;
`;

const TOGGLE_CASH_REVERSAL = `
UPDATE cash
JOIN voucher ON cash.uuid = voucher.reference_uuid
SET cash.reversed = 0
WHERE voucher.uuid = ?;
`;

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

transaction
.addQuery(DELETE_TRANSACTION, binaryUuid)
.addQuery(DELETE_TRANSACTION_HISTORY, binaryUuid)

// note that we have to delete the toggles before removing the voucher
// wholesale.
.addQuery(TOGGLE_INVOICE_REVERSAL, binaryUuid)
.addQuery(TOGGLE_CASH_REVERSAL, binaryUuid)

.addQuery(DELETE_VOUCHER, binaryUuid)
.addQuery(DELETE_TRANSACTION_HISTORY, binaryUuid)
.addQuery(DELETE_DOCUMENT_MAP, binaryUuid);

return transaction.execute();
Expand Down
7 changes: 7 additions & 0 deletions server/models/procedures/voucher.sql
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ BEGIN
FROM general_ledger AS gl WHERE gl.record_uuid = uuid
) AS zz;

-- update the "amount" with the sum of the voucher_items. We could choose either
-- debits or credits to sum here ... they should be equivalent.
UPDATE voucher SET amount = (
SELECT SUM(vi.debit) FROM (
SELECT * FROM voucher_item) AS vi WHERE vi.voucher_uuid = voucher.uuid
) WHERE voucher.uuid = voucher_uuid;

-- make sure we update the invoice with the fact that it got reversed.
IF isInvoice THEN
UPDATE invoice SET reversed = 1 WHERE invoice.uuid = uuid;
Expand Down
2 changes: 1 addition & 1 deletion server/models/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1397,7 +1397,7 @@ CREATE TABLE `purchase` (
`user_id` SMALLINT(5) UNSIGNED NOT NULL,
`payment_method` TEXT,
`note` TEXT,
`status_id` TINYINT(3) UNSIGNED NOT NULL,
`status_id` TINYINT(3) UNSIGNED NOT NULL,
PRIMARY KEY (`uuid`),
UNIQUE KEY `purchase_1` (`project_id`, `reference`),
KEY `project_id` (`project_id`),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('Check Inter-Registry Links', () => {
it('Checks the link between Cash Registry -> Voucher Registry', () => {
helpers.navigate('#!/payments');
filters.resetFilters();
const row = new GridRow('CP.TPA.1');
const row = new GridRow('CP.TPA.3');
row.dropdown().click();
row.goToVoucher().click();

Expand Down

0 comments on commit 92ccb55

Please sign in to comment.