[16.0][ADD] stock_valuation_specific_identification #2139

matt454357 · 2024-08-14T20:28:33Z

New module to add Specific Identification Inventory Valuation Method.

yostashiro · 2024-08-15T01:08:51Z

I think this module shares a similar objective with OCA/stock-logistics-workflow#1527.

One major difference, I understand, is that this module creates an SVL record per serial and the other one (stock_valuation_fifo_lot) does not (multiple lots/serials can be linked to an SVL record). Do you see a conceptual issue with the approach of the other module?

matt454357 · 2024-08-15T21:11:11Z

Hi @yostashiro,

Thanks for pointing out stock_valuation_fifo_lot. I had not noticed it.

I like the approach taken in stock_valuation_fifo_lot, but it doesn't correctly handle revaluations.

I believe stock_valuation_specific_identification better describes the module. There are similar 3rd party modules with similar descriptions (https://apps.odoo.com/apps/modules/browse?search=specific+identification). However, if stock_valuation_fifo_lot is the preferred name, I can move the changes to that module.

AungKoKoLin1997

Partial code review.

AungKoKoLin1997 · 2024-08-19T10:05:26Z

stock_valuation_specific_identification/models/stock_lot.py

+    _inherit = "stock.lot"
+
+    specific_ident_cost = fields.Boolean(
+        related="product_id.categ_id.property_specific_ident_cost",


Suggested change

related="product_id.categ_id.property_specific_ident_cost",

related="product_id.property_specific_ident_cost",

AungKoKoLin1997 · 2024-08-19T10:07:15Z

stock_valuation_specific_identification/models/product.py

+class ProductProduct(models.Model):
+    _inherit = "product.product"
+
+    specific_ident_cost = fields.Boolean(


I failed to understand to add this field in product.product model with readonly instead of just using from product.template

AungKoKoLin1997 · 2024-08-19T10:32:40Z

stock_valuation_specific_identification/models/stock_lot.py

+        """Prepare the values for a stock valuation layer created by a delivery.
+        This should only be called for specific identification valuation method.
+
+        :param quantity: the quantity to value, expressed in `product_id..uom_id`


Suggested change

:param quantity: the quantity to value, expressed in `product_id..uom_id`

:param quantity: the quantity to value, expressed in `product_id.uom_id`

AungKoKoLin1997 · 2024-08-19T10:40:08Z

stock_valuation_specific_identification/models/stock_lot.py

+        quantity = -1 * quantity  # negative for out valuation layers
+        sid_vals = self._run_out_spec_ident(abs(quantity), company)


Suggested change

quantity = -1 * quantity # negative for out valuation layers

sid_vals = self._run_out_spec_ident(abs(quantity), company)

sid_vals = self._run_out_spec_ident(quantity, company)

AungKoKoLin1997 · 2024-08-19T10:40:26Z

stock_valuation_specific_identification/models/stock_lot.py

+            "product_id": self.product_id.id,
+            "value": sid_vals.get("value"),
+            "unit_cost": sid_vals.get("unit_cost"),
+            "quantity": quantity,


Suggested change

"quantity": quantity,

"quantity": -1 * quantity,

AungKoKoLin1997 · 2024-08-19T15:03:17Z

stock_valuation_specific_identification/models/product.py

+                    for (
+                        candidate_line
+                    ) in candidate.stock_move_id.move_line_ids.filtered(
+                        lambda x: x.lot_id == lot and x.remaining_qty > 0.0
+                    ):
+                        qty_taken_on_candidate = min(
+                            qty_to_take_on_candidates, candidate_line.remaining_qty
+                        )


I don't understand this part. candidate_line is an instance of stock move line, isn't? I don't think stock move line record has remaining_qty field. Am I missing something?

AungKoKoLin1997 · 2024-08-19T15:07:54Z

stock_valuation_specific_identification/models/stock_lot.py

+
+        # Find back incoming stock valuation layers (called candidates here) to
+        # value `quantity`.
+        qty_to_take_on_candidates = quantity


Is this for the purpose of code readable?

Currently translated at 100.0% (185 of 185 strings) Translation: stock-logistics-warehouse-16.0/stock-logistics-warehouse-16.0-stock_reserve Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-warehouse-16-0/stock-logistics-warehouse-16-0-stock_reserve/it/

We were lacking the default location id so we could select the tray. It was also needed to avoid compting a pos when no tray is selected.

AungKoKoLin1997 · 2024-08-20T01:21:45Z

@matt454357 There are some changes and commits that are not related to the PR. It seems like something went wrong during your rebase.

AungKoKoLin1997 · 2024-08-20T06:44:58Z

@matt454357 I decided to go with stock_valuation_fifo_lot for two reasons:

The community is more likely to accept an existing module than a new one.
In my opinion, this module's approach is simpler and involves fewer complete overrides of Odoo's standard functions.

I will create a new PR with some adjustments since the author of the existing PR has not followed up on it. Would you be interested in contributing your ideas (e.g., the revaluation process) after I create the PR?

matt454357 · 2024-08-20T15:50:59Z

Hi @AungKoKoLin1997,

It has been a long time since I last did a PR. I shouldn't have merged the upstream changes made after starting my PR. I thought it might fix a test that was failing in stock_valuation_layer_accounting_date. It seems stock_valuation_layer_accounting_date has a poorly written test that sometimes fails, depending on the timezone and time of day.

Anyway, I am happy to contribute to the stock_valuation_fifo_lot module. I would like to add more than just the revaluation stuff. For example, drop the stock_no_negative requirement, and enforce no-negative for specific identification moves only.

Revaluation is accessed from the valuation tree view, after grouping by lot/serial. Therefore, we may want to change stock_valuation_fifo_lot to a single lot/serial per SVL record, rather than multiple per SVL record.

New module to add Specific Identification Inventory Valuation Method.

AungKoKoLin1997 · 2024-08-21T02:13:10Z

Anyway, I am happy to contribute to the stock_valuation_fifo_lot module. I would like to add more than just the revaluation stuff. For example, drop the stock_no_negative requirement, and enforce no-negative for specific identification moves only.

@matt454357 Thank you for your interest. I have created draft state PR OCA/stock-logistics-workflow#1676 and I have to add test cases and consider fixing issue that is proposed OCA/stock-logistics-workflow#1350 (comment). Let me ping you when it is ready.

matt454357 force-pushed the 16.0-add-stock_valuation_specific_identification branch from 7fcb654 to 7349ed2 Compare August 14, 2024 20:51

matt454357 force-pushed the 16.0-add-stock_valuation_specific_identification branch from 7349ed2 to 84b204c Compare August 15, 2024 15:03

AungKoKoLin1997 reviewed Aug 19, 2024

View reviewed changes

matt454357 force-pushed the 16.0-add-stock_valuation_specific_identification branch from 84b204c to b1d111d Compare August 19, 2024 17:21

mymage and others added 13 commits August 19, 2024 15:39

[12.0][ADD] stock_production_lot_quantity_tree

7800f9b

[UPD] Update stock_production_lot_quantity_tree.pot

94fc268

[MIG] stock_production_lot_quantity_tree: Migration to 13.0

7d91652

[UPD] Update stock_production_lot_quantity_tree.pot

f764474

[UPD] README.rst

1b1f9c5

[UPD] README.rst

259e7a3

[IMP] stock_production_lot_quantity_tree: pre-commit stuff

4d4dcc4

[MIG] stock_production_lot_quantity_tree: Migration to 16.0

4e0e212

[UPD] Update stock_production_lot_quantity_tree.pot

0c171ec

[BOT] post-merge updates

21c5d4b

[FIX] stock_vlm_mgmt: location general inventory

5658d59

We were lacking the default location id so we could select the tray. It was also needed to avoid compting a pos when no tray is selected.

[BOT] post-merge updates

cc3addc

matt454357 force-pushed the 16.0-add-stock_valuation_specific_identification branch 2 times, most recently from 912701c to 3f70122 Compare August 19, 2024 22:03

rousseldenis added the needs review label Aug 20, 2024

rousseldenis added this to the 16.0 milestone Aug 20, 2024

[16.0][ADD] stock_valuation_specific_identification

bd806b3

New module to add Specific Identification Inventory Valuation Method.

matt454357 force-pushed the 16.0-add-stock_valuation_specific_identification branch from 3f70122 to bd806b3 Compare August 20, 2024 16:27

AungKoKoLin1997 mentioned this pull request Aug 21, 2024

[16.0][MIG] stock_valuation_fifo_lot: Migration to 16.0 OCA/stock-logistics-workflow#1676

Open

AungKoKoLin1997 mentioned this pull request Sep 20, 2024

[16.0][FIX] stock_valuation_layer_accounting_date: prevent tests failed for some timezone #2159

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][ADD] stock_valuation_specific_identification #2139

[16.0][ADD] stock_valuation_specific_identification #2139

matt454357 commented Aug 14, 2024

yostashiro commented Aug 15, 2024

matt454357 commented Aug 15, 2024

AungKoKoLin1997 left a comment

AungKoKoLin1997 Aug 19, 2024

AungKoKoLin1997 Aug 19, 2024

AungKoKoLin1997 Aug 19, 2024

AungKoKoLin1997 Aug 19, 2024

AungKoKoLin1997 Aug 19, 2024

AungKoKoLin1997 Aug 19, 2024

AungKoKoLin1997 Aug 19, 2024

AungKoKoLin1997 commented Aug 20, 2024

AungKoKoLin1997 commented Aug 20, 2024

matt454357 commented Aug 20, 2024

AungKoKoLin1997 commented Aug 21, 2024

	related="product_id.categ_id.property_specific_ident_cost",
	related="product_id.property_specific_ident_cost",

	:param quantity: the quantity to value, expressed in `product_id..uom_id`
	:param quantity: the quantity to value, expressed in `product_id.uom_id`

		quantity = -1 * quantity # negative for out valuation layers
		sid_vals = self._run_out_spec_ident(abs(quantity), company)

	quantity = -1 * quantity # negative for out valuation layers
	sid_vals = self._run_out_spec_ident(abs(quantity), company)
	sid_vals = self._run_out_spec_ident(quantity, company)

[16.0][ADD] stock_valuation_specific_identification #2139

Are you sure you want to change the base?

[16.0][ADD] stock_valuation_specific_identification #2139

Conversation

matt454357 commented Aug 14, 2024

yostashiro commented Aug 15, 2024

matt454357 commented Aug 15, 2024

AungKoKoLin1997 left a comment

Choose a reason for hiding this comment

AungKoKoLin1997 Aug 19, 2024

Choose a reason for hiding this comment

AungKoKoLin1997 Aug 19, 2024

Choose a reason for hiding this comment

AungKoKoLin1997 Aug 19, 2024

Choose a reason for hiding this comment

AungKoKoLin1997 Aug 19, 2024

Choose a reason for hiding this comment

AungKoKoLin1997 Aug 19, 2024

Choose a reason for hiding this comment

AungKoKoLin1997 Aug 19, 2024

Choose a reason for hiding this comment

AungKoKoLin1997 Aug 19, 2024

Choose a reason for hiding this comment

AungKoKoLin1997 commented Aug 20, 2024

AungKoKoLin1997 commented Aug 20, 2024

matt454357 commented Aug 20, 2024

AungKoKoLin1997 commented Aug 21, 2024