951339 - [Messages][Drafts] Existing thread drafts not properly populated when a new message is sent to that same recipient

Status: RESOLVED FIXED

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

Description

[Evelyn Eastmond [:evhan55]]

*This STR is on top of rwaldron/v1.3drafts + rwaldron/947211

STR*
1. Go to existing thread with sent messages with recipient A
2. Start a draft 'AAA'
3. Click back. Choose 'Save as Draft'
4. Start a brand new thread, also to recipient A
5. Start a draft 'BBB'
6. Click back. Choose 'Save as Draft'. (There are now 2 drafts to recipient A)
7. Click into the thread-less thread with draft 'BBB' and click send
8. Original thread is found and replaces new composer

Expected:
- Original 'AAA' draft is pre-populated in composer

Actual:
- Composer is empty

Comment 1

[Rick Waldron [:rwaldron]]

Instead of pre-populating with the old draft, we should just clear drafts to be consistent.

Comment 2

[Rick Waldron [:rwaldron]]

Attachment: https://github.com/rwaldron/gaia/pull/10

Comment 3

[Evelyn Eastmond [:evhan55]]

Comment on attachment 8349009 [details] [review]
https://github.com/rwaldron/gaia/pull/10

Left a comment about maybe making the change in onSendClick instead

Comment 4

[Evelyn Eastmond [:evhan55]]

(In reply to Rick Waldron [:rwaldron] from comment #1)
> Instead of pre-populating with the old draft, we should just clear drafts to
> be consistent.

I'm going to rename the bug

Comment 5

[Evelyn Eastmond [:evhan55]]

Comment on attachment 8349009 [details] [review]
https://github.com/rwaldron/gaia/pull/10

Thanks for the clarification.  I left a comment about something that may happen anyway given the current code.  Not suggesting we fix it, just pointing out it's behavior that may be encountered and possibly confusing:

STR
1) Click send for draft 'BBB' to a person with whom you have a separate thread with another draft 'AAA'
2) Composer is cleared of message 'BBB', but original 'AAA' draft is not deleted yet
3) Message doesn't send successfully
4) Composer remains empty
5) Go back to ThreadList, go back to the assimilated thread
6) Composer is now populated with draft 'AAA' (because it never got deleted, because message never sent)

Comment 6

[Julien Wajsberg [:julienw]]

Hey Ayman,

I perfectly agree with what Rick did here, but I'd like to have your UX stamp.

Here is the STR:

* enter a thread A
* enter some text, click back, save draft
* wait a few days (not necessary ;) )
* enter new message view
(optionally save a draft and recall it)
* send a message to user A
=> You're being redirected to the thread that has a saved draft

The thinking here is that we should delete that saved draft, and show an empty composer.

Ayman, would you agree with this?

Comment 7

[Julien Wajsberg [:julienw]]

Comment on attachment 8349009 [details] [review]
https://github.com/rwaldron/gaia/pull/10

Added a comment on github: I think this should move either to MessageManager.onHashChange or to ThreadUI.onMessageSending

Comment 8

[Rick Waldron [:rwaldron]]

(In reply to Julien Wajsberg [:julienw] from comment #7)
> Comment on attachment 8349009 [details] [review]
> https://github.com/rwaldron/gaia/pull/10
> 
> Added a comment on github: I think this should move either to
> MessageManager.onHashChange or to ThreadUI.onMessageSending


Updated and pushed

Comment 9

[ayman maat :maat]

(In reply to Julien Wajsberg [:julienw] from comment #6)
> Hey Ayman,
> 
> I perfectly agree with what Rick did here, but I'd like to have your UX
> stamp.
> 
> Here is the STR:
> 
> * enter a thread A
> * enter some text, click back, save draft
> * wait a few days (not necessary ;) )
> * enter new message view
> (optionally save a draft and recall it)
> * send a message to user A
> => You're being redirected to the thread that has a saved draft
> 
> The thinking here is that we should delete that saved draft, and show an
> empty composer.
> 
> Ayman, would you agree with this?

Let me just clarify the flow:

1) open existing thread to contact A
2) compose message in message field
3) select back button thus saving ‘draft’ message composed in step 2 within the message field of contact A’s thread.
4) open new message composer
5) by whatever means enter contact A’s contact number into the ‘to field’ 
6) select send thus sending the message to contact A

..so what I think your saying is:

7) user is automatically directed to the thread they opened in step A with the outgoing message they sent in step 6 now present in the thread list *BUT* the ‘draft’ they composed in step 2) and saved in this thread in step 3) is deleted?

…if this is the case. no. i don’t agree. at all.

One of the things we have been *attempting* to do with the message app is to differentiate from our competitors. To overcome their shortcomings. One of the larger shortcomings of our competitors is that in certain scenarios like the one you describe they silently loose draft messages and are therefore silently discarding users content. I know that this is a scenario that end users find frustrating. Therefore overcoming this shortcoming was one of the reasons why drafts was specified the way it was before it got shredded and this is why in the final wireframe pack FFOS_MessageApp_V1.3_20131125_V8.0, in frame ‘3. Draft Thread’ of page 38 it was scaled back to this:

“When the user returns to the thread it will be presented in exactly the same state as it was in when the user moved away from it with the exception of any new incoming (though you could also extrapolate this to ‘outgoing’) messages which will be added to the existing thread. The user will be unable to compose another message in that thread without either deleting or sending the content that is currently held in the message composer.”

This way the user still has the choice to discard or send the draft message. This has also been agreed by Product.

Its a huge UX no no to silently discard users data/content. I am not going to underwrite a mechanism that results in that.

Comment 10

[Julien Wajsberg [:julienw]]

Comment on attachment 8349009 [details] [review]
https://github.com/rwaldron/gaia/pull/10

So the patch must be redone

Comment 11

[Rick Waldron [:rwaldron]]

Attachment: https://github.com/rwaldron/gaia/pull/10

Comment 12

[Rick Waldron [:rwaldron]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/14869

Comment 13

[Evelyn Eastmond [:evhan55]]

I put a small comment on the new PR: https://github.com/mozilla-b2g/gaia/pull/14869/files#r8501452

Comment 14

[Julien Wajsberg [:julienw]]

Comment on attachment 8350383 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/14869

r=me with the "replace draft" bug fixed too

Comment 15

[Rick Waldron [:rwaldron]]

https://github.com/mozilla-b2g/gaia/commit/c1ed307f23ecdb82885734f20c1b36fce05b25f5

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4bd85e16a885