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 Stock cover report stock out calculation #7844

Merged
merged 2 commits into from Dec 18, 2017

Conversation

Projects
None yet
6 participants
@marksull

marksull commented Apr 30, 2017

When running the Stock Cover Report from the Back Office > Stock > Stock Cover, the data in the field "Quantity Sold" is displaying either no value ("--") or an incorrect value.

In AdminStockCoverController.php in the getList method the quantity sold is calculated TWICE, in TWO different ways, resulting in two different results.

The first time quantity sold is calculated is getList in the following call:
$coverage = StockManagerFactory::getManager()->getProductCoverage.

StockManagerFactory::getManager()->getProductCoverage is used to determine the amount of stock sold over the coverage period specified then uses this value to calculate the amount of coverage required based on the amount of physical stock available.

The second time quantity sold is calculated in getList in the following call:
$qty_sold = $this->getQuantitySold($item['id_product'], $item['id'], $this->getCurrentCoveragePeriod());

Unfortunately this method performs a different query compared to that in StockManagerFactory::getManager()->getProductCoverage and yields different results to the value used in the first coverage calculation.

Based on my analysis the results reported by getQuantitySold can be significantly different (reporting no results), close or accurate depending on a variety of situations.

This fix creates a new method in StockManager with the query extracted from getProductCoverage to calculate the Product Out over the coverage period so it can be reused by AdminStockCoverController and hence return a consistent result.

Questions Answers
Branch? 1.6.1.x
Description? Done
Type? bug fix
Category? BO
BC breaks? No
Deprecations? No
Fixed ticket? http://forge.prestashop.com/browse/PSCSX-6674
How to test? Run the Stock Coverage Report

Important guidelines


This change is Reviewable

@prestonBot

This comment has been minimized.

Show comment
Hide comment
@prestonBot

prestonBot Apr 30, 2017

Collaborator

Hello marksull!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

Collaborator

prestonBot commented Apr 30, 2017

Hello marksull!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@xBorderie

This comment has been minimized.

Show comment
Hide comment
@xBorderie

xBorderie May 2, 2017

Contributor

Thanks for this one too :) Expect a review soon!

Contributor

xBorderie commented May 2, 2017

Thanks for this one too :) Expect a review soon!

@eternoendless eternoendless added the Bug label Jul 3, 2017

@eternoendless

Some small cosmetic changes are needed, but code looks good for me

Show outdated Hide outdated classes/stock/StockManagerInterface.php
Show outdated Hide outdated classes/stock/StockManagerInterface.php
* Here, $coverage is a number of days
* @return int number of products sold over the coverage period
*/
public function getProductOutForCoverage($id_product, $id_product_attribute, $coverage, $id_warehouse = null) {

This comment has been minimized.

@eternoendless

eternoendless Sep 1, 2017

Member

Please add PHPDoc for the parameters, or simply use @inheritdoc

@eternoendless

eternoendless Sep 1, 2017

Member

Please add PHPDoc for the parameters, or simply use @inheritdoc

This comment has been minimized.

@marksull

marksull Sep 8, 2017

Don't understand what you are asking for here..feel free to commit the change

@marksull

marksull Sep 8, 2017

Don't understand what you are asking for here..feel free to commit the change

Show outdated Hide outdated classes/stock/StockManager.php

@eternoendless eternoendless added this to the 1.6.1.18 milestone Sep 1, 2017

marksull added some commits Apr 30, 2017

BO: Fix Stock cover report stock out calculation
When running the Stock Cover Report from the Back Office > Stock > Stock Cover, the data in the field "Quantity Sold" is displaying either no value ("--") or an incorrect value.

In AdminStockCoverController.php in the getList method the quantity sold is calculated TWICE, in TWO different ways, resulting in two different results.

The first time quantity sold is calculated is getList in the following call:
$coverage = StockManagerFactory::getManager()->getProductCoverage.

StockManagerFactory::getManager()->getProductCoverage is used to determine the amount of stock sold over the coverage period specified then uses this value to calculate the amount of coverage required based on the amount of physical stock available.

The second time quantity sold is calculated in getList in the following call:
$qty_sold = $this->getQuantitySold($item['id_product'], $item['id'], $this->getCurrentCoveragePeriod());

Unfortunately this method performs a different query compared to that in StockManagerFactory::getManager()->getProductCoverage and yields different results to the value used in the first coverage calculation.

Based on my analysis the results reported by getQuantitySold can be significantly different (reporting no results), close or accurate depending on a variety of situations.

This fix creates a new method in StockManager with the query extracted from getProductCoverage to calculate the Product Out over the coverage period so it can be reused by AdminStockCoverController and hence return a consistent result.
@codacy-bot

This comment has been minimized.

Show comment
Hide comment
@codacy-bot

codacy-bot Sep 22, 2017

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- controllers/admin/AdminStockCoverController.php  1
- classes/stock/StockManager.php  1
         

See the complete overview on Codacy

codacy-bot commented Sep 22, 2017

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- controllers/admin/AdminStockCoverController.php  1
- classes/stock/StockManager.php  1
         

See the complete overview on Codacy

@PrestaShop PrestaShop deleted a comment from codacy-bot Sep 22, 2017

@marionf marionf added QA ✔️ and removed waiting for QA labels Oct 4, 2017

@eternoendless eternoendless changed the title from BO: Fix Stock cover report stock out calculation to Fix Stock cover report stock out calculation Dec 18, 2017

@eternoendless

This comment has been minimized.

Show comment
Hide comment
@eternoendless
Member

eternoendless commented Dec 18, 2017

Thank you @marksull

@eternoendless eternoendless merged commit 70b5ad3 into PrestaShop:1.6.1.x Dec 18, 2017

2 checks passed

codacy/pr Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment