Closed Bug 1127398 Opened 5 years ago Closed 5 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: azasypkin, Assigned: azasypkin)

References

Details

(Whiteboard: [sms-papercuts])

Attachments

(1 file)

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.
Whiteboard: [sms-papercuts]
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?
Attachment #8592246 - Flags: feedback?(felash)
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/
Attachment #8592246 - Flags: feedback?(felash) → feedback+
Assignee: nobody → azasypkin
See Also: → 1153885
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.
Status: NEW → ASSIGNED
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!
Attachment #8592246 - Flags: review?(felash)
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.
Attachment #8592246 - Flags: review?(felash)
Blocks: 1168941
(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 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.
Attachment #8592246 - Flags: review?(felash)
Comment on attachment 8592246 [details] [review]
[gaia] azasypkin:bug-1127398-thread-for-send-message > mozilla-b2g:master

r=me if treeherder is green

thanks !
Attachment #8592246 - Flags: review?(felash) → review+
Thanks for review!
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.