Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- **Skip-info band + optimistic `/info` revalidation on the map.** Panning and zooming no longer block on a fresh `/info` round-trip before the GeoJSON layers start loading. The frontend now uses a three-band strategy: below `MIN_DATA_ZOOM` (11) nothing renders; in the gated band, if a recent `/info` is cached the cached decision drives the immediate render and `/info` revalidates in the background with `If-None-Match` (a 304 leaves the screen untouched, a 200 reconciles only if the decision actually changed); at or above `SKIP_INFO_ZOOM` (default 17) `/info` is skipped entirely because the viewport is too small to plausibly cross any hide/cluster threshold. Configurable via `PLUGINS_CONFIG['netbox_pathways']['map_skip_info_zoom']` if a deployment hits the edge case. The pure decision logic lives in `netbox_pathways/static/netbox_pathways/src/load-strategy.ts` (`chooseLoadStrategy`, `decideSkipInfo`, `decisionsDiffer`) and is covered by vitest. `fetchMapInfo`'s callback now also signals whether the response was a 200 (fresh) or 304 (unchanged), so callers can skip the reconciliation render in the common case.
- **Computed `geo_length` on Pathway and subclasses** -- the drawn length of a pathway's LineString, in metres, is now exposed as a read-only `geo_length` property computed by PostGIS (`ST_Length`) rather than entered manually. The existing `length` field stays for as-built / field-measured lengths (slack, sag, riser drops) and is now labelled "Length (m, as-built)" in detail panels alongside the new "Geo length (m, drawn)". A custom `PathwayQuerySet.with_geo_length()` adds an `_geo_length` annotation that the list views (`Pathway`, `Conduit`, `AerialSpan`, `DirectBuried`, `Innerduct`, `ConduitBank`) already apply so the new sortable "Geo length (m)" table column hits PostGIS, not Python. REST and GeoJSON serializers emit `geo_length`; `PathwayFilterSet` (and the per-subclass filtersets) gain `geo_length__gte` / `geo_length__lte` URL range filters via a `GeoLengthFilterMixin`. Requires a projected, metre-based SRID (`PLUGINS_CONFIG['netbox_pathways']['srid']`) -- which is already required for the rest of the plugin's geometry support.
- **`/info` map endpoint and count-based layer gating** -- new `GET /api/plugins/pathways/geo/info/?bbox=...` returns per-layer feature counts (`structures`, `conduit_banks`, `conduits`, `aerial_spans`, `direct_buried`, `circuits`, and an `external` map for reference-mode registered layers) plus the per-layer thresholds the frontend uses to decide whether to render, client-cluster, or hide each layer. Thresholds default to `{structures: {cluster: 200, hide: 5000}, ...others: {hide: 500}}` and are overridable per-layer via `PLUGINS_CONFIG['netbox_pathways']['map_thresholds']`. The map frontend now consults `/info` on every pan/zoom and applies a single "structures clustered -> no supports" rule: whenever structures cross either threshold (client or server cluster), every pathway and reference-mode external layer is suppressed for that viewport. The hardcoded `MIN_BANK_ZOOM = 18` heuristic is removed; banks become visible whenever their viewport count is below the configured threshold. Over-budget layer toggles in the sidebar dim and display a count chip. `MapLayerRegistration` gains an optional `max_features` (default 500) for reference-mode external layers.
- **Geometry on CSV bulk import** -- `StructureImportForm` (Point) and the LineString import forms (`ConduitImportForm`, `AerialSpanImportForm`, `ConduitBankImportForm`) now expose a `location` / `path` column. Values pass through the same forgiving parser as the interactive map widget, so spreadsheets can carry GeoJSON, WKT, DMS (hemispheres optional), or Google-Maps-style decimal `lat,lon` pairs. The parser produces WGS84 and Django GIS reprojects to the configured storage SRID at save time. New helper `netbox_pathways.coord_parser.parse_geometry_input` plus `ForgivingGeometryField` are also importable by downstream code that wants the same lenient parsing.
Expand Down
12 changes: 12 additions & 0 deletions docs/user-guide/interactive-map.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,18 @@ Defaults (override in `PLUGINS_CONFIG['netbox_pathways']['map_thresholds']`):

When the structures layer crosses either threshold (client cluster or server cluster), the supporting infrastructure layers are suppressed regardless of their own counts: at that density a single highlighted line cannot be matched back to a clustered structure marker, so the whole set is hidden until you zoom in. Reference-mode external layers participate in the same gating, using their `max_features` registration value (default 500).

### How the gating performs during panning

To keep the gating logic from making the map feel laggy, the frontend uses three zoom bands:

| Zoom band | Behaviour | `/info` round-trip |
| --- | --- | --- |
| Below `MIN_DATA_ZOOM` (11) | Nothing renders | None |
| `MIN_DATA_ZOOM` to `map_skip_info_zoom` -- 1 (default 16) | Render from the most recent cached `/info` immediately; `/info` revalidates in the background with `If-None-Match`. A 304 leaves the screen untouched; a 200 only triggers a reconcile if the per-layer decision actually changes. First load with an empty cache still waits one round-trip. | Conditional, in the background |
| At or above `map_skip_info_zoom` (default 17) | Render every enabled layer directly. The viewport is too small to plausibly cross any hide/cluster threshold, so the gate is skipped. | None |

Set `PLUGINS_CONFIG['netbox_pathways']['map_skip_info_zoom']` to raise or lower the skip-info threshold for deployments with unusual feature density.

### Sidebar

Clicking any feature opens a sidebar panel with two views:
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

30 changes: 0 additions & 30 deletions netbox_pathways/static/netbox_pathways/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 49 additions & 3 deletions netbox_pathways/static/netbox_pathways/src/data-layers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ export { API_BASE };

export const MIN_DATA_ZOOM = 11;

/**
* Zoom at or above which `/info` is skipped entirely. The viewport at this
* zoom is small enough that hide/cluster thresholds are effectively
* unreachable, so blocking the render on an `/info` round-trip just adds
* latency. Overridable by server config (`PATHWAYS_CONFIG.skipInfoZoom`)
* and by callers of `chooseLoadStrategy` for tests.
*/
export const SKIP_INFO_ZOOM: number = CFG.skipInfoZoom ?? 17;

// ---------------------------------------------------------------------------
// /info endpoint: per-layer counts + thresholds for the current viewport
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -125,6 +134,22 @@ export function decideLayerRendering(info: MapInfo, enabled: Set<string>): Rende
return { clusterMode, layers };
}

/**
* Synthetic "all enabled keys render" decision for the skip-info band.
*
* At these zooms we deliberately skip `/info`, so we have no counts.
* The premise is that any viewport at that zoom holds too few features to
* cross hide/cluster thresholds in practice; if a deployment hits that
* edge case it can raise `PATHWAYS_CONFIG.skipInfoZoom`.
*/
export function decideSkipInfo(enabled: Set<string>): RenderingDecision {
const layers: Record<string, LayerDecision> = {};
for (const key of enabled) {
layers[key] = 'render';
}
return { clusterMode: 'off', layers };
}

// ---------------------------------------------------------------------------
// /info fetch helper
// ---------------------------------------------------------------------------
Expand All @@ -133,9 +158,30 @@ let _infoController: AbortController | null = null;
let _lastInfoEtag = '';
let _lastInfo: MapInfo | null = null;

/** Returns the most recent /info response, or null if none cached yet. */
export function getLastInfo(): MapInfo | null {
return _lastInfo;
}

/** Test-only: clear cached /info state so each test starts deterministically. */
export function _resetInfoCache(): void {
if (_infoController) _infoController.abort();
_infoController = null;
_lastInfoEtag = '';
_lastInfo = null;
}

/**
* Fetch `/info` with conditional revalidation.
*
* `callback(info, changed)` -- `changed` is `true` when the server returned
* fresh data (200) and `false` when it returned 304 Not Modified, in which
* case `info` is the cached value. Callers can short-circuit on `!changed`
* to avoid re-rendering when the previous decision is still valid.
*/
export async function fetchMapInfo(
bbox: string,
callback: (info: MapInfo) => void,
callback: (info: MapInfo, changed: boolean) => void,
): Promise<void> {
if (_infoController) _infoController.abort();
const controller = new AbortController();
Expand All @@ -151,14 +197,14 @@ export async function fetchMapInfo(
const response = await fetch(url, { headers, signal: controller.signal });
if (_infoController === controller) _infoController = null;
if (response.status === 304 && _lastInfo) {
callback(_lastInfo);
callback(_lastInfo, false);
return;
}
if (response.ok) {
const data = await response.json() as MapInfo;
_lastInfoEtag = response.headers.get('ETag') || '';
_lastInfo = data;
callback(data);
callback(data, true);
}
} catch {
if (_infoController === controller) _infoController = null;
Expand Down
102 changes: 102 additions & 0 deletions netbox_pathways/static/netbox_pathways/src/fetch-info.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/**
* Tests for `fetchMapInfo`'s callback contract.
*
* Callers need to distinguish "new info" (200 response, may change render
* decision) from "unchanged" (304 Not Modified, cached info is still valid)
* so they can avoid a needless re-render. The flag must be carried on the
* callback so the cost of revalidation drops to a single round-trip plus
* no DOM churn when nothing changed -- which is the common case during a
* gentle pan.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { fetchMapInfo, _resetInfoCache } from './data-layers';
import type { MapInfo } from './data-layers';

function makeInfoResponse(): MapInfo {
return {
bbox: null,
counts: {
structures: 50,
conduit_banks: 0,
conduits: 0,
aerial_spans: 0,
direct_buried: 0,
circuits: 0,
},
thresholds: {
structures: { cluster: 200, hide: 5000 },
conduit_banks: { hide: 500 },
conduits: { hide: 500 },
aerial_spans: { hide: 500 },
direct_buried: { hide: 500 },
circuits: { hide: 500 },
},
};
}

describe('fetchMapInfo signals changed vs unchanged', () => {
let fetchSpy: ReturnType<typeof vi.spyOn>;

beforeEach(() => {
_resetInfoCache();
fetchSpy = vi.spyOn(globalThis, 'fetch');
});

afterEach(() => {
fetchSpy.mockRestore();
});

it('calls callback with changed=true on a 200 response with fresh data', async () => {
const body = makeInfoResponse();
fetchSpy.mockResolvedValueOnce(
new Response(JSON.stringify(body), {
status: 200,
headers: { 'Content-Type': 'application/json', ETag: '"abc"' },
}),
);
const cb = vi.fn();
await fetchMapInfo('1,2,3,4', cb);
expect(cb).toHaveBeenCalledTimes(1);
const [info, changed] = cb.mock.calls[0];
expect(changed).toBe(true);
expect(info.counts.structures).toBe(50);
});

it('calls callback with changed=false on a 304 response, using cached info', async () => {
// First call: 200 populates the cache.
fetchSpy.mockResolvedValueOnce(
new Response(JSON.stringify(makeInfoResponse()), {
status: 200,
headers: { 'Content-Type': 'application/json', ETag: '"abc"' },
}),
);
await fetchMapInfo('1,2,3,4', vi.fn());

// Second call: 304 must reuse the cached MapInfo and signal unchanged.
fetchSpy.mockResolvedValueOnce(new Response(null, { status: 304 }));
const cb = vi.fn();
await fetchMapInfo('1,2,3,4', cb);
expect(cb).toHaveBeenCalledTimes(1);
const [info, changed] = cb.mock.calls[0];
expect(changed).toBe(false);
expect(info.counts.structures).toBe(50);
});

it('sends If-None-Match on the second call so 304 is reachable', async () => {
fetchSpy.mockResolvedValueOnce(
new Response(JSON.stringify(makeInfoResponse()), {
status: 200,
headers: { 'Content-Type': 'application/json', ETag: '"abc"' },
}),
);
await fetchMapInfo('1,2,3,4', vi.fn());

fetchSpy.mockResolvedValueOnce(new Response(null, { status: 304 }));
await fetchMapInfo('1,2,3,4', vi.fn());

const [, init] = fetchSpy.mock.calls[1];
const headers = (init as RequestInit | undefined)?.headers as Record<string, string> | undefined;
expect(headers && headers['If-None-Match']).toBe('"abc"');
});
});
Loading