From 11a95443ed3004e9275da03152432f5caa9a230b Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Thu, 7 Mar 2024 10:28:12 -0800 Subject: [PATCH] Update onboarding flow to use settings model instead of session data --- lib/pinchflat_web.ex | 8 ++++ .../media_profile_controller.ex | 22 +++++------ .../media_profile_html/new.html.heex | 2 +- .../controllers/pages/page_controller.ex | 5 ++- .../controllers/sources/source_controller.ex | 38 +++++++++---------- .../sources/source_html/new.html.heex | 2 +- .../media_profile_controller_test.exs | 37 +++++++++--------- .../controllers/page_controller_test.exs | 12 +++--- .../controllers/source_controller_test.exs | 32 +++++++--------- 9 files changed, 83 insertions(+), 75 deletions(-) diff --git a/lib/pinchflat_web.ex b/lib/pinchflat_web.ex index 6f301ff..b9b303a 100644 --- a/lib/pinchflat_web.ex +++ b/lib/pinchflat_web.ex @@ -45,6 +45,7 @@ defmodule PinchflatWeb do import Plug.Conn import PinchflatWeb.Gettext + alias Pinchflat.Settings alias PinchflatWeb.Layouts unquote(verified_routes()) @@ -56,6 +57,8 @@ defmodule PinchflatWeb do use Phoenix.LiveView, layout: {PinchflatWeb.Layouts, :app} + alias Pinchflat.Settings + unquote(html_helpers()) end end @@ -64,6 +67,8 @@ defmodule PinchflatWeb do quote do use Phoenix.LiveComponent + alias Pinchflat.Settings + unquote(html_helpers()) end end @@ -76,6 +81,8 @@ defmodule PinchflatWeb do import Phoenix.Controller, only: [get_csrf_token: 0, view_module: 1, view_template: 1] + alias Pinchflat.Settings + # Include general helpers for rendering HTML unquote(html_helpers()) end @@ -93,6 +100,7 @@ defmodule PinchflatWeb do import PinchflatWeb.CustomComponents.TableComponents import PinchflatWeb.CustomComponents.ButtonComponents + alias Pinchflat.Settings alias Pinchflat.Utils.StringUtils # Shortcut for generating JS commands diff --git a/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex b/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex index f0d78d9..4b130e7 100644 --- a/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex +++ b/lib/pinchflat_web/controllers/media_profiles/media_profile_controller.ex @@ -13,29 +13,21 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileController do def new(conn, _params) do changeset = Profiles.change_media_profile(%MediaProfile{}) - if get_session(conn, :onboarding) do - render(conn, :new, changeset: changeset, layout: {Layouts, :onboarding}) - else - render(conn, :new, changeset: changeset) - end + render(conn, :new, changeset: changeset, layout: get_onboarding_layout()) end def create(conn, %{"media_profile" => media_profile_params}) do case Profiles.create_media_profile(media_profile_params) do {:ok, media_profile} -> redirect_location = - if get_session(conn, :onboarding), do: ~p"/?onboarding=1", else: ~p"/media_profiles/#{media_profile}" + if Settings.get!(:onboarding), do: ~p"/?onboarding=1", else: ~p"/media_profiles/#{media_profile}" conn |> put_flash(:info, "Media profile created successfully.") |> redirect(to: redirect_location) {:error, %Ecto.Changeset{} = changeset} -> - if get_session(conn, :onboarding) do - render(conn, :new, changeset: changeset, layout: {Layouts, :onboarding}) - else - render(conn, :new, changeset: changeset) - end + render(conn, :new, changeset: changeset, layout: get_onboarding_layout()) end end @@ -85,4 +77,12 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileController do |> put_flash(:info, flash_message) |> redirect(to: ~p"/media_profiles") end + + defp get_onboarding_layout do + if Settings.get!(:onboarding) do + {Layouts, :onboarding} + else + {Layouts, :app} + end + end end diff --git a/lib/pinchflat_web/controllers/media_profiles/media_profile_html/new.html.heex b/lib/pinchflat_web/controllers/media_profiles/media_profile_html/new.html.heex index 707c9f6..a0311ab 100644 --- a/lib/pinchflat_web/controllers/media_profiles/media_profile_html/new.html.heex +++ b/lib/pinchflat_web/controllers/media_profiles/media_profile_html/new.html.heex @@ -1,5 +1,5 @@
- <.link :if={!Plug.Conn.get_session(@conn, :onboarding)} navigate={~p"/media_profiles"}> + <.link :if={!Settings.get!(:onboarding)} navigate={~p"/media_profiles"}> <.icon name="hero-arrow-left" class="w-10 h-10 hover:dark:text-white" />

New Media Profile

diff --git a/lib/pinchflat_web/controllers/pages/page_controller.ex b/lib/pinchflat_web/controllers/pages/page_controller.ex index 5265796..3f9ccf8 100644 --- a/lib/pinchflat_web/controllers/pages/page_controller.ex +++ b/lib/pinchflat_web/controllers/pages/page_controller.ex @@ -20,12 +20,12 @@ defmodule PinchflatWeb.Pages.PageController do end defp render_home_page(conn) do + Settings.set!(:onboarding, false) media_profile_count = Repo.aggregate(MediaProfile, :count, :id) source_count = Repo.aggregate(Source, :count, :id) media_item_count = Repo.aggregate(MediaItem, :count, :id) conn - |> put_session(:onboarding, false) |> render(:home, media_profile_count: media_profile_count, source_count: source_count, @@ -34,8 +34,9 @@ defmodule PinchflatWeb.Pages.PageController do end defp render_onboarding_page(conn, media_profiles_exist, sources_exist) do + Settings.set!(:onboarding, true) + conn - |> put_session(:onboarding, true) |> render(:onboarding_checklist, media_profiles_exist: media_profiles_exist, sources_exist: sources_exist, diff --git a/lib/pinchflat_web/controllers/sources/source_controller.ex b/lib/pinchflat_web/controllers/sources/source_controller.ex index f2cb342..757fb44 100644 --- a/lib/pinchflat_web/controllers/sources/source_controller.ex +++ b/lib/pinchflat_web/controllers/sources/source_controller.ex @@ -16,37 +16,29 @@ defmodule PinchflatWeb.Sources.SourceController do def new(conn, _params) do changeset = Sources.change_source(%Source{}) - if get_session(conn, :onboarding) do - render(conn, :new, - changeset: changeset, - media_profiles: media_profiles(), - layout: {Layouts, :onboarding} - ) - else - render(conn, :new, changeset: changeset, media_profiles: media_profiles()) - end + render(conn, :new, + changeset: changeset, + media_profiles: media_profiles(), + layout: get_onboarding_layout() + ) end def create(conn, %{"source" => source_params}) do case Sources.create_source(source_params) do {:ok, source} -> redirect_location = - if get_session(conn, :onboarding), do: ~p"/?onboarding=1", else: ~p"/sources/#{source}" + if Settings.get!(:onboarding), do: ~p"/?onboarding=1", else: ~p"/sources/#{source}" conn |> put_flash(:info, "Source created successfully.") |> redirect(to: redirect_location) {:error, %Ecto.Changeset{} = changeset} -> - if get_session(conn, :onboarding) do - render(conn, :new, - changeset: changeset, - media_profiles: media_profiles(), - layout: {Layouts, :onboarding} - ) - else - render(conn, :new, changeset: changeset, media_profiles: media_profiles()) - end + render(conn, :new, + changeset: changeset, + media_profiles: media_profiles(), + layout: get_onboarding_layout() + ) end end @@ -107,4 +99,12 @@ defmodule PinchflatWeb.Sources.SourceController do defp media_profiles do Profiles.list_media_profiles() end + + defp get_onboarding_layout do + if Settings.get!(:onboarding) do + {Layouts, :onboarding} + else + {Layouts, :app} + end + end end diff --git a/lib/pinchflat_web/controllers/sources/source_html/new.html.heex b/lib/pinchflat_web/controllers/sources/source_html/new.html.heex index 5914e03..4c9fbc4 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/new.html.heex +++ b/lib/pinchflat_web/controllers/sources/source_html/new.html.heex @@ -1,5 +1,5 @@
- <.link :if={!Plug.Conn.get_session(@conn, :onboarding)} navigate={~p"/sources"}> + <.link :if={!Settings.get!(:onboarding)} navigate={~p"/sources"}> <.icon name="hero-arrow-left" class="w-10 h-10 hover:dark:text-white" />

New Source

diff --git a/test/pinchflat_web/controllers/media_profile_controller_test.exs b/test/pinchflat_web/controllers/media_profile_controller_test.exs index c5cdd19..1a2387b 100644 --- a/test/pinchflat_web/controllers/media_profile_controller_test.exs +++ b/test/pinchflat_web/controllers/media_profile_controller_test.exs @@ -6,6 +6,7 @@ defmodule PinchflatWeb.MediaProfileControllerTest do import Pinchflat.ProfilesFixtures alias Pinchflat.Repo + alias Pinchflat.Settings @create_attrs %{name: "some name", output_path_template: "some output_path_template"} @update_attrs %{ @@ -14,6 +15,12 @@ defmodule PinchflatWeb.MediaProfileControllerTest do } @invalid_attrs %{name: nil, output_path_template: nil} + setup do + Settings.set!(:onboarding, false) + + :ok + end + describe "index" do test "lists all media_profiles", %{conn: conn} do conn = get(conn, ~p"/media_profiles") @@ -27,13 +34,11 @@ defmodule PinchflatWeb.MediaProfileControllerTest do assert html_response(conn, 200) =~ "New Media Profile" end - test "renders correct layout when onboarding", %{session_conn: session_conn} do - session_conn = - session_conn - |> put_session(:onboarding, true) - |> get(~p"/media_profiles/new") + test "renders correct layout when onboarding", %{conn: conn} do + Settings.set!(:onboarding, true) + conn = get(conn, ~p"/media_profiles/new") - refute html_response(session_conn, 200) =~ "MENU" + refute html_response(conn, 200) =~ "MENU" end end @@ -53,22 +58,18 @@ defmodule PinchflatWeb.MediaProfileControllerTest do assert html_response(conn, 200) =~ "New Media Profile" end - test "redirects to onboarding when onboarding", %{session_conn: session_conn} do - session_conn = - session_conn - |> put_session(:onboarding, true) - |> post(~p"/media_profiles", media_profile: @create_attrs) + test "redirects to onboarding when onboarding", %{conn: conn} do + Settings.set!(:onboarding, true) + conn = post(conn, ~p"/media_profiles", media_profile: @create_attrs) - assert redirected_to(session_conn) == ~p"/?onboarding=1" + assert redirected_to(conn) == ~p"/?onboarding=1" end - test "renders correct layout on error when onboarding", %{session_conn: session_conn} do - session_conn = - session_conn - |> put_session(:onboarding, true) - |> post(~p"/media_profiles", media_profile: @invalid_attrs) + test "renders correct layout on error when onboarding", %{conn: conn} do + Settings.set!(:onboarding, true) + conn = post(conn, ~p"/media_profiles", media_profile: @invalid_attrs) - refute html_response(session_conn, 200) =~ "MENU" + refute html_response(conn, 200) =~ "MENU" end end diff --git a/test/pinchflat_web/controllers/page_controller_test.exs b/test/pinchflat_web/controllers/page_controller_test.exs index 572e07d..ab1b536 100644 --- a/test/pinchflat_web/controllers/page_controller_test.exs +++ b/test/pinchflat_web/controllers/page_controller_test.exs @@ -4,10 +4,12 @@ defmodule PinchflatWeb.PageControllerTest do import Pinchflat.ProfilesFixtures import Pinchflat.SourcesFixtures + alias Pinchflat.Settings + describe "GET / when testing onboarding" do test "sets the onboarding session to true when onboarding", %{conn: conn} do - conn = get(conn, ~p"/") - assert get_session(conn, :onboarding) + _conn = get(conn, ~p"/") + assert Settings.get!(:onboarding) end test "displays the onboarding page when no media profiles exist", %{conn: conn} do @@ -32,13 +34,13 @@ defmodule PinchflatWeb.PageControllerTest do test "sets the onboarding session to false when not onboarding", %{conn: conn} do conn = get(conn, ~p"/") - assert get_session(conn, :onboarding) + assert Settings.get!(:onboarding) _ = media_profile_fixture() _ = source_fixture() - conn = get(conn, ~p"/") - refute get_session(conn, :onboarding) + _conn = get(conn, ~p"/") + refute Settings.get!(:onboarding) end test "displays the home page when not onboarding", %{conn: conn} do diff --git a/test/pinchflat_web/controllers/source_controller_test.exs b/test/pinchflat_web/controllers/source_controller_test.exs index 4f641f5..342ce17 100644 --- a/test/pinchflat_web/controllers/source_controller_test.exs +++ b/test/pinchflat_web/controllers/source_controller_test.exs @@ -7,9 +7,11 @@ defmodule PinchflatWeb.SourceControllerTest do import Pinchflat.ProfilesFixtures alias Pinchflat.Repo + alias Pinchflat.Settings setup do media_profile = media_profile_fixture() + Settings.set!(:onboarding, false) { :ok, @@ -42,13 +44,11 @@ defmodule PinchflatWeb.SourceControllerTest do assert html_response(conn, 200) =~ "New Source" end - test "renders correct layout when onboarding", %{session_conn: session_conn} do - session_conn = - session_conn - |> put_session(:onboarding, true) - |> get(~p"/sources/new") + test "renders correct layout when onboarding", %{conn: conn} do + Settings.set!(:onboarding, true) + conn = get(conn, ~p"/sources/new") - refute html_response(session_conn, 200) =~ "MENU" + refute html_response(conn, 200) =~ "MENU" end end @@ -69,24 +69,20 @@ defmodule PinchflatWeb.SourceControllerTest do assert html_response(conn, 200) =~ "New Source" end - test "redirects to onboarding when onboarding", %{session_conn: session_conn, create_attrs: create_attrs} do + test "redirects to onboarding when onboarding", %{conn: conn, create_attrs: create_attrs} do expect(YtDlpRunnerMock, :run, 1, &runner_function_mock/3) - session_conn = - session_conn - |> put_session(:onboarding, true) - |> post(~p"/sources", source: create_attrs) + Settings.set!(:onboarding, true) + conn = post(conn, ~p"/sources", source: create_attrs) - assert redirected_to(session_conn) == ~p"/?onboarding=1" + assert redirected_to(conn) == ~p"/?onboarding=1" end - test "renders correct layout on error when onboarding", %{session_conn: session_conn, invalid_attrs: invalid_attrs} do - session_conn = - session_conn - |> put_session(:onboarding, true) - |> post(~p"/sources", source: invalid_attrs) + test "renders correct layout on error when onboarding", %{conn: conn, invalid_attrs: invalid_attrs} do + Settings.set!(:onboarding, true) + conn = post(conn, ~p"/sources", source: invalid_attrs) - refute html_response(session_conn, 200) =~ "MENU" + refute html_response(conn, 200) =~ "MENU" end end