Impacted versions

Tested in 15.0 but the same buggy code

• odoo/addons/pos_restaurant/models/pos_order.py

  Line 96 in bd366a6

  for order_id, order_lines in groupby(extended_order_lines, key=lambda x:x[2]['order_id']):

Found for older versions

13.0

• odoo/addons/pos_restaurant/models/pos_order.py

  Line 84 in 4344d39

  for order_id, order_lines in groupby(extended_order_lines, key=lambda x:x[2]['order_id']):

14.0

• odoo/addons/pos_restaurant/models/pos_order.py

  Line 91 in c1bfba9

  for order_id, order_lines in groupby(extended_order_lines, key=lambda x:x[2]['order_id']):

Steps to reproduce

Check the following code and output

from itertools import groupby


extended_order_lines = [
    [0, 0, {"order_id": (16433, "/"), "qty": 1.0}],
    [0, 0, {"order_id": (16434, "/"), "qty": 1.0}],
    [0, 0, {"order_id": (16433, "/"), "qty": 1.0}],
    [0, 0, {"order_id": (16434, "/"), "qty": 1.0}],
]

extended_order_lines_sorted = sorted(extended_order_lines, key=lambda x:x[2]['order_id'][0])

groupby_res = groupby(extended_order_lines, key=lambda x:x[2]['order_id'][0])
groupby_res_list = [(order_id, len(list(order_lines))) for order_id, order_lines in groupby_res]
print(groupby_res_list)
# The output is: [(16433, 1), (16434, 1), (16433, 1), (16434, 1)]

groupby_res_sorted = groupby(extended_order_lines_sorted, key=lambda x:x[2]['order_id'][0])
groupby_res_list_sorted = [(order_id, len(list(order_lines))) for order_id, order_lines in groupby_res_sorted]
print(groupby_res_list_sorted)
# The output is: [(16433, 2), (16434, 2)]

Current behavior

Notice using the same data but changing the order the output is different

Notice the first unordered output:

• [(16433, 1), (16434, 1), (16433, 1), (16434, 1)]

Neither the id 16433 was not grouped nor the id 16434 (in fact the same key is duplicated)

It is expected for python since the documentation says:

• Generally, the iterable needs to already be sorted on the same key function.

But it is not expected for odoo where it is used.

The code above is based on:

• odoo/addons/pos_restaurant/models/pos_order.py

  Lines 94 to 95 in 230f1b4

  	for order_id, order_lines in groupby(extended_order_lines, key=lambda x:x[2]['order_id']):
  	next(order for order in orders if order['id'] == order_id[0])['lines'] = list(order_lines)

Analyzing these lines of code:

for order_id, order_lines in groupby(extended_order_lines, key=lambda x:x[2]['order_id']):
    next(order for order in orders if order['id'] == order_id[0])['lines'] = list(order_lines)

If the extended_order_lines are not ordered with the key (or maybe yes, depending on your luck) so the result will be missing lines since it is assigning order_lines for the first item found for each order_id so the second item is ignored

Raising flaky errors losing data

Expected behavior

No loose data

No odoo logic calling groupby without sorting consideration

We can fix it in different ways:

Fixing current odoo state

We can generate a PR to Odoo for all the lines of code in odoo using this way, to make sure a sorted based on key generator is sent to groupby method

It is the more logical solution in the short term but what about if a future developer does the same wrong way

Deprecate iterators.groupby

We can deprecateiterators.groupby method from lint here:

• odoo/odoo/addons/test_lint/tests/test_pylint.py

  Lines 38 to 40 in 87698d9

  	BAD_FUNCTIONS = [
  	'input',
  	]

In pro to use odoo.tools.groupby method:

• odoo/odoo/tools/misc.py

  Lines 1168 to 1173 in bd366a6

  	def groupby(iterable, key=None):
  	""" Return a collection of pairs ``(key, elements)`` from ``iterable``. The
  	``key`` is a function computing a key value for each element. This
  	function is similar to ``itertools.groupby``, but aggregates all
  	elements under the same key, not only consecutive elements.
  	"""

Other solutions?
I have created this issue in order to discuss the best way to fix this flaky error in a global and standard way in order to avoid reproducing it in the future again and again for other pieces of code

It was so hard for us to detect, reproduced then justify the fix

Using an OPW ticket the team will request us to reproduce in runbot functionally but it is not easy since it is hard to reproduce because it is flaky

But technically is easier to realize where the issue is

We have customers losing data (e.g. pos order lines) because of this

The places calling itertools.groupby methods:

• grep -rn " groupby(\|\.groupby(" odoo enterprise --include="*.py" --exclude-dir="test*" |grep -v "E\.groupby"

https://github.com/odoo/odoo/tree/a138875b/addons/account/wizard/account_automatic_entry_wizard.py#L221
https://github.com/odoo/odoo/tree/a138875b/addons/lunch/populate/lunch.py#L38
https://github.com/odoo/odoo/tree/a138875b/addons/point_of_sale/models/pos_order.py#L1111
https://github.com/odoo/odoo/tree/a138875b/addons/point_of_sale/models/product.py#L83
https://github.com/odoo/odoo/tree/a138875b/addons/point_of_sale/models/stock_picking.py#L124
https://github.com/odoo/odoo/tree/a138875b/addons/point_of_sale/models/stock_picking.py#L89
https://github.com/odoo/odoo/tree/a138875b/addons/pos_restaurant/models/pos_order.py#L132
https://github.com/odoo/odoo/tree/a138875b/addons/pos_restaurant/models/pos_order.py#L47
https://github.com/odoo/odoo/tree/a138875b/addons/pos_restaurant/models/pos_order.py#L94
https://github.com/odoo/odoo/tree/a138875b/addons/purchase_stock/models/stock_rule.py#L120
https://github.com/odoo/odoo/tree/a138875b/addons/purchase_stock/models/stock_rule.py#L205
https://github.com/odoo/odoo/tree/a138875b/addons/purchase/models/purchase.py#L544
https://github.com/odoo/odoo/tree/a138875b/addons/sale/models/sale_order.py#L784
https://github.com/odoo/odoo/tree/a138875b/addons/stock/models/stock_move.py#L1058
https://github.com/odoo/odoo/tree/a138875b/addons/stock/models/stock_move.py#L1421
https://github.com/odoo/odoo/tree/a138875b/addons/stock/models/stock_move.py#L1446
https://github.com/odoo/odoo/tree/a138875b/addons/stock/models/stock_move.py#L1451
https://github.com/odoo/odoo/tree/a138875b/addons/stock/models/stock_package_level.py#L182
https://github.com/odoo/odoo/tree/a138875b/addons/stock/models/stock_package_level.py#L186
https://github.com/odoo/odoo/tree/a138875b/addons/stock/models/stock_picking.py#L1218
https://github.com/odoo/odoo/tree/a138875b/addons/stock/models/stock_picking.py#L890
https://github.com/odoo/odoo/tree/a138875b/addons/stock/models/stock_picking.py#L894
https://github.com/odoo/odoo/tree/a138875b/addons/web/controllers/main.py#L1754
https://github.com/odoo/odoo/tree/a138875b/addons/website_blog/controllers/main.py#L57
https://github.com/odoo/odoo/tree/a138875b/odoo/addons/base/models/ir_model.py#L2153
https://github.com/odoo/odoo/tree/a138875b/odoo/addons/base/models/ir_model.py#L2153
https://github.com/odoo/odoo/tree/a138875b/odoo/addons/base/models/ir_translation.py#L896
https://github.com/odoo/odoo/tree/a138875b/odoo/addons/base/models/ir_translation.py#L896

https://github.com/odoo/enterprise/tree/9988b9aae/account_asset/report/account_assets_report.py#L133
https://github.com/odoo/enterprise/tree/9988b9aae/hr_holidays_gantt/models/hr_leave.py#L68
https://github.com/odoo/enterprise/tree/9988b9aae/l10n_be_reports/models/partner_vat_listing.py#L189
https://github.com/odoo/enterprise/tree/9988b9aae/l10n_cl_edi_boletas/models/l10n_cl_daily_sales_book.py#L70
https://github.com/odoo/enterprise/tree/9988b9aae/l10n_de_pos_res_cert/models/pos_order.py#L98
https://github.com/odoo/enterprise/tree/9988b9aae/l10n_in_reports/report/account_gst_report.py#L229
https://github.com/odoo/enterprise/tree/9988b9aae/l10n_lu_reports/models/account_general_ledger.py#L110
https://github.com/odoo/enterprise/tree/9988b9aae/pos_blackbox_be/models/pos_blackbox_be.py#L320
https://github.com/odoo/enterprise/tree/9988b9aae/pos_hr_l10n_be/models/hr_employee.py#L88
https://github.com/odoo/enterprise/tree/9988b9aae/sale_subscription_dashboard/models/sale_subscription.py#L280

UPDATED: I just found a blog about this matter:

• https://sharats.me/posts/python-itertools-groupby-callable/