Skip to content

[5664][ADD] stock_request_analytic - #237

Draft
nobuQuartile wants to merge 1 commit into
16.0from
5664-add-stock_request_analytic
Draft

nobuQuartile wants to merge 1 commit into
16.0from
5664-add-stock_request_analytic

Conversation

@nobuQuartile

Copy link
Copy Markdown
Contributor

@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: ddf5bc3f77

ℹ️ 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 +10 to +12
stock_request_ids = fields.One2many(
comodel_name="stock.request",
inverse_name="analytic_distribution",

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 Use a relational inverse for stock requests

When Odoo initializes this module, analytic_distribution is a JSON field inherited from analytic.mixin, but a One2many inverse must be a Many2one field on the comodel. The registry therefore rejects this field definition and the addon cannot be installed; implement this as a computed/search-based relation or add a valid relational inverse.

Useful? React with 👍 / 👎.

Comment on lines +11 to +13
<field
name="analytic_account_id"
groups="analytic.group_analytic_accounting"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Edit the propagated analytic distribution on requests

When a user assigns an account on the standalone stock-request form, this field writes only the legacy analytic_account_id; nothing synchronizes it with analytic_distribution, while models/stock_rule.py propagates only stock_request.analytic_distribution. Consequently, confirming such a request creates stock moves without the analytic account selected in this form.

Useful? React with 👍 / 👎.

Comment on lines +23 to +26
default_analytic_account_id = fields.Many2one(
comodel_name="account.analytic.account",
string="Default Analytic Account",
help="Set this if you want to define a default analytic account on requests",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Apply the default account to request lines

When an order user selects this default, the field is never read by an onchange, create override, or child-field context anywhere in the addon, so existing and newly added stock requests retain an empty analytic_distribution. The visible option therefore does not provide the default behavior promised by its label and help text.

Useful? React with 👍 / 👎.

Comment on lines +36 to +40
for key in distribution.keys():
try:
analytic_account_ids.add(int(key))
except (TypeError, ValueError):
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Split composite analytic-distribution keys

For multi-plan analytic distributions, a valid key can contain multiple comma-separated account IDs such as "12,34". Passing that key to int() raises ValueError, which is silently caught here, so both accounts disappear from analytic_account_ids, analytic_count, and the order's analytic-account action even though the request and resulting move retain the distribution.

Useful? React with 👍 / 👎.

@nobuQuartile
nobuQuartile marked this pull request as draft September 9, 2026 09:14
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.

1 participant