mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-12 14:22:21 +00:00
fix(street-level): keep explicit provider choices on filter-only updates
Review fixes on top of #1601: - Only redirect a provider the params switch on to a keyless source, so a filter-only setParams (360°, SINCE) no longer moves an explicitly chosen keyless Mapillary to KartaView. - Keyless QA gate: assert the new KartaView default and that the Mapillary chip explains its key instead of switching (it used to expect KEY REQUIRED). - Tests: Graph-proxy keyReady, filter-only updates; shared test helper. - README and CURRENT-STATE describe the keyless default. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
61dfdc771e
commit
8a7402d80c
6 files changed
+89
-64
No files matched your search
@@ -126,7 +126,7 @@ export function createStreetLevelLayer({
|
||||
});
|
||||
}
|
||||
|
||||
/** `def`, or the first keyless provider not refused when `def` has no key. */
|
||||
/** `def` if its key is ready, else the first keyless provider not refused. */
|
||||
function keylessInstead(def, refused = () => false) {
|
||||
if (def.keyReady?.() !== false) return def;
|
||||
return defs.find((d) => !d.requiresKeyId && !refused(d.id)) || def;
|
||||
@@ -594,15 +594,17 @@ export function createStreetLevelLayer({
|
||||
providerIds: state.providers.keys(),
|
||||
filter: state.filter,
|
||||
});
|
||||
// Links and stored state omit Mapillary when it is on (the codec
|
||||
// default), so a keyless install would restore a source it cannot draw.
|
||||
// A restored "Mapillary on" (the codec default) would light a source a
|
||||
// keyless install cannot draw. Only redirect a provider the params switch
|
||||
// on: a filter-only update keeps whatever the user chose.
|
||||
const asked = litByParams(decoded.providers);
|
||||
const lit =
|
||||
asked &&
|
||||
keylessInstead(
|
||||
state.providers.get(asked).def,
|
||||
(id) => decoded.providers.get(id) === false,
|
||||
).id;
|
||||
asked && decoded.providers.get(asked) === true
|
||||
? keylessInstead(
|
||||
state.providers.get(asked).def,
|
||||
(id) => decoded.providers.get(id) === false,
|
||||
).id
|
||||
: asked;
|
||||
const before = litIds();
|
||||
for (const [id, entry] of state.providers)
|
||||
if (id !== lit) switchProvider(entry, false);
|
||||
|
||||
@@ -1128,63 +1128,58 @@ test('the panel says why a lit Street View draws nothing off Google 3D', async (
|
||||
assert.match(view.meta, /Google Street View needs the Google 3D map/);
|
||||
});
|
||||
|
||||
test('without a Mapillary key the first keyless source is lit instead', async (t) => {
|
||||
const kartaViewSource = {
|
||||
getTile: async () => ({ lines: [] }),
|
||||
nearestPhotos: async () => [],
|
||||
getPhoto: async () => {
|
||||
throw new Error('KartaView has no such photo');
|
||||
},
|
||||
getSequencePhotos: async () => ({ photos: [], hasMore: false }),
|
||||
probeImage: async () => true,
|
||||
};
|
||||
const lit = (hasToken) => {
|
||||
const layer = createStreetLevelLayer({
|
||||
providers: [
|
||||
createMapillaryProvider({
|
||||
source: fakeMapillarySource({ hasToken: () => hasToken }),
|
||||
photoViewer: fakePhotoViewer(),
|
||||
}),
|
||||
createKartaViewProvider({ source: kartaViewSource }),
|
||||
],
|
||||
});
|
||||
t.after(() => layer.destroy());
|
||||
return layer
|
||||
.getUIState()
|
||||
.providers.filter((provider) => provider.on)
|
||||
.map((provider) => provider.id);
|
||||
};
|
||||
assert.deepEqual(lit(false), ['kartaview']);
|
||||
assert.deepEqual(lit(true), ['mapillary']);
|
||||
});
|
||||
|
||||
test('restoring the default "Mapillary on" without its key keeps a keyless source', (t) => {
|
||||
/** Mapillary (keyless unless `mapillary` says otherwise) plus KartaView. */
|
||||
function keylessLayer(t, mapillary = { hasToken: () => false }) {
|
||||
const layer = createStreetLevelLayer({
|
||||
providers: [
|
||||
createMapillaryProvider({
|
||||
source: fakeMapillarySource({ hasToken: () => false }),
|
||||
source: fakeMapillarySource(mapillary),
|
||||
photoViewer: fakePhotoViewer(),
|
||||
}),
|
||||
createKartaViewProvider({
|
||||
source: {
|
||||
getTile: async () => ({ lines: [] }),
|
||||
nearestPhotos: async () => [],
|
||||
getPhoto: async () => null,
|
||||
getSequencePhotos: async () => ({ photos: [], hasMore: false }),
|
||||
probeImage: async () => true,
|
||||
},
|
||||
}),
|
||||
createKartaViewProvider(),
|
||||
],
|
||||
});
|
||||
t.after(() => layer.destroy());
|
||||
const litIds = () =>
|
||||
layer
|
||||
.getUIState()
|
||||
.providers.filter((provider) => provider.on)
|
||||
.map((provider) => provider.id);
|
||||
return layer;
|
||||
}
|
||||
const litProviders = (layer) =>
|
||||
layer
|
||||
.getUIState()
|
||||
.providers.filter((provider) => provider.on)
|
||||
.map((provider) => provider.id);
|
||||
|
||||
test('without a Mapillary key the first keyless source is lit instead', (t) => {
|
||||
assert.deepEqual(litProviders(keylessLayer(t)), ['kartaview']);
|
||||
assert.deepEqual(litProviders(keylessLayer(t, {})), ['mapillary']);
|
||||
// A token-adding Graph proxy counts as a key.
|
||||
const proxied = keylessLayer(t, {
|
||||
hasToken: () => false,
|
||||
graphEndpoint: '/api/mapillary/graph',
|
||||
});
|
||||
assert.deepEqual(litProviders(proxied), ['mapillary']);
|
||||
});
|
||||
|
||||
test('restoring the default "Mapillary on" without its key keeps a keyless source', (t) => {
|
||||
const layer = keylessLayer(t);
|
||||
layer.setParams({ mapillary: true, providerSwitches: '' });
|
||||
assert.deepEqual(litIds(), ['kartaview']);
|
||||
assert.deepEqual(litProviders(layer), ['kartaview']);
|
||||
// A link that switches KartaView off keeps its word.
|
||||
layer.setParams({ mapillary: true, providerSwitches: 'kartaview-0' });
|
||||
assert.deepEqual(litIds(), ['mapillary']);
|
||||
assert.deepEqual(litProviders(layer), ['mapillary']);
|
||||
});
|
||||
|
||||
test('filter-only updates keep the lit source, even a chosen keyless Mapillary', (t) => {
|
||||
const layer = keylessLayer(t);
|
||||
layer.setParams({ pano: 'pano' });
|
||||
assert.deepEqual(litProviders(layer), ['kartaview']);
|
||||
// What the MAPILLARY chip sends, and what stored state restores.
|
||||
layer.setParams({ mapillary: true, providerSwitches: 'kartaview-0' });
|
||||
for (const params of [{ pano: 'pano' }, { sinceDays: 365 }, {}]) {
|
||||
layer.setParams(params);
|
||||
assert.deepEqual(
|
||||
litProviders(layer),
|
||||
['mapillary'],
|
||||
JSON.stringify(params),
|
||||
);
|
||||
}
|
||||
});
|
||||
@@ -74,7 +74,7 @@ export function createMapillaryProvider({ source, photoViewer = null }) {
|
||||
name: MAPILLARY_NAME,
|
||||
label: MAPILLARY_LABEL,
|
||||
requiresKeyId: MAPILLARY_KEY_ID,
|
||||
/** A browser token or a token-adding Graph proxy; else KEY REQUIRED. */
|
||||
/** A browser token, or a Graph proxy that adds one server-side. */
|
||||
keyReady: () =>
|
||||
source.hasToken?.() !== false || Boolean(source.graphEndpoint),
|
||||
pickPrefix: PICK_PREFIX.root,
|
||||
|
||||
Reference in new issue
Block a user