Skip to content

Conversation

Tisho99
Copy link

@Tisho99 Tisho99 commented Sep 16, 2025

This PR fixes some errors of the module:

  • In the price unit calculation, use the default value if the move is a return. Odoo has an special case for that to calculate it correctly (with this module installed, the result in that case is wrong)
  • In the price unit calculation, take into account the related landed costs of the candidate SVL
  • Add a new module, stock_valuation_fifo_lot_mrp_landed_cost, to cover the needed mrp_landed_costs dependency. The mrp_production_ids field, used in this module, is created in the mrp_landed_costs module

Unit tests have also been added to cover the case of returning a outgoing shipping

The README has also been improved

T-8760

@OCA-git-bot
Copy link
Contributor

Hi @newtratip,
some modules you are maintaining are being modified, check this out!

@Tisho99 Tisho99 changed the title [15.0][FIX] stock_valuation_fifo_lot: _get_price_unit() function [15.0][FIX] stock_valuation_fifo_lot: fix _get_price_unit() function Sep 16, 2025
@Tisho99 Tisho99 marked this pull request as draft September 16, 2025 13:33
@Tisho99 Tisho99 force-pushed the 15.0-fix-stock_valuation_fifo_lot branch from e8eaa47 to 093087c Compare October 1, 2025 14:37
@Tisho99 Tisho99 marked this pull request as ready for review October 1, 2025 14:45
@Tisho99 Tisho99 marked this pull request as draft October 2, 2025 12:06
@Tisho99 Tisho99 force-pushed the 15.0-fix-stock_valuation_fifo_lot branch 3 times, most recently from c278787 to a576c1f Compare October 2, 2025 14:03
@Tisho99 Tisho99 marked this pull request as ready for review October 2, 2025 14:05
Copy link

@luis-ron luis-ron left a comment

Choose a reason for hiding this comment

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

I have carried out various tests, which I list below:

Test characteristics:
Product categories have been configured using the FIFO valuation method.
Various products have been created and configured with the following characteristics:

  • Storable products.

  • With a product category using the FIFO valuation method.

  • Products with batch traceability.

  • The landed costs will be divided equally among all purchase units.

Test flow:

  1. Various purchases of these products have been made, covering various aspects:
  • Purchase of PRODUCT A with 1 batch. Price per unit: €2
  • Purchase of PRODUCT B with more than 1 batch (2 or more).
  • Various purchases of PRODUCT C, with various batches.

In each of these purchases, landed costs have been added, thus creating a more detailed valuation and ensuring the correct behavior of the system. To simplify matters, a landed cost of €10 has been applied to each of the purchases.

The result of the purchase valuation is as follows:

  • Purchase 1: PRODUCT A | 10 units | €2 per unit | Landed cost €10 | Lot 1 -> Total cost: €3

  • Purchase 2: PRODUCT B | 10 units | €2 per unit | Landed cost €10 | Lot 1 | Lot 2 -> Total cost: €3

  • Purchase 3: Product C | 10 units | €2 per unit | Landed cost €10 | Lot 1 -> Total cost: €3

  • Purchase 4: Product C | 10 units | €3 per unit | Landed cost €10 | Lot 2 -> Total cost €4.

  • Purchase 5: Product C | 10 units | €4 per unit | Landed cost €10 | Lot 3 -> Total cost €5.

I then made a sale of each of the products purchased.

The cost shown on the sales order line is correct.

I then carried out return tests, validating the delivery note for this type of operation.

When validating the delivery note, the cost of the sales order line is correct, maintaining a relationship with the landed cost indicated in the original purchase.

This allows for a correct valuation of the inventory, even when returning products.

It has also been tested with purchases that have more than one landed cost asociated, maintaining correct traceability.

It has been tested with the sale of various products with various landed costs.

The result is correct.

"stock_account",
"stock_no_negative",
"stock_landed_costs",
"mrp_landed_costs",
Copy link
Contributor

Choose a reason for hiding this comment

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

why mandatory install mrp?

Copy link
Author

Choose a reason for hiding this comment

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

The mrp_production_ids field, used in this module, is created in the mrp_landed_costs module.

lot_ids = self.mrp_production_ids.mapped("lot_producing_id")

https://github.com/odoo/odoo/blob/15.0/addons/mrp_landed_costs/models/stock_landed_cost.py#L13

Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely correct, but that comes from the "mrp_landed_costs" module. The dependency on MRP isn't necessary for this module. Several functionalities are being combined in this module that would have to be separated into another module or implemented with a soft dependency.

Copy link
Member

Choose a reason for hiding this comment

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

Just FYI, we have split the module to stock_valuation_fifo_lot and stock_valuation_fifo_lot_mrp_landed_cost in the 16.0 migration PR.

- Do not override the return if the move is a return, odoo calculates it correctly (this module does not)
- Calculate the resulting price unit with the related landed costs of the candidate SVL
@Tisho99 Tisho99 force-pushed the 15.0-fix-stock_valuation_fifo_lot branch from a576c1f to b21c5f4 Compare October 7, 2025 11:58
@Tisho99 Tisho99 force-pushed the 15.0-fix-stock_valuation_fifo_lot branch from b21c5f4 to badcc2c Compare October 7, 2025 12:03
@Tisho99 Tisho99 force-pushed the 15.0-fix-stock_valuation_fifo_lot branch from 3c424ad to 0ea4554 Compare October 7, 2025 14:20
Copy link

@luis-ron luis-ron left a comment

Choose a reason for hiding this comment

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

2nd Functional review: LGTM 👍🏻

Copy link
Contributor

@ValentinVinagre ValentinVinagre left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@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). 🤖

@Tisho99 Tisho99 changed the title [15.0][FIX] stock_valuation_fifo_lot: fix _get_price_unit() function [15.0][FIX] stock_valuation_fifo_lot: fix _get_price_unit() function and mrp dependence Oct 15, 2025
@Tisho99
Copy link
Author

Tisho99 commented Oct 15, 2025

Hello @newtratip

This PR includes some fixes for the module you created. It’s ready to merge.
Would you be interested in reviewing it?

Thanks!

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.

6 participants