Studio: improve error handling, fix renumbering

src/Models/ThemeConfig/Conversion/Conversion.ts

@@ -2,7 +2,6 @@ import { LayerConfigJson } from "../Json/LayerConfigJson"
 import { Utils } from "../../../Utils"
 import { QuestionableTagRenderingConfigJson } from "../Json/QuestionableTagRenderingConfigJson"
 import { ConversionContext } from "./ConversionContext"
-import { T } from "vitest/dist/types-aac763a5"
 
 export interface DesugaringContext {
     tagRenderings: Map<string, QuestionableTagRenderingConfigJson>
@@ -11,10 +10,11 @@ export interface DesugaringContext {
 }
 
 export type ConversionMsgLevel = "debug" | "information" | "warning" | "error"
+
 export interface ConversionMessage {
-    context: ConversionContext
+    readonly context: ConversionContext
-    message: string
+    readonly message: string
-    level: ConversionMsgLevel
+    readonly level: ConversionMsgLevel
 }
 
 export abstract class Conversion<TIn, TOut> {
@@ -85,6 +85,7 @@ export class Pure<TIn, TOut> extends Conversion<TIn, TOut> {
 export class Bypass<T> extends DesugaringStep<T> {
     private readonly _applyIf: (t: T) => boolean
     private readonly _step: DesugaringStep<T>
+
     constructor(applyIf: (t: T) => boolean, step: DesugaringStep<T>) {
         super("Applies the step on the object, if the object satisfies the predicate", [], "Bypass")
         this._applyIf = applyIf
@@ -102,7 +103,6 @@ export class Bypass<T> extends DesugaringStep<T> {
 export class Each<X, Y> extends Conversion<X[], Y[]> {
     private readonly _step: Conversion<X, Y>
     private readonly _msg: string
-    private readonly _filter: (x: X) => boolean
 
     constructor(step: Conversion<X, Y>, options?: { msg?: string }) {
         super(
@@ -224,6 +224,7 @@ export class FirstOf<T, X> extends Conversion<T, X> {
 export class Cached<TIn, TOut> extends Conversion<TIn, TOut> {
     private _step: Conversion<TIn, TOut>
     private readonly key: string
+
     constructor(step: Conversion<TIn, TOut>) {
         super("Secretly caches the output for the given input", [], "cached")
         this._step = step
@@ -242,9 +243,11 @@ export class Cached<TIn, TOut> extends Conversion<TIn, TOut> {
         return converted
     }
 }
+
 export class Fuse<T> extends DesugaringStep<T> {
-    private readonly steps: DesugaringStep<T>[]
     protected debug = false
+    private readonly steps: DesugaringStep<T>[]
+
     constructor(doc: string, ...steps: DesugaringStep<T>[]) {
         super(
             (doc ?? "") +

src/Models/ThemeConfig/Conversion/ConversionContext.ts

@@ -1,4 +1,5 @@
 import { ConversionMessage, ConversionMsgLevel } from "./Conversion"
+import { Context } from "maplibre-gl"
 
 export class ConversionContext {
     /**
@@ -42,6 +43,31 @@ export class ConversionContext {
         return new ConversionContext([], msg ? [msg] : [], ["test"])
     }
 
+    /**
+     * Does an inline edit of the messages for which a new path is defined
+     * This is a slight hack
+     * @param rewritePath
+     */
+    public rewriteMessages(
+        rewritePath: (
+            p: ReadonlyArray<number | string>
+        ) => undefined | ReadonlyArray<number | string>
+    ): void {
+        for (let i = 0; i < this.messages.length; i++) {
+            const m = this.messages[i]
+            const newPath = rewritePath(m.context.path)
+            if (!newPath) {
+                continue
+            }
+            const rewrittenContext = new ConversionContext(
+                this.messages,
+                newPath,
+                m.context.operation
+            )
+            this.messages[i] = <ConversionMessage>{ ...m, context: rewrittenContext }
+        }
+    }
+
     static print(msg: ConversionMessage) {
         const noString = msg.context.path.filter(
             (p) => typeof p !== "string" && typeof p !== "number"

src/Models/ThemeConfig/Conversion/Validation.ts

@@ -13,7 +13,10 @@ import { And } from "../../../Logic/Tags/And"
 import Translations from "../../../UI/i18n/Translations"
 import FilterConfigJson from "../Json/FilterConfigJson"
 import DeleteConfig from "../DeleteConfig"
-import { QuestionableTagRenderingConfigJson } from "../Json/QuestionableTagRenderingConfigJson"
+import {
+    MappingConfigJson,
+    QuestionableTagRenderingConfigJson,
+} from "../Json/QuestionableTagRenderingConfigJson"
 import Validators from "../../../UI/InputElement/Validators"
 import TagRenderingConfig from "../TagRenderingConfig"
 import { parse as parse_html } from "node-html-parser"
@@ -21,9 +24,7 @@ import PresetConfig from "../PresetConfig"
 import { TagsFilter } from "../../../Logic/Tags/TagsFilter"
 import { Translatable } from "../Json/Translatable"
 import { ConversionContext } from "./ConversionContext"
-import * as eli from "../../../assets/editor-layer-index.json"
 import { AvailableRasterLayers } from "../../RasterLayers"
-import Back from "../../../assets/svg/Back.svelte"
 import PointRenderingConfigJson from "../Json/PointRenderingConfigJson"
 
 class ValidateLanguageCompleteness extends DesugaringStep<LayoutConfig> {
@@ -831,6 +832,7 @@ class MiscTagRenderingChecks extends DesugaringStep<TagRenderingConfigJson> {
         json: TagRenderingConfigJson | QuestionableTagRenderingConfigJson,
         context: ConversionContext
     ): TagRenderingConfigJson {
+        console.log(">>> Validating TR", context.path.join("."), json)
         if (json["special"] !== undefined) {
             context.err(
                 'Detected `special` on the top level. Did you mean `{"render":{ "special": ... }}`'
@@ -848,13 +850,32 @@ class MiscTagRenderingChecks extends DesugaringStep<TagRenderingConfigJson> {
                 CheckTranslation.allowUndefined.convert(json[key], context.enter(key))
             }
             for (let i = 0; i < json.mappings?.length ?? 0; i++) {
-                const mapping = json.mappings[i]
+                const mapping: MappingConfigJson = json.mappings[i]
                 CheckTranslation.noUndefined.convert(
                     mapping.then,
                     context.enters("mappings", i, "then")
                 )
                 if (!mapping.if) {
-                    context.enters("mappings", i).err("No `if` is defined")
+                    console.log(
+                        "Checking mappings",
+                        i,
+                        "if",
+                        mapping.if,
+                        context.path.join("."),
+                        mapping.then
+                    )
+                    context.enters("mappings", i, "if").err("No `if` is defined")
+                }
+                if (mapping.addExtraTags) {
+                    for (let j = 0; j < mapping.addExtraTags.length; j++) {
+                        if (!mapping.addExtraTags[j]) {
+                            context
+                                .enters("mappings", i, "addExtraTags", j)
+                                .err(
+                                    "Detected a 'null' or 'undefined' value. Either specify a tag or delete this item"
+                                )
+                        }
+                    }
                 }
                 const en = mapping?.then?.["en"]
                 if (en && this.detectYesOrNo(en)) {
@@ -977,6 +998,9 @@ class MiscTagRenderingChecks extends DesugaringStep<TagRenderingConfigJson> {
             }
         }
 
+        if (context.hasErrors()) {
+            return undefined
+        }
         return json
     }
 
@@ -996,6 +1020,7 @@ export class ValidateTagRenderings extends Fuse<TagRenderingConfigJson> {
     constructor(layerConfig?: LayerConfigJson, doesImageExist?: DoesImageExist) {
         super(
             "Various validation on tagRenderingConfigs",
+            new MiscTagRenderingChecks(),
             new DetectShadowedMappings(layerConfig),
             new DetectConflictingAddExtraTags(),
             // TODO enable   new DetectNonErasedKeysInMappings(),
@@ -1003,8 +1028,7 @@ export class ValidateTagRenderings extends Fuse<TagRenderingConfigJson> {
             new On("render", new ValidatePossibleLinks()),
             new On("question", new ValidatePossibleLinks()),
             new On("questionHint", new ValidatePossibleLinks()),
-            new On("mappings", new Each(new On("then", new ValidatePossibleLinks()))),
+            new On("mappings", new Each(new On("then", new ValidatePossibleLinks())))
-            new MiscTagRenderingChecks()
         )
     }
 }
@@ -1107,7 +1131,9 @@ export class PrevalidateLayer extends DesugaringStep<LayerConfigJson> {
             context.enter("pointRendering").err("There are no pointRenderings at all...")
         }
 
-        json.pointRendering?.forEach((pr,i) => this._validatePointRendering.convert(pr, context.enters("pointeRendering", i)))
+        json.pointRendering?.forEach((pr, i) =>
+            this._validatePointRendering.convert(pr, context.enters("pointeRendering", i))
+        )
 
         if (json["mapRendering"]) {
             context.enter("mapRendering").err("This layer has a legacy 'mapRendering'")
@@ -1134,7 +1160,7 @@ export class PrevalidateLayer extends DesugaringStep<LayerConfigJson> {
         }
 
         if (json.tagRenderings !== undefined && json.tagRenderings.length > 0) {
-            new On("tagRendering", new Each(new ValidateTagRenderings(json)))
+            new On("tagRenderings", new Each(new ValidateTagRenderings(json)))
             if (json.title === undefined && json.source !== "special:library") {
                 context
                     .enter("title")
@@ -1424,29 +1450,33 @@ class ValidatePointRendering extends DesugaringStep<PointRenderingConfigJson> {
         }
 
         if (json["markers"]) {
-            context.enter("markers").err(`Detected a field 'markerS' in pointRendering. It is written as a singular case`)
+            context
-        }
+                .enter("markers")
-        if (json.marker && !Array.isArray(json.marker)) {
+                .err(
-            context.enter("marker").err(
+                    `Detected a field 'markerS' in pointRendering. It is written as a singular case`
-                "The marker in a pointRendering should be an array"
                 )
         }
+        if (json.marker && !Array.isArray(json.marker)) {
+            context.enter("marker").err("The marker in a pointRendering should be an array")
+        }
         if (json.location.length == 0) {
-            context.enter("location").err (
+            context
+                .enter("location")
+                .err(
                     "A pointRendering should have at least one 'location' to defined where it should be rendered. "
                 )
         }
         return json
-
-
     }
 }
+
 export class ValidateLayer extends Conversion<
     LayerConfigJson,
     { parsed: LayerConfig; raw: LayerConfigJson }
 > {
     private readonly _skipDefaultLayers: boolean
     private readonly _prevalidation: PrevalidateLayer
+
     constructor(
         path: string,
         isBuiltin: boolean,

src/UI/Studio/EditLayer.svelte

@@ -180,7 +180,7 @@
 
         <div slot="title4" class="flex">
           Advanced functionality
-          <ErrorIndicatorForRegion firstPaths={firstPathsFor("advanced", "expert")} {state} />
+          <ErrorIndicatorForRegion firstPaths={firstPathsFor("advanced")} {state} />
         </div>
         <div slot="content4">
           <Region configs={perRegion["advanced"]} {state} />

src/UI/Studio/EditLayerState.ts

@@ -22,6 +22,7 @@ import { LayoutConfigJson } from "../../Models/ThemeConfig/Json/LayoutConfigJson
 import { PrepareTheme } from "../../Models/ThemeConfig/Conversion/PrepareTheme"
 import { ConversionContext } from "../../Models/ThemeConfig/Conversion/ConversionContext"
 import { LocalStorageSource } from "../../Logic/Web/LocalStorageSource"
+import { TagRenderingConfigJson } from "../../Models/ThemeConfig/Json/TagRenderingConfigJson"
 
 export interface HighlightedTagRendering {
     path: ReadonlyArray<string | number>
@@ -66,7 +67,6 @@ export abstract class EditJsonState<T> {
         this.messages = this.setupErrorsForLayers()
 
         const layerId = this.getId()
-        this.highlightedItem.addCallbackD((hl) => console.log("Highlighted item is", hl))
         this.configuration
             .mapD((config) => {
                 if (!this.sendingUpdates) {
@@ -110,6 +110,7 @@ export abstract class EditJsonState<T> {
     public async delete() {
         await this.server.delete(this.getId().data, this.category)
     }
+
     public getStoreFor<T>(path: ReadonlyArray<string | number>): UIEventSource<T | undefined> {
         const key = path.join(".")
 
@@ -172,7 +173,6 @@ export abstract class EditJsonState<T> {
 
     public setValueAt(path: ReadonlyArray<string | number>, v: any) {
         let entry = this.configuration.data
-        console.trace("Setting value at", path,"to",v)
         const isUndefined =
             v === undefined ||
             v === null ||
@@ -249,6 +249,62 @@ export abstract class EditJsonState<T> {
     }
 }
 
+class ContextRewritingStep<T> extends Conversion<LayerConfigJson, T> {
+    private readonly _step: Conversion<LayerConfigJson, T>
+    private readonly _state: DesugaringContext
+    private readonly _getTagRenderings: (t: T) => TagRenderingConfigJson[]
+
+    constructor(
+        state: DesugaringContext,
+        step: Conversion<LayerConfigJson, T>,
+        getTagRenderings: (t: T) => TagRenderingConfigJson[]
+    ) {
+        super(
+            "When validating a layer, the tagRenderings are first expanded. Some builtin tagRendering-calls (e.g. `contact`) will introduce _multiple_ tagRenderings, causing the count to be off. This class rewrites the error messages to fix this",
+            [],
+            "ContextRewritingStep"
+        )
+        this._state = state
+        this._step = step
+        this._getTagRenderings = getTagRenderings
+    }
+
+    convert(json: LayerConfigJson, context: ConversionContext): T {
+        const converted = this._step.convert(json, context)
+        const originalIds = json.tagRenderings?.map(
+            (tr) => (<QuestionableTagRenderingConfigJson>tr)["id"]
+        )
+        if (!originalIds) {
+            return converted
+        }
+
+        let newTagRenderings: TagRenderingConfigJson[]
+        if (converted === undefined) {
+            const prepared = new PrepareLayer(this._state)
+            newTagRenderings = <TagRenderingConfigJson[]>(
+                prepared.convert(json, context).tagRenderings
+            )
+        } else {
+            newTagRenderings = this._getTagRenderings(converted)
+        }
+        context.rewriteMessages((path) => {
+            if (path[0] !== "tagRenderings") {
+                return undefined
+            }
+            const newPath = [...path]
+            const idToSearch = newTagRenderings[newPath[1]].id
+            const oldIndex = originalIds.indexOf(idToSearch)
+            if (oldIndex < 0) {
+                console.warn("Original ID was not found: ", idToSearch)
+                return undefined // We don't modify the message
+            }
+            newPath[1] = oldIndex
+            return newPath
+        })
+        return converted
+    }
+}
+
 export default class EditLayerState extends EditJsonState<LayerConfigJson> {
     // Needed for the special visualisations
     public readonly osmConnection: OsmConnection
@@ -334,9 +390,10 @@ export default class EditLayerState extends EditJsonState<LayerConfigJson> {
     }
 
     protected buildValidation(state: DesugaringContext) {
-        return new Pipe(
+        return new ContextRewritingStep(
-            new PrepareLayer(state),
+            state,
-            new ValidateLayer("dynamic", false, undefined, true)
+            new Pipe(new PrepareLayer(state), new ValidateLayer("dynamic", false, undefined, true)),
+            (t) => <TagRenderingConfigJson[]>t.raw.tagRenderings
         )
     }
 

src/UI/Studio/ErrorIndicatorForRegion.svelte

@@ -2,7 +2,7 @@
   import EditLayerState from "./EditLayerState"
   import { ExclamationIcon } from "@rgossiaux/svelte-heroicons/solid"
 
-  export let firstPaths: Set<string>
+  export let firstPaths: Set<string | number>
   export let state: EditLayerState
   let messagesCount = state.messages.map(
     (msgs) =>

src/UI/Studio/MappingInput.svelte

@@ -11,9 +11,11 @@
   import { Utils } from "../../Utils"
   import ToSvelte from "../Base/ToSvelte.svelte"
   import { VariableUiElement } from "../Base/VariableUIElement"
+  import { ExclamationTriangle } from "@babeard/svelte-heroicons/solid/ExclamationTriangle"
 
   export let state: EditLayerState
   export let path: (string | number)[]
+  let messages = state.messagesFor(path)
   let tag: UIEventSource<TagConfigJson> = state.getStoreFor([...path, "if"])
   let parsedTag = tag.map((t) => (t ? TagUtils.Tag(t) : undefined))
   let exampleTags = parsedTag.map((pt) => {
@@ -27,7 +29,6 @@
     }
     return o
   })
-  let uploadableOnly: boolean = true
 
   let thenText: UIEventSource<Record<string, string>> = state.getStoreFor([...path, "then"])
   let thenTextEn = thenText.mapD((translation) =>
@@ -71,5 +72,11 @@
       <i>No then is set</i>
     {/if}
     <FromHtml src={$parsedTag?.asHumanString(false, false, $exampleTags)} />
+    {#if $messages.length > 0}
+      <div class="alert m-2 flex">
+        <ExclamationTriangle class="w-6 h-6"/>
+        {$messages.length} errors
+      </div>
+    {/if}
   </div>
 {/if}

src/UI/Studio/StudioServer.ts

@@ -2,6 +2,7 @@ import { Utils } from "../../Utils"
 import Constants from "../../Models/Constants"
 import { LayerConfigJson } from "../../Models/ThemeConfig/Json/LayerConfigJson"
 import { Store } from "../../Logic/UIEventSource"
+import { LayoutConfigJson } from "../../Models/ThemeConfig/Json/LayoutConfigJson"
 
 export default class StudioServer {
     private readonly url: string
@@ -47,11 +48,13 @@ export default class StudioServer {
         return layerOverview
     }
 
+    async fetch(layerId: string, category: "layers", uid?: number): Promise<LayerConfigJson>
+    async fetch(layerId: string, category: "themes", uid?: number): Promise<LayoutConfigJson>
     async fetch(
         layerId: string,
         category: "layers" | "themes",
         uid?: number
-    ): Promise<LayerConfigJson> {
+    ): Promise<LayerConfigJson | LayoutConfigJson> {
         try {
             return await Utils.downloadJson(this.urlFor(layerId, category, uid))
         } catch (e) {

src/UI/Studio/TagRenderingInput.svelte

@@ -24,13 +24,13 @@
   import { onMount } from "svelte"
 
   export let state: EditLayerState
-  export let schema: ConfigMeta
+  export let path: ReadonlyArray<string | number>
-  export let path: (string | number)[]
+  let messages = state.messagesFor(path)
   let expertMode = state.expertMode
   const store = state.getStoreFor(path)
   let value = store.data
   let hasSeenIntro = UIEventSource.asBoolean(
-    LocalStorageSource.Get("studio-seen-tagrendering-tutorial", "false")
+    LocalStorageSource.Get("studio-seen-tagrendering-tutorial", "false"),
   )
   onMount(() => {
     if (!hasSeenIntro.data) {
@@ -43,7 +43,7 @@
    * Should only be enabled for 'tagrenderings' in the theme, if the source is OSM
    */
   let allowQuestions: Store<boolean> = state.configuration.mapD(
-    (config) => path.at(0) === "tagRenderings" && config.source?.geoJson === undefined
+    (config) => path.at(0) === "tagRenderings" && config.source?.["geoJson"] === undefined,
   )
 
   let mappingsBuiltin: MappingConfigJson[] = []
@@ -119,7 +119,7 @@
 
   const freeformSchemaAll = <ConfigMeta[]>(
     questionableTagRenderingSchemaRaw.filter(
-      (schema) => schema.path.length == 2 && schema.path[0] === "freeform" && $allowQuestions
+      (schema) => schema.path.length == 2 && schema.path[0] === "freeform" && $allowQuestions,
     )
   )
   let freeformSchema = $expertMode
@@ -128,7 +128,7 @@
   const missing: string[] = questionableTagRenderingSchemaRaw
     .filter(
       (schema) =>
-        schema.path.length >= 1 && !items.has(schema.path[0]) && !ignored.has(schema.path[0])
+        schema.path.length >= 1 && !items.has(schema.path[0]) && !ignored.has(schema.path[0]),
     )
     .map((schema) => schema.path.join("."))
   console.log({ state })
@@ -164,7 +164,7 @@
     {/if}
     {#each $mappings ?? [] as mapping, i (mapping)}
       <div class="interactive flex w-full">
-        <MappingInput {state} path={path.concat(["mappings", i])}>
+        <MappingInput {state} path={[...path, "mappings", i]}>
           <button
             slot="delete"
             class="no-image-background rounded-full"
@@ -178,6 +178,7 @@
           </button>
         </MappingInput>
       </div>
+
     {/each}
 
     <button

src/UI/StudioGUI.svelte

@@ -35,19 +35,18 @@
       ? "http://127.0.0.1:1235"
       : "https://studio.mapcomplete.org"
 
-  let osmConnection = new OsmConnection(
+  const oauth_token = QueryParameters.GetQueryParameter(
-    new OsmConnection({
-      oauth_token: QueryParameters.GetQueryParameter(
     "oauth_token",
     undefined,
-        "Used to complete the login"
+    "Used to complete the login",
-      ),
-    })
   )
+  let osmConnection = new OsmConnection({
+    oauth_token,
+  })
   const expertMode = UIEventSource.asBoolean(
     osmConnection.GetPreference("studio-expert-mode", "false", {
       documentation: "Indicates if more options are shown in mapcomplete studio",
-    })
+    }),
   )
   expertMode.addCallbackAndRunD((expert) => console.log("Expert mode is", expert))
   const createdBy = osmConnection.userDetails.data.name
@@ -55,23 +54,23 @@
   const studio = new StudioServer(studioUrl, uid)
 
   let layersWithErr = UIEventSource.FromPromiseWithErr(studio.fetchOverview())
-  let layers: Store<{ owner: number }[]> = layersWithErr.mapD((l) =>
+  let layers: Store<{ owner: number, id: string }[]> = layersWithErr.mapD((l) =>
-    l.success?.filter((l) => l.category === "layers")
+    l["success"]?.filter((l) => l.category === "layers"),
   )
   let selfLayers = layers.mapD((ls) => ls.filter((l) => l.owner === uid.data), [uid])
   let otherLayers = layers.mapD(
     (ls) => ls.filter((l) => l.owner !== undefined && l.owner !== uid.data),
-    [uid]
+    [uid],
   )
   let officialLayers = layers.mapD((ls) => ls.filter((l) => l.owner === undefined), [uid])
 
-  let themes: Store<{ owner: number }[]> = layersWithErr.mapD((l) =>
+  let themes: Store<{ owner: number, id: string }[]> = layersWithErr.mapD((l) =>
-    l.success?.filter((l) => l.category === "themes")
+    l["success"]?.filter((l) => l.category === "themes"),
   )
   let selfThemes = themes.mapD((ls) => ls.filter((l) => l.owner === uid.data), [uid])
   let otherThemes = themes.mapD(
     (ls) => ls.filter((l) => l.owner !== undefined && l.owner !== uid.data),
-    [uid]
+    [uid],
   )
   let officialThemes = themes.mapD((ls) => ls.filter((l) => l.owner === undefined), [uid])
 
@@ -90,8 +89,6 @@
   const layoutSchema: ConfigMeta[] = <any>layoutSchemaRaw
   let editThemeState = new EditThemeState(layoutSchema, studio, { expertMode })
 
-  let layerId = editLayerState.configuration.map((layerConfig) => layerConfig.id)
-
   const version = meta.version
 
   async function editLayer(event: Event) {
@@ -107,7 +104,8 @@
     const id: { id: string; owner: number } = event["detail"]
     state = "loading"
     editThemeState.startSavingUpdates(false)
-    editThemeState.configuration.setData(await studio.fetch(id.id, "themes", id.owner))
+    const layout = await studio.fetch(id.id, "themes", id.owner)
+    editThemeState.configuration.setData(layout)
     editThemeState.startSavingUpdates()
     state = "editing_theme"
   }
@@ -142,7 +140,7 @@
   }
 </script>
 
-<If condition={layersWithErr.map((d) => d?.error !== undefined)}>
+<If condition={layersWithErr.map((d) => d?.["error"] !== undefined) }>
   <div>
     <div class="alert">
       Something went wrong while contacting the MapComplete Studio Server: {$layersWithErr["error"]}