Test entering rooms feature, except branded domain #92
Conversation
* With this setup, System Test and System Test Branded Domain workspaces can have same slug when room names are the same
* Skip branded domain cases for now, pending CI setup and test on Heroku review app
69a831f to
38c64e3
Compare
* Now that we have unique room slug by workspace scope, to ensure unique jitsi room name, jitsi room name is now constructed using `workspaceSlug--roomSlug` format.
| @@ -12,7 +12,7 @@ class Room < ApplicationRecord | |||
|
|
|||
| # FriendlyId's does the legwork to make the slug uri-friendly | |||
| extend FriendlyId | |||
| friendly_id :name, use: :slugged | |||
| friendly_id :name, use: :scoped, scope: :workspace | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Nice call. Perhaps adding a comment near the implementation or test linking to the docs would provide a breadcrumb for folks to follow in the future without having to immediately go through search.
| await this.driver.wait(until.elementLocated(By.css("[name='jitsiConferenceFrame0']"))); | ||
| return await this.driver.findElement(By.css("[name='jitsiConferenceFrame0']")); | ||
| await this.driver.wait(until.elementLocated(By.css("[name*='jitsiConferenceFrame']"))); | ||
| return await this.driver.findElement(By.css("[name*='jitsiConferenceFrame']")); |
There was a problem hiding this comment.
Relax the selector a bit because when entering a second room, we will have jitsiConferenceFrame1 instead.
There was a problem hiding this comment.
Hmm, that's concerning. Does that mean we're inserting a new iFrame every time we switch rooms? Is there a possibility for leaking Video/Audio to a room unintentionally then?
There was a problem hiding this comment.
Legit concern, will fix in the next pr.
| @@ -12,7 +12,7 @@ class Room < ApplicationRecord | |||
|
|
|||
| # FriendlyId's does the legwork to make the slug uri-friendly | |||
| extend FriendlyId | |||
| friendly_id :name, use: :slugged | |||
| friendly_id :name, use: :scoped, scope: :workspace | |||
There was a problem hiding this comment.
Nice call. Perhaps adding a comment near the implementation or test linking to the docs would provide a breadcrumb for folks to follow in the future without having to immediately go through search.
|
|
||
| def video_room_name(workspace, room) | ||
| "#{workspace.slug}--#{room.slug}" | ||
| end |
There was a problem hiding this comment.
Could we put this on the Room class, perhaps with a method name like cross_system_unique_identifer or something to indicate that it's used to figure out how to reference the particular room across Jitsi/DOM/etc? It's almost like a URI but not quite...
Also, are the two dashes semantically meaningful? My understanding of the direction that semantic HTML/CSS are trending towards is that: or _ seperates ideas, - seperates words, -- gives you a state.
There was a problem hiding this comment.
Apparently this is called "block element modifier" http://getbem.com/
There was a problem hiding this comment.
Yes and great name suggestion.
| Given I am in a Room | ||
| When I tap a different Room in the Room Picker | ||
| Then I am in the Room | ||
| Given the Workspace Member is in the "System Test" Workspace and in the "Listed Room 1" Room |
There was a problem hiding this comment.
Good call on moving this to the Workspace Member instead of I to make it more clear who the actor is.
| await this.driver.wait(until.elementLocated(By.css("[name='jitsiConferenceFrame0']"))); | ||
| return await this.driver.findElement(By.css("[name='jitsiConferenceFrame0']")); | ||
| await this.driver.wait(until.elementLocated(By.css("[name*='jitsiConferenceFrame']"))); | ||
| return await this.driver.findElement(By.css("[name*='jitsiConferenceFrame']")); |
There was a problem hiding this comment.
Hmm, that's concerning. Does that mean we're inserting a new iFrame every time we switch rooms? Is there a possibility for leaking Video/Audio to a room unintentionally then?
Skipping branded domain test case for now because it requires testing on Heroku review app or any deployed instance or
etc/hostsetup, will test branded domain after setting up CI.Also allow unique room slug by workspace scope so same room name in different workspace can have same room slug.
Now that we have unique room slug by workspace scope, to ensure unique Jitsi room name, Jitsi room name is now constructed using
workspaceSlug--roomSlugformat.This unique Jitsi room logic seems important and the fact that we don't have test around this seems scary but then the logic seems solid.