Copyable link to enter specific rooms

[user512]

Part of #39 & #59:
This pull request intended to allow copyable link to enter specific rooms by initialize Jitsi connection when user go to /workspaces/workspace-slug/rooms/room-slug.
Since branded-domain link looks nicer, all rooms/ workspaces links will use custom domain and use conventional RESTful path as a fallback.

Since we migrated to server side path, previous JS solution is cleaned up.

Part of #23, remove Fellow Jitser display name.

TODO:

• Switching rooms connect to another Jitsi rooms
• Gracefully handle hang up call
• Look into Copyable link to enter specific rooms #73 (comment)
• Clean up video room js.

[zspencer]

Looking quite nice! If you want me to take a stab at building on top of this I can try it this evening.

[zspencer]

Don't we still want to dispose of Jitsi when we exit a room?

[user512]

No, going to clean up unnecessary js on next commit.

[user512]

I believe what dispose() does is to remove all listeners and remove the iframe.
https://github.com/jitsi/jitsi-meet/blob/342a00a6af323d98eef33fc60f4e2a515c8e254d/modules/API/external/external_api.js#L521-L527
Since we are reloading the page anyway, so I decided to remove all logic using dispose().

[zspencer]

I am slightly worried it leaves the server thinking the connection is not closed; but we can validate that by seeing that when user A leaves by clicking a button, user B sees them leave immediately.

[user512]

Good concern, I have confirmed once user A leaves, user B sees them leave.
But I wasn't sure how the server was able to detect that.

[zspencer]

I'm not sure what current_room means in this case; but it looks kind of like we're starting to define loader methods; which makes me want to reach for a tool like decent_exposure

[user512]

I was trying to expose the room show in rooms/show.html.erb.
Could be I haven't understand the benefit of decent_exposure but don't feel like pull in a library to expose a room just yet.

[zspencer]

Yea, the value in decent_exposure is it creates a standard mechanism for exposing the data that a controller wants to expose in a way that is consistent across all controllers. I think it’s something to look into as we start wiring in pundit; or if we start defining helper methods to expose data to the UI instead of a single instance variable.

[zspencer]

I think that we want room names on branded domains to purely be an entrance point, so we don't want to overload the room_path and instead we want to continue to use workspace_room_path(workspace.slug, room.slug) when creating links.

[user512]

Just realized we want to keep the host/workspaces/:workspaces_slug/rooms/:room_slug, now I understand this, will make change.

[zspencer]

I think this is good enough to merge, but I’m worried that we’re defining custom route handlers instead of leaning into the built in Rails routing. This kind of divergence is how Rails projects start accumulating cruft that makes working with them surprising.

[zspencer]

These custom paths methods indicate to me that we’re “going off the Rails.” I’d rather we rely on workspace_path and workspace_room_path and don’t create custom path methods.

We can look into overriding those methods in a Rails-y way; but creating methods that feel like routes but aren’t in the actual routes system makes me nervous.

[user512]

I feel the same way but also saw doorkeeper was doing something similar so I thought it's ok.
https://github.com/doorkeeper-gem/doorkeeper/blob/2fd1556ade7572d01e507c0fbc53eaf6c5bbfd19/app/helpers/doorkeeper/dashboard_helper.rb#L17-L19

[user512]

One thing I don't like about this branded_domain route is it's making smoke test locally more difficult, I am more concern on how we write e2e test on this.

[zspencer]

Yea, that's why I'm trying to make sure it's something that only exists as an entrance point into the system; and not as something that the system tries to maintain consistency on. Essentially it's something you put on the business card; but it's not something we project back into the world.