955972 - [Fugu][buri][Contact] Search result is not updated after modifying a contact in the search results

Status: RESOLVED DUPLICATE of bug 909201

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

Description

[Peipei Cheng (needinfo if you need my action)]

Environment:
--------------------------
V1.3
Gaia 01e9da49be2cc4bc134eeefc434740d572ec2246
Gecko http://hg.mozilla.org/releases/mozilla-aurora/rev/61f553e5db49
BuildID 20140101004001
Version 28.0a2
ro.build.version.incremental=eng.archermind.20131114.105818
ro.build.date=Thu Nov 14 10:58:33 CST 2013

Steps to reproduce:
---------------------------
0. Create one contact testA
1. Goto Contact
2. Tap the search bar
3. Tap on testA in the search results to open the contact info page
4. Tap on "Edit"
5. Modify this contact's name.
6. Save the modification and return to search result page

Actual results:
---------------------------
TestA on the search result pages is not updated.

Expected results:
---------------------------
The search page should be updated.

Comment 1

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

Is this valid just for fugu or for other devices as well?

Comment 2

[Peipei Cheng (needinfo if you need my action)]

(In reply to Francisco Jordano [:arcturus] from comment #1)
> Is this valid just for fugu or for other devices as well?

This can also be reproduced on Buri. Thanks!

Comment 3

[David Scravaglieri [:scravag]]

tricky to fix it in 1.3 moved to 1.4?

Comment 4

[Noemí Freire (:noemi)]

Triage: an edge case so moving to v1.5?; ni to UX for knowing their point of view here. Thanks!

Comment 5

[ayman maat :maat]

This is something we should address as a priority because it gives incorrect feedback to the end user potentially leading them to conclude that the device has not carried out the task they wish.

**PREREQUISITE**
P1) Contact list contains at least one contact with content in either its first or last name fields 

**PATH**
1) Open contact list
2) Select search to enter search mode 
3) Select contact defined in step P1
4) Select on ‘Edit’
5) Modify the contact's name.
6) Select ‘Done’ to save the contact 
7) Select the back button to return to the search result page

**ACTUAL RESULT**

The name of the contact updated in step 6 is presented in the search results exactly as it was in step 3. This is the point where the system misleads the user. Though in step 7 on the header of the contact detail card prior to selecting the back button and returning to the search list reflects the change made, the entry in the search result does not. Therefore the user is uncertain if the alteration has been committed. 


**SOLUTION**
I would suggest two paths to a solutions here:

S1) ensure that the search results presented after step 7 reflect the change. This is the best solution as it provides flexibility of use if either there were multiple contacts within the search results that the user wished to engage with or the user selected the wrong contact by mistake from the contact list. This is IOS behaviour.

S2) we could explore the possibly of closing the search results when the user selects the contact in step 3. Therefore when the user selects the back button in step 7 they return to the contact list and not the search results. This is android behaviour. I am not as happy with this solution because if the user accidentally selects the wrong contact in step 3 then immediately selects the back button they are presented with the contact list again and therefore have to repeat their search in order to find the contact they want.

Out of the two solutions i would recommend s1.

Comment 6

[Joe Cheng [:jcheng] (please needinfo)]

ni? Francisco on the effort to fix this and how this can be planned along with all the activities planned for contacts app

Comment 7

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

Hei,

we are planning to change the way we do search right now, from DOM based (omg i don't want anyone to read this), to proper (probably indexeddb) search.

We won't have time to do this new search as it's part of the new contact data refactor, to fix this, we could try to do it with current model, so will take it and have a look.

Cheers,
F.

Comment 8

[Joe Cheng [:jcheng] (please needinfo)]

triage: lets fix this in 1.5. thanks

Comment 9

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

I might have a 'eurostar patch' (patch done in the train), still with no tests but probably good to have until we have the proper search that I was commenting before.

Will upload something for Ben Kelly to review soon.

Comment 10

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

Attachment: Pointer to PR 17459

Well, I made a patch based on the 'refresh' method from the contacts list, if we are in search mode we 'invalidate' the search and make it fetch again the nodes to display.

Again I hope this changes with a new search, but will be doing the work for 1.5, also I had to modify two files to remove the jshint linting errors.

Ben would you mind to take a look?

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Francisco, is this a dupe of bug 909201?  I think the patch there might be more straightforward.  It also landed once and just needs to be fixed to use |inSearchMode| instead of |searchEnabled|.  I just got swamped with blockers and lost track of that bug.

Can you help Robert get that fixed in bug 909201?

Comment 12

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

Hi Ben, it's exactly the same issue. It felt under my radar too.

Actually I realised this patch was fixing that too. One quick question, there is a proposed patch in bug 909201 (seriously, this bug reminds me to Beverly Hills 90210), that in github appears as merged.

Comment 13

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

Will mark this as a duplicate, and at least we have the patch and the unit tests.

Actually, this patch looks better than the previous one, IMHO (damn it I cannot say that since I created the patch)

Comment 14

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Francisco Jordano [:arcturus] from comment #12)
> Actually I realised this patch was fixing that too. One quick question,
> there is a proposed patch in bug 909201 (seriously, this bug reminds me to
> Beverly Hills 90210), that in github appears as merged.

Yes, it was merged, but then backed-out due to bustage.  The bustage occurred because that patch checks |searchEnabled| instead of |inSearchMode|.