Review: Needs Fixing Hello,
On a functional aspect this change looks fine to me. I double-checked our addons and there is no place where we put prodlot_id: False value in the context for get_product_available's, except here. Therefore we can safely introduce this new semantics: if prodlot_id is False in context, it means we're explicitly looking for products that have *no* production lot. When we mean to count all products regardless of their production lot, the key should be omitted instead. I also agree with Lionel's remarks: 1. We could really use an extra test to cover this fix. It should be as simple as adding 1-2 extra steps at the end of stock/test/opening_stock.yml to perform an extra product qty change with Ice Cream and prod_lot_id = False. The result before and after the fix should be different, so you can assert that the final stock quantity for Ice Cream is correct. 2. I don't think the change on l.32-34 is needed, appropriate exceptions will be triggered when confirming the stock moves anyway, if the production lot tracking conditions are not met, so this change is unnecessary. Now one small technical remark on the patch: - l.9 and 19: the code is becoming quite unreadable due to this overlong inline condition. Please make it a separate variable (e.g. prodlot_condition) that is computed above the call to cr.execute() and inserted in the query string. In fact you need to update your code with the latest 6.1 changes, as the code was refactored a little bit. Thanks! -- https://code.launchpad.net/~openerp-dev/openobject-addons/6.1-opw-575875-rha/+merge/110601 Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-addons/6.1-opw-575875-rha. _______________________________________________ Mailing list: https://launchpad.net/~openerp-dev-gtk Post to : [email protected] Unsubscribe : https://launchpad.net/~openerp-dev-gtk More help : https://help.launchpad.net/ListHelp
