[6.1] [delivery] delivery.carrier.write() fails to handle ids=int case
Bug #930127 reported by
Benoit Guillot - http://www.akretion.com
This bug affects 1 person
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| Odoo Addons (MOVED TO GITHUB) |
Fix Released
|
Low
|
OpenERP R&D Addons Team 2 | ||
Bug Description
Hello,
In the module delivery, the method create_grid_lines breaks if there is a write request with only one id.
I propose the following patch to solve the problem.
=== modified file 'delivery/
--- delivery/
+++ delivery/
@@ -98,6 +98,8 @@
return False
def create_
+ if isinstance(ids, int):
+ ids = [ids]
if context == None:
Best regards,
Benoît
Related branches
Revision history for this message
| Olivier Dony (Odoo) (odo-openerp) wrote | #1 |
| Changed in openobject-addons: | |
| assignee: | nobody → OpenERP R&D Addons Team 2 (openerp-dev-addons2) |
| importance: | Undecided → Low |
| status: | New → In Progress |
| summary: |
- [6.1] [delivery] method create_grid_lines should support request with id - or ids + [6.1] [delivery] delivery.carrier.write() fails to handle ids=int case |
Revision history for this message
| Olivier Dony (Odoo) (odo-openerp) wrote | #2 |
| Changed in openobject-addons: | |
| status: | In Progress → Fix Released |
To post a comment you must log in.
Hello Benoit,
I think you indeed face a bug, but the proposed solution is not correct.
All exposed OpenERP API methods (all the model methods that can be used through XML-RPC) are only supposed to work on batches, and must accept a list of IDs as parameters. There are a few exception such as some core ORM methods giving greater flexibility by accepting a single integer/long parameter too, but that is a case-by-case exception, and in fact it is a legacy and discouraged thing to do, because it forces all overriders to copy/paste the same boilerplate code everywhere.
This bad practice is the very reason for the bug... for example if you are calling write() on delivery.carrier with a single ID it will fail because the overridden write() on delivery.carrier does not properly care for it - not because of create_grid_lines() which is correct.
When a method is supposed to compute something it's even worse because the return value will change depending on the parameters (e.g. read() or browse() have a variable result type, that's not very good for consistency)
In general we would like to avoid repeating these lines everywhere and have a consistent API: all methods should work on a list of IDs.
I suggest to fix this by correcting the overridden delivery.
Thanks for reporting