1147375 - Don't fall over if roomName isn't supplied, let users continue to use Loop

Status: RESOLVED FIXED

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

Description

[Mark Banner (:standard8)]

As we move to context for conversations we're encrypting the roomName as part of the context, and the old roomName field will stop being supplied.

Currently, the UI fails if we don't have a roomName - the panel stops listing rooms and you can't get to existing rooms. If you create a new room it doesn't stick around after restart.

We should improve that experience if a user of FF 38 upgraded then did decide to go back.

There's also the fact that writing this as a patch now makes it easier to introduce the context code, as we'll already be treating roomName as optional.

Comment 1

[Mark Banner (:standard8)]

Attachment: Don't fall over if roomName isn't supplied, let users continue to use Loop.

Simple patch to make some of the roomName parameters optional.

I've tested this locally on a store that I've started the context work on. Otherwise testing this is pretty difficult - you'd need to delete roomName from the room object in StandaloneMozLoopRooms.get, LoopRooms.getAll and LoopRooms.get.

Comment 2

[Mike de Boer [:mikedeboer]]

Comment on attachment 8583067 [details] [diff] [review]
Don't fall over if roomName isn't supplied, let users continue to use Loop.

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

::: browser/components/loop/MozLoopService.jsm
@@ +1118,5 @@
>        if (window) {
>          window.LoopUI.showNotification({
>            sound: "room-joined",
> +          // Fallback to the brand short name if the roomName isn't available.
> +          title: room.roomName || MozLoopServiceInternal.localizedStrings.get("brandShortname"),

Hmm, why not `clientShortname2`?

::: browser/components/loop/content/shared/js/roomStore.js
@@ +36,5 @@
>     */
>    var roomSchema = {
>      roomToken:    String,
>      roomUrl:      String,
> +    //roomName:     String - Optional.

nit: add a space.

Comment 3

[Mark Banner (:standard8)]

Attachment: Don't fall over if roomName isn't supplied, let users continue to use Loop. v2

Yeah, I debated between that and just the brand name, but I think we can go with client. Its just a fallback anyway.

Comment 4

[Mike de Boer [:mikedeboer]]

Comment on attachment 8583266 [details] [diff] [review]
Don't fall over if roomName isn't supplied, let users continue to use Loop. v2

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

LGTM, thanks!

Comment 5

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/fx-team/rev/c2c24e1903f4

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8583266 [details] [diff] [review]
Don't fall over if roomName isn't supplied, let users continue to use Loop. v2

Approval Request Comment
[Feature/regressing bug #]: Context in conversations for Loop
[User impact if declined]: Once context in conversations lands in 39, if a user reverts to 38 for some reason (e.g. aurora -> beta, or after merges beta -> release) then the user will not be able to access any of their existing rooms, nor enter new rooms.
[Describe test coverage new/current, TreeHerder]: Landed in fx-team
[Risks and why]: Low, makes a parameter received from the server as optional, rather than required. This at least lets the UI continue working enough that the user can access their rooms.
[String/UUID change made/needed]: None

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c2c24e1903f4

Comment 8

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8a31fbd7a448