Odoo Addons (MOVED TO GITHUB)

Shipping creation procedure in sale.order is not modular enough, makes inheriting too complex

Bug #463192 reported by Numérigraphe
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Fix Released
Undecided
OpenERP Core Team
Nominated for Trunk by Numérigraphe

Bug Description

The date planned for shipping is computed for each Sale Order Line in the big fat method action_ship_create().
A hook should be added to allow a clean way of changing the computation in a inherited module.

The creation of the pickings, stock moves and procurements should be made modular too, to allow inherited modules to override it.

See original description
Revision history for this message
Numérigraphe (numerigraphe) wrote :

I'm going to try a bit of refactoring.

Changed in openobject-addons:
assignee: nobody → Numérigraphe (numerigraphe)
status: New → In Progress
Revision history for this message
Numérigraphe (numerigraphe) wrote :

Please find attached a tentative patch to add the needed hook.
This patch applies cleanly to the current trunk.
Could you please integrate it in the trunk ?

FYI it also applies cleanly to the current v5.0-bzr.

Revision history for this message
Numérigraphe (numerigraphe) wrote :

This is an example module using the hook.

Revision history for this message
Numérigraphe (numerigraphe) wrote :

Could you please review this patch and tell me what you think?
If it's ok, could you please commit it to the trunk addons?
Lionel

Changed in openobject-addons:
assignee: Numérigraphe (numerigraphe) → OpenERP Quality Team (openerp)
Numérigraphe (numerigraphe)
Changed in openobject-addons:
status: In Progress → Confirmed
Revision history for this message
Dhruti Shastri(OpenERP) (dhs-openerp) wrote :

Hello Lionel,

We agree to your point.
But, is it really feasible to allow such changes for one field?

What if in future there comes another requirement for a different field?

Share your views.
Thanks.

Revision history for this message
Numérigraphe (numerigraphe) wrote :

Actually it's not a field change. It's just that the value for this field is computed in the wrong place.
I read many times that long functions in OpenERP should be split in smaller ones, to make it clear what each function is doing.
action_ship_create() is so long and does so much too much, I think we should refactor it every time we can.
Lionel

Numérigraphe (numerigraphe)
Changed in openobject-addons:
status: Confirmed → In Progress
Revision history for this message
Numérigraphe (numerigraphe) wrote :

I pushed a branch and asked it to be merged into the trunk.
Here's a patch for v5.0.10 if anyone's interrested.
Lionel

Numérigraphe (numerigraphe)
Changed in openobject-addons:
status: In Progress → Fix Committed
Numérigraphe (numerigraphe)
summary: - Date planned for shipping can not be changed by inheriting sale.order
+ Shipping creation procedure in sale.order is not modular enough, makes
+ inheriting too complex
description: updated
Numérigraphe (numerigraphe)
Changed in openobject-addons:
status: Fix Committed → Confirmed
Revision history for this message
Numérigraphe (numerigraphe) wrote :

Raphaël Valyi's refactoring was accepted some time ago, so I'm marking this fixed.
Lionel Sausin

Changed in openobject-addons:
status: Confirmed → Fix Released
Revision history for this message
Numérigraphe (numerigraphe) wrote :

In case someone is interested, I pushed a backport of the refactored action_ship_create from the current trunk to v6.0 to lp:~numerigraphe/openobject-addons/6.0-sale-action-ship-create-refactoring
Lionel.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.