stock-logistics-workflow icon indicating copy to clipboard operation
stock-logistics-workflow copied to clipboard

[16.0][MIG] stock_valuation_fifo_lot: Migration to 16.0

Open AungKoKoLin1997 opened this issue 1 year ago • 18 comments

Superseding of https://github.com/OCA/stock-logistics-workflow/pull/1527

In this PR, I address the following points:

  • Standard migration
  • Improved code by using the Odoo standard function _get_fifo_candidates() and adjustments to _get_price_unit()
  • Fixed the issue proposed in https://github.com/OCA/stock-logistics-workflow/pull/1350#issuecomment-1840407086
  • Avoid completely overriding _run_fifo() and use the standard behavior.
  • Update the product's standard price to the first available lot price.
  • Improve _get_price_unit() to retrieve the value from the most recent incoming move line for the lot for the No PO (e.g. customer returns) stock moves.
  • Add quantity and value-related fields in stock_move_line to cover all cases including multiple lots exist in an SVL record, and it is unclear which lot's remaining quantity is being delivered(Eg. Receive serials 001, 002 and 003 for an incoming move. Deliver 002, return 002 and deliver 002 again.)
  • Add force_fifo_lot_id in stock_move_line to handle cases where a delivery needs to be made for an existing lot created before the installation of this module (e.g., lots 001 and 002 were received, lot 002 was delivered before the module's installation, and lot 001 is delivered after the module is installed).
  • Add a post_init_hook to update the lot_ids in SVL for existing records and to update the field values in existing stock_move_line records.

Depends on https://github.com/odoo/odoo/pull/180245 @qrtl QT4650

AungKoKoLin1997 avatar Aug 20 '24 11:08 AungKoKoLin1997

Test failed from stock_picking_line_sequence is fixed at https://github.com/OCA/stock-logistics-workflow/pull/1557.

AungKoKoLin1997 avatar Aug 21 '24 05:08 AungKoKoLin1997

@matt454357 Can you please review this PR and bring your ideas from your module https://github.com/OCA/stock-logistics-warehouse/pull/2139 as we talked?

AungKoKoLin1997 avatar Aug 21 '24 08:08 AungKoKoLin1997

I started a review, and will post some suggestions.

matt454357 avatar Aug 21 '24 13:08 matt454357

@matt454357 What I am thinking is that I will handle adding the option for a single lot/serial per SVL record or multiple lots/serials per SVL record in this PR (since some users may not need a single lot/serial per SVL).

After you review and the PR is merged, you can create a new PR to follow up on your idea. Or would you prefer to include your idea as a new commit in this PR by creating a PR in our repository?

What do you think?

AungKoKoLin1997 avatar Aug 22 '24 01:08 AungKoKoLin1997

I will handle adding the option for a single lot/serial per SVL record or multiple lots/serials per SVL record in this PR (since some users may not need a single lot/serial per SVL).

@matt454357 I will not add this enhancement in the current PR because it is not related to migration, and there is no use case with the current design. So, could you please handle this when you work on revaluation?

AungKoKoLin1997 avatar Aug 22 '24 06:08 AungKoKoLin1997

I think it's a good idea to finish this migration PR, and then do a follow up PR. And, I'm happy to do the follow up PR. However, there a couple of problems with allowing multiple lots/serials per SVL.

Maybe this initial migration should prevent revaluation of tracked products?

Maybe also raise an error when _get_price_unit() is called with multiple lot_ids?

matt454357 avatar Aug 22 '24 18:08 matt454357

The believe PR https://github.com/OCA/stock-logistics-workflow/pull/1677 supports the idea of enforcing single lot/serial per SVL.

matt454357 avatar Aug 22 '24 18:08 matt454357

I think it's a good idea to finish this migration PR, and then do a follow up PR. And, I'm happy to do the follow up PR. However, there a couple of problems with allowing multiple lots/serials per SVL.

Thanks for the pointer and your interest in following up. What do you think of my idea of adding an option to choose a single or multiple lot/serial per SVL? As I mentioned, I don't want to make this adjustment in the migration PR because it would make the module bigger, and the community might be less interested in reviewing it. So, my idea is to get this PR merged quickly and then follow up with the changes.

AungKoKoLin1997 avatar Aug 23 '24 01:08 AungKoKoLin1997

Yes, keep this PR small.

Regarding the option to choose between multiple or single lot per SVL: I'm not sure why someone would want multiple, but it sounds like you have a use case in mind. So, I suppose it's fine, as long as we raise errors for the issues described above.

matt454357 avatar Aug 23 '24 04:08 matt454357

@TheerayutEncoder I have separated the module for passing lot_producing_id of mrp_production_ids to the SVL when adding the landed cost for these MOs, as you proposed in this commit. What do you think about it? Additionally, I am not entirely sure of the purpose of this, since the remaining_qty of the SVL for landed costs is 0.0, and I believe these SVLs will not be considered when _run_fifo() is called, if I understand correctly. Is this intended for user experience, to see the related lot_ids in the SVL? Could you please explain the initial purpose?

AungKoKoLin1997 avatar Aug 23 '24 09:08 AungKoKoLin1997

What do you think of my idea of adding an option to choose a single or multiple lot/serial per SVL?

@matt454357 Please let me handle this part and include it in this PR because we urgently need this module, and I don't know when this PR will be merged.

AungKoKoLin1997 avatar Aug 27 '24 10:08 AungKoKoLin1997

@AungKoKoLin1997 any contribution you make is very much appreciated, Do it the way you feel is best, and I will work with it.

matt454357 avatar Aug 27 '24 23:08 matt454357

/ocabot migration stock_valuation_fifo_lot

rousseldenis avatar Sep 20 '24 06:09 rousseldenis

We will also need https://github.com/odoo/odoo/pull/179447, in order to do revaluation (later).

matt454357 avatar Sep 23 '24 14:09 matt454357

@AungKoKoLin1997 @yostashiro Is this PR dead? 🦴

ValentinVinagre avatar Feb 12 '25 12:02 ValentinVinagre

@AungKoKoLin1997 @yostashiro Is this PR dead? 🦴

@ValentinVinagre It's alive and you are more than welcome to review it. 😁

yostashiro avatar Feb 17 '25 09:02 yostashiro

@AungKoKoLin1997 @yostashiro Is this PR dead? 🦴

@ValentinVinagre It's alive and you are more than welcome to review it. 😁

Perfect, I'll sign up to review it 👍🏻 . I thought it was at a standstill, so I wanted to know the status.

ValentinVinagre avatar Feb 17 '25 12:02 ValentinVinagre

I squashed the commits.

AungKoKoLin1997 avatar Mar 07 '25 08:03 AungKoKoLin1997