1162444 - Update conversation window context view data after editing context

Status: VERIFIED FIXED

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

Description

[Mike de Boer [:mikedeboer]]

In bug 1142515 we added the ability to edit the context information that is attached to a room.
After editing the context information, the view inside the conversation window is updated accordingly, but not the rooms list inside the Loop panel.
We should fix this by emitting an 'update' event from LoopRooms once the context data has been updated.

Comment 1

[Mark Banner (:standard8)]

(In reply to Mike de Boer [:mikedeboer] from comment #0)
> After editing the context information, the view inside the conversation
> window is updated accordingly, but not the rooms list inside the Loop panel.
> We should fix this by emitting an 'update' event from LoopRooms once the
> context data has been updated.

Mike, I don't see any issues with the panel here - I've just been testing with latest fx-team and that gets updated automatically.

However, from what I see, the conversation window doesn't get updated - if you add context, the "add context" remains there, if you edit it, then the view remains with the old information.

Comment 2

[Mark Banner (:standard8)]

Attachment: Update context views when the context changes in Loop rooms.

As I still can't see an issue in the panel, I'm assuming its definitely the conversation window that has the issue.

Here's a simple patch that fixes the update in the conversation window - basically activeRoomStore needs to listen to updates from LoopRooms and update itself appropraitely.

Comment 3

[Mike de Boer [:mikedeboer]]

Comment on attachment 8609282 [details] [diff] [review]
Update context views when the context changes in Loop rooms.

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

One simple question below...

::: browser/components/loop/content/shared/js/activeRoomStore.js
@@ +201,5 @@
>              socialShareButtonAvailable: this._mozLoop.isSocialShareButtonAvailable(),
>              socialShareProviders: this._mozLoop.getSocialShareProviders()
>            }));
>  
> +          this._mozLoop.rooms.on("update:" + actionData.roomToken, this._onRoomUpated.bind(this));

no `off` ?

Comment 4

[Mark Banner (:standard8)]

Attachment: Simpler patch - update via existing mechanism

Its a good job you asked about off, as it turns out, we already have an update listening mechanism - we just weren't supplying all the required attributes for it.

Comment 5

[Mike de Boer [:mikedeboer]]

Comment on attachment 8609344 [details] [diff] [review]
Simpler patch - update via existing mechanism

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

Ah, indeed! Well done :)

Comment 6

[Pulsebot]

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

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/30f9247cf363

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 8609344 [details] [diff] [review]
Simpler patch - update via existing mechanism

Approval Request Comment
[Feature/regressing bug #]: Context in conversations for Hello
[User impact if declined]: User may be confused - after having edited context the in-room display will remain as it was before the edit with no visible evidence of the edit having been completed.
[Describe test coverage new/current, TreeHerder]: Landed in m-c, has unit tests
[Risks and why]: Simple update to take existing data and ensure it gets displayed.
[String/UUID change made/needed]: None

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/86dfb2a6b0af

Comment 10

[Bogdan Maris, Desktop QA]

Reproduced the initial issue using old Nightly build from 2015-05-22, verified across platforms (Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit) that the issue is fixed using Nightly build from 2015-05-28 and latest Aurora and Nightly builds (even though the layout was a bit changed in latest builds).