From 21fd148f384b0b5517c88f417da04b91c3eec1f7 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Mon, 4 Oct 2021 03:12:42 +0200 Subject: [PATCH] Add propagation of metadata in changedescriptions, aggregate metadata in changeset tags --- Logic/Osm/Actions/ChangeDescription.ts | 18 ++ Logic/Osm/Actions/ChangeTagAction.ts | 13 +- Logic/Osm/Actions/CreateNewNodeAction.ts | 21 +- Logic/Osm/Actions/DeleteAction.ts | 253 ++++---------------- Logic/Osm/Actions/RelationSplitHandler.ts | 16 +- Logic/Osm/Actions/SplitAction.ts | 19 +- Logic/Osm/Changes.ts | 109 ++++++--- Logic/Osm/ChangesetHandler.ts | 226 ++++++++++------- Logic/Osm/OsmConnection.ts | 7 - Logic/Osm/OsmObject.ts | 3 +- Models/Constants.ts | 2 +- Models/ThemeConfig/Json/LayoutConfigJson.ts | 5 +- Models/ThemeConfig/LayoutConfig.ts | 2 - UI/BigComponents/ImportButton.ts | 5 +- UI/BigComponents/SimpleAddUI.ts | 5 +- UI/Popup/DeleteWizard.ts | 227 +++++++++++++++--- UI/Popup/SplitRoadWizard.ts | 4 +- UI/Popup/TagRenderingQuestion.ts | 5 +- Utils.ts | 8 + 19 files changed, 545 insertions(+), 403 deletions(-) diff --git a/Logic/Osm/Actions/ChangeDescription.ts b/Logic/Osm/Actions/ChangeDescription.ts index 1787d1b1b..42b018cfe 100644 --- a/Logic/Osm/Actions/ChangeDescription.ts +++ b/Logic/Osm/Actions/ChangeDescription.ts @@ -5,6 +5,24 @@ import {OsmNode, OsmRelation, OsmWay} from "../OsmObject"; */ export interface ChangeDescription { + /** + * Metadata to be included in the changeset + */ + meta: { + /* + * The theme with which this changeset was made + */ + theme: string, + /** + * The type of the change + */ + changeType: "answer" | "create" | "split" | "delete" | string + /** + * THe motivation for the change, e.g. 'deleted because does not exist anymore' + */ + specialMotivation?: string + }, + /** * Identifier of the object */ diff --git a/Logic/Osm/Actions/ChangeTagAction.ts b/Logic/Osm/Actions/ChangeTagAction.ts index 784f4d4dc..fd55953d1 100644 --- a/Logic/Osm/Actions/ChangeTagAction.ts +++ b/Logic/Osm/Actions/ChangeTagAction.ts @@ -7,12 +7,17 @@ export default class ChangeTagAction extends OsmChangeAction { private readonly _elementId: string; private readonly _tagsFilter: TagsFilter; private readonly _currentTags: any; + private readonly _meta: {theme: string, changeType: string}; - constructor(elementId: string, tagsFilter: TagsFilter, currentTags: any) { + constructor(elementId: string, tagsFilter: TagsFilter, currentTags: any, meta: { + theme: string, + changeType: "answer" | "soft-delete" + }) { super(); this._elementId = elementId; this._tagsFilter = tagsFilter; this._currentTags = currentTags; + this._meta = meta; } /** @@ -43,10 +48,10 @@ export default class ChangeTagAction extends OsmChangeAction { const type = typeId[0] const id = Number(typeId [1]) return [{ - // @ts-ignore - type: type, + type: <"node"|"way"|"relation"> type, id: id, - tags: changedTags + tags: changedTags, + meta: this._meta }] } } \ No newline at end of file diff --git a/Logic/Osm/Actions/CreateNewNodeAction.ts b/Logic/Osm/Actions/CreateNewNodeAction.ts index 4841180bd..e644eb2c6 100644 --- a/Logic/Osm/Actions/CreateNewNodeAction.ts +++ b/Logic/Osm/Actions/CreateNewNodeAction.ts @@ -14,8 +14,14 @@ export default class CreateNewNodeAction extends OsmChangeAction { private readonly _lon: number; private readonly _snapOnto: OsmWay; private readonly _reusePointDistance: number; + private meta: { changeType: "create" | "import"; theme: string }; - constructor(basicTags: Tag[], lat: number, lon: number, options?: { snapOnto: OsmWay, reusePointWithinMeters?: number }) { + constructor(basicTags: Tag[], + lat: number, lon: number, + options: { + snapOnto?: OsmWay, + reusePointWithinMeters?: number, + theme: string, changeType: "create" | "import" }) { super() this._basicTags = basicTags; this._lat = lat; @@ -25,6 +31,10 @@ export default class CreateNewNodeAction extends OsmChangeAction { } this._snapOnto = options?.snapOnto; this._reusePointDistance = options?.reusePointWithinMeters ?? 1 + this.meta = { + theme: options.theme, + changeType: options.changeType + } } async CreateChangeDescriptions(changes: Changes): Promise { @@ -47,7 +57,8 @@ export default class CreateNewNodeAction extends OsmChangeAction { changes: { lat: this._lat, lon: this._lon - } + }, + meta: this.meta } if (this._snapOnto === undefined) { return [newPointChange] @@ -78,7 +89,8 @@ export default class CreateNewNodeAction extends OsmChangeAction { return [{ tags: new And(this._basicTags).asChange(properties), type: "node", - id: reusedPointId + id: reusedPointId, + meta: this.meta }] } @@ -99,7 +111,8 @@ export default class CreateNewNodeAction extends OsmChangeAction { changes: { coordinates: locations, nodes: ids - } + }, + meta:this.meta } ] } diff --git a/Logic/Osm/Actions/DeleteAction.ts b/Logic/Osm/Actions/DeleteAction.ts index 68919bd12..34adc50e7 100644 --- a/Logic/Osm/Actions/DeleteAction.ts +++ b/Logic/Osm/Actions/DeleteAction.ts @@ -1,225 +1,62 @@ -import {UIEventSource} from "../../UIEventSource"; -import {Translation} from "../../../UI/i18n/Translation"; import State from "../../../State"; import {OsmObject} from "../OsmObject"; -import Translations from "../../../UI/i18n/Translations"; -import Constants from "../../../Models/Constants"; +import OsmChangeAction from "./OsmChangeAction"; +import {Changes} from "../Changes"; +import {ChangeDescription} from "./ChangeDescription"; +import ChangeTagAction from "./ChangeTagAction"; +import {TagsFilter} from "../../Tags/TagsFilter"; +import {And} from "../../Tags/And"; +import {Tag} from "../../Tags/Tag"; -export default class DeleteAction { +export default class DeleteAction extends OsmChangeAction { - public readonly canBeDeleted: UIEventSource<{ canBeDeleted?: boolean, reason: Translation }>; - public readonly isDeleted = new UIEventSource(false); + private readonly _softDeletionTags: TagsFilter; + private readonly meta: { + theme: string, + specialMotivation: string, + changeType: "deletion" + }; private readonly _id: string; - private readonly _allowDeletionAtChangesetCount: number; + private _hardDelete: boolean; - constructor(id: string, allowDeletionAtChangesetCount?: number) { + constructor(id: string, + softDeletionTags: TagsFilter, + meta: { + theme: string, + specialMotivation: string + }, + hardDelete: boolean) { + super() this._id = id; - this._allowDeletionAtChangesetCount = allowDeletionAtChangesetCount ?? Number.MAX_VALUE; + this._hardDelete = hardDelete; + this.meta = {...meta, changeType: "deletion"}; + this._softDeletionTags = new And([softDeletionTags, + new Tag("fixme", `A mapcomplete user marked this feature to be deleted (${meta.specialMotivation})`) + ]); - this.canBeDeleted = new UIEventSource<{ canBeDeleted?: boolean; reason: Translation }>({ - canBeDeleted: undefined, - reason: Translations.t.delete.loading - }) - - this.CheckDeleteability(false) } + protected async CreateChangeDescriptions(changes: Changes): Promise { - /** - * Does actually delete the feature; returns the event source 'this.isDeleted' - * If deletion is not allowed, triggers the callback instead - */ - public DoDelete(reason: string, onNotAllowed: () => void): void { - const isDeleted = this.isDeleted - const self = this; - let deletionStarted = false; - this.canBeDeleted.addCallbackAndRun( - canBeDeleted => { - if (isDeleted.data || deletionStarted) { - // Already deleted... - return; + const osmObject = await OsmObject.DownloadObjectAsync(this._id) + + if (this._hardDelete) { + return [{ + meta: this.meta, + doDelete: true, + type: osmObject.type, + id: osmObject.id, + }] + } else { + return await new ChangeTagAction( + this._id, this._softDeletionTags, osmObject.tags, + { + theme: State.state?.layoutToUse?.id ?? "unkown", + changeType: "soft-delete" } - - if (canBeDeleted.canBeDeleted === false) { - // We aren't allowed to delete - deletionStarted = true; - onNotAllowed(); - isDeleted.setData(true); - return; - } - - if (!canBeDeleted) { - // We are not allowed to delete (yet), this might change in the future though - return; - } - - - deletionStarted = true; - OsmObject.DownloadObject(self._id).addCallbackAndRun(obj => { - if (obj === undefined) { - return; - } - State.state.osmConnection.changesetHandler.DeleteElement( - obj, - State.state.layoutToUse, - reason, - State.state.allElements, - () => { - isDeleted.setData(true) - } - ) - }) - - } - ) - } - - /** - * Checks if the currently logged in user can delete the current point. - * State is written into this._canBeDeleted - * @constructor - * @private - */ - public CheckDeleteability(useTheInternet: boolean): void { - const t = Translations.t.delete; - const id = this._id; - const state = this.canBeDeleted - if (!id.startsWith("node")) { - this.canBeDeleted.setData({ - canBeDeleted: false, - reason: t.isntAPoint - }) - return; + ).CreateChangeDescriptions(changes) } - - // Does the currently logged in user have enough experience to delete this point? - - const deletingPointsOfOtherAllowed = State.state.osmConnection.userDetails.map(ud => { - if (ud === undefined) { - return undefined; - } - if (!ud.loggedIn) { - return false; - } - return ud.csCount >= Math.min(Constants.userJourney.deletePointsOfOthersUnlock, this._allowDeletionAtChangesetCount); - }) - - const previousEditors = new UIEventSource(undefined) - - const allByMyself = previousEditors.map(previous => { - if (previous === null || previous === undefined) { - // Not yet downloaded - return null; - } - const userId = State.state.osmConnection.userDetails.data.uid; - return !previous.some(editor => editor !== userId) - }, [State.state.osmConnection.userDetails]) - - - // User allowed OR only edited by self? - const deletetionAllowed = deletingPointsOfOtherAllowed.map(isAllowed => { - if (isAllowed === undefined) { - // No logged in user => definitively not allowed to delete! - return false; - } - if (isAllowed === true) { - return true; - } - - // At this point, the logged in user is not allowed to delete points created/edited by _others_ - // however, we query OSM and if it turns out the current point has only be edited by the current user, deletion is allowed after all! - - if (allByMyself.data === null && useTheInternet) { - // We kickoff the download here as it hasn't yet been downloaded. Note that this is mapped onto 'all by myself' above - OsmObject.DownloadHistory(id).map(versions => versions.map(version => version.tags["_last_edit:contributor:uid"])).syncWith(previousEditors) - } - if (allByMyself.data === true) { - // Yay! We can download! - return true; - } - if (allByMyself.data === false) { - // Nope, downloading not allowed... - return false; - } - - - // At this point, we don't have enough information yet to decide if the user is allowed to delete the current point... - return undefined; - }, [allByMyself]) - - - const hasRelations: UIEventSource = new UIEventSource(null) - const hasWays: UIEventSource = new UIEventSource(null) - deletetionAllowed.addCallbackAndRunD(deletetionAllowed => { - - if (deletetionAllowed === false) { - // Nope, we are not allowed to delete - state.setData({ - canBeDeleted: false, - reason: t.notEnoughExperience - }) - return true; // unregister this caller! - } - - if (!useTheInternet) { - return; - } - - // All right! We have arrived at a point that we should query OSM again to check that the point isn't a part of ways or relations - OsmObject.DownloadReferencingRelations(id).then(rels => { - hasRelations.setData(rels.length > 0) - }) - - OsmObject.DownloadReferencingWays(id).then(ways => { - hasWays.setData(ways.length > 0) - }) - return true; // unregister to only run once - }) - - - const hasWaysOrRelations = hasRelations.map(hasRelationsData => { - if (hasRelationsData === true) { - return true; - } - if (hasWays.data === true) { - return true; - } - if (hasWays.data === null || hasRelationsData === null) { - return null; - } - if (hasWays.data === false && hasRelationsData === false) { - return false; - } - return null; - }, [hasWays]) - - hasWaysOrRelations.addCallbackAndRun( - waysOrRelations => { - if (waysOrRelations == null) { - // Not yet loaded - we still wait a little bit - return; - } - if (waysOrRelations) { - // not deleteble by mapcomplete - state.setData({ - canBeDeleted: false, - reason: t.partOfOthers - }) - } else { - // alright, this point can be safely deleted! - state.setData({ - canBeDeleted: true, - reason: allByMyself.data === true ? t.onlyEditedByLoggedInUser : t.safeDelete - }) - } - - - } - ) - - } - } \ No newline at end of file diff --git a/Logic/Osm/Actions/RelationSplitHandler.ts b/Logic/Osm/Actions/RelationSplitHandler.ts index 42562c035..00c5a6706 100644 --- a/Logic/Osm/Actions/RelationSplitHandler.ts +++ b/Logic/Osm/Actions/RelationSplitHandler.ts @@ -16,14 +16,16 @@ export interface RelationSplitInput { */ export default class RelationSplitHandler extends OsmChangeAction { private readonly _input: RelationSplitInput; + private readonly _theme: string; - constructor(input: RelationSplitInput) { + constructor(input: RelationSplitInput, theme: string) { super() this._input = input; + this._theme = theme; } async CreateChangeDescriptions(changes: Changes): Promise { - return new InPlaceReplacedmentRTSH(this._input).CreateChangeDescriptions(changes) + return new InPlaceReplacedmentRTSH(this._input, this._theme).CreateChangeDescriptions(changes) } @@ -39,10 +41,12 @@ export default class RelationSplitHandler extends OsmChangeAction { */ export class InPlaceReplacedmentRTSH extends OsmChangeAction { private readonly _input: RelationSplitInput; + private readonly _theme: string; - constructor(input: RelationSplitInput) { + constructor(input: RelationSplitInput, theme: string) { super(); this._input = input; + this._theme = theme; } /** @@ -137,7 +141,11 @@ export class InPlaceReplacedmentRTSH extends OsmChangeAction { return [{ id: relation.id, type: "relation", - changes: {members: newMembers} + changes: {members: newMembers}, + meta:{ + changeType: "relation-fix", + theme: this._theme + } }]; } diff --git a/Logic/Osm/Actions/SplitAction.ts b/Logic/Osm/Actions/SplitAction.ts index 085aab413..740bc13f2 100644 --- a/Logic/Osm/Actions/SplitAction.ts +++ b/Logic/Osm/Actions/SplitAction.ts @@ -14,16 +14,19 @@ interface SplitInfo { export default class SplitAction extends OsmChangeAction { private readonly wayId: string; private readonly _splitPointsCoordinates: [number, number] []// lon, lat + private _meta: { theme: string, changeType: "split" }; /** * * @param wayId * @param splitPointCoordinates: lon, lat + * @param meta */ - constructor(wayId: string, splitPointCoordinates: [number, number][]) { + constructor(wayId: string, splitPointCoordinates: [number, number][], meta: {theme: string}) { super() this.wayId = wayId; this._splitPointsCoordinates = splitPointCoordinates + this._meta = {...meta, changeType: "split"}; } private static SegmentSplitInfo(splitInfo: SplitInfo[]): SplitInfo[][] { @@ -89,7 +92,8 @@ export default class SplitAction extends OsmChangeAction { changes: { lon: element.lngLat[0], lat: element.lngLat[1] - } + }, + meta: this._meta }) } @@ -110,7 +114,8 @@ export default class SplitAction extends OsmChangeAction { changes: { coordinates: wayPart.map(p => p.lngLat), nodes: nodeIds - } + }, + meta: this._meta }) allWayIdsInOrder.push(originalElement.id) allWaysNodesInOrder.push(nodeIds) @@ -135,7 +140,8 @@ export default class SplitAction extends OsmChangeAction { changes: { coordinates: wayPart.map(p => p.lngLat), nodes: nodeIds - } + }, + meta: this._meta }) allWayIdsInOrder.push(id) @@ -152,8 +158,8 @@ export default class SplitAction extends OsmChangeAction { allWayIdsInOrder: allWayIdsInOrder, originalNodes: originalNodes, allWaysNodesInOrder: allWaysNodesInOrder, - originalWayId: originalElement.id - }).CreateChangeDescriptions(changes) + originalWayId: originalElement.id, + }, this._meta.theme).CreateChangeDescriptions(changes) changeDescription.push(...changDescrs) } @@ -240,7 +246,6 @@ export default class SplitAction extends OsmChangeAction { closest = prevPoint } // Ok, we have a closest point! - if(closest.originalIndex === 0 || closest.originalIndex === originalPoints.length){ // We can not split on the first or last points... continue diff --git a/Logic/Osm/Changes.ts b/Logic/Osm/Changes.ts index 866c78c7a..6c90cccbb 100644 --- a/Logic/Osm/Changes.ts +++ b/Logic/Osm/Changes.ts @@ -64,9 +64,9 @@ export class Changes { if (deletedElements.length > 0) { changes += - "\n\n" + + "\n\n" + deletedElements.map(e => e.ChangesetXML(csId)).join("\n") + - "\n" + "\n" } changes += ""; @@ -99,7 +99,7 @@ export class Changes { } this.isUploading.setData(true) - this.flushChangesAsync(flushreason) + this.flushChangesAsync() .then(_ => { this.isUploading.setData(false) console.log("Changes flushed!"); @@ -110,39 +110,94 @@ export class Changes { }) } - private async flushChangesAsync(flushreason: string = undefined): Promise { + /** + * UPload the selected changes to OSM. + * Returns 'true' if successfull and if they can be removed + * @param pending + * @private + */ + private async flushSelectChanges(pending: ChangeDescription[]): Promise{ + const self = this; + const neededIds = Changes.GetNeededIds(pending) + const osmObjects = await Promise.all(neededIds.map(id => OsmObject.DownloadObjectAsync(id))); + console.log("Got the fresh objects!", osmObjects, "pending: ", pending) + const changes: { + newObjects: OsmObject[], + modifiedObjects: OsmObject[] + deletedObjects: OsmObject[] + } = self.CreateChangesetObjects(pending, osmObjects) + if (changes.newObjects.length + changes.deletedObjects.length + changes.modifiedObjects.length === 0) { + console.log("No changes to be made") + return true + } + + const meta = pending[0].meta + + const perType = Array.from(Utils.Hist(pending.map(descr => descr.meta.changeType)), ([key, count]) => ({ + key: key, + value: count, + aggregate: true + })) + const motivations = pending.filter(descr => descr.meta.specialMotivation !== undefined) + .map(descr => ({ + key: descr.meta.changeType+":"+descr.type+"/"+descr.id, + value: descr.meta.specialMotivation + })) + const metatags = [{ + key: "comment", + value: "Adding data with #MapComplete for theme #"+meta.theme + }, + { + key:"theme", + value:meta.theme + }, + ...perType, + ...motivations + ] + + await State.state.osmConnection.changesetHandler.UploadChangeset( + (csId) => Changes.createChangesetFor(""+csId, changes), + metatags + ) + + console.log("Upload successfull!") + return true; + } + + private async flushChangesAsync(): Promise { const self = this; try { - console.log("Beginning upload... " + flushreason ?? ""); // At last, we build the changeset and upload const pending = self.pendingChanges.data; - const neededIds = Changes.GetNeededIds(pending) - const osmObjects = await Promise.all(neededIds.map(id => OsmObject.DownloadObjectAsync(id))); - console.log("Got the fresh objects!", osmObjects, "pending: ", pending) - const changes: { - newObjects: OsmObject[], - modifiedObjects: OsmObject[] - deletedObjects: OsmObject[] - } = self.CreateChangesetObjects(pending, osmObjects) - if (changes.newObjects.length + changes.deletedObjects.length + changes.modifiedObjects.length === 0) { - console.log("No changes to be made") - self.pendingChanges.setData([]) - self.isUploading.setData(false) + + const pendingPerTheme = new Map() + for (const changeDescription of pending) { + const theme = changeDescription.meta.theme + if(!pendingPerTheme.has(theme)){ + pendingPerTheme.set(theme, []) + } + pendingPerTheme.get(theme).push(changeDescription) + } + + const successes = await Promise.all(Array.from(pendingPerTheme, ([key , value]) => value) + .map(async pendingChanges => { + try{ + return await self.flushSelectChanges(pendingChanges); + }catch(e){ + console.error("Could not upload some changes:",e) + return false + } + })) + + if(!successes.some(s => s == false)){ + // All changes successfull, we clear the data! + this.pendingChanges.setData([]); } - - await State.state.osmConnection.UploadChangeset( - State.state.layoutToUse, - State.state.allElements, - (csId) => Changes.createChangesetFor(csId, changes), - ) - - console.log("Upload successfull!") - this.pendingChanges.setData([]); - this.isUploading.setData(false) } catch (e) { console.error("Could not handle changes - probably an old, pending changeset in localstorage with an invalid format; erasing those", e) self.pendingChanges.setData([]) + }finally { self.isUploading.setData(false) } diff --git a/Logic/Osm/ChangesetHandler.ts b/Logic/Osm/ChangesetHandler.ts index aae51d4ee..4357fa054 100644 --- a/Logic/Osm/ChangesetHandler.ts +++ b/Logic/Osm/ChangesetHandler.ts @@ -6,29 +6,47 @@ import {ElementStorage} from "../ElementStorage"; import State from "../../State"; import Locale from "../../UI/i18n/Locale"; import Constants from "../../Models/Constants"; -import {OsmObject} from "./OsmObject"; -import LayoutConfig from "../../Models/ThemeConfig/LayoutConfig"; import {Changes} from "./Changes"; +import {Utils} from "../../Utils"; + +export interface ChangesetTag { + key: string, + value: string | number, + aggregate?: boolean +} export class ChangesetHandler { - public readonly currentChangeset: UIEventSource; + public readonly currentChangeset: UIEventSource; private readonly allElements: ElementStorage; + private osmConnection: OsmConnection; private readonly changes: Changes; private readonly _dryRun: boolean; private readonly userDetails: UIEventSource; private readonly auth: any; + private readonly backend: string; - constructor(layoutName: string, dryRun: boolean, osmConnection: OsmConnection, + constructor(layoutName: string, dryRun: boolean, + osmConnection: OsmConnection, allElements: ElementStorage, changes: Changes, auth) { + this.osmConnection = osmConnection; this.allElements = allElements; this.changes = changes; this._dryRun = dryRun; this.userDetails = osmConnection.userDetails; + this.backend = osmConnection._oauth_config.url this.auth = auth; - this.currentChangeset = osmConnection.GetPreference("current-open-changeset-" + layoutName); + this.currentChangeset = osmConnection.GetPreference("current-open-changeset-" + layoutName).map( + str => { + const n = Number(str); + if (isNaN(n)) { + return undefined + } + return n + }, [], n => "" + n + ); if (dryRun) { console.log("DRYRUN ENABLED"); @@ -39,7 +57,7 @@ export class ChangesetHandler { const oldId = parseInt(node.attributes.old_id.value); if (node.attributes.new_id === undefined) { // We just removed this point! - const element =this. allElements.getEventSourceById("node/" + oldId); + const element = this.allElements.getEventSourceById("node/" + oldId); element.data._deleted = "yes" element.ping(); return; @@ -56,6 +74,10 @@ export class ChangesetHandler { } console.log("Rewriting id: ", type + "/" + oldId, "-->", type + "/" + newId); const element = this.allElements.getEventSourceById("node/" + oldId); + if(element === undefined){ + // Element to rewrite not found, probably a node or relation that is not rendered + return undefined + } element.data.id = type + "/" + newId; this.allElements.addElementById(type + "/" + newId, element); this.allElements.ContainingFeatures.set(type + "/" + newId, this.allElements.ContainingFeatures.get(type + "/" + oldId)) @@ -83,7 +105,7 @@ export class ChangesetHandler { } } this.changes.registerIdRewrites(mappings) - + } /** @@ -97,102 +119,96 @@ export class ChangesetHandler { * */ public async UploadChangeset( - layout: LayoutConfig, - generateChangeXML: (csid: string) => string): Promise { + generateChangeXML: (csid: number) => string, + extraMetaTags: ChangesetTag[]): Promise { + + 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`" + } + if (this.userDetails.data.csCount == 0) { // The user became a contributor! this.userDetails.data.csCount = 1; this.userDetails.ping(); } - if (this._dryRun) { - const changesetXML = generateChangeXML("123456"); + const changesetXML = generateChangeXML(123456); console.log(changesetXML); return; } - if (this.currentChangeset.data === undefined || this.currentChangeset.data === "") { + if (this.currentChangeset.data === undefined) { // We have to open a new changeset try { - const csId = await this.OpenChangeset(layout) + const csId = await this.OpenChangeset(extraMetaTags) this.currentChangeset.setData(csId); const changeset = generateChangeXML(csId); console.log("Current changeset is:", changeset); await this.AddChange(csId, changeset) } catch (e) { console.error("Could not open/upload changeset due to ", e) - this.currentChangeset.setData("") + this.currentChangeset.setData(undefined) } } else { // There still exists an open changeset (or at least we hope so) + // Let's check! const csId = this.currentChangeset.data; try { + const oldChangesetMeta = await this.GetChangesetMeta(csId) + if (!oldChangesetMeta.open) { + // Mark the CS as closed... + this.currentChangeset.setData(undefined); + // ... and try again. As the cs is closed, no recursive loop can exist + await this.UploadChangeset(generateChangeXML, extraMetaTags) + return; + } + + const extraTagsById = new Map() + for (const extraMetaTag of extraMetaTags) { + extraTagsById.set(extraMetaTag.key, extraMetaTag) + } + const oldCsTags = oldChangesetMeta.tags + for (const key in oldCsTags) { + const newMetaTag = extraTagsById.get(key) + if (newMetaTag === undefined) { + extraMetaTags.push({ + key: key, + value: oldCsTags[key] + }) + } else if (newMetaTag.aggregate) { + let n = Number(newMetaTag.value) + if (isNaN(n)) { + n = 0 + } + let o = Number(oldCsTags[key]) + if (isNaN(o)) { + o = 0 + } + // We _update_ the tag itself, as it'll be updated in 'extraMetaTags' straight away + newMetaTag.value = "" + (n + o) + } else { + // The old value is overwritten, thus we drop + } + } + + await this.UpdateTags(csId, extraMetaTags.map(csTag => <[string, string]>[csTag.key, csTag.value])) + + await this.AddChange( csId, generateChangeXML(csId)) + + } catch (e) { console.warn("Could not upload, changeset is probably closed: ", e); - // Mark the CS as closed... - this.currentChangeset.setData(""); - // ... and try again. As the cs is closed, no recursive loop can exist - await this.UploadChangeset(layout, generateChangeXML) + this.currentChangeset.setData(undefined); } } } - /** - * Deletes the element with the given ID from the OSM database. - * DOES NOT PERFORM ANY SAFETY CHECKS! - * - * For the deletion of an element, a new, separate changeset is created with a slightly changed comment and some extra flags set. - * The CS will be closed afterwards. - * - * If dryrun is specified, will not actually delete the point but print the CS-XML to console instead - * - */ - public DeleteElement(object: OsmObject, - layout: LayoutConfig, - reason: string, - allElements: ElementStorage, - continuation: () => void) { - return this.DeleteElementAsync(object, layout, reason, allElements).then(continuation) - } - - public async DeleteElementAsync(object: OsmObject, - layout: LayoutConfig, - reason: string, - allElements: ElementStorage): Promise { - - function generateChangeXML(csId: string) { - let [lat, lon] = object.centerpoint(); - - let changes = ``; - changes += - `<${object.type} id="${object.id}" version="${object.version}" changeset="${csId}" lat="${lat}" lon="${lon}" />`; - changes += ""; - return changes; - } - - - if (this._dryRun) { - const changesetXML = generateChangeXML("123456"); - console.log(changesetXML); - return; - } - - const csId = await this.OpenChangeset(layout, { - isDeletionCS: true, - deletionReason: reason - }) - // The cs is open - let us actually upload! - const changes = generateChangeXML(csId) - await this.AddChange(csId, changes) - await this.CloseChangeset(csId) - } - - private async CloseChangeset(changesetId: string = undefined): Promise { + private async CloseChangeset(changesetId: number = undefined): Promise { const self = this return new Promise(function (resolve, reject) { if (changesetId === undefined) { @@ -202,7 +218,7 @@ export class ChangesetHandler { return; } console.log("closing changeset", changesetId); - self.currentChangeset.setData(""); + self.currentChangeset.setData(undefined); self.auth.xhr({ method: 'PUT', path: '/api/0.6/changeset/' + changesetId + '/close', @@ -217,39 +233,63 @@ export class ChangesetHandler { }) } - private OpenChangeset( - layout: LayoutConfig, - options?: { - isDeletionCS?: boolean, - deletionReason?: string, - } - ): Promise { + private async GetChangesetMeta(csId: number): Promise<{ + id: number, + open: boolean, + uid: number, + changes_count: number, + tags: any + }> { + const url = `${this.backend}/api/0.6/changeset/${csId}` + const csData = await Utils.downloadJson(url) + return csData.elements[0] + } + + private async UpdateTags( + csId: number, + tags: [string, string][]) { + const self = this; return new Promise(function (resolve, reject) { - options = options ?? {} - options.isDeletionCS = options.isDeletionCS ?? false - const commentExtra = layout.changesetmessage !== undefined ? " - " + layout.changesetmessage : ""; - let comment = `Adding data with #MapComplete for theme #${layout.id}${commentExtra}` - if (options.isDeletionCS) { - comment = `Deleting a point with #MapComplete for theme #${layout.id}${commentExtra}` - if (options.deletionReason) { - comment += ": " + options.deletionReason; + + tags = Utils.NoNull(tags).filter(([k, v]) => k !== undefined && v !== undefined && k !== "" && v !== "") + const metadata = tags.map(kv => ``) + + self.auth.xhr({ + method: 'PUT', + path: '/api/0.6/changeset/' + csId, + options: {header: {'Content-Type': 'text/xml'}}, + content: [``, + metadata, + ``].join("") + }, function (err, response) { + if (response === undefined) { + console.log("err", err); + reject(err) + } else { + resolve(response); } - } + }); + }) + + } + + private OpenChangeset( + changesetTags: ChangesetTag[] + ): Promise { + const self = this; + return new Promise(function (resolve, reject) { let path = window.location.pathname; path = path.substr(1, path.lastIndexOf("/")); const metadata = [ ["created_by", `MapComplete ${Constants.vNumber}`], - ["comment", comment], - ["deletion", options.isDeletionCS ? "yes" : undefined], - ["theme", layout.id], ["language", Locale.language.data], ["host", window.location.host], ["path", path], ["source", State.state.currentGPSLocation.data !== undefined ? "survey" : undefined], ["imagery", State.state.backgroundLayer.data.id], - ["theme-creator", layout.maintainer] + ...changesetTags.map(cstag => [cstag.key, cstag.value]) ] .filter(kv => (kv[1] ?? "") !== "") .map(kv => ``) @@ -268,7 +308,7 @@ export class ChangesetHandler { console.log("err", err); reject(err) } else { - resolve(response); + resolve(Number(response)); } }); }) @@ -278,8 +318,8 @@ export class ChangesetHandler { /** * Upload a changesetXML */ - private AddChange(changesetId: string, - changesetXML: string): Promise { + private AddChange(changesetId: number, + changesetXML: string): Promise { const self = this; return new Promise(function (resolve, reject) { self.auth.xhr({ diff --git a/Logic/Osm/OsmConnection.ts b/Logic/Osm/OsmConnection.ts index 4e1a9297a..c0795aad0 100644 --- a/Logic/Osm/OsmConnection.ts +++ b/Logic/Osm/OsmConnection.ts @@ -124,13 +124,6 @@ export class OsmConnection { } } - public UploadChangeset( - layout: LayoutConfig, - allElements: ElementStorage, - generateChangeXML: (csid: string) => string): Promise { - return this.changesetHandler.UploadChangeset(layout, generateChangeXML); - } - public GetPreference(key: string, prefix: string = "mapcomplete-"): UIEventSource { return this.preferencesHandler.GetPreference(key, prefix); } diff --git a/Logic/Osm/OsmObject.ts b/Logic/Osm/OsmObject.ts index 9c1efbae9..2c88b01c5 100644 --- a/Logic/Osm/OsmObject.ts +++ b/Logic/Osm/OsmObject.ts @@ -11,7 +11,7 @@ export abstract class OsmObject { private static polygonFeatures = OsmObject.constructPolygonFeatures() private static objectCache = new Map>(); private static historyCache = new Map>(); - type: string; + type: "node" | "way" | "relation"; id: number; /** * The OSM tags as simple object @@ -23,6 +23,7 @@ export abstract class OsmObject { protected constructor(type: string, id: number) { this.id = id; + // @ts-ignore this.type = type; this.tags = { id: `${this.type}/${id}` diff --git a/Models/Constants.ts b/Models/Constants.ts index 0daf78e2e..50ee3543a 100644 --- a/Models/Constants.ts +++ b/Models/Constants.ts @@ -14,7 +14,7 @@ export default class Constants { "https://overpass.kumi.systems/api/interpreter", // Offline: "https://overpass.nchc.org.tw/api/interpreter", "https://overpass.openstreetmap.ru/cgi/interpreter", - // Doesn't support nwr "https://overpass.openstreetmap.fr/api/interpreter" + // Doesn't support nwr: "https://overpass.openstreetmap.fr/api/interpreter" ] diff --git a/Models/ThemeConfig/Json/LayoutConfigJson.ts b/Models/ThemeConfig/Json/LayoutConfigJson.ts index 89439f529..40621bcd1 100644 --- a/Models/ThemeConfig/Json/LayoutConfigJson.ts +++ b/Models/ThemeConfig/Json/LayoutConfigJson.ts @@ -36,10 +36,7 @@ export interface LayoutConfigJson { * Who does maintian this preset? */ maintainer: string; - /** - * Extra piece of text that can be added to the changeset - */ - changesetmessage?: string; + /** * A version number, either semantically or by date. * Should be sortable, where the higher value is the later version diff --git a/Models/ThemeConfig/LayoutConfig.ts b/Models/ThemeConfig/LayoutConfig.ts index 8b10f6c54..06d3af4ad 100644 --- a/Models/ThemeConfig/LayoutConfig.ts +++ b/Models/ThemeConfig/LayoutConfig.ts @@ -12,7 +12,6 @@ export default class LayoutConfig { public readonly id: string; public readonly maintainer: string; public readonly credits?: string; - public readonly changesetmessage?: string; public readonly version: string; public readonly language: string[]; public readonly title: Translation; @@ -61,7 +60,6 @@ export default class LayoutConfig { context = (context ?? "") + "." + this.id; this.maintainer = json.maintainer; this.credits = json.credits; - this.changesetmessage = json.changesetmessage; this.version = json.version; this.language = []; if (typeof json.language === "string") { diff --git a/UI/BigComponents/ImportButton.ts b/UI/BigComponents/ImportButton.ts index 160405264..cf5210006 100644 --- a/UI/BigComponents/ImportButton.ts +++ b/UI/BigComponents/ImportButton.ts @@ -37,7 +37,10 @@ export default class ImportButton extends Toggle { } originalTags.data["_imported"] = "yes" originalTags.ping() // will set isImported as per its definition - const newElementAction = new CreateNewNodeAction(newTags.data, lat, lon) + const newElementAction = new CreateNewNodeAction(newTags.data, lat, lon, { + theme: State.state.layoutToUse.id, + changeType: "import" + }) await State.state.changes.applyAction(newElementAction) State.state.selectedElement.setData(State.state.allElements.ContainingFeatures.get( newElementAction.newElementId diff --git a/UI/BigComponents/SimpleAddUI.ts b/UI/BigComponents/SimpleAddUI.ts index ad6a73ec5..2ab3b75f3 100644 --- a/UI/BigComponents/SimpleAddUI.ts +++ b/UI/BigComponents/SimpleAddUI.ts @@ -56,7 +56,10 @@ export default class SimpleAddUI extends Toggle { async function createNewPoint(tags: any[], location: { lat: number, lon: number }, snapOntoWay?: OsmWay) { - const newElementAction = new CreateNewNodeAction(tags, location.lat, location.lon, {snapOnto: snapOntoWay}) + const newElementAction = new CreateNewNodeAction(tags, location.lat, location.lon, { + theme: State.state?.layoutToUse?.id ?? "unkown", + changeType: "create", + snapOnto: snapOntoWay}) await State.state.changes.applyAction(newElementAction) selectedPreset.setData(undefined) isShown.setData(false) diff --git a/UI/Popup/DeleteWizard.ts b/UI/Popup/DeleteWizard.ts index 6025ff890..fec675e84 100644 --- a/UI/Popup/DeleteWizard.ts +++ b/UI/Popup/DeleteWizard.ts @@ -4,7 +4,6 @@ import Toggle from "../Input/Toggle"; import Translations from "../i18n/Translations"; import Svg from "../../Svg"; import DeleteAction from "../../Logic/Osm/Actions/DeleteAction"; -import {Tag} from "../../Logic/Tags/Tag"; import {UIEventSource} from "../../Logic/UIEventSource"; import {TagsFilter} from "../../Logic/Tags/TagsFilter"; import TagRenderingQuestion from "./TagRenderingQuestion"; @@ -13,13 +12,11 @@ import {SubtleButton} from "../Base/SubtleButton"; import {FixedUiElement} from "../Base/FixedUiElement"; import {Translation} from "../i18n/Translation"; import BaseUIElement from "../BaseUIElement"; -import {Changes} from "../../Logic/Osm/Changes"; -import {And} from "../../Logic/Tags/And"; import Constants from "../../Models/Constants"; -import ChangeTagAction from "../../Logic/Osm/Actions/ChangeTagAction"; import TagRenderingConfig from "../../Models/ThemeConfig/TagRenderingConfig"; import {AndOrTagConfigJson} from "../../Models/ThemeConfig/Json/TagConfigJson"; import DeleteConfig from "../../Models/ThemeConfig/DeleteConfig"; +import {OsmObject} from "../../Logic/Osm/OsmObject"; export default class DeleteWizard extends Toggle { /** @@ -43,44 +40,32 @@ export default class DeleteWizard extends Toggle { constructor(id: string, options: DeleteConfig) { - const deleteAction = new DeleteAction(id, options.neededChangesets); + const deleteAbility = new DeleteabilityChecker(id, options.neededChangesets) const tagsSource = State.state.allElements.getEventSourceById(id) + const isDeleted = new UIEventSource(false) const allowSoftDeletion = !!options.softDeletionTags const confirm = new UIEventSource(false) - async function softDelete(reason: string, tagsToApply: { k: string, v: string }[]) { - if (reason !== undefined) { - tagsToApply.splice(0, 0, { - k: "fixme", - v: `A mapcomplete user marked this feature to be deleted (${reason})` - }) - } - await (State.state?.changes ?? new Changes()) - .applyAction(new ChangeTagAction( - id, new And(tagsToApply.map(kv => new Tag(kv.k, kv.v))), tagsSource.data - )) - } - function doDelete(selected: TagsFilter) { + // Selected == the reasons, not the tags of the object const tgs = selected.asChange(tagsSource.data) const deleteReasonMatch = tgs.filter(kv => kv.k === "_delete_reason") - if (deleteReasonMatch.length > 0) { - // We should actually delete! - const deleteReason = deleteReasonMatch[0].v - deleteAction.DoDelete(deleteReason, () => { - // The user doesn't have sufficient permissions to _actually_ delete the feature - // We 'soft delete' instead (and add a fixme) - softDelete(deleteReason, tgs.filter(kv => kv.k !== "_delete_reason")) - - }); - return - } else { - // This is a 'non-delete'-option that was selected - softDelete(undefined, tgs) + if (deleteReasonMatch.length === 0) { + return; } + const deleteAction = new DeleteAction(id, + options.softDeletionTags, + { + theme: State.state?.layoutToUse?.id ?? "unkown", + specialMotivation: deleteReasonMatch[0]?.v + }, + deleteAbility.canBeDeleted.data.canBeDeleted + ) + State.state.changes.applyAction(deleteAction) + isDeleted.setData(true) } @@ -98,7 +83,7 @@ export default class DeleteWizard extends Toggle { saveButtonConstr: (v) => DeleteWizard.constructConfirmButton(v).onClick(() => { doDelete(v.data) }), - bottomText: (v) => DeleteWizard.constructExplanation(v, deleteAction) + bottomText: (v) => DeleteWizard.constructExplanation(v, deleteAbility) } ) })) @@ -110,7 +95,7 @@ export default class DeleteWizard extends Toggle { const deleteButton = new SubtleButton( Svg.delete_icon_ui().SetStyle("width: 2rem; height: 2rem;"), t.delete.Clone()).onClick( () => { - deleteAction.CheckDeleteability(true) + deleteAbility.CheckDeleteability(true) confirm.setData(true); } ).SetClass("w-1/2 float-right"); @@ -132,13 +117,13 @@ export default class DeleteWizard extends Toggle { deleteButton, confirm), - new VariableUiElement(deleteAction.canBeDeleted.map(cbd => new Combine([cbd.reason.Clone(), t.useSomethingElse.Clone()]))), - deleteAction.canBeDeleted.map(cbd => allowSoftDeletion || cbd.canBeDeleted !== false)), + new VariableUiElement(deleteAbility.canBeDeleted.map(cbd => new Combine([cbd.reason.Clone(), t.useSomethingElse.Clone()]))), + deleteAbility.canBeDeleted.map(cbd => allowSoftDeletion || cbd.canBeDeleted !== false)), t.loginToDelete.Clone().onClick(State.state.osmConnection.AttemptLogin), State.state.osmConnection.isLoggedIn ), - deleteAction.isDeleted), + isDeleted), undefined, isShown) @@ -167,7 +152,7 @@ export default class DeleteWizard extends Toggle { } - private static constructExplanation(tags: UIEventSource, deleteAction: DeleteAction) { + private static constructExplanation(tags: UIEventSource, deleteAction: DeleteabilityChecker) { const t = Translations.t.delete; return new VariableUiElement(tags.map( currentTags => { @@ -263,4 +248,172 @@ export default class DeleteWizard extends Toggle { ) } +} + +class DeleteabilityChecker { + + public readonly canBeDeleted: UIEventSource<{ canBeDeleted?: boolean, reason: Translation }>; + private readonly _id: string; + private readonly _allowDeletionAtChangesetCount: number; + + + constructor(id: string, + allowDeletionAtChangesetCount?: number) { + this._id = id; + this._allowDeletionAtChangesetCount = allowDeletionAtChangesetCount ?? Number.MAX_VALUE; + + this.canBeDeleted = new UIEventSource<{ canBeDeleted?: boolean; reason: Translation }>({ + canBeDeleted: undefined, + reason: Translations.t.delete.loading + }) + this.CheckDeleteability(false) + } + + /** + * Checks if the currently logged in user can delete the current point. + * State is written into this._canBeDeleted + * @constructor + * @private + */ + public CheckDeleteability(useTheInternet: boolean): void { + const t = Translations.t.delete; + const id = this._id; + const state = this.canBeDeleted + if (!id.startsWith("node")) { + this.canBeDeleted.setData({ + canBeDeleted: false, + reason: t.isntAPoint + }) + return; + } + + // Does the currently logged in user have enough experience to delete this point? + + const deletingPointsOfOtherAllowed = State.state.osmConnection.userDetails.map(ud => { + if (ud === undefined) { + return undefined; + } + if (!ud.loggedIn) { + return false; + } + return ud.csCount >= Math.min(Constants.userJourney.deletePointsOfOthersUnlock, this._allowDeletionAtChangesetCount); + }) + + const previousEditors = new UIEventSource(undefined) + + const allByMyself = previousEditors.map(previous => { + if (previous === null || previous === undefined) { + // Not yet downloaded + return null; + } + const userId = State.state.osmConnection.userDetails.data.uid; + return !previous.some(editor => editor !== userId) + }, [State.state.osmConnection.userDetails]) + + + // User allowed OR only edited by self? + const deletetionAllowed = deletingPointsOfOtherAllowed.map(isAllowed => { + if (isAllowed === undefined) { + // No logged in user => definitively not allowed to delete! + return false; + } + if (isAllowed === true) { + return true; + } + + // At this point, the logged in user is not allowed to delete points created/edited by _others_ + // however, we query OSM and if it turns out the current point has only be edited by the current user, deletion is allowed after all! + + if (allByMyself.data === null && useTheInternet) { + // We kickoff the download here as it hasn't yet been downloaded. Note that this is mapped onto 'all by myself' above + OsmObject.DownloadHistory(id).map(versions => versions.map(version => version.tags["_last_edit:contributor:uid"])).syncWith(previousEditors) + } + if (allByMyself.data === true) { + // Yay! We can download! + return true; + } + if (allByMyself.data === false) { + // Nope, downloading not allowed... + return false; + } + + + // At this point, we don't have enough information yet to decide if the user is allowed to delete the current point... + return undefined; + }, [allByMyself]) + + + const hasRelations: UIEventSource = new UIEventSource(null) + const hasWays: UIEventSource = new UIEventSource(null) + deletetionAllowed.addCallbackAndRunD(deletetionAllowed => { + + if (deletetionAllowed === false) { + // Nope, we are not allowed to delete + state.setData({ + canBeDeleted: false, + reason: t.notEnoughExperience + }) + return true; // unregister this caller! + } + + if (!useTheInternet) { + return; + } + + // All right! We have arrived at a point that we should query OSM again to check that the point isn't a part of ways or relations + OsmObject.DownloadReferencingRelations(id).then(rels => { + hasRelations.setData(rels.length > 0) + }) + + OsmObject.DownloadReferencingWays(id).then(ways => { + hasWays.setData(ways.length > 0) + }) + return true; // unregister to only run once + }) + + + const hasWaysOrRelations = hasRelations.map(hasRelationsData => { + if (hasRelationsData === true) { + return true; + } + if (hasWays.data === true) { + return true; + } + if (hasWays.data === null || hasRelationsData === null) { + return null; + } + if (hasWays.data === false && hasRelationsData === false) { + return false; + } + return null; + }, [hasWays]) + + hasWaysOrRelations.addCallbackAndRun( + waysOrRelations => { + if (waysOrRelations == null) { + // Not yet loaded - we still wait a little bit + return; + } + if (waysOrRelations) { + // not deleteble by mapcomplete + state.setData({ + canBeDeleted: false, + reason: t.partOfOthers + }) + } else { + // alright, this point can be safely deleted! + state.setData({ + canBeDeleted: true, + reason: allByMyself.data === true ? t.onlyEditedByLoggedInUser : t.safeDelete + }) + } + + + } + ) + + + } + + } \ No newline at end of file diff --git a/UI/Popup/SplitRoadWizard.ts b/UI/Popup/SplitRoadWizard.ts index 17e1bd013..86e137fa3 100644 --- a/UI/Popup/SplitRoadWizard.ts +++ b/UI/Popup/SplitRoadWizard.ts @@ -136,7 +136,9 @@ export default class SplitRoadWizard extends Toggle { // Save button const saveButton = new Button(t.split.Clone(), () => { hasBeenSplit.setData(true) - State.state.changes.applyAction(new SplitAction(id, splitPoints.data.map(ff => ff.feature.geometry.coordinates))) + State.state.changes.applyAction(new SplitAction(id, splitPoints.data.map(ff => ff.feature.geometry.coordinates), { + theme: State.state?.layoutToUse?.id + })) }) saveButton.SetClass("btn btn-primary mr-3"); diff --git a/UI/Popup/TagRenderingQuestion.ts b/UI/Popup/TagRenderingQuestion.ts index a4eb83b0f..16811a3b1 100644 --- a/UI/Popup/TagRenderingQuestion.ts +++ b/UI/Popup/TagRenderingQuestion.ts @@ -86,7 +86,10 @@ export default class TagRenderingQuestion extends Combine { if (selection) { (State.state?.changes ?? new Changes()) .applyAction(new ChangeTagAction( - tags.data.id, selection, tags.data + tags.data.id, selection, tags.data, { + theme: State.state?.layoutToUse?.id ?? "unkown", + changeType: "answer", + } )).then(_ => { console.log("Tagchanges applied") }) diff --git a/Utils.ts b/Utils.ts index 5c04c9547..7763bc884 100644 --- a/Utils.ts +++ b/Utils.ts @@ -101,6 +101,14 @@ export class Utils { return ls; } + public static Hist(array: string[]): Map{ + const hist = new Map(); + for (const s of array) { + hist.set(s, 1 + (hist.get(s) ?? 0)) + } + return hist; + } + public static NoEmpty(array: string[]): string[] { const ls: string[] = []; for (const t of array) {