Closed
Bug 1160054
Opened 10 years ago
Closed 8 years ago
[Messages][refactoring] Migrate mozActivity DOM request to promise base
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: steveck, Unassigned, Mentored)
References
Details
(Whiteboard: [lang=js][good first bug])
Attachments
(1 file)
(deleted),
text/x-github-pull-request
|
Details |
After Bug 839838 the mozActivity DOM request should return promise and it might be more readable and easier to use in some case that could use promise chain. Also the unit test for mock mozActivity could be simpler as well.
Reporter | ||
Updated•10 years ago
|
Blocks: sms-refactoring
Updated•9 years ago
|
Assignee: schung → nobody
Mentor: schung
Whiteboard: [lang=js][good first bug]
Comment 1•9 years ago
|
||
Hey steve,
What i understand from the description is, as DOMrequest now supports .then() properties. We have to replace .onsuccess/onerror etc with .then() . Am i correct?
Thanks
Flags: needinfo?(schung)
Reporter | ||
Comment 2•9 years ago
|
||
(In reply to kumar rishav (:rishav_) from comment #1)
> Hey steve,
> What i understand from the description is, as DOMrequest now supports
> .then() properties. We have to replace .onsuccess/onerror etc with .then() .
> Am i correct?
>
> Thanks
Sorry for the late reply, and yes you are right. It seem not a difficult task but you still need to pay attention about the unit test and adjust the mock activity. And we also need to rethink about the interface of activity picker.
Flags: needinfo?(schung)
Updated•9 years ago
|
Assignee: nobody → rishav006
Comment 3•9 years ago
|
||
hey steve,
"And we also need to rethink about the interface of activity picker" ?
I didn't get this. Are we planning for new interface for activity picker?
Thanks
Flags: needinfo?(schung)
Reporter | ||
Comment 4•9 years ago
|
||
(In reply to kumar rishav (:rishav_) from comment #3)
> hey steve,
> "And we also need to rethink about the interface of activity picker" ?
> I didn't get this. Are we planning for new interface for activity picker?
>
> Thanks
If we apply promise base for MozActivity in ActivityPicker, we can remove handleActivity function in ActivityPicker and return MozActivity promise directly. For example:
ActivityPicker.dial(number, onsuccess, onerror) could become
ActivityPicker.dial(number).then(onsuccess, onerror)
Flags: needinfo?(schung)
Comment 5•9 years ago
|
||
Hey steve,
Apart from unit test, Do activity picker need .then method (though it's better to have it) ? As rest of codes in method (where ActivityPicker is used), this async method seems independent from rest of code as these name 'new' 'view' 'update' 'open' 'dial', doesn't need onsuccess/onerror.
Thanks
Flags: needinfo?(schung)
Reporter | ||
Comment 7•9 years ago
|
||
Well, some methods like createNewContact does not have onsuccess/onerror originally, does not mean that we can't handle it in the new ActivityPicker. We can still return the MozActivity promise and developers can decide whether they want to handle anything else in success or error.
Comment 8•9 years ago
|
||
Comment 9•9 years ago
|
||
Comment on attachment 8678169 [details]
[gaia] kumarrishav:Bug-1160054 > mozilla-b2g:master
Here is the patch. Hope it's fine.
Attachment #8678169 -
Flags: review?(schung)
Reporter | ||
Comment 10•9 years ago
|
||
Comment on attachment 8678169 [details]
[gaia] kumarrishav:Bug-1160054 > mozilla-b2g:master
I left some thoughts about the unit test on github. Even the part you rewrite didn't break the tests, I think it would be great if we can clean up the outdated test case and mock setup all together since we are not rush for the refactoring right now and it's not difficult to rewrite the activity picker test, so let's make everything ready in this patch :)
Attachment #8678169 -
Flags: review?(schung)
Updated•9 years ago
|
Assignee: rishav006 → nobody
Comment 11•8 years ago
|
||
Mass closing of Gaia::SMS bugs. End of an era :(
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Comment 12•8 years ago
|
||
Mass closing of Gaia::SMS bugs. End of an era :(
You need to log in
before you can comment on or make changes to this bug.
Description
•