982029 - [MMS/SMS] Display email in SMS/MMS Threads

Status: RESOLVED FIXED

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

Description

[Taichi Matsuura]

For display SMS/MMS threads, email adress needs to be shown.

 [User story]
1. Run message app (some messages(sms/mms) with email address already recieved)
2. Go to the thread list.
3. Go into the thread.

[Eexpected]
- the subheader containing the email in the thread list[2] and the thread[3] both.
- in [2], we'll need to show the email if the receiver/senderis not a contact.

Comment 1

[Naoya Matsumoto]

Attachment: Display email in SMS/MMS Threads patch on github

Comment 2

[Naoya Matsumoto]

Hi, Julien and Steve.
I PR Attachment 8450890 [details].
Please review it.

Comment 3

[Julien Wajsberg [:julienw]]

I had a first look but I'm too tired to review properly. Will look at it monday.

Comment 4

[Naoya Matsumoto]

Hi, Julien. Thanks for your answer.
How about the situation of the reviews?

Comment 5

[Naoya Matsumoto]

Hi, Julien. Thanks for your answer.
How about the situation of the reviews?

Comment 6

[Julien Wajsberg [:julienw]]

I should be able to look at it today

Comment 7

[Julien Wajsberg [:julienw]]

Sorry, still couldn't today (lots of meetings). Tomorrow first thing...

Comment 8

[Julien Wajsberg [:julienw]]

Comment on attachment 8450890 [details] [review]
Display email in SMS/MMS Threads patch on github

commented on github

I didn't try on a device yet but I think you missed some locations and that the implementation for findByAddress is not correct.

Comment 9

[Naoya Matsumoto]

Attachment: Display email in SMS/MMS Threads patch on github

Comment 10

[Naoya Matsumoto]

Hi, Julien. Thanks for your review.
I modified of your pointed out and rebased it.
Could you please review Attachment 8453569 [details]?

Comment 11

[Julien Wajsberg [:julienw]]

Comment on attachment 8453569 [details] [review]
Display email in SMS/MMS Threads patch on github

Sorry, I couldn't review completely but I've put comments on github so that you can work.

I'll also try on a device and report later today whether this works as expected.

Comment 12

[Naoya Matsumoto]

Attachment: Display email in SMS/MMS Threads patch on github

Comment 13

[Naoya Matsumoto]

Hi, Julien. Thanks for your review.
I modified of your pointed out and rebased it.
Could you please review Attachment 8455148 [details]?

Comment 14

[Julien Wajsberg [:julienw]]

Comment on attachment 8455148 [details] [review]
Display email in SMS/MMS Threads patch on github

It's really a good work !

I found 2 issues, one of them can be fixed now, the other can be fixed in a separate bug. And some minor nits otherwise.

Please request review again once you're ready. Also, it can be easier if you add a separate commit with your new changes, so that I can review it quicker.

Thanks a lot!

Comment 15

[Naoya Matsumoto]

Attachment: Display email in SMS/MMS Threads patch on github

Comment 16

[Naoya Matsumoto]

Hi, Julien. Thanks for your review.
I modified of your pointed out and rebased it.
And I corrected STR2.
STR1 is going to regist another Bug.

Could you please review Attachment 8456684 [details]?

Comment 17

[Julien Wajsberg [:julienw]]

Comment on attachment 8456684 [details] [review]
Display email in SMS/MMS Threads patch on github

r=me but please fix the latest nit I pointed out in the pull request.

Once you've fixed the nit, you can put "r=julien" in the commit log, then you can push the new version, keep r+ here, and put "checkin-needed" in the "keywords" section of the bug.

Thanks for your work !

Comment 18

[Julien Wajsberg [:julienw]]

I filed bug 1039531 for the remaining issue I found while reviewing.

Comment 19

[Naoya Matsumoto]

Hi, Julien. Thanks for your review.
I modified of your pointed out.

Could you please review  it?

Comment 20

[Naoya Matsumoto]

Now, I think STR1(bug 1039531) does not happen.
How about ?

Comment 21

[Julien Wajsberg [:julienw]]

I'll squash+merge myself.

Comment 22

[Julien Wajsberg [:julienw]]

(In reply to Naoya Matsumoto from comment #20)
> Now, I think STR1(bug 1039531) does not happen.
> How about ?

Looks like you're right :) Was probably fixed by the same fix than for STR2. I thought it was a separate bug because it was happening both for contacts and unknown recipients, but looks like I was wrong.

Thanks !

Comment 23

[Julien Wajsberg [:julienw]]

master: 09961a72e6651786d56876113673847b2f85015e

Comment 24

[at-kitamura]

[Blocking Requested - why for this release]:
Nominating this for v2.0, as desired on a 2.0 product.

Comment 25

[Tony Chung [:tchung]]

ni? Wayne on how we should be treating these late requests for 2.0.  Wayne, can you drive this bug offline?

Comment 26

[Lawrence Mandel [:lmandel] (use needinfo)]

From a thread with Wayne and Lucas I can confirm that it is too late to accept this new feature work in 2.0. If this work is required for partner product, it will need to be cherry picked off of the 2.1 branch.

Comment 27

[Makoto Kato [:m_kato]]

set partner flag requested by partner.