[User story] As a Hello user, I want to share a room URL as opposed to a call URL when a direct call fails.

RESOLVED FIXED in Firefox 35

Status

P1
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: RT, Unassigned)

Tracking

unspecified
mozilla37
Points:
2
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox35+ fixed, firefox36+ fixed, firefox37 fixed)

Details

(Whiteboard: [rooms])

User Story

As a Hello user, I want to share a room URL as opposed to a call URL when a direct call fails. 

Rough implementation:
* When pressing the "Email link" on the screen informing the user that his direct call failed), rather than creating a new call URL on the server, create a new room URL on the server and provide that URL on the e-mail body
* The Room now appears in the list of rooms if the user opens the panel

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Currently when initiating a direct call to a contact, the fall-back solution consists in sharing a call-back URL as opposed to a room URL.
This bug is about replacing the shared call URL with a room URL.

The benefits would be:
* Link clicker consistency (all links clicked are rooms)
* Desktop client consistency (all URLs are rooms)
* FxOS app simplicity (current scope for next version requires support for call URLs in case you sign into an FxA where previously you generated a call URL on desktop)
* Ability to retire the call URL server implementation and link clicker UI code as the next version of Hello on FxOS gets released
(Reporter)

Updated

4 years ago
User Story: (updated)

Comment 1

4 years ago
Maire will update with info from conversation with Mark and then need info sevaan and darrin on UX.  If no string changes - uplift to Fx35.
backlog: --- → Fx36+
Flags: needinfo?(mreavy)
Priority: -- → P1

Updated

4 years ago
backlog: Fx36+ → Fx35+
Priority: P1 → P2
Whiteboard: [rooms]
Darrin, Sevaan -- To make this change, we need to know: do we share a new Rooms link, or do we give the user the option to share an existing Room?

I'm thinking we should use a new Rooms link for pre-populating the Email message.  (The user can always modify it.) And we can look to give the user an option in a future Firefox revisions.

I'm not sure we need this for Fx35, but it would be good to make the switchover so that we don't need to continue maintaining that link-clicker calling code past Fx34.
Flags: needinfo?(sfranks)
Flags: needinfo?(mreavy)
Flags: needinfo?(dhenein)
(Reporter)

Comment 3

4 years ago
Agreed, let's create a new room link.
Sevaan do you agree?
Flags: needinfo?(dhenein)
(Reporter)

Comment 4

4 years ago
Shell can you confirm if it will make it for Fx35? The reporting requirements we discuss in Fx35 depend on it (easier if only room URLs are shared).
Flags: needinfo?(sescalante)
Assignee: nobody → nperriault
Iteration: --- → 36.3
Points: --- → 2
Niko's going to start on this bug now.  If there are no surprises, we should be able to get this uplifted into Fx35.
Flags: needinfo?(sescalante) → needinfo?(standard8)
Created attachment 8530378 [details] [diff] [review]
Share a room url by email when Loop direct call fails.

This works, though I'm using the contact email to name the new room… Using the incremental name generation feature would involve a bit more refactoring, as that feature is tightly coupled with the RoomStore for now - and we don't want direct calling stuff to know about it.

Feedback and thoughts welcome.
Attachment #8530378 - Flags: feedback?(standard8)
[Tracking Requested - why for this release]:
We need this change for "Rooms", which is already enabled in Fx35.
tracking-firefox35: --- → ?
tracking-firefox36: --- → ?
(In reply to Romain Testard [:RT] from comment #3)
> Agreed, let's create a new room link.
> Sevaan do you agree?

New Room link sounds right to me.
Flags: needinfo?(sfranks)

Comment 9

4 years ago
moved to P1 based on timing that we want to uplift
Priority: P2 → P1
Flags: needinfo?(standard8)
Created attachment 8532176 [details] [diff] [review]
Share a room url by email when Loop direct call fails.

Rebased patch against latest fx-team.
Attachment #8530378 - Attachment is obsolete: true
Attachment #8530378 - Flags: feedback?(standard8)
Attachment #8532176 - Flags: review?(standard8)
Comment on attachment 8532176 [details] [diff] [review]
Share a room url by email when Loop direct call fails.

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

I don't think this is far off, but there's a couple of improvements to be had.

::: browser/components/loop/content/js/conversationViews.jsx
@@ +259,5 @@
>        });
>  
> +      this.props.dispatcher.dispatch(new sharedActions.FetchRoomEmailLink({
> +        roomOwner: navigator.mozLoop.userProfile.email,
> +        roomName: _getPreferredEmail(this.props.contact).value

I think it might be nicer to use the contact name as primary, and fallback to email.

::: browser/components/loop/content/shared/js/conversationStore.js
@@ +336,2 @@
>          if (err) {
> +          console.error("fetchRoomEmailLink error", err);

Can you make sure we've got a bug on handling the failure nicely (I think we might have one already, but would be good to reference here).

@@ +338,1 @@
>            this.trigger("error:emailLink");

We're not doing anything with the trigger, do we really need it here?
Attachment #8532176 - Flags: review?(standard8)
Hey Niko -- Just FYI: I feel this bug is more important priority-wise than bug 1104051.  

Do you think you could finish this bug off today (or tomorrow)?  Thanks!
Flags: needinfo?(nperriault)
(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #12)
> Do you think you could finish this bug off today (or tomorrow)?  Thanks!

Yes.
Flags: needinfo?(nperriault)
Created attachment 8533744 [details] [diff] [review]
Share a room url by email when Loop direct call fails.

Addressed comments.

(In reply to Mark Banner (:standard8) from comment #11)
> > +      this.props.dispatcher.dispatch(new sharedActions.FetchRoomEmailLink({
> > +        roomOwner: navigator.mozLoop.userProfile.email,
> > +        roomName: _getPreferredEmail(this.props.contact).value
> 
> I think it might be nicer to use the contact name as primary, and fallback
> to email.

Fixed.

> ::: browser/components/loop/content/shared/js/conversationStore.js
> @@ +336,2 @@
> >          if (err) {
> > +          console.error("fetchRoomEmailLink error", err);
> 
> Can you make sure we've got a bug on handling the failure nicely (I think we
> might have one already, but would be good to reference here).

We do display an error message already, in CallFailedView#render and CallFailedView#_renderError. I removed the console.error call, which was there for debugging purpose.

> @@ +338,1 @@
> >            this.trigger("error:emailLink");
> 
> We're not doing anything with the trigger, do we really need it here?

We subscribe to this event in CallFailedView#componentDidMount, that's how we can render the error message.
Attachment #8532176 - Attachment is obsolete: true
Attachment #8533744 - Flags: review?(standard8)
(Reporter)

Comment 15

4 years ago
Shell, is this confirmed for uplift in 35?
Maria, if we get this in Fx35 I think we need the same approach in the 1.1.1 mobile app (fallback to room URLs when direct call fails).
Flags: needinfo?(sescalante)
Flags: needinfo?(oteo)
Attachment #8533744 - Flags: review?(standard8) → review+
https://hg.mozilla.org/mozilla-central/rev/46bf7d2c4ca5
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(In reply to Romain Testard [:RT] from comment #15)
> Shell, is this confirmed for uplift in 35?
> Maria, if we get this in Fx35 I think we need the same approach in the 1.1.1
> mobile app (fallback to room URLs when direct call fails).

Clearing my ni, I'm also waiting for Shell's answer...
Flags: needinfo?(oteo)
I plan to ask for uplift to Fx35 today.  We were just waiting for it to land on m-c first and verify it works.  I hope to have this in Beta before the next Beta build.
Flags: needinfo?(sescalante)
Comment on attachment 8533744 [details] [diff] [review]
Share a room url by email when Loop direct call fails.

Approval Request Comment
[Feature/regressing bug #]: Rooms

[User impact if declined]: UI update for rooms - default share option when a call fails should be email a room URL, not a direct call url.

[Describe test coverage new/current, TBPL]: On m-c.  Includes test updates to make sure the right thing is presented for email.

[Risks and why]: Mostly refactoring to centralize the code for selecting what to share.  Low risk; no significant logic flow changes.

[String/UUID change made/needed]: none
Attachment #8533744 - Flags: approval-mozilla-beta?
Attachment #8533744 - Flags: approval-mozilla-aurora?
status-firefox35: --- → affected
status-firefox36: --- → affected
status-firefox37: --- → fixed
tracking-firefox35: ? → +
tracking-firefox36: ? → +
Attachment #8533744 - Flags: approval-mozilla-beta?
Attachment #8533744 - Flags: approval-mozilla-beta+
Attachment #8533744 - Flags: approval-mozilla-aurora?
Attachment #8533744 - Flags: approval-mozilla-aurora+
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.