Skip to content

Commit

Permalink
Room: Don't always provide a current_room
Browse files Browse the repository at this point in the history
- #926
- #1155

There are many pages that are not in a `room`, and the `policy_scope` of
`current_room` could result in a false safety.

That said, we probably should *also* be writing `request` specs to
confirm that folks get 404 on endpoints outside of their scope...
  • Loading branch information
zspencer committed Mar 2, 2023
1 parent 0187a71 commit d923849
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 25 deletions.
13 changes: 0 additions & 13 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,6 @@ def space_repository
policy_scope(Space.includes(:rooms, entrance: [:furniture_placements]))
end

# Retrieves the room based upon the current_space and params
# @return [nil, Room]
helper_method def current_room
return nil if current_space.blank?

@current_room ||=
policy_scope(current_space.rooms).friendly.find(
params[:room_id] || params[:id]
)
rescue ActiveRecord::RecordNotFound
current_space.entrance
end

helper_method def current_access_code(room)
session.dig(room.id, "access_code")
end
Expand Down
6 changes: 4 additions & 2 deletions app/controllers/furniture_placements_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class FurniturePlacementsController < ApplicationController
include Room::ControllerMixins

def edit
respond_to do |format|
format.turbo_stream
Expand Down Expand Up @@ -56,9 +58,9 @@ def destroy
end

def find_or_build
return current_room.furniture_placements.find(params[:id]) if params[:id]
return room.furniture_placements.find(params[:id]) if params[:id]

current_room.furniture_placements.new(furniture_placement_params)
room.furniture_placements.new(furniture_placement_params)
end

def furniture_placement_params
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/rooms_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def room_params
end

helper_method def page_title
["[Convene]", current_room&.name, current_space&.name].compact.join(" - ")
["[Convene]", room&.name, current_space&.name].compact.join(" - ")
end

helper_method def room
Expand All @@ -73,7 +73,7 @@ def room_params
return unless room.persisted?

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

Expand Down
2 changes: 2 additions & 0 deletions app/controllers/utility_hookups_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

class UtilityHookupsController < ApplicationController
include Room::ControllerMixins

def index
utility_hookup
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/waiting_rooms_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ def show

def update
if waiting_room.update(params.require(:waiting_room).permit(:access_code, :redirect_url))
session[current_room.id] = {access_code: waiting_room.access_code}
session[room.id] = {access_code: waiting_room.access_code}
redirect_to waiting_room.redirect_url
else
render :show, status: :unprocessable_entity
end
end

helper_method def waiting_room
@waiting_room ||= authorize(WaitingRoom.new(room: current_room))
@waiting_room ||= authorize(WaitingRoom.new(room: room))
end
end
4 changes: 1 addition & 3 deletions app/furniture/furniture_controller.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# frozen_string_literal: true

class FurnitureController < ::ApplicationController
helper_method def room
current_space.rooms.friendly.find(params[:room_id])
end
include Room::ControllerMixins

helper_method def space
current_space
Expand Down
15 changes: 15 additions & 0 deletions app/models/room/controller_mixins.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class Room
module ControllerMixins
def self.included(controller)
controller.helper_method(:room)
end

def room
return @room if defined?(@room)

@room ||= if params[:room_id]
policy_scope(current_space.rooms).friendly.find(params[:room_id])
end
end
end
end
2 changes: 1 addition & 1 deletion app/views/furniture_placements/create.turbo_stream.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
<%= render(furniture_placement, { editing: true }) %>
<%- end %>

<%= turbo_stream.replace(:new_furniture_placement, partial: 'furniture_placements/new', locals: { furniture_placement: current_room.furniture_placements.new }) %>
<%= turbo_stream.replace(:new_furniture_placement, partial: 'furniture_placements/new', locals: { furniture_placement: room.furniture_placements.new }) %>
4 changes: 2 additions & 2 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
<%= render partial: "breadcrumbs" %>

<div>
<%- if current_room && policy(current_room).edit? %>
<%= link_to t('icons.edit'), [:edit, current_space, current_room], class: 'no-underline', aria: { label: "Configure Room"} %>
<%- if defined?(room) && room&.persisted? && policy(room).edit? %>
<%= link_to t('icons.edit'), [:edit, current_space, room], class: 'no-underline', aria: { label: "Configure Room"} %>
<%- end %>
<%= link_to "Convene", root_url %>: Space to Work, Play, or Simply Be
</div>
Expand Down

0 comments on commit d923849

Please sign in to comment.