[6756][ADD] partner_shipping_rank - #255
Conversation
nobuQuartile
left a comment
There was a problem hiding this comment.
The module name, sale_delivery_rank, is better.
Could you add ja.po?
| <record id="view_order_tree" model="ir.ui.view"> | ||
| <field name="name">sale.order.list</field> | ||
| <field name="model">sale.order</field> | ||
| <field name="inherit_id" ref="sale.view_order_tree" /> | ||
| <field name="arch" type="xml"> | ||
| <xpath expr="//field[@name='partner_id']" position="after"> | ||
| <field name="delivery_rank" optional="hide" /> | ||
| </xpath> | ||
| </field> | ||
| </record> |
There was a problem hiding this comment.
I changed inherit_id.
| delivery_rank = fields.Selection( | ||
| related="partner_shipping_id.delivery_rank", | ||
| store=True, | ||
| readonly=True, |
There was a problem hiding this comment.
We can remove it as readonly is True by default for related fields.
| readonly=True, |
|
|
||
| delivery_rank = fields.Selection( | ||
| related="partner_shipping_id.delivery_rank", | ||
| store=True, |
There was a problem hiding this comment.
A design question. We should generally avoid storing related/computed values that depend on master records, since changing the value in the master record ends up triggering updates to many transactions. Does this need to be stored?
There was a problem hiding this comment.
There is no reason to store this field in sale.order, so I removed it(SO is as well)
| delivery_rank = fields.Selection( | ||
| related="partner_shipping_id.delivery_rank", | ||
| ) |
There was a problem hiding this comment.
Shouldn't it be compute store field? Then, we can use that in search field.
With current design, it will always up to date with partner master data.
Is it intentional?
There was a problem hiding this comment.
The rank is intended to classify delivery partners, so I don't think the field should be stored on the transactional records, which means SOs don't need to have the delivery rank at the time of the order.
The use case is to export the delivery_rank via sale.order.line and send it to salespersons periodically.
Regarding searchability, searching SOs or invoices by rank is not a frequent use case (JOIN is sufficient).
There was a problem hiding this comment.
yostashiro
left a comment
There was a problem hiding this comment.
Module name suggestion: partner_shipping_rank
'Delivery' could mean something else in Odoo.
|
|
||
| delivery_rank = fields.Selection( | ||
| [("a", "A"), ("b", "B"), ("c", "C")], | ||
| help="Rank assigned to a delivery destination. ", |
There was a problem hiding this comment.
| help="Rank assigned to a delivery destination. ", | |
| help="Rank assigned to a delivery destination.", |
| "summary": "Assign a rank to partners as delivery destinations and " | ||
| "propagate it to sales orders and invoices for aggregation by rank.", |
There was a problem hiding this comment.
"Propagate" may be misleading now that it's not kept in transactions.
AungKoKoLin1997
left a comment
There was a problem hiding this comment.
Code Review: LGTM
There was a problem hiding this comment.
Don't we need to add in the list view like sale order?
There was a problem hiding this comment.
I added the list view.
6b5c58a to
0f29ca0
Compare
QT6756
This module adds the delivery field in
res.partnerand the delivery field as related tosale.orderandaccont.move, ultimately allowing users to export this field thoroughsale.orderandaccont.move.