Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Space: BYO Domains drop Space from polymorphic_urls #1000

Merged
merged 3 commits into from
Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,38 @@ class ApplicationController < ActionController::Base

helper_method :current_person

helper_method def url_for(options)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two methods are arcane enough that I think they warrant at least a one-sentence comment explaining that they augment the standard Rails helpers to make URLs on branded domains nicer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, I've linked to the core docs and included a sentance.

space = if options[0].is_a?(Space) && options[0].branded_domain.present?
options.delete_at(0)
elsif [:edit, :new].include?(options[0]) && options[1].is_a?(Space) && options[1].branded_domain.present?
options.delete_at(1)
elsif options.is_a?(Space) && options.branded_domain.present?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it is worth it, or if it would make the code even harder to understand, but these checks for "is this a Space and does it have a branded domain" could be tightened up like:

thing_were_checking.try(:branded_domain).present?

Which is a little bit more flexible (will work for any object that has a branded_domain, even if it is not a Space), but I think works for our purposes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always forget about .try(:foo)!

options
end

if space
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could return early unless space.present?, I think it would be a tad more readable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I need to return super unless space but ya!

if options.respond_to?(:last) && options.last.is_a?(Hash)
options.last[:domain] = space.branded_domain
elsif options.respond_to?(:<<) && options.length > 0
options << {domain: space.branded_domain}
else
options = [:root, {domain: space.branded_domain}]
end
end

super
end

helper_method def polymorphic_path(options, **attributes)
if options[0].is_a?(Space) && options[0].branded_domain.present? && options.length > 1
options.delete_at(0)
elsif [:edit, :new].include?(options[0]) && options[1].is_a?(Space) && options[1].branded_domain.present?
options.delete_at(1)
end

super
end

private

OPERATOR_TOKEN = ENV["OPERATOR_API_KEY"]
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/authenticated_sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ def create
end
end

alias_method :update, :create

def destroy
authenticated_session.destroy
redirect_to current_space
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/furniture_placements_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ def create
if furniture_placement.save!
format.html do
redirect_to(
space_room_path(furniture_placement.room.space, furniture_placement.room),
[furniture_placement.room.space, furniture_placement.room],
notice: t(".success", name: furniture_placement.furniture.model_name.human)
)
end
Expand All @@ -25,7 +25,7 @@ def update
if furniture_placement.update!(furniture_placement_params)
format.html do
redirect_to(
edit_space_room_path(furniture_placement.room.space, furniture_placement.room),
[:edit, furniture_placement.room.space, furniture_placement.room],
notice: t(".success", name: furniture_placement.furniture.model_name.human)
)
end
Expand All @@ -38,7 +38,7 @@ def destroy
respond_to do |format|
format.html do
redirect_to(
space_room_path(furniture_placement.room.space, furniture_placement.room),
[furniture_placement.room.space, furniture_placement.room],
notice: t(".success", name: furniture_placement.furniture.model_name.human.titleize)
)
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/memberships_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def destroy

respond_to do |format|
format.html do
redirect_to(space_memberships_path(membership.space))
redirect_to([membership.space, :memberships])
end

format.turbo_stream do
Expand Down
12 changes: 6 additions & 6 deletions app/controllers/rooms_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def edit
def create
if room.save
flash[:notice] = t(".success", room_name: room.name)
redirect_to edit_space_room_path(room.space, room)
redirect_to [:edit, room.space, room]
else
render :new, status: :unprocessable_entity
end
Expand All @@ -29,7 +29,7 @@ def update
respond_to do |format|
if room.update(room_params)
format.html do
redirect_to edit_space_path(room.space), notice: t(".success", room_name: room.name)
redirect_to [:edit, room.space], notice: t(".success", room_name: room.name)
end
else
format.html { render :edit, status: :unprocessable_entity }
Expand All @@ -41,7 +41,7 @@ def update

def destroy
if room.destroy
redirect_to edit_space_path(room.space), notice: t(".success", room_name: room.name)
redirect_to [:edit, room.space], notice: t(".success", room_name: room.name)
else
flash.now[:alert] = t(".failure", room_name: room.name)
render :edit
Expand Down Expand Up @@ -69,16 +69,16 @@ def room_params
return unless room.persisted?

unless room.enterable?(current_access_code(room))
redirect_to space_room_waiting_room_path(current_space, current_room, redirect_url: after_authorization_redirect_url)
redirect_to [current_space, current_room, :waiting_room, redirect_url: after_authorization_redirect_url]
end
end

# TODO: Unit test authorize and redirect url
private def after_authorization_redirect_url
if %i[edit update].include?(action_name.to_sym)
return edit_space_room_path(room.space, room)
return url_for([:edit, room.space, room])
end

space_room_path(room.space, room)
url_for([room.space, room])
end
end
2 changes: 1 addition & 1 deletion app/controllers/spaces_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def create
def update
if space.update(space_params)
flash[:notice] = t(".success")
redirect_to space_path(space)
redirect_to space
else
flash.now[:alert] = t(".error")
render :edit
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/utility_hookups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ def edit

def create
if utility_hookup.save
redirect_to edit_space_path(space)
redirect_to [:edit, space]
else
render :new
end
end

def update
if utility_hookup.update(utility_hookup_params)
redirect_to edit_space_path(space)
redirect_to [:edit, space]
else
render :edit
end
Expand Down
6 changes: 1 addition & 5 deletions app/helpers/rooms_helper.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
module RoomsHelper
def end_call_path(space)
if space.branded_domain.present?
"https://#{space.branded_domain}/"
else
space_path(space)
end
[space]
end
end
2 changes: 1 addition & 1 deletion app/models/authenticated_session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def authentication_method_id=(authentication_method_id)
end

def persisted?
false
true
end

def destroy
Expand Down
2 changes: 1 addition & 1 deletion app/views/authenticated_sessions/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%= form_with model: authenticated_session, url: space_authenticated_session_path(current_space), local: true do |form| %>
<%= form_with model: [current_space, authenticated_session] do |form| %>
<fieldset>
<%- if form.object.contact_location.blank? %>
<%= form.hidden_field :contact_method, value: :email %>
Expand Down
6 changes: 3 additions & 3 deletions app/views/layouts/_header.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
<nav aria-label="Menu" class="profile-menu" data-person-email="<%= current_person.email %>">
<%- if current_person.authenticated? %>
<%- if current_membership.present? %>
<%= link_to "Account", space_membership_path(current_space, current_membership), title: current_membership.member.email %>
<%= link_to "Account", [current_space, current_membership], title: current_membership.member.email %>
<%- end %>
<%= link_to "Sign out", space_authenticated_session_path(current_space), data: { turbo: true, turbo_method: :delete }, class: 'sign-out' %>
<%= link_to "Sign out", [current_space, :authenticated_session], data: { turbo: true, turbo_method: :delete }, class: 'sign-out' %>
<%- else %>
<%= link_to "Sign in", new_space_authenticated_session_path(current_space), class: "sign-in" %>
<%= link_to "Sign in", [:new, current_space, :authenticated_session], class: "sign-in" %>
<%- end %>
</nav>
</header>
Expand Down
2 changes: 1 addition & 1 deletion app/views/spaces/_room_card.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</header>
<footer>
<div class="room-door_enter">
<%= link_to space_room_path(room.space, room) do %>
<%= link_to [room.space, room] do %>
<span class="icon --enter" role="img" aria-label="Enter <%= room.name %>">Enter Room</span>
<% end %>
</div>
Expand Down
4 changes: 2 additions & 2 deletions app/views/spaces/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<%- space.rooms.each do |room| %>
<%- if policy(room).edit? %>
<div data-access-level="<%=room.access_level %>" data-slug="<%=room.slug%>" data-model="room" data-id="<%=room.id%>">
<%= link_to edit_space_room_path(room.space, room) do %>
<%= link_to [:edit, room.space, room] do %>
<span class="icon --configure" role="img" aria-label="Configure"></span><%= room.name %>
<% end %>
</div>
Expand All @@ -26,7 +26,7 @@
</div>

<footer>
<%= link_to new_space_room_path(space) do %>
<%= link_to [:new, space, :room] do %>
<span class="icon --add" role="img" aria-label="Add"></span>Add New Room
<% end %>
</footer>
Expand Down
4 changes: 2 additions & 2 deletions app/views/utility_hookups/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%= form_with(model: utility_hookup, url: space_utility_hookup_path(utility_hookup.space, utility_hookup), local: true) do |form| %>
<%= form_with(model: [space, utility_hookup]) do |form| %>
<%= form.hidden_field :utility_slug %>
<header>
<h4>
Expand All @@ -15,4 +15,4 @@
<%= form.submit "Save changes to #{utility_hookup.utility.display_name} ✔️" %>
<%- end %>
</footer>
<%- end %>
<%- end %>
4 changes: 2 additions & 2 deletions app/views/waiting_rooms/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
<div class="min-h-screen flex items-center justify-between">
<div class="m-auto">
<div class="bg-neutral-500 text-white font-extrabold px-8 py-4 rounded">
<%= current_room.name %>
<%= waiting_room.room.name %>
</div>
</div>

<%= form_with model: waiting_room, url: space_room_waiting_room_path(waiting_room.space, waiting_room.room), class: "access-code-form", local: true do |form| %>
<%= form_with model: [waiting_room.room.space, waiting_room.room, waiting_room], class: "access-code-form", local: true do |form| %>
<%= render "password_field", form: form, attribute: :access_code%>

<%= form.hidden_field :redirect_url %>
Expand Down
13 changes: 6 additions & 7 deletions config/breadcrumbs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@
# @see https://github.com/kzkn/gretel
crumb :root do
if current_space.present?
link current_space.name, space_path(current_space)
link current_space.name, current_space
else
link t("home.title"), root_path
end
end

crumb :edit_space do |space|
link "Configure Space", edit_space_path(space)
link "Configure Space", [:edit, space]
end

crumb :memberships do |space|
link "Members", [space, :memberships]
if policy(space).edit?
parent :edit_space, Space
parent :edit_space, space
else
parent :root
end
Expand All @@ -33,7 +33,7 @@
end

crumb :utility_hookups do |space|
link "Utility Hookups", space_utility_hookups_path(space)
link "Utility Hookups", [space, :utility_hookups]
parent :edit_space, space
end

Expand All @@ -51,12 +51,12 @@
end

crumb :new_room do |room|
link t("helpers.submit.room.create"), new_space_room_path(room.space)
link t("helpers.submit.room.create"), [:new, room.space, :room]
parent :edit_space, room.space
end

crumb :edit_room do |room|
link t("helpers.submit.room.edit", {room_name: room.name}), edit_space_room_path(room.space, room)
link t("helpers.submit.room.edit", {room_name: room.name}), [:edit, room.space, room]
parent :room, room
end

Expand All @@ -74,4 +74,3 @@
link "Configure #{furniture_placement.title}"
parent :edit_room, furniture_placement.room
end

20 changes: 17 additions & 3 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
# post "/auth/:provider/callback", "sessions#create"
# For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html
resources :spaces, only: %I[show edit update create destroy] do
resource :authenticated_session, only: %i[new create destroy show]
resource :authenticated_session, only: %i[new create update destroy show]

resources :invitations, only: %i[create destroy index] do
resource :rsvp, only: %i[show update]
Expand Down Expand Up @@ -41,8 +41,22 @@
end

constraints BrandedDomainConstraint.new(Space) do
resources :authenticated_sessions, only: %i[new create delete show]
get :edit, to: "spaces#edit"
get "/" => "spaces#show"
put "/" => "spaces#update"

get "/:id", to: "rooms#show"
resources :invitations, only: %i[create destroy index] do
resource :rsvp, only: %i[show update]
end

resources :memberships, only: %i[index show destroy]

resources :utility_hookups, only: %I[create edit update destroy index]

resource :authenticated_session, only: %i[new create update destroy show]

resources :rooms, only: %i[show edit update new create destroy] do
Furniture.append_routes(self)
end
end
end
10 changes: 5 additions & 5 deletions features/harness/RoomEditPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ class RoomEditPage extends Page {
* @param {AccessCode} accessCode
* @returns {Promise<this>}
*/
async unlock(accessCode) {
unlock(accessCode) {
const waitingRoom = new WaitingRoomPage(this.driver);
await waitingRoom.submitAccessCode(accessCode);
await this.accessLevel().select("unlocked");
await this.submitButton().click();
return this;
return waitingRoom.submitAccessCode(accessCode)
.then(() => this.accessLevel().select('unlocked'))
.then(() => this.submitButton().click())
.finally(() => this)
}
/**
* @param {string} accessLevel
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/spaces/rooms_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
it "removes the room" do
delete path
expect { room.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect(response).to redirect_to(edit_space_path(room.space))
expect(response).to redirect_to([:edit, room.space])
expect(flash[:notice]).to eql(I18n.t("rooms.destroy.success", room_name: room.name))
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/spaces/utility_hookups_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
it "Updates the Utility Hookup" do
expect { perform_request }.to(change { utility_hookup.reload.attributes })

expect(response).to redirect_to edit_space_path(space)
expect(response).to redirect_to [:edit, space]
expect(utility_hookup.utility_slug).to eq(utility_hookup_attributes[:utility_slug])
expect(utility_hookup.utility.configuration).to eq(utility_hookup_attributes[:utility_attributes])
end
Expand Down Expand Up @@ -109,7 +109,7 @@
expect { perform_request }
.to(change { space.utility_hookups.count }.by(1))

expect(response).to redirect_to edit_space_path(space)
expect(response).to redirect_to [:edit, space]
expect(space.utility_hookups.last.utility_slug).to eql("jitsi")
expect(space.utility_hookups.last.utility.configuration).to eq(utility_hookup_attributes[:utility_attributes])
end
Expand Down
Loading