Transfer widgets on room upgrade

[anoadragon453]

Transfer room widgets on room upgrade.

Sytest PR: matrix-org/sytest#603

Closes #4994

[codecov]

Codecov Report

Merging #5000 into develop will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #5000      +/-   ##
===========================================
+ Coverage    61.49%   61.49%   +<.01%     
===========================================
  Files          332      332              
  Lines        34223    34224       +1     
  Branches      5645     5645              
===========================================
+ Hits         21046    21047       +1     
  Misses       11669    11669              
  Partials      1508     1508

[erikjohnston]

Do we expect blindly copying widgets to work? Will this cause scalar to get confused with a widget being in a different room that it expects?

[anoadragon453]

Lemme test.

[anoadragon453]

Looks like it does indeed work.

[anoadragon453]

@rxl881 @jaywink Do you know whether sending the same 'add widget' event to another room would break anything? I know the state event in the second room would have the room ID of the first room, but I don't think that's actually used for anything other than for something unique.

[jaywink]

I don't see why it would be a problem, not from Scalar's point of view at least. As the rfc says, room based widgets are stored in room state. Scalar doesn't store room widgets.

Might be worth waiting for @rxl881 though and also possibly @turt2live might want to have a look?

[turt2live]

None of my stuff should care about the room ID it lives in, and if it does then I'd call that a bug in my stuff. I think Scalar might get confused about etherpad widgets migrating rooms because it appears to expect the pad name to be the room ID, however use of the widget should be fine.

[anoadragon453]

Have tested with an etherpad and was able to successfully share a document using the same pad in both rooms so... all good?

…

On 04/04/2019 21:21, Travis Ralston wrote: None of my stuff should care about the room ID it lives in, and if it does then I'd call that a bug in my stuff. I think Scalar might get confused about etherpad widgets migrating rooms because it appears to expect the pad name to be the room ID, however use of the widget should be fine. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5000 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABR7mFUHcxhk2kK-7iCYOVPWw5517BP4ks5vdl7lgaJpZM4catpt>.

[turt2live]

This is more of my concern, which is probably a Scalar bug:

(manually copied the widget verbatim into a new state event in a new room)

Edit: it might not be super clear, but the Scalar interface tells you the wrong pad URL.

[anoadragon453]

Oh, well, yeah that's just a Scalar bug. Time to file!

[erikjohnston]

What about things like go-neb which post stuff into the room? Doesn't something need to poke it to ensure it knows about the new room?

[anoadragon453]

@erikjohnston I think this would be up to go-neb to recognize a tombstone event in the sync stream and inform each plugin that a room has been upgraded, rather than a server thing.

[erikjohnston]

On further discussion with folk it sounds like scalar very much is planning to store per room per widget state, so we need to be a little bit careful here. On top of that we'd also want to support migrating integrations etc, which also have state, so we've decided to punt this for now until we have time to have a proper think about how this should all work.

[richvdh]

This also sounds like an important datapoint in the "should we just copy all the state over" debate?

Does this mean that #4994 is now descoped, or?

[anoadragon453]

I believe so. Holding pen?