Closed Bug 1109849 Opened 5 years ago Closed 5 years ago

Don't show the rooms feedback form if no-one has entered the room yet

Categories

(Hello (Loop) :: Client, defect, P2)

defect

Tracking

(firefox36 fixed, firefox37 fixed)

RESOLVED FIXED
mozilla37
Iteration:
37.1
Tracking Status
firefox36 --- fixed
firefox37 --- fixed
Blocking Flags:
backlog Fx36+

People

(Reporter: standard8, Assigned: rgauthier)

Details

Attachments

(1 file, 1 obsolete file)

Applicable to the standalone & desktop - if no-one has entered the room, we shouldn't display the feedback view.

If someone else has entered the room, then we should give the user the option for feedback, even if they other party left first.
backlog: --- → Fx35+
Priority: -- → P1
Summary: Don't show the rooms feedback view if no-one has entered the room yet → Don't show the rooms feedback form if no-one has entered the room yet
Assignee: nobody → rgauthier
Attachment #8538546 - Flags: review?(nperriault)
Comment on attachment 8538546 [details] [diff] [review]
Bypass the feedback form if no-one has entered the room yet

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

::: browser/components/loop/content/shared/js/activeRoomStore.js
@@ +95,5 @@
> +        // Tracks if the room has been used during this
> +        // session. 'Used' means at least one call has been placed
> +        // with it. Entering and leaving the room without seeing
> +        // anyone is not considered as 'used'
> +        used: false

Note: we need to make sure we reset this value after the feedback has been given (so that re-entering the room without reloading the page works correctly as well).
Priority: P1 → P2
I still want to get this fixed ASAP, but we'll likely only uplift it to Fx36 at this point.
backlog: Fx35+ → Fx36+
(In reply to Mark Banner (:standard8) from comment #2)
> Comment on attachment 8538546 [details] [diff] [review]
> Bypass the feedback form if no-one has entered the room yet
> 
> Review of attachment 8538546 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/components/loop/content/shared/js/activeRoomStore.js
> @@ +95,5 @@
> > +        // Tracks if the room has been used during this
> > +        // session. 'Used' means at least one call has been placed
> > +        // with it. Entering and leaving the room without seeing
> > +        // anyone is not considered as 'used'
> > +        used: false
> 
> Note: we need to make sure we reset this value after the feedback has been
> given (so that re-entering the room without reloading the page works
> correctly as well).

The method feedbackComplete resets the state, so we should be fine here.
Attachment #8538546 - Attachment is obsolete: true
Attachment #8538546 - Flags: review?(nperriault)
Comment on attachment 8540146 [details] [diff] [review]
Bypass the feedback form if no-one has entered the room yet

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

Looks and works great!
Attachment #8540146 - Flags: review?(nperriault) → review+
https://hg.mozilla.org/mozilla-central/rev/6436dc7bcf91
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment on attachment 8540146 [details] [diff] [review]
Bypass the feedback form if no-one has entered the room yet

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

[User impact if declined]: User will be presented with a feedback form if they hit the "leave" button and no one had been in the room with them.  Causes some user confusion and skew in the statistics.

[Describe test coverage new/current, TBPL]: Includes test changes, for browser and standalone

[Risks and why]: moderately low risk, very contained to loop; easy to test.  Only asking for Aurora at this time and likely will live with it in 35.
 
[String/UUID change made/needed]: none
Attachment #8540146 - Flags: approval-mozilla-aurora?
Attachment #8540146 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.