after a quick review, i noticed following things that should be checked/fixed/discussed
1) line 104: the method _merge_find_oldest() returns a browse record, not an int + the code should be a bit more robust. What if ids == [] ? what if opportunity_ids == [] ? 2) line 132 and 177: list of fields is identical, could be define as a global variable. 3) line 177: categ_ids is given to _merge_data() but the docstring of this function seems to say to *2many fields aren't process... that seems doubtful 4) _merge_notification: actually it, if i'm not wrong, creates one notification to say that 2 leads have been merged... it doesn't merge notification as the function name tends to say. A docstring documenting the code (+ _mail_body) would also be welcome. 5) line 229: in allocate_salesman() you could set the default value of user_ids to [] directly instead of None and then later on transform it to []. 6) line 277 (278,298;299): - form_view = models_data.get_object_reference(cr, uid, 'crm', 'crm_case_form_view_oppor') + dummy, form_view = models_data.get_object_reference(cr, uid, 'crm', 'crm_case_form_view_oppor') this gives you more explicitely the type of form_view variable. 7) line 533: implement the TODO statement? 8) line 1343: 'do stuff'? i guess you meant '#TODO: do stuff'... :-)... cfr comment 7) -- https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-improve_crm_leads_flux-abo/+merge/133937 Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-addons/trunk-improve_crm_leads_flux-abo. _______________________________________________ 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
