From f6207124f94aa2f02b138324db904773cc71f41c Mon Sep 17 00:00:00 2001 From: Anael Closson Date: Mon, 3 Feb 2014 13:58:54 +0100 Subject: [PATCH 1/8] [FIX] OPW 598942 : orm read_group doesn't take referenced fields or aggregated fields in account when sorting Sorting was done using a search on ids that where found in a custom SQL field, only 1 record among aggregated records with same groupby value was used when using search for ordering, resulting data ordered on max(aggregated_data).field_value instead of sum(aggregated_data.field_value). lp bug: https://launchpad.net/bugs/106086 fixed bzr revid: acl@openerp.com-20140203125854-ypi0bu0lbhatg9b1 --- openerp/osv/orm.py | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index e7121bb4635..3b57a66f877 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -2678,6 +2678,16 @@ class BaseModel(object): qualified_field = '"%s"."%s"' % (self._table, f) flist += "%s(%s) AS %s" % (group_operator, qualified_field, f) + orderby_clause = "" + if orderby: + orderby_list = [] + order_splits = [o.strip().split(' ') for o in orderby.split(',')] + for order_split in order_splits: + if order_split[0] in aggregated_fields or order_split[0] == groupby: + orderby_list += [(order_split[0], (len(order_split) == 2 and order_split[1].strip() == 'DESC') and 'DESC' or 'ASC')] + if orderby_list: + orderby_clause = 'ORDER BY %s' % (','.join([ '%s %s' % (order[0], order[1]) for order in orderby_list])) + gb = groupby and (' GROUP BY ' + qualified_groupby_field) or '' from_clause, where_clause, where_clause_params = query.get_sql() @@ -2686,28 +2696,34 @@ class BaseModel(object): offset_str = offset and ' offset %d' % offset or '' if len(groupby_list) < 2 and context.get('group_by_no_leaf'): group_count = '_' - cr.execute('SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist + ' FROM ' + from_clause + where_clause + gb + limit_str + offset_str, where_clause_params) + cr.execute('SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist + ' FROM ' + from_clause + where_clause + gb + orderby_clause + limit_str + offset_str, where_clause_params) alldata = {} groupby = group_by - for r in cr.dictfetchall(): + + fetched_data = cr.dictfetchall() + + # we need to keep order of previous search to properly order data on aggregated fields + order = orderby or groupby + data_ids = [data['id'] for data in fetched_data] + + if order and order.split(',')[0].replace(' ASC', '').replace(' DESC', '') not in aggregated_fields: + data_ids = self.search(cr, uid, [('id', 'in', data_ids)], order=order, context=context) + # the IDs of records that have groupby field value = False or '' should be included too + data_ids += set(alldata.keys()).difference(data_ids) + + for r in fetched_data: for fld, val in r.items(): if val is None: r[fld] = False alldata[r['id']] = r del r['id'] - order = orderby or groupby - data_ids = self.search(cr, uid, [('id', 'in', alldata.keys())], order=order, context=context) - - # the IDs of records that have groupby field value = False or '' should be included too - data_ids += set(alldata.keys()).difference(data_ids) - - if groupby: + if groupby: data = self.read(cr, uid, data_ids, [groupby], context=context) # restore order of the search as read() uses the default _order (this is only for groups, so the footprint of data should be small): data_dict = dict((d['id'], d[groupby] ) for d in data) result = [{'id': i, groupby: data_dict[i]} for i in data_ids] else: - result = [{'id': i} for i in data_ids] + result = [{'id': i} for i in data_ids] for d in result: if groupby: @@ -2717,7 +2733,7 @@ class BaseModel(object): d['__context'] = {'group_by': groupby_list[1:]} if groupby and groupby in fget: if d[groupby] and fget[groupby]['type'] in ('date', 'datetime'): - dt = datetime.datetime.strptime(alldata[d['id']][groupby][:7], '%Y-%m') + dt = datetime.datetime.strptime1(alldata[d['id']][groupby][:7], '%Y-%m') days = calendar.monthrange(dt.year, dt.month)[1] date_value = datetime.datetime.strptime(d[groupby][:10], '%Y-%m-%d') From 30d22d327ef4518ec9628fd1c22b149bd72d28c0 Mon Sep 17 00:00:00 2001 From: Anael Closson Date: Wed, 5 Feb 2014 13:52:08 +0100 Subject: [PATCH 2/8] [FIX] typo, wrong ordering of lines bzr revid: acl@openerp.com-20140205125208-8ds2hxfqnuqsngbd --- openerp/osv/orm.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 3b57a66f877..3e43523a029 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -2706,17 +2706,17 @@ class BaseModel(object): order = orderby or groupby data_ids = [data['id'] for data in fetched_data] - if order and order.split(',')[0].replace(' ASC', '').replace(' DESC', '') not in aggregated_fields: - data_ids = self.search(cr, uid, [('id', 'in', data_ids)], order=order, context=context) - # the IDs of records that have groupby field value = False or '' should be included too - data_ids += set(alldata.keys()).difference(data_ids) - for r in fetched_data: for fld, val in r.items(): if val is None: r[fld] = False alldata[r['id']] = r del r['id'] + if order and order.split(',')[0].replace(' ASC', '').replace(' DESC', '') not in aggregated_fields: + data_ids = self.search(cr, uid, [('id', 'in', data_ids)], order=order, context=context) + # the IDs of records that have groupby field value = False or '' should be included too + data_ids += set(alldata.keys()).difference(data_ids) + if groupby: data = self.read(cr, uid, data_ids, [groupby], context=context) # restore order of the search as read() uses the default _order (this is only for groups, so the footprint of data should be small): @@ -2733,7 +2733,7 @@ class BaseModel(object): d['__context'] = {'group_by': groupby_list[1:]} if groupby and groupby in fget: if d[groupby] and fget[groupby]['type'] in ('date', 'datetime'): - dt = datetime.datetime.strptime1(alldata[d['id']][groupby][:7], '%Y-%m') + dt = datetime.datetime.strptime(alldata[d['id']][groupby][:7], '%Y-%m') days = calendar.monthrange(dt.year, dt.month)[1] date_value = datetime.datetime.strptime(d[groupby][:10], '%Y-%m-%d') From 5ef7c4a6a1838a3821ee1bcf14d93d0203e9fbc1 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Mon, 17 Feb 2014 11:49:20 +0100 Subject: [PATCH 3/8] [FIX] getParent() js error when clicking to fast on menu items that load a list view after loading a form view bzr revid: cto@openerp.com-20140217104920-fu8hcqd51bh7qnrp --- addons/web/static/src/js/view_list.js | 7 +++++-- addons/web/static/src/js/views.js | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/addons/web/static/src/js/view_list.js b/addons/web/static/src/js/view_list.js index f3ecc373ca5..5bee82c28d2 100644 --- a/addons/web/static/src/js/view_list.js +++ b/addons/web/static/src/js/view_list.js @@ -1623,9 +1623,12 @@ instance.web.ListView.Groups = instance.web.Class.extend( /** @lends instance.we function synchronized(fn) { var fn_mutex = new $.Mutex(); return function () { + var obj = this; var args = _.toArray(arguments); - args.unshift(this); - return fn_mutex.exec(fn.bind.apply(fn, args)); + return fn_mutex.exec(function () { + if (obj.isDestroyed()) { return $.when(); } + return fn.apply(obj, args) + }); }; } var DataGroup = instance.web.Class.extend({ diff --git a/addons/web/static/src/js/views.js b/addons/web/static/src/js/views.js index 8d04110885c..875b113a82f 100644 --- a/addons/web/static/src/js/views.js +++ b/addons/web/static/src/js/views.js @@ -1301,7 +1301,7 @@ instance.web.View = instance.web.Widget.extend({ "context": this.dataset.get_context(), }); } - return view_loaded_def.then(function(r) { + return this.alive(view_loaded_def).then(function(r) { self.fields_view = r; // add css classes that reflect the (absence of) access rights self.$el.addClass('oe_view') From ac88131405e34873de7aef9f2f7d615c65a5c134 Mon Sep 17 00:00:00 2001 From: Anael Closson Date: Mon, 17 Feb 2014 15:01:44 +0100 Subject: [PATCH 4/8] [IMP] Speed and quality improvement - removing the need of the use of search when groupby is set on a relation field. - creation and use of dedicated helper method to compute the orderby clause for easier reading bzr revid: acl@openerp.com-20140217140144-2rm3o00g76tyhqxn --- openerp/osv/orm.py | 54 ++++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 3e43523a029..8ea3568a65c 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -2599,6 +2599,32 @@ class BaseModel(object): r['__fold'] = folded.get(r[groupby] and r[groupby][0], False) return result + def _read_group_generate_order_by(self, orderby, aggregated_fields, groupby, query): + """ + Generates the ORDER BY sql clause for the read group method. Adds the missing JOIN clause + to the query if order should be computed against m2o field. + :param orderby: the orderby definition in the form "%(field)s %(order)s" + :param aggregated_fields: list of aggregated fields in the query + :param groupby: the current groupby field name + :param query: the query object used to construct the query afterwards + """ + orderby_list = [] + ob = '' + for order_splits in orderby.split(','): + order_split = order_splits.split() + orderby_field = order_split[0] + orderby_dir = len(order_split) == 2 and order_split[1].upper() == 'ASC' and 'ASC' or 'DESC' + if orderby_field == groupby: + ob = self._generate_order_by(order_splits, query).replace('ORDER BY ', '') + orderby_list.append(ob) + elif orderby_field in aggregated_fields: + orderby_list.append('%s %s' % (orderby_field,orderby_dir)) + + if orderby_list: + return ' ORDER BY %s' % (','.join(orderby_list)), ob + else: + return '', ob + def read_group(self, cr, uid, domain, fields, groupby, offset=0, limit=None, context=None, orderby=False): """ Get the list of records in list view grouped by the given ``groupby`` fields @@ -2678,15 +2704,11 @@ class BaseModel(object): qualified_field = '"%s"."%s"' % (self._table, f) flist += "%s(%s) AS %s" % (group_operator, qualified_field, f) - orderby_clause = "" - if orderby: - orderby_list = [] - order_splits = [o.strip().split(' ') for o in orderby.split(',')] - for order_split in order_splits: - if order_split[0] in aggregated_fields or order_split[0] == groupby: - orderby_list += [(order_split[0], (len(order_split) == 2 and order_split[1].strip() == 'DESC') and 'DESC' or 'ASC')] - if orderby_list: - orderby_clause = 'ORDER BY %s' % (','.join([ '%s %s' % (order[0], order[1]) for order in orderby_list])) + order = orderby or groupby + orderby_clause = '' + ob = '' + if order: + orderby_clause, ob = self._read_group_generate_order_by(order, aggregated_fields, groupby, query) gb = groupby and (' GROUP BY ' + qualified_groupby_field) or '' @@ -2696,27 +2718,21 @@ class BaseModel(object): offset_str = offset and ' offset %d' % offset or '' if len(groupby_list) < 2 and context.get('group_by_no_leaf'): group_count = '_' - cr.execute('SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist + ' FROM ' + from_clause + where_clause + gb + orderby_clause + limit_str + offset_str, where_clause_params) + query = 'SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist + ' FROM ' + from_clause + where_clause + gb + (ob and ',') + ob + orderby_clause + limit_str + offset_str + cr.execute(query , where_clause_params) alldata = {} groupby = group_by fetched_data = cr.dictfetchall() - # we need to keep order of previous search to properly order data on aggregated fields - order = orderby or groupby - data_ids = [data['id'] for data in fetched_data] - + data_ids = [] for r in fetched_data: for fld, val in r.items(): if val is None: r[fld] = False alldata[r['id']] = r + data_ids.append(r['id']) del r['id'] - if order and order.split(',')[0].replace(' ASC', '').replace(' DESC', '') not in aggregated_fields: - data_ids = self.search(cr, uid, [('id', 'in', data_ids)], order=order, context=context) - # the IDs of records that have groupby field value = False or '' should be included too - data_ids += set(alldata.keys()).difference(data_ids) - if groupby: data = self.read(cr, uid, data_ids, [groupby], context=context) # restore order of the search as read() uses the default _order (this is only for groups, so the footprint of data should be small): From f4f6bc3d9a5c42a3ad17f92c84f3dde47ac0e1f4 Mon Sep 17 00:00:00 2001 From: Anael Closson Date: Mon, 17 Feb 2014 15:07:05 +0100 Subject: [PATCH 5/8] [REF] removing debug code bzr revid: acl@openerp.com-20140217140705-b2erer6w4mxba3l7 --- openerp/osv/orm.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 8ea3568a65c..e9dfbbbcbcb 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -2718,8 +2718,7 @@ class BaseModel(object): offset_str = offset and ' offset %d' % offset or '' if len(groupby_list) < 2 and context.get('group_by_no_leaf'): group_count = '_' - query = 'SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist + ' FROM ' + from_clause + where_clause + gb + (ob and ',') + ob + orderby_clause + limit_str + offset_str - cr.execute(query , where_clause_params) + cr.execute('SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist + ' FROM ' + from_clause + where_clause + gb + (ob and ',') + ob + orderby_clause + limit_str + offset_str, where_clause_params) alldata = {} groupby = group_by From 775ae7d728abbeb0ee25330eb9b6fad145133396 Mon Sep 17 00:00:00 2001 From: Anael Closson Date: Tue, 18 Feb 2014 09:30:45 +0100 Subject: [PATCH 6/8] [REF] removing useless comment bzr revid: acl@openerp.com-20140218083045-aimu376msu052k5d --- openerp/addons/base/tests/test_base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openerp/addons/base/tests/test_base.py b/openerp/addons/base/tests/test_base.py index 5e6ff787a2d..edf2ec2e3f7 100644 --- a/openerp/addons/base/tests/test_base.py +++ b/openerp/addons/base/tests/test_base.py @@ -292,7 +292,6 @@ class test_base(common.TransactionCase): {'name': 'Nab', 'login': 'nab', 'color': 2, 'function': '5$ Wrench'}, ]: self.res_users.create(cr, uid, user_data) - #groupby='name',order='name DESC, color ASC' groups_data = self.res_users.read_group(cr, uid, domain=[('login', 'in', ('alice', 'bob', 'eve'))], fields=['name', 'color', 'function'], groupby='function') self.assertEqual(len(groups_data), 2, "Incorrect number of results when grouping on a field") From 2e831ab7c377408782ab96c303f6fa50884d76e4 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 18 Feb 2014 11:14:38 +0100 Subject: [PATCH 7/8] [IMP] data: skip read_ids call when got no id (opw 604099). This avoids making useless calls to to read the values of o2m fields when the read on a record already told us there was no ids. bzr revid: mat@openerp.com-20140218101438-yqim33cvtg6wt4mw --- addons/web/static/src/js/data.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/addons/web/static/src/js/data.js b/addons/web/static/src/js/data.js index abfb4c15347..04b02294463 100644 --- a/addons/web/static/src/js/data.js +++ b/addons/web/static/src/js/data.js @@ -428,6 +428,9 @@ instance.web.DataSet = instance.web.Class.extend(instance.web.PropertiesMixin, * @returns {$.Deferred} */ read_ids: function (ids, fields, options) { + if (_.isEmpty(ids)) + return $.Deferred().resolve([]); + options = options || {}; // TODO: reorder results to match ids list return this._model.call('read', From 871b2dc73dec604282970b0cb0ac700ca3e4bc74 Mon Sep 17 00:00:00 2001 From: Launchpad Translations on behalf of openerp <> Date: Wed, 19 Feb 2014 05:39:40 +0000 Subject: [PATCH 8/8] Launchpad automatic translations update. bzr revid: launchpad_translations_on_behalf_of_openerp-20140219053940-dvi4g7ajttoj6h9k --- openerp/addons/base/i18n/fi.po | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openerp/addons/base/i18n/fi.po b/openerp/addons/base/i18n/fi.po index 85f5c2bf1b8..300b24bac3c 100644 --- a/openerp/addons/base/i18n/fi.po +++ b/openerp/addons/base/i18n/fi.po @@ -8,13 +8,13 @@ msgstr "" "Project-Id-Version: openobject-server\n" "Report-Msgid-Bugs-To: FULL NAME \n" "POT-Creation-Date: 2013-06-07 19:35+0000\n" -"PO-Revision-Date: 2014-02-15 12:52+0000\n" +"PO-Revision-Date: 2014-02-18 15:52+0000\n" "Last-Translator: Harri Luuppala \n" "Language-Team: Finnish \n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" -"X-Launchpad-Export-Date: 2014-02-16 06:41+0000\n" +"X-Launchpad-Export-Date: 2014-02-19 05:39+0000\n" "X-Generator: Launchpad (build 16916)\n" #. module: base @@ -10408,7 +10408,7 @@ msgstr "" #: view:res.groups:0 #: field:res.partner,comment:0 msgid "Notes" -msgstr "Huomautukset" +msgstr "Muistiinpanot" #. module: base #: field:ir.config_parameter,value:0 @@ -15940,7 +15940,7 @@ msgstr "" #. module: base #: view:res.partner:0 msgid "Internal Notes" -msgstr "Sisäiset huomautukset" +msgstr "Sisäiset muistiinpanot" #. module: base #: model:res.partner.title,name:base.res_partner_title_pvt_ltd