Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
Review: Resubmit Hello, The management of the project has moved to Github: https://github.com/OCA/sale-financial Please migrate your merge proposal to Github. You may want to check https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub for an explanation on how to proceed. Thanks for contributing to the project -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
Review: Approve I tend to agree with Pedro. But I would say, given that Yannick refactored several modules to use this interface and that it seems working. I would say, let's approve that 1st change and get the stuff working. Next, if somebody finds it useful to kill that module and refactor dependent modules in v8 and if that stays simple I would approve such new change. But meanwhile, I approve this change because it's already an improvement and it's there. -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
In that case this module is providing an interface to declare the onchanges it seems cleaner to me. Cheers -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
Review: Abstain I think on contrary, but let others give their opinion. Regards. -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
Why don't you check super method existence?: parent = super(children, self) if hasattr(parent, 'method') and callable(getattr(parent, 'method')): parent.method(cr, uid, ... This seems a little tricky, but it removes the needed of the glue module. What do you think? Regards. -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
[Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
The proposal to merge lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial has been updated. Status: Needs review = Work in progress For more details, see: https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
I rewrite all modules sale_line_watcher, sale_line_floor and sale_markup using web_context_tunnel. It allowed me to break the dependancy from sale_markup on sale_line_floor Looking for reviewers ! -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
[Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
Yannick Vaucher @ Camptocamp has proposed merging
lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial.
Requested reviews:
Pedro Manuel Baeza (pedro.baeza): code review
For more details, see:
https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058
Portage of module sale_line_watcher to v7
--
https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058
Your team Sale Core Editors is subscribed to branch lp:sale-financial.
=== modified file 'sale_line_watcher/__openerp__.py'
--- sale_line_watcher/__openerp__.py 2013-11-14 08:16:22 +
+++ sale_line_watcher/__openerp__.py 2014-04-04 14:22:40 +
@@ -23,19 +23,37 @@
'author' : 'Camptocamp',
'maintainer': 'Camptocamp',
'category': 'Hidden',
- 'complexity': normal, # easy, normal, expert
- 'depends' : ['base', 'sale'],
- 'description': Add new base onchange methods on sale_order class
-
- onchange_price_unit and onchange_discount both accept the following arguments:
- (self, cr, uid, ids, price_unit, product_id, discount, product_uom, pricelist, **kwargs)
+ 'complexity': normal,
+ 'depends' : ['base', 'sale', 'web_context_tunnel'],
+ 'description':
+Sale order line watcher
+===
+
+Add new base onchange methods on Sale Order Line form.
+
+`onchange_price_unit` and `onchange_discount`
+
+Those onchanges can be extendend with additionnal contexts using
+`web_context_tunnel` module
+
+Dependencies
+
+
+`web_context_tunnel` module from lp:server-env-tools branch
+
+Contributors
+
+
+* Nicolas Bessi nicolas.be...@camptocamp.com
+* Yannick Vaucher yannick.vauc...@camptocamp.com
+
,
'website': 'http://www.camptocamp.com',
'init_xml': [],
'update_xml': ['sale_view.xml'],
'demo_xml': [],
'tests': [],
- 'installable': False,
+ 'installable': True,
'auto_install': False,
'license': 'AGPL-3',
'application': True}
=== modified file 'sale_line_watcher/sale_view.xml'
--- sale_line_watcher/sale_view.xml 2012-05-29 07:06:32 +
+++ sale_line_watcher/sale_view.xml 2014-04-04 14:22:40 +
@@ -3,27 +3,24 @@
data
record model=ir.ui.view id=sale_watcher_sale_order_line_form2
field name=namesales_line_watcher.view.form2/field
- field name=typeform/field
field name=modelsale.order.line/field
field name=inherit_id ref=sale.view_order_line_form2 /
field name=arch type=xml
-xpath expr=//field[@name='price_unit'] position=replace
- field name=price_unit
- on_change=onchange_price_unit(price_unit ,product_id, discount, product_uom_qty, parent.pricelist_id)
- select=2/
+xpath expr=//field[@name='price_unit'] position=attributes
+ attribute name=context{}/attribute
+ attribute name=on_changeonchange_price_unit(context)/attribute
/xpath
/field
/record
record model=ir.ui.view id=sale_watcher_sale_order_line_form3
field name=namesales_line_watcher.view.form3/field
- field name=typeform/field
field name=modelsale.order.line/field
field name=inherit_id ref=sale.view_order_line_form2 /
field name=arch type=xml
-xpath expr=//field[@name='discount'] position=replace
- field name=discount
- on_change=onchange_discount(price_unit, product_id, discount, product_uom_qty, parent.pricelist_id)/
+xpath expr=//field[@name='discount'] position=attributes
+ attribute name=context{}/attribute
+ attribute name=on_changeonchange_discount(context)/attribute
/xpath
/field
/record
@@ -32,25 +29,22 @@
field name=namesales_line_watcher.form.floorprice/field
field name=modelsale.order/field
field name=inherit_id ref=sale.view_order_form /
- field name=typeform/field
field name=arch type=xml
-field name=discount position=replace
- field name=discount
- on_change=onchange_discount(price_unit, product_id,discount, product_uom_qty, parent.pricelist_id)/
+field name=discount position=attributes
+ attribute name=context{}/attribute
+ attribute name=on_changeonchange_discount(context)/attribute
/field
/field
-/recordw
+/record
record id=sale_watcher_order_form2 model=ir.ui.view
field name=namesales_line_watcher.form.floorprice/field
field name=modelsale.order/field
field name=inherit_id ref=sale.view_order_form /
- field name=typeform/field
field name=arch type=xml
-field name=price_unit position=replace
- field name=price_unit
- on_change=onchange_price_unit(price_unit ,product_id ,discount, product_uom_qty, parent.pricelist_id)
- select=2/
+field name=price_unit position=attributes
+ attribute name=context{}/attribute
+ attribute
[Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
The proposal to merge lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial has been updated. Commit Message changed to: Portage to v7 of module sale_line_watcher For more details, see: https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
[Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
The proposal to merge lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial has been updated. Description changed to: Portage of module sale_line_watcher to v7 Rewritten using web_context_tunnel For more details, see: https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
Review: Needs Information Hi, Yannick, thanks for the changes. Do you it's worth while to keep this module instead of dropping it and implementing corresponding on_change when needed on dependant modules? One of the advantages of web_context_tunnel is that the context dictionary passed on each on_change is aggregated to previous ones, so there will not be possible incompatibilities (except a reuse of a key name with different meaning). Regards. -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
How would I call super() in onchange if it doesn't exist ? My intend was to break dependency of sale_markup on sale_floor_price but both use those on_changes which doesn't exists. -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
[Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
Yannick Vaucher @ Camptocamp has proposed merging
lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial.
Requested reviews:
Sale Core Editors (sale-core-editors)
For more details, see:
https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058
Portage of module sale_line_watcher to v7
--
https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058
Your team Sale Core Editors is requested to review the proposed merge of
lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial.
=== modified file 'sale_line_watcher/__openerp__.py'
--- sale_line_watcher/__openerp__.py 2013-11-14 08:16:22 +
+++ sale_line_watcher/__openerp__.py 2014-04-03 15:31:56 +
@@ -23,11 +23,15 @@
'author' : 'Camptocamp',
'maintainer': 'Camptocamp',
'category': 'Hidden',
- 'complexity': normal, # easy, normal, expert
+ 'complexity': normal,
'depends' : ['base', 'sale'],
- 'description': Add new base onchange methods on sale_order class
-
- onchange_price_unit and onchange_discount both accept the following arguments:
+ 'description':
+Sale order line watcher
+===
+
+Add new base onchange methods on sale_order class
+
+onchange_price_unit and onchange_discount both accept the following arguments:
(self, cr, uid, ids, price_unit, product_id, discount, product_uom, pricelist, **kwargs)
,
'website': 'http://www.camptocamp.com',
@@ -35,7 +39,7 @@
'update_xml': ['sale_view.xml'],
'demo_xml': [],
'tests': [],
- 'installable': False,
+ 'installable': True,
'auto_install': False,
'license': 'AGPL-3',
'application': True}
=== modified file 'sale_line_watcher/sale_view.xml'
--- sale_line_watcher/sale_view.xml 2012-05-29 07:06:32 +
+++ sale_line_watcher/sale_view.xml 2014-04-03 15:31:56 +
@@ -3,7 +3,6 @@
data
record model=ir.ui.view id=sale_watcher_sale_order_line_form2
field name=namesales_line_watcher.view.form2/field
- field name=typeform/field
field name=modelsale.order.line/field
field name=inherit_id ref=sale.view_order_line_form2 /
field name=arch type=xml
@@ -17,7 +16,6 @@
record model=ir.ui.view id=sale_watcher_sale_order_line_form3
field name=namesales_line_watcher.view.form3/field
- field name=typeform/field
field name=modelsale.order.line/field
field name=inherit_id ref=sale.view_order_line_form2 /
field name=arch type=xml
@@ -32,7 +30,6 @@
field name=namesales_line_watcher.form.floorprice/field
field name=modelsale.order/field
field name=inherit_id ref=sale.view_order_form /
- field name=typeform/field
field name=arch type=xml
field name=discount position=replace
field name=discount
@@ -45,11 +42,10 @@
field name=namesales_line_watcher.form.floorprice/field
field name=modelsale.order/field
field name=inherit_id ref=sale.view_order_form /
- field name=typeform/field
field name=arch type=xml
field name=price_unit position=replace
field name=price_unit
- on_change=onchange_price_unit(price_unit ,product_id ,discount, product_uom_qty, parent.pricelist_id)
+ on_change=onchange_price_unit(price_unit, product_id ,discount, product_uom_qty, parent.pricelist_id)
select=2/
/field
/field
=== modified file 'sale_line_watcher/sale_watcher.py'
--- sale_line_watcher/sale_watcher.py 2012-05-29 07:06:32 +
+++ sale_line_watcher/sale_watcher.py 2014-04-03 15:31:56 +
@@ -25,20 +25,20 @@
_inherit = 'sale.order.line'
def onchange_price_unit(self, cr, uid, ids,
-price_unit, product_id, discount, product_uom, pricelist,
-**kwargs):
-'''
+price_unit, product_id, discount, product_uom,
+pricelist, **kwargs):
+
Place holder function for onchange unit price
-'''
+
res = {}
return res
def onchange_discount(self, cr, uid, ids,
- price_unit, product_id, discount, product_uom, pricelist,
- **kwargs):
-'''
+ price_unit, product_id, discount, product_uom,
+ pricelist, **kwargs):
+
Place holder function for onchange discount
-'''
+
res = {}
return res
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to : openerp-community-reviewer@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openerp-community-reviewer
More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/sale-financial/7.0-port-sale_line_watcher into lp:sale-financial
Review: Needs Fixing code review Hi, Yannick, several things: - Instead of replacing a field, it's better to put position=attributes and only overwrite attribute on_change, so that you don't replace another attributes of the field. - I think it's better to use module web_context_tunnel to avoid changes on the signature of the method that provokes incompatibilities between modules. Regards. -- https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058 Your team Sale Core Editors is subscribed to branch lp:sale-financial. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
