banking-addons-team team mailing list archive

["Ronald Portier \(Therp\)" <ronald@xxxxxxxx>]

Review: Needs Fixing

Why not replace filter with criteria (for example)? IMHO the trailing underscore is a pain on the eyes.

More importantly, I get the impression that the way the code is changed will result in a great multitude of false positives. Especially when searching for very common names.

Maybe I see it completely wrong, but it seems that on l.26 you empty all criteria, and then on l.46 you are going to search for ALL partners with an address in the same country. Later, on lines 59-62 you filter out all partners with a name (that should be longer then 3 positions) that is not part of the name passed.

I think the search for city and postal code are not disabled in the original code, and should not be in the adapted code.

My apologies in advance if I somehow misread your changes.

One thing more: there should no print statement left in the final code.
-- 
https://code.launchpad.net/~therp-nl/banking-addons/ba61-multicompany_safe_partner_search/+merge/146795
Your team Banking Addons Team is subscribed to branch lp:banking-addons.