-
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
Add more details about stock exit on the voucher receipt #4000
Add more details about stock exit on the voucher receipt #4000
Conversation
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.
Technically, this solves the problem, but it can't be translated. Did you look into creating the string in JS as we've done with other descriptions? That would make the most sense to me, if it is possible.
server/models/procedures/stock.sql
Outdated
SET voucher_description = CONCAT('Distribution to ', v_patient_description, ' from ', v_depot_description, ' : ', v_voucher_description); | ||
END IF; | ||
|
||
IF (v_depot_description IS NOT NULL AND v_patient_description IS NOT NULL AND v_invoice_description IS NOT NULL) THEN |
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.
Oof. This is super complicated. Can't we find a way to do this in JavaScript to get regular translation support? For example, this is how the patient invoice description is created: https://github.com/IMA-WorldHealth/bhima/blob/master/client/src/i18n/en/patient_invoice.json#L15.
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.
We must add a server side api end point for getting details of the description, and before to create a stock movement we must ask the API end point for having description details and after that we can create the stock movement on the client side.
@@ -755,7 +755,7 @@ INSERT INTO `voucher` (uuid, `date`, project_id, currency_id, amount, descriptio | |||
(@fourth_voucher, CURRENT_TIMESTAMP, 1, 1, 75, 'Fourth Voucher to be Posted', 1, 9); | |||
|
|||
-- voucher items sample data | |||
INSERT INTO `voucher_item` VALUES | |||
INSERT INTO `voucher_item` (`uuid`, `account_id`, `debit`, `credit`, `voucher_uuid`, `document_uuid`, `entity_uuid`) VALUES |
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.
👍
@@ -1865,6 +1865,7 @@ CREATE TABLE IF NOT EXISTS `voucher_item` ( | |||
`voucher_uuid` BINARY(16) NOT NULL, | |||
`document_uuid` binary(16) default null, | |||
`entity_uuid` binary(16) default null, | |||
`description` TEXT NULL, |
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.
👍
7f9e515
to
e1c4bcb
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.
@mbayopanda, this is a lot better! I've made a few comments to tighten up the code in the client controller. Let me know what you think.
"EXIT_SERVICE" : "Exit to a service", | ||
"EXIT_DEPOT" : "Distribution to a depot", | ||
"EXIT_PATIENT" : "Distribution to a patient", | ||
"EXIT_PATIENT_ADVANCED" : "Distribution to the patient {{patient}} from depot {{depot}}", |
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.
👍
server/controllers/admin/services.js
Outdated
|
||
db.exec(sql) | ||
filters.custom('uuid', 's.uuid = ?'); |
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 equals not work here?
filters.equals('uuid', 'uuid', 's');
@@ -61,7 +61,13 @@ | |||
{{#each items}} | |||
<tr> | |||
<td>{{number}}</td> | |||
<td>{{label}}</td> | |||
<td> | |||
{{#if description}} |
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.
👍
@@ -427,6 +429,53 @@ function StockExitController( | |||
}; | |||
} | |||
|
|||
function buildDescription(entityUuid, invoiceUuid) { | |||
const glb = {}; | |||
return PatientService.read(null, { uuid : entityUuid }) |
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.
If these services are based on the PrototypeAPIService
, you should be able to call .read()
with the uuid
directly to return a JSON object. For example:
PatientService.read(null, { uuid : entityUuid })
.then(([patient]) => {});
becomes
PatientService.read(entityUuid)
.then(patient => {});
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 reason here is i don't want to get an error if there is nothing, i want to have an empty array instead.
function buildDescription(entityUuid, invoiceUuid) { | ||
const glb = {}; | ||
return PatientService.read(null, { uuid : entityUuid }) | ||
.then(([patient]) => { |
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.
I think you can probably combine a lot of these calls together to save space and time. For example:
// we use all settled here, since any of these might be undefined.
return Promise.allSettled([
PatientService.read(entityUuid),
PatientInvoiceService.read(invoiceUuid),
ServiceService.read(entityUuid),
]).then(([patient, invoice, service]) => {
// etc..
});
return PatientService.read(null, { uuid : entityUuid }) | ||
.then(([patient]) => { | ||
if (patient) { | ||
glb.patientDescription = patient.display_name.concat(` (${patient.reference})`); |
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.
If we are mixing both $translate and String.concat(), it probably means we can write a better translation template.
I think what you are trying to achieve is to have a translation with something like this:
Distribution to the patient JNILES (PA.TBD.123) from ...
And your template is
Distribution to the patient {{patient}} from...
I propose you change your template to be:
Distribution to the patient {{patientName}} ({{patientReference}})
And then you could just provide the patientName
and patientReference
as two different translation keys.
} | ||
|
||
if (vm.depot.text && glb.patientDescription && glb.invoiceDescription) { | ||
description = $translate.instant('STOCK.EXIT_PATIENT_ADVANCED_WITH_INVOICE', { |
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.
It looks like your translation key object doesn't change all that much. Let's put all the keys in a single object and use them for each translation. Like this:
const i18nKeys = {
patient : glb.patientDescription,
depot : vm.depot.text,
invoice : glb.invoiceDescription
};
// later..
if (x && y) {
description = $translate.instant('STOCK.EXIT_PATIENT_ADVANCED', i18nKeys);
}
if (y && z) {
description = $translate.instant('STOCK.EXIT_PATIENT_ADVANCED_WITH_INVOICE', i18nKeys);
}
e1c4bcb
to
ddcf514
Compare
@jniles could you review this PR again please |
…o upgrade-stock-voucher-receipt
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.
@mbayopanda, this looks like a good first implementation to me. I would still like to see the stock movement description translated, but we can do that after this is merged.
Once the merge issues are fixed, we can "bors" it.
bors r+ |
4000: Add more details about stock exit on the voucher receipt r=jniles a=mbayopanda This PR update description of the voucher and items of the voucher receipt. ![voucher stock details](https://user-images.githubusercontent.com/5445251/68940634-051c6780-07a4-11ea-8ecf-e7be588f43f6.png) Close #3990 Co-authored-by: mbayopanda <mbayopanda@gmail.com>
This PR update description of the voucher and items of the voucher receipt.
Close #3990