From 80ba4a2d3066ed265f14aed999ee03322eb37adc Mon Sep 17 00:00:00 2001 From: Steve Foale Date: Thu, 2 Jul 2026 07:05:59 +0100 Subject: [PATCH 1/2] Fix parsing of polygon instrument footprints and add a clearer message for non-logged in users. --- .../src/lib/services/instrumentService.ts | 20 ++++++++ server/routes/ui/preview_footprint.py | 24 ++++++---- tests/fastapi/test_ui.py | 47 +++++++++++++++++++ 3 files changed, 83 insertions(+), 8 deletions(-) diff --git a/frontend/src/lib/services/instrumentService.ts b/frontend/src/lib/services/instrumentService.ts index 3e1d5fa3..8d743025 100644 --- a/frontend/src/lib/services/instrumentService.ts +++ b/frontend/src/lib/services/instrumentService.ts @@ -90,6 +90,18 @@ export async function submitInstrument( }; } + // Handle authentication/authorization errors. The backend returns a + // token-centric message ("provide a valid JWT token or API token"), + // which is confusing on the website where users log in via a session. + if (axiosError.response?.status === 401 || axiosError.response?.status === 403) { + const message = 'You must be logged in to submit an instrument. Please log in and try again.'; + return { + success: false, + message, + errors: [message] + }; + } + // Handle 400 Bad Request errors if (axiosError.response?.status === 400) { const detail = @@ -197,6 +209,14 @@ export async function previewFootprint( } const response = await api.client.get(`/ajax_preview_footprint?${params.toString()}`); + + // The endpoint returns { error } when the footprint params can't be + // parsed (e.g. malformed polygon text). Surface it instead of returning + // an object with no data/layout, which would silently fail to render. + if (response.data?.error) { + throw new Error(response.data.error); + } + return response.data; } catch (error) { console.error('Failed to preview footprint:', error); diff --git a/server/routes/ui/preview_footprint.py b/server/routes/ui/preview_footprint.py index d036280b..29219466 100644 --- a/server/routes/ui/preview_footprint.py +++ b/server/routes/ui/preview_footprint.py @@ -21,10 +21,11 @@ async def preview_footprint( ): """Generate a preview of an instrument footprint.""" import math - import json import plotly import plotly.graph_objects as go + from server.utils.footprint_processing import parse_multi_polygon + # This is a UI helper endpoint to visualize a footprint before saving # It generates the appropriate visualization for the given parameters vertices = [] @@ -70,13 +71,20 @@ async def preview_footprint( vertices.append(rect_points) elif shape == "Polygon" and polygon: - # For custom polygon, parse the points - try: - poly_points = json.loads(polygon) - poly_points.append(poly_points[0]) # Close the polygon - vertices.append(poly_points) - except json.JSONDecodeError: + # For custom polygon, parse the UI text format: + # [(x, y) ... ] # [(x, y) ... ] (multi-polygon, "#"-delimited) + # or a single newline-separated list of "(x, y)" coordinate pairs. + # This is the same parser used by the instrument-submit path, so the + # preview matches what will actually be stored. + polygons, errors = parse_multi_polygon(polygon, scale=1.0) + if errors: + return {"error": "; ".join(errors)} + if not polygons: return {"error": "Invalid polygon format"} + + for poly_points in polygons: + # parse_multi_polygon already closes each ring; offset to ra/dec. + vertices.append([[ra + v[0], dec + v[1]] for v in poly_points]) else: return {"error": "Invalid shape type or missing required parameters"} @@ -86,7 +94,7 @@ async def preview_footprint( xs = [v[0] for v in vert] ys = [v[1] for v in vert] trace = go.Scatter( - x=xs, y=ys, line_color="blue", fill="tozeroy", fillcolor="violet" + x=xs, y=ys, mode="lines", line_color="blue", fill="toself", fillcolor="violet" ) traces.append(trace) diff --git a/tests/fastapi/test_ui.py b/tests/fastapi/test_ui.py index f775a60a..bf7b1d82 100644 --- a/tests/fastapi/test_ui.py +++ b/tests/fastapi/test_ui.py @@ -96,6 +96,53 @@ def test_ajax_preview_footprint_rectangle(self): except json.JSONDecodeError: assert False, "Response is not valid JSON" + def test_ajax_preview_footprint_polygon_single(self): + """Test previewing a single-polygon footprint in the UI text format.""" + polygon = "(-1, 1)\n(1, 1)\n(1, -1)\n(-1, -1)\n(-1, 1)" + response = requests.get( + self.get_url("/ajax_preview_footprint"), + params={"ra": 0, "dec": 0, "shape": "Polygon", "polygon": polygon}, + headers={"api_token": self.admin_token}, + ) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert "error" not in data + assert "data" in data + assert len(data["data"]) == 1 + + def test_ajax_preview_footprint_polygon_multi(self): + """Test previewing a multi-polygon footprint (# delimited, [] bracketed).""" + polygon = ( + "[(-1, 1)\n(-0.3, 1)\n(-0.3, -1)\n(-1, -1)\n(-1, 1)]\n" + "#\n" + "[(0.3, 1)\n(1, 1)\n(1, -1)\n(0.3, -1)\n(0.3, 1)]" + ) + response = requests.get( + self.get_url("/ajax_preview_footprint"), + params={"ra": 0, "dec": 0, "shape": "Polygon", "polygon": polygon}, + headers={"api_token": self.admin_token}, + ) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert "error" not in data + assert "data" in data + # One plotly trace per polygon + assert len(data["data"]) == 2 + + def test_ajax_preview_footprint_polygon_invalid(self): + """Test previewing a malformed polygon returns a descriptive error.""" + response = requests.get( + self.get_url("/ajax_preview_footprint"), + params={"ra": 0, "dec": 0, "shape": "Polygon", "polygon": "not a polygon"}, + headers={"api_token": self.admin_token}, + ) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert "error" in data + def test_ajax_preview_footprint_invalid_shape(self): """Test previewing a footprint with invalid shape.""" response = requests.get( From 46b9652aa2109b044941b27c0363a0e9345caedf Mon Sep 17 00:00:00 2001 From: Steve Foale Date: Thu, 2 Jul 2026 09:57:00 +0100 Subject: [PATCH 2/2] Scale preview footprints by unit, lock 1:1 aspect, POST large payloads. --- frontend/src/lib/api/services/ajax.service.ts | 23 +-- .../src/lib/services/instrumentService.ts | 26 ++-- .../routes/submit/instruments/+page.svelte | 4 +- server/routes/ui/preview_footprint.py | 138 +++++++++--------- tests/fastapi/test_ui.py | 72 +++++---- 5 files changed, 140 insertions(+), 123 deletions(-) diff --git a/frontend/src/lib/api/services/ajax.service.ts b/frontend/src/lib/api/services/ajax.service.ts index 2bf2cc54..563cbc0b 100644 --- a/frontend/src/lib/api/services/ajax.service.ts +++ b/frontend/src/lib/api/services/ajax.service.ts @@ -43,18 +43,19 @@ export const ajaxService = { height?: number, width?: number, shape?: string, - polygon?: string + polygon?: string, + unit?: string ): Promise => { - const response = await client.get('/ajax_preview_footprint', { - params: { - ra, - dec, - radius, - height, - width, - shape, - polygon - } + // POST a JSON body: large multi-polygon footprints can exceed URL limits. + const response = await client.post('/ajax_preview_footprint', { + ra, + dec, + radius, + height, + width, + shape, + unit, + polygon }); return response.data; }, diff --git a/frontend/src/lib/services/instrumentService.ts b/frontend/src/lib/services/instrumentService.ts index 8d743025..87136616 100644 --- a/frontend/src/lib/services/instrumentService.ts +++ b/frontend/src/lib/services/instrumentService.ts @@ -192,23 +192,27 @@ export async function previewFootprint( instrumentData: Partial ): Promise<{ data: unknown; layout: unknown }> { try { - const params = new URLSearchParams(); - - // Set default coordinates for preview (centered at 0,0) - params.append('ra', '0'); - params.append('dec', '0'); - params.append('shape', instrumentData.footprint_type || 'Circular'); + // POST the params as a JSON body: multi-polygon footprints (e.g. LSST's + // ~170 CCDs) can exceed URL length limits, causing intermittent 414s. + const body: Record = { + // Preview centered at 0,0; unit drives the same degree scaling as + // instrument creation so the preview axes are faithful to reality. + ra: 0, + dec: 0, + shape: instrumentData.footprint_type || 'Circular', + unit: instrumentData.unit || 'deg' + }; if (instrumentData.footprint_type === 'Rectangular') { - if (instrumentData.height) params.append('height', instrumentData.height.toString()); - if (instrumentData.width) params.append('width', instrumentData.width.toString()); + if (instrumentData.height) body.height = instrumentData.height; + if (instrumentData.width) body.width = instrumentData.width; } else if (instrumentData.footprint_type === 'Circular') { - if (instrumentData.radius) params.append('radius', instrumentData.radius.toString()); + if (instrumentData.radius) body.radius = instrumentData.radius; } else if (instrumentData.footprint_type === 'Polygon') { - if (instrumentData.polygon) params.append('polygon', instrumentData.polygon); + if (instrumentData.polygon) body.polygon = instrumentData.polygon; } - const response = await api.client.get(`/ajax_preview_footprint?${params.toString()}`); + const response = await api.client.post('/ajax_preview_footprint', body); // The endpoint returns { error } when the footprint params can't be // parsed (e.g. malformed polygon text). Surface it instead of returning diff --git a/frontend/src/routes/submit/instruments/+page.svelte b/frontend/src/routes/submit/instruments/+page.svelte index 284d622e..661bf186 100644 --- a/frontend/src/routes/submit/instruments/+page.svelte +++ b/frontend/src/routes/submit/instruments/+page.svelte @@ -594,8 +594,10 @@ } .preview-plot { + /* Square container so the 1:1-constrained plot fills it without wide + empty margins. */ width: 100%; - height: 400px; + aspect-ratio: 1 / 1; } .preview-error { diff --git a/server/routes/ui/preview_footprint.py b/server/routes/ui/preview_footprint.py index 29219466..81930b8c 100644 --- a/server/routes/ui/preview_footprint.py +++ b/server/routes/ui/preview_footprint.py @@ -1,110 +1,106 @@ """Preview footprint endpoint.""" -from fastapi import APIRouter, Depends -from sqlalchemy.orm import Session +from fastapi import APIRouter +from pydantic import BaseModel -from server.db.database import get_db +from server.utils.footprint_processing import ( + get_scale_factor, + create_circular_footprint, + create_rectangular_footprint, + parse_multi_polygon, +) router = APIRouter(tags=["UI"]) -@router.get("/ajax_preview_footprint") -async def preview_footprint( - ra: float, - dec: float, - radius: float = None, - height: float = None, - width: float = None, - shape: str = "Circular", - polygon: str = None, - db: Session = Depends(get_db), -): - """Generate a preview of an instrument footprint.""" - import math - import plotly - import plotly.graph_objects as go +class FootprintPreviewRequest(BaseModel): + """Parameters for a footprint preview. + + Sent as a POST body rather than a query string: multi-polygon footprints + (e.g. the ~170-CCD LSST camera) can produce very large payloads that would + exceed URL/request-line length limits and fail with a 414. + """ + + shape: str = "Circular" + unit: str = "deg" + ra: float = 0.0 + dec: float = 0.0 + radius: float | None = None + height: float | None = None + width: float | None = None + polygon: str | None = None + + +@router.post("/ajax_preview_footprint") +async def preview_footprint(request: FootprintPreviewRequest): + """Generate a preview of an instrument footprint. - from server.utils.footprint_processing import parse_multi_polygon + Uses the same footprint parsing/scaling helpers as instrument creation, so + the preview is faithful to what will be stored — including unit scaling. + This lets a user spot a mistake (wrong unit or dimensions) before + submitting, since the axes are in real degrees. + """ + import plotly.graph_objects as go - # This is a UI helper endpoint to visualize a footprint before saving - # It generates the appropriate visualization for the given parameters + # Scale raw inputs to degrees using the selected unit, matching the + # conversion applied on instrument creation (get_scale_factor). + scale = get_scale_factor(request.unit) vertices = [] - if shape == "Circular" and radius: - # For circle, generate points around the circumference - circle_points = [] - for i in range(36): # 36 points for a smooth circle - # Proper spherical coordinate calculation for circles - # Convert angle to offset in RA/Dec using spherical trigonometry - x = radius * math.cos(math.radians(90 - i * 10)) - y = radius * math.sin(math.radians(90 - i * 10)) - # Adjust for spherical coordinates - if abs(x) < 1e-10: - x = 0.0 - if abs(y) < 1e-10: - y = 0.0 - point_ra = ra + x - point_dec = dec + y - circle_points.append([point_ra, point_dec]) - - # Close the polygon - circle_points.append(circle_points[0]) - vertices.append(circle_points) - - elif shape == "Rectangular" and height and width: - # For rectangle, generate corners - # Convert height/width in degrees to ra/dec coordinates - # Proper calculation accounting for coordinate system - half_width = width / 2 - half_height = height / 2 - - # No cos(dec) correction needed for simple rectangular footprints - ra_offset = half_width - - rect_points = [ - [ra - ra_offset, dec - half_height], # bottom left - [ra - ra_offset, dec + half_height], # top left - [ra + ra_offset, dec + half_height], # top right - [ra + ra_offset, dec - half_height], # bottom right - [ra - ra_offset, dec - half_height], # close the polygon - ] - vertices.append(rect_points) - - elif shape == "Polygon" and polygon: - # For custom polygon, parse the UI text format: + if request.shape == "Circular" and request.radius: + vertices.append(create_circular_footprint(request.radius, scale)) + + elif request.shape == "Rectangular" and request.height and request.width: + vertices.append( + create_rectangular_footprint(request.height, request.width, scale) + ) + + elif request.shape == "Polygon" and request.polygon: + # Parse the UI text format: # [(x, y) ... ] # [(x, y) ... ] (multi-polygon, "#"-delimited) # or a single newline-separated list of "(x, y)" coordinate pairs. - # This is the same parser used by the instrument-submit path, so the - # preview matches what will actually be stored. - polygons, errors = parse_multi_polygon(polygon, scale=1.0) + polygons, errors = parse_multi_polygon(request.polygon, scale) if errors: return {"error": "; ".join(errors)} if not polygons: return {"error": "Invalid polygon format"} + vertices.extend(polygons) - for poly_points in polygons: - # parse_multi_polygon already closes each ring; offset to ra/dec. - vertices.append([[ra + v[0], dec + v[1]] for v in poly_points]) else: return {"error": "Invalid shape type or missing required parameters"} + # The helpers produce origin-centered rings; offset them to the preview + # ra/dec so the axes read as sky coordinates. + vertices = [ + [[request.ra + v[0], request.dec + v[1]] for v in ring] for ring in vertices + ] + # Create a plotly figure traces = [] for vert in vertices: xs = [v[0] for v in vert] ys = [v[1] for v in vert] trace = go.Scatter( - x=xs, y=ys, mode="lines", line_color="blue", fill="toself", fillcolor="violet" + x=xs, + y=ys, + mode="lines", + line_color="blue", + fill="toself", + fillcolor="violet", ) traces.append(trace) fig = go.Figure(data=traces) + # Lock a 1:1 data aspect ratio so footprints render true to shape. + # constrain="domain" must sit on the axis with excess space; the preview + # container is wider than tall, so it belongs on x. Setting it on both is + # robust to either orientation. fig.update_layout( showlegend=False, xaxis_title="degrees", yaxis_title="degrees", + xaxis=dict(constrain="domain"), yaxis=dict( - matches="x", scaleanchor="x", scaleratio=1, constrain="domain", diff --git a/tests/fastapi/test_ui.py b/tests/fastapi/test_ui.py index bf7b1d82..a94359f0 100644 --- a/tests/fastapi/test_ui.py +++ b/tests/fastapi/test_ui.py @@ -56,27 +56,22 @@ def test_ajax_alertinstruments_footprints(self): def test_ajax_preview_footprint_circle(self): """Test previewing a circular footprint.""" - response = requests.get( + response = requests.post( self.get_url("/ajax_preview_footprint"), - params={"ra": 123.456, "dec": -12.345, "radius": 0.5, "shape": "Circular"}, + json={"ra": 123.456, "dec": -12.345, "radius": 0.5, "shape": "Circular"}, headers={"api_token": self.admin_token}, ) assert response.status_code == status.HTTP_200_OK - # The response should be a JSON string containing plotly figure data - assert isinstance(response.text, str) - # Try parsing as JSON to confirm it's valid - try: - json_data = json.loads(response.text) - assert "data" in json_data - except json.JSONDecodeError: - assert False, "Response is not valid JSON" + data = response.json() + assert "error" not in data + assert "data" in data def test_ajax_preview_footprint_rectangle(self): """Test previewing a rectangular footprint.""" - response = requests.get( + response = requests.post( self.get_url("/ajax_preview_footprint"), - params={ + json={ "ra": 123.456, "dec": -12.345, "height": 0.5, @@ -87,21 +82,16 @@ def test_ajax_preview_footprint_rectangle(self): ) assert response.status_code == status.HTTP_200_OK - # The response should be a JSON string containing plotly figure data - assert isinstance(response.text, str) - # Try parsing as JSON to confirm it's valid - try: - json_data = json.loads(response.text) - assert "data" in json_data - except json.JSONDecodeError: - assert False, "Response is not valid JSON" + data = response.json() + assert "error" not in data + assert "data" in data def test_ajax_preview_footprint_polygon_single(self): """Test previewing a single-polygon footprint in the UI text format.""" polygon = "(-1, 1)\n(1, 1)\n(1, -1)\n(-1, -1)\n(-1, 1)" - response = requests.get( + response = requests.post( self.get_url("/ajax_preview_footprint"), - params={"ra": 0, "dec": 0, "shape": "Polygon", "polygon": polygon}, + json={"ra": 0, "dec": 0, "shape": "Polygon", "polygon": polygon}, headers={"api_token": self.admin_token}, ) @@ -118,9 +108,9 @@ def test_ajax_preview_footprint_polygon_multi(self): "#\n" "[(0.3, 1)\n(1, 1)\n(1, -1)\n(0.3, -1)\n(0.3, 1)]" ) - response = requests.get( + response = requests.post( self.get_url("/ajax_preview_footprint"), - params={"ra": 0, "dec": 0, "shape": "Polygon", "polygon": polygon}, + json={"ra": 0, "dec": 0, "shape": "Polygon", "polygon": polygon}, headers={"api_token": self.admin_token}, ) @@ -131,11 +121,36 @@ def test_ajax_preview_footprint_polygon_multi(self): # One plotly trace per polygon assert len(data["data"]) == 2 + def test_ajax_preview_footprint_unit_scaling(self): + """Preview axes should reflect the selected unit, matching stored size. + + A 60 arcmin width equals 1 degree, so the arcmin preview should span + the same coordinate range as a 1 degree preview. + """ + deg = requests.post( + self.get_url("/ajax_preview_footprint"), + json={"height": 1.0, "width": 1.0, "shape": "Rectangular", "unit": "deg"}, + headers={"api_token": self.admin_token}, + ).json() + arcmin = requests.post( + self.get_url("/ajax_preview_footprint"), + json={ + "height": 60.0, + "width": 60.0, + "shape": "Rectangular", + "unit": "arcmin", + }, + headers={"api_token": self.admin_token}, + ).json() + + assert deg["data"][0]["x"] == arcmin["data"][0]["x"] + assert deg["data"][0]["y"] == arcmin["data"][0]["y"] + def test_ajax_preview_footprint_polygon_invalid(self): """Test previewing a malformed polygon returns a descriptive error.""" - response = requests.get( + response = requests.post( self.get_url("/ajax_preview_footprint"), - params={"ra": 0, "dec": 0, "shape": "Polygon", "polygon": "not a polygon"}, + json={"ra": 0, "dec": 0, "shape": "Polygon", "polygon": "not a polygon"}, headers={"api_token": self.admin_token}, ) @@ -145,9 +160,9 @@ def test_ajax_preview_footprint_polygon_invalid(self): def test_ajax_preview_footprint_invalid_shape(self): """Test previewing a footprint with invalid shape.""" - response = requests.get( + response = requests.post( self.get_url("/ajax_preview_footprint"), - params={"ra": 123.456, "dec": -12.345, "shape": "invalid"}, + json={"ra": 123.456, "dec": -12.345, "shape": "invalid"}, headers={"api_token": self.admin_token}, ) @@ -321,7 +336,6 @@ def test_authentication_patterns(self): # Most UI GET endpoints should be publicly accessible for viewing data public_get_endpoints = [ "/ajax_alertinstruments_footprints?graceid=S190425z", - "/ajax_preview_footprint?ra=123.456&dec=-12.345&radius=0.5&shape=Circular", "/ajax_icecube_notice?graceid=S190425z", "/ajax_event_galaxies?alertid=1", "/ajax_candidate?graceid=S190425z",