883756 - [Call log] Tapping on an entry from a contact does not take to that contact's detail view screen but to a different one

Status: RESOLVED FIXED

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

Description

[isabelrios]

Leo device, testrun3 build:

Gaia: ea18de8
Test case id: #5786

PROCEDURE
Create several contacts starting with for example A; Antonio, Ana, Ap
1. From contact Ap receive a missed call
2. Tap on the missed call to go to the contact detail view

EXPECTED
Ap contact's detail view is displayed

ACTUAL
The first time a call is received from contact Ap, tapping on it, the device opens another contact name also starting by A but not the correct one.

It can be reproduced easily the first time dialer app is used.

Comment 1

[Germán Toro del Valle (:gtorodelvalle)]

Attachment: Associated PR.

We were not passing the contact's id from handled_call.js to call_log.js and this id is needed for the proper rendering of the Call Log and navigation.

Comment 2

[Julien Wajsberg [:julienw]]

Is it the same bug than bug 883750 ?

Comment 3

[Germán Toro del Valle (:gtorodelvalle)]

It is definitely the same as the issue pointed out in comment 2 of bug 883750.

Currently investigating the "same phone number assigned to distinct contacts" issue pointed out in comment 1 of bug 883750.

Comment 4

[Germán Toro del Valle (:gtorodelvalle)]

Definitely this patch does not solve bug 883750 so not a duplicate :-( bug 883750 will require its own patch :-)

Comment 5

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

Comment on attachment 764058 [details]
Associated PR.

Hi G!

Reading the patch couldn't understand how this could help to the fact of opening the contact detail view within the dialer.

In fact, just tried master branch, without this patch and it's working for me. 

Could you give more insights? Perhaps I'm missing something.

Thanks!
F.

Comment 6

[Alex Keybl [:akeybl]]

Please renominate if this is a regression.

Comment 8

[John Hammink]

Please retest this on a recent build.

Comment 9

[Germán Toro del Valle (:gtorodelvalle)]

Hi guys and also to F :-p ! Until cbarker confirms it, I can still reproduce the issue using the latest Gecko and Gaia as for now :-) and it is kind of logical that I do. Let me try to explain why. The problem is that after the refactoring of the Call Log contacts ids play a fundamental role and with the current code in master we are not passing this id from oncall.js to dialer.js and consequently to call_log.js. Please, have a look at this lines of code:
1. When creating the 'contactCopy' we do not store the contact id. See https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/handled_call.js#L132
2. As you can see here https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/handled_call.js#L160 , the contactCopy is stored in the recentEntry.contactInfo.contact object.
3. This info is passed from oncall.js to dialer.js here https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/handled_call.js#L278 and more specifically here https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L738
4. This message is handled here https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/dialer.js#L247 and more specifically here https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/dialer.js#L124
5. As for now there is no contact id info in entry.contactInfo :-( although the call_log.js file needs it as you can see https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/call_log.js#L522 and more specifically here https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/call_log.js#L583 to show the selected contact details :-)

So, I still support my patch as the solution to this bug. In fact, I have retested and using the patch it works as desired whereas not using it, it doesn't :-(

Comment 10

[Germán Toro del Valle (:gtorodelvalle)]

Requesting reconsideration from F... :-p If you want to we can chat offline. Thanks!

Comment 11

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

Sorry, but with latest gaia master and gecko master I'm unable to reproduce the error with the STR in comment #0.

Let's take a look this thursday I'm traveling to Madrid and you can show me :)

Thanks!

Comment 12

[Germán Toro del Valle (:gtorodelvalle)]

:-D I reproduce it with a 100% of success :-p

This is what I do:
 1. Create a contact "A" associated to the phone number 111111111.
 2. Create a contact "An" associated to the phone number 222222222.
 3. Create a contact "Ap" associated to the phone number 666666666.
 4. Call to the DuT from 666666666.
 5. Go to the Call Log, tap on the 666666666 entry and I get the details of "A" instead of the details of "Ap" as I should.

Are you doing the same, F.? :-)

Comment 13

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

Hi,

with the supplied STK I'm NOT able to reproduce this in gaia master.

With that said, if I start playing generating several entries in the call log from the 3 contacts, then I see this problem.

Will apply the patch and try again.

Thanks.

Comment 14

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

Comment on attachment 764058 [details]
Associated PR.

Applied the patch and working the weird behaviour.

Thanks German ;)

Comment 15

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

btw I had to rebase manually, so please do before mergin :)

Comment 16

[Germán Toro del Valle (:gtorodelvalle)]

Merged in master: https://github.com/mozilla-b2g/gaia/commit/faadeec569bce9be9b1e8859b351ac7878a080c7 , once rebased ;) Thanks Francisco!

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This is a kinda bad user experience and a regression from v1.0.1. So renominating for blocking b2g18 as given by comment 6.

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

John - is this in your need-to-uplift queries?  It looks like this is just looking for landing on v1.1

Comment 19

[Marcia Knous [:marcia]]

jammink: Removing cbarker needs info, will need to replace him with another tester.

Comment 20

[John Ford [:jhford] CET/CEST Berlin Time]

can this bug be RESOLVED->FIXED?  It should be in the uplift queries once it's no longer NEW.

Comment 21

[Germán Toro del Valle (:gtorodelvalle)]

Damn it! I forgot to evolve it to FIXED :-O Sorry guys!

Comment 22

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted faadeec569bce9be9b1e8859b351ac7878a080c7 to:
v1-train: 1d011a05513b3eeeca4e6a03230a1113da5ed4a1

Comment 23

[John Ford [:jhford] CET/CEST Berlin Time]

v1.1.0hd: 1d011a05513b3eeeca4e6a03230a1113da5ed4a1

Comment 24

[Jinyoon]

I just added milestone & whiteboard comment for sync-up with vendor build.