From 8a6ccde8d9e443d7e08ff925fe08b741e626a3ad Mon Sep 17 00:00:00 2001 From: Leandro Pereira Date: Tue, 17 Dec 2024 14:31:16 -0500 Subject: [PATCH 1/2] fix: load visual editor on new pages `Beacon.Content.get_page/2` fails since there's no page saved yet. --- lib/beacon/web/beacon_assigns.ex | 2 +- lib/beacon/web/data_source.ex | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/beacon/web/beacon_assigns.ex b/lib/beacon/web/beacon_assigns.ex index dfad25d9..c6e9115a 100644 --- a/lib/beacon/web/beacon_assigns.ex +++ b/lib/beacon/web/beacon_assigns.ex @@ -74,7 +74,7 @@ defmodule Beacon.Web.BeaconAssigns do page_module = Beacon.Loader.Page.module_name(site, page.id) live_data = Beacon.Web.DataSource.live_data(site, path_info, Map.drop(query_params, ["path"])) path_params = Beacon.Router.path_params(page.path, path_info) - page_title = Beacon.Web.DataSource.page_title(site, page.id, live_data, source) + page_title = Beacon.Web.DataSource.page_title(site, page, live_data, source) components_module = Beacon.Loader.Components.module_name(site) info_handlers_module = Beacon.Loader.InfoHandlers.module_name(site) event_handlers_module = Beacon.Loader.EventHandlers.module_name(site) diff --git a/lib/beacon/web/data_source.ex b/lib/beacon/web/data_source.ex index 840172c2..8f9528c3 100644 --- a/lib/beacon/web/data_source.ex +++ b/lib/beacon/web/data_source.ex @@ -7,8 +7,8 @@ defmodule Beacon.Web.DataSource do Beacon.apply_mfa(site, Beacon.Loader.fetch_live_data_module(site), :live_data, [path_info, params]) end - def page_title(site, page_id, live_data, source) do - %{path: path, title: title} = page_assigns = page_assigns(site, page_id, source) + def page_title(site, page, live_data, source) do + %{path: path, title: title} = page_assigns = page_assigns(site, page, source) with {:ok, page_title} <- Beacon.Content.render_snippet(title, %{page: page_assigns, live_data: live_data}) do page_title @@ -58,13 +58,14 @@ defmodule Beacon.Web.DataSource do end # only published pages will have the title evaluated for beacon - defp page_assigns(site, page_id, :beacon) do - Beacon.apply_mfa(site, Beacon.Loader.fetch_page_module(site, page_id), :page_assigns, []) + defp page_assigns(site, page, :beacon) do + Beacon.apply_mfa(site, Beacon.Loader.fetch_page_module(site, page.id), :page_assigns, []) end # beacon_live_admin needs the title for unpublished pages also - defp page_assigns(site, page_id, :admin) do - page = Beacon.Content.get_page(site, page_id) + defp page_assigns(site, page, :admin) do + # new pages are not yet in the database + page = if page.id, do: Beacon.Content.get_page(site, page.id), else: page %{ id: page.id, From a1f003e27fe0c6c51ddb2143d0706a7aa31fc658 Mon Sep 17 00:00:00 2001 From: Leandro Pereira Date: Wed, 18 Dec 2024 13:11:05 -0500 Subject: [PATCH 2/2] rework @beacon assign - remove the need to pass around the source - simplify building it --- lib/beacon/router.ex | 2 + lib/beacon/template.ex | 4 +- lib/beacon/web/beacon_assigns.ex | 56 +++++---------------- lib/beacon/web/controllers/api/page_json.ex | 2 +- lib/beacon/web/data_source.ex | 36 +++++++------ lib/beacon/web/live/page_live.ex | 14 +++--- test/beacon_web/beacon_assigns_test.exs | 56 ++++++++++++++------- test/beacon_web/data_source_test.exs | 4 +- 8 files changed, 84 insertions(+), 90 deletions(-) diff --git a/lib/beacon/router.ex b/lib/beacon/router.ex index debb1224..9bbfbc69 100644 --- a/lib/beacon/router.ex +++ b/lib/beacon/router.ex @@ -250,6 +250,8 @@ defmodule Beacon.Router do end) end + def path_params(_page_path, _path_info), do: %{} + @doc false # Tells if a `beacon_site` is reachable in the current environment. # diff --git a/lib/beacon/template.ex b/lib/beacon/template.ex index ea7e58ff..36df6805 100644 --- a/lib/beacon/template.ex +++ b/lib/beacon/template.ex @@ -27,8 +27,8 @@ defmodule Beacon.Template do page -> page_module = Beacon.Loader.fetch_page_module(page.site, page.id) - live_data = Beacon.Web.DataSource.live_data(site, path_info) - beacon_assigns = BeaconAssigns.new(site, page, live_data, path_info, query_params, :beacon) + live_data = Beacon.Web.DataSource.live_data(site, path_info, query_params) + beacon_assigns = BeaconAssigns.new(page, path_info: path_info, query_params: query_params) assigns = Map.put(live_data, :beacon, beacon_assigns) env = Beacon.Web.PageLive.make_env(site) diff --git a/lib/beacon/web/beacon_assigns.ex b/lib/beacon/web/beacon_assigns.ex index c6e9115a..454a0b64 100644 --- a/lib/beacon/web/beacon_assigns.ex +++ b/lib/beacon/web/beacon_assigns.ex @@ -33,8 +33,8 @@ defmodule Beacon.Web.BeaconAssigns do } defstruct site: nil, - path_params: %{}, - query_params: %{}, + path_params: nil, + query_params: nil, page: %{path: nil, title: nil}, private: %{ page_module: nil, @@ -53,31 +53,18 @@ defmodule Beacon.Web.BeaconAssigns do end @doc false - def new(site, %Beacon.Content.Page{} = page, variant_roll) do - components_module = Beacon.Loader.Components.module_name(site) - page_module = Beacon.Loader.Page.module_name(site, page.id) + def new(%Beacon.Content.Page{} = page, metadata \\ []) do + path_info = Keyword.get(metadata, :path_info, []) + query_params = Keyword.get(metadata, :query_params, %{}) + variant_roll = Keyword.get(metadata, :variant_roll, nil) - %__MODULE__{ - site: site, - private: %{ - components_module: components_module, - page_module: page_module, - variant_roll: variant_roll - } - } - end - - @doc false - def new(site, %Beacon.Content.Page{} = page, live_data, path_info, query_params, source, variant_roll \\ nil) - when is_atom(site) and is_map(live_data) and is_list(path_info) and is_map(query_params) and source in [:beacon, :admin] do - %{site: ^site} = page - page_module = Beacon.Loader.Page.module_name(site, page.id) - live_data = Beacon.Web.DataSource.live_data(site, path_info, Map.drop(query_params, ["path"])) + page_module = Beacon.Loader.Page.module_name(page.site, page.id) + live_data = Beacon.Web.DataSource.live_data(page.site, path_info, Map.drop(query_params, ["path"])) path_params = Beacon.Router.path_params(page.path, path_info) - page_title = Beacon.Web.DataSource.page_title(site, page, live_data, source) - components_module = Beacon.Loader.Components.module_name(site) - info_handlers_module = Beacon.Loader.InfoHandlers.module_name(site) - event_handlers_module = Beacon.Loader.EventHandlers.module_name(site) + page_title = Beacon.Web.DataSource.page_title(page, live_data) + components_module = Beacon.Loader.Components.module_name(page.site) + info_handlers_module = Beacon.Loader.InfoHandlers.module_name(page.site) + event_handlers_module = Beacon.Loader.EventHandlers.module_name(page.site) %__MODULE__{ site: page.site, @@ -95,23 +82,4 @@ defmodule Beacon.Web.BeaconAssigns do } } end - - @doc false - def update(_socket_or_assigns, _key, _value) - - def update(%{assigns: %{beacon: _beacon}} = socket, key, value) do - do_update_socket_or_assigns(socket, key, value) - end - - def update(%{beacon: _beacon} = assigns, key, value) do - do_update_socket_or_assigns(assigns, key, value) - end - - def update(_socket_or_assigns, _key, _value), do: raise("expected :beacon assign in socket but none found") - - defp do_update_socket_or_assigns(socket_or_assigns, key, value) do - Phoenix.Component.update(socket_or_assigns, :beacon, fn beacon -> - Map.put(beacon, key, value) - end) - end end diff --git a/lib/beacon/web/controllers/api/page_json.ex b/lib/beacon/web/controllers/api/page_json.ex index 0ecc1157..cd4a651e 100644 --- a/lib/beacon/web/controllers/api/page_json.ex +++ b/lib/beacon/web/controllers/api/page_json.ex @@ -23,7 +23,7 @@ defmodule Beacon.Web.API.PageJSON do defp data(%Page{} = page) do path_info = for segment <- String.split(page.path, "/"), segment != "", do: segment live_data = Beacon.Web.DataSource.live_data(page.site, path_info, %{}) - beacon_assigns = BeaconAssigns.new(page.site, page, live_data, path_info, %{}, :admin) + beacon_assigns = BeaconAssigns.new(page, path_info: path_info) assigns = live_data diff --git a/lib/beacon/web/data_source.ex b/lib/beacon/web/data_source.ex index 8f9528c3..8b3f4e96 100644 --- a/lib/beacon/web/data_source.ex +++ b/lib/beacon/web/data_source.ex @@ -3,12 +3,14 @@ defmodule Beacon.Web.DataSource do require Logger - def live_data(site, path_info, params \\ %{}) when is_atom(site) and is_list(path_info) and is_map(params) do - Beacon.apply_mfa(site, Beacon.Loader.fetch_live_data_module(site), :live_data, [path_info, params]) + def live_data(site, path_info, query_params) when is_atom(site) and is_list(path_info) and is_map(query_params) do + Beacon.apply_mfa(site, Beacon.Loader.fetch_live_data_module(site), :live_data, [path_info, query_params]) end - def page_title(site, page, live_data, source) do - %{path: path, title: title} = page_assigns = page_assigns(site, page, source) + def live_data(_site, _path_info, _query_params), do: %{} + + def page_title(%Beacon.Content.Page{} = page, live_data) do + %{path: path, title: title} = page_assigns = page_assigns(page) with {:ok, page_title} <- Beacon.Content.render_snippet(title, %{page: page_assigns, live_data: live_data}) do page_title @@ -19,7 +21,7 @@ defmodule Beacon.Web.DataSource do will return the original unmodified page title - site: #{site} + site: #{page.site} title: #{title} page path: #{path} @@ -36,13 +38,14 @@ defmodule Beacon.Web.DataSource do # TODO: revisit this logic to evaluate meta_tags for unpublished pages def meta_tags(assigns) do %{beacon: %{site: site, private: %{page_module: page_module, live_data_keys: live_data_keys}}} = assigns - %{site: ^site, id: page_id} = Beacon.apply_mfa(site, page_module, :page_assigns, [[:site, :id]]) + %{site: ^site} = page_assigns = Beacon.apply_mfa(site, page_module, :page_assigns, []) + live_data = Map.take(assigns, live_data_keys) assigns |> Beacon.Web.Layouts.meta_tags() |> List.wrap() - |> Enum.map(&interpolate_meta_tag(&1, %{page: page_assigns(site, page_id, :beacon), live_data: live_data})) + |> Enum.map(&interpolate_meta_tag(&1, %{page: page_assigns, live_data: live_data})) end defp interpolate_meta_tag(meta_tag, values) when is_map(meta_tag) do @@ -57,16 +60,21 @@ defmodule Beacon.Web.DataSource do end end - # only published pages will have the title evaluated for beacon - defp page_assigns(site, page, :beacon) do - Beacon.apply_mfa(site, Beacon.Loader.fetch_page_module(site, page.id), :page_assigns, []) + # return the page assigns from unpublished page assigns, + # either saved in the database or page in-memory (for new pages) + # this fallback is here mostly to support beacon_live_admin visual editor, + # which makes use of the `@beacon` assign when creating or editing pages + defp page_assigns(%Beacon.Content.Page{id: nil} = page) do + unpublished_page_assigns(page) end - # beacon_live_admin needs the title for unpublished pages also - defp page_assigns(site, page, :admin) do - # new pages are not yet in the database - page = if page.id, do: Beacon.Content.get_page(site, page.id), else: page + defp page_assigns(%Beacon.Content.Page{} = page) do + Beacon.apply_mfa(page.site, Beacon.Loader.fetch_page_module(page.site, page.id), :page_assigns, []) + rescue + _ -> unpublished_page_assigns(page) + end + defp unpublished_page_assigns(page) do %{ id: page.id, site: page.site, diff --git a/lib/beacon/web/live/page_live.ex b/lib/beacon/web/live/page_live.ex index de0466b3..159dab72 100644 --- a/lib/beacon/web/live/page_live.ex +++ b/lib/beacon/web/live/page_live.ex @@ -39,7 +39,7 @@ defmodule Beacon.Web.PageLive do end page = RouterServer.lookup_page!(site, path) - socket = Component.assign(socket, beacon: BeaconAssigns.new(site, page, variant_roll)) + socket = Component.assign(socket, beacon: BeaconAssigns.new(page, variant_roll: variant_roll)) {:ok, socket, layout: {Beacon.Web.Layouts, :dynamic}} end @@ -130,16 +130,14 @@ defmodule Beacon.Web.PageLive do site -> %{"path" => path_info} = params - if socket.assigns.beacon.site != site do - if Beacon.Config.fetch!(site).mode == :live do - Beacon.PubSub.unsubscribe_to_page(socket.assigns.beacon.site, path_info) - Beacon.PubSub.subscribe_to_page(site, path_info) - end + if socket.assigns.beacon.site != site && Beacon.Config.fetch!(site).mode == :live do + Beacon.PubSub.unsubscribe_to_page(socket.assigns.beacon.site, path_info) + Beacon.PubSub.subscribe_to_page(site, path_info) end page = RouterServer.lookup_page!(site, path_info) live_data = Beacon.Web.DataSource.live_data(site, path_info, Map.drop(params, ["path"])) - beacon_assigns = BeaconAssigns.new(site, page, live_data, path_info, params, :beacon, socket.assigns.beacon.private.variant_roll) + beacon_assigns = BeaconAssigns.new(page, path_info: path_info, query_params: params, variant_roll: socket.assigns.beacon.private.variant_roll) socket = socket @@ -151,7 +149,7 @@ defmodule Beacon.Web.PageLive do # TODO: remove deprecated @beacon_query_params |> Component.assign(:beacon_query_params, beacon_assigns.query_params) |> Component.assign(:beacon, beacon_assigns) - |> Component.assign(:page_title, Beacon.Web.DataSource.page_title(site, page.id, live_data, :beacon)) + |> Component.assign(:page_title, beacon_assigns.page.title) {:noreply, push_event(socket, "beacon:page-updated", %{meta_tags: Beacon.Web.DataSource.meta_tags(socket.assigns)})} end diff --git a/test/beacon_web/beacon_assigns_test.exs b/test/beacon_web/beacon_assigns_test.exs index c62664f9..38acdb3e 100644 --- a/test/beacon_web/beacon_assigns_test.exs +++ b/test/beacon_web/beacon_assigns_test.exs @@ -14,12 +14,7 @@ defmodule Beacon.Web.BeaconAssignsTest do Process.put(:__beacon_site__, site) Process.flag(:error_handler, Beacon.ErrorHandler) - [ - socket: %Phoenix.LiveView.Socket{ - assigns: %{__changed__: %{beacon: true}, beacon: %BeaconAssigns{}} - }, - site: site - ] + [site: site] end test "build with site", %{site: site} do @@ -29,10 +24,10 @@ defmodule Beacon.Web.BeaconAssignsTest do } = BeaconAssigns.new(site) end - test "build with published page resolves page title", %{site: site} do + test "build with published page resolves page title" do page = beacon_published_page_fixture(path: "/blog", title: "blog index") - assigns = BeaconAssigns.new(site, page, %{}, ["blog"], %{}, :beacon) + assigns = BeaconAssigns.new(page, path_info: ["blog"]) assert %BeaconAssigns{ site: @site, @@ -43,10 +38,38 @@ defmodule Beacon.Web.BeaconAssignsTest do } = assigns end - test "build with path info and query params", %{site: site} do + test "build with unpublished page stored in the database" do + page = beacon_page_fixture(path: "/blog", title: "blog index") + + assigns = BeaconAssigns.new(page, path_info: ["blog"]) + + assert %BeaconAssigns{ + site: @site, + page: %{path: "/blog", title: "blog index"}, + private: %{ + live_path: ["blog"] + } + } = assigns + end + + test "build with new in-memory page " do + page = %Beacon.Content.Page{site: @site, path: "/blog", title: "blog index"} + + assigns = BeaconAssigns.new(page, path_info: ["blog"]) + + assert %BeaconAssigns{ + site: @site, + page: %{path: "/blog", title: "blog index"}, + private: %{ + live_path: ["blog"] + } + } = assigns + end + + test "build with path info and query params" do page = beacon_published_page_fixture(path: "/blog") - assigns = BeaconAssigns.new(site, page, %{}, ["blog"], %{source: "search"}, :beacon) + assigns = BeaconAssigns.new(page, path_info: ["blog"], query_params: %{source: "search"}) assert %BeaconAssigns{ site: @site, @@ -57,10 +80,10 @@ defmodule Beacon.Web.BeaconAssignsTest do } = assigns end - test "build with path params", %{site: site} do + test "build with path params" do page = beacon_published_page_fixture(path: "/blog/:post") - assigns = BeaconAssigns.new(site, page, %{}, ["blog", "hello"], %{}, :beacon) + assigns = BeaconAssigns.new(page, path_info: ["blog", "hello"]) assert %BeaconAssigns{ site: @site, @@ -68,13 +91,13 @@ defmodule Beacon.Web.BeaconAssignsTest do } = assigns end - test "build with live data", %{site: site} do + test "build with live data" do page = beacon_published_page_fixture(path: "/blog") live_data = beacon_live_data_fixture(path: "/blog") beacon_live_data_assign_fixture(live_data: live_data, format: :text, key: "customer_id", value: "123") - assigns = BeaconAssigns.new(site, page, live_data, ["blog"], %{}, :beacon) + assigns = BeaconAssigns.new(page, path_info: ["blog"]) assert %BeaconAssigns{ site: @site, @@ -83,9 +106,4 @@ defmodule Beacon.Web.BeaconAssignsTest do } } = assigns end - - test "update/3", %{socket: socket} do - assert %{assigns: %{beacon: %BeaconAssigns{site: "one"}}} = BeaconAssigns.update(socket, :site, "one") - assert %{assigns: %{beacon: %BeaconAssigns{site: "two"}}} = BeaconAssigns.update(socket, :site, "two") - end end diff --git a/test/beacon_web/data_source_test.exs b/test/beacon_web/data_source_test.exs index 53f5a05e..f35eaffd 100644 --- a/test/beacon_web/data_source_test.exs +++ b/test/beacon_web/data_source_test.exs @@ -24,12 +24,12 @@ defmodule Beacon.Web.DataSourceTest do describe "page_title" do test "renders static content", %{site: site} do page = beacon_published_page_fixture(site: site, title: "my title") - assert DataSource.page_title(page.site, page.id, %{}, :beacon) == "my title" + assert DataSource.page_title(page, %{}) == "my title" end test "renders snippet", %{site: site} do page = beacon_published_page_fixture(site: site, title: "{{ page.path | upcase }}") - assert DataSource.page_title(page.site, page.id, %{}, :beacon) == "/HOME" + assert DataSource.page_title(page, %{}) == "/HOME" end end end