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

[12.0][FIX] stock_account: Correct order in candidates_receipt search for FIFO valuation #1230

Open
wants to merge 1 commit into
base: 12.0
Choose a base branch
from

Conversation

kanda999
Copy link

@kanda999 kanda999 commented May 15, 2024

Description of issue/feature this PR addresses:
Incorrectly orders the search results by date (a datetime field) in ascending order and then by id in descending order.
This results in the selection of the oldest record instead of the last one.

Current behavior before PR:
The search query extracts the oldest stock.move record instead of the last one when updating the value and remaining quantity in FIFO cost method during the decrease of qty_done in an outgoing move.

Desired behavior after PR is merged:
The search query correctly extracts the last stock.move record by ordering the results first by date in descending order.

@qrtl QT4562


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

Copy link
Sponsor Member

@yostashiro yostashiro left a comment

Choose a reason for hiding this comment

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

@kanda999 Can you please add a short description in the commit title?

…IFO valuation

When decreasing `qty_done` in an outgoing stock move,
the search query for `candidates_receipt` was ordering by `date` in ascending order,
resulting in the selection of the oldest record instead of the most recent one.

This commit changes the order to `date desc, id desc`
to ensure the most recent record is selected,
as intended by the comments in the write method.
Copy link
Sponsor Member

@yostashiro yostashiro 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.

@pedrobaeza
Copy link
Member

I think this behavior change in such an old version is too much to be accepted, unless this is something already changed in later versions.

@kanda999
Copy link
Author

I'm not sure the reasoning behind not accepting the change because it is an older version.

I have made the code modification based on the comment written in the method: "If the move is outgoing and the user decreases qty_done, we either increase the last receipt candidate if one is found or we decrease the value with the last fifo price."

In the future, users of OCB 12.0 might encounter this strange behavior. In fact, one of our customers has already experienced this issue.

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

Successfully merging this pull request may close these issues.

5 participants