Re: [Openerp-community-reviewer] [Merge] lp:~agilebg/stock-logistic-flows/adding_product_customer_code_picking into lp:stock-logistic-flows
2014-06-22 23:10 GMT+02:00 Alex Comba - Agile BG alex.co...@agilebg.com: Lorenzo, please have a look at my last commit, now it should be ok. Thanks Alex, maybe we could handle even more particular cases by using the address_get method to retrieve the company? -- https://code.launchpad.net/~agilebg/stock-logistic-flows/adding_product_customer_code_picking/+merge/202472 Your team Stock and Logistic Core Editors is subscribed to branch lp:stock-logistic-flows. -- 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:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries
Review: Needs Fixing Hi You don't have to test if available_option.mandatory is True but you can only use if available_option.mandatory as it's a boolean the is True is useless. -- https://code.launchpad.net/~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl/+merge/224027 Your team Stock and Logistic Core Editors is subscribed to branch lp:carriers-deliveries. -- 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:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries
@sebastien, fix done, thank you for review -- https://code.launchpad.net/~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl/+merge/224027 Your team Stock and Logistic Core Editors is subscribed to branch lp:carriers-deliveries. -- 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:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl/+merge/224027 Your team Stock and Logistic Core Editors is subscribed to branch lp:carriers-deliveries. -- 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:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries
Review: Approve code review Hi, David, thanks for the improvement. You have to change module version number and provide a proper migration script for those that have previously the module installed. Regards. -- https://code.launchpad.net/~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl/+merge/224027 Your team Stock and Logistic Core Editors is subscribed to branch lp:carriers-deliveries. -- 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:~serpentcs/server-env-tools/base_module_record into lp:server-env-tools
Review: Needs Fixing code review, no tests Never used it, code looks shaggy, and there are not tests. Are there known issues? In it's current state, this module is not on par with the standards of OCA and it needs care and love. If someone has use for this module and is volunteering to maintain it, I'm willing to perform a detailed review of the code and help that person to improve the module. -- https://code.launchpad.net/~serpentcs/server-env-tools/base_module_record/+merge/196613 Your team Server Environment And Tools Core Editors is subscribed to branch lp:server-env-tools. -- 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:~akretion-team/partner-contact-management/7.0-partner-helper-dbl into lp:partner-contact-management
Hi Pedro module is renamed and ready for merge Thank you Regards -- https://code.launchpad.net/~akretion-team/partner-contact-management/7.0-partner-helper-dbl/+merge/221501 Your team Partner and Contact Core Editors is subscribed to branch lp:partner-contact-management. -- 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:~akretion-team/partner-contact-management/7.0-partner-helper-dbl into lp:partner-contact-management
Review: Approve code review Great, thanks. Regards. -- https://code.launchpad.net/~akretion-team/partner-contact-management/7.0-partner-helper-dbl/+merge/221501 Your team Partner and Contact Core Editors is subscribed to branch lp:partner-contact-management. -- 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:~initos.com/sale-reports/7.0-fix_lang_for_draft into lp:sale-reports
Katja Matthes has proposed merging
lp:~initos.com/sale-reports/7.0-fix_lang_for_draft into lp:sale-reports.
Requested reviews:
Sale Core Editors (sale-core-editors)
For more details, see:
https://code.launchpad.net/~initos.com/sale-reports/7.0-fix_lang_for_draft/+merge/224106
Steps to reproduce:
1) In sale order tree view klick 'Create' to open draft formular for a new sale
order
2) Choose for patner_id a partner with an other language than English
3) Set text_condition{1,2}
Result
English version of condition text is set to note{1,2}.
Expected Result:
Use language of set partner while setting note{1,2}.
Solution:
Add partner_id as parameter to on_change function for text_condition{1,2}
--
https://code.launchpad.net/~initos.com/sale-reports/7.0-fix_lang_for_draft/+merge/224106
Your team OpenERP Community Reviewer/Maintainer is subscribed to branch
lp:sale-reports.
=== modified file 'sale_order_webkit/sale.py'
--- sale_order_webkit/sale.py 2013-03-14 14:14:24 +
+++ sale_order_webkit/sale.py 2014-06-23 11:09:44 +
@@ -18,8 +18,9 @@
#along with this program. If not, see http://www.gnu.org/licenses/.
#
##
-from openerp.osv import orm, fields
+from openerp.osv import orm, fields, osv
from openerp import netsvc
+from openerp.tools.translate import _
class SaleConditionText(orm.Model):
@@ -48,23 +49,22 @@
'note1': fields.html('Header'),
'note2': fields.html('Footer')}
-def _set_condition(self, cursor, uid, inv_id, commentid, key):
+def _set_condition(self, cursor, uid, inv_id, commentid, key, partner_id=False):
Set the text of the notes in invoices
if not commentid:
return {}
-try:
-lang = self.browse(cursor, uid, inv_id)[0].partner_id.lang
-except Exception as exc:
-lang = 'en_US'
+if not partner_id:
+raise osv.except_osv(_('No Customer Defined !'), _('Before choosing condition text select a customer.'))
+lang = self.pool.get('res.partner').browse(cursor, uid, partner_id).lang or 'en_US'
cond = self.pool.get('sale.condition_text').browse(cursor, uid,
commentid, {'lang': lang})
return {'value': {key: cond.text}}
-def set_header(self, cursor, uid, inv_id, commentid):
-return self._set_condition(cursor, uid, inv_id, commentid, 'note1')
+def set_header(self, cursor, uid, inv_id, commentid, partner_id=False):
+return self._set_condition(cursor, uid, inv_id, commentid, 'note1', partner_id)
-def set_footer(self, cursor, uid, inv_id, commentid):
-return self._set_condition(cursor, uid, inv_id, commentid, 'note2')
+def set_footer(self, cursor, uid, inv_id, commentid, partner_id=False):
+return self._set_condition(cursor, uid, inv_id, commentid, 'note2', partner_id)
def print_quotation(self, cursor, uid, ids, context=None):
'''
=== modified file 'sale_order_webkit/view/sale_view.xml'
--- sale_order_webkit/view/sale_view.xml 2013-03-12 12:33:46 +
+++ sale_order_webkit/view/sale_view.xml 2014-06-23 11:09:44 +
@@ -68,9 +68,9 @@
page string=Conditions
group
field name=text_condition1 domain=[('type','=','header')]
- on_change=set_header(text_condition1) colspan=2/
+ on_change=set_header(text_condition1, partner_id) colspan=2/
field name=text_condition2 domain=[('type','=','footer')]
- on_change=set_footer(text_condition2) colspan=2/
+ on_change=set_footer(text_condition2, partner_id) colspan=2/
field name=note1 colspan=4 nolabel=1 placeholder=Your top conditions here/
field name=note2 colspan=4 nolabel=1 placeholder=Your bottom conditions here/
/group
--
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:~initos.com/account-invoice-report/7.0-fix_lang_for_draft into lp:account-invoice-report
Katja Matthes has proposed merging
lp:~initos.com/account-invoice-report/7.0-fix_lang_for_draft into
lp:account-invoice-report.
Requested reviews:
Account Core Editors (account-core-editors)
For more details, see:
https://code.launchpad.net/~initos.com/account-invoice-report/7.0-fix_lang_for_draft/+merge/224109
Steps to reproduce:
1) In invoice tree view klick 'Create' to open draft formular for a new invoice
2) Choose for partner_id a partner with an other language than English
3) Set text_condition{1,2}
Result
English version of condition text is set to note{1,2}.
Expected Result:
Use language of set partner while setting note{1,2}.
Solution:
Add partner_id as parameter to on_change function for text_condition{1,2}
--
https://code.launchpad.net/~initos.com/account-invoice-report/7.0-fix_lang_for_draft/+merge/224109
Your team OpenERP Community Reviewer/Maintainer is subscribed to branch
lp:account-invoice-report.
=== modified file 'invoice_webkit/invoice.py'
--- invoice_webkit/invoice.py 2013-02-01 08:46:04 +
+++ invoice_webkit/invoice.py 2014-06-23 11:29:29 +
@@ -18,6 +18,8 @@
#along with this program. If not, see http://www.gnu.org/licenses/.
##
from openerp.osv.orm import Model, fields
+from openerp.osv import osv
+from openerp.tools.translate import _
class InvoiceConditionText(Model):
@@ -38,22 +40,21 @@
_inherit = account.invoice
-def _set_condition(self, cr, uid, inv_id, commentid, key):
+def _set_condition(self, cr, uid, inv_id, commentid, key, partner_id=False):
Set the text of the notes in invoices
if not commentid:
return {}
-try :
-lang = self.browse(cr, uid, inv_id)[0].partner_id.lang
-except :
-lang = 'en_US'
+if not partner_id:
+raise osv.except_osv(_('No Customer Defined !'), _('Before choosing condition text select a customer.'))
+lang = self.pool.get('res.partner').browse(cr, uid, partner_id).lang or 'en_US'
cond = self.pool.get('account.condition_text').browse(cr, uid, commentid, {'lang': lang})
return {'value': {key: cond.text}}
-def set_header(self, cr, uid, inv_id, commentid):
-return self._set_condition(cr, uid, inv_id, commentid, 'note1')
+def set_header(self, cursor, uid, inv_id, commentid, partner_id=False):
+return self._set_condition(cursor, uid, inv_id, commentid, 'note1', partner_id)
-def set_footer(self, cr, uid, inv_id, commentid):
-return self._set_condition(cr, uid, inv_id, commentid, 'note2')
+def set_footer(self, cursor, uid, inv_id, commentid, partner_id=False):
+return self._set_condition(cursor, uid, inv_id, commentid, 'note2', partner_id)
_columns = {'text_condition1': fields.many2one('account.condition_text', 'Header condition',
domain=[('type', '=', 'header')]),
=== modified file 'invoice_webkit/view/invoice_view.xml'
--- invoice_webkit/view/invoice_view.xml 2013-03-04 06:52:00 +
+++ invoice_webkit/view/invoice_view.xml 2014-06-23 11:29:29 +
@@ -53,9 +53,9 @@
page string=Conditions
group
field name=text_condition1 domain=[('type','=','header')]
- on_change=set_header(text_condition1) colspan=2/
+ on_change=set_header(text_condition1, partner_id) colspan=2/
field name=text_condition2 domain=[('type','=','footer')]
- on_change=set_footer(text_condition2) colspan=2/
+ on_change=set_footer(text_condition2, partner_id) colspan=2/
field name=note1 colspan=4 nolabel=1 placeholder=Your top conditions here/
field name=note2 colspan=4 nolabel=1 placeholder=Your bottom conditions here/
/group
--
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:~initos.com/account-invoice-report/7.0-fix_lang_for_draft into lp:account-invoice-report
Review: Needs Fixing code review Hi, Katja, You're right that current code doesn't work, but this is because the line that made the work is incorrect: try : lang = self.browse(cr, uid, inv_id).partner_id.lang inv_id is an id, not a list of ids, so you don't need to put index 0 to access browse_record. I think it's better to simply fix this instead of changing all the code you put. Regards. -- https://code.launchpad.net/~initos.com/account-invoice-report/7.0-fix_lang_for_draft/+merge/224109 Your team OpenERP Community Reviewer/Maintainer is subscribed to branch lp:account-invoice-report. -- 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:~initos.com/sale-reports/7.0-fix_lang_for_draft into lp:sale-reports
Review: Needs Fixing code review This is the same as the other MP you have made (https://code.launchpad.net/~initos.com/account-invoice-report/7.0-fix_lang_for_draft/+merge/224109), so please change it in the same way. Regards. -- https://code.launchpad.net/~initos.com/sale-reports/7.0-fix_lang_for_draft/+merge/224106 Your team OpenERP Community Reviewer/Maintainer is subscribed to branch lp:sale-reports. -- 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:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries
Migrate script works for me. I think it could merge Only miss Yannick review -- https://code.launchpad.net/~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl/+merge/224027 Your team Stock and Logistic Core Editors is subscribed to branch lp:carriers-deliveries. -- 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:~alejandrosantana/account-financial-tools/7.0-account-financial_tools--add--account_account_extended_search into lp:account-financial-tools
setting the MP as work in progress. Please set it back to needs review when requested changes are made. -- https://code.launchpad.net/~alejandrosantana/account-financial-tools/7.0-account-financial_tools--add--account_account_extended_search/+merge/200092 Your team OpenERP Community Reviewer/Maintainer is subscribed to branch lp:account-financial-tools. -- 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:~gdgellatly/server-env-tools/base-synchro-7.0 into lp:server-env-tools
Review: Needs Fixing code review, no tests If people are willing to maintain this, then I'm not opposed to merging in server-env-tools. 2 points, though, to gain my approval: * the various size constraints seem overzealous to me (esp. server URL and domain which are obviously too short for no good reason) * there are no automated tests, and for functionality such as this one, this is definitely needed. -- https://code.launchpad.net/~gdgellatly/server-env-tools/base-synchro-7.0/+merge/200127 Your team Server Environment And Tools Core Editors is subscribed to branch lp:server-env-tools. -- 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:~pedro.baeza/account-financial-report/6.1-balance-reporting into lp:account-financial-report/6.1
Review: Needs Fixing I would really love to see this code exercised by a couple of yaml tests... -- https://code.launchpad.net/~pedro.baeza/account-financial-report/6.1-balance-reporting/+merge/201166 Your team Account Report Core Editors is subscribed to branch lp:account-financial-report/6.1. -- 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:~akretion-team/partner-contact-management/base-location-geonames-import into lp:partner-contact-management
Hello Alexis, many thanks for the module. What do you think about creating the res.country.state records if they don't exist, before mapping them in the 'states' dictionary? The current version is supposed to correctly work with states if you first create states data by modules like l10n_fr_state. But if base_location_geonames_import also imported states data from geonames (creating records if they don't exist yet) we would not need an extra module just to create states data. -- https://code.launchpad.net/~akretion-team/partner-contact-management/base-location-geonames-import/+merge/214564 Your team Partner and Contact Core Editors is subscribed to branch lp:partner-contact-management. -- 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
