1041124 - [Messages][Refactoring] Make use of EventDispatcher in MessageManager

Status: RESOLVED FIXED

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

Description

[Oleg Zasypkin [:azasypkin]]

It would beneficial for MessageManager to get rid of direct dependencies to UI Components (ThreadsUI, ThreadListUI, ReportView, LinkActionHandler, Navigation, Threads). MessageManager can just fire appropriate events using EventDispatcher implemented in bug 1039585.

It should also help to make transition to Messages Datastore smoother.

Comment 1

[Oleg Zasypkin [:azasypkin]]

Attachment: GitHub pull request URL

Comment 2

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8460067 [details] [review]
GitHub pull request URL

Hey Julien,

Here is the patch that removes all UI dependencies from MessageManager. I have few questions that I left at GitHub. At the moment I see only one part that can be easily extracted to a separate patch - "Utils.defineLazyGetter", but since I'm not sure that you're ok with it's currently in this patch :)

Thanks!

Comment 3

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Comment on attachment 8460067 [details] [review]
GitHub pull request URL

Please flip the review flag once you're ready!

Comment 4

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8460067 [details] [review]
GitHub pull request URL

I believe it's ready for review now! :)

Comment 5

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8460067 [details] [review]
GitHub pull request URL

Removing r? until I rebase on the latest master that brings some conflicts

Comment 6

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8460067 [details] [review]
GitHub pull request URL

Ready, please take a look!

Comment 7

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Comment on attachment 8460067 [details] [review]
GitHub pull request URL

Some remaining comments, but this looks really good and everything I tried works perfectly well on my phone !

Comment 8

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8460067 [details] [review]
GitHub pull request URL

(In reply to Julien Wajsberg [:julienw] from comment #7)
> Comment on attachment 8460067 [details] [review]
> GitHub pull request URL
> 
> Some remaining comments, but this looks really good and everything I tried
> works perfectly well on my phone !

Thanks for review! Fixed remaining comments..

Comment 9

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Comment on attachment 8460067 [details] [review]
GitHub pull request URL

r=me with some nits in the tests

please wait for a green Gu before merging (I know the other test suites are green already)

Comment 10

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #9)
> Comment on attachment 8460067 [details] [review]
> GitHub pull request URL
> 
> r=me with some nits in the tests
> 
> please wait for a green Gu before merging (I know the other test suites are
> green already)

Thanks for review! Try is green now.

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