[Messages][Drafts] Duplicate threadless drafts get saved

RESOLVED FIXED

Status

Firefox OS
Gaia::SMS
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: evhan55, Assigned: evhan55)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
This is based on rwaldron/gaia/v1.3drafts

STR:
1. Create a new message draft to an example recipient '444'
2. Click back to thread list. Choose 'save as draft'
3. Create a new message draft to another example recipient: '333'
4. Click back to thread list. Choose 'save as draft'
5. Open the previous thread with recipient '444'
6. Click back to thread list

Expected:
- Thread list has the same two threads, to recipients '444' and '333' only

Actual:
- Thread list has three threads, to recipients '444', '333' and '444' again.  The first thread has been duplicated.
(Assignee)

Updated

4 years ago
Summary: [Messages] [Drafts] Duplicate threadless drafts get saved → [Messages][Drafts] Duplicate threadless drafts get saved
(Assignee)

Updated

4 years ago
Assignee: nobody → evelyn
Here is my simple idea: instead of trying to match drafts, we can simply delete the draft when the user taps it.

Then the draft will be recreated in all usual cases (visibility change or pressing back).

What do you think Evelyn?
Flags: needinfo?(evelyn)
(Assignee)

Updated

4 years ago
Blocks: 879143
Flags: needinfo?(evelyn)
(Assignee)

Comment 2

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #1)
> Here is my simple idea: instead of trying to match drafts, we can simply
> delete the draft when the user taps it.
> 
> Then the draft will be recreated in all usual cases (visibility change or
> pressing back).
> 
> What do you think Evelyn?

Hi Julien,

Thank you. Yes I am taking that approach, I'm just running into issues with the threadlist not updating correctly to show the newly replaced draft details.

Evelyn
(Assignee)

Comment 3

4 years ago
Note for myself.

PR is here: https://github.com/rwaldron/gaia/pull/3

Possible reason for the thread not updating correctly.  We don't really properly register these new pseudo-threads internally with registerMessage from Threads:

https://github.com/evhan55/gaia/blob/57651b7b3a40f9edfde0b243f9f99285a4aaf368/apps/sms/js/threads.js#L102

So even if we correctly make the psuedo-thread's id the incoming draft.id, we never have a way to recall it since it's not properly stashed into that internal threads map.  Without being able to recall that thread, we can't ask it to update in the thread list via ThreadListUI.updateThread(threadId).
(Assignee)

Comment 4

4 years ago
Seems fixed now:

https://github.com/rwaldron/gaia/pull/3/files

Will file official PR soon
(Assignee)

Comment 5

4 years ago
Created attachment 8344729 [details] [review]
https://github.com/rwaldron/gaia/pull/3
Attachment #8344729 - Flags: review?(waldron.rick)
(Assignee)

Comment 6

4 years ago
Created attachment 8344730 [details] [review]
https://github.com/rwaldron/gaia/pull/3

- `drafts.js`
    - changed `add()` to delete existing, threadless drafts if the new draft being added matches draft.id
- `thread_ui.js`
    - ensures that the thread in the thread list is updated if a draft gets updated
- `drafts_test.js`
    - tests that threadless drafts are properly replaced
- `thread_ui_test.js`
    - tests that thread update is called upon draft saving
Attachment #8344730 - Flags: review?(waldron.rick)
Comment on attachment 8344730 [details] [review]
https://github.com/rwaldron/gaia/pull/3

landed in v1.3-drafts https://github.com/rwaldron/gaia/commit/549aa62937835b3a7aae8dcbc40fb35d2c06518d
Attachment #8344730 - Flags: review?(waldron.rick) → review+
https://github.com/mozilla-b2g/gaia/commit/b3945f083ee091467c6eba091a7a1412e8b21f97
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.