[6926][ADD] purchase_order_analytic_account_from_lines - #154
[6926][ADD] purchase_order_analytic_account_from_lines#154nobuQuartile wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6db2c2d5f1
ℹ️ 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".
6db2c2d to
ed38aca
Compare
The analytic distribution of a purchase order line is held in a json field, which cannot be shown as a column, grouped or filtered on. An order level distribution is no answer either, as it can only stand for the lines as long as every one of them is distributed the same way. Hold the analytic accounts of the lines on the order itself, so what an order is distributed to can be seen from the purchase order list whichever way its lines are distributed. The accounts are shown as tags coloured by their analytic plan, the way the analytic distribution itself is, and the search view searches the orders by the name of an analytic account.
ed38aca to
5a38c30
Compare
| class PurchaseOrder(models.Model): | ||
| _inherit = "purchase.order" | ||
|
|
||
| analytic_account_ids = fields.Many2many( |
There was a problem hiding this comment.
This field name clashes with the one from analytic_mixin_analytic_account. I think this one should be line_analycit_account_ids or something like that to better represent what it is.
| "account.analytic.account", | ||
| string="Analytic Accounts of Lines", | ||
| compute="_compute_analytic_account_ids", | ||
| store=True, |
There was a problem hiding this comment.
Should we consider adding pre-init hook (+ server action to update existing records)?
There was a problem hiding this comment.
I don't think we need either here.
line_analytic_account_ids is already filled for every existing purchase order at install time.
And installing it on Test took about 4 seconds, so recomputing isn't a practical concern at our data volume either.
|
Maybe |
AungKoKoLin1997
left a comment
AungKoKoLin1997
left a comment
There was a problem hiding this comment.
I believe we don't need to add new field or add the logic. We can just made some adjustment in existing module purchase_view_adj. If you want to make it store, keep the current module but just add store attribute only to the field.
OCA/account-analytic#565 (comment)
@AungKoKoLin1997 I think you miss the point of taking analytic accounts from lines. |
|
Pushed 1. 2. Field rename — 3. Module rename —
|
nobuQuartile
changed the title
3 participants
QT6926
Adds Analytic Accounts of Lines on the purchase order: the analytic accounts the lines of the order are distributed to, as an optional column of the purchase order list and as a filter of its search view.
The analytic distribution of a line is held in a json field, which cannot be shown as a column, grouped or filtered on. An order level distribution is no answer either, as it can only stand for the lines as long as every one of them is distributed the same way — it says nothing as soon as one line differs. The field holds the accounts of every line, whichever analytic plan they belong to, and it is stored, so the orders can be filtered by analytic account.
Split out of #153, which keeps the budget number of the order lines.