1127398 - [Messages] Display existing thread when sending a message using phone-number-/email-link context menu

Status: RESOLVED FIXED

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

Description

[Oleg Zasypkin [:azasypkin]]

After a bug 918970 we have functionality that allows us to find thread by phone number or email. We should extract this peace of functionality from ActivityHandler and reuse it to resolve this issue as well.

STR:

* Open a thread with message that contains number/email corresponding to another existing thread;
* Tap on it and choose "Send message\Send multimedia message";

Expected result: user is forwarded to appropriate existing thread;

Actual result: user is forwarded to Composer.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] azasypkin:bug-1127398-thread-for-send-message > mozilla-b2g:master

Comment 2

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8592246 [details] [review]
[gaia] azasypkin:bug-1127398-thread-for-send-message > mozilla-b2g:master

Hey Julien,

Patch for bug 1142540 has opened "fix-them-all" season for recipient related bugs, so here is one more WIP patch :)

With this patch "Send message/multimedia message" context menu action in Thread\Participants panel should lead to existent Thread if it's available or to Composer otherwise, + "number" should be recongnized as valid contact recipient for these cases.

Can I have your early feedback on the approach I use here?

Comment 3

[Julien Wajsberg [:julienw]]

Comment on attachment 8592246 [details] [review]
[gaia] azasypkin:bug-1127398-thread-for-send-message > mozilla-b2g:master

yeah, this looks good :)

lots of removed code ! \o/

Comment 4

[Oleg Zasypkin [:azasypkin]]

This patch makes both activity handler and conversation view simpler that will help some other NG-related tasks. So I'll just remove changes conflicting with bug 1153885 from this patch and proceed.

Comment 5

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8592246 [details] [review]
[gaia] azasypkin:bug-1127398-thread-for-send-message > mozilla-b2g:master

Hey Julien, I think the PR is ready for the first round of review.

It turned out that we didn't support Participant#Thread1 --> Conversation#Thread2 transition (it's when you tap on contact in participants view and choose "Send message" _and_ we have existing 1-to-1 thread for this contact). So I added second commit related this issue only.


Thanks!

Comment 6

[Julien Wajsberg [:julienw]]

Comment on attachment 8592246 [details] [review]
[gaia] azasypkin:bug-1127398-thread-for-send-message > mozilla-b2g:master

I would give r+ with only the first commit. So feel free to put the second commit in a separate bug.

I think your change also fixes this STR:
1. have an entry in the call log that is not a contact _but_ has a conversation in the SMS app.
2. long press on this call log entry, press "send message"
=> we should enter the conversation.

Comment 7

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #6)
> Comment on attachment 8592246 [details] [review]
> [gaia] azasypkin:bug-1127398-thread-for-send-message > mozilla-b2g:master
> 
> I would give r+ with only the first commit. So feel free to put the second
> commit in a separate bug.
> 
> I think your change also fixes this STR:
> 1. have an entry in the call log that is not a contact _but_ has a
> conversation in the SMS app.
> 2. long press on this call log entry, press "send message"
> => we should enter the conversation.

Agree, it will be better to handle second commit as a separate bug. Filed bug 1168941 for that.

Comment 8

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8592246 [details] [review]
[gaia] azasypkin:bug-1127398-thread-for-send-message > mozilla-b2g:master

Added fixed review comments in a separate commit. Could you please take a look?

> I think your change also fixes this STR:
> 1. have an entry in the call log that is not a contact _but_ has a
> conversation in the SMS app.
> 2. long press on this call log entry, press "send message"
> => we should enter the conversation.

Hmm, looks like it worked before patch as well.

Comment 9

[Julien Wajsberg [:julienw]]

Comment on attachment 8592246 [details] [review]
[gaia] azasypkin:bug-1127398-thread-for-send-message > mozilla-b2g:master

r=me if treeherder is green

thanks !

Comment 10

[Oleg Zasypkin [:azasypkin]]

Thanks for review!

Comment 11

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/de8f88c7efbc84adc47a9d4fee604579314a052c