Skip to content
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 analytics report shows incorrect data there's no stock movement in a period #30945

Merged
merged 4 commits into from
May 12, 2022

Conversation

ankush
Copy link
Member

@ankush ankush commented May 10, 2022

This PR fixes 3 bugs and some minor UX improvements.

  1. Stock analytics report prepares data by binning SLEs into specific time periods. This means when a particular period doesn't have any SLEs it wont be processed and will have 0 qty/value.

  2. Similarly when all periods are processed but item is inactive for some time the current values are shown as 0 and not carried forward.

  3. When batched item Stock Reco is encountered, the qty computation is incorrect. This is because batched SR maintains actual qty. This edge case is just 💩

Tested:

  • Compared all qty computation with stock balance report (on local database and a production site)
  • Added unittests

Before

Screenshot 2022-05-10 at 3 46 47 PM

After

Screenshot 2022-05-10 at 3 44 48 PM

@ankush ankush changed the title fix: stock analytics fix: stock analytics report shows incorrect data there's no stock movement in a bucket May 10, 2022
@ankush ankush force-pushed the stock_analytics_fix branch from 9505faa to 604002b Compare May 10, 2022 08:38
@codecov
Copy link

codecov bot commented May 10, 2022

Codecov Report

Merging #30945 (198b91f) into develop (d258413) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #30945      +/-   ##
===========================================
+ Coverage    62.74%   62.78%   +0.03%     
===========================================
  Files          983      983              
  Lines        67103    67117      +14     
===========================================
+ Hits         42102    42137      +35     
+ Misses       25001    24980      -21     
Impacted Files Coverage Δ
...xt/stock/report/stock_analytics/stock_analytics.py 93.91% <100.00%> (+13.71%) ⬆️
.../report/delayed_item_report/delayed_item_report.py 60.78% <0.00%> (-35.30%) ⬇️
...pnext/stock/doctype/delivery_note/delivery_note.py 66.21% <0.00%> (-2.71%) ⬇️
.../report/bom_operations_time/bom_operations_time.py 89.13% <0.00%> (-2.18%) ⬇️
erpnext/crm/doctype/prospect/prospect.py 55.22% <0.00%> (-1.50%) ⬇️
erpnext/stock/reorder_item.py 76.06% <0.00%> (-0.86%) ⬇️
erpnext/selling/doctype/sales_order/sales_order.py 79.74% <0.00%> (-0.72%) ⬇️
erpnext/stock/doctype/batch/batch.py 82.80% <0.00%> (-0.64%) ⬇️
erpnext/hr/doctype/expense_claim/expense_claim.py 82.41% <0.00%> (-0.51%) ⬇️
...ext/payroll/doctype/payroll_entry/payroll_entry.py 72.84% <0.00%> (-0.44%) ⬇️
... and 20 more

@ankush ankush force-pushed the stock_analytics_fix branch 2 times, most recently from 332f38a to 030f087 Compare May 10, 2022 09:08
ankush added 4 commits May 10, 2022 15:38
Also remove `total`, total of total is a meaningless value.
Showing data in future doesn't make sense. Only carry-forward till last
bucket that contains today's day.
@ankush ankush force-pushed the stock_analytics_fix branch from 3322165 to 198b91f Compare May 10, 2022 10:08
@ankush ankush marked this pull request as ready for review May 10, 2022 10:08
@ankush ankush changed the title fix: stock analytics report shows incorrect data there's no stock movement in a bucket fix: stock analytics report shows incorrect data there's no stock movement in a period May 10, 2022
@marination marination merged commit 5b8c743 into frappe:develop May 12, 2022
ankush added a commit that referenced this pull request May 12, 2022
…ement in a period (backport #30945) (#30980)

* test: basic test for stock analytics report

(cherry picked from commit d81422f)

* fix: consider previous balance is missing

Also remove `total`, total of total is a meaningless value.

(cherry picked from commit 6ab0046)

* fix: batch_no doesn't maintain qty_after_transaction

(cherry picked from commit 287b255)

* fix: only carry-forward balances till today's period

Showing data in future doesn't make sense. Only carry-forward till last
bucket that contains today's day.

(cherry picked from commit 198b91f)

Co-authored-by: Ankush Menat <me@ankush.dev>
@ankush ankush deleted the stock_analytics_fix branch May 12, 2022 09:01
frappe-pr-bot pushed a commit that referenced this pull request May 17, 2022
# [13.30.0](v13.29.2...v13.30.0) (2022-05-17)

### Bug Fixes

* `set_missing_values` in SE and re-use the same on all SE mappings ([fe52c1f](fe52c1f))
* Add validation for SEZ and Export invoices without payment of taxes ([cb8453d](cb8453d))
* allow to use formatting for the field to_discuss in opportunity ([e126d4e](e126d4e))
* Block 0 Qty via Update Items to be consistent with form validation ([5647875](5647875))
* Calculate totals even though pricing rule is not applied on mapped doc ([678a01d](678a01d))
* **charts:** Pass fieldtype for chart data in selling reports ([917e7c3](917e7c3))
* conflicts ([87fd933](87fd933))
* conflicts ([fb62bbf](fb62bbf))
* disable pricing rules for internal transfers (backport [#31034](#31034)) ([#31036](#31036)) ([d5eb9fb](d5eb9fb))
* discount ledger entry in case of multicurrency invoice ([#31047](#31047)) ([c3417e4](c3417e4))
* dont fail repost for recoverable errors (backport [#30979](#30979)) ([#31023](#31023)) ([a019cb6](a019cb6))
* **Employee Advance:** Return/Deduction from Salary button visibility (backport [#31011](#31011)) ([#31012](#31012)) ([5b1d85e](5b1d85e))
* Failing accounting dimension patch ([b14a7b8](b14a7b8))
* german translations for Employee ([b9bda04](b9bda04))
* gl entry validation for miniscule loan penalty ([e958ef2](e958ef2))
* hide template items from sales/purchase order ([8b99f43](8b99f43))
* IN time not captured in Attendance through Employee Checkin (backport [#31029](#31029)) ([#31031](#31031)) ([477bbcc](477bbcc))
* Item rate reset on changing posting date ([#30990](#30990)) ([8ef649f](8ef649f))
* Just add one rate in GST HSN Code ([ed76687](ed76687))
* Merge Conflicts ([3abf264](3abf264))
* Multiple fixes in GSTR-1 report ([f2cbb70](f2cbb70))
* **patch:** avoid checking for return field if it doesnt exits (backport [#30995](#30995)) ([#30997](#30997)) ([a94b5c0](a94b5c0))
* per_billed for return DN (backport [#30868](#30868)) ([#30971](#30971)) ([97ea1f5](97ea1f5))
* precision loss when transferring  (backport [#30834](#30834)) ([#31032](#31032)) ([fc80a50](fc80a50))
* precision of total penalty paid ([ad21853](ad21853))
* precision of total penalty paid ([5c45737](5c45737))
* prevent bypassing forced valuation rate (backport [#30987](#30987)) ([#31020](#31020)) ([706c19d](706c19d))
* pro rata calculation for monthly depreciation ([#30989](#30989)) ([408d952](408d952))
* remove item attribute limit from variant selector (backport [#31026](#31026)) ([#31028](#31028)) ([1f016e9](1f016e9))
* Set actual qty and basic rate in SE on warehouse triggers (`get_warehouse_details`) ([30b0aee](30b0aee))
* stock analytics report shows incorrect data there's no stock movement in a period (backport [#30945](#30945)) ([#30980](#30980)) ([295ffb3](295ffb3))
* translation for status filter ([e5f8231](e5f8231))
* **translations:** Update ru translations ([#30992](#30992)) ([f797005](f797005))
* TypeError in add_indicator_for_multicompany (backport [#31042](#31042)) ([#31048](#31048)) ([e24bb1d](e24bb1d))
* unlink Attendance from Employee Checkins on cancellation (backport [#31045](#31045)) ([#31049](#31049)) ([e03fe97](e03fe97))
* UOM in HSN-wise summary of outward supply ([cd7d5cd](cd7d5cd))
* user can select disabled accounts in taxes table ([047c879](047c879))
* validate disabled accounts before posting ledger entries ([515e49b](515e49b))
* validate on hold purchase invoices in payment entry ([9fbd170](9fbd170))

### Features

* add Employee Status filter in leave balance reports ([716b525](716b525))
* add Link to Opportunity ([#30614](#30614)) ([bc23bc7](bc23bc7))
* request_for_quotation ([db4e264](db4e264))
* request_for_quotation - refactor ([b6a3e69](b6a3e69))
* select multiple values for accounting dimension (backport [#31015](#31015)) ([#31041](#31041)) ([9c21eb5](9c21eb5))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants