🧹 Rooms: Get rid of locked/unlocked/internal Rooms in favor of public/internal
#1183
Conversation
zspencer
force-pushed
the
rooms/delete-wating-rooms
branch
from
d3f6248 to
7d4613f
Compare
zspencer
force-pushed
the
rooms/delete-wating-rooms
branch
3 times, most recently
from
f3ee93c to
96492d2
Compare
zspencer
force-pushed
the
rooms/delete-wating-rooms
branch
from
96492d2 to
5d49843
Compare
zspencer
force-pushed
the
rooms/delete-wating-rooms
branch
from
5d49843 to
c819266
Compare
| access_level&.to_sym == :unlocked | ||
| end | ||
| # `internal` only Members may access the Room | ||
| attribute :access_level, :string, default: :public |
There was a problem hiding this comment.
Not from this PR, and not blocking, but I'm surprised this is not an enum. It would save us from defining the internal? and public? methods, and also help make sure we don't accidentally set it to something that is not in the enum (like, say, during a refactor...).
enum :access_level, {
internal: "internal",
public: "public"
}, default: :publicThen our one use of Room::ACCESS_LEVELS.map(&:to_s) becomes instead Room.access_levels.values.
There was a problem hiding this comment.
I considered piling that into this PR, but dissuaded myself because of the weirdness I ran into with the ENUMs on Heroku last time.
spec/policies/room_policy_spec.rb
Outdated
| @@ -55,16 +42,14 @@ | |||
| it "includes all the rooms" do | |||
| space = create(:space) | |||
| internal_room = create(:room, :internal, space: space) | |||
| locked_room = create(:room, :locked, space: space) | |||
| unlocked_room = create(:room, :unlocked, space: space) | |||
| public_rom = create(:room, :public, space: space) | |||
There was a problem hiding this comment.
public_roOm, here and on line 52
| @@ -101,38 +101,27 @@ def space_attributes | |||
| { | |||
| name: "Listed Room 1", | |||
| publicity_level: :listed, | |||
| access_level: :unlocked, | |||
| access_code: nil, | |||
| access_level: :public, | |||
| furniture_placements: { | |||
| markdown_text_block: {content: "# Welcome!"} | |||
| } | |||
| }, | |||
| { | |||
| name: "Listed Room 2", | |||
There was a problem hiding this comment.
Should we rename all of these to "Public", instead of "Listed"?
There was a problem hiding this comment.
Oo, I remember why I didn't!!! If I had, it would have broken feature tests which have those names hardcoded... And technically they are listed they're just also public, since listed indicates whether or not they are enumerated in the navigation menu.
Sections#1155Gizmo:Usher#1151It's not really serving us, and is a hold-over from when Convene was
more video-forward.
Instead, we may want to create a
Furniturethat can "bounce" peoplefrom a
Roomunless they know the secret code / areMembers of theSpaceor in an appropriateGroup.