-
Notifications
You must be signed in to change notification settings - Fork 105
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
improvement(Stock Mouvement Document) #4114
improvement(Stock Mouvement Document) #4114
Conversation
@mbayopanda can you review this PR |
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.
Made a few comments on the code - haven't checked the functionality yet, but the idea is the right one!
@@ -28,7 +28,23 @@ async function stockAdjustmentReceipt(documentUuid, session, options) { | |||
WHERE m.flux_id IN (${Stock.flux.FROM_ADJUSTMENT}, ${Stock.flux.TO_ADJUSTMENT}) AND m.document_uuid = ? | |||
`; | |||
|
|||
const rows = await db.exec(sql, [db.bid(documentUuid)]); | |||
const sqlGetVoucherReference = ` |
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.
Does every receipt need sqlGetVoucherReference
? If so, I would make it a function and put it in the stock/reports/common.js
.
Like this:
function getVoucherReferenceForStockMovement(documentUuid) {
const sql = `
SELECT v.uuid, dm.text AS voucher_reference
FROM voucher AS v
JOIN voucher_item AS vi ON vi.voucher_uuid = v.uuid
JOIN document_map AS dm ON dm.uuid = v.uuid
WHERE vi.document_uuid = ?
LIMIT 1;
`;
return db.exec(sql, [db.bid(documentUuid)]);
}
exports.getVoucherReferenceForStockMovement = getVoucherReferenceForStockMovement;
Then every stock module can import it and just use:
const { getVoucherReferenceForStockMovement } = require('../common');
// later ..
const results = await q.all([
db.exec(sql, [db.bid(documentUuid)]),
getVoucherReferenceForStockMovement(documentUuid),
]);
// etc
This would be more DRY.
LIMIT 1; | ||
`; | ||
|
||
return q.all([ |
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.
By the way, you don't need to use q
here. You can just call the native Promise.all()
directly.
Set Voucher reference to - Adjustment Receipt - Donation Receipt - Integration Receipt - Purchase Receipt - Loss Receipt - Patient Receipt closes Third-Culture-Software#2694
- Set the function getVoucherReferenceForStockMovement in stock/reports/common.js
85e40ba
to
5625a89
Compare
@jniles all checks have passed |
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.
Looks good to me. Thanks a ton!
bors r+ |
4114: improvement(Stock Mouvement Document) r=jniles a=lomamech Set Voucher reference to - Adjustment Receipt - Donation Receipt - Integration Receipt - Purchase Receipt - Loss Receipt - Patient Receipt closes #2694 Co-authored-by: Chris Lomame <[email protected]>
Set Voucher reference to
closes #2694