From 3ce21f61cb92f50c8ae90a97b7b090773f7f3e86 Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Thu, 23 Nov 2023 17:06:30 +0100 Subject: [PATCH] Favourites: stabilize preferences and adding/removing favourites --- .../Sources/FavouritesFeatureSource.ts | 95 +++++++++++-------- src/Logic/Osm/OsmPreferences.ts | 33 ++++++- src/Logic/State/UserRelatedState.ts | 11 ++- src/Logic/Web/MangroveReviews.ts | 2 +- src/UI/Map/DynamicMarker.svelte | 2 +- src/UI/Map/Icon.svelte | 4 +- src/Utils.ts | 6 +- .../Conversion/PrepareTheme.spec.ts | 16 +++- 8 files changed, 122 insertions(+), 47 deletions(-) diff --git a/src/Logic/FeatureSource/Sources/FavouritesFeatureSource.ts b/src/Logic/FeatureSource/Sources/FavouritesFeatureSource.ts index 4589d9493..bd6d52b4d 100644 --- a/src/Logic/FeatureSource/Sources/FavouritesFeatureSource.ts +++ b/src/Logic/FeatureSource/Sources/FavouritesFeatureSource.ts @@ -1,6 +1,6 @@ import StaticFeatureSource from "./StaticFeatureSource" import { Feature } from "geojson" -import { Store, UIEventSource } from "../../UIEventSource" +import { Store, Stores, UIEventSource } from "../../UIEventSource" import { OsmConnection } from "../../Osm/OsmConnection" import { OsmId } from "../../../Models/OsmFeature" import { GeoOperations } from "../../GeoOperations" @@ -14,6 +14,7 @@ import LayoutConfig from "../../../Models/ThemeConfig/LayoutConfig" export default class FavouritesFeatureSource extends StaticFeatureSource { public static readonly prefix = "mapcomplete-favourite-" private readonly _osmConnection: OsmConnection + private readonly _detectedIds: Store constructor( connection: OsmConnection, @@ -21,62 +22,78 @@ export default class FavouritesFeatureSource extends StaticFeatureSource { allFeatures: IndexedFeatureSource, layout: LayoutConfig ) { - const detectedIds = new UIEventSource>(undefined) - const features: Store = connection.preferencesHandler.preferences.map( - (prefs) => { + const features: Store = Stores.ListStabilized( + connection.preferencesHandler.preferences.map((prefs) => { const feats: Feature[] = [] const allIds = new Set() for (const key in prefs) { if (!key.startsWith(FavouritesFeatureSource.prefix)) { continue } - const id = key.substring(FavouritesFeatureSource.prefix.length) - const osmId = id.replace("-", "/") - if (id.indexOf("-property-") > 0 || id.indexOf("-layer") > 0) { - continue + + try { + const feat = FavouritesFeatureSource.ExtractFavourite(key, prefs) + if (!feat) { + continue + } + feats.push(feat) + allIds.add(feat.properties.id) + } catch (e) { + console.error("Could not create favourite from", key, "due to", e) } - allIds.add(osmId) - const geometry = <[number, number]>JSON.parse(prefs[key]) - const properties = FavouritesFeatureSource.getPropertiesFor(connection, id) - properties._orig_layer = prefs[FavouritesFeatureSource.prefix + id + "-layer"] - if (layout.layers.some((l) => l.id === properties._orig_layer)) { - continue - } - properties.id = osmId - properties._favourite = "yes" - feats.push({ - type: "Feature", - properties, - geometry: { - type: "Point", - coordinates: geometry, - }, - }) } - console.log("Favouritess are", feats) - detectedIds.setData(allIds) return feats - } + }) ) - super(features) + const featuresWithoutAlreadyPresent = features.map((features) => + features.filter( + (feat) => !layout.layers.some((l) => l.id === feat.properties._orig_layer) + ) + ) + + super(featuresWithoutAlreadyPresent) this._osmConnection = connection - detectedIds.addCallbackAndRunD((detected) => + this._detectedIds = Stores.ListStabilized( + features.map((feats) => feats.map((f) => f.properties.id)) + ) + this._detectedIds.addCallbackAndRunD((detected) => this.markFeatures(detected, indexedSource, allFeatures) ) // We use the indexedFeatureSource as signal to update allFeatures.features.map((_) => - this.markFeatures(detectedIds.data, indexedSource, allFeatures) + this.markFeatures(this._detectedIds.data, indexedSource, allFeatures) ) } + private static ExtractFavourite(key: string, prefs: Record): Feature { + const id = key.substring(FavouritesFeatureSource.prefix.length) + const osmId = id.replace("-", "/") + if (id.indexOf("-property-") > 0 || id.endsWith("-layer") || id.endsWith("-theme")) { + return undefined + } + const geometry = <[number, number]>JSON.parse(prefs[key]) + const properties = FavouritesFeatureSource.getPropertiesFor(prefs, id) + properties._orig_layer = prefs[FavouritesFeatureSource.prefix + id + "-layer"] + + properties.id = osmId + properties._favourite = "yes" + return { + type: "Feature", + properties, + geometry: { + type: "Point", + coordinates: geometry, + }, + } + } + private static getPropertiesFor( - osmConnection: OsmConnection, + prefs: Record, id: string ): Record { const properties: Record = {} - const prefs = osmConnection.preferencesHandler.preferences.data const minLength = FavouritesFeatureSource.prefix.length + id.length + "-property-".length for (const key in prefs) { if (key.length < minLength) { @@ -104,14 +121,18 @@ export default class FavouritesFeatureSource extends StaticFeatureSource { if (isFavourite) { const center = GeoOperations.centerpointCoordinates(feature) pref.setData(JSON.stringify(center)) + this._osmConnection.GetPreference("favourite-" + id + "-layer").setData(layer) this._osmConnection.GetPreference("favourite-" + id + "-theme").setData(theme) - for (const key in tags.data) { const pref = this._osmConnection.GetPreference( "favourite-" + id + "-property-" + key.replaceAll(":", "__") ) - pref.setData(tags.data[key]) + const v = tags.data[key] + if (v === "" || !v) { + continue + } + pref.setData("" + v) } } else { this._osmConnection.preferencesHandler.removeAllWithPrefix( @@ -129,7 +150,7 @@ export default class FavouritesFeatureSource extends StaticFeatureSource { } private markFeatures( - detected: Set, + detected: string[], featureProperties: FeaturePropertiesStore, allFeatures: IndexedFeatureSource ) { @@ -141,7 +162,7 @@ export default class FavouritesFeatureSource extends StaticFeatureSource { } const store = featureProperties.getStore(id) const origValue = store.data._favourite - if (detected.has(id)) { + if (detected.indexOf(id) >= 0) { if (origValue !== "yes") { store.data._favourite = "yes" store.ping() diff --git a/src/Logic/Osm/OsmPreferences.ts b/src/Logic/Osm/OsmPreferences.ts index e36536258..1035dec82 100644 --- a/src/Logic/Osm/OsmPreferences.ts +++ b/src/Logic/Osm/OsmPreferences.ts @@ -12,6 +12,10 @@ export class OsmPreferences { "all-osm-preferences", {} ) + /** + * A map containing the individual preference sources + * @private + */ private readonly preferenceSources = new Map>() private auth: any private userDetails: UIEventSource @@ -21,7 +25,10 @@ export class OsmPreferences { this.auth = auth this.userDetails = osmConnection.userDetails const self = this - osmConnection.OnLoggedIn(() => self.UpdatePreferences()) + osmConnection.OnLoggedIn(() => { + self.UpdatePreferences(true) + return true + }) } /** @@ -72,11 +79,19 @@ export class OsmPreferences { let i = 0 while (str !== "") { if (str === undefined || str === "undefined") { + source.setData(undefined) throw ( "Got 'undefined' or a literal string containing 'undefined' for a long preference with name " + key ) } + if (str === "undefined") { + source.setData(undefined) + throw ( + "Got a literal string containing 'undefined' for a long preference with name " + + key + ) + } if (i > 100) { throw "This long preference is getting very long... " } @@ -197,7 +212,7 @@ export class OsmPreferences { }) } - private UpdatePreferences() { + private UpdatePreferences(forceUpdate?: boolean) { const self = this this.auth.xhr( { @@ -210,11 +225,22 @@ export class OsmPreferences { return } const prefs = value.getElementsByTagName("preference") + const seenKeys = new Set() for (let i = 0; i < prefs.length; i++) { const pref = prefs[i] const k = pref.getAttribute("k") const v = pref.getAttribute("v") self.preferences.data[k] = v + seenKeys.add(k) + } + if (forceUpdate) { + for (let key in self.preferences.data) { + if (seenKeys.has(key)) { + continue + } + console.log("Deleting key", key, "as we didn't find it upstream") + delete self.preferences.data[key] + } } // We merge all the preferences: new keys are uploaded @@ -289,9 +315,10 @@ export class OsmPreferences { removeAllWithPrefix(prefix: string) { for (const key in this.preferences.data) { if (key.startsWith(prefix)) { - this.GetPreference(key, undefined, { prefix: "" }).setData(undefined) + this.GetPreference(key, "", { prefix: "" }).setData(undefined) console.log("Clearing preference", key) } } + this.preferences.ping() } } diff --git a/src/Logic/State/UserRelatedState.ts b/src/Logic/State/UserRelatedState.ts index f38f32bad..bbc7c9f8c 100644 --- a/src/Logic/State/UserRelatedState.ts +++ b/src/Logic/State/UserRelatedState.ts @@ -294,6 +294,9 @@ export default class UserRelatedState { osmConnection.preferencesHandler.preferences.addCallback((newPrefs) => { for (const k in newPrefs) { const v = newPrefs[k] + if (v === "undefined" || !v) { + continue + } if (k.endsWith("-combined-length")) { const l = Number(v) const key = k.substring(0, k.length - "length".length) @@ -308,7 +311,6 @@ export default class UserRelatedState { } amendedPrefs.ping() - console.log("Amended prefs are:", amendedPrefs.data) }) const translationMode = osmConnection.GetPreference("translation-mode") @@ -395,6 +397,13 @@ export default class UserRelatedState { } if (tags[key + "-combined-0"]) { // A combined value exists + console.log( + "Trying to get a long preference for ", + key, + "with length value", + tags[key], + "as -combined-0 exists" + ) this.osmConnection.GetLongPreference(key, "").setData(tags[key]) } else { this.osmConnection diff --git a/src/Logic/Web/MangroveReviews.ts b/src/Logic/Web/MangroveReviews.ts index 19e2228d4..8bcfe3807 100644 --- a/src/Logic/Web/MangroveReviews.ts +++ b/src/Logic/Web/MangroveReviews.ts @@ -14,7 +14,7 @@ export class MangroveIdentity { const keypairEventSource = new UIEventSource(undefined) this.keypair = keypairEventSource mangroveIdentity.addCallbackAndRunD(async (data) => { - if (data === "") { + if (!data) { return } const keypair = await MangroveReviews.jwkToKeypair(JSON.parse(data)) diff --git a/src/UI/Map/DynamicMarker.svelte b/src/UI/Map/DynamicMarker.svelte index 609fbda60..a332cda0e 100644 --- a/src/UI/Map/DynamicMarker.svelte +++ b/src/UI/Map/DynamicMarker.svelte @@ -8,7 +8,7 @@ * Renders a 'marker', which consists of multiple 'icons' */ export let marker: IconConfig[] = config?.marker; - export let rotation: TagRenderingConfig; + export let rotation: TagRenderingConfig = undefined; export let tags: Store>; let _rotation = rotation ? tags.map(tags => rotation.GetRenderValue(tags).Subs(tags).txt) : new ImmutableStore(0); diff --git a/src/UI/Map/Icon.svelte b/src/UI/Map/Icon.svelte index a6a49eb52..dc8783494 100644 --- a/src/UI/Map/Icon.svelte +++ b/src/UI/Map/Icon.svelte @@ -32,8 +32,8 @@ */ export let icon: string | undefined; - export let color: string | undefined; - export let clss: string | undefined + export let color: string | undefined = undefined + export let clss: string | undefined = undefined {#if icon} diff --git a/src/Utils.ts b/src/Utils.ts index c89e4a468..7c5c67012 100644 --- a/src/Utils.ts +++ b/src/Utils.ts @@ -301,10 +301,14 @@ In the case that MapComplete is pointed to the testing grounds, the edit will be if (str === undefined || str === null) { return undefined } + if (typeof str !== "string") { + console.error("Not a string:", str) + return undefined + } if (str.length <= l) { return str } - return str.substr(0, l - 3) + "..." + return str.substr(0, l - 1) + "…" } /** diff --git a/test/Models/ThemeConfig/Conversion/PrepareTheme.spec.ts b/test/Models/ThemeConfig/Conversion/PrepareTheme.spec.ts index f6ab8d60a..009c9f08a 100644 --- a/test/Models/ThemeConfig/Conversion/PrepareTheme.spec.ts +++ b/test/Models/ThemeConfig/Conversion/PrepareTheme.spec.ts @@ -125,7 +125,21 @@ describe("PrepareTheme", () => { en: "Test layer - please ignore", }, titleIcons: [], - pointRendering: [{ location: ["point"], label: "xyz" }], + pointRendering: [ + { + location: ["point"], + label: "xyz", + iconBadges: [ + { + if: "_favourite=yes", + then: { + id: "circlewhiteheartred", + render: "circle:white;heart:red", + }, + }, + ], + }, + ], lineRendering: [{ width: 1 }], } const sharedLayers = constructSharedLayers()