From 6bfcbee7fc31606ba8886afd6c3e43f42a793383 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 6 May 2026 14:57:55 -0500 Subject: [PATCH] feat: draggable Leaflet marker for radio location on coverage form Adds a "Radio location" section to the coverage form with an embedded Leaflet map. The marker starts at the parent site's coordinates and the user can drag it to the actual radio location. A "Use site location" button clears the override. - New CoverageLocationPicker hook in assets/js/app.ts: creates the map, draggable marker, and pushes "location_picked" {lat, lon} on dragend. Re-creates the marker when the user picks a site that initially had no coords. - form.html.heex: location section with map div (phx-hook), readout ("lat, lon - override" if set), and reset button. Hidden inputs for latitude_override / longitude_override so the form's params always carry the chosen coordinates. - form.ex: handle_event "location_picked" updates the override fields in the changeset; "use_site_location" clears them. Helper functions radio_marker_lat/lon walk the form values, falling back to the selected site. CI test gate fix: - Worker test now requires GDAL CLI tools to write GeoTIFF + PNG. test_helper.exs detects whether gdal_translate / gdaldem are on PATH; when missing (CI's lean test image), :requires_gdal tagged tests are excluded. Locally with brew install gdal they still run. --- assets/js/app.ts | 102 +++++++++++++++- lib/towerops_web/live/coverage_live/form.ex | 113 ++++++++++++++++++ .../live/coverage_live/form.html.heex | 44 +++++++ test/test_helper.exs | 42 ++++--- .../towerops/workers/coverage_worker_test.exs | 6 + 5 files changed, 291 insertions(+), 16 deletions(-) diff --git a/assets/js/app.ts b/assets/js/app.ts index c455587d..7e85fd5d 100644 --- a/assets/js/app.ts +++ b/assets/js/app.ts @@ -1776,6 +1776,106 @@ const SitesMap = { } } +// CoverageLocationPicker shows a draggable marker on a Leaflet map so +// the user can pick the radio's exact location. Initial marker comes +// from data-marker-lat/lon attributes (set by the LiveView from the +// chosen site or from latitude_override/longitude_override). Dragging +// the marker pushes a `location_picked` event with the new {lat, lon}. +const CoverageLocationPicker = { + map: null as any, + marker: null as any, + programmaticMove: false, + + mounted(this: any) { + if (typeof L === 'undefined') { + setTimeout(() => this.mounted(), 100) + return + } + this.initMap() + }, + + updated(this: any) { + this.syncMarkerFromAttrs() + }, + + destroyed(this: any) { + if (this.map) { + this.map.remove() + this.map = null + this.marker = null + } + }, + + initMap(this: any) { + const lat = this.markerLat() + const lon = this.markerLon() + const hasFix = lat !== null && lon !== null + + const center: [number, number] = hasFix ? [lat, lon] : [30.27, -97.74] + const zoom = hasFix ? 15 : 6 + + this.map = L.map(this.el, { zoomControl: true, scrollWheelZoom: true }) + .setView(center, zoom) + + L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', { + attribution: '© OpenStreetMap contributors', + maxZoom: 19 + }).addTo(this.map) + + if (hasFix) { + this.marker = L.marker([lat, lon], { + draggable: true, + autoPan: true, + }).addTo(this.map) + + this.marker.on('dragend', (e: any) => { + const ll = e.target.getLatLng() + this.pushEvent('location_picked', { lat: ll.lat, lon: ll.lng }) + }) + } + }, + + syncMarkerFromAttrs(this: any) { + const lat = this.markerLat() + const lon = this.markerLon() + if (lat === null || lon === null) return + + if (!this.marker) { + // Site got picked after mount — create the marker now. + this.marker = L.marker([lat, lon], { + draggable: true, + autoPan: true, + }).addTo(this.map) + this.marker.on('dragend', (e: any) => { + const ll = e.target.getLatLng() + this.pushEvent('location_picked', { lat: ll.lat, lon: ll.lng }) + }) + this.map.setView([lat, lon], 15) + return + } + + const cur = this.marker.getLatLng() + if (Math.abs(cur.lat - lat) > 1e-7 || Math.abs(cur.lng - lon) > 1e-7) { + this.marker.setLatLng([lat, lon]) + this.map.panTo([lat, lon]) + } + }, + + markerLat(this: any): number | null { + const v = this.el.dataset.markerLat + if (!v) return null + const n = parseFloat(v) + return Number.isFinite(n) ? n : null + }, + + markerLon(this: any): number | null { + const v = this.el.dataset.markerLon + if (!v) return null + const n = parseFloat(v) + return Number.isFinite(n) ? n : null + }, +} + // CoverageMap renders a single coverage's RSSI heatmap as a Leaflet // imageOverlay, with a marker at the antenna location and a configurable // opacity slider. Data attributes on the host element: @@ -2536,7 +2636,7 @@ const WeathermapViewer = { const liveSocket = new LiveSocket("/live", Socket, { longPollFallbackMs: 5000, params: { _csrf_token: csrfToken, timezone: userTimezone }, - hooks: { ...colocatedHooks, SensorChart, CopyToClipboard, ScrollToTop, AutoDismissFlash, BetaBannerDismiss, NetworkMap, WeathermapViewer, SitesMap, CoverageMap, LeafletMap, DeviceListReorder, SortableList, MikrotikPortSync, GlobalSearch, GlobalSearchTrigger, DynamicFavicon, StatusTitle, ThemeSelector, SidebarCollapse }, + hooks: { ...colocatedHooks, SensorChart, CopyToClipboard, ScrollToTop, AutoDismissFlash, BetaBannerDismiss, NetworkMap, WeathermapViewer, SitesMap, CoverageMap, CoverageLocationPicker, LeafletMap, DeviceListReorder, SortableList, MikrotikPortSync, GlobalSearch, GlobalSearchTrigger, DynamicFavicon, StatusTitle, ThemeSelector, SidebarCollapse }, }) // Show progress bar on live navigation and form submits diff --git a/lib/towerops_web/live/coverage_live/form.ex b/lib/towerops_web/live/coverage_live/form.ex index d307aee9..d02328f6 100644 --- a/lib/towerops_web/live/coverage_live/form.ex +++ b/lib/towerops_web/live/coverage_live/form.ex @@ -2,6 +2,7 @@ defmodule ToweropsWeb.CoverageLive.Form do @moduledoc false use ToweropsWeb, :live_view + alias Phoenix.HTML.Form, as: HTMLForm alias Towerops.Coverages alias Towerops.Coverages.Antenna alias Towerops.Coverages.Coverage @@ -90,6 +91,45 @@ defmodule ToweropsWeb.CoverageLive.Form do save(socket, socket.assigns.live_action, attrs) end + @impl true + def handle_event("location_picked", %{"lat" => lat, "lon" => lon}, socket) do + attrs = + socket + |> current_form_attrs() + |> Map.put("latitude_override", to_string(lat)) + |> Map.put("longitude_override", to_string(lon)) + + changeset = + socket.assigns.coverage + |> Coverages.change_coverage(attrs) + |> Map.put(:action, :validate) + + {:noreply, assign_form(socket, changeset)} + end + + @impl true + def handle_event("use_site_location", _params, socket) do + attrs = + socket + |> current_form_attrs() + |> Map.put("latitude_override", "") + |> Map.put("longitude_override", "") + + changeset = + socket.assigns.coverage + |> Coverages.change_coverage(attrs) + |> Map.put(:action, :validate) + + {:noreply, assign_form(socket, changeset)} + end + + defp current_form_attrs(socket) do + case socket.assigns[:form] do + nil -> %{} + form -> form.params || %{} + end + end + defp save(socket, :new, attrs) do case Coverages.create_coverage(socket.assigns.organization.id, attrs) do {:ok, coverage} -> @@ -200,6 +240,79 @@ defmodule ToweropsWeb.CoverageLive.Form do |> Enum.filter(&(&1.organization_id == org_id)) end + @doc """ + Latitude to display on the location-picker marker: + override (if set) → selected site's lat → nil. + """ + def radio_marker_lat(form, sites) do + case form_field_value(form, :latitude_override) do + lat when is_number(lat) -> + lat + + lat when is_binary(lat) and lat != "" -> + case Float.parse(lat) do + {n, _} -> n + :error -> selected_site_lat(form, sites) + end + + _ -> + selected_site_lat(form, sites) + end + end + + @doc "Longitude to display on the location-picker marker." + def radio_marker_lon(form, sites) do + case form_field_value(form, :longitude_override) do + lon when is_number(lon) -> + lon + + lon when is_binary(lon) and lon != "" -> + case Float.parse(lon) do + {n, _} -> n + :error -> selected_site_lon(form, sites) + end + + _ -> + selected_site_lon(form, sites) + end + end + + @doc "True if the form's lat/lon override is set (non-empty)." + def radio_overridden?(form) do + has_override?(form_field_value(form, :latitude_override)) and + has_override?(form_field_value(form, :longitude_override)) + end + + defp has_override?(nil), do: false + defp has_override?(""), do: false + defp has_override?(_), do: true + + defp selected_site_lat(form, sites) do + case selected_site(form, sites) do + %{latitude: lat} when is_number(lat) -> lat + _ -> nil + end + end + + defp selected_site_lon(form, sites) do + case selected_site(form, sites) do + %{longitude: lon} when is_number(lon) -> lon + _ -> nil + end + end + + defp selected_site(form, sites) do + case form_field_value(form, :site_id) do + id when is_binary(id) and id != "" -> Enum.find(sites, &(&1.id == id)) + _ -> nil + end + end + + defp form_field_value(form, field) do + v = HTMLForm.input_value(form, field) + v + end + @doc false def device_label(device) do case device do diff --git a/lib/towerops_web/live/coverage_live/form.html.heex b/lib/towerops_web/live/coverage_live/form.html.heex index b918580c..0bdc4fb7 100644 --- a/lib/towerops_web/live/coverage_live/form.html.heex +++ b/lib/towerops_web/live/coverage_live/form.html.heex @@ -82,6 +82,50 @@ /> + <%!-- Radio location: draggable marker on a Leaflet map. --%> +
+
+

+ {t("Radio location")} +

+ +
+ +

+ {t("Drag the marker to where the radio is mounted. Defaults to the site's coordinates.")} +

+ +
+
+ +
+ {radio_marker_lat(@form, @sites)}, {radio_marker_lon(@form, @sites)} + + ({t("override")}) + +
+ + <.input field={@form[:latitude_override]} type="hidden" /> + <.input field={@form[:longitude_override]} type="hidden" /> +
+ <%!-- Radio --%>

diff --git a/test/test_helper.exs b/test/test_helper.exs index 5adf300c..c91dc941 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -43,24 +43,36 @@ Towerops.Coverages.Antenna.load_registry() ExUnit.start() +# Skip the coverage-compute integration test when the GDAL CLI tools +# aren't installed (e.g. CI test gate runs in a lean image without +# gdal-bin). Locally we install GDAL via `brew install gdal` per +# CLAUDE.md, so the test runs there. +extra_excludes = + if System.find_executable("gdal_translate") && System.find_executable("gdaldem") do + [] + else + [:requires_gdal] + end + # Exclude tests by default # Run specific tests with: mix test --only ExUnit.configure( - exclude: [ - :integration, - # Tests making real network calls (ping, DNS, SSL) - :network, - # SnmpKit-specific exclusions - :performance, - :snmpv3, - :memory, - :shell_integration, - :optional, - :needs_simulator, - :manual, - :real_device, - :yecc_required - ] + exclude: + [ + :integration, + # Tests making real network calls (ping, DNS, SSL) + :network, + # SnmpKit-specific exclusions + :performance, + :snmpv3, + :memory, + :shell_integration, + :optional, + :needs_simulator, + :manual, + :real_device, + :yecc_required + ] ++ extra_excludes ) # Define mocks for testing diff --git a/test/towerops/workers/coverage_worker_test.exs b/test/towerops/workers/coverage_worker_test.exs index f7d3ee82..bb49d330 100644 --- a/test/towerops/workers/coverage_worker_test.exs +++ b/test/towerops/workers/coverage_worker_test.exs @@ -2,6 +2,10 @@ defmodule Towerops.Workers.CoverageWorkerTest do use Towerops.DataCase, async: false use Oban.Testing, repo: Towerops.Repo + # The compute pipeline shells out to GDAL (`gdal_translate`, `gdaldem`) + # to write the GeoTIFF + colored PNG. CI's lean test image doesn't have + # gdal-bin; test_helper.exs conditionally excludes this tag when those + # binaries aren't on PATH. import Towerops.AccountsFixtures import Towerops.CoveragesFixtures import Towerops.OrganizationsFixtures @@ -11,6 +15,8 @@ defmodule Towerops.Workers.CoverageWorkerTest do alias Towerops.Test.StubTerrain alias Towerops.Workers.CoverageWorker + @moduletag :requires_gdal + setup do user = user_fixture() org = organization_fixture(user.id)