[14.0][ADD] purchase_order_supplierinfo_update#18
Conversation
mathieudelva
force-pushed
the
14.0-add_purchase_order_supplierinfo_update
branch
from
301c1e5 to
4d6100c
Compare
metaminux
force-pushed
the
14.0-add_purchase_order_supplierinfo_update
branch
from
3956c77 to
d9e566d
Compare
sebastienbeau
force-pushed
the
14.0-add_purchase_order_supplierinfo_update
branch
from
d9e566d to
f72fa99
Compare
There was a problem hiding this comment.
We need to fill the fr.po file before deploying to production...
There was a problem hiding this comment.
Missing copyright and if possible sorting alphabetically for maintainability
| if supplierinfo: | ||
| if line._is_matching_supplierinfo(supplierinfo): | ||
| continue | ||
|
|
||
| lines.append((0, 0, line._prepare_supplier_wizard_line(supplierinfo))) |
There was a problem hiding this comment.
If supplierinfo is an empty recordset, line._is_matching_supplierinfo will most probably be False (when line.product_uom is set and line.price_unit is not 0.0) and a line will be added with an empty supplierinfo.
| if supplierinfo: | |
| if line._is_matching_supplierinfo(supplierinfo): | |
| continue | |
| lines.append((0, 0, line._prepare_supplier_wizard_line(supplierinfo))) | |
| if supplierinfo: | |
| if line._is_matching_supplierinfo(supplierinfo): | |
| continue | |
| lines.append((0, 0, line._prepare_supplier_wizard_line(supplierinfo))) |
| <tree | ||
| create="false" | ||
| editable="bottom" | ||
| decoration-info="not supplierinfo_id" |
There was a problem hiding this comment.
Is it really needed if no lines are added when supplierinfo_id is empty ?
| supplierinfos = line.product_id._select_seller( | ||
| line.order_id.partner_id.commercial_partner_id, | ||
| quantity=None, | ||
| ) |
There was a problem hiding this comment.
Calling _select_seller without giving all parameters from the line can retrieve inaccurate supplierinfo (not in the right date range, not for the right quantity, etc.)
| supplierinfos = line.product_id._select_seller( | |
| line.order_id.partner_id.commercial_partner_id, | |
| quantity=None, | |
| ) | |
| supplierinfos = line.product_id._select_seller( | |
| partner_id=line.order_id.partner_id.commercial_partner_id, | |
| quantity=line.product_qty, | |
| date=line.order_id.date_order, | |
| uom_id=line.product_uom, | |
| ) |
| # Copyright 2025 Akretion (https://www.akretion.com). | ||
| # @author Mathieu DELVA <mathieu.delva@akretion.com> | ||
| # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). |
There was a problem hiding this comment.
Copyright present but with a different format than in the other files of the module...
I personally use this format, but I don't know if there is one "official" format so feel to choose your preferred one, and keep the same in all files
| import setuptools | ||
|
|
||
| setuptools.setup( | ||
| setup_requires=['setuptools-odoo'], |
There was a problem hiding this comment.
| setup_requires=['setuptools-odoo'], | |
| setup_requires=["setuptools-odoo"], |
| <field name="supplierinfo_id" /> | ||
| <field | ||
| name="current_price" | ||
| attrs="{'invisible': [('supplierinfo_id', '=', 'False')]}" |
There was a problem hiding this comment.
| attrs="{'invisible': [('supplierinfo_id', '=', 'False')]}" | |
| attrs="{'invisible': [('supplierinfo_id', '=', False)]}" |
| cls.purchase = cls.env.ref("purchase_stock.purchase_order_8") | ||
| product = cls.env.ref("product.product_product_25_product_template") | ||
| vendor = cls.env.ref("base.res_partner_4") |
There was a problem hiding this comment.
It is not recommended to rely on demo data for tests : https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#avoid-relying-on-demo-data
There was a problem hiding this comment.
To highlight some points discussed in this review, can you add some tests with :
- product with several supplierinfo using start and end dates, different quantities, etc. : should retrieve the one corresponding to this given line
- product without supplierinfo for this partner shouldn't create a line in the wizard
- same product purchased in several lines but with different prices, quantitites, etc.
- test with draft purchase
|
Hello @mathieudelva Can you look at those comments ? I think some of them should be addressed before migrating to higher version. Regards, |
4 participants
No description provided.