diff --git a/lib/pinchflat/boot/pre_job_startup_tasks.ex b/lib/pinchflat/boot/pre_job_startup_tasks.ex index bc52ee1..d4f4168 100644 --- a/lib/pinchflat/boot/pre_job_startup_tasks.ex +++ b/lib/pinchflat/boot/pre_job_startup_tasks.ex @@ -65,8 +65,6 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do defp apply_default_settings do {:ok, yt_dlp_version} = CommandRunner.version() - Settings.fetch!(:onboarding, true) - Settings.fetch!(:pro_enabled, false) - Settings.set!(:yt_dlp_version, yt_dlp_version) + Settings.set(yt_dlp_version: yt_dlp_version) end end diff --git a/lib/pinchflat/settings/setting.ex b/lib/pinchflat/settings/setting.ex index fab97b9..5fb7cf6 100644 --- a/lib/pinchflat/settings/setting.ex +++ b/lib/pinchflat/settings/setting.ex @@ -1,24 +1,32 @@ defmodule Pinchflat.Settings.Setting do @moduledoc """ - A Setting is a key-value pair with a datatype used to track user-level settings. + The Setting schema. """ use Ecto.Schema import Ecto.Changeset - schema "settings" do - field :name, :string - field :value, :string - field :datatype, Ecto.Enum, values: ~w(boolean string integer float)a + @allowed_fields [ + :onboarding, + :pro_enabled, + :yt_dlp_version + ] - timestamps(type: :utc_datetime) + @required_fields ~w( + onboarding + pro_enabled + )a + + schema "settings" do + field :onboarding, :boolean, default: true + field :pro_enabled, :boolean, default: false + field :yt_dlp_version, :string end @doc false def changeset(setting, attrs) do setting - |> cast(attrs, [:name, :value, :datatype]) - |> validate_required([:name, :value, :datatype]) - |> unique_constraint([:name]) + |> cast(attrs, @allowed_fields) + |> validate_required(@required_fields) end end diff --git a/lib/pinchflat/settings/settings.ex b/lib/pinchflat/settings/settings.ex index 29db085..4b133c8 100644 --- a/lib/pinchflat/settings/settings.ex +++ b/lib/pinchflat/settings/settings.ex @@ -2,94 +2,63 @@ defmodule Pinchflat.Settings do @moduledoc """ The Settings context. """ - import Ecto.Query, warn: false - alias Pinchflat.Repo + alias Pinchflat.Repo alias Pinchflat.Settings.Setting @doc """ - Returns the list of settings. + Returns the only setting record. It _should_ be impossible + to create or delete this record, so it's assertive about + assuming it's the only one. - Returns [%Setting{}, ...] + Returns %Setting{} """ - def list_settings do - Repo.all(Setting) + def record do + Setting + |> limit(1) + |> Repo.one() end @doc """ - Creates or updates a setting, returning the parsed value. - Raises if an unsupported datatype is used. Optionally allows - specifying the datatype. + Updates a setting, returning the new value. + Is setup to take a keyword list argument so you + can call it like `Settings.set(onboarding: true)` - Returns value in type of `Ecto.Enum.mappings(Setting, :datatype)` + Returns {:ok, value} | {:error, :invalid_key} | {:error, %Ecto.Changeset{}} """ - def set!(name, value) do - set!(name, value, infer_datatype(value)) - end - - def set!(name, value, datatype) do - # Only create if doesn't exist - case Repo.get_by(Setting, name: to_string(name)) do - nil -> create_setting!(name, value, datatype) - setting -> update_setting!(setting, value, datatype) + def set([{attr, value}]) do + record() + |> Setting.changeset(%{attr => value}) + |> Repo.update() + |> case do + {:ok, %{^attr => _}} -> {:ok, value} + {:ok, _} -> {:error, :invalid_key} + {:error, changeset} -> {:error, changeset} end end @doc """ - Gets the parsed value of a setting. Raises if the setting does not exist. + Gets the value of a setting. - Returns value in type of `Ecto.Enum.mappings(Setting, :datatype)` + Returns {:ok, value} | {:error, :invalid_key} + """ + def get(name) do + case Map.fetch(record(), name) do + {:ok, value} -> {:ok, value} + :error -> {:error, :invalid_key} + end + end + + @doc """ + Gets the value of a setting, raising if it doesn't exist. + + Returns value """ def get!(name) do - Setting - |> Repo.get_by!(name: to_string(name)) - |> read_setting() - end - - @doc """ - Attempts to find a setting by name or creates a setting with value - if one doesn't exist, returning the parsed value. Optionally allows - specifying the datatype. - - Returns value in type of `Ecto.Enum.mappings(Setting, :datatype)` - """ - def fetch!(name, value) do - fetch!(name, value, infer_datatype(value)) - end - - def fetch!(name, value, datatype) do - case Repo.get_by(Setting, name: to_string(name)) do - nil -> create_setting!(name, value, datatype) - setting -> read_setting(setting) + case get(name) do + {:ok, value} -> value + {:error, _} -> raise "Setting `#{name}` not found" end end - - defp change_setting(setting, attrs) do - Setting.changeset(setting, attrs) - end - - defp create_setting!(name, value, datatype) do - %Setting{} - |> change_setting(%{name: to_string(name), value: to_string(value), datatype: datatype}) - |> Repo.insert!() - |> read_setting() - end - - defp update_setting!(setting, value, datatype) do - setting - |> change_setting(%{value: to_string(value), datatype: datatype}) - |> Repo.update!() - |> read_setting() - end - - defp read_setting(%{value: value, datatype: :string}), do: value - defp read_setting(%{value: value, datatype: :boolean}), do: value in ["true", "t", "1"] - defp read_setting(%{value: value, datatype: :integer}), do: String.to_integer(value) - defp read_setting(%{value: value, datatype: :float}), do: String.to_float(value) - - defp infer_datatype(value) when is_boolean(value), do: :boolean - defp infer_datatype(value) when is_integer(value), do: :integer - defp infer_datatype(value) when is_float(value), do: :float - defp infer_datatype(value) when is_binary(value), do: :string end diff --git a/lib/pinchflat_web/components/layouts/partials/upgrade_button_live.ex b/lib/pinchflat_web/components/layouts/partials/upgrade_button_live.ex index 89c205b..959edae 100644 --- a/lib/pinchflat_web/components/layouts/partials/upgrade_button_live.ex +++ b/lib/pinchflat_web/components/layouts/partials/upgrade_button_live.ex @@ -30,7 +30,7 @@ defmodule Pinchflat.UpgradeButtonLive do |> String.downcase() if normalized_text == "got it!" do - Settings.set!(:pro_enabled, true) + Settings.set(pro_enabled: true) {:noreply, update(socket, :button_disabled, fn _ -> false end)} else diff --git a/lib/pinchflat_web/controllers/pages/page_controller.ex b/lib/pinchflat_web/controllers/pages/page_controller.ex index 50d1a46..ef1b154 100644 --- a/lib/pinchflat_web/controllers/pages/page_controller.ex +++ b/lib/pinchflat_web/controllers/pages/page_controller.ex @@ -11,7 +11,7 @@ defmodule PinchflatWeb.Pages.PageController do done_onboarding = params["onboarding"] == "0" force_onboarding = params["onboarding"] == "1" - if done_onboarding, do: Settings.set!(:onboarding, false) + if done_onboarding, do: Settings.set(onboarding: false) if force_onboarding || Settings.get!(:onboarding) do render_onboarding_page(conn) @@ -30,7 +30,7 @@ defmodule PinchflatWeb.Pages.PageController do end defp render_onboarding_page(conn) do - Settings.set!(:onboarding, true) + Settings.set(onboarding: true) conn |> render(:onboarding_checklist, diff --git a/priv/repo/migrations/20240404172256_rename_settings_table.exs b/priv/repo/migrations/20240404172256_rename_settings_table.exs new file mode 100644 index 0000000..2577e76 --- /dev/null +++ b/priv/repo/migrations/20240404172256_rename_settings_table.exs @@ -0,0 +1,7 @@ +defmodule Pinchflat.Repo.Migrations.RenameSettingsTable do + use Ecto.Migration + + def change do + rename table(:settings), to: table(:settings_backup) + end +end diff --git a/priv/repo/migrations/20240404174144_create_new_settings.exs b/priv/repo/migrations/20240404174144_create_new_settings.exs new file mode 100644 index 0000000..d8b6131 --- /dev/null +++ b/priv/repo/migrations/20240404174144_create_new_settings.exs @@ -0,0 +1,29 @@ +defmodule Pinchflat.Repo.Migrations.CreateNewSettings do + use Ecto.Migration + + def up do + create table(:settings) do + add :onboarding, :boolean, default: true, null: false + add :pro_enabled, :boolean, default: false, null: false + add :yt_dlp_version, :string + end + + # Make an initial record because this will be the only one ever inserted + execute "INSERT INTO settings (onboarding, pro_enabled, yt_dlp_version) VALUES (true, false, NULL)" + + # Set the value of onboarding to the previous version set in `settings_backup` + execute """ + UPDATE settings + SET onboarding = COALESCE((SELECT value = 'true' FROM settings_backup WHERE name = 'onboarding'), true) + """ + + execute """ + UPDATE settings + SET pro_enabled = COALESCE((SELECT value = 'true' FROM settings_backup WHERE name = 'pro_enabled'), false) + """ + end + + def down do + drop table(:settings) + end +end diff --git a/test/pinchflat/boot/pre_job_startup_tasks_test.exs b/test/pinchflat/boot/pre_job_startup_tasks_test.exs index 0c41c24..f2cdd66 100644 --- a/test/pinchflat/boot/pre_job_startup_tasks_test.exs +++ b/test/pinchflat/boot/pre_job_startup_tasks_test.exs @@ -1,25 +1,47 @@ defmodule Pinchflat.Boot.PreJobStartupTasksTest do use Pinchflat.DataCase + import Pinchflat.JobFixtures + alias Pinchflat.Settings - alias Pinchflat.Settings.Setting alias Pinchflat.Boot.PreJobStartupTasks - describe "apply_default_settings" do - setup do - Repo.delete_all(Setting) + describe "reset_executing_jobs" do + test "resets executing jobs" do + job = job_fixture() + Repo.update_all(Oban.Job, set: [state: "executing"]) - :ok - end - - test "sets default settings" do - assert_raise Ecto.NoResultsError, fn -> Settings.get!(:onboarding) end - assert_raise Ecto.NoResultsError, fn -> Settings.get!(:pro_enabled) end + assert Repo.reload!(job).state == "executing" PreJobStartupTasks.start_link() - assert Settings.get!(:onboarding) - refute Settings.get!(:pro_enabled) + assert Repo.reload!(job).state == "retryable" + end + end + + describe "create_blank_cookie_file" do + test "creates a blank cookie file" do + base_dir = Application.get_env(:pinchflat, :extras_directory) + filepath = Path.join(base_dir, "cookies.txt") + File.rm(filepath) + + refute File.exists?(filepath) + + PreJobStartupTasks.start_link() + + assert File.exists?(filepath) + end + end + + describe "apply_default_settings" do + test "sets default settings" do + Settings.set(yt_dlp_version: nil) + + refute Settings.get!(:yt_dlp_version) + + PreJobStartupTasks.start_link() + + assert Settings.get!(:yt_dlp_version) end end end diff --git a/test/pinchflat/settings_test.exs b/test/pinchflat/settings_test.exs index 443538a..bcfb97f 100644 --- a/test/pinchflat/settings_test.exs +++ b/test/pinchflat/settings_test.exs @@ -9,100 +9,55 @@ defmodule Pinchflat.SettingsTest do # are always created on app boot (including in the test env), # so we can't treat these like a clean slate. - describe "list_settings/0" do - test "returns all settings" do - Settings.set!("foo", "bar") - results = Settings.list_settings() + setup do + # Ensure we have a clean slate + Settings.set(onboarding: false) + Settings.set(pro_enabled: false) + Settings.set(yt_dlp_version: nil) - assert Enum.all?(results, fn setting -> match?(%Setting{}, setting) end) + :ok + end + + describe "record/0" do + test "returns the only setting" do + assert %Setting{} = Settings.record() end end - describe "set/2" do - test "creates a new setting if one does not exist" do - original = Repo.aggregate(Setting, :count, :id) - Settings.set!("foo", "bar") - assert Repo.aggregate(Setting, :count, :id) == original + 1 + describe "set/1" do + test "updates the setting" do + assert {:ok, true} = Settings.set(onboarding: true) + assert {:ok, true} = Settings.get(:onboarding) end - test "updates an existing setting if one exists" do - Settings.set!("foo", "bar") - original = Repo.aggregate(Setting, :count, :id) - Settings.set!("foo", "baz") - assert Repo.aggregate(Setting, :count, :id) == original - assert Settings.get!("foo") == "baz" + test "returns an error if the setting key doesn't exist" do + assert {:error, :invalid_key} = Settings.set(foo: "bar") end - test "returns the parsed value" do - assert Settings.set!("foo", true) == true - assert Settings.set!("foo", false) == false - assert Settings.set!("foo", 123) == 123 - assert Settings.set!("foo", 12.34) == 12.34 - assert Settings.set!("foo", "bar") == "bar" - end - - test "allows for atom keys" do - assert Settings.set!(:foo, "bar") == "bar" - end - - test "blows up when an unsupported datatype is used" do - assert_raise FunctionClauseError, fn -> - Settings.set!("foo", nil) - end - end - end - - describe "set/3" do - test "allows manual specification of datatype" do - assert Settings.set!("foo", "true", :boolean) == true - assert Settings.set!("foo", "false", :boolean) == false - assert Settings.set!("foo", "123", :integer) == 123 - assert Settings.set!("foo", "12.34", :float) == 12.34 + test "returns an error if the setting value is invalid" do + assert {:error, %Ecto.Changeset{}} = Settings.set(onboarding: "bar") end end describe "get/1" do - test "returns the value of the setting" do - Settings.set!("str", "bar") - Settings.set!("bool", true) - Settings.set!("int", 123) - Settings.set!("float", 12.34) - - assert Settings.get!("str") == "bar" - assert Settings.get!("bool") == true - assert Settings.get!("int") == 123 - assert Settings.get!("float") == 12.34 + test "returns the setting value" do + assert {:ok, false} = Settings.get(:onboarding) end - test "allows for atom keys" do - Settings.set!("str", "bar") - assert Settings.get!(:str) == "bar" + test "returns an error if the setting key doesn't exist" do + assert {:error, :invalid_key} = Settings.get(:foo) + end + end + + describe "get!/1" do + test "returns the setting value" do + assert Settings.get!(:onboarding) == false end - test "blows up when the setting does not exist" do - assert_raise Ecto.NoResultsError, fn -> - Settings.get!("foo") + test "raises an error if the setting key doesn't exist" do + assert_raise RuntimeError, "Setting `foo` not found", fn -> + Settings.get!(:foo) end end end - - describe "fetch/2" do - test "creates a setting if one doesn't exist" do - original = Repo.aggregate(Setting, :count, :id) - assert Settings.fetch!("foo", "bar") == "bar" - assert Repo.aggregate(Setting, :count, :id) == original + 1 - end - - test "returns an existing setting if one does exist" do - Settings.set!("foo", "bar") - - assert Settings.fetch!("foo", "baz") == "bar" - end - end - - describe "fetch/3" do - test "allows manual specification of datatype" do - assert Settings.fetch!("foo", "true", :boolean) == true - end - end end diff --git a/test/pinchflat_web/controllers/media_profile_controller_test.exs b/test/pinchflat_web/controllers/media_profile_controller_test.exs index 4a81025..df1013b 100644 --- a/test/pinchflat_web/controllers/media_profile_controller_test.exs +++ b/test/pinchflat_web/controllers/media_profile_controller_test.exs @@ -16,7 +16,7 @@ defmodule PinchflatWeb.MediaProfileControllerTest do @invalid_attrs %{name: nil, output_path_template: nil} setup do - Settings.set!(:onboarding, false) + Settings.set(onboarding: false) :ok end @@ -35,7 +35,7 @@ defmodule PinchflatWeb.MediaProfileControllerTest do end test "renders correct layout when onboarding", %{conn: conn} do - Settings.set!(:onboarding, true) + Settings.set(onboarding: true) conn = get(conn, ~p"/media_profiles/new") refute html_response(conn, 200) =~ "MENU" @@ -59,14 +59,14 @@ defmodule PinchflatWeb.MediaProfileControllerTest do end test "redirects to onboarding when onboarding", %{conn: conn} do - Settings.set!(:onboarding, true) + Settings.set(onboarding: true) conn = post(conn, ~p"/media_profiles", media_profile: @create_attrs) assert redirected_to(conn) == ~p"/?onboarding=1" end test "renders correct layout on error when onboarding", %{conn: conn} do - Settings.set!(:onboarding, true) + Settings.set(onboarding: true) conn = post(conn, ~p"/media_profiles", media_profile: @invalid_attrs) refute html_response(conn, 200) =~ "MENU" diff --git a/test/pinchflat_web/controllers/page_controller_test.exs b/test/pinchflat_web/controllers/page_controller_test.exs index 3037764..e549942 100644 --- a/test/pinchflat_web/controllers/page_controller_test.exs +++ b/test/pinchflat_web/controllers/page_controller_test.exs @@ -10,7 +10,7 @@ defmodule PinchflatWeb.PageControllerTest do end test "displays the onboarding page when onboarding is forced", %{conn: conn} do - Settings.set!(:onboarding, false) + Settings.set(onboarding: false) conn = get(conn, ~p"/?onboarding=1") assert html_response(conn, 200) =~ "Welcome to Pinchflat" @@ -25,7 +25,7 @@ defmodule PinchflatWeb.PageControllerTest do end test "displays the home page when not onboarding", %{conn: conn} do - Settings.set!(:onboarding, false) + Settings.set(onboarding: false) conn = get(conn, ~p"/") assert html_response(conn, 200) =~ "MENU" diff --git a/test/pinchflat_web/controllers/source_controller_test.exs b/test/pinchflat_web/controllers/source_controller_test.exs index 016a01a..c0ecc5f 100644 --- a/test/pinchflat_web/controllers/source_controller_test.exs +++ b/test/pinchflat_web/controllers/source_controller_test.exs @@ -13,7 +13,7 @@ defmodule PinchflatWeb.SourceControllerTest do setup do media_profile = media_profile_fixture() - Settings.set!(:onboarding, false) + Settings.set(onboarding: false) { :ok, @@ -47,7 +47,7 @@ defmodule PinchflatWeb.SourceControllerTest do end test "renders correct layout when onboarding", %{conn: conn} do - Settings.set!(:onboarding, true) + Settings.set(onboarding: true) conn = get(conn, ~p"/sources/new") refute html_response(conn, 200) =~ "MENU" @@ -74,14 +74,14 @@ defmodule PinchflatWeb.SourceControllerTest do test "redirects to onboarding when onboarding", %{conn: conn, create_attrs: create_attrs} do expect(YtDlpRunnerMock, :run, 1, &runner_function_mock/3) - Settings.set!(:onboarding, true) + Settings.set(onboarding: true) conn = post(conn, ~p"/sources", source: create_attrs) assert redirected_to(conn) == ~p"/?onboarding=1" end test "renders correct layout on error when onboarding", %{conn: conn, invalid_attrs: invalid_attrs} do - Settings.set!(:onboarding, true) + Settings.set(onboarding: true) conn = post(conn, ~p"/sources", source: invalid_attrs) refute html_response(conn, 200) =~ "MENU"