1208532 - [Messages][Refactoring] Extract SystemMessageHandler from ActivityHandler

Status: RESOLVED FIXED

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

Description

[Oleg Zasypkin [:azasypkin]]

In this we'd like to split ActivityHandler into ActivityHandler that will handle activity requests only and SystemMessageHandler that will handle "sms-received" and "notification" system messages.

Code that handles activity requests is quite different from one that handles other system messages, so it makes sense to split them into separate modules.

Comment 1

[Oleg Zasypkin [:azasypkin]]

Will take it, hope it will make solution for bug 1192263 easier.

Comment 2

[Oleg Zasypkin [:azasypkin]]

Working on the patch, discovered funny issue: if you use any l10n-id as the name of contact and receive the message from this contact - you'll see localized string as the sender name :D. E.g. name your contact as "discard-new-message" and enjoy receiving new message notifications from "Discard the message you’re composing and open the new message?" sender :)

It seems like an edgy case, but there is still chance to collide with shorter l10n ids.

I'll fix that.

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] azasypkin:bug-1208532-system-message-handler > mozilla-b2g:master

Comment 4

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8667921 [details] [review]
[gaia] azasypkin:bug-1208532-system-message-handler > mozilla-b2g:master

Hey Steve,

Here is WIP PR for the separated SystemMessageHandler, it doesn't contain any haven't fixed unit tests yet) and it works only in non-split mode (though I haven't tested it heavily, almost solely relying on existing integration tests).

Would be glad to have your early feedback!

Thanks!

Comment 5

[Steve Chung [:steveck]]

Comment on attachment 8667921 [details] [review]
[gaia] azasypkin:bug-1208532-system-message-handler > mozilla-b2g:master

It looks good and we want to refactor the message receiving part for a long time. It good to see we split the logic and split the huge message receiving logic into smaller pieces. I just have a small question: Would you plan to move the system message handling into service/shim once the system message is workable in shim?

Comment 6

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8667921 [details] [review]
[gaia] azasypkin:bug-1208532-system-message-handler > mozilla-b2g:master

Hey Steve,

I think it's now ready for the first round of review :)

(In reply to Steve Chung [:steveck] from comment #5)
> Comment on attachment 8667921 [details] [review]
> [gaia] azasypkin:bug-1208532-system-message-handler > mozilla-b2g:master
> 
> It looks good and we want to refactor the message receiving part for a long
> time. It good to see we split the logic and split the huge message receiving
> logic into smaller pieces.

Thanks for the feedback!

> I just have a small question: Would you plan to move
> the system message handling into service/shim once the system message
> is workable in shim?

Yep, I think so. In this PR I used the chance to clear this code up a bit, should not be difficult to move it to service/shim later if we decide so.

Comment 7

[Steve Chung [:steveck]]

Comment on attachment 8667921 [details] [review]
[gaia] azasypkin:bug-1208532-system-message-handler > mozilla-b2g:master

Okey I can't find critical issue in the patch. Only some trivial question and nits. Thanks for the splitting!

Comment 8

[Oleg Zasypkin [:azasypkin]]

(In reply to Steve Chung [:steveck] from comment #7)
> Comment on attachment 8667921 [details] [review]
> [gaia] azasypkin:bug-1208532-system-message-handler > mozilla-b2g:master
> 
> Okey I can't find critical issue in the patch. Only some trivial question
> and nits. Thanks for the splitting!

Thanks for review!

Nits are fixed and questions are answered - please tell me if it's what you've asked for :)

Comment 10

[Oleg Zasypkin [:azasypkin]]

Treeherder is green (only one unrelated failure), landed!

Master: https://github.com/mozilla-b2g/gaia/commit/a9ee136440a7d57c3d47ef1587e72be8e9136074