837994 - [MMS][User Story] Multiple recipient messaging

Status: RESOLVED FIXED

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

Description

[Peter Dolanjski [:pdol]]

UCID: Messages-001

User Story:
As a user, when I am composing a new message or replying to a group message, I want be able to send to multiple recipients to make it easier to communicate with a group of individuals.

Comment 1

[Arun Balachandran Ganesan [:abc]]

Here's the first draft. There are many concerns that some of us share (including me) which I will try to explain here. Looking for feedback. Thanks!

https://www.dropbox.com/s/koue1l7stvlr02f/sms-multiple-recipients.pdf

Tech concerns:
How does sending SMS to multiple people work? Is there any metadata that would help us to differentiate between an SMS send to just one person and another sent to multiple people?

Design concerns:
The approach taken here to show the members in a conversation might introduce new design patterns (I'm still looking into the possibility of following a similar flow using existing patterns.)

An alternative approach (not shown in specs) would be to use patterns similar to the mail app. Which means the members in a conversation would be listed in a 'To' field right under the title bar. That would however make it a little cumbersome to add or remove people which is not a frequently made operation in any single thread. (the design spec'ced takes this out of the way). The other concern with this approach is, when 4 people are in a conversation and they send 20 SMS each, it will be 80 SMS totally that you have to scroll through to go to the 'To' field and add/remove people.

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Arun Balachandran Ganesan [:abc] from comment #1)
> Tech concerns:
> How does sending SMS to multiple people work?

SMS is for one recipient only, exactly one.  Sending SMS to multiple recipients is actually sending the same text message to them one by one.

> Is there any metadata that would help us to differentiate
> between an SMS send to just one person and another sent to
> multiple people?

That's not defined in the protocol, but should be doable in API implementation.  Gecko isn't capable of this now.

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #2)
> (In reply to Arun Balachandran Ganesan [:abc] from comment #1)
> > Is there any metadata that would help us to differentiate
> > between an SMS send to just one person and another sent to
> > multiple people?
> 
> That's not defined in the protocol, but should be doable in API
> implementation.  Gecko isn't capable of this now.

To be more clearer, when Gaia submits an outgoing request with multiple recipients to Gecko, Gecko could store all the information needed to stitch these outgoing messages. But, when any of the recipients replies, the replied message won't contain any multiple recipients info because SMS is for single recipient only. So we can't put the replied message into the same thread with the sent one.

Comment 4

[Arun Balachandran Ganesan [:abc]]

 > To be more clearer, when Gaia submits an outgoing request with multiple
> recipients to Gecko, Gecko could store all the information needed to stitch
> these outgoing messages. But, when any of the recipients replies, the
> replied message won't contain any multiple recipients info because SMS is
> for single recipient only. So we can't put the replied message into the same
> thread with the sent one.

Unless, the recipient also has a Firefox OS mobile?

Thanks, Vicamo.

Comment 6

[Joe Cheng [:jcheng] (please needinfo)]

assign to vicamo first as he took a similar one in MMS

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Arun Balachandran Ganesan [:abc] from comment #4)
> Unless, the recipient also has a Firefox OS mobile?

It's not about the implementation. It's the protocol doesn't support it.

Comment 8

[Arun Balachandran Ganesan [:abc]]

Okay. This is probably going to require some more thought.

What I have on mind right now is this:

When I send out a message to 50 people, I'm going to see that as a single message I sent with it's contents and list of receivers/contacts in one place. I can send another message to the same list of people if I want. I can also forward the message if I want to other people and that will be another new message.

If someone replies to my message, that will be shown as a new message on my list of messages. If technically possible, I should be able to see "to which message I got this reply" in the same thread as well.

Before I make the specs, I want to gather some feedback (I will continue to think about it as well). Thoughts?

Arun

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Arun Balachandran Ganesan [:abc] from comment #8)
> Okay. This is probably going to require some more thought.
> 
> What I have on mind right now is this:
> 
> When I send out a message to 50 people, I'm going to see that
> as a single message I sent with it's contents and list of
> receivers/contacts in one place.

For SMS, the message is actually duplicated multiple times depending on the number of recipients you selected in Gaia.  So, what's the definition of "a single message"?  In current Gecko implementation, that means "one or multiple transactions of SMS protocol sent to the same single receiver within one DOM request."  So actually "a single message" may be consisted of several SMS TPDUs and charged differently.  That's why Cost Control app has to call getSegmentInfoForText() again to determine the number of actual segments to know the exact cost of that message.

But, judging from your description, "a single message" should be exactly one entry shown in UI, at least in thread list view[1].  That's only possible for outgoing SMS messages.  For incoming SMS messages, I'll never know how many copies were sent.  So the sender recognizes all recipients as CC, but they're actually BCC in email's language.  Of course, they won't be able to "reply all", either.

> I can send another message to the same list of people if I
> want.

Can't do this with current Gecko, but possible in theory.

> I can also forward the message if I want to other people and
> that will be another new message.

Same here.  Can't do this with current Gecko, but possible in theory.

> If someone replies to my message, that will be shown as a new
> message on my list of messages. If technically possible, I
> should be able to see "to which message I got this reply" in
> the same thread as well.

Unfortunately, it's not technically possible.  We just can't tell the replying message from a new message of a different context from the same, single sender.

> Before I make the specs, I want to gather some feedback (I
> will continue to think about it as well). Thoughts?

Thank you.  And I will keep you updated with technical details at my best.

[1]: In Android, there is only one item in thread list view, but multiple items in the conversation view. So they're able to recognize "multiple messages" sent to different recipients as the same thread.

Comment 10

[Dylan Oliver [:doliver]]

Clearing tracking-b2g18 flag from User Story bugs. This flag is for bugs that we would take fixes for in the 1.x branch. Feature work should be officially slotted into a release instead with leo+. If this story is intended for v1.1, please nominate for leo? blocking.

Comment 11

[Arun Balachandran Ganesan [:abc]]

Thanks for the comments, Vicamo. I will post updated specs once they're ready.

Comment 12

[Rob MacDonald [:robmac]]

Hi Peter - Although multiple recipient messaging has been deferred, I propose creating a separate bug to modify the current implementation of the "To" field to match Arun's spec (https://www.dropbox.com/s/koue1l7stvlr02f/sms-multiple-recipients.pdf).

The way it's set up currently, messaging has the "To" field in the header bar. Not only is this inconsistent from email, but, in its current implementation, it looks more like a search field than a a "To" field. So it's not just a pattern issue, but a potential usability issue as well.

How do you feel about creating a separate story for changing the layout of the "To" field and adding it to an upcoming release?

Comment 13

[Peter Dolanjski [:pdol]]

(In reply to Rob MacDonald [:robmac] from comment #12)
> How do you feel about creating a separate story for changing the layout of
> the "To" field and adding it to an upcoming release?

Rob, that makes sense to me.  In this instance, I'd file it as a usability bug rather than a user story.  We can then mark it with a tracking flag to be fixed when an individual has the cycles.

Comment 14

[ayman maat :maat]

Attachment: HTML5_SMS-MMSUserStorySpecifications_20130404_V4.0

Wireframe release:
HTML5_SMS-MMSUserStorySpecifications_20130404_V4.0

**new wireframes**
- Multi-recipient : discard message dialogue
- Multi-recipient : phone number not in contant list
- Multi-recipient : phone number already in contant list
- Multi-recipient : email already in contant list
- Multi-recipient : multiple SMS packets

**updated wireframes**
b.2) Phone number long press options
- CTA ‘call’ email removed due to change in use case

g.2) Email long press options
- CTA ‘send email’ removed due to change in use case

Multi-recipient : new message
- annotation 06 (tid feedback)
- annotation 07, 08, 09 removed (architacture change)
- wireframe architecture updated

Multi-recipient : auto suggestions
- rule: ‘Selection of same result’ removed
- annotation 02 updated

Multi-recipient : Save / discard message dialogue
- annotation indicating that this is not a V1.1 requirement added

Multi-recipient : Message thread listing
- annotation 03 updated

Multi-recipient : group participants
- presentation switched from a layer to a screen

**deleted wireframes**
- none

**new flows**
- Received from a number in the contact list

**updated flows**
Received from a number not in the contact list
annotation updated
- 03
- 05

Add email address from message
updated wireframe: g.2) Email long press options
- CTA ‘send email’ removed due to change in the use case
annotation removed
- 03 - due to change in the use case
annotations renumbered due to removal of 03
annotation updated
- 04 (was anotation 05 prior to renumbering)

**deleted flows**
- none

Comment 15

[Borja Salguero [:borjasalguero]]

Stealing due to it's a Gaia stuff!

Comment 16

[Dietrich Ayala (:dietrich)]

Assignee changed to Rick, as Borja is away this week.

Comment 17

[Julien Wajsberg [:julienw]]

I understand this is a leo feature, so requesting leo here.

Comment 18

[Rick Waldron [:rwaldron]]

Attachment: Fix for 837994

Comment 19

[Julien Wajsberg [:julienw]]

note to borja> I think I'll have time to review this one, but you can have an early look too, especially compared to your bug 861227

Comment 20

[Borja Salguero [:borjasalguero]]

Comment on attachment 744849 [details] [review]
Fix for 837994

Stealing this review.

Comment 21

[Julien Wajsberg [:julienw]]

Comment on attachment 744849 [details] [review]
Fix for 837994

taking back the review

first round of review done tonight

Comment 22

[Borja Salguero [:borjasalguero]]

Comment on attachment 744849 [details] [review]
Fix for 837994

Im gonna keep my review as well. This is a key point of the SMS/MMS App and due to I was discussing this functionality with UX & product I would like to verify that the functionality implemented it's the expected.

Comment 23

[Borja Salguero [:borjasalguero]]

Comment on attachment 744849 [details] [review]
Fix for 837994

There are a lot of pending work in this PR for fitting the requirements, so It's a good idea to work together during our work-week in Portland. Currently it's not ready for merging due to the issues found while testing https://github.com/mozilla-b2g/gaia/pull/9521#issuecomment-17457545 . See you on Monday!

Comment 24

[isabelrios]

Hi Jason, just saw you set the flag: in-moztrap? FYi, we are defining the test cases for this US, we are almost done, will share all the test cases when ready to be imported into moztrap

Comment 25

[Julien Wajsberg [:julienw]]

Comment on attachment 744849 [details] [review]
Fix for 837994

removing myself as I won't be able to review before next Monday.

I didn't review in depth the latest patch but it seemed overall quite good, so the only thing left is to test thoroughly and I'm sure borja will do this just fine !

Comment 26

[Borja Salguero [:borjasalguero]]

Attachment: Issue 1.1 - 'z-index' mess without keyboard

Comment 27

[Borja Salguero [:borjasalguero]]

Attachment: Issue 1.2 - 'z-index' mess with keyboard

Comment 28

[Borja Salguero [:borjasalguero]]

Attachment: Issue 2 - Editable recipient issue

Comment 29

[Borja Salguero [:borjasalguero]]

Attachment: Issue 3 - Live search not cleaned after sending

Comment 30

[Borja Salguero [:borjasalguero]]

Attachment: Issue 4.1

Comment 31

[Borja Salguero [:borjasalguero]]

Attachment: Issue 4.2 - Duplicates of the message sent.

Comment 32

[Borja Salguero [:borjasalguero]]

Attachment: Issue 5.1

Comment 33

[Borja Salguero [:borjasalguero]]

Attachment: Issue 5.2

Comment 34

[Borja Salguero [:borjasalguero]]

Attachment: Issue 5.3

Comment 35

[Borja Salguero [:borjasalguero]]

Comment on attachment 744849 [details] [review]
Fix for 837994

As we agreed, due to this code is risky, we are going to merge in a 'unstable' branch for moving forward. There are pending issues/regressions to be fixed, so we have to create a bug per each. Pending issues:

- Test 'related' with single recipient. We have to update all these tests (currently are only commented) or remove them if we have new ones for multi-recipients covering the same.
- Fix z-index of all layers in the composer. Currently are not working in several situations (Issue 1.1 & 1.2 attachments). Follow the behaviour explained in page 19 of WF (https://www.dropbox.com/s/mrtjwvzk6xgoc02/HTML5_SMS-MMSUserStorySpecifications_20130503_V8.0.pdf)
- Some 'editable' recipients are not working as expected. It's becoming 'editable' without changing the 'non-editable' style. (Issue 2 attachment)
- Live-search panel is not cleaned after searching a contact in previous steps. (Issue 3 attachment).
- Sending a SMS to 'n' recipients, generates 'n' messages in *only* one thread. For fixing this should be enough going back to 'thread-list' when multirecipient SMS only (because in the case of MMS we are going to use thread id) (Issue 4.1 & 4.2 attachments). This is one of the most important one to be fixed asap.
- Header is not updated properly (counter is not updated properly, check Issue 4.1 attachment).
- Recipient container is not working as expected. Instead of having a 'limit' of max-height, we should take all the available space (Issue 5.1 attachment). Furthermore, adding 'overflow: scroll' is creating a wrong overflow-x in the recipients-container (check Issue 5.2 attachment). As well we should avoid 'pull down' effect when having only one recipient (check Issue 5.3 attachment),
- When the 'recipients-container' is pulled down, if you focus in the input suddenly suddenly a blur event appears, and the cursor is again in the 'to' field.

This is R+ **only** for merging in the dev-branch. All these pending issues should be addressed for considering this US done.

Comment 36

[Borja Salguero [:borjasalguero]]

Additional info for Issue 2 attachment:
- Type 'b' in recipient
- Click 'enter'
- Tap on it for editing it
- Tap again on it

EXPECTED: Nothing happens because you're in edit mode
CURRENTLY: We change the style to 'non-editable', and the cursor/caret goes to the first position.

Comment 37

[Dan Heberden]

regarding the z-index portion, i'm going to make that into a separate bug (unless Rick decides to fix it) as this looks like it was introduced in the new layout stuff (#860680/#865411) and isn't related to this patch. 

The Sending to 'n' recipients is pending UX approval, so i'll make a new bug for that with pending status on what to do.

I'll also make a new bug for the header not updating properly. 

I think the other things will most likely be addressed by Rick as a followup.

Comment 39

[Borja Salguero [:borjasalguero]]

I've seen almost the majority of things fixed (thanks!) . 3 small pending issues:

- https://github.com/mozilla-b2g/gaia/pull/9521#issuecomment-17643784
- When editing a recipient when the recipients-container is pulled-down, we have to go back to 'one-line' layout
- Create a recipient (One message is shown in the header). Tap for editing it. EXPECTED: 'New message' string. CURRENTLY: '0 messages'.

Comment 40

[Rick Waldron [:rwaldron]]

All of the above has been addressed

https://github.com/mozilla-b2g/gaia/pull/9521/commits

Comment 41

[Julien Wajsberg [:julienw]]

could you put the commit url of the commit please ?

Comment 42

[Chris Lee [:clee]]

Can we marked this resolved if all issues are addressed?  What else are we waiting on here?  Thanks.

Comment 43

[Julien Wajsberg [:julienw]]

Chris, this was not landed on gaia master but only on bocoup's dev branch (as most other MMS bugs). We'll mark as resolved when we'll land on gaia master.

Comment 44

[Julien Wajsberg [:julienw]]

Corey, please add the commit hash here.

Comment 45

[Corey Frang [:gnarf]]

https://github.com/mozilla-b2g/gaia/commit/2e6a20d256cd79a3cf806abe01605c6446a8629c

Comment 46

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

v1-train:98b0930