Skip to content

[7121][ADD] stock_svl_removal_date: store the removal date of the lots on stock valuation layers - #260

Closed
nobuQuartile wants to merge 2 commits into
18.0from
7121-add-stock_svl_removal_date
Closed

nobuQuartile wants to merge 2 commits into
18.0from
7121-add-stock_svl_removal_date

Conversation

@nobuQuartile

Copy link
Copy Markdown

QT7121

Adds a Removal Date field to stock valuation layers, taken from the lots/serials of the related stock move.

The standard Lot/Serial Number field of a valuation layer is only filled for products with lot valuation enabled, so the lots are read from the stock move (stock_move_id.lot_ids) instead, which works regardless of that setting. When the move holds several lots, the earliest removal date is kept.

The field is stored and indexed, so that the layers can be filtered, sorted and grouped by removal date from Inventory > Reporting > Valuation. Existing layers are filled in SQL by a pre-install hook rather than by the ORM, which keeps the installation fast even on databases holding a large number of valuation layers.

Notes for review:

  • The realignment on stock.lot.write() compares removal_date before and after super() rather than looking for the key in vals. removal_date is a stored computed field of product_expiry, so it also changes as a side effect of writing expiration_date or product_id; such a recomputation is flushed through _write_multi() and never reaches the write() override. test_expiration_date_change_realigns_layers is the regression guard for this.
  • sudo() in _update_layer_removal_date() is required because stock.valuation.layer is only writable by stock managers while lot dates are maintained by any stock user.

…tock valuation layers

The valuation layers cannot be filtered or grouped by removal date, because
the standard Lot/Serial Number field of a layer is only filled for products
with lot valuation enabled. Store the removal date on the layer instead,
taken from the lots of the stock move so that it works regardless of that
setting, and keep the earliest one when the move holds several lots.

Existing layers are filled in SQL by a pre-install hook rather than by the
ORM, so that installing the module stays fast on databases holding a large
number of valuation layers.

task-7121
@nobuQuartile
nobuQuartile marked this pull request as ready for review August 28, 2026 06:41
@nobuQuartile

Copy link
Copy Markdown
Author

Ready to review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b83eb702be

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +42 to +44
self.env["stock.move.line"]
.search([("lot_id", "in", self.ids), ("quantity", "!=", 0)])
.move_id

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Sudo the move-line lookup for shared lots

When a company-neutral lot is used by multiple companies and a user updates it with only one company enabled, this search remains subject to stock.move.line company record rules because it does not inherit the sudo() applied to the separate layers recordset. Consequently, moves from the other companies are omitted and their valuation layers without a direct lot_id retain the old removal date, despite this method explicitly intending to realign cross-company layers. Perform this lookup with elevated access as well.

Useful? React with 👍 / 👎.

@nobuQuartile

Copy link
Copy Markdown
Author

Add ja.po

Translation notes

Translations are referenced from existing Odoo translations where available (odoo/product_expiry, odoo/stock_account).

The following terms have no reference and are translated independently:

msgid msgstr
Removal date of the lot/serial of the stock move. When the move holds several lots, the earliest removal date is kept. 在庫移動のロット/シリアルの払出期限。在庫移動が複数のロットを含む場合は、最も早い払出期限が採用されます。

Terminology follows product_expiry (removal date → 払出期限, lot/serial → ロット/シリアル, stock move → 在庫移動).

Comment on lines +10 to +15
removal_date = fields.Datetime(
readonly=True,
index=True,
help="Removal date of the lot/serial of the stock move. When the move "
"holds several lots, the earliest removal date is kept.",
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nobuQuartile I think current implementation is bloated. I believe below code can cover the functionality of current PR.

Suggested change
removal_date = fields.Datetime(
readonly=True,
index=True,
help="Removal date of the lot/serial of the stock move. When the move "
"holds several lots, the earliest removal date is kept.",
)
removal_date = fields.Datetime(
compute="_compute_removal_date", store=True, index=True, compute_sudo=True,
help="Removal date of the lot/serial of the stock move. When the move "
"holds several lots, the earliest removal date is kept.",
)
@api.depends("lot_id.removal_date", "stock_move_id.move_line_ids.lot_id.removal_date")
def _compute_removal_date(self):
for layer in self:
# Layers of a lot valuated product carry the lot themselves; the
# others take the earliest removal date of their move's lots.
lots = layer.lot_id or layer.stock_move_id.lot_ids
layer.removal_date = min(filter(None, lots.mapped("removal_date")), default=False)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's good!
Maybe we should create this as an OCA Module, shouldn't we?
If you can, could you create the OCA PR?

@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

@nobuQuartile Please close this PR. I created in hls-oca.
qrtl/hls-oca#170

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants