feat(ex/skate/detours): add activated_at column (#2910)

* feat(ex/skate/detours): add `activated_at` column

* fix: commit all keys to db, except inserted at

* feat(ex/skate/detours): deserialize `activatedAt` from snapshot

* feat(ts/detours): add `activated_at` to detour snapshot

* feat(ts/detours): add `activated_at` when detour is activated

* feat(ex/skate-web/detours-controller): add tests for new `activated_at` field

Also update factory to add new field

assets/src/models/createDetourMachine.ts

@@ -11,6 +11,7 @@ import {
 } from "../api"
 import { DetourShape, FinishedDetour } from "./detour"
 import { fullStoryEvent } from "../helpers/fullStory"
+import { type, optional, coerce, date, string } from "superstruct"
 
 export const createDetourMachine = setup({
   types: {
@@ -35,6 +36,8 @@ export const createDetourMachine = setup({
 
       selectedDuration?: string
       selectedReason?: string
+
+      activatedAt?: Date
     },
 
     input: {} as
@@ -623,6 +626,11 @@ export const createDetourMachine = setup({
                     },
                     "detour.share.activate-modal.activate": {
                       target: "Done",
+                      actions: assign({
+                        // Record current time, should be done on the backend,
+                        // but that requires a larger refactor of the state machine
+                        activatedAt: new Date(),
+                      }),
                     },
                   },
                 },
@@ -701,3 +709,15 @@ export const createDetourMachine = setup({
 export type CreateDetourMachineInput = InputFrom<
   ActorLogicFrom<typeof createDetourMachine>
 >
+
+/**
+ * Defines expected keys and type coercions in Superstruct to enable the
+ * {@linkcode createDetourMachine} to use rich types when rehydrating from a
+ * API response.
+ */
+export const DetourSnapshotData = type({
+  context: type({
+    // Convert serialized dates back into `Date`'s
+    activatedAt: optional(coerce(date(), string(), (str) => new Date(str))),
+  }),
+})

assets/src/models/detour.ts

@@ -1,7 +1,10 @@
 import { LatLngLiteral } from "leaflet"
 import { ShapePoint, Stop } from "../schedule"
-import { CreateDetourMachineInput } from "./createDetourMachine"
-import { any, Infer, number, string, type } from "superstruct"
+import {
+  CreateDetourMachineInput,
+  DetourSnapshotData,
+} from "./createDetourMachine"
+import { Infer, number, string, type } from "superstruct"
 
 export interface DetourWithState {
   author: string
@@ -11,7 +14,7 @@ export interface DetourWithState {
 
 export const DetourWithStateData = type({
   author: string(),
-  state: any(),
+  state: DetourSnapshotData,
   updated_at: number(),
 })
 

lib/skate/detours/db/detour.ex

@@ -12,12 +12,15 @@ defmodule Skate.Detours.Db.Detour do
     field :state, :map
     belongs_to :author, User
 
+    # When this detour was activated
+    field :activated_at, :utc_datetime_usec
+
     timestamps()
   end
 
   def changeset(detour, attrs) do
     detour
-    |> cast(attrs, [:state])
+    |> cast(attrs, [:state, :activated_at])
     |> validate_required([:state])
   end
 end

lib/skate/detours/detours.ex

@@ -203,7 +203,7 @@ defmodule Skate.Detours.Detours do
       |> Skate.Repo.insert(
         returning: true,
         conflict_target: [:id],
-        on_conflict: {:replace, [:state, :updated_at]}
+        on_conflict: {:replace_all_except, [:inserted_at]}
       )
 
     case detour_db_result do

lib/skate/detours/snapshot_serde.ex

@@ -21,11 +21,18 @@ defmodule Skate.Detours.SnapshotSerde do
       %{
         # Save Snapshot to DB until we've fully transitioned to serializing
         # snapshots from DB data
-        state: snapshot
+        state: snapshot,
+        activated_at: activated_at_from_snapshot(snapshot)
       }
     )
   end
 
+  defp activated_at_from_snapshot(%{"context" => %{"activatedAt" => activated_at}}),
+    do: activated_at
+
+  defp activated_at_from_snapshot(_),
+    do: nil
+
   @doc """
   Extracts the Detour ID from a XState Snapshot
   """
@@ -89,7 +96,8 @@ defmodule Skate.Detours.SnapshotSerde do
       "finishedDetour" => finisheddetour_from_detour(detour),
       "editedDirections" => editeddirections_from_detour(detour),
       "selectedDuration" => selectedduration_from_detour(detour),
-      "selectedReason" => selectedreason_from_detour(detour)
+      "selectedReason" => selectedreason_from_detour(detour),
+      "activatedAt" => activated_at_from_detour(detour)
     })
   end
 
@@ -281,6 +289,25 @@ defmodule Skate.Detours.SnapshotSerde do
 
   defp selectedreason_from_detour(_), do: nil
 
+  defp activated_at_from_detour(%Detour{activated_at: %DateTime{} = activated_at}) do
+    activated_at
+    # For the time being, the frontend is responsible for generating the
+    # `activated_at` snapshot. Because browsers are limited to millisecond
+    # resolution and Ecto doesn't preserve the `milliseconds` field of a
+    # `DateTime`, we need to truncate the date if we want it to match what's in
+    # the stored snapshot.
+    #
+    # Once we're not trying to be equivalent with the stored snapshot, we could
+    # probably remove this.
+    #
+    # See `Skate.DetourFactory.browser_date/1` and `Skate.DetourFactory.db_date`
+    # for more context.
+    |> DateTime.truncate(:millisecond)
+    |> DateTime.to_iso8601()
+  end
+
+  defp activated_at_from_detour(%Detour{activated_at: nil}), do: nil
+
   # defp snapshot_children_from_detour(%Detour{snapshot_children: snapshot_children}), do: snapshot_children
   defp snapshot_children_from_detour(%Detour{
          state: %{

priv/repo/async_migrations/20241218135700_backfill_detour_activated_at.exs

@@ -0,0 +1,49 @@
+defmodule Skate.Repo.Migrations.BackfillDetourActivatedAt do
+  # https://fly.io/phoenix-files/backfilling-data/
+
+
+  import Ecto.Query
+  use Ecto.Migration
+
+  @disable_ddl_transaction true
+  @disable_migration_lock true
+
+  def up do
+    # The 'backfilling-data' blog post suggests using a "throttle" function
+    # so that we don't update too many at once, but we currently have less than
+    # 1000 detours, so I think this will be negligable and not worth the
+    # complexity cost at _this_ time.
+    repo().update_all(
+      from(
+        r in Skate.Repo.Migrations.BackfillDetourActivatedAt.MigratingSchema,
+        select: r.id,
+        where: not is_nil(r.state["value"]["Detour Drawing"]["Active"]) and is_nil(r.activated_at),
+        update: [set: [activated_at: r.updated_at]]
+      ),
+      [],
+      log: :info
+    )
+   end
+
+  def down, do: :ok
+end
+
+defmodule Skate.Repo.Migrations.BackfillDetourActivatedAt.MigratingSchema do
+  @moduledoc """
+  Detours database table schema frozen at this point in time.
+  """
+
+  use Skate.Schema
+
+  alias Skate.Settings.Db.User
+
+  typed_schema "detours" do
+    field :state, :map
+    belongs_to :author, User
+
+    # When this detour was activated
+    field :activated_at, :utc_datetime_usec
+
+    timestamps()
+  end
+end

priv/repo/migrations/20241211183227_add_detour_activated_at_field.exs

@@ -0,0 +1,9 @@
+defmodule Skate.Repo.Migrations.AddDetourActivatedAtField do
+  use Ecto.Migration
+
+  def change do
+    alter table(:detours) do
+      add :activated_at, :utc_datetime_usec
+    end
+  end
+end

test/skate_web/controllers/detours_controller_test.exs

@@ -81,6 +81,23 @@ defmodule SkateWeb.DetoursControllerTest do
       assert Skate.Repo.aggregate(Notifications.Db.Detour, :count) == 1
     end
 
+    @tag :authenticated
+    test "adds `activated_at` field when provided", %{conn: conn} do
+      %Skate.Detours.Db.Detour{id: id, state: snapshot, activated_at: nil} = insert(:detour)
+
+      activated_at_time =
+        DateTime.utc_now() |> Skate.DetourFactory.browser_date() |> Skate.DetourFactory.db_date()
+
+      put(conn, ~p"/api/detours/update_snapshot", %{
+        "snapshot" => snapshot |> activated(activated_at_time) |> with_id(id)
+      })
+
+      Process.sleep(10)
+
+      assert Skate.Detours.Detours.get_detour!(id).activated_at ==
+               activated_at_time
+    end
+
     @tag :authenticated
     test "does not create a new notification if detour was already activated", %{conn: conn} do
       setup_notification_server()
@@ -238,11 +255,45 @@ defmodule SkateWeb.DetoursControllerTest do
 
       put(conn, "/api/detours/update_snapshot", %{"snapshot" => detour_snapshot})
 
-      conn = get(conn, "/api/detours/#{detour_id}")
+      {conn, log} =
+        CaptureLog.with_log(fn ->
+          get(conn, "/api/detours/#{detour_id}")
+        end)
+
+      refute log =~
+               "Serialized detour doesn't match saved snapshot. Falling back to snapshot for detour_id=#{detour_id}"
 
       assert detour_snapshot == json_response(conn, 200)["data"]["state"]
     end
 
+    @tag :authenticated
+    test "serialized snapshot `activatedAt` value is formatted as iso-8601", %{conn: conn} do
+      activated_at = Skate.DetourFactory.browser_date()
+
+      %{id: id} =
+        detour =
+        :detour
+        |> build()
+        |> activated(activated_at)
+        |> insert()
+
+      # Make ID match snapshot
+      detour
+      |> Skate.Detours.Detours.change_detour(detour |> update_id() |> Map.from_struct())
+      |> Skate.Repo.update!()
+
+      {conn, log} =
+        CaptureLog.with_log(fn ->
+          get(conn, "/api/detours/#{id}")
+        end)
+
+      refute log =~
+               "Serialized detour doesn't match saved snapshot. Falling back to snapshot for detour_id=#{id}"
+
+      assert DateTime.to_iso8601(activated_at) ==
+               json_response(conn, 200)["data"]["state"]["context"]["activatedAt"]
+    end
+
     @tag :authenticated
     test "log an error if the serialized detour does not match db state", %{conn: conn} do
       detour_id = 4

test/support/factories/detour_factory.ex

@@ -57,12 +57,27 @@ defmodule Skate.DetourFactory do
         put_in(snapshot["context"]["uuid"], id)
       end
 
-      def activated(%Skate.Detours.Db.Detour{} = detour) do
-        %{detour | state: activated(detour.state)}
+      def update_id(%Skate.Detours.Db.Detour{id: id} = detour) do
+        with_id(detour, id)
       end
 
-      def activated(%{"value" => %{}} = state) do
-        put_in(state["value"], %{"Detour Drawing" => %{"Active" => "Reviewing"}})
+      def activated(update_arg, activated_at \\ DateTime.utc_now())
+
+      def activated(%Skate.Detours.Db.Detour{} = detour, activated_at) do
+        activated_at = Skate.DetourFactory.browser_date(activated_at)
+        %{detour | state: activated(detour.state, activated_at), activated_at: activated_at}
+      end
+
+      def activated(%{"value" => %{}, "context" => %{}} = state, activated_at) do
+        state =
+          put_in(state["value"], %{"Detour Drawing" => %{"Active" => "Reviewing"}})
+
+        put_in(
+          state["context"]["activatedAt"],
+          activated_at
+          |> Skate.DetourFactory.browser_date()
+          |> DateTime.to_iso8601()
+        )
       end
 
       def deactivated(%Skate.Detours.Db.Detour{} = detour) do
@@ -95,4 +110,32 @@ defmodule Skate.DetourFactory do
       end
     end
   end
+
+  @doc """
+  Browsers cannot generate javascript `Date` objects with more precision than a
+  `millisecond` for security reasons.
+  https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/now#reduced_time_precision
+
+  This function truncates a `DateTime` to milliseconds to create `DateTime` objects
+  that are similar to that of one made in a Browser JS context.
+  """
+  def browser_date(%DateTime{} = date \\ DateTime.utc_now()) do
+    DateTime.truncate(date, :millisecond)
+  end
+
+  @doc """
+  While a Browser may generate a date truncated to milliseconds
+  (see `browser_date` for more context) a `DateTime` stored into Postgres with
+  the `:utc_datetime_usec` type does not store the extra information about the
+  non-presence of nanoseconds that a `DateTime` object does.
+  This means a `DateTime` object that's been truncated by `browser_date` cannot
+  be compared to a `DateTime` object reconstructed by Ecto after a Database query.
+
+  This function adds 0 nanoseconds to a `DateTime` object to make the `DateTime`
+  object match what Ecto would return to make testing easier when comparing
+  values.
+  """
+  def db_date(%DateTime{} = date) do
+    DateTime.add(date, 0, :nanosecond)
+  end
 end