Closed Bug 1081854 Opened 10 years ago Closed 10 years ago

GAIA follow-up to Bug 978639 (for GCF case 31.4.2.1.3)

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Dialer
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:2.0M+, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Status:
RESOLVED FIXED
Milestone:
2.1 S7 (24Oct)
Project Flags:
blocking-b2g 2.0M+
Tracking Flags:
Tracking Status
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: wesley_huang, Assigned: thills)

References

Details

(Whiteboard: [caf priority: p2][cr 730147][planned-sprint c=3])

Attachments

(1 file, 2 obsolete files)

Changes after review
(deleted), text/x-github-pull-request
drs
: review+
bajaj
: approval-gaia-v2.1+
Details
Per the comment: https://bugzilla.mozilla.org/show_bug.cgi?id=978639#c13 This bugs for derived efforts from new API telephonyCallGroup.hangUp()
[Blocking Requested - why for this release]: This one is GAIA portion to fix bug 978639 which is CAF blocker.
Blocks: 978639
blocking-b2g: --- → 2.1?
Preemptively blocking as this blocks CAF release and needs 978639 to be resolved. Please let me know if the effort/risk is not manageable to close this in the next few days..
blocking-b2g: 2.1? → 2.1+
Whiteboard: [cr 730147]
Considering that bug 978639 doesn't even have a WIP patch posted yet, I'm skeptical of us being able to land the Gaia side in the next few days, which I interpret as meaning by the end of this week. Having said that, if bug 978639 lands soon, I don't think there's much risk on the Gaia side.
Whiteboard: [cr 730147] → [cr 730147][planned-sprint c=]
Target Milestone: --- → 2.1 S7 (24Oct)
Whiteboard: [cr 730147][planned-sprint c=] → [caf priority: p2][cr 730147][planned-sprint c=]
bug 978639 has an ETA of 10/17. I think we can do this in 3 days. Bhavana, what did you mean by "next few days"?
Flags: needinfo?(bbajaj)
Assignee: nobody → thills
Whiteboard: [caf priority: p2][cr 730147][planned-sprint c=] → [caf priority: p2][cr 730147][planned-sprint c=3]
(In reply to Doug Sherk (:drs) (use needinfo?) from comment #4) > bug 978639 has an ETA of 10/17. I think we can do this in 3 days. Bhavana, > what did you mean by "next few days"? Thanks for making the dependency clear. Given this is blocking the partner CS release we needed a clear eta here and a quick fix. I am going to set expectation that once 978639 is resolved we will be able to fix this in a couple of days as you mentioned.
Flags: needinfo?(bbajaj)
No longer blocks: CAF-v2.1-FC-metabug
Blocks: CAF-v2.1-CC-metabug
Status: NEW → ASSIGNED
Attached patch Feedback patch (obsolete) (deleted) — — Splinter Review
Hi Doug, I'm working on tests, but wanted to get your feedback. Also, if you can comment on where I moved the line: setEndConferenceCall. I put it inside the .then clause, but I could also see that it could be put before the hangUp is called (that's where it was before). But if the API fails, we could have an incorrect state between UI and what's actually there with the calls(maybe?). Thanks, -tamara
Attachment #8507991 - Flags: feedback?(drs.bugzilla)
Comment on attachment 8507991 [details] [diff] [review] Feedback patch Review of attachment 8507991 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, this looks reasonable. My one concern is that this will run anyways even if `endConferenceCall()` fails: https://github.com/mozilla-b2g/gaia/blob/bae49d63304b4980c7f17681ba87bec38d6eb8ca/apps/callscreen/js/calls_handler.js#L453 Perhaps we could return the `hangUp()` Promise and chain it instead.
Attachment #8507991 - Flags: feedback?(drs.bugzilla) → feedback+
Attached file Patch for review (obsolete) (deleted) —
Hi Doug, Attached is PR updated based on feedback. I also modified the suite I was working in to use the new spy pattern since I was in there. Thanks, -tamara
Attachment #8507991 - Attachment is obsolete: true
Attachment #8508984 - Flags: review?(drs.bugzilla)
Comment on attachment 8508984 [details] Patch for review Please see my comments on the PR.
Attachment #8508984 - Flags: review?(drs.bugzilla) → review-
Attached file Changes after review (deleted) —
Hi Doug, The changes are in the second commit. Thanks, -tamara
Attachment #8508984 - Attachment is obsolete: true
Attachment #8509151 - Flags: review?(drs.bugzilla)
Comment on attachment 8509151 [details] Changes after review Looks good. I left 3 nits on the PR.
Attachment #8509151 - Flags: review?(drs.bugzilla) → review+
https://github.com/mozilla-b2g/gaia/commit/2d7386db0496283b10731ba459994b3cff8b6d26 Please request approval for uplift to v2.1.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → fixed
Flags: needinfo?(thills)
Resolution: --- → FIXED
Comment on attachment 8509151 [details] Changes after review This should be landed on 2.1 *after* bug 978639 lands on gecko. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): gap in testing [User impact] if declined: adherence to standard will be missing. [Testing completed]: yes [Risk to taking this patch] (and alternatives if risky): low [String changes made]:
Flags: needinfo?(thills)
Attachment #8509151 - Flags: approval-gaia-v2.1?(release-mgmt)
Attachment #8509151 - Flags: approval-gaia-v2.1?(release-mgmt) → approval-gaia-v2.1+
Hi Ryan, Just reading bug 978639 and it looks like we need it on 2.0m as well. I haven't had a chance to try it out on that branch yet, so would like to do it before we go ahead and merge it. Thanks, -tamara
Flags: needinfo?(thills)
Comment on attachment 8509151 [details] Changes after review See comment 13. I checked and this shouldn't need anything special to land on 2.0m. The affected code is almost identical.
Attachment #8509151 - Flags: approval-gaia-v2.0?(release-mgmt)
Unable to verify as it's a back-end issue
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-] → [QAnalyst-Triage+][QAnalyst-verify-]
Flags: needinfo?(ktucker)
NI kai-zhen for 2.0M landing.
Flags: needinfo?(kli)
blocking-b2g: 2.1+ → 2.0M?
Comment on attachment 8509151 [details] Changes after review Clearing the approval as kai-zhen should take care of this landing on 2.0M.
Attachment #8509151 - Flags: approval-gaia-v2.0?(release-mgmt)
blocking-b2g: 2.0M? → 2.0M+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: