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.

Sign up for GitHub

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

Jump to bottom

Hide room name if the room is closed #535

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Hide room name if the room is closed #535

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47efbf3
Hide room name if the room is closed
johnshaughnessy Nov 24, 2021
0be4f34
Scrub other potentially sensitive room data before displaying it on t…
johnshaughnessy Nov 25, 2021
2544c5d
Fix formatting
johnshaughnessy Nov 30, 2021
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
17 changes: 16 additions & 1 deletion lib/ret_web/controllers/page_controller.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ defmodule RetWeb.PageController do

hub_meta_tags =
Phoenix.View.render_to_string(RetWeb.PageView, "hub-meta.html",
hub: hub,
hub: hub |> maybe_scrub_room_data(),
scene: hub.scene,
Copy link
Contributor

Choose a reason for hiding this comment

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

If we were trying to "scrub" the scene we are still returning it here.

ret_meta: Ret.Meta.get_meta(include_repo: false),
available_integrations_script: {:safe, available_integrations_script |> with_script_tags},
Expand Down Expand Up @@ -792,4 +792,19 @@ defmodule RetWeb.PageController do
defp with_script_tags(script), do: "<script>#{script}</script>"

defp module_config(key), do: Application.get_env(:ret, __MODULE__)[key]

defp maybe_scrub_room_data(hub) do
if hub.entry_mode == :deny do
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably more idiomatic, not sure if better.. But you could pattern match on the entry_mode instead of using a conditional. As in:

defp maybe_scrub_room_data(%Hub{entry_mode: :deny}) do

Map.merge(hub, %{
Copy link
Contributor

Choose a reason for hiding this comment

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

I might just return a new struct here instead, we could easily add new fields and then forget to "scrub" them later.

name: "Closed Room",
description: "This room is closed.",
slug: "",
scene: nil,
scene_listing: nil,
user_data: nil
})
else
hub
end
end
end