Closed Bug 1047410 Opened 5 years ago Closed 5 years ago

Desktop client should display Call Failed if an incoming call fails during set-up

Categories

(Hello (Loop) :: Client, enhancement, P1)

enhancement
Points:
3

Tracking

(firefox33 unaffected, firefox34+ verified, firefox35+ verified, firefox36+ verified)

VERIFIED FIXED
mozilla36
Iteration:
36.1
Tracking Status
firefox33 --- unaffected
firefox34 + verified
firefox35 + verified
firefox36 + verified
Blocking Flags:
backlog Fx34+

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Whiteboard: [loop-uplift][loop-inccall1][landed in fx-team])

User Story

When an incoming call fails (setup, connnect etc) on Firefox desktop, the user should be notified.

* Base on bug 1000240 which implements standalone view: Implement a "Call Failed" view.

* Display the view, rather than a notification if anything fails during call-setup.

https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#link-prompt

Attachments

(2 files)

No description provided.
User Story: (updated)
This UX is for outgoing call failures (to contacts).
The UX implementation is already tracked under https://bugzilla.mozilla.org/show_bug.cgi?id=1000186

Or am I misunderstanding this bug?
Flags: needinfo?(standard8)
(In reply to Romain Testard [:RT - on PTO back on August 14th] from comment #1)
> This UX is for outgoing call failures (to contacts).
> The UX implementation is already tracked under
> https://bugzilla.mozilla.org/show_bug.cgi?id=1000186
> 
> Or am I misunderstanding this bug?

We've just been discussing this with Darrin - we need a call failed indication for the hopefully rare cases where the incoming call fails to set-up correctly. Hence, although the mock-up is intended for outgoing, using it (without most of the buttons) for incoming is is also fine.
Flags: needinfo?(standard8)
So, does this cover the "Cancel button" and "E-mail link" from https://bugzilla.mozilla.org/show_bug.cgi?id=1000186#c7 ?
Flags: needinfo?(standard8)
(In reply to Paul Silaghi, QA [:pauly] from comment #3)
> So, does this cover the "Cancel button" and "E-mail link" from
> https://bugzilla.mozilla.org/show_bug.cgi?id=1000186#c7 ?

I'm not expecting it to currently, I think lets file new bugs to be safe.
Flags: needinfo?(standard8)
Priority: -- → P2
User Story: (updated)
Depends on: 1000240
Darrin -- Does this require any new strings or would we reuse strings from other views?
Flags: needinfo?(dhenein)
Whiteboard: [p=1, spike]
I think the strings in https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#link-prompt should be fine, unless someone had needinfo'd matej for alternate copy elsewhere.
Flags: needinfo?(dhenein)
Assignee: nobody → aoprea
QA Contact: aoprea → anthony.s.hughes
QA Contact: anthony.s.hughes
What would the cancel button do in this case?
Flags: needinfo?(standard8)
Also would the email button generate a new URL to send?
Hmm, this is part of the desktop direct calling case. We can't do this until we have the start of direct calling on desktop.
Flags: needinfo?(standard8)
Priority: P2 → P1
Whiteboard: [p=1, spike] → [p=1, spike][loop-uplift]
Target Milestone: mozilla34 → mozilla35
(In reply to Mark Banner (:standard8) from comment #9)
> Hmm, this is part of the desktop direct calling case. We can't do this until
> we have the start of direct calling on desktop.

So having another look at the bugs, it is actually a bit unclear. I am therefore going to make the decision that this bug is for implementing the view. Just display the appropriate generic failure text.

I'll file new bugs to cover adding the buttons later.
Blocks: 1066014
Severity: normal → enhancement
Points: --- → 3
User Story: (updated)
Whiteboard: [p=1, spike][loop-uplift] → [loop-uplift]
Blocks: 1048161
Blocks: 1048162
Duplicate of this bug: 1065721
Whiteboard: [loop-uplift] → [loop-uplift][loop-inccall1]
Is there a way to simulate a call failure so this can be tested?
Flags: qe-verify+
QA Contact: anthony.s.hughes
Depends on: 1067519
Blocks: 1067591
Flags: firefox-backlog+
backlog: --- → Fx35+
Assignee: aoprea → nobody
I just talked to Darrin and given that this is only going to happen in the case where the Desktop user is called back, it makes sense to display "Something went wrong" (instead of "Call Failed") -- and to not show the "Retry" or "Email" buttons.  At most we'd show the "cancel" button (if that didn't look odd).
I think this bug should include adding the "cancel" button (again, assuming the "cancel" button by itself doesn't look odd).
Target Milestone: mozilla35 → ---
WIP patch that is almost ready, just needs tests writing.
Assignee: nobody → standard8
backlog: Fx35+ → Fx34+
This fixes the tests as well.

Although we could try and share the view with the flux implementation of the call failed for outgoing calls, this seems a bit difficult at the moment - they are based on different ideas, and the outgoing one is tied to flux. Hence I'm thinking we should leave sharing the view until we fluxify the incoming call code.

The best way to test this at the moment is to start an incoming call from a url, but not accept audio/video media on the caller's side after the call is accepted. Then just let it time out.
Attachment #8511028 - Flags: review?(nperriault)
Comment on attachment 8511028 [details] [diff] [review]
Desktop client should display Call Failed if an incoming call fails during set-up

Review of attachment 8511028 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, r=me with the nit addressed.

::: browser/components/loop/content/js/conversation.jsx
@@ +181,5 @@
>    /**
> +   * Incoming Call failed view. Displayed when a call fails.
> +   *
> +   * XXX Based on CallFailedView, but built specially until we flux-ify the
> +   * incoming call views.

Nit: Please add the bug number as a reference.
Attachment #8511028 - Flags: review?(nperriault) → review+
https://hg.mozilla.org/integration/fx-team/rev/705cec97abf1

For test steps, see comment 16.
Flags: needinfo?(jaws)
Iteration: --- → 36.1
Target Milestone: --- → mozilla36
Whiteboard: [loop-uplift][loop-inccall1] → [loop-uplift][loop-inccall1][landed in fx-team]
Comment on attachment 8511028 [details] [diff] [review]
Desktop client should display Call Failed if an incoming call fails during set-up

Approval Request Comment
[Feature/regressing bug #]:Displaying call failed to user
[User impact if declined]:  the user would see a feedback form for a call that was not successfully completed
[Describe test coverage new/current, TBPL]: unit tests, manually verified
[Risks and why]: No risk outside of Hello; low risk to Hello; without this, users who couldn't connect would see a feedback form for a call that didn't happen -- which would be confusing and could skew user metrics (which product is depending on)
[String/UUID change made/needed]: No strings
Attachment #8511028 - Flags: approval-mozilla-beta?
Attachment #8511028 - Flags: approval-mozilla-aurora?
(In reply to Wes Kocher (:KWierso) from comment #20)
> https://hg.mozilla.org/mozilla-central/rev/705cec97abf1

Paul, can you please confirm this is fixed in the latest Nightly?
Flags: needinfo?(paul.silaghi)
QA Contact: anthony.s.hughes → paul.silaghi
Tested on Nightly 10/25; looks good.
Comment on attachment 8511028 [details] [diff] [review]
Desktop client should display Call Failed if an incoming call fails during set-up

Aurora+

The plan is to verify on Aurora on Sunday before uplift for beta4.
Attachment #8511028 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified on Aurora Nightly 10/26 Linux
Comment on attachment 8511028 [details] [diff] [review]
Desktop client should display Call Failed if an incoming call fails during set-up

Beta+
Attachment #8511028 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #13)
> I just talked to Darrin and given that this is only going to happen in the
> case where the Desktop user is called back, it makes sense to display
> "Something went wrong" (instead of "Call Failed") -- and to not show the
> "Retry" or "Email" buttons.  At most we'd show the "cancel" button (if that
> didn't look odd).
"Something went wrong" and "Cancel" button displayed for guest users.
Verified fixed FF 34b4.
Status: RESOLVED → VERIFIED
Flags: needinfo?(paul.silaghi)
You need to log in before you can comment on or make changes to this bug.