From 4c180c1b823b6cf9ae68c16176afe641c5038dec Mon Sep 17 00:00:00 2001 From: Kayla Firestack Date: Thu, 6 Feb 2025 10:50:20 -0500 Subject: [PATCH] feat(ex/detours): add detour `:status` field, and populate from changeset (#2952) * feat(ex/detours): add detour :status field, and populate from changeset This adds a new database column and starts populating it from the changeset function. Once we deploy this and have all instances become aware of the field. We'll then deploy a data migration which adds this to previously made detours, so that there are no running instances without this change to the changeset function * chore(ex/db/detour): add type to field * refactor(ex/detours): move `detour_type` into `Skate.Detours.Db.Detour` * refactor(ex/detours): replace `detour_type` with `Detour.status` * test(ex/detours): add tests for `:status` column * feat(ex/db/detour): set `:status` in changeset when `:state` changes` --- lib/skate/detours/db/detour.ex | 20 ++++++++++++++- lib/skate/detours/detours.ex | 16 ++++++------ ...20250131195636_add_detour_status_field.exs | 9 +++++++ ...50204034128_create_detour_status_index.exs | 10 ++++++++ test/skate/detours/db_test.exs | 24 ++++++++++++++++++ .../controllers/detours_controller_test.exs | 25 +++++++++++++++++++ test/support/factories/detour_factory.ex | 8 +++--- 7 files changed, 99 insertions(+), 13 deletions(-) create mode 100644 priv/repo/migrations/20250131195636_add_detour_status_field.exs create mode 100644 priv/repo/migrations/20250204034128_create_detour_status_index.exs diff --git a/lib/skate/detours/db/detour.ex b/lib/skate/detours/db/detour.ex index 9a8b32b55..69a0c4c10 100644 --- a/lib/skate/detours/db/detour.ex +++ b/lib/skate/detours/db/detour.ex @@ -8,8 +8,13 @@ defmodule Skate.Detours.Db.Detour do alias Skate.Settings.Db.User + @type status :: :active | :draft | :past + typed_schema "detours" do field :state, :map + + field(:status, Ecto.Enum, values: [:draft, :active, :past]) :: status() + belongs_to :author, User # When this detour was activated @@ -21,7 +26,20 @@ defmodule Skate.Detours.Db.Detour do def changeset(detour, attrs) do detour |> cast(attrs, [:state, :activated_at]) - |> validate_required([:state]) + |> add_status() + |> validate_required([:state, :status]) |> foreign_key_constraint(:author_id) end + + defp add_status(changeset) do + case fetch_change(changeset, :state) do + {:ok, state} -> + # Once this column is added for all detours, `categorize_detour` logic + # should be moved here and should not be needed anymore + put_change(changeset, :status, Skate.Detours.Detours.categorize_detour(%{state: state})) + + _ -> + changeset + end + end end diff --git a/lib/skate/detours/detours.ex b/lib/skate/detours/detours.ex index 5d9c4922b..828be4a93 100644 --- a/lib/skate/detours/detours.ex +++ b/lib/skate/detours/detours.ex @@ -84,7 +84,7 @@ defmodule Skate.Detours.Detours do end @spec db_detour_to_detour(Detour.t()) :: DetailedDetour.t() | nil - @spec db_detour_to_detour(status :: detour_type(), Detour.t()) :: DetailedDetour.t() | nil + @spec db_detour_to_detour(status :: Detour.status(), Detour.t()) :: DetailedDetour.t() | nil def db_detour_to_detour(%{} = db_detour) do db_detour_to_detour(categorize_detour(db_detour), db_detour) end @@ -121,16 +121,14 @@ defmodule Skate.Detours.Detours do nil end - @type detour_type :: :active | :draft | :past - @doc """ Takes a `Skate.Detours.Db.Detour` struct and a `Skate.Settings.Db.User` id - and returns a `t:detour_type/0` based on the state of the detour. + and returns a `t:Detour.status/0` based on the state of the detour. otherwise returns `nil` if it is a draft but does not belong to the provided user """ - @spec categorize_detour(detour :: map()) :: detour_type() + @spec categorize_detour(detour :: map()) :: Detour.status() def categorize_detour(%{state: %{"value" => %{"Detour Drawing" => %{"Active" => _}}}}), do: :active @@ -284,7 +282,7 @@ defmodule Skate.Detours.Detours do ) end - @spec broadcast_detour(detour_type(), Detour.t(), DbUser.id()) :: :ok + @spec broadcast_detour(Detour.status(), Detour.t(), DbUser.id()) :: :ok defp broadcast_detour(:draft, detour, author_id) do author_uuid = author_id @@ -351,7 +349,7 @@ defmodule Skate.Detours.Detours do Retrieves a `Skate.Detours.Db.Detour` from the database by it's ID and then resolves the detour's category via `categorize_detour/2` """ - @spec categorize_detour_by_id(detour_id :: nil | integer()) :: detour_type() | nil + @spec categorize_detour_by_id(detour_id :: nil | integer()) :: Detour.status() | nil def categorize_detour_by_id(nil = _detour_id), do: nil def categorize_detour_by_id(detour_id) do @@ -367,8 +365,8 @@ defmodule Skate.Detours.Detours do ) :: :ok | nil @spec send_notification(%{ next_detour: Skate.Detours.Db.Detour.t() | nil, - next: detour_type() | nil, - previous: detour_type() | nil + next: Detour.status() | nil, + previous: Detour.status() | nil }) :: :ok | nil defp send_notification( %Detour{} = new_record, diff --git a/priv/repo/migrations/20250131195636_add_detour_status_field.exs b/priv/repo/migrations/20250131195636_add_detour_status_field.exs new file mode 100644 index 000000000..371c3873b --- /dev/null +++ b/priv/repo/migrations/20250131195636_add_detour_status_field.exs @@ -0,0 +1,9 @@ +defmodule Skate.Repo.Migrations.AddDetourStatusField do + use Ecto.Migration + + def change do + alter table(:detours) do + add :status, :string + end + end +end diff --git a/priv/repo/migrations/20250204034128_create_detour_status_index.exs b/priv/repo/migrations/20250204034128_create_detour_status_index.exs new file mode 100644 index 000000000..92c3fb32b --- /dev/null +++ b/priv/repo/migrations/20250204034128_create_detour_status_index.exs @@ -0,0 +1,10 @@ +defmodule Skate.Repo.Migrations.CreateDetourStatusIndex do + use Ecto.Migration + + @disable_ddl_transaction true + @disable_migration_lock true + + def change do + create index(:detours, [:status], concurrently: true) + end +end diff --git a/test/skate/detours/db_test.exs b/test/skate/detours/db_test.exs index a0fa534a6..cdafe3aab 100644 --- a/test/skate/detours/db_test.exs +++ b/test/skate/detours/db_test.exs @@ -58,5 +58,29 @@ defmodule Skate.Detours.DbTest do detour = detour_fixture() assert %Ecto.Changeset{} = Detours.change_detour(detour) end + + test "change_detour/1 changes :status when :state updates" do + detour = build(:detour, status: nil) + + assert nil == + detour + |> Detours.change_detour(%{}) + |> Ecto.Changeset.get_change(:status) + + assert :draft == + detour + |> Detours.change_detour(%{state: with_id(detour.state, 100)}) + |> Ecto.Changeset.get_change(:status) + + assert :active == + detour + |> Detours.change_detour(%{state: activated(detour.state)}) + |> Ecto.Changeset.get_change(:status) + + assert :past == + detour + |> Detours.change_detour(%{state: deactivated(detour.state)}) + |> Ecto.Changeset.get_change(:status) + end end end diff --git a/test/skate_web/controllers/detours_controller_test.exs b/test/skate_web/controllers/detours_controller_test.exs index bf8a8ac14..caf53be41 100644 --- a/test/skate_web/controllers/detours_controller_test.exs +++ b/test/skate_web/controllers/detours_controller_test.exs @@ -67,6 +67,31 @@ defmodule SkateWeb.DetoursControllerTest do } = Detours.get_detour!(8) end + @tag :authenticated + test "updates :status to match snapshot", %{conn: conn} do + setup_notification_server() + + draft_id = 1 + activated_id = 2 + past_id = 3 + + conn + |> put(~p"/api/detours/update_snapshot", %{ + "snapshot" => :detour_snapshot |> build() |> with_id(draft_id) + }) + |> put(~p"/api/detours/update_snapshot", %{ + "snapshot" => :detour_snapshot |> build() |> activated |> with_id(activated_id) + }) + |> put(~p"/api/detours/update_snapshot", %{ + "snapshot" => :detour_snapshot |> build() |> deactivated |> with_id(past_id) + }) + + Process.sleep(10) + assert Skate.Detours.Detours.get_detour!(draft_id).status === :draft + assert Skate.Detours.Detours.get_detour!(activated_id).status === :active + assert Skate.Detours.Detours.get_detour!(past_id).status === :past + end + @tag :authenticated test "creates a new notification when detour is activated", %{conn: conn} do setup_notification_server() diff --git a/test/support/factories/detour_factory.ex b/test/support/factories/detour_factory.ex index e5ceada0a..91e4d7dee 100644 --- a/test/support/factories/detour_factory.ex +++ b/test/support/factories/detour_factory.ex @@ -18,7 +18,8 @@ defmodule Skate.DetourFactory do def detour_factory do %Skate.Detours.Db.Detour{ author: build(:user), - state: build(:detour_snapshot) + state: build(:detour_snapshot), + status: :draft } end @@ -80,7 +81,8 @@ defmodule Skate.DetourFactory do %{ detour | state: activated(detour.state, activated_at, estimated_duration), - activated_at: activated_at + activated_at: activated_at, + status: :active } end @@ -103,7 +105,7 @@ defmodule Skate.DetourFactory do end def deactivated(%Skate.Detours.Db.Detour{} = detour) do - %{detour | state: deactivated(detour.state)} + %{detour | state: deactivated(detour.state), status: :past} end def deactivated(%{"value" => %{}} = state) do