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_quantity_history_location: qty by location #86

Conversation

chienandalu
Copy link
Member

When a qty at date is asked the location context could be overriden if
the company_owned one was inserted after we removed it. We ensure to
remove it to always obtain the right values.

As a side effect, stock_account valuation is now correctly computed when
AVCO or Standard methods are implied. So should be with FIFO manual (as
it computes the quantities the same way) but doubtful in FIFO real time.

So maybe stock_account_quantity_history_location is of no much use (warning advising that the computation is not true, wich isn't accurate and show 0 qtys values to dectect errors)

cc @Tecnativa TT21183

@chienandalu
Copy link
Member Author

@rafaelbn please test

@chienandalu chienandalu force-pushed the 12.0-fix-stock_account_quantity_history_location branch from 3c0831f to c33ca54 Compare December 30, 2019 13:12
@pedrobaeza pedrobaeza added this to the 12.0 milestone Dec 30, 2019
@chienandalu chienandalu force-pushed the 12.0-fix-stock_account_quantity_history_location branch from c33ca54 to ae5e934 Compare December 31, 2019 08:31
Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally tested 👍 😄

@rousseldenis
Copy link
Sponsor Contributor

@chienandalu Do you plan to add a test for that fix ?

@chienandalu
Copy link
Member Author

@rousseldenis Yes, I'd like to

When a qty at date is asked the location context could be overriden if
the company_owned one was inserted after we removed it. We ensure to
remove it to always obtain the right values.

As a side effect, stock_account valuation is now correctly computed when
AVCO or Standard methods are implied. So should be with FIFO manual (as
it computes the quantities the same way) but doubtful in FIFO real time.
@chienandalu chienandalu force-pushed the 12.0-fix-stock_account_quantity_history_location branch from ae5e934 to 6f856e7 Compare April 15, 2020 09:22
@chienandalu
Copy link
Member Author

@rousseldenis Added tests

Copy link
Sponsor Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review

@rousseldenis
Copy link
Sponsor Contributor

@chienandalu Is this ready ?

@chienandalu
Copy link
Member Author

Yeah :)

@rousseldenis
Copy link
Sponsor Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 12.0-ocabot-merge-pr-86-by-rousseldenis-bump-minor, awaiting test results.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot OCA-git-bot merged commit b37f433 into OCA:12.0 Apr 15, 2020
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 3af4963. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants