Fix bug: override preserves null's again

Models/ThemeConfig/Conversion/AddContextToTranslations.ts

@@ -38,16 +38,43 @@ export class AddContextToTranslations<T> extends DesugaringStep<T> {
      *   ]  
      * }
      * rewritten // => expected
+     * 
+     * // should preserve nulls
+     * const theme = {
+     *   layers: [
+     *       {
+     *           builtin: ["abc"],
+     *           override: {
+     *               name:null
+     *           }
+     *       }
+     *   ]  
+     * }
+     * const rewritten = new AddContextToTranslations<any>("prefix:").convert(theme, "context").result
+     * const expected = {
+     *   layers: [
+     *       {
+     *           builtin: ["abc"],
+     *           override: {
+     *               name: null
+     *           }
+     *       }
+     *   ]  
+     * }
+     * rewritten // => expected
      */
     convert(json: T, context: string): { result: T; errors?: string[]; warnings?: string[]; information?: string[] } {
 
         const result = Utils.WalkJson(json, (leaf, path) => {
+            if(leaf === undefined || leaf === null){
+                return leaf
+            }
             if (typeof leaf === "object") {
                 return {...leaf, _context: this._prefix + context + "." + path.join(".")}
             } else {
                 return leaf
             }
-        }, obj => obj !== undefined && obj !== null && Translations.isProbablyATranslation(obj))
+        }, obj => obj === undefined || obj === null || Translations.isProbablyATranslation(obj))
 
         return {
             result

Models/ThemeConfig/Conversion/Conversion.ts

@@ -158,6 +158,20 @@ export class On<P, T> extends DesugaringStep<T> {
     }
 }
 
+export class Pass<T> extends Conversion<T, T> {
+    constructor(message?: string) {
+        super(message??"Does nothing, often to swap out steps in testing", [], "Pass");
+    }
+
+
+    convert(json: T, context: string): { result: T; errors?: string[]; warnings?: string[]; information?: string[] } {
+        return {
+            result: json
+        };
+    }
+    
+}
+
 export class Concat<X, T> extends Conversion<X[], T[]> {
     private readonly _step: Conversion<X, T[]>;
 

Models/ThemeConfig/Conversion/PrepareTheme.ts

@@ -1,4 +1,4 @@
-import {Concat, Conversion, DesugaringContext, DesugaringStep, Each, Fuse, On, SetDefault} from "./Conversion";
+import {Concat, Conversion, DesugaringContext, DesugaringStep, Each, Fuse, On, Pass, SetDefault} from "./Conversion";
 import {LayoutConfigJson} from "../Json/LayoutConfigJson";
 import {PrepareLayer} from "./PrepareLayer";
 import {LayerConfigJson} from "../Json/LayerConfigJson";
@@ -13,6 +13,7 @@ import {AddContextToTranslations} from "./AddContextToTranslations";
 
 class SubstituteLayer extends Conversion<(string | LayerConfigJson), LayerConfigJson[]> {
     private readonly _state: DesugaringContext;
+
     constructor(
         state: DesugaringContext,
     ) {
@@ -24,6 +25,7 @@ class SubstituteLayer extends Conversion<(string | LayerConfigJson), LayerConfig
         const errors = []
         const information = []
         const state = this._state
+
         function reportNotFound(name: string) {
             const knownLayers = Array.from(state.sharedLayers.keys())
             const withDistance = knownLayers.map(lname => [lname, Utils.levenshteinDistance(name, lname)])
@@ -222,6 +224,7 @@ class AddImportLayers extends DesugaringStep<LayoutConfigJson> {
 
 export class AddMiniMap extends DesugaringStep<LayerConfigJson> {
     private readonly _state: DesugaringContext;
+
     constructor(state: DesugaringContext,) {
         super("Adds a default 'minimap'-element to the tagrenderings if none of the elements define such a minimap", ["tagRenderings"], "AddMiniMap");
         this._state = state;
@@ -325,6 +328,7 @@ class ApplyOverrideAll extends DesugaringStep<LayoutConfigJson> {
 
 class AddDependencyLayersToTheme extends DesugaringStep<LayoutConfigJson> {
     private readonly _state: DesugaringContext;
+
     constructor(state: DesugaringContext,) {
         super("If a layer has a dependency on another layer, these layers are added automatically on the theme. (For example: defibrillator depends on 'walls_and_buildings' to snap onto. This layer is added automatically)", ["layers"], "AddDependencyLayersToTheme");
         this._state = state;
@@ -418,6 +422,7 @@ class AddDependencyLayersToTheme extends DesugaringStep<LayoutConfigJson> {
 
 class PreparePersonalTheme extends DesugaringStep<LayoutConfigJson> {
     private readonly _state: DesugaringContext;
+
     constructor(state: DesugaringContext) {
         super("Adds every public layer to the personal theme", ["layers"], "PreparePersonalTheme");
         this._state = state;
@@ -469,7 +474,9 @@ class WarnForUnsubstitutedLayersInTheme extends DesugaringStep<LayoutConfigJson>
 }
 
 export class PrepareTheme extends Fuse<LayoutConfigJson> {
-    constructor(state: DesugaringContext) {
+    constructor(state: DesugaringContext, options?: {
+        skipDefaultLayers: false | boolean
+    }) {
         super(
             "Fully prepares and expands a theme",
             new AddContextToTransltionsInLayout(),
@@ -483,7 +490,7 @@ export class PrepareTheme extends Fuse<LayoutConfigJson> {
             new ApplyOverrideAll(),
             // And then we prepare all the layers _again_ in case that an override all contained unexpanded tagrenderings!
             new On("layers", new Each(new PrepareLayer(state))),
-            new AddDefaultLayers(state),
+            options?.skipDefaultLayers ? new Pass("AddDefaultLayers is disabled due to the set flag") : new AddDefaultLayers(state),
             new AddDependencyLayersToTheme(state),
             new AddImportLayers(),
             new On("layers", new Each(new AddMiniMap(state)))

Utils.ts

@@ -518,11 +518,35 @@ In the case that MapComplete is pointed to the testing grounds, the edit will be
      * Apply a function on every leaf of the JSON; used to rewrite parts of the JSON.
      * Returns a modified copy of the original object.
      * 
+     * 'null' and 'undefined' are _always_ considered a leaf, even if 'isLeaf' says it isn't
+     * 
      * Hangs if the object contains a loop
+     * 
+     * // should walk a json
+     * const walked = Utils.WalkJson({
+     *     key: "value"
+     * }, (x: string) => x + "!")
+     * walked // => {key: "value!"}
+     * 
+     * // should preserve undefined and null:
+     * const walked = Utils.WalkJson({
+     *   u: undefined,
+     *   n: null,
+     *   v: "value"
+     * }, (x) => {if(x !== undefined && x !== null){return x+"!}; return x})
+     * walked // => {v: "value!", u: undefined, n: null}
+     * 
+     * // should preserve undefined and null, also with a negative isLeaf:
+     * const walked = Utils.WalkJson({
+     *   u: undefined,
+     *   n: null,
+     *   v: "value"
+     * }, (x) => return x}, _ => false)
+     * walked // => {v: "value", u: undefined, n: null}
      */
     static WalkJson(json: any, f: (v: object | number | string | boolean | undefined, path: string[]) => any, isLeaf: (object) => boolean = undefined, path: string[] = []) {
-        if (json === undefined) {
+        if (json === undefined || json === null) {
-            return f(undefined, path)
+            return f(json, path)
         }
         const jtp = typeof json
         if (isLeaf !== undefined) {

assets/themes/mapcomplete-changes/mapcomplete-changes.json

@@ -1,16 +1,13 @@
 {
   "id": "mapcomplete-changes",
   "title": {
-    "en": "Changes made with MapComplete",
+    "en": "Changes made with MapComplete"
-    "de": "Änderungen mit MapComplete"
   },
   "shortDescription": {
-    "en": "Shows changes made by MapComplete",
+    "en": "Shows changes made by MapComplete"
-    "de": "Zeigt Änderungen von MapComplete"
   },
   "description": {
-    "en": "This maps shows all the changes made with MapComplete",
+    "en": "This maps shows all the changes made with MapComplete"
-    "de": "Diese Karte zeigt alle Änderungen die mit MapComplete gemacht wurden"
   },
   "maintainer": "",
   "icon": "./assets/svg/logo.svg",
@@ -39,27 +36,23 @@
       ],
       "title": {
         "render": {
-          "en": "Changeset for {theme}",
+          "en": "Changeset for {theme}"
-          "de": "Änderungen für {theme}"
         }
       },
       "description": {
-        "en": "Shows all MapComplete changes",
+        "en": "Shows all MapComplete changes"
-        "de": "Zeigt alle MapComplete Änderungen"
       },
       "tagRenderings": [
         {
           "id": "render_id",
           "render": {
-            "en": "Changeset <a href='https://openstreetmap.org/changeset/{id}' target='_blank'>{id}</a>",
+            "en": "Changeset <a href='https://openstreetmap.org/changeset/{id}' target='_blank'>{id}</a>"
-            "de": "Änderung <a href='https://openstreetmap.org/changeset/{id}' target='_blank'>{id}</a>"
           }
         },
         {
           "id": "contributor",
           "render": {
-            "en": "Change made by <a href='https://openstreetmap.org/user/{_last_edit:contributor}' target='_blank'>{_last_edit:contributor}</a>",
+            "en": "Change made by <a href='https://openstreetmap.org/user/{_last_edit:contributor}' target='_blank'>{_last_edit:contributor}</a>"
-            "de": "Änderung wurde von <a href='https://openstreetmap.org/user/{_last_edit:contributor}' target='_blank'>{_last_edit:contributor}</a> gemacht"
           }
         },
         {
@@ -335,8 +328,7 @@
                 }
               ],
               "question": {
-                "en": "Themename contains {search}",
+                "en": "Themename contains {search}"
-                "de": "Themenname enthält {search}"
               }
             }
           ]
@@ -352,8 +344,7 @@
                 }
               ],
               "question": {
-                "en": "Made by contributor {search}",
+                "en": "Made by contributor {search}"
-                "de": "Erstellt von {search}"
               }
             }
           ]
@@ -369,8 +360,7 @@
                 }
               ],
               "question": {
-                "en": "<b>Not</b> made by contributor {search}",
+                "en": "<b>Not</b> made by contributor {search}"
-                "de": "<b>Nicht</b> erstellt von {search}"
               }
             }
           ]
@@ -385,8 +375,7 @@
           {
             "id": "link_to_more",
             "render": {
-              "en": "More statistics can be found <a href='https://github.com/pietervdvn/MapComplete/tree/develop/Docs/Tools/graphs' target='_blank'>here</a>",
+              "en": "More statistics can be found <a href='https://github.com/pietervdvn/MapComplete/tree/develop/Docs/Tools/graphs' target='_blank'>here</a>"
-              "de": "Weitere Statistiken finden Sie <a href='https://github.com/pietervdvn/MapComplete/tree/develop/Docs/Tools/graphs' target='_blank'>hier</a>"
             }
           },
           {

test/Models/ThemeConfig/Conversion/PrepareTheme.spec.ts

@@ -10,6 +10,7 @@ import LayerConfig from "../../../../Models/ThemeConfig/LayerConfig";
 import {ExtractImages} from "../../../../Models/ThemeConfig/Conversion/FixImages";
 import * as cyclofix from "../../../../assets/generated/themes/cyclofix.json"
 import {Tag} from "../../../../Logic/Tags/Tag";
+import {DesugaringContext} from "../../../../Models/ThemeConfig/Conversion/Conversion";
 
 
 const themeConfigJson: LayoutConfigJson = {
@@ -69,7 +70,6 @@ describe("PrepareTheme", () => {
 
     })
 
-
     it("should apply override", () => {
 
         const sharedLayers = new Map<string, LayerConfigJson>()
@@ -82,8 +82,65 @@ describe("PrepareTheme", () => {
         const layerUnderTest = <LayerConfig> themeConfig.layers.find(l => l.id === "public_bookcase")
         expect(layerUnderTest.source.geojsonSource).eq("https://example.com/data.geojson")
     })
-})
     
+    it("should remove names which are overriden with null", () => {
+        const testLayer: LayerConfigJson = {
+            source: {
+                osmTags: "x=y"
+            },
+            id: "layer-example",
+            name: {
+                en: "Test layer - please ignore"
+            },
+            titleIcons : [],
+            mapRendering: null
+        }
+       const ctx: DesugaringContext =  {
+           sharedLayers: new Map<string, LayerConfigJson>([[
+               "layer-example", testLayer
+           ]]),
+           tagRenderings: new Map<string, TagRenderingConfigJson>()
+
+       }
+       const layout : LayoutConfigJson=
+           {
+               description: "A testing theme",
+               icon: "",
+               id: "",
+               layers: [
+                   "layer-example",
+                   {
+                       "builtin": "layer-example",
+                       "override": {
+                           "name": null,
+                           "minzoom": 18
+                       }
+                   }
+               ],
+               maintainer: "Me",
+               startLat: 0,
+               startLon: 0,
+               startZoom: 0,
+               title: "Test theme",
+               version: ""
+
+           }
+        const rewritten = new PrepareTheme(ctx, {
+            skipDefaultLayers: true
+        }).convertStrict(layout, "test")
+        expect(rewritten.layers[0]).deep.eq(testLayer)
+        expect(rewritten.layers[1]).deep.eq({
+            source: {
+                osmTags: "x=y"
+            },
+            id: "layer-example",
+            name:null,
+            minzoom: 18,
+            mapRendering: null,
+            titleIcons: []
+        })
+    })
+})
 
 describe("ExtractImages", () => {
     it("should find all images in a themefile", () => {