-
Notifications
You must be signed in to change notification settings - Fork 100
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
Use of decimal for stock minimum months #5877
Use of decimal for stock minimum months #5877
Conversation
…o use-of-decimal-for-stock-minimum-months
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 need to also do similar input form modifications on Stock > Stock Settings (client/src/modules/stock/settings/stock-settings.html).
@@ -129,8 +129,7 @@ | |||
FORM.LABELS.MIN_MONTHS_SECURITY_STOCK | |||
</label> | |||
<input name="min_months_security_stock" class="form-control" | |||
ng-model="DepotModalCtrl.depot.min_months_security_stock" type="number" min="1" autocomplete="off" required | |||
bh-integer> | |||
ng-model="DepotModalCtrl.depot.min_months_security_stock" type="number" min="0" autocomplete="off" required> |
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 should consider making min="0.00001" (or similar) to prevent acceptance of zero.
bors try |
tryBuild succeeded: |
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 code changes look good (see my notes).
I tested this with bhima_test. Changing the values in stock settings works fine.
However when I tried it with the Depot editor, it seems like the values are set properly, but when I edit again, I see that the value "Default Purchase order interval (in months)" is still 0 (I set it to 0.25). See:
server/models/schema.sql
Outdated
@@ -461,7 +461,7 @@ CREATE TABLE `depot` ( | |||
`allow_exit_transfer` TINYINT(1) UNSIGNED NOT NULL DEFAULT 0, | |||
`allow_exit_loss` TINYINT(1) UNSIGNED NOT NULL DEFAULT 0, | |||
`location_uuid` BINARY(16) NULL, | |||
`min_months_security_stock` SMALLINT(5) NOT NULL DEFAULT 2, | |||
`min_months_security_stock` DECIMAL(19,4) NOT NULL DEFAULT 2, | |||
`parent_uuid` BINARY(16) NULL, | |||
`dhis2_uid` VARCHAR(150) DEFAULT NULL, | |||
`default_purchase_interval` SMALLINT(5) NOT NULL DEFAULT 0, |
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.
This needs to be DECIMAL as well. That is the problem @jmcameron ran into.
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 had done changes only in migrate.js
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.
This worked for me.
bors r+
Build succeeded: |
This PR is related to #5876
Close #5876