DedupeFind: Treat zip search input strings as prefix

[niels-heinemann]

When filtering by postal code it's more intuitive to treat the search input as a prefix, i.e. like a startsWith request.
See CRM-18694
Depends on PR #8463

[JoeMurray]

@yurg would you be interested in reviewing this PR?

[yurg]

@JoeMurray
Why not, one thing though: will you please guide me through a proper review process, since it will be the 1st time I'm doing this. Thanks.

[JoeMurray]

Thanks again for your help. Sure I'll try to guide you. Check out the Review Tips and FAQ tabs as well as Overview of https://docs.google.com/spreadsheets/d/1fmHFOZ83ectSPCMWvXwCeJgrw4KpYi8JEV2ZDkd6XDU/edit#gid=616660117. Then ask questions in dev channel of https://chat.civicrm.org.

[eileenmcnaughton]

Actually this is dependent on the other PR I'm looking at - but @yurg we can review them both together.

This is the other PR

#8463

So, the questions we need to answer as reviewers are

1. does this address a valid issue
2. is it safe from a security point of view (that question is more relevant to the other PR)
3. does it have performance implications (ditto)
4. does it work
5. does it change behaviour in a way that not everyone might agree with
6. is it safe from a code standpoint - which has a few sub-questions

• is the code tested
• does it increase test coverage and if not is it increasing code complexity (we should never increase code complexity without increasing test coverage)
• does it affect multiple code flows and if so has the impact of them been considered
• does it alter function signature on classes like the CRM_Core_Payment class or CRM_Report_Form class that we expect extensions to override & is it likely to impact on them (changing the signature of an overridden function can hurt classes that extend them).
  7) is the coding approach good, does it increase or reduce complexity.

[JoeMurray]

@eileenmcnaughton just FYI I cribbed your response and worked it into the Reviewer Tips tab. Thanks!!

[eileenmcnaughton]

yeah - we need to put up a blog - I had hoped one of the sprinters would - not sure if there is some doco that they DID record

[eileenmcnaughton]

OK an update on this - in the process of fixing https://issues.civicrm.org/jira/browse/CRM-18577 I have done the much -overdue re-write of the function this relies on change to. So, I guess I will have to recut this.

@yurg if you feel up for reviewing a different PR you could try this one #8266 - it's fairly simple - just a case of making sure my logic is good

[eileenmcnaughton]

@yurg You could also take a look at testing #8456 ... it could probably do with a couple of eyes-on

[eileenmcnaughton]

closing per discussion on #8463