952554 - [Messages] We don't find contacts for multiple terms search with some parts of the phone number

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Julien Wajsberg [:julienw]]

STR:
* have a contact with "givenName: Julien", "familyName: Foo", a phone number "0123456789"
* launch the Messages app
* start a new message
* search for "julien 234"

Expected:
* we find the contact

Actual:
* we don't

The issue is that "julien" is the predominate term, and we look for it using mozContact. Then we try to match "234", but using the "startsWith" filter whereas phone number should be looked up with a "contains" filter instead.

To fix this, we should have a map "field name" => "filter function" instead of passing the filter function as argument. Except we still need an argument to distinguish exact vs contains :)

So there is some work, not so difficult, but some work :)

Adding the "mentor" flag because this can be fixed using only the unit tests.

Comment 1

[Julien Wajsberg [:julienw]]

Cc Patrick, who worked a little bit on this area lately.

Remember we are in holiday until the start of January :)

Comment 2

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Pull Request #15823 - Find contact with multi-term query which contains tel

The initial work that was required for this was done by the patch for bug 959839, the only things left were some minor adjustments for the matching (i.e. to also look within the 'tel' field).

Comment 3

[Julien Wajsberg [:julienw]]

Comment on attachment 8367710 [details] [review]
Pull Request #15823 - Find contact with multi-term query which contains tel

Added comments and questions on the bug, keeping my review flag until this is answered

Comment 4

[Julien Wajsberg [:julienw]]

Comment on attachment 8367710 [details] [review]
Pull Request #15823 - Find contact with multi-term query which contains tel

Per github, removing the review flag until the highlight works. Please request review once you're ready!

If you change your mind and rather do this in another bug, then feel free to request review again.

Comment 5

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Comment on attachment 8367710 [details] [review]
Pull Request #15823 - Find contact with multi-term query which contains tel

Patch updated to fix highlighting for the tel number too. Thanks for the feedback, Julien! Let me know if it looks good now.

Comment 6

[Julien Wajsberg [:julienw]]

Comment on attachment 8367710 [details] [review]
Pull Request #15823 - Find contact with multi-term query which contains tel

r=me

thanks, works perfectly!

Comment 7

[Julien Wajsberg [:julienw]]

master: d47fbce3b912b88916f38dc0ca20fea35af0cf55