Import helper: Improve error messages of non-matching presets; fix bug if a value is 'null' in source geojson

Logic/Tags/And.ts

@@ -148,12 +148,6 @@ export class And extends TagsFilter {
         return result;
     }
 
-    AsJson() {
-        return {
-            and: this.and.map(a => a.AsJson())
-        }
-    }
-    
     optimize(): TagsFilter | boolean {
         if(this.and.length === 0){
             return true

Logic/Tags/Or.ts

@@ -85,12 +85,6 @@ export class Or extends TagsFilter {
         return result;
     }
 
-    AsJson() {
-        return {
-            or: this.or.map(o => o.AsJson())
-        }
-    }
-
     optimize(): TagsFilter | boolean {
         
         if(this.or.length === 0){

Logic/Tags/TagUtils.ts

@@ -167,6 +167,34 @@ export class TagUtils {
         return new Tag(tag[0], tag[1]);
     }
 
+    /**
+     * Returns wether or not a keys is (probably) a valid key.
+     * 
+     * // should accept common keys
+     * TagUtils.isValidKey("name") // => true
+     * TagUtils.isValidKey("image:0") // => true
+     * TagUtils.isValidKey("alt_name") // => true
+     * 
+     * // should refuse short keys
+     * TagUtils.isValidKey("x") // => false
+     * TagUtils.isValidKey("xy") // => false
+     * 
+     * // should refuse a string with >255 characters
+     * let a255 = ""
+     * for(let i = 0; i < 255; i++) { a255 += "a"; }
+     * a255.length // => 255
+     * TagUtils.isValidKey(a255) // => true
+     * TagUtils.isValidKey("a"+a255) // => false
+     * 
+     * // Should refuse unexpected characters
+     * TagUtils.isValidKey("with space") // => false
+     * TagUtils.isValidKey("some$type") // => false
+     * TagUtils.isValidKey("_name") // => false
+     */
+    public static isValidKey(key: string): boolean {
+        return key.match(/^[a-z][a-z0-9:_]{2,253}[a-z0-9]$/) !== null
+    }
+    
     /**
      * Parses a tag configuration (a json) into a TagsFilter
      * 

Logic/Tags/TagsFilter.ts

@@ -26,8 +26,6 @@ export abstract class TagsFilter {
      */
     abstract asChange(properties: any): { k: string, v: string }[]
 
-    abstract AsJson() ;
-
     /**
      * Returns an optimized version (or self) of this tagsFilter
      */

Logic/Web/QueryParameters.ts

@@ -91,8 +91,10 @@ export class QueryParameters {
 
             parts.push(encodeURIComponent(key) + "=" + encodeURIComponent(QueryParameters.knownSources[key].data))
         }
-        // Don't pollute the history every time a parameter changes
+        if(!Utils.runningFromConsole){
-        history.replaceState(null, "", "?" + parts.join("&") + Hash.Current());
+            // Don't pollute the history every time a parameter changes
+            history.replaceState(null, "", "?" + parts.join("&") + Hash.Current());
+        }
 
     }
 }

UI/ImportFlow/AskMetadata.ts

@@ -93,7 +93,6 @@ export class AskMetadata extends Combine implements FlowStep<{
                 return false;
             }
             if ([ obj.features, obj.intro, obj.wikilink, obj.source].some(v => v === undefined)){
-                console.log("Obj is", obj)
                 return false;
             }
             

UI/ImportFlow/CreateNotes.ts

@@ -30,10 +30,14 @@ export class CreateNotes extends Combine {
 
             const tags: string [] = []
             for (const key in f.properties) {
+                if (f.properties[key] === null || f.properties[key] === undefined) {
+                    console.warn("Null or undefined key for ", f.properties)
+                    continue
+                }
                 if (f.properties[key] === "") {
                     continue
                 }
-                tags.push(key + "=" + f.properties[key].replace(/=/, "\\=").replace(/;/g, "\\;").replace(/\n/g, "\\n"))
+                tags.push(key + "=" + (f.properties[key]+"").replace(/=/, "\\=").replace(/;/g, "\\;").replace(/\n/g, "\\n"))
             }
             const lat = f.geometry.coordinates[1]
             const lon = f.geometry.coordinates[0]

UI/ImportFlow/RequestFile.ts

@@ -10,6 +10,8 @@ import FileSelectorButton from "../Input/FileSelectorButton";
 import {FlowStep} from "./FlowStep";
 import {parse} from "papaparse";
 import {FixedUiElement} from "../Base/FixedUiElement";
+import {del} from "idb-keyval";
+import {TagUtils} from "../../Logic/Tags/TagUtils";
 
 class FileSelector extends InputElementMap<FileList, { name: string, contents: Promise<string> }> {
     constructor(label: BaseUIElement) {
@@ -72,6 +74,17 @@ export class RequestFile extends Combine implements FlowStep<{features: any[]}>
                 if (parsed.features.some(f => f.geometry.type != "Point")) {
                     return {error: t.errPointsOnly}
                 }
+                parsed.features.forEach(f => {
+                    const props = f.properties
+                    for (const key in props) {
+                        if(props[key] === undefined || props[key] === null || props[key] === ""){
+                            delete props[key]
+                        } 
+                    if(!TagUtils.isValidKey(key)){
+                        return {error: "Probably an invalid key: "+key}
+                    }
+                    }
+                })
                 return parsed;
 
             } catch (e) {

UI/ImportFlow/SelectTheme.ts

@@ -13,6 +13,9 @@ import {VariableUiElement} from "../Base/VariableUIElement";
 import {FixedUiElement} from "../Base/FixedUiElement";
 import Toggleable from "../Base/Toggleable";
 import {BBox} from "../../Logic/BBox";
+import BaseUIElement from "../BaseUIElement";
+import PresetConfig from "../../Models/ThemeConfig/PresetConfig";
+import List from "../Base/List";
 
 export default class SelectTheme extends Combine implements FlowStep<{
     features: any[],
@@ -29,7 +32,7 @@ export default class SelectTheme extends Combine implements FlowStep<{
     }>;
     public readonly IsValid: UIEventSource<boolean>;
 
-    constructor(params: ({ features: any[], layer: LayerConfig, bbox: BBox,  })) {
+    constructor(params: ({ features: any[], layer: LayerConfig, bbox: BBox, })) {
 
         let options: InputElement<string>[] = AllKnownLayouts.layoutsList
             .filter(th => th.layers.some(l => l.id === params.layer.id))
@@ -37,7 +40,7 @@ export default class SelectTheme extends Combine implements FlowStep<{
             .map(th => new FixedInputElement<string>(
                 new Combine([
                     new Img(th.icon).SetClass("block h-12 w-12 br-4"),
-                    new Title( th.title)
+                    new Title(th.title)
                 ]).SetClass("flex items-center"),
                 th.id))
 
@@ -47,9 +50,8 @@ export default class SelectTheme extends Combine implements FlowStep<{
         })
 
 
-
         const applicablePresets = themeRadios.GetValue().map(theme => {
-            if(theme === undefined){
+            if (theme === undefined) {
                 return []
             }
             // we get the layer with the correct ID via the actual theme config, as the actual theme might have different presets due to overrides
@@ -60,7 +62,7 @@ export default class SelectTheme extends Combine implements FlowStep<{
 
 
         const nonMatchedElements = applicablePresets.map(presets => {
-            if(presets === undefined || presets.length === 0){
+            if (presets === undefined || presets.length === 0) {
                 return undefined
             }
             return params.features.filter(feat => !presets.some(preset => new And(preset.tags).matchesProperties(feat.properties)))
@@ -71,27 +73,15 @@ export default class SelectTheme extends Combine implements FlowStep<{
             "All of the following themes will show the import notes. However, the note on OpenStreetMap can link to only one single theme. Choose which theme that the created notes will link to",
             themeRadios,
             new VariableUiElement(applicablePresets.map(applicablePresets => {
-                if(themeRadios.GetValue().data === undefined){
+                if (themeRadios.GetValue().data === undefined) {
                     return undefined
                 }
-                if(applicablePresets === undefined || applicablePresets.length === 0){
+                if (applicablePresets === undefined || applicablePresets.length === 0) {
                     return new FixedUiElement("This theme has no presets loaded. As a result, imports won't work here").SetClass("alert")
                 }
-            },[themeRadios.GetValue()])),
+            }, [themeRadios.GetValue()])),
-            new VariableUiElement(nonMatchedElements.map(unmatched => {
-                if(unmatched === undefined || unmatched.length === 0){
-                    return
-                }
-                return new Combine([new FixedUiElement(unmatched.length+" objects dont match any presets").SetClass("alert"),
-                    ...applicablePresets.data.map(preset => preset.title.txt +" needs tags "+ preset.tags.map(t => t.asHumanString()).join(" & ")),
-                    ,
-                    new Toggleable( new Title( "The following elements don't match any of the presets"),
-                        new Combine( unmatched.map(feat => JSON.stringify(feat.properties))).SetClass("flex flex-col")
-                    )
 
-                ]) .SetClass("flex flex-col")
+            new VariableUiElement(nonMatchedElements.map(unmatched => SelectTheme.nonMatchedElementsPanel(unmatched, applicablePresets.data), [applicablePresets]))
-
-            }))
         ]);
         this.SetClass("flex flex-col")
 
@@ -106,13 +96,13 @@ export default class SelectTheme extends Combine implements FlowStep<{
             if (obj === undefined) {
                 return false;
             }
-            if ([obj.theme, obj.features].some(v => v === undefined)){
+            if ([obj.theme, obj.features].some(v => v === undefined)) {
                 return false;
             }
-            if(applicablePresets.data === undefined || applicablePresets.data.length === 0){
+            if (applicablePresets.data === undefined || applicablePresets.data.length === 0) {
                 return false
             }
-            if((nonMatchedElements.data?.length??0) > 0){
+            if ((nonMatchedElements.data?.length ?? 0) > 0) {
                 return false;
             }
 
@@ -121,5 +111,60 @@ export default class SelectTheme extends Combine implements FlowStep<{
         }, [applicablePresets])
     }
 
+    private static nonMatchedElementsPanel(unmatched: any[], applicablePresets: PresetConfig[]): BaseUIElement {
+        if (unmatched === undefined || unmatched.length === 0) {
+            return
+        }
+
+        const applicablePresetsOverview = applicablePresets.map(preset => new Combine([
+            preset.title.txt, "needs tags",
+            new FixedUiElement(preset.tags.map(t => t.asHumanString()).join(" & ")).SetClass("thanks")
+        ]))
+
+        const unmatchedPanels: BaseUIElement[] = []
+        for (const feat of unmatched) {
+            const parts: BaseUIElement[] = []
+            parts.push(new Combine(Object.keys(feat.properties).map(k => 
+                k+"="+feat.properties[k]
+            )).SetClass("flex flex-col"))
+
+            for (const preset of applicablePresets) {
+                const tags = new And(preset.tags).asChange({})
+                const missing = []
+                for (const {k, v} of tags) {
+                    if (preset[k] === undefined) {
+                        missing.push(
+                            `Expected ${k}=${v}, but it is completely missing`
+                        )
+                    } else if (feat.properties[k] !== v) {
+                        missing.push(
+                            `Property with key ${k} does not have expected value ${v}; instead it is ${feat.properties}`
+                        )
+                    }
+                }
+
+                if (missing.length > 0) {
+                    parts.push(
+                        new Combine([
+                            new FixedUiElement(`Preset ${preset.title.txt} is not applicable:`),
+                            new List(missing)
+                        ]).SetClass("flex flex-col alert")
+                    )
+                }
+
+            }
+
+            unmatchedPanels.push(new Combine(parts).SetClass("flex flex-col"))
+        }
+
+        return new Combine([
+            new FixedUiElement(unmatched.length + " objects dont match any presets").SetClass("alert"),
+            ...applicablePresetsOverview,
+            new Toggleable(new Title("The following elements don't match any of the presets"),
+                new Combine(unmatchedPanels))
+        ]).SetClass("flex flex-col")
+
+    }
+
 
 }

package-lock.json

@@ -26,7 +26,7 @@
         "@types/wikidata-sdk": "^6.1.0",
         "@types/xml2js": "^0.4.9",
         "country-language": "^0.1.7",
-        "doctest-ts-improved": "^0.8.4",
+        "doctest-ts-improved": "^0.8.5",
         "email-validator": "^2.0.4",
         "escape-html": "^1.0.3",
         "geojson2svg": "^1.3.1",
@@ -6024,9 +6024,9 @@
       "integrity": "sha512-+HlytyjlPKnIG8XuRG8WvmBP8xs8P71y+SKKS6ZXWoEgLuePxtDoUEiH7WkdePWrQ5JBpE6aoVqfZfJUQkjXwA=="
     },
     "node_modules/doctest-ts-improved": {
-      "version": "0.8.4",
+      "version": "0.8.5",
-      "resolved": "https://registry.npmjs.org/doctest-ts-improved/-/doctest-ts-improved-0.8.4.tgz",
+      "resolved": "https://registry.npmjs.org/doctest-ts-improved/-/doctest-ts-improved-0.8.5.tgz",
-      "integrity": "sha512-CMVSnyDB00sLkTqHIZ20Z/kHD2XczNHwWkD4UC4retGaSfuP8XG4cnAGwkr8qoQq3mjc3w8O4w3OTWrC4HC2vA==",
+      "integrity": "sha512-4zU8fQV263CU3jAi+K7xohhT9b2ZDGw20M4O7AgzW1IoKklmNkSlHMoKZX6gqN1DAouo08R+MD5aSgACG5ILRw==",
       "dependencies": {
         "@types/chai": "^4.3.0",
         "chai": "^4.3.6",
@@ -21413,9 +21413,9 @@
       "integrity": "sha512-+HlytyjlPKnIG8XuRG8WvmBP8xs8P71y+SKKS6ZXWoEgLuePxtDoUEiH7WkdePWrQ5JBpE6aoVqfZfJUQkjXwA=="
     },
     "doctest-ts-improved": {
-      "version": "0.8.4",
+      "version": "0.8.5",
-      "resolved": "https://registry.npmjs.org/doctest-ts-improved/-/doctest-ts-improved-0.8.4.tgz",
+      "resolved": "https://registry.npmjs.org/doctest-ts-improved/-/doctest-ts-improved-0.8.5.tgz",
-      "integrity": "sha512-CMVSnyDB00sLkTqHIZ20Z/kHD2XczNHwWkD4UC4retGaSfuP8XG4cnAGwkr8qoQq3mjc3w8O4w3OTWrC4HC2vA==",
+      "integrity": "sha512-4zU8fQV263CU3jAi+K7xohhT9b2ZDGw20M4O7AgzW1IoKklmNkSlHMoKZX6gqN1DAouo08R+MD5aSgACG5ILRw==",
       "requires": {
         "@types/chai": "^4.3.0",
         "chai": "^4.3.6",

package.json

@@ -72,7 +72,7 @@
     "@types/wikidata-sdk": "^6.1.0",
     "@types/xml2js": "^0.4.9",
     "country-language": "^0.1.7",
-    "doctest-ts-improved": "^0.8.4",
+    "doctest-ts-improved": "^0.8.5",
     "email-validator": "^2.0.4",
     "escape-html": "^1.0.3",
     "geojson2svg": "^1.3.1",

tests/Chai.spec.ts

@@ -4,7 +4,7 @@ import {Utils} from "../Utils";
 
 describe("TestSuite", () => {
     
-    describe("function onder test", () => {
+    describe("function under test", () => {
         it("should work", () => {
         expect("abc").eq("abc")
         })