Closed Bug 942076 Opened 12 years ago Closed 10 years ago

[WAP push] When DUT receives SI message with delete action attribute as a wap push with same SI-ID is opened, opened one will be closed.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v1.3T affected)

RESOLVED FIXED
Tracking Status
b2g-v1.3T --- affected

People

(Reporter: echu, Assigned: gsvelto)

Details

(Whiteboard: permafail)

Attachments

(2 files)

An opened SI message will not be closed when a new SI message with same ID and delete action attribute arrives. * Build Number Gaia: ce276842c9ac1746073271fb736dfdb626a89240 Gecko: http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/36c4c667b9f2 BuildID 20131121004002 Version 26.0 * Reproduce Steps 1. Send 2 SI messages with same SI-ID. 2. Open one of the SI message and do not close it. 2. Send another SI message with same SI-ID and delete action attribute as step 1. * Expected Result Opened SI message should be closed. * Actual Result Opened SI message is not closed. * Occurrence rate 100%
Component: Gaia::System → Gaia::Wappush
Hardware: x86_64 → ARM
Whiteboard: burirun1.3-2
Whiteboard: burirun1.3-2 → burirun1.3-2, burirun1.3-3
Whiteboard: burirun1.3-2, burirun1.3-3 → permafail
NI on Gabriele to take a look, this is a long standing issue and doesn't seem to be tracked by flags or by block/dependencies. If this is being actively tracked, sorry for the noise.
Flags: needinfo?(gsvelto)
This is a valid bug and I have the infrastructure in place to fix it now; leaving the NI so I'll do it ASAP.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Flags: needinfo?(gsvelto)
Comment on attachment 8634086 [details] [review] [gaia] gabrielesvelto:bug-942076-close-si-id-when-deleted > mozilla-b2g:master Big patch. This fixes a lot more than just this bug: once I moved updating the UI via an event-based system I would have introduced some regressions if I had left the existing code as-is so I preferred fixing everything in one go. Here's a summary of the changes: - The database code now sends 'new', 'update', 'discard' and 'delete' events whenever a message is added to the database, updated, discarded or deleted. This events are used to drive the UI. - The database does not delete SI/SL messages right away, instead it marks them as already viewed by adding them to a separate table and then deletes them the next time the app is started. This is explicitly made to ensure reliable deletions (which we cannot do straight away due to LMK) while preserving messages for updates, etc... - The UI is updated in response to the events emitted by the database. The changes required for this address a number of issues: - If the current displayed SI message is deleted by an incoming SI message with action=delete set the app is closed - Notifications for deleted messages are now removed - If the current displayed SI message is updated by a newer incoming SI message then the UI is updated - If the user would tap on a message notification of a different type while the app was open the UI would display both the old and the new messages. Now message panels are properly hidden when switching message types with app open. - If a message had an expiration date which was later the moment in which it was received we would save the message. However if the user would open such message after the expiration date we'd show it as still valid. We know show all expired messages correctly. - Last but not least errors are now printed out correctly to the log - Unit-tests were tweaked to accommodate for the changes and more were introduced to cover the new functionality. Those in wappush_test.js are still more like poor man's integration tests rather than proper unit-tests which is something we should address but too large of a change for this bug.
Attachment #8634086 - Flags: review?(felash)
Comment on attachment 8634086 [details] [review] [gaia] gabrielesvelto:bug-942076-close-si-id-when-deleted > mozilla-b2g:master There are a lot of good things in this patch. I didn't try it for real (I don't really know how to do it BTW) but I left some comments. Tell me what you think !
Attachment #8634086 - Flags: review?(felash)
Comment on attachment 8634086 [details] [review] [gaia] gabrielesvelto:bug-942076-close-si-id-when-deleted > mozilla-b2g:master Thanks for the review Julien, testing can be done via the emulator using the sms pdu command but it's quite cumbersome (see bug 891762 comment 11). I did test all the scenarios I possibly could; it would be nice though to add some kind of debugging mode to the app that would allow testing by mocking the incoming messages rather than having to use the emulator. Or maybe, you know, integration tests :-P Anyway I've addressed all your comments, updated the unit-tests to reflect the changes and pushed a separate patch with the changes.
Attachment #8634086 - Flags: review?(felash)
Comment on attachment 8634086 [details] [review] [gaia] gabrielesvelto:bug-942076-close-si-id-when-deleted > mozilla-b2g:master r=me with the latest nits and a green try Feel free to request a last review if you feel like it though.
Attachment #8634086 - Flags: review?(felash) → review+
Thanks for the review, I've adjusted the ParsedMessage.load() functionality and documentation and added unit-tests covering it. I'll wait for the try-run to pass and then merge; the changes are trivial enough that I don't think another review is needed.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: