diff --git a/app/src/components/Map.js b/app/src/components/Map.js
index 6e4bd932..3c5ed0d5 100644
--- a/app/src/components/Map.js
+++ b/app/src/components/Map.js
@@ -1,4 +1,4 @@
-import React, {useEffect, useState} from "react";
+import React, {useEffect, useRef, useState} from "react";
import {
APIButton,
ErrorMessage,
@@ -284,11 +284,15 @@ function MapCatalogRow({item, subscribedRegions, fetchData}) {
function ManageMap() {
const [files, setFiles] = useState(null);
- const [catalog, setCatalog] = useState([]);
+ // null = pending, undefined = fetch failed, [] = no regions in the catalog.
+ const [catalog, setCatalog] = useState(null);
const [subscriptions, setSubscriptions] = useState([]);
const [allRegionsOpen, setAllRegionsOpen] = useState(false);
const {status} = React.useContext(StatusContext);
const searchBuilding = status?.flags?.map_search_building;
+ // Only the newest subscriptions request may write state: a subscribe or unsubscribe refetches
+ // while the mount request may still be in flight.
+ const subscriptionsGen = useRef(0);
const fetchFiles = async () => {
try {
@@ -303,14 +307,24 @@ function ManageMap() {
};
const fetchSubscriptionData = async () => {
+ const gen = ++subscriptionsGen.current;
try {
const data = await fetchMapSubscriptions();
+ if (gen !== subscriptionsGen.current) {
+ return;
+ }
if (data) {
setCatalog(data.catalog || []);
setSubscriptions(data.subscriptions || []);
+ } else {
+ // The api helper already toasted the non-OK response.
+ setCatalog(undefined);
}
} catch (e) {
console.error(e);
+ if (gen === subscriptionsGen.current) {
+ setCatalog(undefined);
+ }
}
};
@@ -397,6 +411,8 @@ function ManageMap() {
>;
}
+ const catalogLoaded = Array.isArray(catalog);
+
/*
* A fragment, not a `PageContainer`. `MapRoute` already provides the page chrome -- a
* `wrolpi-stack` wrapper with its own top margin -- so a second one put this page a further
@@ -419,18 +435,26 @@ function ManageMap() {
- setAllRegionsOpen(true)}>View All Regions
+ {/* A pending or failed catalog is not "zero regions": the preview waits for one. The
+ modal is mounted only then, so it receives the state array itself and its map is
+ not rebuilt by a fresh `[]` on every status tick. */}
+ setAllRegionsOpen(true)}>
+ View All Regions
+
- setAllRegionsOpen(false)}/>
+ {catalogLoaded && setAllRegionsOpen(false)}/>}
-
+ {catalog === undefined
+ ? Could not fetch map subscriptions
+ : }
How to get map files
@@ -497,17 +521,29 @@ function PinEditRow({pin, onSave, onCancel}) {
}
function MapPins() {
- const [pins, setPins] = useState([]);
+ // null = pending, undefined = fetch failed, [] = no pins. The empty row invites the user to
+ // add one, so it may only appear for a confirmed empty list.
+ const [pins, setPins] = useState(null);
const [filter, setFilter] = useState('');
const [editingId, setEditingId] = useState(null);
const navigate = useNavigate();
+ // Only the newest pins request may write state: an edit or delete refetches while the mount
+ // request may still be in flight.
+ const pinsGen = useRef(0);
const fetchPins = async () => {
+ const gen = ++pinsGen.current;
try {
const data = await getMapPins();
- if (data) setPins(data.pins || []);
+ if (gen === pinsGen.current) {
+ // The api helper returns undefined on a non-OK response.
+ setPins(data ? data.pins || [] : undefined);
+ }
} catch (e) {
console.error(e);
+ if (gen === pinsGen.current) {
+ setPins(undefined);
+ }
}
};
@@ -627,8 +663,13 @@ function MapPins() {
No pins. Right-click on the map to add one.
;
+ if (pins === undefined) {
+ return Could not fetch map pins ;
+ }
+
const lowerFilter = filter.toLowerCase();
- const filteredPins = lowerFilter
+ // Pending (null) passes through so SortableTable shows its placeholder.
+ const filteredPins = lowerFilter && pins
? pins.filter(p =>
(p.label || '').toLowerCase().includes(lowerFilter) ||
`${p.lat.toFixed(4)}, ${p.lon.toFixed(4)}`.includes(lowerFilter)
diff --git a/app/src/components/MapTables.test.js b/app/src/components/MapTables.test.js
new file mode 100644
index 00000000..efbf0242
--- /dev/null
+++ b/app/src/components/MapTables.test.js
@@ -0,0 +1,148 @@
+import React from 'react';
+import {act, screen} from '@testing-library/react';
+import {Route, Routes} from 'react-router';
+import {renderWithProviders as render} from '../test-utils';
+import {MapRoute} from './Map';
+
+// What the map's Pins and Manage tabs show while their lists are still loading, when the
+// fetch failed, and when the server confirmed the list is empty. The empty messages invite
+// the user to act ("Right-click on the map to add one"), so they may only appear for a
+// confirmed empty list.
+
+jest.mock('maplibre-gl', () => ({
+ __esModule: true,
+ default: {Map: class {
+ }, addProtocol: jest.fn(), Marker: class {
+ }},
+}));
+jest.mock('pmtiles', () => ({
+ __esModule: true, Protocol: class {
+ tile = jest.fn();
+ },
+}));
+jest.mock('protomaps-themes-base', () => ({__esModule: true, default: () => []}));
+jest.mock('./MapViewer', () => ({__esModule: true, default: () => null}));
+
+jest.mock('../api', () => ({
+ ...jest.requireActual('../api'),
+ getMapFiles: jest.fn(),
+ fetchMapSubscriptions: jest.fn(),
+ getMapPins: jest.fn(),
+}));
+
+const api = require('../api');
+
+const pending = () => new Promise(() => {
+});
+const NO_PINS = /No pins\. Right-click/;
+const NO_RESULTS = /^No results$/;
+
+// The pins page renders one table per breakpoint through `Media`, so the real media provider is
+// needed; the queries below tolerate either variant being in the tree.
+const renderAt = (route) => render(
+ }/> ,
+ {route, withMedia: true},
+);
+
+beforeEach(() => {
+ jest.clearAllMocks();
+ // The media provider reads matchMedia; the global stub's implementation is reset between
+ // tests, so give it one here. Only the computer breakpoint (1024px) matches, so the
+ // provider reports a desktop viewport and the pins page renders its desktop table.
+ window.matchMedia = jest.fn().mockImplementation(query => ({
+ matches: query.includes('1024'), media: query, onchange: null,
+ addListener: jest.fn(), removeListener: jest.fn(),
+ addEventListener: jest.fn(), removeEventListener: jest.fn(), dispatchEvent: jest.fn(),
+ }));
+ api.getMapFiles.mockResolvedValue({files: []});
+ api.fetchMapSubscriptions.mockResolvedValue({catalog: [], subscriptions: []});
+ api.getMapPins.mockResolvedValue({pins: []});
+});
+
+describe('the Pins tab', () => {
+ test('pending: shows a placeholder, not "No pins"', () => {
+ api.getMapPins.mockReturnValue(pending());
+ const {container} = renderAt('/map/pins');
+ expect(container.querySelector('.wrolpi-placeholder')).toBeInTheDocument();
+ expect(screen.queryAllByText(NO_PINS)).toHaveLength(0);
+ });
+
+ test('failed: shows an error, not "No pins"', async () => {
+ // The api helper returns undefined on a non-OK response.
+ api.getMapPins.mockResolvedValue(undefined);
+ renderAt('/map/pins');
+ await act(async () => {
+ });
+ expect(screen.getByText(/Could not fetch map pins/)).toBeInTheDocument();
+ expect(screen.queryAllByText(NO_PINS)).toHaveLength(0);
+ });
+
+ test('empty: shows "No pins"', async () => {
+ renderAt('/map/pins');
+ await act(async () => {
+ });
+ expect(screen.getAllByText(NO_PINS).length).toBeGreaterThan(0);
+ });
+
+ test('loaded: lists the pins', async () => {
+ api.getMapPins.mockResolvedValue({pins: [{id: 1, lat: 40.1, lon: -111.2, label: 'Camp', color: '#f00'}]});
+ renderAt('/map/pins');
+ await act(async () => {
+ });
+ expect(screen.getAllByText('Camp').length).toBeGreaterThan(0);
+ expect(screen.queryAllByText(NO_PINS)).toHaveLength(0);
+ });
+});
+
+describe('the Manage tab subscriptions table', () => {
+ test('pending: shows a placeholder, not "No results"', async () => {
+ api.fetchMapSubscriptions.mockReturnValue(pending());
+ const {container} = renderAt('/map/manage');
+ await act(async () => {
+ });
+ // The files table has loaded (empty); the subscriptions table is still pending.
+ expect(container.querySelector('.wrolpi-placeholder')).toBeInTheDocument();
+ expect(screen.queryByText(NO_RESULTS)).not.toBeInTheDocument();
+ });
+
+ test('failed: shows an error, not "No results"', async () => {
+ api.fetchMapSubscriptions.mockResolvedValue(undefined);
+ renderAt('/map/manage');
+ await act(async () => {
+ });
+ expect(screen.getByText(/Could not fetch map subscriptions/)).toBeInTheDocument();
+ expect(screen.queryByText(NO_RESULTS)).not.toBeInTheDocument();
+ });
+
+ test('empty: shows "No results"', async () => {
+ renderAt('/map/manage');
+ await act(async () => {
+ });
+ expect(screen.getByText(NO_RESULTS)).toBeInTheDocument();
+ });
+
+ // The "View All Regions" preview draws the catalog; a pending or failed catalog must not be
+ // previewed as zero regions.
+ test('pending: the regions preview is disabled', async () => {
+ api.fetchMapSubscriptions.mockReturnValue(pending());
+ renderAt('/map/manage');
+ await act(async () => {
+ });
+ expect(screen.getByRole('button', {name: 'View All Regions'})).toBeDisabled();
+ });
+
+ test('failed: the regions preview is disabled', async () => {
+ api.fetchMapSubscriptions.mockResolvedValue(undefined);
+ renderAt('/map/manage');
+ await act(async () => {
+ });
+ expect(screen.getByRole('button', {name: 'View All Regions'})).toBeDisabled();
+ });
+
+ test('loaded: the regions preview is enabled', async () => {
+ renderAt('/map/manage');
+ await act(async () => {
+ });
+ expect(screen.getByRole('button', {name: 'View All Regions'})).toBeEnabled();
+ });
+});