888465 - contacts search and ordering changed after bug 887564

Status: RESOLVED FIXED

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

Description

[Ben Kelly [:bkelly, not reviewing]]

While trying to merge my fix for bug 887564 into a work-in-progress I realized that I had inadvertently created a new bug.

The fix for bug 887564 changes the code so that givenName is not modified on the contacts object.  Unfortunately, though, the order and search strings are calculated after this modification was taking place.  So the fix for bug 887564 inadvertently changed how ordering and search is done in these cases.

For example, if a contact does not have a telephone number, org, or email then its name is displayed as "noName".  Previously, these contacts could be searched for as "noName", but now they cannot.  Also, they used to sort as "noName", but now they sort as "#" instead.

To be honest, all of this behavior seems odd to me and I'm not sure if it was intentional or not.  In any case, I will put together a pull request to restore it so we can turn it around quickly.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10703

Pointer to Github pull-request

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 769147 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10703

This should fix the regression.

Note, I can't seem to run the unit tests in the browser at the moment.  I did test on my phone as well as I could.  We need to check travis results on the PR.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Tests pass on my local machine.  Looking at why they fail on travis.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

The travis test issue is captured in bug 888498.  I propose a fix over there since its not specific to the search/ordering issue here.

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 769147 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10703

Tried on the phone and working well.

Just one question, can we add a unit test to check this?

Thanks!
F.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Francisco Jordano [:arcturus] from comment #5)
> Just one question, can we add a unit test to check this?

Yea, that's a good idea.  This is definitely a corner case that could break easily in the future as well.  I'll do that this morning.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Ok, I added the tests for ordering and searching on empty contacts.  I also did some other test cleanup that I noticed while I was in there.  (Should I have broken that out into a separate bug/PR?)

https://github.com/mozilla-b2g/gaia/commit/aa1798207a9fc9e65cd58284ce561dc031a4c41b