Fix dissappearing default tags, add tests

Logic/Osm/Changes.ts

@@ -363,7 +363,7 @@ export class Changes {
 
     }
 
-    private CreateChangesetObjects(changes: ChangeDescription[], downloadedOsmObjects: OsmObject[]): {
+    public CreateChangesetObjects(changes: ChangeDescription[], downloadedOsmObjects: OsmObject[]): {
         newObjects: OsmObject[],
         modifiedObjects: OsmObject[]
         deletedObjects: OsmObject[]

Logic/Osm/ChangesetHandler.ts

@@ -51,13 +51,14 @@ export class ChangesetHandler {
     }
 
     /**
+     * Inplace rewrite of extraMetaTags
      * If the metatags contain a special motivation of the format "<change-type>:node/-<number>", this method will rewrite this negative number to the actual ID
      * The key is changed _in place_; true will be returned if a change has been applied
      * @param extraMetaTags
      * @param rewriteIds
      * @private
      */
-    private static rewriteMetaTags(extraMetaTags: ChangesetTag[], rewriteIds: Map<string, string>) {
+    static rewriteMetaTags(extraMetaTags: ChangesetTag[], rewriteIds: Map<string, string>) {
         let hasChange = false;
         for (const tag of extraMetaTags) {
             const match = tag.key.match(/^([a-zA-Z0-9_]+):(node\/-[0-9])$/)
@@ -92,6 +93,8 @@ export class ChangesetHandler {
         if (!extraMetaTags.some(tag => tag.key === "comment") || !extraMetaTags.some(tag => tag.key === "theme")) {
             throw "The meta tags should at least contain a `comment` and a `theme`"
         }
+        
+        extraMetaTags = [...extraMetaTags, ...this.defaultChangesetTags()]
 
         if (this.userDetails.data.csCount == 0) {
             // The user became a contributor!
@@ -112,7 +115,7 @@ export class ChangesetHandler {
                 openChangeset.setData(csId);
                 const changeset = generateChangeXML(csId);
                 console.trace("Opened a new changeset (openChangeset.data is undefined):", changeset);
-                const changes = await this.AddChange(csId, changeset)
+                const changes = await this.UploadChange(csId, changeset)
                 const hasSpecialMotivationChanges = ChangesetHandler.rewriteMetaTags(extraMetaTags, changes)
                 if(hasSpecialMotivationChanges){
                     // At this point, 'extraMetaTags' will have changed - we need to set the tags again
@@ -139,11 +142,12 @@ export class ChangesetHandler {
                     return;
                 }
 
-                const rewritings = await this.AddChange(
+                const rewritings = await this.UploadChange(
                     csId,
                     generateChangeXML(csId))
 
-                await this.RewriteTagsOf(extraMetaTags, rewritings, oldChangesetMeta)
+                const rewrittenTags = this.RewriteTagsOf(extraMetaTags, rewritings, oldChangesetMeta)
+                await this.UpdateTags(csId, rewrittenTags)
 
             } catch (e) {
                 console.warn("Could not upload, changeset is probably closed: ", e);
@@ -153,13 +157,13 @@ export class ChangesetHandler {
     }
 
     /**
-     * Updates the metatag of a changeset -
+     * Given an existing changeset with metadata and extraMetaTags to add, will fuse them to a new set of metatags
+     * Does not yet send data
      * @param extraMetaTags: new changeset tags to add/fuse with this changeset
+     * @param rewriteIds: the mapping of ids
      * @param oldChangesetMeta: the metadata-object of the already existing changeset
-     * @constructor
-     * @private
      */
-    private async RewriteTagsOf(extraMetaTags: ChangesetTag[],
+    public RewriteTagsOf(extraMetaTags: ChangesetTag[],
                                 rewriteIds: Map<string, string>,
                                 oldChangesetMeta: {
                                     open: boolean,
@@ -167,9 +171,8 @@ export class ChangesetHandler {
                                     uid: number, // User ID
                                     changes_count: number,
                                     tags: any
-                                }) {
+                                }) : ChangesetTag[] {
 
-        const csId = oldChangesetMeta.id
 
         // Note: extraMetaTags is where all the tags are collected into
 
@@ -214,10 +217,8 @@ export class ChangesetHandler {
 
 
         ChangesetHandler.rewriteMetaTags(extraMetaTags, rewriteIds)
-
+        return extraMetaTags
-        await this.UpdateTags(csId, extraMetaTags)
+    
-
-
     }
 
     private handleIdRewrite(node: any, type: string): [string, string] {
@@ -295,7 +296,7 @@ export class ChangesetHandler {
         })
     }
 
-    private async GetChangesetMeta(csId: number): Promise<{
+    async GetChangesetMeta(csId: number): Promise<{
         id: number,
         open: boolean,
         uid: number,
@@ -340,21 +341,30 @@ export class ChangesetHandler {
             });
         })
     }
+    
+    private defaultChangesetTags() : ChangesetTag[]{
+      return  [ ["created_by", `MapComplete ${Constants.vNumber}`],
+            ["locale", Locale.language.data],
+            ["host", `${window.location.origin}${window.location.pathname}`],
+            ["source", this.changes.state["currentUserLocation"]?.features?.data?.length > 0 ? "survey" : undefined],
+            ["imagery", this.changes.state["backgroundLayer"]?.data?.id]].map(([key, value]) => ({
+            key, value, aggretage: false
+        }))
+    }
 
+    /**
+     * Opens a changeset with the specified tags
+     * @param changesetTags
+     * @constructor
+     * @private
+     */
     private OpenChangeset(
         changesetTags: ChangesetTag[]
     ): Promise<number> {
         const self = this;
         return new Promise<number>(function (resolve, reject) {
 
-            const metadata = [
+            const metadata = changesetTags.map(cstag => [cstag.key, cstag.value])
-                ["created_by", `MapComplete ${Constants.vNumber}`],
-                ["locale", Locale.language.data],
-                ["host", `${window.location.origin}${window.location.pathname}`],
-                ["source", self.changes.state["currentUserLocation"]?.features?.data?.length > 0 ? "survey" : undefined],
-                ["imagery", self.changes.state["backgroundLayer"]?.data?.id],
-                ...changesetTags.map(cstag => [cstag.key, cstag.value])
-            ]
                 .filter(kv => (kv[1] ?? "") !== "")
                 .map(kv => `<tag k="${kv[0]}" v="${escapeHtml(kv[1])}"/>`)
                 .join("\n")
@@ -382,7 +392,7 @@ export class ChangesetHandler {
     /**
      * Upload a changesetXML
      */
-    private AddChange(changesetId: number,
+    private UploadChange(changesetId: number,
                       changesetXML: string): Promise<Map<string, string>> {
         const self = this;
         return new Promise(function (resolve, reject) {

test/Changes.spec.ts

@@ -0,0 +1,51 @@
+import T from "./TestHelper";
+import {Changes} from "../Logic/Osm/Changes";
+import {ChangeDescription, ChangeDescriptionTools} from "../Logic/Osm/Actions/ChangeDescription";
+
+export default class ChangesSpec extends T {
+
+    constructor() {
+        super([
+            ["Generate preXML from changeDescriptions", () => {
+                const changeDescrs: ChangeDescription[] = [
+                    {
+                        type: "node",
+                        id: -1,
+                        changes: {
+                            lat: 42,
+                            lon: -8
+                        },
+                        tags: [{k: "someKey", v: "someValue"}],
+                        meta: {
+                            changeType: "create",
+                            theme: "test"
+                        }
+                    },
+                    {
+                        type: "node",
+                        id: -1,
+                        tags: [{k: 'foo', v: 'bar'}],
+                        meta: {
+                            changeType: "answer",
+                            theme: "test"
+                        }
+                    }
+                ]
+                const c = new Changes()
+               const descr = c.CreateChangesetObjects(
+                    changeDescrs,
+                    []
+                )
+                T.equals(0, descr.modifiedObjects.length)
+                T.equals(0, descr.deletedObjects.length)
+                T.equals(1, descr.newObjects.length)
+                const ch = descr.newObjects[0]
+                T.equals("bar", ch.tags["foo"])
+                T.equals("someValue", ch.tags["someKey"])
+            }]
+        ]);
+        
+    }
+
+
+}

test/ChangesetHandler.spec.ts

@@ -0,0 +1,181 @@
+import T from "./TestHelper";
+import {ChangesetHandler, ChangesetTag} from "../Logic/Osm/ChangesetHandler";
+import {UIEventSource} from "../Logic/UIEventSource";
+import {OsmConnection} from "../Logic/Osm/OsmConnection";
+import {ElementStorage} from "../Logic/ElementStorage";
+import {Changes} from "../Logic/Osm/Changes";
+
+export default class ChangesetHandlerSpec extends T {
+    
+    private static asDict(tags: {key: string, value: string | number}[]) : Map<string, string | number>{
+        const d= new Map<string, string | number>()
+
+        for (const tag of tags) {
+            d.set(tag.key, tag.value)
+        }
+        
+        return d
+    }
+    
+    constructor() {
+        super([
+            [
+                "Test rewrite tags", () => {
+                const cs = new ChangesetHandler(new UIEventSource<boolean>(true),
+                    new OsmConnection({}),
+                    new ElementStorage(),
+                    new Changes(),
+                    new UIEventSource(undefined)
+                );
+
+
+                const oldChangesetMeta = {
+                    "type": "changeset",
+                    "id": 118443748,
+                    "created_at": "2022-03-13T19:52:10Z",
+                    "closed_at": "2022-03-13T20:54:35Z",
+                    "open": false,
+                    "user": "Pieter Vander Vennet",
+                    "uid": 3818858,
+                    "minlat": 51.0361902,
+                    "minlon": 3.7092939,
+                    "maxlat": 51.0364194,
+                    "maxlon": 3.7099520,
+                    "comments_count": 0,
+                    "changes_count": 3,
+                    "tags": {
+                        "answer": "5",
+                        "comment": "Adding data with #MapComplete for theme #toerisme_vlaanderen",
+                        "created_by": "MapComplete 0.16.6",
+                        "host": "https://mapcomplete.osm.be/toerisme_vlaanderen.html",
+                        "imagery": "osm",
+                        "locale": "nl",
+                        "source": "survey",
+                        "source:node/-1":"note/1234",
+                        "theme": "toerisme_vlaanderen",
+                    }
+                }
+                let rewritten = cs.RewriteTagsOf(
+                    [{
+                       key: "newTag",
+                       value: "newValue",
+                       aggregate: false 
+                    }],
+                    new Map<string, string>(),
+                    oldChangesetMeta)
+                let d = ChangesetHandlerSpec.asDict(rewritten)
+                T.equals(10, d.size)
+                T.equals("5", d.get("answer"))
+                T.equals("Adding data with #MapComplete for theme #toerisme_vlaanderen", d.get("comment"))
+                T.equals("MapComplete 0.16.6", d.get("created_by"))
+                T.equals("https://mapcomplete.osm.be/toerisme_vlaanderen.html", d.get("host"))
+                T.equals("osm", d.get("imagery"))
+                T.equals("survey", d.get("source"))
+                T.equals("note/1234", d.get("source:node/-1"))
+                T.equals("toerisme_vlaanderen", d.get("theme"))
+                
+                T.equals("newValue", d.get("newTag"))
+
+
+
+
+                rewritten = cs.RewriteTagsOf(
+                    [{
+                        key: "answer",
+                        value: "37",
+                        aggregate: true
+                    }],
+                    new Map<string, string>(),
+                    oldChangesetMeta)
+                d = ChangesetHandlerSpec.asDict(rewritten)
+                
+                T.equals(9, d.size)
+                T.equals("42", d.get("answer"))
+                T.equals("Adding data with #MapComplete for theme #toerisme_vlaanderen", d.get("comment"))
+                T.equals("MapComplete 0.16.6", d.get("created_by"))
+                T.equals("https://mapcomplete.osm.be/toerisme_vlaanderen.html", d.get("host"))
+                T.equals("osm", d.get("imagery"))
+                T.equals("survey", d.get("source"))
+                T.equals("note/1234", d.get("source:node/-1"))
+                T.equals("toerisme_vlaanderen", d.get("theme"))
+
+                rewritten = cs.RewriteTagsOf(
+                    [],
+                    new Map<string, string>([["node/-1", "node/42"]]),
+                    oldChangesetMeta)
+                d = ChangesetHandlerSpec.asDict(rewritten)
+
+                T.equals(9, d.size)
+                T.equals("5", d.get("answer"))
+                T.equals("Adding data with #MapComplete for theme #toerisme_vlaanderen", d.get("comment"))
+                T.equals("MapComplete 0.16.6", d.get("created_by"))
+                T.equals("https://mapcomplete.osm.be/toerisme_vlaanderen.html", d.get("host"))
+                T.equals("osm", d.get("imagery"))
+                T.equals("survey", d.get("source"))
+                T.equals("note/1234", d.get("source:node/42"))
+                T.equals("toerisme_vlaanderen", d.get("theme"))
+
+            },
+            ],[
+                "Test rewrite on special motivation", () => {
+
+
+                    const cs = new ChangesetHandler(new UIEventSource<boolean>(true),
+                        new OsmConnection({}),
+                        new ElementStorage(),
+                        new Changes(),
+                        new UIEventSource(undefined)
+                    );
+
+
+                    const oldChangesetMeta = {
+                        "type": "changeset",
+                        "id": 118443748,
+                        "created_at": "2022-03-13T19:52:10Z",
+                        "closed_at": "2022-03-13T20:54:35Z",
+                        "open": false,
+                        "user": "Pieter Vander Vennet",
+                        "uid": 3818858,
+                        "minlat": 51.0361902,
+                        "minlon": 3.7092939,
+                        "maxlat": 51.0364194,
+                        "maxlon": 3.7099520,
+                        "comments_count": 0,
+                        "changes_count": 3,
+                        "tags": {
+                            "answer": "5",
+                            "comment": "Adding data with #MapComplete for theme #toerisme_vlaanderen",
+                            "created_by": "MapComplete 0.16.6",
+                            "host": "https://mapcomplete.osm.be/toerisme_vlaanderen.html",
+                            "imagery": "osm",
+                            "locale": "nl",
+                            "source": "survey",
+                            "source:-1":"note/1234",
+                            "theme": "toerisme_vlaanderen",
+                        }
+                    }
+            
+                    const extraMetaTags : ChangesetTag[] = [
+                        {
+                            key: "created_by",
+                            value:"mapcomplete"
+                        },
+                        {
+                            key: "source:node/-1",
+                            value:"note/1234"
+                        }
+                    ]
+                    const changes = new Map<string, string>([["node/-1","node/42"]])
+                    const hasSpecialMotivationChanges = ChangesetHandler.rewriteMetaTags(extraMetaTags, changes)
+                    T.isTrue(hasSpecialMotivationChanges, "Special rewrite did not trigger")
+                    // Rewritten inline by rewriteMetaTags
+                    T.equals("source:node/42", extraMetaTags[1].key)
+                    T.equals("note/1234", extraMetaTags[1].value)
+                    T.equals("created_by", extraMetaTags[0].key)
+                    T.equals("mapcomplete", extraMetaTags[0].value)
+                }
+                
+            ]
+        ]);
+    }
+}

test/TestAll.ts

@@ -21,11 +21,16 @@ import ValidatedTextFieldTranslationsSpec from "./ValidatedTextFieldTranslations
 import CreateCacheSpec from "./CreateCache.spec";
 import CodeQualitySpec from "./CodeQuality.spec";
 import ImportMultiPolygonSpec from "./ImportMultiPolygon.spec";
+import {ChangesetHandler} from "../Logic/Osm/ChangesetHandler";
+import ChangesetHandlerSpec from "./ChangesetHandler.spec";
+import ChangesSpec from "./Changes.spec";
 
 
 async function main() {
 
     const allTests: T[] = [
+        new ChangesSpec(),
+        new ChangesetHandlerSpec(),
         new OsmObjectSpec(),
         new TagSpec(),
         new ImageAttributionSpec(),
@@ -45,7 +50,7 @@ async function main() {
         new ValidatedTextFieldTranslationsSpec(),
         new CreateCacheSpec(),
         new CodeQualitySpec(),
-        new ImportMultiPolygonSpec()
+        new ImportMultiPolygonSpec(),
     ]
     ScriptUtils.fixUtils();
     const realDownloadFunc = Utils.externalDownloadFunction;