From 449a160ef99ec4e1723545a9499e821514517df6 Mon Sep 17 00:00:00 2001 From: Denis Ledoux Date: Tue, 17 May 2016 16:30:05 +0200 Subject: [PATCH 01/19] [FIX] account: usererror takes only one argument opw-677589 --- addons/account/models/account_move.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 95f58fe972c..0b8db4bf796 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -715,7 +715,7 @@ class AccountMoveLine(models.Model): :param new_mv_line_dicts: list of dicts containing values suitable fot account_move_line.create() """ if len(self) < 1 or len(self) + len(new_mv_line_dicts) < 2: - raise UserError(_('Error!'), _('A reconciliation must involve at least 2 move lines.')) + raise UserError(_('A reconciliation must involve at least 2 move lines.')) # Create writeoff move lines if len(new_mv_line_dicts) > 0: From cab2dce0d2a05d800027553e2919dfd75dda8b5d Mon Sep 17 00:00:00 2001 From: Nicolas Martinelli Date: Tue, 17 May 2016 15:04:43 +0200 Subject: [PATCH 02/19] [FIX] web: Y-axis formatting In case of a line graph, the values of the Y axis are not formatted as numbers, although they are on the bar graph. --- addons/web/static/src/js/views/graph_widget.js | 1 + 1 file changed, 1 insertion(+) diff --git a/addons/web/static/src/js/views/graph_widget.js b/addons/web/static/src/js/views/graph_widget.js index a86ea35e449..048838ad9cd 100644 --- a/addons/web/static/src/js/views/graph_widget.js +++ b/addons/web/static/src/js/views/graph_widget.js @@ -301,6 +301,7 @@ return Widget.extend({ }); chart.xAxis.tickValues(tickValues) .tickFormat(tickFormat); + chart.yAxis.tickFormat(function(d) { return openerp.web.format_value(d, { type : 'float' });}); chart(svg); this.to_remove = chart.update; From 68b127797e717ee71b5b0552d69f0f4b4e083f03 Mon Sep 17 00:00:00 2001 From: Nicolas Martinelli Date: Tue, 17 May 2016 15:09:23 +0200 Subject: [PATCH 03/19] [FIX] web: left margin of graph The left margin of the graph doesn't take into account the width of the maximum Y-axis value. In case the values are high (> 10^7), they are cropped. The fix is to adapt the left margin as a function of the size of the maximum value. opw-676538 --- addons/web/static/src/js/views/graph_widget.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/addons/web/static/src/js/views/graph_widget.js b/addons/web/static/src/js/views/graph_widget.js index 048838ad9cd..e458cc483ed 100644 --- a/addons/web/static/src/js/views/graph_widget.js +++ b/addons/web/static/src/js/views/graph_widget.js @@ -162,7 +162,9 @@ return Widget.extend({ svg.transition().duration(0); var chart = nv.models.multiBarChart(); + var maxVal = _.max(values, function(v) {return v.y}) chart.options({ + margin: {left: 12 * String(maxVal && maxVal.y || 0).length}, delay: 250, transition: 10, showLegend: _.size(data) <= MAX_LEGEND_LENGTH, @@ -292,8 +294,9 @@ return Widget.extend({ svg.transition().duration(0); var chart = nv.models.lineChart(); + var maxVal = _.max(values, function(v) {return v.y}) chart.options({ - margin: {left: 50, right: 50}, + margin: {left: 12 * String(maxVal && maxVal.y || 0).length, right: 50}, useInteractiveGuideline: true, showLegend: _.size(data) <= MAX_LEGEND_LENGTH, showXAxis: true, From 96e085355d2e29d81ace06e08dc76ea5668846ef Mon Sep 17 00:00:00 2001 From: Denis Ledoux Date: Tue, 17 May 2016 17:51:16 +0200 Subject: [PATCH 04/19] [FIX] account: partial reconcile unreconcile with deleted credit/debit move `debit_move_id` and `credit_move_id` of `account.partial.reconcile` must be required. Not having one of them set on this model does not make sense, you cannot have a reconciliation with a debit move but without a credit move, for instance. Databases already having the trouble, having partial reconciliations without either a debit or a credit move, had issues for the computation of the `payment_move_line_ids` compute field, which relied on the fact these fields were supposed to be required. This revision makes a retro-compatibility patch for these databases already having the issue. opw-677709 --- addons/account/models/account_invoice.py | 4 ++-- addons/account/models/account_move.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/addons/account/models/account_invoice.py b/addons/account/models/account_invoice.py index 89a6dcd1cb0..4465d4ff7f0 100644 --- a/addons/account/models/account_invoice.py +++ b/addons/account/models/account_invoice.py @@ -180,8 +180,8 @@ class AccountInvoice(models.Model): def _compute_payments(self): payment_lines = [] for line in self.move_id.line_ids: - payment_lines.extend([rp.credit_move_id.id for rp in line.matched_credit_ids]) - payment_lines.extend([rp.debit_move_id.id for rp in line.matched_debit_ids]) + payment_lines.extend(filter(None, [rp.credit_move_id.id for rp in line.matched_credit_ids])) + payment_lines.extend(filter(None, [rp.debit_move_id.id for rp in line.matched_debit_ids])) self.payment_move_line_ids = self.env['account.move.line'].browse(list(set(payment_lines))) name = fields.Char(string='Reference/Description', index=True, diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 0b8db4bf796..5cc899b9fad 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -1208,8 +1208,8 @@ class AccountPartialReconcile(models.Model): _name = "account.partial.reconcile" _description = "Partial Reconcile" - debit_move_id = fields.Many2one('account.move.line', index=True) - credit_move_id = fields.Many2one('account.move.line', index=True) + debit_move_id = fields.Many2one('account.move.line', index=True, required=True) + credit_move_id = fields.Many2one('account.move.line', index=True, required=True) amount = fields.Monetary(currency_field='company_currency_id', help="Amount concerned by this matching. Assumed to be always positive") amount_currency = fields.Monetary(string="Amount in Currency") currency_id = fields.Many2one('res.currency', string='Currency') From 7f1d7c1cce6dac89695c57d8507b609aecaf0c51 Mon Sep 17 00:00:00 2001 From: Goffin Simon Date: Tue, 17 May 2016 10:04:30 +0200 Subject: [PATCH 05/19] [FIX] account: invoice has not the same currency as the purchase order Used case: -Configure company in EUR, and user in multi-currency -Create a purchase order in USD and confirm it -Click on the smart button "0 invoices" then click on "create" Result before the fix: The system created an invoice in EUR, currency of the company, instead of in USD, currency of the purchase order. Result after the fix: The system created an invoice in USD, the currency of the related purchase order. opw:677568 --- addons/account/models/account_invoice.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/addons/account/models/account_invoice.py b/addons/account/models/account_invoice.py index 4465d4ff7f0..103b736f124 100644 --- a/addons/account/models/account_invoice.py +++ b/addons/account/models/account_invoice.py @@ -69,6 +69,9 @@ class AccountInvoice(models.Model): @api.model def _default_currency(self): + if 'default_purchase_id' in self.env.context: + purchase_id = self.env.context['default_purchase_id'] + return self.env['purchase.order'].browse(purchase_id).currency_id journal = self._default_journal() return journal.currency_id or journal.company_id.currency_id From 252bd47f94afbaa75c2ce55967c80952b80eaa42 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 12 May 2016 15:51:46 +0200 Subject: [PATCH 06/19] [FIX] calendar: don't block UI on slow /calendar/notify Safe imp for a stable version. Better fix could be to send notifications through the bus, but this would be in master. --- addons/calendar/static/src/js/base_calendar.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/calendar/static/src/js/base_calendar.js b/addons/calendar/static/src/js/base_calendar.js index 7e92f23f181..4f9b43ddc70 100644 --- a/addons/calendar/static/src/js/base_calendar.js +++ b/addons/calendar/static/src/js/base_calendar.js @@ -210,7 +210,7 @@ WebClient.include({ get_next_notif: function() { var self = this; - this.rpc("/calendar/notify") + this.rpc("/calendar/notify", {}, {shadow: true}) .done(function(result) { _.each(result, function(res) { setTimeout(function() { From 16d950f5ada165a2a290338d0d5e3313786a5120 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Wed, 11 May 2016 13:52:26 +0200 Subject: [PATCH 07/19] [FIX] web_calendar: search date range when no date_stop field --- addons/web_calendar/static/src/js/web_calendar.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/addons/web_calendar/static/src/js/web_calendar.js b/addons/web_calendar/static/src/js/web_calendar.js index 438e5f28000..93dfc195e77 100644 --- a/addons/web_calendar/static/src/js/web_calendar.js +++ b/addons/web_calendar/static/src/js/web_calendar.js @@ -842,13 +842,11 @@ var CalendarView = View.extend({ */ get_range_domain: function(domain, start, end) { var format = time.date_to_str; - var extend_domain = [[this.date_start, '<=', format(end)]]; - if (this.date_stop) { - extend_domain.push( - [this.date_stop, '>=', format(start)] - ); + extend_domain.push([this.date_stop, '>=', format(start)]); + } else if (!this.date_delay) { + extend_domain.push([this.date_start, '>=', format(start)]); } return new CompoundDomain(domain, extend_domain); }, From 8ce16c3abe6fe42ec609a952eb12d01ad4f6e897 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Wed, 18 May 2016 10:53:05 +0200 Subject: [PATCH 08/19] [FIX] doc: white h2 title Cherry-pick from odoo/documentation-user@d193f7c0 --- doc/_extensions/odoo/static/typography.less | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/_extensions/odoo/static/typography.less b/doc/_extensions/odoo/static/typography.less index bcf7fef2acf..32fb821f8f0 100644 --- a/doc/_extensions/odoo/static/typography.less +++ b/doc/_extensions/odoo/static/typography.less @@ -18,7 +18,7 @@ h1, h2, h3, h4, h5, h6, line-height: 1.4; @media (min-width: @screen-lg-min) { - .has_code_col &{ + .has_code_col .doc-aside &{ color: white; } } From 6c66365b7b262959d4987f60a86963d1b656ed05 Mon Sep 17 00:00:00 2001 From: Richard Mathot Date: Wed, 18 May 2016 12:27:14 +0200 Subject: [PATCH 09/19] [FIX] website_sale_delivery: reset carrier on cart update It makes no sense to recompute delivery costs at each update of the cart; furthermore, it prevents users that already selected a carrier to remove the last products of their carts This fix is additional to e4e1e571c2e5854b16b913b9b7bce9774b395055 --- addons/website_sale_delivery/models/sale_order.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/addons/website_sale_delivery/models/sale_order.py b/addons/website_sale_delivery/models/sale_order.py index ecefc8b1fad..098e6d86a43 100644 --- a/addons/website_sale_delivery/models/sale_order.py +++ b/addons/website_sale_delivery/models/sale_order.py @@ -161,6 +161,13 @@ class SaleOrder(orm.Model): """ Override to update carrier quotation if quantity changed """ self._delivery_unset(cr, uid, ids, context=context) + + # When you update a cart, it is not enouf to remove the "delivery cost" line + # The carrier might also be invalid, eg: if you bought things that are too heavy + # -> this may cause a bug if you go to the checkout screen, choose a carrier, + # then update your cart (the cart becomes uneditable) + self.write(cr, uid, ids, {'carrier_id': False}, context=context) + values = super(SaleOrder, self)._cart_update( cr, uid, ids, product_id, line_id, add_qty, set_qty, context, **kwargs) From 216d328a11f254a3c037eb9e23ab362b984ca0ba Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Wed, 18 May 2016 14:17:50 +0200 Subject: [PATCH 10/19] [FIX] web_editor: serialise translations of html_translate fields The html_translate fields (e.g. website_descripton on product.product) were not serialised and the raw value from the web_editor was saved as a translation. This was an issue in case of special characters that may be present in the translation. A translation containing non-breaking space was sent in html (`foo bar`) while lxml converts such characters to unicode (`foo\xa0bar`). When writing a translation, the value is checked against incorrect format using ``` value0 = field.translate(lambda term: None, record[fname]) value1 = field.translate({trans.src: trans.value}.get, value0) value2 = field.translate({trans.value: trans.src}.get, value1) if value2 != value0: raise ValidationError(_("Translation is not valid:\n%s") % trans.value) ``` As value1 is the unicode version of the translation and `trans.value` is the html version of the translation, the last substitution in the callback method was never made and the ValidationError was raised. This commit forces the serialisation through lxml to be sure the compared strings are using the same parser instead of comparing value from summernote and lxml that may be both valid but still different. opw-675767 --- addons/web_editor/models/ir_translation.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/addons/web_editor/models/ir_translation.py b/addons/web_editor/models/ir_translation.py index 19627b58c57..df56cf41011 100644 --- a/addons/web_editor/models/ir_translation.py +++ b/addons/web_editor/models/ir_translation.py @@ -4,7 +4,7 @@ from lxml import etree from openerp import models, api -from openerp.tools.translate import encode, xml_translate +from openerp.tools.translate import encode, xml_translate, html_translate def edit_translation_mapping(data): data = dict(data, model=data['name'].partition(',')[0]) @@ -35,4 +35,11 @@ class ir_translation(models.Model): # root is html > body > div # serialize div as XML and discard surrounding tags value = etree.tostring(root[0][0], encoding='utf-8')[5:-6] + elif field.translate == html_translate: + # wrap value inside a div and parse it as HTML + div = "
%s
" % encode(value) + root = etree.fromstring(div, etree.HTMLParser(encoding='utf-8')) + # root is html > body > div + # serialize div as HTML and discard surrounding tags + value = etree.tostring(root[0][0], encoding='utf-8', method='html')[5:-6] return self.write({'value': value}) From 8acfb0d72dd1d3f570df700e3483e639a5e4d666 Mon Sep 17 00:00:00 2001 From: vbh-odoo Date: Fri, 9 Oct 2015 11:23:31 +0530 Subject: [PATCH 11/19] [FIX] website_gengo: fixed button click and translations - Fixed linking issue after click on 'Translate' button. - Controller added to post terms in gengo opw-648546 --- .../base_gengo/controller/gengo_callback.py | 2 +- addons/website_gengo/controllers/main.py | 27 +++++++++++++++++++ .../static/src/css/website_gengo.css | 6 ++--- .../static/src/js/website_gengo.js | 27 ++++++++----------- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/addons/base_gengo/controller/gengo_callback.py b/addons/base_gengo/controller/gengo_callback.py index 328be0ae357..85af7f8585d 100644 --- a/addons/base_gengo/controller/gengo_callback.py +++ b/addons/base_gengo/controller/gengo_callback.py @@ -16,7 +16,7 @@ class website_gengo(http.Controller): icp = request.registry['ir.config_parameter'] return icp.get_param(cr, SUPERUSER_ID, request.registry['base.gengo.translations'].GENGO_KEY, default="") - @http.route('/website/gengo_callback', type='http', auth='none') + @http.route('/website/gengo_callback', type='http', auth='none', csrf=False) def gengo_callback(self, **post): print "IN website/gengo_callback" cr, uid, context = request.cr, openerp.SUPERUSER_ID, request.context diff --git a/addons/website_gengo/controllers/main.py b/addons/website_gengo/controllers/main.py index 8c0a345029d..cdb1ade7a99 100644 --- a/addons/website_gengo/controllers/main.py +++ b/addons/website_gengo/controllers/main.py @@ -37,3 +37,30 @@ class website_gengo(http.Controller): def post_gengo_jobs(self): request.registry['base.gengo.translations']._sync_request(request.cr, request.uid, limit=GENGO_DEFAULT_LIMIT, context=request.context) return True + + @http.route('/website_gengo/set_translations', type='json', auth='user', website=True) + def set_translations(self, data, lang): + IrTranslation = request.env['ir.translation'] + for term in data: + initial_content = term['initial_content'].strip() + translation_ids = term['translation_id'] + if not translation_ids: + translations = IrTranslation.search_read([('lang', '=', lang), ('src', '=', initial_content)], fields=['id']) + if translations: + translation_ids = map(lambda t_id: t_id['id'], translations) + + vals = { + 'gengo_comment': term['gengo_comment'], + 'gengo_translation': term['gengo_translation'], + 'state': 'to_translate', + } + if translation_ids: + IrTranslation.browse(translation_ids).write(vals) + else: + vals.update({ + 'name': 'website', + 'lang': lang, + 'source': initial_content, + }) + IrTranslation.create(vals) + return True diff --git a/addons/website_gengo/static/src/css/website_gengo.css b/addons/website_gengo/static/src/css/website_gengo.css index c623e749963..a1682aed00f 100644 --- a/addons/website_gengo/static/src/css/website_gengo.css +++ b/addons/website_gengo/static/src/css/website_gengo.css @@ -1,3 +1,3 @@ -.o_translatable_inprogress { - background: #b7e4ff; -} \ No newline at end of file +html[lang] > body.editor_enable [data-oe-translation-state="inprogress"] { + background: #b7e4ff !important; +} diff --git a/addons/website_gengo/static/src/js/website_gengo.js b/addons/website_gengo/static/src/js/website_gengo.js index bc49f035748..91c8455273b 100644 --- a/addons/website_gengo/static/src/js/website_gengo.js +++ b/addons/website_gengo/static/src/js/website_gengo.js @@ -18,14 +18,13 @@ if (!translate.edit_translations) { ajax.loadXML('/website_gengo/static/src/xml/website.gengo.xml', qweb); -website.TopBar.include({ - events: _.extend({}, website.TopBar.prototype.events, { +translate.Class.include({ + events: _.extend({}, translate.Class.prototype.events, { 'click a[data-action=translation_gengo_post]': 'translation_gengo_post', 'click a[data-action=translation_gengo_info]': 'translation_gengo_info', }), edit:function () { this.gengo_translate = true; - this._super.apply(this, arguments); var self = this; var gengo_langs = ["ar_SY","id_ID","nl_NL","fr_CA","pl_PL","zh_TW","sv_SE","ko_KR","pt_PT","en_US","ja_JP","es_ES","zh_CN","de_DE","fr_FR","fr_BE","ru_RU","it_IT","pt_BR","pt_BR","th_TH","nb_NO","ro_RO","tr_TR","bg_BG","da_DK","en_GB","el_GR","vi_VN","he_IL","hu_HU","fi_FI"]; if (gengo_langs.indexOf(base.get_context()['lang']) !== -1){ @@ -33,6 +32,7 @@ website.TopBar.include({ self.$('button[data-action=save]') .after(qweb.render('website.ButtonGengoTranslator')); } + this._super.apply(this, arguments); }, translation_gengo_display:function(){ var self = this; @@ -57,29 +57,25 @@ website.TopBar.include({ dialog.$el.modal('hide'); self.$el.find('.gengo_post').addClass("hidden"); self.$el.find('.gengo_wait').removeClass("hidden"); - var trans ={} + var trans = []; $('[data-oe-translation-state="to_translate"], [data-oe-translation-state="None"]').each(function () { var $node = $(this); var data = $node.data(); - if (!trans[data.oeTranslationViewId]) { - trans[data.oeTranslationViewId] = []; - } - trans[data.oeTranslationViewId].push({ - initial_content: qweb.tools.html_escape(self.getInitialContent(this)), - new_content: qweb.tools.html_escape(self.getInitialContent(this)), + + var val = ($node.is('img')) ? $node.attr('alt') : $node.text(); + trans.push({ + initial_content: qweb.tools.html_escape(val), translation_id: data.oeTranslationId || null, gengo_translation: gengo_service_level, gengo_comment:"\nOriginal Page: " + document.URL }); }); - ajax.jsonRpc('/website/set_translations', 'call', { + ajax.jsonRpc('/website_gengo/set_translations', 'call', { 'data': trans, 'lang': base.get_context()['lang'], }).then(function () { - self.$el.find('.gengo_wait').addClass("hidden"); - self.$el.find('.gengo_inprogress,.gengo_discard').removeClass("hidden"); ajax.jsonRpc('/website/post_gengo_jobs', 'call', {}); - self.save(); + self.save_and_reload(); }).fail(function () { alert("Could not Post translation"); }); @@ -96,7 +92,7 @@ website.TopBar.include({ translation_gengo_info: function () { var repr = $(document.documentElement).data('mainObject'); var translated_ids = []; - $('.oe_translatable_text').not(".oe_translatable_inprogress").each(function(){ + $('[data-oe-translation-state="translated"]').each(function () { translated_ids.push($(this).attr('data-oe-translation-id')); }); ajax.jsonRpc('/website/get_translated_length', 'call', { @@ -105,7 +101,6 @@ website.TopBar.include({ }).done(function(res){ var dialog = new GengoTranslatorStatisticDialog(res); dialog.appendTo($(document.body)); - }); }, }); From c286e2d5b2000ca7a9b388d21b0a4af4fd71a914 Mon Sep 17 00:00:00 2001 From: Goffin Simon Date: Wed, 18 May 2016 16:24:40 +0200 Subject: [PATCH 12/19] [FIX] project_issue: number of issues When no stage has been defined for a project, the new issues are set in an undefined stage(stage_id = False). Then these issues must be counted. opw:676985 --- addons/project_issue/project_issue.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/project_issue/project_issue.py b/addons/project_issue/project_issue.py index 54be75411c6..78bab712022 100644 --- a/addons/project_issue/project_issue.py +++ b/addons/project_issue/project_issue.py @@ -450,7 +450,7 @@ class project(osv.Model): def _issue_count(self, cr, uid, ids, field_name, arg, context=None): Issue = self.pool['project.issue'] return { - project_id: Issue.search_count(cr,uid, [('project_id', '=', project_id), ('stage_id.fold', '=', False)], context=context) + project_id: Issue.search_count(cr,uid, [('project_id', '=', project_id), '|', ('stage_id.fold', '=', False), ('stage_id', '=', False)], context=context) for project_id in ids } From 9365482df05f49fda86735343013163871cd33e0 Mon Sep 17 00:00:00 2001 From: Nicolas Martinelli Date: Thu, 19 May 2016 08:55:39 +0200 Subject: [PATCH 13/19] [FIX] account: sequence of statement The sequence used in the bank statement lines is not reliable. Indeed, several lines might have the same sequence. opw-676924 --- addons/account/models/account_bank_statement.py | 1 + 1 file changed, 1 insertion(+) diff --git a/addons/account/models/account_bank_statement.py b/addons/account/models/account_bank_statement.py index c77240b482d..e518964d7f2 100644 --- a/addons/account/models/account_bank_statement.py +++ b/addons/account/models/account_bank_statement.py @@ -815,6 +815,7 @@ class AccountBankStatementLine(models.Model): st_line_currency_rate = self.currency_id and (self.amount_currency / self.amount) or False # Create the move + self.sequence = self.statement_id.line_ids.ids.index(self.id) + 1 move_name = (self.statement_id.name or self.name) + "/" + str(self.sequence) move_vals = self._prepare_reconciliation_move(move_name) move = self.env['account.move'].create(move_vals) From 80200333fcfe1b616b2b77ae0b2c9d92e16f3cfa Mon Sep 17 00:00:00 2001 From: Nicolas Martinelli Date: Thu, 19 May 2016 08:56:29 +0200 Subject: [PATCH 14/19] [FIX] account: statement line amount rounding See comment in code. opw-676924 --- addons/account/models/account_bank_statement.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/addons/account/models/account_bank_statement.py b/addons/account/models/account_bank_statement.py index e518964d7f2..1b809fe59a9 100644 --- a/addons/account/models/account_bank_statement.py +++ b/addons/account/models/account_bank_statement.py @@ -392,6 +392,20 @@ class AccountBankStatementLine(models.Model): if self.amount_currency != 0 and self.amount == 0: raise ValidationError(_('If "Amount Currency" is specified, then "Amount" must be as well.')) + @api.model + def create(self, vals): + line = super(AccountBankStatementLine, self).create(vals) + # The most awesome fix you will ever see is below. + # Explanation: during a 'create', the 'convert_to_cache' method is not called. Moreover, at + # that point 'journal_currency_id' is not yet known since it is a related field. It means + # that the 'amount' field will not be properly rounded. The line below triggers a write on + # the 'amount' field, which will trigger the 'convert_to_cache' method, and ultimately round + # the field correctly. + # This is obviously an awful workaround, but at the time of writing, the ORM does not + # provide a clean mechanism to fix the issue. + line.amount = line.amount + return line + @api.multi def unlink(self): for line in self: From 1def9911df74c3df4d28b8a609d537cb156f0952 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Thu, 19 May 2016 12:10:32 +0200 Subject: [PATCH 15/19] [FIX] account: duplicate journal should not use the same bank account There is several override of create and write that assumes that a bank account is present on only one account (see 4530e5d). It is very hard to change or remove the account bank once linked to a journal (see 4a65bc38) so it should not be set in the first place when duplicating a journal. opw-676374 --- addons/account/models/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/account/models/account.py b/addons/account/models/account.py index d0b770c1df9..3e98bdf40d7 100644 --- a/addons/account/models/account.py +++ b/addons/account/models/account.py @@ -251,7 +251,7 @@ class AccountJournal(models.Model): loss_account_id = fields.Many2one('account.account', string='Loss Account', domain=[('deprecated', '=', False)], help="Used to register a loss when the ending balance of a cash register differs from what the system computes") # Bank journals fields - bank_account_id = fields.Many2one('res.partner.bank', string="Bank Account", ondelete='restrict') + bank_account_id = fields.Many2one('res.partner.bank', string="Bank Account", ondelete='restrict', copy=False) display_on_footer = fields.Boolean("Show in Invoices Footer", help="Display this bank account on the footer of printed documents like invoices and sales orders.") bank_statements_source = fields.Selection([('manual', 'Record Manually')], string='Bank Feeds') bank_acc_number = fields.Char(related='bank_account_id.acc_number') From 03c032ecb9061ca30bea59a86d7373517ff67f46 Mon Sep 17 00:00:00 2001 From: Nicolas Martinelli Date: Thu, 19 May 2016 13:52:08 +0200 Subject: [PATCH 16/19] [FIX] web: company logo file extension The company logo file extension is always '.png', even if the logo is of another type such as 'jpg' or 'gif'. This is not an issue for decent browsers, but IE might not display the image under specific circumstancies, e.g. in combination with nginx. opw-676836 --- addons/web/controllers/main.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index 4b7634da132..3cd15189496 100644 --- a/addons/web/controllers/main.py +++ b/addons/web/controllers/main.py @@ -4,6 +4,7 @@ import base64 import csv import functools import glob +import imghdr import itertools import jinja2 import logging @@ -1077,7 +1078,8 @@ class Binary(http.Controller): '/logo.png', ], type='http', auth="none", cors="*") def company_logo(self, dbname=None, **kw): - imgname = 'logo.png' + imgname = 'logo' + imgext = '.png' placeholder = functools.partial(get_resource_path, 'web', 'static', 'src', 'img') uid = None if request.session.db: @@ -1090,7 +1092,7 @@ class Binary(http.Controller): uid = openerp.SUPERUSER_ID if not dbname: - response = http.send_file(placeholder(imgname)) + response = http.send_file(placeholder(imgname + imgext)) else: try: # create an empty registry @@ -1104,12 +1106,14 @@ class Binary(http.Controller): """, (uid,)) row = cr.fetchone() if row and row[0]: - image_data = StringIO(str(row[0]).decode('base64')) - response = http.send_file(image_data, filename=imgname, mtime=row[1]) + image_base64 = str(row[0]).decode('base64') + image_data = StringIO(image_base64) + imgext = '.' + (imghdr.what(None, h=image_base64) or 'png') + response = http.send_file(image_data, filename=imgname + imgext, mtime=row[1]) else: response = http.send_file(placeholder('nologo.png')) except Exception: - response = http.send_file(placeholder(imgname)) + response = http.send_file(placeholder(imgname + imgext)) return response From b04d6e0ac325a9cafece210ce70032a02264ada6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9rome=20Maes?= Date: Fri, 3 Jul 2015 15:09:39 +0200 Subject: [PATCH 17/19] [IMP] doc : expand the coding guidelines - correct typos - clarify some points, according to the community comments - precise the xml notation : record tag vs act_window/menuitem/... tags - add organization of report, wizard, data/demo, ... (directories and naming conventions) - new conventions for 'symbols' - ... Thanks to @rim-odoo for its grammar extremist review, and @tde-banana-odoo for our long debates about wizard conventions. --- doc/reference/guidelines.rst | 149 ++++++++++++++++++++++++++--------- 1 file changed, 111 insertions(+), 38 deletions(-) diff --git a/doc/reference/guidelines.rst b/doc/reference/guidelines.rst index 54a00097705..ad33f2ba56d 100644 --- a/doc/reference/guidelines.rst +++ b/doc/reference/guidelines.rst @@ -6,10 +6,9 @@ Odoo Guidelines =============== -This page introduce the new Odoo Coding Guidelines. These guidelines -aim to improve the quality of the code (better readability of source, -...) and Odoo Apps. Indeed, proper code ought ease maintenance, aid -debugging, lower complexity and promote reliability. +This page introduce the new Odoo Coding Guidelines. These guidelines aim to improve the quality of the code (better readability of source, ...) and Odoo Apps. Indeed, proper code ought ease maintenance, aid debugging, lower complexity and promote reliability. + +These guidelines should be applied to every new module, and new developpment. These guidelines will be applied to old module **only** in case of code refactoring (migration to new API, big refactoring, ...). .. warning:: @@ -24,14 +23,22 @@ Module structure Directories ----------- -A module is organised in a few directories : +A module is organised in some important directories. These directories aim to contain the business core of the module; having a look at them should make understand the purpose of the module. - *data/* : demo and data xml - *models/* : models definition -- *controllers/* : contains controllers (http routes). +- *controllers/* : contains controllers (HTTP routes). - *views/* : contains the views and templates - *static/* : contains the web assets, separated into *css/, js/, img/, lib/, ...* +Other directories compose the module. + +- *data/* : contains the data (in XML form) +- *wizard/* : regroup the transient models (formerly *osv_memory*) and their views. +- *report/* : contains the reports (RML report **[deprecated]**, models based on SQL views (for reporting) and other complex reports). Python objects and XML views are included in this directory. +- *tests/* : contains the Python/YML tests + + File naming ----------- For *views* declarations, split backend views from (frontend) @@ -52,16 +59,36 @@ For instance, *sale* module introduces ``sale_order`` and ```` files will be named :file:`models/sale_order.py` and :file:`views/sale_order_views.py`. - For *data*, split them by purpose : demo or data. The filename will be the main_model name, suffixed by *_demo.xml* or *_data.xml*. -For *controller*, the only file should be named *main.py*. +For *controllers*, the only file should be named *main.py*. Otherwise, if you need to inherit an existing controller from another module, its name will be *.py*. Unlike *models*, each controller should be contained in a separated file. + +For *static files*, since the resources can be used in different contexts (frontend, backend, both), they will be included in only one bundle. So, CSS/Less, JavaScript and XML files should be suffixed with the name of the bundle type. i.e.: *im_chat_common.css*, *im_chat_common.js* for 'assets_common' bundle, and *im_chat_backend.css*, *im_chat_backend.js* for 'assets_backend' bundle. +For modules having only one file, the convention will be *.ext* (i.e.: *project.js*). +Don't link data (image, libraries) outside Odoo: don't use an +URL to an image but copy it in our codebase instead. + +For *data*, split them by purpose: data or demo. The filename will be +the *main_model* name, suffixed by *_data.xml* or *_demo.xml*. + +For *wizards*, the naming convention is : + +- :file:`{}.py` +- :file:`{}_views.xml` + +Where ** is the name of the dominant transient model, just like for *models*. .py can contains the models 'model.action' and 'model.action.line'. + +For *statistics reports*, their names should look like : + +- :file:`{}_report.py` +- :file:`{}_report_views.py` (often pivot and graph views) + +For *printable reports*, you should have : + +- :file:`{}_reports.py` (report actions, paperformat definition, ...) +- :file:`{}_templates.xml` (xml report templates) -For *static files*, the name pattern is *.ext* (i.e. : -static/js/im_chat.js, static/css/im_chat.css, static/xml/im_chat.xml, -...). Don't link data (image, libraries) outside Odoo : don't use an -url to an image but copy it in our codebase instead. The complete tree should look like @@ -72,6 +99,7 @@ The complete tree should look like |-- __openerp__.py |-- controllers/ | |-- __init__.py + | |-- .py | `-- main.py |-- data/ | |-- _data.xml @@ -80,6 +108,12 @@ The complete tree should look like | |-- __init__.py | |-- .py | `-- .py + |-- report/ + | |-- __init__.py + | |-- .py + | |-- _views.xml + | |-- _reports.xml + | `-- _templates.xml |-- security/ | |-- ir.model.access.csv | `-- _security.xml @@ -98,12 +132,16 @@ The complete tree should look like | | `-- .less | `-- xml/ | `-- .xml - `-- views/ - |-- _templates.xml - |-- _views.xml - |-- _templates.xml - `-- _views.xml - + |-- views/ + | |-- _templates.xml + | |-- _views.xml + | |-- _templates.xml + | `-- _views.xml + `-- wizard/ + |-- .py + |-- _views.xml + |-- .py + `-- _views.xml .. note:: File names should only contain ``[a-z0-9_]`` (lowercase alphanumerics and ``_``) @@ -115,7 +153,7 @@ XML files Format ------ -When declaring a record in XML, +To declare a record in XML, the **record** notation (using **) is recommended: - Place ``id`` attribute before ``model`` - For field declaration, ``name`` attribute is first. Then place the @@ -124,7 +162,7 @@ When declaring a record in XML, ordered by importance. - Try to group the record by model. In case of dependencies between - action/menu/views, the convention may not be applicable. + action/menu/views, this convention may not be applicable. - Use naming convention defined at the next point - The tag ** is only used to set not-updatable data with ``noupdate=1`` @@ -142,6 +180,16 @@ When declaring a record in XML, +Some syntax equivalences exists, and can be used: + +- menuitem: use it as a shortcut to declare a ``ir.ui.menu`` +- workflow: the tag sends a signal to an existing workflow. +- template: use it to declare a QWeb View requiring only the ``arch`` section of the view. +- report: use to declare a :ref:`report action ` +- act_window: use it if the record notation can't do what you want + +The 4 first tags are prefered over the *record* notation. + Naming xml_id ------------- @@ -165,14 +213,11 @@ Use the following pattern : *concerned_group* is the short name of the concerned group ('user' for the 'model_name_group_user', 'public' for public user, 'company' for multi-company rules, ...). +* For a group : :samp:`{}_group_{}` where *group_name* is the name of the group, generally 'user', 'manager', ... .. code-block:: xml - - ... - - ... @@ -181,6 +226,19 @@ Use the following pattern : ... + + + ... @@ -191,7 +249,7 @@ Use the following pattern : - + ... @@ -261,6 +319,7 @@ Inside these 3 groups, the imported lines are alphabetically sorted. import base64 import re import time + from datetime import datetime # 2 : imports of openerp import openerp from openerp import api, fields, models # alphabetically ordered @@ -274,22 +333,27 @@ Inside these 3 groups, the imported lines are alphabetically sorted. Idioms ------ -- Prefer ``%`` over ``.format()``, prefer ``%(varname)`` instead of position (This is better for translations) -- Try to avoid generators and decorators -- Always favor *Readability* over *conciseness* or using the language features or idioms. +- Prefer ``%`` over ``.format()`` (when only one variable to replace in a string), prefer ``%(varname)`` instead of position (when multiple variables are to be replaced in a string). This makes the translation easier for the translators community. +- Avoid to create generators and decorators: only use the ones provide by the Odoo API. +- Always favor *readability* over *conciseness* or using the language features or idioms. - Use list comprehension, dict comprehension, and basic manipulation using ``map``, ``filter``, ``sum``, ... They make the code easier to read. - The same applies for recordset methods : use ``filtered``, ``mapped``, ``sorted``, ... -- Each python file should have ``# coding: utf-8 `` as first line -- Use the ``UserError`` defined in ``openerp.exceptions`` instead of overriding ``Warning``, or find a more appropriate exception in *exceptions.py* +- Each python file should have ``# -*- coding: utf-8 -*-`` as first line. - Document your code (docstring on methods, simple comments for the tricky part of the code) -- Use meaningful variable/class/method names - +- Use meaningful variable/class/method names. +- Every method used to compute data for a 'stat button' should use a ``read_group`` or a SQL query. This aims to improve performance (by computing data in only on query). Symbols ------- -- Odoo Python Class : use camelcase for code in api v8, underscore lowercase notation for old api. +- Model name (using the dot notation, prefix by the module name) : + - When defining an Odoo Model : use singular form of the name (*res.partner* and *sale.order* instead of *res.partnerS* and *saleS.orderS*) + - When defining an Odoo Transient (wizard) : use ``.`` where *related_base_model* is the base model (defined in *models/*) related to the transient, and *action* is the short name of what the transient do. For instance : ``account.invoice.make``, ``project.task.delegate.batch``, ... + - When defining *report* model (SQL views e.i.) : use ``.report.``, based on the Transient convention. + +- Odoo Python Class : use camelcase for code in api v8 (Object-oriented style), underscore lowercase notation for old api (SQL style). + .. code-block:: python @@ -349,7 +413,7 @@ Symbols store=True, readonly=True, compute='_compute_seats') price = fields.Integer(string='Price') - # compute and search fields, in the same order that fields declaration + # compute and search fields, in the same order of fields declaration @api.multi @api.depends('seats_max', 'registration_ids.state', 'registration_ids.nb_register') def _compute_seats(self): @@ -364,8 +428,8 @@ Symbols def _onchange_date_begin(self): ... - # CRUD methods - def create(self): + # CRUD methods (and name_get, name_search, ...) overrides + def create(self, values): ... # Action methods @@ -387,12 +451,20 @@ Javascript and CSS - Use a linter (jshint, ...) - Never add minified Javascript Libraries - Use camelcase for class declaration +- Unless your code is supposed to run on every page, target specific pages using the ``if_dom_contains`` function of website module. Target an element which is specific to the pages your code needs to run on using JQuery. + +.. code-block:: javascript + + openerp.website.if_dom_contains('.jquery_class_selector', function () { + /*your code here*/ + }); + **For CSS :** -- Prefix all your class with *o_* where *module_name* is the technical name of the module ('sale', 'im_chat', ...) or the main route reserved by the module (for website module mainly, i.e. : 'o_forum' for website_forum module). The only exception for this rule is the webclient : it simply use *o_* prefix. +- Prefix all your classes with *o_* where *module_name* is the technical name of the module ('sale', 'im_chat', ...) or the main route reserved by the module (for website module mainly, i.e. : 'o_forum' for *website_forum* module). The only exception for this rule is the webclient: it simply uses *o_* prefix. - Avoid using id -- Use bootstrap native class +- Use Bootstrap native classes - Use underscore lowercase notation to name class Git @@ -408,6 +480,7 @@ Prefix your commit with - **[REF]** for refactoring - **[ADD]** for adding new resources - **[REM]** for removing of resources +- **[MOV]** for moving files (Do not change content of moved file, otherwise Git will loose track, and the history will be lost !), or simply moving code from a file to another one. - **[MERGE]** for merge commits (only for forward/back-port) - **[CLA]** for signing the Odoo Individual Contributor License From dfcbcb02cea2b2f5c996933ea1d813e51da5f9d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9rome=20Maes?= Date: Wed, 6 Apr 2016 14:33:43 +0200 Subject: [PATCH 18/19] [IMP] doc : add python idioms and odoo specific coding guidelines - add python idioms - odoo specific guidelines (for ORM, translations, ...) inspired from old guidelines v6.0 - some other best pratices --- doc/reference/guidelines.rst | 537 ++++++++++++++++++++++++++++++++--- 1 file changed, 496 insertions(+), 41 deletions(-) diff --git a/doc/reference/guidelines.rst b/doc/reference/guidelines.rst index ad33f2ba56d..5ae52963b42 100644 --- a/doc/reference/guidelines.rst +++ b/doc/reference/guidelines.rst @@ -6,7 +6,7 @@ Odoo Guidelines =============== -This page introduce the new Odoo Coding Guidelines. These guidelines aim to improve the quality of the code (better readability of source, ...) and Odoo Apps. Indeed, proper code ought ease maintenance, aid debugging, lower complexity and promote reliability. +This page introduces the new Odoo Coding Guidelines. Those aim to improve the quality of the code (e.g. better readability of source) and Odoo Apps. Indeed, proper code eases maintenance, aids debugging, lowers complexity and promotes reliability. These guidelines should be applied to every new module, and new developpment. These guidelines will be applied to old module **only** in case of code refactoring (migration to new API, big refactoring, ...). @@ -23,7 +23,7 @@ Module structure Directories ----------- -A module is organised in some important directories. These directories aim to contain the business core of the module; having a look at them should make understand the purpose of the module. +A module is organised in important directories. Those contain the business logic; having a look at them should make understand the purpose of the module. - *data/* : demo and data xml - *models/* : models definition @@ -31,10 +31,9 @@ A module is organised in some important directories. These directories aim to co - *views/* : contains the views and templates - *static/* : contains the web assets, separated into *css/, js/, img/, lib/, ...* -Other directories compose the module. +Other optional directories compose the module. -- *data/* : contains the data (in XML form) -- *wizard/* : regroup the transient models (formerly *osv_memory*) and their views. +- *wizard/* : regroups the transient models (formerly *osv_memory*) and their views. - *report/* : contains the reports (RML report **[deprecated]**, models based on SQL views (for reporting) and other complex reports). Python objects and XML views are included in this directory. - *tests/* : contains the Python/YML tests @@ -62,17 +61,17 @@ For instance, *sale* module introduces ``sale_order`` and For *data*, split them by purpose : demo or data. The filename will be the main_model name, suffixed by *_demo.xml* or *_data.xml*. -For *controllers*, the only file should be named *main.py*. Otherwise, if you need to inherit an existing controller from another module, its name will be *.py*. Unlike *models*, each controller should be contained in a separated file. +For *controllers*, the only file should be named *main.py*. Otherwise, if you need to inherit an existing controller from another module, its name will be *.py*. Unlike *models*, each controller class should be contained in a separated file. For *static files*, since the resources can be used in different contexts (frontend, backend, both), they will be included in only one bundle. So, CSS/Less, JavaScript and XML files should be suffixed with the name of the bundle type. i.e.: *im_chat_common.css*, *im_chat_common.js* for 'assets_common' bundle, and *im_chat_backend.css*, *im_chat_backend.js* for 'assets_backend' bundle. -For modules having only one file, the convention will be *.ext* (i.e.: *project.js*). -Don't link data (image, libraries) outside Odoo: don't use an +If the module owns only one file, the convention will be *.ext* (i.e.: *project.js*). +Don't link data (image, libraries) outside Odoo: do not use an URL to an image but copy it in our codebase instead. -For *data*, split them by purpose: data or demo. The filename will be +Regarding *data*, split them by purpose: data or demo. The filename will be the *main_model* name, suffixed by *_data.xml* or *_demo.xml*. -For *wizards*, the naming convention is : +Regarding *wizards*, naming convention is : - :file:`{}.py` - :file:`{}_views.xml` @@ -180,7 +179,7 @@ To declare a record in XML, the **record** notation (using **) is recomm -Some syntax equivalences exists, and can be used: +Odoo supports custom tags acting as syntactic sugar: - menuitem: use it as a shortcut to declare a ``ir.ui.menu`` - workflow: the tag sends a signal to an existing workflow. @@ -308,7 +307,7 @@ Imports The imports are ordered as #. External libraries (one per line sorted and split in python stdlib) -#. Imports of ``openerp`` +#. Imports of ``odoo`` #. Imports from Odoo modules (rarely, and only if necessary) Inside these 3 groups, the imported lines are alphabetically sorted. @@ -320,39 +319,480 @@ Inside these 3 groups, the imported lines are alphabetically sorted. import re import time from datetime import datetime - # 2 : imports of openerp - import openerp - from openerp import api, fields, models # alphabetically ordered - from openerp.tools.safe_eval import safe_eval as eval - from openerp.tools.translate import _ + # 2 : imports of odoo + import odoo + from odoo import api, fields, models # alphabetically ordered + from odoo.tools.safe_eval import safe_eval as eval + from odoo.tools.translate import _ # 3 : imports from odoo modules - from openerp.addons.website.models.website import slug - from openerp.addons.web.controllers.main import login_redirect + from odoo.addons.website.models.website import slug + from odoo.addons.web.controllers.main import login_redirect -Idioms ------- +Idiomatics Python Programming +----------------------------- -- Prefer ``%`` over ``.format()`` (when only one variable to replace in a string), prefer ``%(varname)`` instead of position (when multiple variables are to be replaced in a string). This makes the translation easier for the translators community. -- Avoid to create generators and decorators: only use the ones provide by the Odoo API. -- Always favor *readability* over *conciseness* or using the language features or idioms. -- Use list comprehension, dict comprehension, and basic manipulation using ``map``, ``filter``, ``sum``, ... They make the code easier to read. -- The same applies for recordset methods : use ``filtered``, ``mapped``, ``sorted``, ... - Each python file should have ``# -*- coding: utf-8 -*-`` as first line. -- Document your code (docstring on methods, simple comments for the tricky part of the code) -- Use meaningful variable/class/method names. -- Every method used to compute data for a 'stat button' should use a ``read_group`` or a SQL query. This aims to improve performance (by computing data in only on query). +- Always favor *readability* over *conciseness* or using the language features or idioms. +- Don't use ``.clone()`` + +.. code-block:: python + + # bad + new_dict = my_dict.clone() + new_list = old_list.clone() + # good + new_dict = dict(my_dict) + new_list = list(old_list) + +- Python dictionnary : creation and update + +.. code-block:: python + + # -- creation empty dict + my_dict = {} + my_dict2 = dict() + + # -- creation with values + # bad + my_dict = {} + my_dict['foo'] = 3 + my_dict['bar'] = 4 + # good + my_dict = {'foo': 3, 'bar': 4} + + # -- update dict + # bad + my_dict['foo'] = 3 + my_dict['bar'] = 4 + my_dict['baz'] = 5 + # good + my_dict.update(foo=3, bar=4, baz=5) + my_dict = dict(my_dict, **my_dict2) + +- Use meaningful variable/class/method names +- Useless variable : Temporary variables can make the code clearer by giving + names to objects, but that doesn't mean you should create temporary variables + all the time: + +.. code-block:: python + + # pointless + schema = kw['schema'] + params = {'schema': schema} + # simpler + params = {'schema': kw['schema']} + +- Multiple return points are OK, when they're simpler + +.. code-block:: python + + # a bit complex and with a redundant temp variable + def axes(self, axis): + axes = [] + if type(axis) == type([]): + axes.extend(axis) + else: + axes.append(axis) + return axes + + # clearer + def axes(self, axis): + if type(axis) == type([]): + return list(axis) # clone the axis + else: + return [axis] # single-element list + +- Know your builtins : You should at least have a basic understanding of all + the Python builtins (http://docs.python.org/library/functions.html) + +.. code-block:: python + + value = my_dict.get('key', None) # very very redundant + value= my_dict.get('key') # good + +Also, ``if 'key' in my_dict`` and ``if my_dict.get('key')`` have very different +meaning, be sure that you're using the right one. + +- Learn list comprehensions : Use list comprehension, dict comprehension, and + basic manipulation using ``map``, ``filter``, ``sum``, ... They make the code + easier to read. + +.. code-block:: python + + # not very good + cube = [] + for i in res: + cube.append((i['id'],i['name'])) + # better + cube = [(i['id'], i['name']) for i in res] + +- Collections are booleans too : In python, many objects have "boolean-ish" value + when evaluated in a boolean context (such as an if). Among these are collections + (lists, dicts, sets, ...) which are "falsy" when empty and "truthy" when containing + items: + +.. code-block:: python + + bool([]) is False + bool([1]) is True + bool([False]) is True + +So, you can write ``if some_collection:`` instead of ``if len(some_collection):``. -Symbols -------- +- Iterate on iterables + +.. code-block:: python + + # creates a temporary list and looks bar + for key in my_dict.keys(): + "do something..." + # better + for key in my_dict: + "do something..." + # creates a temporary list + for key, value in my_dict.items(): + "do something..." + # only iterates + for key, value in my_dict.iteritems(): + "do something..." + +- Use dict.setdefault + +.. code-block:: python + + # longer.. harder to read + values = {} + for element in iterable: + if element not in values: + values[element] = [] + values[element].append(other_value) + + # better.. use dict.setdefault method + values = {} + for element in iterable: + values.setdefault(element, []).append(other_value) + +- As a good developper, document your code (docstring on methods, simple + comments for tricky part of code) +- In additions to these guidelines, you may also find the following link + interesting: http://python.net/~goodger/projects/pycon/2007/idiomatic/handout.html + (a little bit outdated, but quite relevant) + +Programming in Odoo +------------------- + +- Avoid to create generators and decorators: only use the ones provided by + the Odoo API. +- As in python, use ``filtered``, ``mapped``, ``sorted``, ... methods to + ease code reading and performance. + + +Make your method works in batch +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +When adding a function, make sure it can process multiple records. Typically, +such method is decorated with ``api.multi`` decorator (or takes a list of *id*, +if written in old api). Then you will have to iterate on ``self`` to treat each +record. + +.. code-block:: python + + @api.multi + def my_method(self) + for record in self: + record.do_cool_stuff() + +Avoid to use ``api.one`` decorator : this will probably not do what you expected, +and extending a such method is not as easy than a *api.multi* method, since it +returns a list of result (ordered by recordset ids). + +For performance issue, when developping a 'stat button' (for instance), do not +perform a ``search`` or a ``search_count`` in a loop in a ``api.multi`` method. It +is recommended to use ``read_group`` method, to compute all value in only one request. + +.. code-block:: python + + @api.multi + def _compute_equipment_count(self): + """ Count the number of equipement per category """ + equipment_data = self.env['hr.equipment'].read_group([('category_id', 'in', self.ids)], ['category_id'], ['category_id']) + mapped_data = dict([(m['category_id'][0], m['category_id_count']) for m in equipment_data]) + for category in self: + category.equipment_count = mapped_data.get(category.id, 0) + + +Propagate the context +~~~~~~~~~~~~~~~~~~~~~ +In new API, the context is a ``frozendict`` that cannot be modified. To call +a method with a different context, the ``with_context`` method should be used : + +.. code-block:: python + + records.with_context(new_context).do_stuff() # all the context is replaced + records.with_context(**additionnal_context).do_other_stuff() # additionnal_context values override native context ones + +Passing parameter in context can have dangerous side-effects. Since the values +are propagated automatically, some behavior can appears. Calling ``create()`` +method of a model with *default_my_field* key in context will set the default +value of *my_field* for the concerned model. But if curing this creation, other +object (such as sale.order.line, on sale.order creation) having a field +name *my_field*, their default value will be set too. + +If you need to create a key context influencing the behavior of some object, +choice a good name, and eventually prefix it by the name of the module to +isolate its impact. A good example are the keys of ``mail`` module : +*mail_create_nosubscribe*, *mail_notrack*, *mail_notify_user_signature*, ... + + +Do not bypass the ORM +~~~~~~~~~~~~~~~~~~~~~ +You should never use the database cursor directly when the ORM can do the same +thing! By doing so you are bypassing all the ORM features, possibly the +transactions, access rights and so on. + +And chances are that you are also making the code harder to read and probably +less secure. + +.. code-block:: python + + # very very wrong + self.env.cr.execute('SELECT id FROM auction_lots WHERE auction_id in (' + ','.join(map(str, ids))+') AND state=%s AND obj_price > 0', ('draft',)) + auction_lots_ids = [x[0] for x in self.env.cr.fetchall()] + + # no injection, but still wrong + self.env.cr.execute('SELECT id FROM auction_lots WHERE auction_id in %s '\ + 'AND state=%s AND obj_price > 0', (tuple(ids), 'draft',)) + auction_lots_ids = [x[0] for x in self.env.cr.fetchall()] + + # better + auction_lots_ids = self.search([('auction_id','in',ids), ('state','=','draft'), ('obj_price','>',0)]) + + +No SQL injections, please ! +~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Care must be taken not to introduce SQL injections vulnerabilities when using +manual SQL queries. The vulnerability is present when user input is either +incorrectly filtered or badly quoted, allowing an attacker to introduce +undesirable clauses to a SQL query (such as circumventing filters or +executing UPDATE or DELETE commands). + +The best way to be safe is to never, NEVER use Python string concatenation (+) +or string parameters interpolation (%) to pass variables to a SQL query string. + +The second reason, which is almost as important, is that it is the job of the +database abstraction layer (psycopg2) to decide how to format query parameters, +not your job! For example psycopg2 knows that when you pass a list of values +it needs to format them as a comma-separated list, enclosed in parentheses ! + +.. code-block:: python + + # the following is very bad: + # - it's a SQL injection vulnerability + # - it's unreadable + # - it's not your job to format the list of ids + self.env.cr.execute('SELECT distinct child_id FROM account_account_consol_rel ' + + 'WHERE parent_id IN ('+','.join(map(str, ids))+')') + + # better + self.env.cr.execute('SELECT DISTINCT child_id '\ + 'FROM account_account_consol_rel '\ + 'WHERE parent_id IN %s', + (tuple(ids),)) + +This is very important, so please be careful also when refactoring, and most +importantly do not copy these patterns! + +Here is a memorable example to help you remember what the issue is about (but +do not copy the code there). Before continuing, please be sure to read the +online documentation of pyscopg2 to learn of to use it properly: + +- The problem with query parameters (http://initd.org/psycopg/docs/usage.html#the-problem-with-the-query-parameters) +- How to pass parameters with psycopg2 (http://initd.org/psycopg/docs/usage.html#passing-parameters-to-sql-queries) +- Advanced parameter types (http://initd.org/psycopg/docs/usage.html#adaptation-of-python-values-to-sql-types) + + +Keep your methods short/simple when possible +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Functions and methods should not contain too much logic: having a lot of small +and simple methods is more advisable than having few large and complex methods. +A good rule of thumb is to split a method as soon as: +- it has more than one responsibility (see http://en.wikipedia.org/wiki/Single_responsibility_principle) +- it is too big to fit on one screen. + +Also, name your functions accordingly: small and properly named functions are the starting point of readable/maintainable code and tighter documentation. + +This recommendation is also relevant for classes, files, modules and packages. (See also http://en.wikipedia.org/wiki/Cyclomatic_complexity) + + +Never commit the transaction +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +The Odoo framework is in charge of providing the transactional context for +all RPC calls. The principle is that a new database cursor is opened at the +beginning of each RPC call, and committed when the call has returned, just +before transmitting the answer to the RPC client, approximately like this: + +.. code-block:: python + + def execute(self, db_name, uid, obj, method, *args, **kw): + db, pool = pooler.get_db_and_pool(db_name) + # create transaction cursor + cr = db.cursor() + try: + res = pool.execute_cr(cr, uid, obj, method, *args, **kw) + cr.commit() # all good, we commit + except Exception: + cr.rollback() # error, rollback everything atomically + raise + finally: + cr.close() # always close cursor opened manually + return res + +If any error occurs during the execution of the RPC call, the transaction is +rolled back atomically, preserving the state of the system. + +Similarly, the system also provides a dedicated transaction during the execution +of tests suites, so it can be rolled back or not depending on the server +startup options. + +The consequence is that if you manually call ``cr.commit()`` anywhere there is +a very high chance that you will break the system in various ways, because you +will cause partial commits, and thus partial and unclean rollbacks, causing +among others: + +#. inconsistent business data, usually data loss +#. workflow desynchronization, documents stuck permanently +#. tests that can't be rolled back cleanly, and will start polluting the + database, and triggering error (this is true even if no error occurs + during the transaction) + +Here is the very simple rule: + You should **NEVER** call ``cr.commit()`` yourself, **UNLESS** you have + created your own database cursor explicitly! And the situations where you + need to do that are exceptional! + + And by the way if you did create your own cursor, then you need to handle + error cases and proper rollback, as well as properly close the cursor when + you're done with it. + +And contrary to popular belief, you do not even need to call ``cr.commit()`` +in the following situations: +- in the ``_auto_init()`` method of an *models.Model* object: this is taken +care of by the addons initialization method, or by the ORM transaction when +creating custom models +- in reports: the ``commit()`` is handled by the framework too, so you can +update the database even from within a report +- within *models.Transient* methods: these methods are called exactly like +regular *models.Model* ones, within a transaction and with the corresponding +``cr.commit()/rollback()`` at the end +- etc. (see general rule above if you have in doubt!) + +All ``cr.commit()`` calls outside of the server framework from now on must +have an **explicit comment** explaining why they are absolutely necessary, why +they are indeed correct, and why they do not break the transactions. Otherwise +they can and will be removed ! + + +Use translation method correctly +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Odoo uses a GetText-like method named "underscore" ``_( )`` to indicate that +a static string used in the code needs to be translated at runtime using the +language of the context. This pseudo-method is accessed within your code by +importing as follows: + +.. code-block:: python + + from odoo.tools.translate import _ + +A few very important rules must be followed when using it, in order for it to +work and to avoid filling the translations with useless junk. + +Basically, this method should only be used for static strings written manually +in the code, it will not work to translate field values, such as Product names, +etc. This must be done instead using the translate flag on the corresponding +field. + +The rule is very simple: calls to the underscore method should always be in +the form ``_('literal string')`` and nothing else: + +.. code-block:: python + + # good: plain strings + error = _('This record is locked!') + + # good: strings with formatting patterns included + error = _('Record %s cannot be modified!') % record + + # ok too: multi-line literal strings + error = _("""This is a bad multiline example + about record %s!""") % record + error = _('Record %s cannot be modified' \ + 'after being validated!') % record + + # bad: tries to translate after string formatting + # (pay attention to brackets!) + # This does NOT work and messes up the translations! + error = _('Record %s cannot be modified!' % record) + + # bad: dynamic string, string concatenation, etc are forbidden! + # This does NOT work and messes up the translations! + error = _("'" + que_rec['question'] + "' \n") + + # bad: field values are automatically translated by the framework + # This is useless and will not work the way you think: + error = _("Product %s is out of stock!") % _(product.name) + # and the following will of course not work as already explained: + error = _("Product %s is out of stock!" % product.name) + + # bad: field values are automatically translated by the framework + # This is useless and will not work the way you think: + error = _("Product %s is not available!") % _(product.name) + # and the following will of course not work as already explained: + error = _("Product %s is not available!" % product.name) + + # Instead you can do the following and everything will be translated, + # including the product name if its field definition has the + # translate flag properly set: + error = _("Product %s is not available!") % product.name + + +Also, keep in mind that translators will have to work with the literal values +that are passed to the underscore function, so please try to make them easy to +understand and keep spurious characters and formatting to a minimum. Translators +must be aware that formatting patterns such as %s or %d, newlines, etc. need +to be preserved, but it's important to use these in a sensible and obvious manner: + +.. code-block:: python + + # Bad: makes the translations hard to work with + error = "'" + question + _("' \nPlease enter an integer value ") + + # Better (pay attention to position of the brackets too!) + error = _("Answer to question %s is not valid.\n" \ + "Please enter an integer value.") % question + +In general in Odoo, when manipulating strings, prefer ``%`` over ``.format()`` +(when only one variable to replace in a string), and prefer ``%(varname)`` instead +of position (when multiple variables have to be replaced). This makes the +translation easier for the community translators. + + +Symbols and Conventions +----------------------- - Model name (using the dot notation, prefix by the module name) : - - When defining an Odoo Model : use singular form of the name (*res.partner* and *sale.order* instead of *res.partnerS* and *saleS.orderS*) - - When defining an Odoo Transient (wizard) : use ``.`` where *related_base_model* is the base model (defined in *models/*) related to the transient, and *action* is the short name of what the transient do. For instance : ``account.invoice.make``, ``project.task.delegate.batch``, ... - - When defining *report* model (SQL views e.i.) : use ``.report.``, based on the Transient convention. + - When defining an Odoo Model : use singular form of the name (*res.partner* + and *sale.order* instead of *res.partnerS* and *saleS.orderS*) + - When defining an Odoo Transient (wizard) : use ``.`` + where *related_base_model* is the base model (defined in *models/*) related + to the transient, and *action* is the short name of what the transient do. + For instance : ``account.invoice.make``, ``project.task.delegate.batch``, ... + - When defining *report* model (SQL views e.i.) : use + ``.report.``, based on the Transient convention. -- Odoo Python Class : use camelcase for code in api v8 (Object-oriented style), underscore lowercase notation for old api (SQL style). +- Odoo Python Class : use camelcase for code in api v8 (Object-oriented style), + underscore lowercase notation for old api (SQL style). .. code-block:: python @@ -366,7 +806,9 @@ Symbols - Variable name : - use camelcase for model variable - use underscore lowercase notation for common variable. - - since new API works with record or recordset instead of id list, don't suffix variable name with *_id* or *_ids* if they not contain id or list of id. + - since new API works with record or recordset instead of id list, don't + suffix variable name with *_id* or *_ids* if they not contain id or list + of id. .. code-block:: python @@ -382,7 +824,9 @@ Symbols - Default method : the default method pattern is *_default_* - Onchange method : the onchange method pattern is *_onchange_* - Constraint method : the constraint method pattern is *_check_* - - Action method : an object action method is prefix with *action_*. Its decorator is ``@api.multi``, but since it use only one record, add ``self.ensure_one()`` at the beginning of the method. + - Action method : an object action method is prefix with *action_*. Its decorator is + ``@api.multi``, but since it use only one record, add ``self.ensure_one()`` + at the beginning of the method. - In a Model attribute order should be #. Private attributes (``_name``, ``_description``, ``_inherit``, ...) @@ -451,7 +895,10 @@ Javascript and CSS - Use a linter (jshint, ...) - Never add minified Javascript Libraries - Use camelcase for class declaration -- Unless your code is supposed to run on every page, target specific pages using the ``if_dom_contains`` function of website module. Target an element which is specific to the pages your code needs to run on using JQuery. +- Unless your code is supposed to run on every page, target specific pages + using the ``if_dom_contains`` function of website module. Target an + element which is specific to the pages your code needs to run on + using JQuery. .. code-block:: javascript @@ -462,7 +909,11 @@ Javascript and CSS **For CSS :** -- Prefix all your classes with *o_* where *module_name* is the technical name of the module ('sale', 'im_chat', ...) or the main route reserved by the module (for website module mainly, i.e. : 'o_forum' for *website_forum* module). The only exception for this rule is the webclient: it simply uses *o_* prefix. +- Prefix all your classes with *o_* where *module_name* is the + technical name of the module ('sale', 'im_chat', ...) or the main route + reserved by the module (for website module mainly, i.e. : 'o_forum' for + *website_forum* module). The only exception for this rule is the + webclient: it simply uses *o_* prefix. - Avoid using id - Use Bootstrap native classes - Use underscore lowercase notation to name class @@ -480,11 +931,15 @@ Prefix your commit with - **[REF]** for refactoring - **[ADD]** for adding new resources - **[REM]** for removing of resources -- **[MOV]** for moving files (Do not change content of moved file, otherwise Git will loose track, and the history will be lost !), or simply moving code from a file to another one. +- **[MOV]** for moving files (Do not change content of moved file, otherwise + Git will loose track, and the history will be lost !), or simply moving code + from a file to another one. - **[MERGE]** for merge commits (only for forward/back-port) - **[CLA]** for signing the Odoo Individual Contributor License -Then, in the message itself, specify the part of the code impacted by your changes (module name, lib, transversal object, ...) and a description of the changes. +Then, in the message itself, specify the part of the code impacted by +your changes (module name, lib, transversal object, ...) and a description +of the changes. - Always include a meaningful commit message: it should be self explanatory (long enough) including the name of the module that has been changed and the From 3b234a23cb3cf8b659f849c231fa3b2e160213b5 Mon Sep 17 00:00:00 2001 From: Nicolas Lempereur Date: Thu, 19 May 2016 12:03:44 +0200 Subject: [PATCH 19/19] [FIX] web: download file and editor firefox bug When we download a file (as can be done with attachments in Odoo), firefox is opening the link in current window but cancel the action once it is aware the resource is to be downloaded. But this get firefox into a corrupt state for some contenteditable features, for example document.queryCommandState which is needed for the editor will after downloading a file throw an error consistently instead of working as expected. It can be reproduced on firefox (at least from version 32 to 46) with the following snippet: https://jsfiddle.net/s246u3ay/3/ The issue is probably linked to the following reported issue since 2005: https://bugzilla.mozilla.org/show_bug.cgi?id=297494 This commit avoid the bug by opening attachments download links in a new window instead of the current one. closes #12109 opw-676918 --- addons/web/static/src/xml/base.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/addons/web/static/src/xml/base.xml b/addons/web/static/src/xml/base.xml index 6049c64c9c0..ef57881d633 100644 --- a/addons/web/static/src/xml/base.xml +++ b/addons/web/static/src/xml/base.xml @@ -1064,7 +1064,7 @@ - + @@ -1079,7 +1079,7 @@