1074601 - The SIM 2 Voice mail displayed as a "Unknown contact" in dialer view and call log view

Status: RESOLVED FIXED

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

Description

[Tzu-Lin Huang [:dwi2][:tzhuang]]

[Initial condition]
1. SIM information:
   SIM1 :CMCC 3G.
   SIM2：CU 3G
2. Data connection:Off.   
3. Issue happen time: 2014-09-18 20:32

[Step]
1.Set the Voice mail for SIM1 and SIM2
2.Launch Dialer->Long press "1"->Slect SIM2.
   **The "Voice mail" cant' be displayed in dialer screen and coll log  

[Expected Result]
The characters of "Voice mail" should be displayed 

[Error]
The characters of "Voice mail" Can't displayed

Comment 1

[Tzu-Lin Huang [:dwi2][:tzhuang]]

This bug happens on v2.0, v2.1, and master as well

Comment 3

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Attachment: pull request

Comment 4

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Hi Josh, Howie,

Not sure whom to ask, so I just NI you both. 
This bug is duped from bug 1070808 which is 2.0+, however 1070808 is a confidential bug.
So I just made a public version of it.

Raise 2.0m? for you to decide correct flag.

Thanks

Comment 5

[howie [:howie]]

Thank you Tzu-Lin. qawanted to confirm this happens on Flame. If so, this bug should be nominated as 2.0?

Comment 6

[Josh Cheng [:josh]]

Hi Hubert, 
qawanted, thanks!

Comment 7

[Evelyn Hung]

per comment 4, 2.0+ since the original confidential bug is 2.0+.

Comment 8

[Evelyn Hung]

Tzu-lin, do you forget to ask review for your PR?

Comment 9

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Douglas has gave me some comment but I haven't finish them yet. I'll set review once I address them.

Comment 10

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8497250 [details] [review]
pull request

Hi Douglas,

I've addressed your comments. And I was not sure how to make 'scenarios' to be more readable, so I added some comments to explain it's purpose.

Comment 11

[Anthony Ricaud (:rik)]

Comment on attachment 8497250 [details] [review]
pull request

I did a pass by review while looking at our blockers. I think the API change for Voicemail makes our code less readable.

Comment 12

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8497250 [details] [review]
pull request

Clearing review until Anthony's comments are addressed.

Comment 13

[Tzu-Lin Huang [:dwi2][:tzhuang]]

to update my status: 

I've address feedback comment to update Voicemail.check to use Promise and finished its unit test. 
However I found out that I also broke many other unit tests.

Still trying to fixing them.

Comment 14

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8497250 [details] [review]
pull request

Hi Anthony and Douglas,

I've address your comments. Please kindly give some feedback and review.

Because I change Voicemail.check() to return promise now, I also need to manually resolve promise of Voicemail.check() in handle_call_test and dialer_test to make these two tests run synchronously.

Thanks

Comment 15

[Anthony Ricaud (:rik)]

Comment on attachment 8497250 [details] [review]
pull request

Thanks for this, all my concerns have been addressed.

Comment 16

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8497250 [details] [review]
pull request

This is looking pretty good now. I left a few fairly nit-picky comments in the PR.

Comment 17

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8497250 [details] [review]
pull request

Hi Douglas,

Comment addressed. Please help to review it. 

Thanks

Comment 18

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8497250 [details] [review]
pull request

Looks good, thanks for sticking with us!

Comment 19

[Tzu-Lin Huang [:dwi2][:tzhuang]]

rebase to master, squash all commits, and waiting for result of Gaia-try

Comment 20

[Tzu-Lin Huang [:dwi2][:tzhuang]]

It seems like Gaia-try stop working now...

Comment 21

[Doug Sherk (:drs) (inactive)]

I went ahead and landed this because the try run looked clean. Please request uplift to v2.0 and v2.1.

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

Comment 22

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8497250 [details] [review]
pull request

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
When user dial voicemail number of SIM 2, it would be displayed as regular phone number instead of 'voicemail number' on both call screen and call log.

[Testing completed]: yes, with full coverage of unit tests
[Risk to taking this patch] (and alternatives if risky): not much risk since it only add one more argument to Voicemail.check
[String changes made]: no

Comment 23

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/a38a9e4376c78a65afe23623f8ba208be0fa0159

Comment 24

[Kai-Zhen Li [:seinlin][:kli]]

Merge into v2.0m conflict.

Comment 25

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Attachment: patch to v2.0m

this patch is cherry-picked from the original one.

Comment 26

[Kai-Zhen Li [:seinlin][:kli]]

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

Comment 27

[Josh Cheng [:josh]]

This is reported by partner for 2.0M. Make it 2.0M+.

Hi Wesley,
This is generic bug. Please decide whether it's important enough to land on 2.0.
Thanks!

Comment 28

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

(In reply to Josh Cheng [:josh] from comment #27)
> This is reported by partner for 2.0M. Make it 2.0M+.
> 
> Hi Wesley,
> This is generic bug. Please decide whether it's important enough to land on
> 2.0.
> Thanks!

Per the triage meeting, let's not aggressively uplift patch to 2.0.

Comment 29

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8497250 [details] [review]
pull request

according to comment 28, revoke approval request

Comment 30

[Josh Cheng [:josh]]

Hi Norry,
PLease verify on Woodduck 2.0M. Thanks!

Comment 31

[Norry.L.F(Leave from Mozilla)]

Hi Josh,

In Chinese mainland, the voice mail is not supported. So, we can't verify it.

Comment 32

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Plussing, but I think it might be a bit challenging to do the SIM and voicemail setup. We should do a dependency exploration before committing to any work.

Comment 33

[Shine(Leave from Mozilla)]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/11665/