From cf8a9efd5681a30f7f53fe75221632c03caf6428 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Mon, 22 Jun 2026 20:47:07 -0400 Subject: [PATCH 1/6] chore(format): mix format pre-existing files These were already unformatted on main and fail `mix format --check-formatted` in CI; formatting them here so the station-journal PR's CI is green. No behavior change. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UyezV5kznW5rbRFVsoXWvh --- lib/gtfs_planner_web/live/gtfs/station_diagram_live.ex | 5 +++-- .../live/gtfs/station_diagram_live_map_mode_test.exs | 3 +-- .../gtfs_planner_web/live/gtfs/station_diagram_live_test.exs | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/gtfs_planner_web/live/gtfs/station_diagram_live.ex b/lib/gtfs_planner_web/live/gtfs/station_diagram_live.ex index 6a33ac78..ba7c45ac 100644 --- a/lib/gtfs_planner_web/live/gtfs/station_diagram_live.ex +++ b/lib/gtfs_planner_web/live/gtfs/station_diagram_live.ex @@ -93,7 +93,7 @@ defmodule GtfsPlannerWeb.Gtfs.StationDiagramLive do |> assign(:naming_excluded_ids, MapSet.new()) |> assign(:reference_level_id, nil) |> assign(:reference_stop_level, nil) - |> assign(:reference_level_index, nil) + |> assign(:reference_level_index, nil) |> assign(:show_reference_overlay, false) |> assign(:audit_ctx, nil) |> assign(:history_open_for, nil) @@ -539,7 +539,8 @@ defmodule GtfsPlannerWeb.Gtfs.StationDiagramLive do |> Enum.map(&child_stop_marker/1) |> Enum.reject(&is_nil/1) - {markers, assign(socket, :reference_child_stop_markers_cache, Map.put(cache, level_id, markers))} + {markers, + assign(socket, :reference_child_stop_markers_cache, Map.put(cache, level_id, markers))} end end diff --git a/test/gtfs_planner_web/live/gtfs/station_diagram_live_map_mode_test.exs b/test/gtfs_planner_web/live/gtfs/station_diagram_live_map_mode_test.exs index cebf54b0..fb359dec 100644 --- a/test/gtfs_planner_web/live/gtfs/station_diagram_live_map_mode_test.exs +++ b/test/gtfs_planner_web/live/gtfs/station_diagram_live_map_mode_test.exs @@ -1621,6 +1621,5 @@ defmodule GtfsPlannerWeb.Gtfs.StationDiagramLiveMapModeTest do assert reloaded.floorplan_scale_mpp == before.floorplan_scale_mpp assert reloaded.floorplan_rotation_deg == before.floorplan_rotation_deg end - end -end \ No newline at end of file +end diff --git a/test/gtfs_planner_web/live/gtfs/station_diagram_live_test.exs b/test/gtfs_planner_web/live/gtfs/station_diagram_live_test.exs index 48199c59..80eee9a8 100644 --- a/test/gtfs_planner_web/live/gtfs/station_diagram_live_test.exs +++ b/test/gtfs_planner_web/live/gtfs/station_diagram_live_test.exs @@ -12149,5 +12149,4 @@ defmodule GtfsPlannerWeb.Gtfs.StationDiagramLiveTest do assert state.socket.assigns.rollback_preview == nil end end - end From d509179782c645ccdd063e6df3a8905dc391d2ee Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Mon, 22 Jun 2026 20:47:07 -0400 Subject: [PATCH 2/6] =?UTF-8?q?feat(api):=20station=20journal=20entries=20?= =?UTF-8?q?=E2=80=94=20schema,=20sync=20upsert,=20bundle=20serialization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend PR A of the station journal (companion specs/api/station-journal.md): the journal_entries table + the entries half of the contract. Photos and pin targets follow in later PRs. - Migration + JournalEntry schema: client-generated id (upsert key), org/version scoping, station_id, polymorphic target (station|node|pathway; pin later), body, author_id, captured_at, resolved_at. Author as a plain id (change_logs precedent). Target_id validity is a changeset rule. - Context: list_journal_entries_for_station/3 and upsert_journal_entry/1 (idempotent on id; LWW on body/resolved_at). - Sync: SyncController.create/2 also upserts a journal_entries[] array, scoped to the path's version/station + the session's org/user, with per-item errors (partial success stays 200, like the pathway loop) and journal_synced_count. Entries for a station outside the org are rejected. - Bundle: station-targeted entries top-level, node/pathway entries on their stop/pathway, each nesting photos: [] (populated once photos land). Tests: bundle serialization across station/node/pathway targets; sync create, idempotent upsert, per-item validation error, and cross-org rejection. Full suite green (1919 tests). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UyezV5kznW5rbRFVsoXWvh --- lib/gtfs_planner/gtfs.ex | 30 ++++ lib/gtfs_planner/gtfs/journal_entry.ex | 103 +++++++++++ .../api/v1/station_controller.ex | 46 ++++- .../api/v1/sync_controller.ex | 106 +++++++++-- .../20260623003416_create_journal_entries.exs | 45 +++++ .../api/v1/station_controller_test.exs | 76 ++++++++ .../api/v1/sync_controller_test.exs | 165 ++++++++++++++++++ 7 files changed, 548 insertions(+), 23 deletions(-) create mode 100644 lib/gtfs_planner/gtfs/journal_entry.ex create mode 100644 priv/repo/migrations/20260623003416_create_journal_entries.exs diff --git a/lib/gtfs_planner/gtfs.ex b/lib/gtfs_planner/gtfs.ex index 98df7211..cc223f9b 100644 --- a/lib/gtfs_planner/gtfs.ex +++ b/lib/gtfs_planner/gtfs.ex @@ -26,6 +26,7 @@ defmodule GtfsPlanner.Gtfs do alias GtfsPlanner.Gtfs.FeedInfo alias GtfsPlanner.Gtfs.FloorplanTransform alias GtfsPlanner.Gtfs.Frequency + alias GtfsPlanner.Gtfs.JournalEntry alias GtfsPlanner.Gtfs.Level alias GtfsPlanner.Gtfs.Location alias GtfsPlanner.Gtfs.Network @@ -2178,6 +2179,35 @@ defmodule GtfsPlanner.Gtfs do |> Repo.all() end + @doc """ + All station-journal entries for a station (any target), oldest first. See the + companion app's `specs/api/station-journal.md`. + """ + def list_journal_entries_for_station(organization_id, gtfs_version_id, station_id) do + from(e in JournalEntry, + where: + e.organization_id == ^organization_id and + e.gtfs_version_id == ^gtfs_version_id and + e.station_id == ^station_id, + order_by: [asc: e.captured_at, asc: e.inserted_at] + ) + |> Repo.all() + end + + @doc """ + Upsert a station-journal entry by its client-generated `id` (idempotent). On + conflict, the mutable fields are replaced (last-write-wins on `body` / + `resolved_at`); scoping and authorship are preserved from the original insert. + """ + def upsert_journal_entry(attrs) do + %JournalEntry{} + |> JournalEntry.changeset(attrs) + |> Repo.insert( + on_conflict: {:replace, [:body, :resolved_at, :target_type, :target_id, :updated_at]}, + conflict_target: :id + ) + end + defp descendant_stop_ids_query(organization_id, gtfs_version_id, station_stop_id) do direct_child_ids = from(s in Stop, diff --git a/lib/gtfs_planner/gtfs/journal_entry.ex b/lib/gtfs_planner/gtfs/journal_entry.ex new file mode 100644 index 00000000..481de8a4 --- /dev/null +++ b/lib/gtfs_planner/gtfs/journal_entry.ex @@ -0,0 +1,103 @@ +defmodule GtfsPlanner.Gtfs.JournalEntry do + @moduledoc """ + A station-journal entry: a field-collected note and/or photos, optionally + anchored to a target within the station. See the companion app's + `specs/api/station-journal.md`. + + The `id` is client-generated (stable across offline capture → sync) and is the + upsert key. `target_type` is polymorphic over `station` (target_id null), + `node`, and `pathway` (the `pin` target and its diagram coordinate columns are + added by a later migration). + """ + use Ecto.Schema + import Ecto.Changeset + import GtfsPlanner.ChangesetHelpers + + @target_types ~w(station node pathway) + + @type t :: %__MODULE__{ + id: Ecto.UUID.t(), + organization_id: Ecto.UUID.t(), + gtfs_version_id: Ecto.UUID.t(), + station_id: Ecto.UUID.t(), + target_type: String.t(), + target_id: Ecto.UUID.t() | nil, + body: String.t() | nil, + author_id: Ecto.UUID.t(), + captured_at: DateTime.t(), + resolved_at: DateTime.t() | nil, + inserted_at: DateTime.t(), + updated_at: DateTime.t() + } + + @primary_key {:id, :binary_id, autogenerate: false} + @foreign_key_type :binary_id + + schema "journal_entries" do + field :target_type, :string + field :target_id, :binary_id + field :body, :string + field :author_id, :binary_id + field :captured_at, :utc_datetime_usec + field :resolved_at, :utc_datetime_usec + field :station_id, :binary_id + + belongs_to :organization, GtfsPlanner.Organizations.Organization, + foreign_key: :organization_id + + belongs_to :gtfs_version, GtfsPlanner.Versions.GtfsVersion + + timestamps(type: :utc_datetime_usec) + end + + def target_types, do: @target_types + + def changeset(entry, attrs) do + entry + |> cast(attrs, [ + :id, + :organization_id, + :gtfs_version_id, + :station_id, + :target_type, + :target_id, + :body, + :author_id, + :captured_at, + :resolved_at + ]) + |> trim_string_fields() + |> validate_required([ + :id, + :organization_id, + :gtfs_version_id, + :station_id, + :target_type, + :author_id, + :captured_at + ]) + |> validate_inclusion(:target_type, @target_types) + |> validate_target_id() + |> foreign_key_constraint(:organization_id) + |> foreign_key_constraint(:gtfs_version_id) + |> foreign_key_constraint(:station_id) + end + + # A node/pathway entry must name its target; a station entry must not. + defp validate_target_id(changeset) do + case get_field(changeset, :target_type) do + type when type in ["node", "pathway"] -> + if get_field(changeset, :target_id), + do: changeset, + else: add_error(changeset, :target_id, "is required for node and pathway targets") + + "station" -> + if get_field(changeset, :target_id), + do: add_error(changeset, :target_id, "must be blank for station targets"), + else: changeset + + _ -> + changeset + end + end +end diff --git a/lib/gtfs_planner_web/api/v1/station_controller.ex b/lib/gtfs_planner_web/api/v1/station_controller.ex index 4df1c4e9..3ed1e2c6 100644 --- a/lib/gtfs_planner_web/api/v1/station_controller.ex +++ b/lib/gtfs_planner_web/api/v1/station_controller.ex @@ -80,6 +80,14 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do levels = Gtfs.list_levels_for_station(org_id, version_id, station.id) pathways = Gtfs.list_pathways_for_station(org_id, version_id, station.id) + # Station journal: group entries by target so each serializes onto its + # node/pathway (and station-targeted ones go top-level). Pin-targeted + # entries land on levels[] once the pin migration is in. + entries_by_target = + org_id + |> Gtfs.list_journal_entries_for_station(version_id, station.id) + |> Enum.group_by(&{&1.target_type, &1.target_id}, &serialize_journal_entry/1) + {station_lat, station_lon} = serialize_coordinates(station.stop_lat, station.stop_lon) @@ -97,8 +105,17 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do lon: station_lon }, levels: Enum.map(levels, &serialize_level(&1, org_id, station.stop_id)), - stops: Enum.map(child_stops, &serialize_stop/1), - pathways: Enum.map(pathways, &serialize_pathway/1), + stops: + Enum.map( + child_stops, + &serialize_stop(&1, Map.get(entries_by_target, {"node", &1.id}, [])) + ), + pathways: + Enum.map( + pathways, + &serialize_pathway(&1, Map.get(entries_by_target, {"pathway", &1.id}, [])) + ), + journal_entries: Map.get(entries_by_target, {"station", nil}, []), diagrams: [], downloaded_at: DateTime.utc_now() |> DateTime.truncate(:second) |> DateTime.to_iso8601() } @@ -178,7 +195,7 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do end end - defp serialize_stop(stop) do + defp serialize_stop(stop, journal_entries) do {lat, lon} = serialize_coordinates(stop.stop_lat, stop.stop_lon) %{ @@ -192,7 +209,8 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do platform_code: stop.platform_code, diagram_coordinate: stop.diagram_coordinate, lat: lat, - lon: lon + lon: lon, + journal_entries: journal_entries } end @@ -233,7 +251,7 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do end end - defp serialize_pathway(pathway) do + defp serialize_pathway(pathway, journal_entries) do %{ id: pathway.id, pathway_id: pathway.pathway_id, @@ -249,7 +267,23 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do signposted_as: pathway.signposted_as, reversed_signposted_as: pathway.reversed_signposted_as, field_notes: pathway.field_notes, - field_completed_at: pathway.field_completed_at + field_completed_at: pathway.field_completed_at, + journal_entries: journal_entries + } + end + + # `photos: []` is always present (forward-compatible); photos are populated + # once the journal-photos table + upload land. See specs/api/station-journal.md. + defp serialize_journal_entry(entry) do + %{ + id: entry.id, + target_type: entry.target_type, + target_id: entry.target_id, + body: entry.body, + author_id: entry.author_id, + captured_at: entry.captured_at, + resolved_at: entry.resolved_at, + photos: [] } end end diff --git a/lib/gtfs_planner_web/api/v1/sync_controller.ex b/lib/gtfs_planner_web/api/v1/sync_controller.ex index cc605304..b8ca0902 100644 --- a/lib/gtfs_planner_web/api/v1/sync_controller.ex +++ b/lib/gtfs_planner_web/api/v1/sync_controller.ex @@ -1,8 +1,9 @@ defmodule GtfsPlannerWeb.Api.V1.SyncController do use GtfsPlannerWeb, :controller - alias GtfsPlanner.Repo + alias GtfsPlanner.Gtfs alias GtfsPlanner.Gtfs.Pathway + alias GtfsPlanner.Repo @editable_fields ~w(traversal_time stair_count min_width signposted_as reversed_signposted_as field_notes field_completed_at)a @@ -14,12 +15,17 @@ defmodule GtfsPlannerWeb.Api.V1.SyncController do # See the companion app's specs/api/sync.md. @doc "POST /api/v1/versions/:version_id/stations/:station_id/sync" - def create(conn, %{ - "version_id" => _version_id, - "station_id" => _station_id, - "pathways" => pathway_updates - }) do + def create( + conn, + %{ + "version_id" => version_id, + "station_id" => station_id, + "pathways" => pathway_updates + } = params + ) do org_id = conn.assigns[:current_organization_id] + author_id = conn.assigns[:current_user_id] + journal_updates = Map.get(params, "journal_entries", []) results = Enum.reduce(pathway_updates, %{synced: 0, errors: []}, fn update, acc -> @@ -92,21 +98,19 @@ defmodule GtfsPlannerWeb.Api.V1.SyncController do end end) - response = %{ - data: %{ + journal = sync_journal_entries(journal_updates, org_id, version_id, station_id, author_id) + + errors = Enum.reverse(results.errors) ++ journal.errors + + data = + %{ synced_count: results.synced, synced_at: DateTime.utc_now() |> DateTime.truncate(:second) |> DateTime.to_iso8601() } - } - - response = - if results.errors != [] do - put_in(response, [:data, :errors], Enum.reverse(results.errors)) - else - response - end + |> maybe_put(:journal_synced_count, journal_updates != [], journal.synced) + |> maybe_put(:errors, errors != [], errors) - json(conn, response) + json(conn, %{data: data}) end def create(conn, _params) do @@ -145,4 +149,72 @@ defmodule GtfsPlannerWeb.Api.V1.SyncController do :invalid_endpoints end end + + defp maybe_put(map, _key, false, _value), do: map + defp maybe_put(map, key, true, value), do: Map.put(map, key, value) + + # Upsert station-journal entries by client-generated id (idempotent). Each item + # is independent, like the pathway loop — partial failure stays 200 with + # per-item errors. Entries are scoped to the path's version/station and the + # session's org/user; a request to a station outside the org fails them all. + defp sync_journal_entries([], _org_id, _version_id, _station_id, _author_id), + do: %{synced: 0, errors: []} + + defp sync_journal_entries(updates, org_id, version_id, station_id, author_id) do + if valid_station?(Gtfs.get_stop(station_id), org_id, version_id) do + updates + |> Enum.reduce(%{synced: 0, errors: []}, fn update, acc -> + attrs = %{ + "id" => update["id"], + "organization_id" => org_id, + "gtfs_version_id" => version_id, + "station_id" => station_id, + "author_id" => author_id, + "target_type" => update["target_type"], + "target_id" => update["target_id"], + "body" => update["body"], + "captured_at" => update["captured_at"], + "resolved_at" => update["resolved_at"] + } + + case Gtfs.upsert_journal_entry(attrs) do + {:ok, _entry} -> + %{acc | synced: acc.synced + 1} + + {:error, _changeset} -> + %{ + acc + | errors: [ + %{ + id: update["id"], + code: "validation_error", + message: "Failed to save journal entry." + } + | acc.errors + ] + } + end + end) + |> then(fn acc -> %{acc | errors: Enum.reverse(acc.errors)} end) + else + %{ + synced: 0, + errors: + Enum.map(updates, fn u -> + %{id: u["id"], code: "not_found", message: "Station not found."} + end) + } + end + end + + # Repeated variables enforce equality: matches only when the station's org and + # version equal the request's. A nil station (not found) falls through to false. + defp valid_station?( + %{organization_id: org_id, gtfs_version_id: version_id}, + org_id, + version_id + ), + do: true + + defp valid_station?(_station, _org_id, _version_id), do: false end diff --git a/priv/repo/migrations/20260623003416_create_journal_entries.exs b/priv/repo/migrations/20260623003416_create_journal_entries.exs new file mode 100644 index 00000000..58aaacf0 --- /dev/null +++ b/priv/repo/migrations/20260623003416_create_journal_entries.exs @@ -0,0 +1,45 @@ +defmodule GtfsPlanner.Repo.Migrations.CreateJournalEntries do + use Ecto.Migration + + def change do + create table(:journal_entries, primary_key: false) do + # Client-generated UUID — stable across offline capture → sync, and the + # upsert key (no autogenerate). + add :id, :binary_id, primary_key: true + + add :organization_id, + references(:organizations, type: :binary_id, on_delete: :delete_all), + null: false + + add :gtfs_version_id, + references(:gtfs_versions, type: :binary_id, on_delete: :delete_all), + null: false + + # The parent station the entry belongs to (every journal endpoint is + # station-scoped); used to gather a station's journal for the bundle. + add :station_id, + references(:stops, type: :binary_id, on_delete: :delete_all), + null: false + + # Polymorphic target (change_logs.entity_type/entity_id precedent): + # "station" (target_id null) | "node" | "pathway". "pin" + its diagram + # coordinate columns are added by a later migration. + add :target_type, :string, null: false + add :target_id, :binary_id + + add :body, :text + # Author as a plain id (the change_logs.actor_id precedent) — no FK, so a + # user deletion never cascades into or nulls journal history. + add :author_id, :binary_id, null: false + add :captured_at, :utc_datetime_usec, null: false + add :resolved_at, :utc_datetime_usec + + timestamps(type: :utc_datetime_usec) + end + + # Bundle gathers a station's entries by (org, version, station). + create index(:journal_entries, [:organization_id, :gtfs_version_id, :station_id]) + # Group node/pathway entries onto their target during serialization. + create index(:journal_entries, [:target_type, :target_id]) + end +end diff --git a/test/gtfs_planner_web/api/v1/station_controller_test.exs b/test/gtfs_planner_web/api/v1/station_controller_test.exs index 311d1659..3ede0e02 100644 --- a/test/gtfs_planner_web/api/v1/station_controller_test.exs +++ b/test/gtfs_planner_web/api/v1/station_controller_test.exs @@ -7,6 +7,7 @@ defmodule GtfsPlannerWeb.Api.V1.StationControllerTest do import GtfsPlanner.GtfsFixtures alias GtfsPlanner.Accounts + alias GtfsPlanner.Gtfs alias GtfsPlanner.Gtfs.StopLevel alias GtfsPlanner.Repo @@ -333,6 +334,81 @@ defmodule GtfsPlannerWeb.Api.V1.StationControllerTest do assert is_binary(data["downloaded_at"]) end + test "attaches journal entries to station, node, and pathway (each with photos: [])", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + + %{station: station, child1: child1, pathway: pathway} = + build_station_data(org.id, version.id) + + base = %{ + organization_id: org.id, + gtfs_version_id: version.id, + station_id: station.id, + author_id: user.id, + captured_at: ~U[2026-06-20 10:00:00.000000Z] + } + + {:ok, _} = + Gtfs.upsert_journal_entry( + Map.merge(base, %{ + id: Ecto.UUID.generate(), + target_type: "station", + body: "Station note" + }) + ) + + {:ok, _} = + Gtfs.upsert_journal_entry( + Map.merge(base, %{ + id: Ecto.UUID.generate(), + target_type: "node", + target_id: child1.id, + body: "Node note" + }) + ) + + {:ok, _} = + Gtfs.upsert_journal_entry( + Map.merge(base, %{ + id: Ecto.UUID.generate(), + target_type: "pathway", + target_id: pathway.id, + body: "Pathway note" + }) + ) + + conn = + conn + |> authed_conn(user) + |> get("/api/v1/versions/#{version.id}/stations/#{station.id}/bundle") + + assert %{"data" => data} = json_response(conn, 200) + + # Station-targeted entries ride at the top level. + assert [station_entry] = data["journal_entries"] + assert station_entry["target_type"] == "station" + assert station_entry["body"] == "Station note" + assert station_entry["photos"] == [] + + # Node-targeted entries ride on their stop. + stop_json = Enum.find(data["stops"], &(&1["id"] == child1.id)) + assert [node_entry] = stop_json["journal_entries"] + assert node_entry["body"] == "Node note" + + # Pathway-targeted entries ride on their pathway. + p = hd(data["pathways"]) + assert [pathway_entry] = p["journal_entries"] + assert pathway_entry["body"] == "Pathway note" + + # An unrelated stop carries an empty array. + other = Enum.find(data["stops"], &(&1["id"] != child1.id)) + assert other["journal_entries"] == [] + end + test "level floorplan carries url + alignment when the stop_level is aligned", %{ conn: conn, user: user, diff --git a/test/gtfs_planner_web/api/v1/sync_controller_test.exs b/test/gtfs_planner_web/api/v1/sync_controller_test.exs index 5ddd0e6f..a0d99ddb 100644 --- a/test/gtfs_planner_web/api/v1/sync_controller_test.exs +++ b/test/gtfs_planner_web/api/v1/sync_controller_test.exs @@ -8,6 +8,7 @@ defmodule GtfsPlannerWeb.Api.V1.SyncControllerTest do alias GtfsPlanner.Accounts alias GtfsPlanner.Repo + alias GtfsPlanner.Gtfs.JournalEntry alias GtfsPlanner.Gtfs.Pathway # --------------------------------------------------------------------------- @@ -370,4 +371,168 @@ defmodule GtfsPlannerWeb.Api.V1.SyncControllerTest do assert updated.traversal_time == 90 end end + + describe "create/2 — journal entries" do + setup [:setup_user_with_org] + + test "creates station/node/pathway journal entries alongside pathways", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + + %{station: station, child1: child1, pathway: pathway} = + build_station_with_pathway(org.id, version.id) + + station_entry_id = Ecto.UUID.generate() + node_entry_id = Ecto.UUID.generate() + pathway_entry_id = Ecto.UUID.generate() + + payload = %{ + "pathways" => [], + "journal_entries" => [ + %{ + "id" => station_entry_id, + "target_type" => "station", + "body" => "Whole-station note", + "captured_at" => "2026-06-20T10:00:00Z" + }, + %{ + "id" => node_entry_id, + "target_type" => "node", + "target_id" => child1.id, + "body" => "Node note", + "captured_at" => "2026-06-20T10:01:00Z" + }, + %{ + "id" => pathway_entry_id, + "target_type" => "pathway", + "target_id" => pathway.id, + "body" => "Pathway note", + "captured_at" => "2026-06-20T10:02:00Z" + } + ] + } + + conn = conn |> authed_conn(user) |> post(sync_url(version.id, station.id), payload) + + assert %{"data" => data} = json_response(conn, 200) + assert data["journal_synced_count"] == 3 + refute Map.has_key?(data, "errors") + + entry = Repo.get!(JournalEntry, node_entry_id) + assert entry.target_type == "node" + assert entry.target_id == child1.id + assert entry.body == "Node note" + assert entry.author_id == user.id + assert entry.organization_id == org.id + assert entry.station_id == station.id + end + + test "upserts by id (idempotent) and updates body on re-sync", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + %{station: station} = build_station_with_pathway(org.id, version.id) + id = Ecto.UUID.generate() + + post1 = %{ + "pathways" => [], + "journal_entries" => [ + %{ + "id" => id, + "target_type" => "station", + "body" => "First", + "captured_at" => "2026-06-20T10:00:00Z" + } + ] + } + + conn |> authed_conn(user) |> post(sync_url(version.id, station.id), post1) + + post2 = %{ + "pathways" => [], + "journal_entries" => [ + %{ + "id" => id, + "target_type" => "station", + "body" => "Edited", + "captured_at" => "2026-06-20T10:00:00Z" + } + ] + } + + conn2 = build_conn() |> authed_conn(user) |> post(sync_url(version.id, station.id), post2) + + assert %{"data" => data} = json_response(conn2, 200) + assert data["journal_synced_count"] == 1 + assert Repo.aggregate(JournalEntry, :count, :id) == 1 + assert Repo.get!(JournalEntry, id).body == "Edited" + end + + test "reports a per-item error without failing the batch", %{conn: conn, user: user, org: org} do + version = gtfs_version_fixture(org.id) + %{station: station} = build_station_with_pathway(org.id, version.id) + good_id = Ecto.UUID.generate() + + payload = %{ + "pathways" => [], + "journal_entries" => [ + %{ + "id" => good_id, + "target_type" => "station", + "body" => "ok", + "captured_at" => "2026-06-20T10:00:00Z" + }, + %{ + "id" => Ecto.UUID.generate(), + "target_type" => "bogus", + "body" => "bad", + "captured_at" => "2026-06-20T10:00:00Z" + } + ] + } + + conn = conn |> authed_conn(user) |> post(sync_url(version.id, station.id), payload) + + assert %{"data" => data} = json_response(conn, 200) + assert data["journal_synced_count"] == 1 + assert [%{"code" => "validation_error"}] = data["errors"] + assert Repo.get(JournalEntry, good_id) + end + + test "rejects journal entries for a station outside the org", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + other_org = organization_fixture() + other_version = gtfs_version_fixture(other_org.id) + %{station: other_station} = build_station_with_pathway(other_org.id, other_version.id) + + payload = %{ + "pathways" => [], + "journal_entries" => [ + %{ + "id" => Ecto.UUID.generate(), + "target_type" => "station", + "body" => "x", + "captured_at" => "2026-06-20T10:00:00Z" + } + ] + } + + # Authed as `user` (org), but posting to another org's station id. + conn = conn |> authed_conn(user) |> post(sync_url(version.id, other_station.id), payload) + + assert %{"data" => data} = json_response(conn, 200) + assert data["journal_synced_count"] == 0 + assert [%{"code" => "not_found"}] = data["errors"] + assert Repo.aggregate(JournalEntry, :count, :id) == 0 + end + end end From 17e0a06f1a8ce0d9182751a8a9eacd8ae9080f38 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Tue, 23 Jun 2026 00:24:25 -0400 Subject: [PATCH 3/6] feat(api): station journal photos + pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the station-journal backend (PRs B + C) on top of journal entries. Photos (B): - journal_photos table + JournalPhoto schema; binary stored under /uploads/field-captures/// and served as a static URL (mirrors floorplans). - POST .../journal-photos multipart upload (idempotent on id; content-type allow-list jpg/png; 25 MB cap; org/version/station + entry scoping). - Bundle nests each entry's photos[] with absolute URLs. Pins (C): - A pin is a journal entry anchored to an arbitrary point on a level — its canonical coordinate is the diagram_coordinate, exactly like a node. Pins work on every station regardless of geo-alignment (most aren't aligned). - lat/lon is optional enrichment imputed at level-alignment time alongside node coordinates (save_and_apply_stop_level_alignment re-imputes the level's pins in the same transaction), never at sync. Re-syncing a pin preserves its imputed lat/lon. - Bundle serves pins on levels[].journal_entries with diagram_coordinate (+ lat/lon when aligned), identical in shape to how nodes carry coordinates. Tests: photo upload (success/idempotent/404/422/bundle nesting); pin sync stores diagram coords with null lat/lon; alignment imputes pin lat/lon alongside nodes; unaligned levels keep null; re-sync preserves imputed coords. Full suite green (1929 tests). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UyezV5kznW5rbRFVsoXWvh --- lib/gtfs_planner/gtfs.ex | 132 ++++++++- lib/gtfs_planner/gtfs/journal_entry.ex | 46 ++- lib/gtfs_planner/gtfs/journal_photo.ex | 75 +++++ .../api/v1/journal_photo_controller.ex | 221 +++++++++++++++ .../api/v1/station_controller.ex | 114 +++++++- .../api/v1/sync_controller.ex | 7 +- lib/gtfs_planner_web/router.ex | 4 + .../20260623034340_create_journal_photos.exs | 34 +++ ...4341_add_pin_target_to_journal_entries.exs | 19 ++ test/gtfs_planner/gtfs_test.exs | 48 ++++ .../api/v1/journal_photo_controller_test.exs | 263 ++++++++++++++++++ .../api/v1/sync_controller_test.exs | 197 +++++++++++++ 12 files changed, 1137 insertions(+), 23 deletions(-) create mode 100644 lib/gtfs_planner/gtfs/journal_photo.ex create mode 100644 lib/gtfs_planner_web/api/v1/journal_photo_controller.ex create mode 100644 priv/repo/migrations/20260623034340_create_journal_photos.exs create mode 100644 priv/repo/migrations/20260623034341_add_pin_target_to_journal_entries.exs create mode 100644 test/gtfs_planner_web/api/v1/journal_photo_controller_test.exs diff --git a/lib/gtfs_planner/gtfs.ex b/lib/gtfs_planner/gtfs.ex index cc223f9b..25c4d4aa 100644 --- a/lib/gtfs_planner/gtfs.ex +++ b/lib/gtfs_planner/gtfs.ex @@ -27,6 +27,7 @@ defmodule GtfsPlanner.Gtfs do alias GtfsPlanner.Gtfs.FloorplanTransform alias GtfsPlanner.Gtfs.Frequency alias GtfsPlanner.Gtfs.JournalEntry + alias GtfsPlanner.Gtfs.JournalPhoto alias GtfsPlanner.Gtfs.Level alias GtfsPlanner.Gtfs.Location alias GtfsPlanner.Gtfs.Network @@ -595,7 +596,15 @@ defmodule GtfsPlanner.Gtfs do |> Repo.update(), {:ok, active_derived} <- derive_child_stop_coords(updated_stop_level, image_w, image_h), - {:ok, active_updated_stops} <- persist_derived_coords_in_tx(active_derived) do + {:ok, active_updated_stops} <- persist_derived_coords_in_tx(active_derived), + # Pins anchor on diagram coordinates like nodes; their lat/lon + # is optional enrichment re-imputed here whenever the level is + # (re)aligned, using the client-supplied image dims — never at + # sync time. + {:ok, active_derived_pins} <- + derive_pin_coords(updated_stop_level, image_w, image_h), + {:ok, _active_updated_pins} <- + persist_derived_pin_coords_in_tx(active_derived_pins) do Logger.debug(fn -> "[ALIGN_APPLY_DEBUG] active_stop_level_persisted " <> inspect(%{ @@ -751,6 +760,74 @@ defmodule GtfsPlanner.Gtfs do end end + # Derives lat/lon enrichment for every `pin` journal entry anchored to this + # level, mirroring `derive_child_stop_coords/3` for nodes: the canonical anchor + # is the pin's diagram coordinate; lat/lon is computed via the same + # `FloorplanTransform.svg_to_lat_lon/4` using the level's alignment and the + # client-supplied image dims. A pin whose diagram point can't normalize is + # skipped (its lat/lon is left untouched). Returns `{:error, :alignment_missing}` + # when the level isn't aligned so the alignment write can't silently produce + # geo-less pins on an aligned level. + defp derive_pin_coords(%StopLevel{} = stop_level, image_w, image_h) do + with {:ok, alignment} <- extract_alignment(stop_level), + :ok <- validate_positive_image_dims(image_w, image_h) do + stop_level.id + |> list_pin_entries_for_level() + |> Enum.reduce_while({:ok, []}, fn entry, {:ok, acc} -> + case Coordinates.normalize_point(%{x: entry.diagram_x, y: entry.diagram_y}) do + nil -> + {:cont, {:ok, acc}} + + %{x: x, y: y} -> + case FloorplanTransform.svg_to_lat_lon(alignment, image_w, image_h, %{x: x, y: y}) do + {:ok, {lat, lon}} -> + {:cont, {:ok, [%{id: entry.id, lat: lat, lon: lon} | acc]}} + + {:error, reason} -> + {:halt, {:error, {:transform, reason}}} + end + end + end) + |> case do + {:ok, entries} -> {:ok, Enum.reverse(entries)} + {:error, _} = error -> error + end + end + end + + defp list_pin_entries_for_level(stop_level_id) do + from(e in JournalEntry, + where: e.target_type == "pin" and e.stop_level_id == ^stop_level_id + ) + |> Repo.all() + end + + defp persist_derived_pin_coords_in_tx(derived) when is_list(derived) do + derived + |> Enum.reduce_while({:ok, []}, fn entry, {:ok, updated} -> + case update_derived_pin_coords(entry) do + {:ok, updated_entry} -> {:cont, {:ok, [updated_entry | updated]}} + {:error, reason} -> {:halt, {:error, reason}} + end + end) + |> case do + {:ok, updated} -> {:ok, Enum.reverse(updated)} + {:error, _} = error -> error + end + end + + defp update_derived_pin_coords(%{id: id, lat: lat, lon: lon}) do + case Repo.get(JournalEntry, id) do + nil -> + {:error, :journal_entry_not_found} + + %JournalEntry{} = entry -> + entry + |> Ecto.Changeset.change(lat: lat, lon: lon) + |> Repo.update() + end + end + @doc """ Infers floorplan alignment for `stop_level` from anchored child stops and eligible cross-level elevator pathways. @@ -2198,16 +2275,67 @@ defmodule GtfsPlanner.Gtfs do Upsert a station-journal entry by its client-generated `id` (idempotent). On conflict, the mutable fields are replaced (last-write-wins on `body` / `resolved_at`); scoping and authorship are preserved from the original insert. + + A `pin` entry's canonical anchor is its diagram coordinate (`stop_level_id` + + `diagram_x/y`), exactly like a node's `diagram_coordinate`. Sync never sets + `lat`/`lon`: that geographic enrichment is imputed at level-alignment time (see + `save_and_apply_stop_level_alignment/4`) and is therefore omitted from the + `on_conflict` replace list so a later metadata re-sync can't reset an + alignment-imputed lat/lon back to nil. """ def upsert_journal_entry(attrs) do %JournalEntry{} |> JournalEntry.changeset(attrs) |> Repo.insert( - on_conflict: {:replace, [:body, :resolved_at, :target_type, :target_id, :updated_at]}, + on_conflict: + {:replace, + [ + :body, + :resolved_at, + :target_type, + :target_id, + :stop_level_id, + :diagram_x, + :diagram_y, + :updated_at + ]}, conflict_target: :id ) end + @doc """ + Upsert a journal photo by its client-generated `id` (idempotent on retry). On + conflict the mutable metadata is replaced; scoping and the owning entry are + preserved from the original insert. See the companion app's + `specs/api/station-journal.md`. + """ + def upsert_journal_photo(attrs) do + %JournalPhoto{} + |> JournalPhoto.changeset(attrs) + |> Repo.insert( + on_conflict: + {:replace, [:filename, :content_type, :byte_size, :width, :height, :updated_at]}, + conflict_target: :id + ) + end + + @doc """ + All journal photos whose owning entry belongs to the given (org, version, + station), oldest first. Used to nest photos under their entries in the bundle. + """ + def list_journal_photos_for_station(organization_id, gtfs_version_id, station_id) do + from(p in JournalPhoto, + join: e in JournalEntry, + on: e.id == p.journal_entry_id, + where: + e.organization_id == ^organization_id and + e.gtfs_version_id == ^gtfs_version_id and + e.station_id == ^station_id, + order_by: [asc: p.captured_at, asc: p.inserted_at] + ) + |> Repo.all() + end + defp descendant_stop_ids_query(organization_id, gtfs_version_id, station_stop_id) do direct_child_ids = from(s in Stop, diff --git a/lib/gtfs_planner/gtfs/journal_entry.ex b/lib/gtfs_planner/gtfs/journal_entry.ex index 481de8a4..8bb0776c 100644 --- a/lib/gtfs_planner/gtfs/journal_entry.ex +++ b/lib/gtfs_planner/gtfs/journal_entry.ex @@ -6,14 +6,16 @@ defmodule GtfsPlanner.Gtfs.JournalEntry do The `id` is client-generated (stable across offline capture → sync) and is the upsert key. `target_type` is polymorphic over `station` (target_id null), - `node`, and `pathway` (the `pin` target and its diagram coordinate columns are - added by a later migration). + `node`, `pathway`, and `pin` (an arbitrary point on a level: `stop_level_id` + + `diagram_x/y` as the canonical anchor — exactly like a node's diagram + coordinate. `lat/lon` is optional enrichment imputed at level-alignment time, + not at sync, and stays nil on unaligned levels). """ use Ecto.Schema import Ecto.Changeset import GtfsPlanner.ChangesetHelpers - @target_types ~w(station node pathway) + @target_types ~w(station node pathway pin) @type t :: %__MODULE__{ id: Ecto.UUID.t(), @@ -22,6 +24,11 @@ defmodule GtfsPlanner.Gtfs.JournalEntry do station_id: Ecto.UUID.t(), target_type: String.t(), target_id: Ecto.UUID.t() | nil, + stop_level_id: Ecto.UUID.t() | nil, + diagram_x: float() | nil, + diagram_y: float() | nil, + lat: float() | nil, + lon: float() | nil, body: String.t() | nil, author_id: Ecto.UUID.t(), captured_at: DateTime.t(), @@ -36,6 +43,11 @@ defmodule GtfsPlanner.Gtfs.JournalEntry do schema "journal_entries" do field :target_type, :string field :target_id, :binary_id + field :stop_level_id, :binary_id + field :diagram_x, :float + field :diagram_y, :float + field :lat, :float + field :lon, :float field :body, :string field :author_id, :binary_id field :captured_at, :utc_datetime_usec @@ -61,6 +73,11 @@ defmodule GtfsPlanner.Gtfs.JournalEntry do :station_id, :target_type, :target_id, + :stop_level_id, + :diagram_x, + :diagram_y, + :lat, + :lon, :body, :author_id, :captured_at, @@ -77,14 +94,20 @@ defmodule GtfsPlanner.Gtfs.JournalEntry do :captured_at ]) |> validate_inclusion(:target_type, @target_types) - |> validate_target_id() + |> validate_target() |> foreign_key_constraint(:organization_id) |> foreign_key_constraint(:gtfs_version_id) |> foreign_key_constraint(:station_id) + |> foreign_key_constraint(:stop_level_id) end - # A node/pathway entry must name its target; a station entry must not. - defp validate_target_id(changeset) do + # Target-shape rules by `target_type`: + # - node/pathway: `target_id` required (the node/pathway UUID). + # - station: `target_id` must be blank. + # - pin: no `target_id`; `stop_level_id` + `diagram_x/y` required + # (the level + diagram-canvas point is the canonical anchor). + # lat/lon are optional enrichment imputed at alignment time. + defp validate_target(changeset) do case get_field(changeset, :target_type) do type when type in ["node", "pathway"] -> if get_field(changeset, :target_id), @@ -96,8 +119,19 @@ defmodule GtfsPlanner.Gtfs.JournalEntry do do: add_error(changeset, :target_id, "must be blank for station targets"), else: changeset + "pin" -> + changeset + |> validate_pin_target_id() + |> validate_required([:stop_level_id, :diagram_x, :diagram_y]) + _ -> changeset end end + + defp validate_pin_target_id(changeset) do + if get_field(changeset, :target_id), + do: add_error(changeset, :target_id, "must be blank for pin targets"), + else: changeset + end end diff --git a/lib/gtfs_planner/gtfs/journal_photo.ex b/lib/gtfs_planner/gtfs/journal_photo.ex new file mode 100644 index 00000000..159657e5 --- /dev/null +++ b/lib/gtfs_planner/gtfs/journal_photo.ex @@ -0,0 +1,75 @@ +defmodule GtfsPlanner.Gtfs.JournalPhoto do + @moduledoc """ + A photo belonging to a station-journal entry. The binary is stored under + `/uploads/field-captures/...` and served as a static URL (like floorplans); + this row is the metadata. See the companion app's `specs/api/station-journal.md`. + + `id` is client-generated (the upload idempotency key). + """ + use Ecto.Schema + import Ecto.Changeset + import GtfsPlanner.ChangesetHelpers + + @type t :: %__MODULE__{ + id: Ecto.UUID.t(), + organization_id: Ecto.UUID.t(), + gtfs_version_id: Ecto.UUID.t(), + journal_entry_id: Ecto.UUID.t(), + filename: String.t(), + content_type: String.t(), + byte_size: integer() | nil, + width: integer() | nil, + height: integer() | nil, + captured_at: DateTime.t(), + inserted_at: DateTime.t(), + updated_at: DateTime.t() + } + + @primary_key {:id, :binary_id, autogenerate: false} + @foreign_key_type :binary_id + + schema "journal_photos" do + field :filename, :string + field :content_type, :string + field :byte_size, :integer + field :width, :integer + field :height, :integer + field :captured_at, :utc_datetime_usec + + belongs_to :organization, GtfsPlanner.Organizations.Organization, + foreign_key: :organization_id + + belongs_to :gtfs_version, GtfsPlanner.Versions.GtfsVersion + belongs_to :journal_entry, GtfsPlanner.Gtfs.JournalEntry + + timestamps(type: :utc_datetime_usec) + end + + def changeset(photo, attrs) do + photo + |> cast(attrs, [ + :id, + :organization_id, + :gtfs_version_id, + :journal_entry_id, + :filename, + :content_type, + :byte_size, + :width, + :height, + :captured_at + ]) + |> trim_string_fields() + |> validate_required([ + :id, + :organization_id, + :gtfs_version_id, + :journal_entry_id, + :filename, + :content_type, + :captured_at + ]) + |> foreign_key_constraint(:journal_entry_id) + |> foreign_key_constraint(:organization_id) + end +end diff --git a/lib/gtfs_planner_web/api/v1/journal_photo_controller.ex b/lib/gtfs_planner_web/api/v1/journal_photo_controller.ex new file mode 100644 index 00000000..abf92d1c --- /dev/null +++ b/lib/gtfs_planner_web/api/v1/journal_photo_controller.ex @@ -0,0 +1,221 @@ +defmodule GtfsPlannerWeb.Api.V1.JournalPhotoController do + @moduledoc """ + Multipart upload of a station-journal photo. The binary is stored under + `/uploads/field-captures///` and served as a static + absolute URL (mirroring the floorplan image flow); this endpoint creates the + metadata row and returns the URL. See the companion app's + `specs/api/station-journal.md`. + + Idempotent on the client-generated photo `id` — re-sending the same file is + safe to retry on timeout. + """ + use GtfsPlannerWeb, :controller + + alias GtfsPlanner.Gtfs + alias GtfsPlanner.Gtfs.Extensions.PathSafety + alias GtfsPlanner.Gtfs.JournalEntry + alias GtfsPlanner.Repo + alias GtfsPlannerWeb.Endpoint + + # 25 MB — a generous cap for a single field photo (modern phone cameras + # produce ~5–15 MB JPEGs); large enough to never reject a real capture, small + # enough to bound a single multipart request. + @max_byte_size 25 * 1024 * 1024 + + # content_type => storage extension. Anything else is rejected (422). + @allowed_content_types %{ + "image/jpeg" => "jpg", + "image/png" => "png" + } + + @doc "POST /api/v1/versions/:version_id/stations/:station_id/journal-photos" + def create(conn, %{"version_id" => version_id, "station_id" => station_id} = params) do + org_id = conn.assigns[:current_organization_id] + + with {:ok, %Plug.Upload{} = upload} <- fetch_upload(params), + {:ok, metadata} <- fetch_metadata(params), + {:ok, content_type, ext} <- resolve_content_type(metadata, upload), + :ok <- check_size(upload), + %{} = station <- Gtfs.get_stop(station_id), + true <- valid_station?(station, org_id, version_id), + %JournalEntry{} = entry <- + get_entry(metadata["journal_entry_id"], org_id, version_id, station_id), + {:ok, filename, byte_size} <- + store_file(upload, metadata["id"], ext, org_id, station.stop_id), + {:ok, photo} <- + Gtfs.upsert_journal_photo(%{ + "id" => metadata["id"], + "organization_id" => org_id, + "gtfs_version_id" => version_id, + "journal_entry_id" => entry.id, + "filename" => filename, + "content_type" => content_type, + "byte_size" => byte_size, + "captured_at" => metadata["captured_at"] + }) do + conn + |> put_status(201) + |> json(%{data: %{photo: serialize_photo(photo, org_id, station.stop_id)}}) + else + :missing_file -> + error(conn, 422, "validation_error", "A 'file' part is required.") + + :missing_metadata -> + error(conn, 422, "validation_error", "A 'metadata' part is required.") + + :invalid_metadata -> + error(conn, 422, "validation_error", "Invalid 'metadata' JSON.") + + :unsupported_content_type -> + error( + conn, + 422, + "validation_error", + "Unsupported content type. Allowed: image/jpeg, image/png." + ) + + :over_size_cap -> + error(conn, 413, "payload_too_large", "Photo exceeds the maximum allowed size.") + + :storage_failed -> + error(conn, 422, "validation_error", "Failed to store photo.") + + nil -> + error(conn, 404, "not_found", "Not found.") + + false -> + error(conn, 404, "not_found", "Not found.") + + {:error, _changeset} -> + error(conn, 422, "validation_error", "Failed to save photo.") + end + end + + defp fetch_upload(params) do + case params["file"] do + %Plug.Upload{} = upload -> {:ok, upload} + _ -> :missing_file + end + end + + defp fetch_metadata(params) do + case params["metadata"] do + json when is_binary(json) -> + case Jason.decode(json) do + {:ok, %{} = metadata} -> {:ok, metadata} + _ -> :invalid_metadata + end + + %{} = metadata -> + {:ok, metadata} + + _ -> + :missing_metadata + end + end + + # Prefer the client-declared content_type from metadata; fall back to the + # upload part's content_type. Reject anything not in the allow-list. + defp resolve_content_type(metadata, %Plug.Upload{content_type: upload_ct}) do + content_type = metadata["content_type"] || upload_ct + + case Map.fetch(@allowed_content_types, content_type) do + {:ok, ext} -> {:ok, content_type, ext} + :error -> :unsupported_content_type + end + end + + defp check_size(%Plug.Upload{path: path}) do + case File.stat(path) do + {:ok, %{size: size}} when size <= @max_byte_size -> :ok + {:ok, _} -> :over_size_cap + {:error, _} -> :storage_failed + end + end + + defp get_entry(nil, _org_id, _version_id, _station_id), do: nil + + defp get_entry(entry_id, org_id, version_id, station_id) when is_binary(entry_id) do + case Ecto.UUID.cast(entry_id) do + {:ok, _} -> + Repo.get_by(JournalEntry, + id: entry_id, + organization_id: org_id, + gtfs_version_id: version_id, + station_id: station_id + ) + + :error -> + nil + end + end + + defp get_entry(_entry_id, _org_id, _version_id, _station_id), do: nil + + # Stores the upload under /field-captures///, + # mirroring the floorplan file-copy pattern (PathSafety + ensure_within_root + + # mkdir_p + cp). Returns {:ok, filename, byte_size} or :storage_failed. + defp store_file(%Plug.Upload{path: src_path}, photo_id, ext, org_id, station_stop_id) do + uploads_base = Application.get_env(:gtfs_planner, :uploads_path) + station_dir = PathSafety.stop_storage_dir(station_stop_id) + storage_filename = "#{photo_id}.#{ext}" + + with true <- is_binary(station_dir), + true <- PathSafety.safe_path_component?(storage_filename), + captures_root <- Path.join([uploads_base, "field-captures", to_string(org_id)]), + dest_dir <- Path.join(captures_root, station_dir), + dest_path <- Path.join(dest_dir, storage_filename), + :ok <- PathSafety.ensure_within_root(captures_root, dest_dir), + :ok <- PathSafety.ensure_within_root(captures_root, dest_path), + :ok <- File.mkdir_p(dest_dir), + :ok <- File.cp(src_path, dest_path), + {:ok, %{size: byte_size}} <- File.stat(dest_path) do + {:ok, storage_filename, byte_size} + else + _ -> :storage_failed + end + end + + defp serialize_photo(photo, org_id, station_stop_id) do + %{ + id: photo.id, + journal_entry_id: photo.journal_entry_id, + url: field_capture_url(org_id, station_stop_id, photo.filename), + content_type: photo.content_type, + width: photo.width, + height: photo.height, + captured_at: photo.captured_at + } + end + + # Field-capture photos are served as static files under /uploads (like + # floorplans), not via an /api/v1 endpoint. Returns an absolute URL. + defp field_capture_url(org_id, station_stop_id, filename) do + case PathSafety.stop_storage_dir(station_stop_id) do + dir when is_binary(dir) -> + encoded_filename = URI.encode(filename, &URI.char_unreserved?/1) + + "#{Endpoint.url()}/uploads/field-captures/#{org_id}/#{dir}/#{encoded_filename}" + + _ -> + nil + end + end + + # Repeated variables enforce equality: matches only when the station's org and + # version equal the request's. A nil station falls through to false. + defp valid_station?( + %{organization_id: org_id, gtfs_version_id: version_id}, + org_id, + version_id + ), + do: true + + defp valid_station?(_station, _org_id, _version_id), do: false + + defp error(conn, status, code, message) do + conn + |> put_status(status) + |> json(%{error: %{code: code, message: message}}) + end +end diff --git a/lib/gtfs_planner_web/api/v1/station_controller.ex b/lib/gtfs_planner_web/api/v1/station_controller.ex index 3ed1e2c6..5b8aeb02 100644 --- a/lib/gtfs_planner_web/api/v1/station_controller.ex +++ b/lib/gtfs_planner_web/api/v1/station_controller.ex @@ -80,13 +80,29 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do levels = Gtfs.list_levels_for_station(org_id, version_id, station.id) pathways = Gtfs.list_pathways_for_station(org_id, version_id, station.id) + # Photos, grouped by their owning entry id and pre-serialized, so each + # journal entry can nest its photos[] (empty list when none). + photos_by_entry = + org_id + |> Gtfs.list_journal_photos_for_station(version_id, station.id) + |> Enum.group_by(& &1.journal_entry_id, &serialize_photo(&1, org_id, station.stop_id)) + + ser_entry = &serialize_journal_entry(&1, photos_by_entry) + # Station journal: group entries by target so each serializes onto its - # node/pathway (and station-targeted ones go top-level). Pin-targeted - # entries land on levels[] once the pin migration is in. + # node/pathway (station-targeted ones go top-level; pin-targeted ones land + # on levels[], keyed by stop_level_id). + all_entries = Gtfs.list_journal_entries_for_station(org_id, version_id, station.id) + entries_by_target = - org_id - |> Gtfs.list_journal_entries_for_station(version_id, station.id) - |> Enum.group_by(&{&1.target_type, &1.target_id}, &serialize_journal_entry/1) + all_entries + |> Enum.reject(&(&1.target_type == "pin")) + |> Enum.group_by(&{&1.target_type, &1.target_id}, ser_entry) + + pin_entries_by_level = + all_entries + |> Enum.filter(&(&1.target_type == "pin")) + |> Enum.group_by(& &1.stop_level_id, &serialize_pin_entry(&1, photos_by_entry)) {station_lat, station_lon} = serialize_coordinates(station.stop_lat, station.stop_lon) @@ -104,7 +120,11 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do lat: station_lat, lon: station_lon }, - levels: Enum.map(levels, &serialize_level(&1, org_id, station.stop_id)), + levels: + Enum.map( + levels, + &serialize_level(&1, org_id, station.stop_id, pin_entries_by_level) + ), stops: Enum.map( child_stops, @@ -134,17 +154,41 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do end end - defp serialize_level(%{level: level} = level_data, org_id, station_stop_id) do - serialize_level(level, Map.get(level_data, :stop_level), org_id, station_stop_id) + defp serialize_level( + %{level: level} = level_data, + org_id, + station_stop_id, + pin_entries_by_level + ) do + serialize_level( + level, + Map.get(level_data, :stop_level), + org_id, + station_stop_id, + pin_entries_by_level + ) end - defp serialize_level(%{id: _} = level, stop_level, org_id, station_stop_id) do + defp serialize_level( + %{id: _} = level, + stop_level, + org_id, + station_stop_id, + pin_entries_by_level + ) do + pin_entries = + case stop_level do + %StopLevel{id: stop_level_id} -> Map.get(pin_entries_by_level, stop_level_id, []) + _ -> [] + end + %{ id: level.id, level_id: level.level_id, level_index: level.level_index, level_name: level.level_name, - floorplan: serialize_floorplan(stop_level, org_id, station_stop_id) + floorplan: serialize_floorplan(stop_level, org_id, station_stop_id), + journal_entries: pin_entries } end @@ -272,9 +316,8 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do } end - # `photos: []` is always present (forward-compatible); photos are populated - # once the journal-photos table + upload land. See specs/api/station-journal.md. - defp serialize_journal_entry(entry) do + # Station/node/pathway entries: each nests its photos[] (empty when none). + defp serialize_journal_entry(entry, photos_by_entry) do %{ id: entry.id, target_type: entry.target_type, @@ -283,7 +326,50 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do author_id: entry.author_id, captured_at: entry.captured_at, resolved_at: entry.resolved_at, - photos: [] + photos: Map.get(photos_by_entry, entry.id, []) } end + + # Pin entries carry the diagram-canvas point + imputed lat/lon (mirroring how + # stops[] carry node coords), in addition to the common entry fields. + defp serialize_pin_entry(entry, photos_by_entry) do + %{ + id: entry.id, + target_type: entry.target_type, + stop_level_id: entry.stop_level_id, + diagram_coordinate: %{x: entry.diagram_x, y: entry.diagram_y}, + lat: entry.lat, + lon: entry.lon, + body: entry.body, + author_id: entry.author_id, + captured_at: entry.captured_at, + resolved_at: entry.resolved_at, + photos: Map.get(photos_by_entry, entry.id, []) + } + end + + defp serialize_photo(photo, org_id, station_stop_id) do + %{ + id: photo.id, + url: field_capture_url(org_id, station_stop_id, photo.filename), + content_type: photo.content_type, + width: photo.width, + height: photo.height, + captured_at: photo.captured_at + } + end + + # Field-capture photos are served as static files under /uploads (like + # floorplans), not via an /api/v1 endpoint. Returns an absolute URL. + defp field_capture_url(org_id, station_stop_id, filename) do + case PathSafety.stop_storage_dir(station_stop_id) do + dir when is_binary(dir) -> + encoded_filename = URI.encode(filename, &URI.char_unreserved?/1) + + "#{Endpoint.url()}/uploads/field-captures/#{org_id}/#{dir}/#{encoded_filename}" + + _ -> + nil + end + end end diff --git a/lib/gtfs_planner_web/api/v1/sync_controller.ex b/lib/gtfs_planner_web/api/v1/sync_controller.ex index b8ca0902..516d9d63 100644 --- a/lib/gtfs_planner_web/api/v1/sync_controller.ex +++ b/lib/gtfs_planner_web/api/v1/sync_controller.ex @@ -172,6 +172,9 @@ defmodule GtfsPlannerWeb.Api.V1.SyncController do "author_id" => author_id, "target_type" => update["target_type"], "target_id" => update["target_id"], + "stop_level_id" => update["stop_level_id"], + "diagram_x" => update["diagram_x"], + "diagram_y" => update["diagram_y"], "body" => update["body"], "captured_at" => update["captured_at"], "resolved_at" => update["resolved_at"] @@ -195,7 +198,9 @@ defmodule GtfsPlannerWeb.Api.V1.SyncController do } end end) - |> then(fn acc -> %{acc | errors: Enum.reverse(acc.errors)} end) + |> then(fn acc -> + %{acc | errors: Enum.reverse(acc.errors)} + end) else %{ synced: 0, diff --git a/lib/gtfs_planner_web/router.ex b/lib/gtfs_planner_web/router.ex index 8ad972c0..5e788264 100644 --- a/lib/gtfs_planner_web/router.ex +++ b/lib/gtfs_planner_web/router.ex @@ -180,6 +180,10 @@ defmodule GtfsPlannerWeb.Router do get "/versions/:version_id/stations", StationController, :index get "/versions/:version_id/stations/:station_id/bundle", StationController, :bundle post "/versions/:version_id/stations/:station_id/sync", SyncController, :create + + post "/versions/:version_id/stations/:station_id/journal-photos", + JournalPhotoController, + :create end # Enable LiveDashboard and Swoosh mailbox preview in development diff --git a/priv/repo/migrations/20260623034340_create_journal_photos.exs b/priv/repo/migrations/20260623034340_create_journal_photos.exs new file mode 100644 index 00000000..0b2858ed --- /dev/null +++ b/priv/repo/migrations/20260623034340_create_journal_photos.exs @@ -0,0 +1,34 @@ +defmodule GtfsPlanner.Repo.Migrations.CreateJournalPhotos do + use Ecto.Migration + + def change do + create table(:journal_photos, primary_key: false) do + # Client-generated UUID — the upload idempotency key. + add :id, :binary_id, primary_key: true + + add :organization_id, + references(:organizations, type: :binary_id, on_delete: :delete_all), + null: false + + add :gtfs_version_id, + references(:gtfs_versions, type: :binary_id, on_delete: :delete_all), + null: false + + add :journal_entry_id, + references(:journal_entries, type: :binary_id, on_delete: :delete_all), + null: false + + add :filename, :string, null: false + add :content_type, :string, null: false + add :byte_size, :integer + add :width, :integer + add :height, :integer + add :captured_at, :utc_datetime_usec, null: false + + timestamps(type: :utc_datetime_usec) + end + + create index(:journal_photos, [:journal_entry_id]) + create index(:journal_photos, [:organization_id, :gtfs_version_id]) + end +end diff --git a/priv/repo/migrations/20260623034341_add_pin_target_to_journal_entries.exs b/priv/repo/migrations/20260623034341_add_pin_target_to_journal_entries.exs new file mode 100644 index 00000000..9f9bbcc4 --- /dev/null +++ b/priv/repo/migrations/20260623034341_add_pin_target_to_journal_entries.exs @@ -0,0 +1,19 @@ +defmodule GtfsPlanner.Repo.Migrations.AddPinTargetToJournalEntries do + use Ecto.Migration + + def change do + # A "pin" entry is anchored to an arbitrary point on a level (no separate + # pin entity): its canonical anchor is the diagram coordinate (like a node's), + # and lat/lon is optional enrichment imputed at level-alignment time (null on + # unaligned levels). + alter table(:journal_entries) do + add :stop_level_id, references(:stop_levels, type: :binary_id, on_delete: :delete_all) + add :diagram_x, :float + add :diagram_y, :float + add :lat, :float + add :lon, :float + end + + create index(:journal_entries, [:stop_level_id]) + end +end diff --git a/test/gtfs_planner/gtfs_test.exs b/test/gtfs_planner/gtfs_test.exs index 09d75517..7f2f8f82 100644 --- a/test/gtfs_planner/gtfs_test.exs +++ b/test/gtfs_planner/gtfs_test.exs @@ -3128,6 +3128,54 @@ defmodule GtfsPlanner.GtfsTest do assert_in_delta Decimal.to_float(reloaded_child_stop.stop_lon), -74.0060, 1.0e-9 end + test "imputes lat/lon for pin journal entries on the level alongside nodes", %{ + organization: org, + gtfs_version: version, + station: station, + stop_level: stop_level + } do + author = user_fixture(%{email: "pin-align@example.com"}) + + pin_id = Ecto.UUID.generate() + + {:ok, _pin} = + Gtfs.upsert_journal_entry(%{ + "id" => pin_id, + "organization_id" => org.id, + "gtfs_version_id" => version.id, + "station_id" => station.id, + "author_id" => author.id, + "target_type" => "pin", + "stop_level_id" => stop_level.id, + # Painted image center in width-normalized units for 1000x800. + "diagram_x" => 50.0, + "diagram_y" => 40.0, + "captured_at" => "2026-06-20T10:00:00Z" + }) + + # Null before alignment — the canonical anchor is the diagram coordinate. + pre = Repo.get!(GtfsPlanner.Gtfs.JournalEntry, pin_id) + assert is_nil(pre.lat) + assert is_nil(pre.lon) + + attrs = %{ + floorplan_center_lat: 40.7128, + floorplan_center_lon: -74.0060, + floorplan_scale_mpp: 0.25, + floorplan_rotation_deg: 0.0 + } + + assert {:ok, _} = + Gtfs.save_and_apply_stop_level_alignment(stop_level.id, attrs, 1000, 800) + + post = Repo.get!(GtfsPlanner.Gtfs.JournalEntry, pin_id) + assert_in_delta post.lat, 40.7128, 1.0e-9 + assert_in_delta post.lon, -74.0060, 1.0e-9 + # The canonical diagram coordinate is untouched. + assert post.diagram_x == 50.0 + assert post.diagram_y == 40.0 + end + test "returns :not_found when active stop level does not exist" do assert {:error, :not_found} = Gtfs.save_and_apply_stop_level_alignment( diff --git a/test/gtfs_planner_web/api/v1/journal_photo_controller_test.exs b/test/gtfs_planner_web/api/v1/journal_photo_controller_test.exs new file mode 100644 index 00000000..494fbcff --- /dev/null +++ b/test/gtfs_planner_web/api/v1/journal_photo_controller_test.exs @@ -0,0 +1,263 @@ +defmodule GtfsPlannerWeb.Api.V1.JournalPhotoControllerTest do + # async: false — uploads write under the global `:uploads_path` app env, which + # another async test (UploadsPlugTest) mutates via Application.put_env. Running + # serially keeps the storage directory stable for the duration of each test. + use GtfsPlannerWeb.ConnCase, async: false + + import GtfsPlanner.AccountsFixtures + import GtfsPlanner.OrganizationsFixtures + import GtfsPlanner.VersionsFixtures + import GtfsPlanner.GtfsFixtures + + alias GtfsPlanner.Accounts + alias GtfsPlanner.Gtfs + alias GtfsPlanner.Gtfs.JournalPhoto + alias GtfsPlanner.Repo + + # Pin the uploads directory to a unique path per test run so file writes are + # isolated from other modules that mutate the global :uploads_path app env. + setup do + original = Application.get_env(:gtfs_planner, :uploads_path) + + path = + Path.join(System.tmp_dir!(), "journal_photo_test_#{System.unique_integer([:positive])}") + + Application.put_env(:gtfs_planner, :uploads_path, path) + + on_exit(fn -> + File.rm_rf!(path) + Application.put_env(:gtfs_planner, :uploads_path, original) + end) + + :ok + end + + # --------------------------------------------------------------------------- + # Helpers + # --------------------------------------------------------------------------- + + defp setup_user_with_org(_context) do + user = user_fixture() + org = organization_fixture() + + {:ok, _membership} = + Accounts.create_user_org_membership(%{ + user_id: user.id, + organization_id: org.id, + roles: ["pathways_studio_editor"] + }) + + %{user: user, org: org} + end + + defp authed_conn(conn, user) do + token = Accounts.generate_api_session_token(user) + + conn + |> put_req_header("accept", "application/json") + |> put_req_header("authorization", "Bearer #{token}") + end + + defp build_station(org_id, version_id) do + stop_fixture(org_id, version_id, %{location_type: 1, parent_station: nil}) + end + + defp journal_entry_fixture(org, version, station, author, attrs \\ %{}) do + base = %{ + "id" => Ecto.UUID.generate(), + "organization_id" => org.id, + "gtfs_version_id" => version.id, + "station_id" => station.id, + "author_id" => author.id, + "target_type" => "station", + "body" => "note", + "captured_at" => "2026-06-20T10:00:00Z" + } + + {:ok, entry} = Gtfs.upsert_journal_entry(Map.merge(base, attrs)) + entry + end + + # Builds a %Plug.Upload{} backed by a temp file of the given bytes. + defp upload_fixture(content, opts \\ []) do + content_type = Keyword.get(opts, :content_type, "image/jpeg") + filename = Keyword.get(opts, :filename, "photo.jpg") + path = Path.join(System.tmp_dir!(), "upload_#{Ecto.UUID.generate()}") + File.write!(path, content) + %Plug.Upload{path: path, filename: filename, content_type: content_type} + end + + defp photos_url(version_id, station_id), + do: "/api/v1/versions/#{version_id}/stations/#{station_id}/journal-photos" + + # --------------------------------------------------------------------------- + # POST .../journal-photos + # --------------------------------------------------------------------------- + + describe "create/2" do + setup [:setup_user_with_org] + + test "uploads a photo, creates the row + file, returns a static url", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + station = build_station(org.id, version.id) + entry = journal_entry_fixture(org, version, station, user) + + photo_id = Ecto.UUID.generate() + upload = upload_fixture("jpegbytes") + + payload = %{ + "file" => upload, + "metadata" => + Jason.encode!(%{ + "id" => photo_id, + "journal_entry_id" => entry.id, + "captured_at" => "2026-06-20T10:05:00Z", + "content_type" => "image/jpeg" + }) + } + + conn = conn |> authed_conn(user) |> post(photos_url(version.id, station.id), payload) + + assert %{"data" => %{"photo" => photo}} = json_response(conn, 201) + assert photo["id"] == photo_id + assert photo["journal_entry_id"] == entry.id + assert photo["content_type"] == "image/jpeg" + assert String.contains?(photo["url"], "/uploads/field-captures/#{org.id}/") + assert String.ends_with?(photo["url"], "#{photo_id}.jpg") + + row = Repo.get!(JournalPhoto, photo_id) + assert row.journal_entry_id == entry.id + assert row.filename == "#{photo_id}.jpg" + assert row.byte_size == byte_size("jpegbytes") + + uploads_base = Application.get_env(:gtfs_planner, :uploads_path) + + dest = + Path.join([uploads_base, "field-captures", org.id, station.stop_id, "#{photo_id}.jpg"]) + + assert File.exists?(dest) + end + + test "is idempotent on id (re-upload returns the same record)", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + station = build_station(org.id, version.id) + entry = journal_entry_fixture(org, version, station, user) + photo_id = Ecto.UUID.generate() + + payload = fn -> + %{ + "file" => upload_fixture("bytes"), + "metadata" => + Jason.encode!(%{ + "id" => photo_id, + "journal_entry_id" => entry.id, + "captured_at" => "2026-06-20T10:05:00Z", + "content_type" => "image/jpeg" + }) + } + end + + conn |> authed_conn(user) |> post(photos_url(version.id, station.id), payload.()) + + conn2 = + build_conn() |> authed_conn(user) |> post(photos_url(version.id, station.id), payload.()) + + assert %{"data" => %{"photo" => photo}} = json_response(conn2, 201) + assert photo["id"] == photo_id + assert Repo.aggregate(JournalPhoto, :count, :id) == 1 + end + + test "returns 404 for an entry not in the org", %{conn: conn, user: user, org: org} do + version = gtfs_version_fixture(org.id) + station = build_station(org.id, version.id) + + other_org = organization_fixture() + other_version = gtfs_version_fixture(other_org.id) + other_station = build_station(other_org.id, other_version.id) + other_user = user_fixture() + other_entry = journal_entry_fixture(other_org, other_version, other_station, other_user) + + payload = %{ + "file" => upload_fixture("bytes"), + "metadata" => + Jason.encode!(%{ + "id" => Ecto.UUID.generate(), + "journal_entry_id" => other_entry.id, + "captured_at" => "2026-06-20T10:05:00Z", + "content_type" => "image/jpeg" + }) + } + + conn = conn |> authed_conn(user) |> post(photos_url(version.id, station.id), payload) + + assert %{"error" => %{"code" => "not_found"}} = json_response(conn, 404) + assert Repo.aggregate(JournalPhoto, :count, :id) == 0 + end + + test "returns 422 for an unsupported content type", %{conn: conn, user: user, org: org} do + version = gtfs_version_fixture(org.id) + station = build_station(org.id, version.id) + entry = journal_entry_fixture(org, version, station, user) + + payload = %{ + "file" => upload_fixture("gifbytes", content_type: "image/gif"), + "metadata" => + Jason.encode!(%{ + "id" => Ecto.UUID.generate(), + "journal_entry_id" => entry.id, + "captured_at" => "2026-06-20T10:05:00Z", + "content_type" => "image/gif" + }) + } + + conn = conn |> authed_conn(user) |> post(photos_url(version.id, station.id), payload) + + assert %{"error" => %{"code" => "validation_error"}} = json_response(conn, 422) + assert Repo.aggregate(JournalPhoto, :count, :id) == 0 + end + + test "an uploaded photo nests under its entry in the bundle", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + station = build_station(org.id, version.id) + entry = journal_entry_fixture(org, version, station, user) + photo_id = Ecto.UUID.generate() + + upload_payload = %{ + "file" => upload_fixture("bytes"), + "metadata" => + Jason.encode!(%{ + "id" => photo_id, + "journal_entry_id" => entry.id, + "captured_at" => "2026-06-20T10:05:00Z", + "content_type" => "image/jpeg" + }) + } + + conn |> authed_conn(user) |> post(photos_url(version.id, station.id), upload_payload) + + bundle_conn = + build_conn() + |> authed_conn(user) + |> get("/api/v1/versions/#{version.id}/stations/#{station.id}/bundle") + + assert %{"data" => %{"journal_entries" => [bundle_entry]}} = json_response(bundle_conn, 200) + assert bundle_entry["id"] == entry.id + assert [photo] = bundle_entry["photos"] + assert photo["id"] == photo_id + assert photo["content_type"] == "image/jpeg" + assert String.ends_with?(photo["url"], "#{photo_id}.jpg") + end + end +end diff --git a/test/gtfs_planner_web/api/v1/sync_controller_test.exs b/test/gtfs_planner_web/api/v1/sync_controller_test.exs index a0d99ddb..134bc47a 100644 --- a/test/gtfs_planner_web/api/v1/sync_controller_test.exs +++ b/test/gtfs_planner_web/api/v1/sync_controller_test.exs @@ -7,6 +7,7 @@ defmodule GtfsPlannerWeb.Api.V1.SyncControllerTest do import GtfsPlanner.GtfsFixtures alias GtfsPlanner.Accounts + alias GtfsPlanner.Gtfs alias GtfsPlanner.Repo alias GtfsPlanner.Gtfs.JournalEntry alias GtfsPlanner.Gtfs.Pathway @@ -535,4 +536,200 @@ defmodule GtfsPlannerWeb.Api.V1.SyncControllerTest do assert Repo.aggregate(JournalEntry, :count, :id) == 0 end end + + describe "create/2 — pin journal entries" do + setup [:setup_user_with_org] + + # A stop_level on the station, optionally aligned + sized. + defp build_stop_level(org_id, version_id, station, opts) do + level = level_fixture(org_id, version_id) + + {:ok, stop_level} = + Gtfs.create_stop_level(%{ + organization_id: org_id, + gtfs_version_id: version_id, + stop_id: station.id, + level_id: level.id + }) + + if Keyword.get(opts, :aligned, false) do + {:ok, stop_level} = + Gtfs.update_stop_level_alignment(stop_level, %{ + floorplan_center_lat: 40.7128, + floorplan_center_lon: -74.0060, + floorplan_scale_mpp: 0.25, + floorplan_rotation_deg: 0.0 + }) + + stop_level + else + stop_level + end + end + + # Posts a single pin entry through sync. Returns the decoded response data. + defp sync_pin(_conn, user, version, station, pin_id, stop_level, extra \\ %{}) do + entry = + Map.merge( + %{ + "id" => pin_id, + "target_type" => "pin", + "stop_level_id" => stop_level.id, + # Painted image center in width-normalized units for 1000x800. + "diagram_x" => 50.0, + "diagram_y" => 40.0, + "captured_at" => "2026-06-20T10:00:00Z" + }, + extra + ) + + conn = + build_conn() + |> authed_conn(user) + |> post(sync_url(version.id, station.id), %{ + "pathways" => [], + "journal_entries" => [entry] + }) + + assert %{"data" => data} = json_response(conn, 200) + data + end + + defp bundle_pin_entry(user, version, station, pin_id) do + bundle_conn = + build_conn() + |> authed_conn(user) + |> get("/api/v1/versions/#{version.id}/stations/#{station.id}/bundle") + + assert %{"data" => %{"levels" => levels}} = json_response(bundle_conn, 200) + + levels + |> Enum.flat_map(& &1["journal_entries"]) + |> Enum.find(&(&1["id"] == pin_id)) + end + + test "syncs a pin storing diagram_x/y with null lat/lon (no sync-time imputation)", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + %{station: station} = build_station_with_pathway(org.id, version.id) + stop_level = build_stop_level(org.id, version.id, station, aligned: true) + + pin_id = Ecto.UUID.generate() + + data = sync_pin(conn, user, version, station, pin_id, stop_level) + + assert data["journal_synced_count"] == 1 + # Sync no longer imputes pin coords, so there is no journal_entries echo. + refute Map.has_key?(data, "journal_entries") + + entry = Repo.get!(JournalEntry, pin_id) + assert entry.target_type == "pin" + assert entry.stop_level_id == stop_level.id + assert entry.diagram_x == 50.0 + assert entry.diagram_y == 40.0 + assert is_nil(entry.lat) + assert is_nil(entry.lon) + + # Bundle serves the canonical diagram coordinate; lat/lon still null. + pin_entry = bundle_pin_entry(user, version, station, pin_id) + assert pin_entry["target_type"] == "pin" + assert pin_entry["diagram_coordinate"] == %{"x" => 50.0, "y" => 40.0} + assert is_nil(pin_entry["lat"]) + assert is_nil(pin_entry["lon"]) + assert pin_entry["photos"] == [] + end + + test "imputes pin lat/lon at alignment time, then bundle serves them", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + %{station: station} = build_station_with_pathway(org.id, version.id) + # Unaligned at sync time, so the pin lands with null lat/lon. + stop_level = build_stop_level(org.id, version.id, station, aligned: false) + + pin_id = Ecto.UUID.generate() + sync_pin(conn, user, version, station, pin_id, stop_level) + + assert is_nil(Repo.get!(JournalEntry, pin_id).lat) + + # Aligning the level imputes lat/lon for its pins alongside nodes. + assert {:ok, _} = + Gtfs.save_and_apply_stop_level_alignment( + stop_level.id, + %{ + floorplan_center_lat: 40.7128, + floorplan_center_lon: -74.0060, + floorplan_scale_mpp: 0.25, + floorplan_rotation_deg: 0.0 + }, + 1000, + 800 + ) + + entry = Repo.get!(JournalEntry, pin_id) + assert_in_delta entry.lat, 40.7128, 1.0e-6 + assert_in_delta entry.lon, -74.0060, 1.0e-6 + + pin_entry = bundle_pin_entry(user, version, station, pin_id) + assert pin_entry["diagram_coordinate"] == %{"x" => 50.0, "y" => 40.0} + assert_in_delta pin_entry["lat"], 40.7128, 1.0e-6 + assert_in_delta pin_entry["lon"], -74.0060, 1.0e-6 + end + + test "pin lat/lon stay null on an unaligned level", %{conn: conn, user: user, org: org} do + version = gtfs_version_fixture(org.id) + %{station: station} = build_station_with_pathway(org.id, version.id) + stop_level = build_stop_level(org.id, version.id, station, aligned: false) + + pin_id = Ecto.UUID.generate() + sync_pin(conn, user, version, station, pin_id, stop_level) + + entry = Repo.get!(JournalEntry, pin_id) + assert is_nil(entry.lat) + assert is_nil(entry.lon) + end + + test "re-syncing a pin after alignment does not reset imputed lat/lon", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + %{station: station} = build_station_with_pathway(org.id, version.id) + stop_level = build_stop_level(org.id, version.id, station, aligned: false) + + pin_id = Ecto.UUID.generate() + sync_pin(conn, user, version, station, pin_id, stop_level) + + assert {:ok, _} = + Gtfs.save_and_apply_stop_level_alignment( + stop_level.id, + %{ + floorplan_center_lat: 40.7128, + floorplan_center_lon: -74.0060, + floorplan_scale_mpp: 0.25, + floorplan_rotation_deg: 0.0 + }, + 1000, + 800 + ) + + imputed = Repo.get!(JournalEntry, pin_id) + assert_in_delta imputed.lat, 40.7128, 1.0e-6 + + # A later metadata re-sync (e.g. edited body) must preserve server-imputed + # lat/lon — sync never carries lat/lon and must not clobber them to null. + sync_pin(conn, user, version, station, pin_id, stop_level, %{"body" => "edited"}) + + re_synced = Repo.get!(JournalEntry, pin_id) + assert re_synced.body == "edited" + assert_in_delta re_synced.lat, imputed.lat, 1.0e-9 + assert_in_delta re_synced.lon, imputed.lon, 1.0e-9 + end + end end From cff7afe2448fc5cc0f1869e4bba8c2899297c701 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Tue, 23 Jun 2026 09:41:27 -0400 Subject: [PATCH 4/6] feat(api): expose stop_level_id on bundle levels for pin anchoring The companion needs a level's StopLevel UUID to anchor a newly-dropped pin journal entry to that level. serialize_level emitted only the Level id; pin entries echo stop_level_id back, but a fresh drop needs it from the level. Add stop_level_id (null when the level has no stop_level association). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UyezV5kznW5rbRFVsoXWvh --- lib/gtfs_planner_web/api/v1/station_controller.ex | 12 +++++++++--- .../api/v1/station_controller_test.exs | 8 ++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/gtfs_planner_web/api/v1/station_controller.ex b/lib/gtfs_planner_web/api/v1/station_controller.ex index 5b8aeb02..3aa6aba2 100644 --- a/lib/gtfs_planner_web/api/v1/station_controller.ex +++ b/lib/gtfs_planner_web/api/v1/station_controller.ex @@ -176,17 +176,23 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do station_stop_id, pin_entries_by_level ) do - pin_entries = + stop_level_id = case stop_level do - %StopLevel{id: stop_level_id} -> Map.get(pin_entries_by_level, stop_level_id, []) - _ -> [] + %StopLevel{id: id} -> id + _ -> nil end + pin_entries = Map.get(pin_entries_by_level, stop_level_id, []) + %{ id: level.id, level_id: level.level_id, level_index: level.level_index, level_name: level.level_name, + # The station↔level join UUID — the companion needs it to anchor a new + # pin entry to this level (pin entries echo it back, but a fresh drop needs + # it from the level). Null when the level has no stop_level association. + stop_level_id: stop_level_id, floorplan: serialize_floorplan(stop_level, org_id, station_stop_id), journal_entries: pin_entries } diff --git a/test/gtfs_planner_web/api/v1/station_controller_test.exs b/test/gtfs_planner_web/api/v1/station_controller_test.exs index 3ede0e02..135de65b 100644 --- a/test/gtfs_planner_web/api/v1/station_controller_test.exs +++ b/test/gtfs_planner_web/api/v1/station_controller_test.exs @@ -417,7 +417,7 @@ defmodule GtfsPlannerWeb.Api.V1.StationControllerTest do version = gtfs_version_fixture(org.id) %{station: station, level: level} = build_station_data(org.id, version.id) - {:ok, _stop_level} = + {:ok, stop_level} = %StopLevel{ stop_id: station.id, level_id: level.id, @@ -437,7 +437,11 @@ defmodule GtfsPlannerWeb.Api.V1.StationControllerTest do |> get("/api/v1/versions/#{version.id}/stations/#{station.id}/bundle") assert %{"data" => data} = json_response(conn, 200) - floorplan = Enum.find(data["levels"], &(&1["id"] == level.id))["floorplan"] + level_json = Enum.find(data["levels"], &(&1["id"] == level.id)) + # The companion needs the stop_level UUID to anchor a new pin to this level. + assert level_json["stop_level_id"] == stop_level.id + + floorplan = level_json["floorplan"] assert floorplan["filename"] == "B1/busway plan.png" assert floorplan["center_lat"] == 39.9536 From d55e1ba85c0469d76446f68a7a3edc99b5d9f173 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Tue, 23 Jun 2026 13:58:40 -0400 Subject: [PATCH 5/6] feat(api): journal workflow field closed_at/closed_by (was resolved_at) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename the open/closed workflow field resolved_at → closed_at (null = open) and add closed_by (the user who closed it, a plain id like author_id). Closing is a desk/Studio action; the contract just carries the fields. Applied across the create_journal_entries migration, the JournalEntry schema, the sync upsert (+ on_conflict replace list), and the bundle serializers; round-trip test added. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UyezV5kznW5rbRFVsoXWvh --- lib/gtfs_planner/gtfs.ex | 6 +- lib/gtfs_planner/gtfs/journal_entry.ex | 9 ++- .../api/v1/station_controller.ex | 6 +- .../api/v1/sync_controller.ex | 3 +- .../20260623003416_create_journal_entries.exs | 3 +- .../api/v1/sync_controller_test.exs | 79 +++++++++++++++++++ 6 files changed, 97 insertions(+), 9 deletions(-) diff --git a/lib/gtfs_planner/gtfs.ex b/lib/gtfs_planner/gtfs.ex index 25c4d4aa..95101352 100644 --- a/lib/gtfs_planner/gtfs.ex +++ b/lib/gtfs_planner/gtfs.ex @@ -2274,7 +2274,8 @@ defmodule GtfsPlanner.Gtfs do @doc """ Upsert a station-journal entry by its client-generated `id` (idempotent). On conflict, the mutable fields are replaced (last-write-wins on `body` / - `resolved_at`); scoping and authorship are preserved from the original insert. + `closed_at` / `closed_by`); scoping and authorship are preserved from the + original insert. A `pin` entry's canonical anchor is its diagram coordinate (`stop_level_id` + `diagram_x/y`), exactly like a node's `diagram_coordinate`. Sync never sets @@ -2291,7 +2292,8 @@ defmodule GtfsPlanner.Gtfs do {:replace, [ :body, - :resolved_at, + :closed_at, + :closed_by, :target_type, :target_id, :stop_level_id, diff --git a/lib/gtfs_planner/gtfs/journal_entry.ex b/lib/gtfs_planner/gtfs/journal_entry.ex index 8bb0776c..8ccc8730 100644 --- a/lib/gtfs_planner/gtfs/journal_entry.ex +++ b/lib/gtfs_planner/gtfs/journal_entry.ex @@ -32,7 +32,8 @@ defmodule GtfsPlanner.Gtfs.JournalEntry do body: String.t() | nil, author_id: Ecto.UUID.t(), captured_at: DateTime.t(), - resolved_at: DateTime.t() | nil, + closed_at: DateTime.t() | nil, + closed_by: Ecto.UUID.t() | nil, inserted_at: DateTime.t(), updated_at: DateTime.t() } @@ -51,7 +52,8 @@ defmodule GtfsPlanner.Gtfs.JournalEntry do field :body, :string field :author_id, :binary_id field :captured_at, :utc_datetime_usec - field :resolved_at, :utc_datetime_usec + field :closed_at, :utc_datetime_usec + field :closed_by, :binary_id field :station_id, :binary_id belongs_to :organization, GtfsPlanner.Organizations.Organization, @@ -81,7 +83,8 @@ defmodule GtfsPlanner.Gtfs.JournalEntry do :body, :author_id, :captured_at, - :resolved_at + :closed_at, + :closed_by ]) |> trim_string_fields() |> validate_required([ diff --git a/lib/gtfs_planner_web/api/v1/station_controller.ex b/lib/gtfs_planner_web/api/v1/station_controller.ex index 3aa6aba2..29866871 100644 --- a/lib/gtfs_planner_web/api/v1/station_controller.ex +++ b/lib/gtfs_planner_web/api/v1/station_controller.ex @@ -331,7 +331,8 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do body: entry.body, author_id: entry.author_id, captured_at: entry.captured_at, - resolved_at: entry.resolved_at, + closed_at: entry.closed_at, + closed_by: entry.closed_by, photos: Map.get(photos_by_entry, entry.id, []) } end @@ -349,7 +350,8 @@ defmodule GtfsPlannerWeb.Api.V1.StationController do body: entry.body, author_id: entry.author_id, captured_at: entry.captured_at, - resolved_at: entry.resolved_at, + closed_at: entry.closed_at, + closed_by: entry.closed_by, photos: Map.get(photos_by_entry, entry.id, []) } end diff --git a/lib/gtfs_planner_web/api/v1/sync_controller.ex b/lib/gtfs_planner_web/api/v1/sync_controller.ex index 516d9d63..395b5a53 100644 --- a/lib/gtfs_planner_web/api/v1/sync_controller.ex +++ b/lib/gtfs_planner_web/api/v1/sync_controller.ex @@ -177,7 +177,8 @@ defmodule GtfsPlannerWeb.Api.V1.SyncController do "diagram_y" => update["diagram_y"], "body" => update["body"], "captured_at" => update["captured_at"], - "resolved_at" => update["resolved_at"] + "closed_at" => update["closed_at"], + "closed_by" => update["closed_by"] } case Gtfs.upsert_journal_entry(attrs) do diff --git a/priv/repo/migrations/20260623003416_create_journal_entries.exs b/priv/repo/migrations/20260623003416_create_journal_entries.exs index 58aaacf0..080ffa81 100644 --- a/priv/repo/migrations/20260623003416_create_journal_entries.exs +++ b/priv/repo/migrations/20260623003416_create_journal_entries.exs @@ -32,7 +32,8 @@ defmodule GtfsPlanner.Repo.Migrations.CreateJournalEntries do # user deletion never cascades into or nulls journal history. add :author_id, :binary_id, null: false add :captured_at, :utc_datetime_usec, null: false - add :resolved_at, :utc_datetime_usec + add :closed_at, :utc_datetime_usec + add :closed_by, :binary_id timestamps(type: :utc_datetime_usec) end diff --git a/test/gtfs_planner_web/api/v1/sync_controller_test.exs b/test/gtfs_planner_web/api/v1/sync_controller_test.exs index 134bc47a..6cd55509 100644 --- a/test/gtfs_planner_web/api/v1/sync_controller_test.exs +++ b/test/gtfs_planner_web/api/v1/sync_controller_test.exs @@ -474,6 +474,85 @@ defmodule GtfsPlannerWeb.Api.V1.SyncControllerTest do assert Repo.get!(JournalEntry, id).body == "Edited" end + test "round-trips closed_at/closed_by through sync and the bundle", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + %{station: station} = build_station_with_pathway(org.id, version.id) + + closed_id = Ecto.UUID.generate() + open_id = Ecto.UUID.generate() + closer_id = Ecto.UUID.generate() + + payload = %{ + "pathways" => [], + "journal_entries" => [ + %{ + "id" => closed_id, + "target_type" => "station", + "body" => "Closed note", + "captured_at" => "2026-06-20T10:00:00Z", + "closed_at" => "2026-06-21T12:00:00Z", + "closed_by" => closer_id + }, + %{ + "id" => open_id, + "target_type" => "station", + "body" => "Open note", + "captured_at" => "2026-06-20T10:00:00Z" + } + ] + } + + conn1 = conn |> authed_conn(user) |> post(sync_url(version.id, station.id), payload) + assert %{"data" => data} = json_response(conn1, 200) + assert data["journal_synced_count"] == 2 + + # Persisted as the close state (null closed_at = open). + closed = Repo.get!(JournalEntry, closed_id) + assert closed.closed_at == ~U[2026-06-21 12:00:00.000000Z] + assert closed.closed_by == closer_id + + open = Repo.get!(JournalEntry, open_id) + assert is_nil(open.closed_at) + assert is_nil(open.closed_by) + + # Comes back out through the bundle. + bundle_conn = + build_conn() + |> authed_conn(user) + |> get("/api/v1/versions/#{version.id}/stations/#{station.id}/bundle") + + assert %{"data" => bundle} = json_response(bundle_conn, 200) + entries = bundle["journal_entries"] + + closed_json = Enum.find(entries, &(&1["id"] == closed_id)) + assert closed_json["closed_at"] == "2026-06-21T12:00:00.000000Z" + assert closed_json["closed_by"] == closer_id + + open_json = Enum.find(entries, &(&1["id"] == open_id)) + assert is_nil(open_json["closed_at"]) + assert is_nil(open_json["closed_by"]) + + # A re-sync can update the close state (on_conflict replaces it). + reopen = %{ + "pathways" => [], + "journal_entries" => [ + %{ + "id" => closed_id, + "target_type" => "station", + "body" => "Closed note", + "captured_at" => "2026-06-20T10:00:00Z" + } + ] + } + + build_conn() |> authed_conn(user) |> post(sync_url(version.id, station.id), reopen) + assert is_nil(Repo.get!(JournalEntry, closed_id).closed_at) + end + test "reports a per-item error without failing the batch", %{conn: conn, user: user, org: org} do version = gtfs_version_fixture(org.id) %{station: station} = build_station_with_pathway(org.id, version.id) From b0f021cc4dd4aa180863c547e0e9cad6d9a0e703 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Tue, 23 Jun 2026 16:12:34 -0400 Subject: [PATCH 6/6] feat(api): persist client-provided photo dimensions on journal-photo upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The journal-photo upload dropped width/height — they were never read from the multipart metadata, so every photo stored null dimensions. Read width/height from metadata and pass them through upsert_journal_photo (the schema already casts them and the on_conflict replaces them). The server doesn't decode uploads; dimensions are client-computed at capture. Absent for older clients → null, unchanged. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UyezV5kznW5rbRFVsoXWvh --- .../api/v1/journal_photo_controller.ex | 4 ++ .../api/v1/journal_photo_controller_test.exs | 65 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/lib/gtfs_planner_web/api/v1/journal_photo_controller.ex b/lib/gtfs_planner_web/api/v1/journal_photo_controller.ex index abf92d1c..55157e23 100644 --- a/lib/gtfs_planner_web/api/v1/journal_photo_controller.ex +++ b/lib/gtfs_planner_web/api/v1/journal_photo_controller.ex @@ -51,6 +51,10 @@ defmodule GtfsPlannerWeb.Api.V1.JournalPhotoController do "filename" => filename, "content_type" => content_type, "byte_size" => byte_size, + # Pixel dimensions are client-computed at capture (the server + # doesn't decode uploads); absent for older clients → stay null. + "width" => metadata["width"], + "height" => metadata["height"], "captured_at" => metadata["captured_at"] }) do conn diff --git a/test/gtfs_planner_web/api/v1/journal_photo_controller_test.exs b/test/gtfs_planner_web/api/v1/journal_photo_controller_test.exs index 494fbcff..7c4b6595 100644 --- a/test/gtfs_planner_web/api/v1/journal_photo_controller_test.exs +++ b/test/gtfs_planner_web/api/v1/journal_photo_controller_test.exs @@ -142,6 +142,71 @@ defmodule GtfsPlannerWeb.Api.V1.JournalPhotoControllerTest do assert File.exists?(dest) end + test "persists client-provided pixel dimensions from metadata", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + station = build_station(org.id, version.id) + entry = journal_entry_fixture(org, version, station, user) + + photo_id = Ecto.UUID.generate() + + payload = %{ + "file" => upload_fixture("jpegbytes"), + "metadata" => + Jason.encode!(%{ + "id" => photo_id, + "journal_entry_id" => entry.id, + "captured_at" => "2026-06-20T10:05:00Z", + "content_type" => "image/jpeg", + "width" => 4032, + "height" => 3024 + }) + } + + conn = conn |> authed_conn(user) |> post(photos_url(version.id, station.id), payload) + + assert %{"data" => %{"photo" => photo}} = json_response(conn, 201) + assert photo["width"] == 4032 + assert photo["height"] == 3024 + + row = Repo.get!(JournalPhoto, photo_id) + assert row.width == 4032 + assert row.height == 3024 + end + + test "leaves dimensions null when metadata omits them (older client)", %{ + conn: conn, + user: user, + org: org + } do + version = gtfs_version_fixture(org.id) + station = build_station(org.id, version.id) + entry = journal_entry_fixture(org, version, station, user) + + photo_id = Ecto.UUID.generate() + + payload = %{ + "file" => upload_fixture("jpegbytes"), + "metadata" => + Jason.encode!(%{ + "id" => photo_id, + "journal_entry_id" => entry.id, + "captured_at" => "2026-06-20T10:05:00Z", + "content_type" => "image/jpeg" + }) + } + + conn = conn |> authed_conn(user) |> post(photos_url(version.id, station.id), payload) + + assert json_response(conn, 201) + row = Repo.get!(JournalPhoto, photo_id) + assert is_nil(row.width) + assert is_nil(row.height) + end + test "is idempotent on id (re-upload returns the same record)", %{ conn: conn, user: user,