feat(stock): add option to view AMC calcs #5317
Conversation
|
@jmcameron, this code isn't ready to be merged (I need to check translations and such), but could you give me a review on the user interface and user experience? I've been looking at this too long to know if my ideas are any good or not. It should "just work" with the latest IMCK/Vanga databases for testing. |
There was a problem hiding this comment.
I looked over the code and it looked good.
I tested this with the IMCK database it looks great. I did spot-check several of the CMM calculations to make sure the numbers worked. Everything looks great.
One small/cosmetic/ignorable suggestion: Make the values for the CMM column to links to the same function the action menu invokes. It could include a tooltip: "Click here to see the details of this calculation.". Ignore this if you think this would slow things down too much.
|
@jmcameron thanks for your review! Let me take a look at figuring out how to do that linking. I'll ping you for a final review when that is done. |
There was a problem hiding this comment.
Good job @jniles
I have noticed that :
-
The calculation of the Algo 3 doesn't match with what is displayed as an example 30.5 × (200 / 1) <> 32.97:
-
The calculation of Algo 2 doesn't match with the example 30.5 × (200 / 185) <> 6100
It seems like the example for Algo 2 is given to Algo 3, and vice-versa
And the french translation
| vm.isAlgo1 = vm.settings.average_consumption_algo === 'algo1'; | ||
| vm.isAlgo2 = vm.settings.average_consumption_algo === 'algo2'; | ||
| vm.isAlgo3 = vm.settings.average_consumption_algo === 'algo3'; | ||
| vm.isAlgo4 = vm.settings.average_consumption_algo === 'algo_msh'; |
|
Good catch @mbayopanda! I'll switch those algorithm examples. |
jniles
force-pushed
the
5291-feat-add-cmm-calculations-to-articles-in-stock-registry
branch
from
f252161 to
6d70a6e
Compare
There was a problem hiding this comment.
I could not get the double-clicking on the CMM value to work. I suggest making it a link with tooltip, if you have time.
I went back and did some more spot-checks (which I should have done the first time!). The calculations for Alg 1 and 4 seem fine. But I'm seeing errors on 2 and 3. See below.
|
Okay, this confusion that @jmcameron pointed out and that @mbayopanda pointed out is because we change the names of our algorithms somewhere between the concept note, the translation, and the calculation. I'll try to track it all down. |
Adds the ability to view the AMC/CMM calculations in the Articles in Stock Registry. A summary of the information is presented for the user to examine and the explicit formula used to calculate the AMC is shown for the user to validate. Closes Third-Culture-Software#5291.
This commit adds the ability to double click on the CMM cell to view the CMM calculation for the row as suggested by @jmcameron.
jniles
force-pushed
the
5291-feat-add-cmm-calculations-to-articles-in-stock-registry
branch
2 times, most recently
from
87d0204 to
b7e0fc9
Compare
Allows you to double click from anywhere in the cell, plus removes the double click on the row header. Starts process towards getting proper AMC quantity in stock projections working. Finally, adds translation for french. Closes Third-Culture-Software#5291.
jniles
force-pushed
the
5291-feat-add-cmm-calculations-to-articles-in-stock-registry
branch
from
b7e0fc9 to
66780fb
Compare
|
@jmcameron can you give me another review? I've integrated the french language translations and attempted to polish up the double-click experience. I believe this should solve the issues you were having with double-clicking. I would prefer not to put a label/title/tooltip on it for performance reasons. I also don't think it enhances the experience that much after testing it myself, and may hurt a users ability copy/paste as it gets in the way. If you feel strongly about it, I can put it in. For completeness, here is what the current version looks like:
I've noticed that in Vanga's database, the min/max isn't being properly computed, making everything either "in stock" or "over maximum". I'm taking a look at that. |
|
Actually, I just realized the screen shot I posted doesn't make any sense. How can we have more days of stock consumption than we do of stock available? Hmm.... I hope our CMM calculation is working. |
There was a problem hiding this comment.
I tried the CMM double-click and it works fine now.
Your decision to not add a tool-tip makes sense.
Let me know if you want me to review it again after straightening out the CMM calculations.
BTW, on the last screen-shot you have of the CMM calculations for Algo 4: The fact that algorithms 1 and 2 have much bigger numbers than 3 and 4 is concerning too.
|
@jmcameron, thanks for your review. I've updated the base branch so that we will merge this off the main branch. |
|
bors r+ |
|
Build succeeded: |
jniles
deleted the
5291-feat-add-cmm-calculations-to-articles-in-stock-registry
branch
Adds the ability to view the AMC/CMM calculations in the Articles in Stock Registry. A summary of the information is presented for the user to examine and the explicit formula used to calculate the AMC is shown for the user to validate.
Closes #5291.
Example using Algorithm 1
Example using Algorithm 2
Example using Algorithm 3
Example using Algorithm 4 (MSH)
Example showing Algorithm (MSH) for 12 months
