From 0cbfa325f6bc3dd39205a2290feb1342b81d09ba Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Thu, 24 Jul 2025 14:41:43 +0200 Subject: [PATCH 1/3] Feature(search): add error message if search failed --- src/Logic/Search/CombinedSearcher.ts | 14 +++-- src/Logic/Search/CoordinateSearch.ts | 4 +- src/Logic/Search/GeocodingProvider.ts | 8 +-- src/Logic/Search/LocalElementSearch.ts | 4 +- src/Logic/Search/NominatimGeocoding.ts | 5 ++ src/Logic/Search/OpenLocationCodeSearch.ts | 32 ++++++---- src/Logic/Search/OpenStreetMapIdSearch.ts | 4 +- src/Logic/Search/PhotonSearch.ts | 8 ++- src/Logic/State/SearchState.ts | 69 +++++++++++++++++----- src/Logic/UIEventSource.ts | 9 +-- src/UI/Search/GeocodeResults.svelte | 31 ++++++++++ 11 files changed, 138 insertions(+), 50 deletions(-) diff --git a/src/Logic/Search/CombinedSearcher.ts b/src/Logic/Search/CombinedSearcher.ts index 10bf7c2f9b..261826c7df 100644 --- a/src/Logic/Search/CombinedSearcher.ts +++ b/src/Logic/Search/CombinedSearcher.ts @@ -7,6 +7,11 @@ export default class CombinedSearcher implements GeocodingProvider { private _providers: ReadonlyArray private _providersWithSuggest: ReadonlyArray + /** + * Merges the various providers together; ignores errors. + * IF all providers fail, no errors will be given + * @param providers + */ constructor(...providers: ReadonlyArray) { this._providers = Utils.NoNull(providers) this._providersWithSuggest = this._providers.filter((pr) => pr.suggest !== undefined) @@ -45,9 +50,10 @@ export default class CombinedSearcher implements GeocodingProvider { return CombinedSearcher.merge(results) } - suggest(query: string, options?: GeocodingOptions): Store { - return Stores.concat( - this._providersWithSuggest.map((pr) => pr.suggest(query, options)) - ).map((gcrss) => CombinedSearcher.merge(gcrss)) + suggest(query: string, options?: GeocodingOptions): Store<{success: GeocodeResult[]}> { + const concatted = Stores.concat( + this._providersWithSuggest.map((pr) => > pr.suggest(query, options).map(result => result["success"] ?? [])) + ); + return concatted.map(gcrss => ({success: CombinedSearcher.merge(gcrss) })) } } diff --git a/src/Logic/Search/CoordinateSearch.ts b/src/Logic/Search/CoordinateSearch.ts index c2c3c1b57b..de74239c19 100644 --- a/src/Logic/Search/CoordinateSearch.ts +++ b/src/Logic/Search/CoordinateSearch.ts @@ -119,8 +119,8 @@ export default class CoordinateSearch implements GeocodingProvider { } } - suggest(query: string): Store { - return new ImmutableStore(this.directSearch(query)) + suggest(query: string): Store<{success: GeocodeResult[]}> { + return new ImmutableStore({success: this.directSearch(query)}) } async search(query: string): Promise { diff --git a/src/Logic/Search/GeocodingProvider.ts b/src/Logic/Search/GeocodingProvider.ts index 0ea017a479..9ec66c40a5 100644 --- a/src/Logic/Search/GeocodingProvider.ts +++ b/src/Logic/Search/GeocodingProvider.ts @@ -57,11 +57,7 @@ export default interface GeocodingProvider { */ search(query: string, options?: GeocodingOptions): Promise - /** - * @param query - * @param options - */ - suggest?(query: string, options?: GeocodingOptions): Store + suggest(query: string, options?: GeocodingOptions): Store<{success: GeocodeResult[]} | {error :any}> } export type ReverseGeocodingResult = Feature< @@ -93,7 +89,7 @@ export class GeocodingUtils { // We are resetting the layeroverview; trying to parse is useless return undefined } - return new LayerConfig(search, "search") + return new LayerConfig( search, "search") } public static categoryToZoomLevel: Record = { diff --git a/src/Logic/Search/LocalElementSearch.ts b/src/Logic/Search/LocalElementSearch.ts index 9db1de6542..b96741eef8 100644 --- a/src/Logic/Search/LocalElementSearch.ts +++ b/src/Logic/Search/LocalElementSearch.ts @@ -141,7 +141,7 @@ export default class LocalElementSearch implements GeocodingProvider { }) } - suggest(query: string, options?: GeocodingOptions): Store { - return this.searchEntries(query, options, true) + suggest(query: string, options?: GeocodingOptions): Store<{success: GeocodeResult[]}> { + return this.searchEntries(query, options, true).mapD(r => ({success:r})) } } diff --git a/src/Logic/Search/NominatimGeocoding.ts b/src/Logic/Search/NominatimGeocoding.ts index 9371fab457..0877be8d9b 100644 --- a/src/Logic/Search/NominatimGeocoding.ts +++ b/src/Logic/Search/NominatimGeocoding.ts @@ -4,6 +4,7 @@ import Constants from "../../Models/Constants" import { FeatureCollection } from "geojson" import Locale from "../../UI/i18n/Locale" import GeocodingProvider, { GeocodeResult, GeocodingOptions } from "./GeocodingProvider" +import { Store, UIEventSource } from "../UIEventSource" export class NominatimGeocoding implements GeocodingProvider { private readonly _host @@ -37,4 +38,8 @@ export class NominatimGeocoding implements GeocodingProvider { }&zoom=${Math.ceil(zoom) + 1}&accept-language=${language}` return Utils.downloadJson(url) } + + suggest(query: string, options?: GeocodingOptions): Store<{ success: GeocodeResult[] } | { error: any }> { + return UIEventSource.FromPromiseWithErr(this.search(query, options)) + } } diff --git a/src/Logic/Search/OpenLocationCodeSearch.ts b/src/Logic/Search/OpenLocationCodeSearch.ts index e294eaff32..6081950f8c 100644 --- a/src/Logic/Search/OpenLocationCodeSearch.ts +++ b/src/Logic/Search/OpenLocationCodeSearch.ts @@ -1,4 +1,4 @@ -import { Store, Stores } from "../UIEventSource" +import { ImmutableStore, Store } from "../UIEventSource" import GeocodingProvider, { GeocodeResult, GeocodingOptions } from "./GeocodingProvider" import { decode as pluscode_decode } from "pluscodes" @@ -24,24 +24,30 @@ export default class OpenLocationCodeSearch implements GeocodingProvider { return str.toUpperCase().match(this._isPlusCode) !== null } + private searchDirectly(query: string): GeocodeResult | undefined { + if (!OpenLocationCodeSearch.isPlusCode(query)) { + return undefined + } + const { latitude, longitude } = pluscode_decode(query) + return { + lon: longitude, + lat: latitude, + description: "Open Location Code", + osm_id: query, + display_name: query.toUpperCase(), + } + } + async search(query: string, options?: GeocodingOptions): Promise { if (!OpenLocationCodeSearch.isPlusCode(query)) { return [] // Must be an empty list and not "undefined", the latter is interpreted as 'still searching' } - const { latitude, longitude } = pluscode_decode(query) - return [ - { - lon: longitude, - lat: latitude, - description: "Open Location Code", - osm_id: query, - display_name: query.toUpperCase(), - }, - ] + return [this.searchDirectly(query)] } - suggest?(query: string, options?: GeocodingOptions): Store { - return Stores.FromPromise(this.search(query, options)) + suggest(query: string, options?: GeocodingOptions): Store<{ success: GeocodeResult[] }> { + const result = OpenLocationCodeSearch.isPlusCode(query) ? [this.searchDirectly(query)] : [] + return new ImmutableStore({ success: result }) } } diff --git a/src/Logic/Search/OpenStreetMapIdSearch.ts b/src/Logic/Search/OpenStreetMapIdSearch.ts index 94f7607cda..75f3ee3732 100644 --- a/src/Logic/Search/OpenStreetMapIdSearch.ts +++ b/src/Logic/Search/OpenStreetMapIdSearch.ts @@ -92,7 +92,7 @@ export default class OpenStreetMapIdSearch implements GeocodingProvider { return [await this.getInfoAbout(id)] } - suggest?(query: string, options?: GeocodingOptions): Store { - return UIEventSource.FromPromise(this.search(query, options)) + suggest(query: string, options?: GeocodingOptions): Store<{success: GeocodeResult[]} | {error: any}> { + return UIEventSource.FromPromiseWithErr(this.search(query, options)) } } diff --git a/src/Logic/Search/PhotonSearch.ts b/src/Logic/Search/PhotonSearch.ts index da93b4eb12..6745b8912f 100644 --- a/src/Logic/Search/PhotonSearch.ts +++ b/src/Logic/Search/PhotonSearch.ts @@ -36,6 +36,10 @@ export default class PhotonSearch implements GeocodingProvider, ReverseGeocoding this.suggestionLimit = suggestionLimit this.searchLimit = searchLimit this._endpoint = endpoint ?? Constants.photonEndpoint ?? "https://photon.komoot.io/" + + if(this.ignoreBounds){ + this.name += " (global)" + } } async reverseSearch( @@ -69,8 +73,8 @@ export default class PhotonSearch implements GeocodingProvider, ReverseGeocoding return `&lang=${language}` } - suggest(query: string, options?: GeocodingOptions): Store { - return Stores.FromPromise(this.search(query, options)) + suggest(query: string, options?: GeocodingOptions): Store<{success: GeocodeResult[]} | {error: any}> { + return Stores.FromPromiseWithErr(this.search(query, options)) } private buildDescription(entry: Feature) { diff --git a/src/Logic/State/SearchState.ts b/src/Logic/State/SearchState.ts index 4a45a892fa..44eaccc3fb 100644 --- a/src/Logic/State/SearchState.ts +++ b/src/Logic/State/SearchState.ts @@ -1,5 +1,5 @@ import GeocodingProvider, { GeocodeResult, GeocodingUtils } from "../Search/GeocodingProvider" -import { ImmutableStore, Store, Stores, UIEventSource } from "../UIEventSource" +import { Store, Stores, UIEventSource } from "../UIEventSource" import CombinedSearcher from "../Search/CombinedSearcher" import FilterSearch, { FilterSearchResult } from "../Search/FilterSearch" import LocalElementSearch from "../Search/LocalElementSearch" @@ -18,6 +18,8 @@ import { Feature } from "geojson" import OpenLocationCodeSearch from "../Search/OpenLocationCodeSearch" import { BBox } from "../BBox" import { QueryParameters } from "../Web/QueryParameters" +import { Utils } from "../../Utils" +import { NominatimGeocoding } from "../Search/NominatimGeocoding" export default class SearchState { public readonly feedback: UIEventSource = new UIEventSource(undefined) @@ -32,7 +34,12 @@ export default class SearchState { private readonly state: ThemeViewState public readonly showSearchDrawer: UIEventSource public readonly suggestionsSearchRunning: Store + public readonly runningEngines: Store public readonly locationResults: FeatureSource + /** + * Indicates failures in the current search + */ + public readonly failedEngines: Store<{ source: GeocodingProvider; error: any }[]> constructor(state: ThemeViewState) { this.state = state @@ -44,31 +51,63 @@ export default class SearchState { new CoordinateSearch(), new OpenLocationCodeSearch(), new OpenStreetMapIdSearch(state.osmObjectDownloader), - new PhotonSearch(true, 2), - new PhotonSearch(), - // new NominatimGeocoding(), + new PhotonSearch(true, 2), // global results + new PhotonSearch(), // local results + new NominatimGeocoding(), ] const bounds = state.mapProperties.bounds - const suggestionsList = this.searchTerm.stabilized(250).mapD( + const suggestionsListWithSource = this.searchTerm.stabilized(250).mapD( (search) => { if (search.length === 0) { return undefined } - return this.locationSearchers.map((ls) => ls.suggest(search, { bbox: bounds.data })) + return this.locationSearchers.map((ls) => ({ + source: ls, + results: ls.suggest(search, { bbox: bounds.data }), + })) }, [bounds] ) - this.suggestionsSearchRunning = suggestionsList.bind((suggestions) => { - if (suggestions === undefined) { - return new ImmutableStore(true) - } - return Stores.concat(suggestions).map((suggestions) => - suggestions.some((list) => list === undefined) - ) - }) + const suggestionsList = suggestionsListWithSource + .mapD(list => list.map(sugg => sugg.results)) + + const isRunningPerEngine: Store[]> = + suggestionsListWithSource.map( + allProviders => allProviders.map(provider => + provider.results.map(result => { + if (result === undefined) { + return provider.source + } else { + return undefined + } + }))) + this.runningEngines = isRunningPerEngine.bindD( + listOfSources => Stores.concat(listOfSources).mapD(list => Utils.NoNull(list))) + + + this.failedEngines = suggestionsListWithSource + .bindD((allProviders: { + source: GeocodingProvider; + results: Store<{ success: GeocodeResult[] } | { error: any }> + }[]) => Stores.concat( + allProviders.map(providerAndResult => + >providerAndResult.results.map(result => { + let error = result?.["error"] + if (error) { + return [{ + source: providerAndResult.source, error, + }] + } else { + return [] + } + }), + ))).map(list => Utils.NoNull(list.flatMap(x => x))) + + this.suggestionsSearchRunning = this.runningEngines.map(running => running?.length > 0) this.suggestions = suggestionsList.bindD((suggestions) => - Stores.concat(suggestions).map((suggestions) => CombinedSearcher.merge(suggestions)) + Stores.concat(suggestions.map(sugg => sugg.map(maybe => maybe?.["success"]))) + .map((suggestions: GeocodeResult[][]) => CombinedSearcher.merge(suggestions)) ) const themeSearch = ThemeSearchIndex.fromState(state) diff --git a/src/Logic/UIEventSource.ts b/src/Logic/UIEventSource.ts index c3c83f887e..c9f5f2f3d0 100644 --- a/src/Logic/UIEventSource.ts +++ b/src/Logic/UIEventSource.ts @@ -45,15 +45,16 @@ export class Stores { ?.then((d) => src.setData(d)) return src } - - public static concat(stores: Store[]): Store<(T[] | undefined)[]> { - const newStore = new UIEventSource<(T[] | undefined)[]>([]) + public static concat(stores: Store[]): Store<(T | undefined)[]> ; + public static concat(stores: Store[]): Store ; + public static concat(stores: Store[]): Store<(T | undefined)[]> { + const newStore = new UIEventSource<(T | undefined)[]>([]) function update() { if (newStore._callbacks.isDestroyed) { return true // unregister } - const results: (T[] | undefined)[] = [] + const results: (T | undefined)[] = [] for (const store of stores) { results.push(store.data) } diff --git a/src/UI/Search/GeocodeResults.svelte b/src/UI/Search/GeocodeResults.svelte index fcbf7f0181..0f2e4edbce 100644 --- a/src/UI/Search/GeocodeResults.svelte +++ b/src/UI/Search/GeocodeResults.svelte @@ -8,22 +8,36 @@ import { default as GeocodeResultSvelte } from "./GeocodeResult.svelte" import Tr from "../Base/Tr.svelte" import { ImmutableStore, Store, UIEventSource } from "../../Logic/UIEventSource" + import type GeocodingProvider from "../../Logic/Search/GeocodingProvider" import type { GeocodeResult } from "../../Logic/Search/GeocodingProvider" import type { MapProperties } from "../../Models/MapProperties" + import { ExclamationTriangle } from "@babeard/svelte-heroicons/solid/ExclamationTriangle" export let state: { searchState: { searchTerm: UIEventSource suggestions: Store suggestionsSearchRunning: Store + runningEngines: Store + failedEngines: Store<{ + source: GeocodingProvider; + error: any + }[]> } + featureSwitchIsTesting?: Store + userRelatedState?: { showTagsB: Store } mapProperties: MapProperties } let searchTerm = state.searchState.searchTerm let results = state.searchState.suggestions let isSearching = state.searchState.suggestionsSearchRunning ?? new ImmutableStore(false) + let runningEngines = state.searchState.runningEngines ?? new ImmutableStore([]) + let failedEngines = state.searchState.failedEngines + + let isTesting = state.featureSwitchIsTesting ?? new ImmutableStore(false) + let showTags = state.userRelatedState?.showTagsB ?? new ImmutableStore(false) const t = Translations.t.general.search @@ -43,6 +57,11 @@
+ {#if $isTesting || $showTags} +
+ Querying {$runningEngines?.map(provider => provider.name)?.join(", ")} +
+ {/if}
{/if} @@ -52,6 +71,18 @@ " + $searchTerm + "" })} /> {/if} + {#if $failedEngines?.length > 0} + {#each $failedEngines as failed} +
+ + Search using {failed.source.name} failed +
+ {failed.error} +
+
+ {/each} + {/if} + {:else} From feb0d47aece897d0d2e035f786c0201546b57bd2 Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Thu, 24 Jul 2025 19:30:22 +0200 Subject: [PATCH 2/3] Fix: crash in search code --- src/Logic/State/SearchState.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Logic/State/SearchState.ts b/src/Logic/State/SearchState.ts index 44eaccc3fb..31b75a7b33 100644 --- a/src/Logic/State/SearchState.ts +++ b/src/Logic/State/SearchState.ts @@ -73,7 +73,7 @@ export default class SearchState { .mapD(list => list.map(sugg => sugg.results)) const isRunningPerEngine: Store[]> = - suggestionsListWithSource.map( + suggestionsListWithSource.mapD( allProviders => allProviders.map(provider => provider.results.map(result => { if (result === undefined) { @@ -102,7 +102,7 @@ export default class SearchState { return [] } }), - ))).map(list => Utils.NoNull(list.flatMap(x => x))) + ))).map(list => Utils.NoNull(list?.flatMap(x => x) ?? [])) this.suggestionsSearchRunning = this.runningEngines.map(running => running?.length > 0) this.suggestions = suggestionsList.bindD((suggestions) => From 4c0c8ae9ea66de89e1f9b89a565c685a3bb23ead Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Thu, 24 Jul 2025 19:46:39 +0200 Subject: [PATCH 3/3] Feature(reviews): add loading screen --- src/Logic/Web/MangroveReviews.ts | 21 ++++++++++++--------- src/UI/Reviews/AllReviews.svelte | 13 +++++++++---- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/Logic/Web/MangroveReviews.ts b/src/Logic/Web/MangroveReviews.ts index 2e51e37e9c..e71fb7b5ad 100644 --- a/src/Logic/Web/MangroveReviews.ts +++ b/src/Logic/Web/MangroveReviews.ts @@ -149,7 +149,7 @@ export default class FeatureReviews { public readonly average: Store private readonly _reviews: UIEventSource< (Review & { kid: string; signature: string; madeByLoggedInUser: Store })[] - > = new UIEventSource([]) + > = new UIEventSource(undefined) public readonly reviews: Store<(Review & { signature: string, madeByLoggedInUser: Store })[]> = this._reviews private readonly _lat: number @@ -159,11 +159,6 @@ export default class FeatureReviews { private readonly _identity: MangroveIdentity private readonly _testmode: Store public readonly loadingAllowed: UIEventSource - private readonly _options: Readonly<{ - nameKey?: "name" | string - fallbackName?: string - uncertaintyRadius?: number - }> private readonly _reportError: (msg: string, extra: string) => Promise private constructor( @@ -186,7 +181,6 @@ export default class FeatureReviews { this._identity = mangroveIdentity this._testmode = testmode ?? new ImmutableStore(false) const nameKey = options?.nameKey ?? "name" - this._options = options if (options.uncertaintyRadius) { this._uncertainty = options.uncertaintyRadius } else if (feature.geometry.type === "Point") { @@ -383,11 +377,18 @@ export default class FeatureReviews { signature: "", madeByLoggedInUser: new ImmutableStore(true), } + this.initReviews() this._reviews.data.push(reviewWithKid) this._reviews.ping() this._identity.addReview(reviewWithKid) } + private initReviews(){ + if(this._reviews.data === undefined){ + this._reviews.set([]) + } + } + /** * Adds given reviews to the 'reviews'-UI-eventsource, if they match close enough. * We assume only geo-reviews are passed in (as they should be queried using the 'geo'-part) @@ -398,9 +399,11 @@ export default class FeatureReviews { reviews: { payload: Review; kid: string; signature: string }[], expectedName: string ) { - const alreadyKnown = new Set(this._reviews.data.map((r) => r.rating + " " + r.opinion)) + const alreadyKnown = new Set(this._reviews.data?.map((r) => r.rating + " " + r.opinion)) let hasNew = false + this.initReviews() + for (const reviewData of reviews) { const review = reviewData.payload @@ -457,7 +460,7 @@ export default class FeatureReviews { public removeReviewLocally(review: Review){ this._reviews.set( - this._reviews.data.filter(r => r !== review) + this._reviews.data?.filter(r => r !== review) ) } diff --git a/src/UI/Reviews/AllReviews.svelte b/src/UI/Reviews/AllReviews.svelte index 54fe943a64..2a0267edc9 100644 --- a/src/UI/Reviews/AllReviews.svelte +++ b/src/UI/Reviews/AllReviews.svelte @@ -11,6 +11,7 @@ import { Store, UIEventSource } from "../../Logic/UIEventSource" import type { Feature } from "geojson" import LayerConfig from "../../Models/ThemeConfig/LayerConfig" + import Loading from "../Base/Loading.svelte" /** * An element showing all reviews */ @@ -33,22 +34,26 @@
- {#if $allReviews.length > 1} + {#if $allReviews?.length > 1} {/if} - {#if $allReviews.length > 0} + {#if !$allReviews} +
+ +
+ {:else if $allReviews.length > 0} {#each $allReviews as review} {/each} {:else} -
+
{/if}