Favourites: stabilize preferences and adding/removing favourites

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<string[]>
 
     constructor(
         connection: OsmConnection,
@@ -21,62 +22,78 @@ export default class FavouritesFeatureSource extends StaticFeatureSource {
         allFeatures: IndexedFeatureSource,
         layout: LayoutConfig
     ) {
-        const detectedIds = new UIEventSource<Set<string>>(undefined)
+        const features: Store<Feature[]> = Stores.ListStabilized(
-        const features: Store<Feature[]> = connection.preferencesHandler.preferences.map(
+            connection.preferencesHandler.preferences.map((prefs) => {
-            (prefs) => {
                 const feats: Feature[] = []
                 const allIds = new Set<string>()
                 for (const key in prefs) {
                     if (!key.startsWith(FavouritesFeatureSource.prefix)) {
                         continue
                     }
-                    const id = key.substring(FavouritesFeatureSource.prefix.length)
+
-                    const osmId = id.replace("-", "/")
+                    try {
-                    if (id.indexOf("-property-") > 0 || id.indexOf("-layer") > 0) {
+                        const feat = FavouritesFeatureSource.ExtractFavourite(key, prefs)
-                        continue
+                        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<string, string>): 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<string, string>,
         id: string
     ): Record<string, string> {
         const properties: Record<string, string> = {}
-        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<string>,
+        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()

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<string, UIEventSource<string>>()
     private auth: any
     private userDetails: UIEventSource<UserDetails>
@@ -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<string>()
                 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()
     }
 }

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

src/Logic/Web/MangroveReviews.ts

@@ -14,7 +14,7 @@ export class MangroveIdentity {
         const keypairEventSource = new UIEventSource<CryptoKeyPair>(undefined)
         this.keypair = keypairEventSource
         mangroveIdentity.addCallbackAndRunD(async (data) => {
-            if (data === "") {
+            if (!data) {
                 return
             }
             const keypair = await MangroveReviews.jwkToKeypair(JSON.parse(data))

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<Record<string, string>>;
   let _rotation = rotation ? tags.map(tags => rotation.GetRenderValue(tags).Subs(tags).txt) : new ImmutableStore(0);
 </script>

src/UI/Map/Icon.svelte

@@ -32,8 +32,8 @@
    */
 
   export let icon: string | undefined;
-  export let color: string | undefined;
+  export let color: string | undefined = undefined
-  export let clss: string | undefined
+  export let clss: string | undefined = undefined
 </script>
 
 {#if icon}

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) + "…"
     }
 
     /**

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: <any>{
+                                id: "circlewhiteheartred",
+                                render: "circle:white;heart:red",
+                            },
+                        },
+                    ],
+                },
+            ],
             lineRendering: [{ width: 1 }],
         }
         const sharedLayers = constructSharedLayers()