-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
FIX Bug Estimated Value stockatdate.php #23782
Conversation
It seems bug was fixed inside v17 but in a different manner. |
Hi @eldy
Don't you think It could be better to clean v14 and above to avoid the 2 concerns above? I will do anyway what you will recommend. |
To avoid conflict and regression the maximum we can, and keep code analysis of past changes easier as possible, changes on old version must be lower as possible to fix the bug for the end user. |
htdocs/product/stock/stockatdate.php
Outdated
@@ -245,12 +245,12 @@ | |||
|
|||
$sql = 'SELECT p.rowid, p.ref, p.label, p.description, p.price, p.pmp,'; | |||
$sql .= ' p.price_ttc, p.price_base_type, p.fk_product_type, p.desiredstock, p.seuil_stock_alerte,'; | |||
$sql .= ' p.tms as datem, p.duration, p.tobuy, p.stock, '; | |||
$sql .= ' p.tms as datem, p.duration, p.tobuy, p.stock, p.pmp, '; |
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.
Field p.pmp seems already added 2 lines before...
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.
Hi @eldy
I will review the code.
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.
Hi @eldy
Sorry for having been so late.
In version 14.0, I just reviewed the latest code and there is no p.pmp in line 246 (or 245 to be exact).
I will anyway make the same change as seen in v17 or even v19 if you want.
I will also submit a new PR on develop to change the estimatedvalue to currentvalue in the SQL request to clear the code.
Let me know your decision.
In stockatdate page, the estimatedvalue is wrong. Backport of V17 solution as explained in Dolibarr#23782
As commented in Dolibarr#23782 this update is to clean and clarify the code for future evolution of stocktodate. In fact the calculated value in the SQL request is the current stock value and not the estimated stock value at date value. Therefore I changed in SQL request the estimatedvalue to currentvalue and reused as a variable for a better understanding and for future use (other PR) as it will be interesting to get a comparison between stockatdate value to currentstock value. Also some others changes are required to improve the page speed.
As commented in #23782 this update is to clean and clarify the code for future evolution of stocktodate. In fact the calculated value in the SQL request is the current stock value and not the estimated stock value at date value. Therefore I changed in SQL request the estimatedvalue to currentvalue and reused as a variable for a better understanding and for future use (other PR) as it will be interesting to get a comparison between stockatdate value to currentstock value. Also some others changes are required to improve the page speed.
In stockatdate page, the estimatedvalue is wrong. Backport of V17 solution as explained in #23782
FIX # Bug Estimated Stock at date value
In stockatdate page, the estimatedvalue is wrong. In fact the calculated value in the SQL request is the current stock value and not the stock at date value.
P.S : I changed in code estimatedvalue to currentvalue and kept it for future use (other PR) as it is in interesting to get a comparison between stockatdate value to currentstock value.