885313 - [SMS/MMS] duplicate threads created when invalid number is included in group SMS.

Status: RESOLVED DUPLICATE of bug 886764

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

Description

[ayman maat :maat]

duplicate threads created when invalid number is included in group SMS.

**PATH**
1) ensure that a contact exists in the contact list with a valid mobile phone number.
2) open message app
3) select new message 
4) add the contact with the valid phone number from the contact to the 'to field'
5) add a completely invalid number to the 'to field', something like 'yyyyyy'
6) type something in the message field
7) select send

**EXPECTED**
upon send the invalid phone number will be dropped from the message and a single thread will be created containing the message to the valid phone number added in step 4)

**ACTUAL**
Two separate threads are created both for the the valid phone number added in step 4)
Upon opening the first thread there are two separate message bubbles within the thread both containing the same message that was send in step 7). After a split second one instance of these duplicated message bubbles disappears.

Comment 1

[Rick Waldron [:rwaldron]]

How is the program supposed to know what is a valid number?

Comment 2

[Rick Waldron [:rwaldron]]

(In reply to ayman maat :maat from comment #0)
> duplicate threads created when invalid number is included in group SMS.
> 
> **PATH**
> 1) ensure that a contact exists in the contact list with a valid mobile
> phone number.
> 2) open message app
> 3) select new message 
> 4) add the contact with the valid phone number from the contact to the 'to
> field'
> 5) add a completely invalid number to the 'to field', something like 'yyyyyy'
> 6) type something in the message field
> 7) select send
> 
> **EXPECTED**
> upon send the invalid phone number will be dropped from the message and a
> single thread will be created containing the message to the valid phone
> number added in step 4)

I started investigating this and it occurred to me that this doesn't match the behaviour of sending a single SMS to an invalid number. Should the two behaviours match, ie. both should result in a "failed message"?

Comment 3

[Alex Keybl [:akeybl]]

Not a hard blocker, very edge casey.

Comment 4

[Rick Waldron [:rwaldron]]

needinfo from Ayman, please see https://bugzilla.mozilla.org/show_bug.cgi?id=885313#c2

Comment 5

[ayman maat :maat]

Hi Rick

NeedInfo to Rick: Can you confirm to me what you believe to be the correct behavior when sending a single SMS to an invalid number please. Currently when i attempt this on the device the app selects the first contact entry in the contact list to populate the header of the newly created thread… which is certainly not correct.

**PATH**

1) ensure that a contact exists in the contact list with a valid mobile phone number.
2) open message app
3) select new message 
4) add a completely invalid number to the 'to field'
5) type something in the message field
6) select send
 
**ACTUAL**

6.1) Message is created, but sent and fails …however name of first contact in contact list is now present in header for some reason

7) Select header

**ACTUAL** 

overlay opens with invalid number input in step 4) in header and 3 CTAs: 'Call', 'Send Message', Cancel'

8) Select 'Call' returns user to thread view
9) Select 'Send Message' opens the New message dialogue with the name of the contact identified in step 6.1) repopulated in the 'to field'. If the user adds text to the message field and selects send the new message is appended to the thread created in 6.1)

10) return to the message app inbox and select new message
11) navigate to the contact list and select the contact who's name was output in the header in step 6.1)
12) write something in the message field
13) select send
14) navigate back to message app inbox

**ACTUAL**
15) Two threads are now present. the one containing the message sent in 6) and added to in 9), and a new thread continuing the  message sent in 13)

Opening up a separate bug to detail issues outlined in steps 1 to 15

Renominating for LEO on the following grounds: 

a) that a user accidentally selecting an invalid character during manual input is not so remote 
b) the chance of an invalid character entering the phone number field of a contact in the address book during import from an external source is not so remote and the user will not be able to notice this from looking at the 'to field' alone. 
c) based on what i have outlined in point 1 to 15 above I suspect that there are further underlying problems in the logic that we need to address before launching this product. We will work towards identifying and addressing them by tackling this bug.

Comment 6

[Rick Waldron [:rwaldron]]

(In reply to ayman maat :maat from comment #5)
> Hi Rick
> 
> NeedInfo to Rick: Can you confirm to me what you believe to be the correct
> behavior when sending a single SMS to an invalid number please. 

STR 

(from Messages app)

1. Tap create new message button.
2. Type an invalid number into the recipient list, "123" should suffice
3. Type a message
4. Press send

EXPECTED

User arrives at new thread with message sending or sent (regardless of recipient's number validity). The carrier responds with a "failure" message that is displayed in the same thread.

ACTUAL

User arrives at new thread with message sending or sent (regardless of recipient's number validity). The message has failed to send, no indication why.





> Currently
> when i attempt this on the device the app selects the first contact entry in
> the contact list to populate the header of the newly created thread… which
> is certainly not correct.
>
> **PATH**
> 
snip.
> 
> Opening up a separate bug to detail issues outlined in steps 1 to 15

Just linking here for anyone catching up: https://bugzilla.mozilla.org/show_bug.cgi?id=886764

> 
> Renominating for LEO on the following grounds: 
> 
> a) that a user accidentally selecting an invalid character during manual
> input is not so remote

I believe this is an edge case.

> b) the chance of an invalid character entering the phone number field of a
> contact in the address book during import from an external source is not so
> remote and the user will not be able to notice this from looking at the 'to
> field' alone. 

This should be dealt with in contact imports.

Comment 7

[Alex Keybl [:akeybl]]

It's not clear to triage why this is any worse with group messaging than with a single SMS (how we shipped in v1.0.1, presumably).

If this is a regression, please mark it as such.

Comment 8

[ayman maat :maat]

(In reply to Alex Keybl [:akeybl] from comment #7)
> It's not clear to triage why this is any worse with group messaging than
> with a single SMS (how we shipped in v1.0.1, presumably).
> 
> If this is a regression, please mark it as such.

My mistake. I thought in 1.0.1 we where preventing the sending of a message to an invalid number, but i have just checked and we dont. So no regression.

This bug is worse than the scenario in 1.0.1 because if one of the recipients in the list of recipients is an invalid phone number, upon send, two separate threads are seeming created for a recipient with a valid phone number. 

...Actually I have just run some tests and what the app is actually doing is presenting the name of a contact with a valid phone number in the header of the thread that belongs to the invalid phone number thus giving the impression of two threads being created for the same valid phone number.   

I therefore believe the issue in this bug will be related to issue observed in bug 886764.

I will put together and attach some screenshots to illustrate

Comment 9

[ayman maat :maat]

Attachment: HTML5_SMS-MMSBugFlow_20130627_Bug885313_V0.1

reference comment 8

"...Actually I have just run some tests and what the app is actually doing is presenting the name of a contact with a valid phone number in the header of the thread that belongs to the invalid phone number thus giving the impression of two threads being created for the same valid phone number.   

I will put together and attach some screenshots to illustrate"

Comment 10

[Rick Waldron [:rwaldron]]

(In reply to ayman maat :maat from comment #9)
> Created attachment 767762 [details]
> HTML5_SMS-MMSBugFlow_20130627_Bug885313_V0.1
> 
> reference comment 8
> 
> "...Actually I have just run some tests and what the app is actually doing
> is presenting the name of a contact with a valid phone number in the header
> of the thread that belongs to the invalid phone number thus giving the
> impression of two threads being created for the same valid phone number.   
> 
> I will put together and attach some screenshots to illustrate"

Following this bug flow, I documented the current behaviour: 

  https://www.dropbox.com/sh/wdy81viodieoook/E9AnETwngU 

as it appears on: 

  gaia/master@d34e713d09ac1fadf1e471cca441fcdb93b119b9


The only obvious issue (that I can see) is the carrier tag (which is fixed in bug 885264)

Comment 11

[ayman maat :maat]

Rick, i got a fresh V1 train build today, flashed my device and did a 'git pull' and 'make reset-gaia' ...and can still reproduce the behavior i stated in comment 0 with the additional behavior now of:

6.1) If the invalid number is made up of alphabetic characters such as 'ayman' the messages app attempts to send the message to '29626' which results in an error message being sent from my carrier (though the name on the incoming message in my inbox and in the header of the thread is the 1st name in my contact list) telling me that 'the speed dial 29626 is not assigned'

6.2) If the invalid number is made up of alphabetic characters such as 'YYY' the messages app attempts to send the message to '999' which results in an error message being sent from my carrier (though the name on the incoming message in my inbox and in the header of the thread is the 1st name in my contact list)telling me that 'the speed dial 999 is not assigned'

6.3) If the invalid number is made up '@@@' the messages app fails to send the message.

The result of 6.1 and 6.2 should certainly not happen, so will raise a separate bug about this.

build information:

V1-train
Gecko - 914243b
Gaia - 477e572
build id - 20130627123702 

NeedInfo to Rick and Julien to look into this further as i do not understand why we are getting different results...

Comment 12

[Julien Wajsberg [:julienw]]

So, there has been some progress in bug 886764, I'm quite confident it is the same bug (or at least part of it). We end up sending the message twice because the invalid number actually matches the valid number in your contacts.

I'm duping against it then. Ayman, feel free to reopen if I missed something.