From 25b1e6fdb2149baa51e5fc430805a560bc5f6ac6 Mon Sep 17 00:00:00 2001 From: nyampire Date: Sun, 19 Jul 2026 17:09:07 +0900 Subject: [PATCH] fix(plateau): name MLIT_PLATEAU as the source, with its source_ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Edits made with this tool set only source=RapiD_Plateau_JP, which says what made the edit but not where the data came from. The parent import convention (wiki.openstreetmap.org/wiki/MLIT_PLATEAU/imports_outline) asks for source=MLIT_PLATEAU together with a source_ref pointing back at that page, so uploads were attributing the data to nothing in particular. Set both values. They answer different questions and the changeset can carry both: MLIT_PLATEAU is the data's origin, RapiD_Plateau_JP is the tool. source = aerial imagery;MLIT_PLATEAU;RapiD_Plateau_JP source_ref = https://wiki.openstreetmap.org/wiki/MLIT_PLATEAU/imports_outline source_ref is withdrawn again when the edit stops using Plateau data — the commit panel recomputes these on every render, so an undone transfer would otherwise leave behind a provenance claim the changeset no longer earns. A source_ref the user typed is never touched, on either path. The logic moved into modules/util/plateau_changeset_tags.js. It was unreachable from a test inside updateSessionChangesetTags(), a closure over context and editor; as a pure function it is directly testable, which is what let the idempotency and undo cases be written as tests first. --- modules/ui/commit.js | 18 ++++-- modules/util/index.js | 1 + modules/util/plateau_changeset_tags.js | 50 ++++++++++++++++ test/browser/util/plateau_changeset_tags.js | 63 +++++++++++++++++++++ 4 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 modules/util/plateau_changeset_tags.js create mode 100644 test/browser/util/plateau_changeset_tags.js diff --git a/modules/ui/commit.js b/modules/ui/commit.js index f6177f78d6..a19eda3868 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -10,7 +10,8 @@ import { uiChangesetEditor } from './changeset_editor.js'; import { uiSectionChanges } from './sections/changes.js'; import { uiCommitWarnings } from './commit_warnings.js'; import { uiSectionRawTagEditor } from './sections/raw_tag_editor.js'; -import { utilDetect, utilRebind } from '../util/index.js'; +import { utilApplyPlateauSourceTags, utilClearPlateauSourceRef, utilDetect, utilRebind } from '../util/index.js'; +import { PLATEAU_SOURCE, PLATEAU_TOOL_SOURCE } from '../util/plateau_changeset_tags.js'; const readOnlyTags = [ @@ -211,7 +212,7 @@ export function uiCommit(context) { const toRemove = [ 'aerial imagery', 'streetlevel imagery', 'mapillary', 'kartaview', 'streetside', - 'mapwithai', 'esri', 'RapiD_Plateau_JP', + 'mapwithai', 'esri', PLATEAU_SOURCE, PLATEAU_TOOL_SOURCE, ]; for (const v of toRemove) { sources.delete(v); @@ -257,19 +258,28 @@ export function uiCommit(context) { // Rapid, Esri, or Custom datasets // Update `data_used` tag let setDataUsed; + let usedPlateau = false; if (used.data.size) { for (const v of used.data) { const match = v.match(/(mapwithai|esri)/i); if (match !== null) { sources.add(match[1]); } - // Add Plateau-specific source tag when Plateau buildings data is used if (/plateau/i.test(v)) { - sources.add('RapiD_Plateau_JP'); + usedPlateau = true; } } setDataUsed = context.cleanTagValue(Array.from(used.data).filter(Boolean).join(';')); } + + // Record the data origin and the tool when Plateau buildings data is used. + // `source_ref` has to be withdrawn again when it isn't -- this runs on every + // render, so an undone transfer must not leave the provenance behind. + if (usedPlateau) { + utilApplyPlateauSourceTags(sources, tags); + } else { + utilClearPlateauSourceRef(tags); + } if (setDataUsed) { tags.data_used = setDataUsed; } else { diff --git a/modules/util/index.js b/modules/util/index.js index bfb1a740f8..f92445a2e1 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -15,3 +15,4 @@ export { utilSetTransform } from './util.js'; export { utilTotalExtent } from './util.js'; export { utilTriggerEvent } from './trigger_event.js'; export { utilBuildingRelationInfo } from './building_relation.js'; +export { utilApplyPlateauSourceTags, utilClearPlateauSourceRef } from './plateau_changeset_tags.js'; diff --git a/modules/util/plateau_changeset_tags.js b/modules/util/plateau_changeset_tags.js new file mode 100644 index 0000000000..b00cfbb480 --- /dev/null +++ b/modules/util/plateau_changeset_tags.js @@ -0,0 +1,50 @@ +// The parent PLATEAU import convention asks for both of these on the changeset: +// https://wiki.openstreetmap.org/wiki/MLIT_PLATEAU/imports_outline +// `source_ref` is what makes the provenance traceable, so naming MLIT_PLATEAU +// without it would satisfy only half the convention. +export const PLATEAU_SOURCE = 'MLIT_PLATEAU'; +export const PLATEAU_TOOL_SOURCE = 'RapiD_Plateau_JP'; +export const PLATEAU_SOURCE_REF = 'https://wiki.openstreetmap.org/wiki/MLIT_PLATEAU/imports_outline'; + + +/** + * utilApplyPlateauSourceTags + * + * Records that an edit used PLATEAU data, on the changeset tags. + * + * Two values, because they answer different questions: `MLIT_PLATEAU` is where + * the data came from (what the import convention asks for), `RapiD_Plateau_JP` + * is which tool made the edit. Joining is left to the caller, which already + * joins the whole source set with ';'. + * + * Mutates both arguments. Safe to call repeatedly — the commit panel re-runs + * its tag update on every render, not just the first. + * + * @param {Set} sources - the changeset `source` values being assembled + * @param {Object} tags - the changeset tags, for `source_ref` + */ +export function utilApplyPlateauSourceTags(sources, tags) { + sources.add(PLATEAU_SOURCE); + sources.add(PLATEAU_TOOL_SOURCE); + + // Don't overwrite a source_ref the user typed; only fill in our own. + if (!tags.source_ref) { + tags.source_ref = PLATEAU_SOURCE_REF; + } +} + + +/** + * utilClearPlateauSourceRef + * + * Removes the `source_ref` this module set, for when the edit no longer uses + * PLATEAU data (the user undid the transfer). Leaves any other value alone -- + * a `source_ref` we did not write is the user's, not ours to delete. + * + * @param {Object} tags - the changeset tags + */ +export function utilClearPlateauSourceRef(tags) { + if (tags.source_ref === PLATEAU_SOURCE_REF) { + delete tags.source_ref; + } +} diff --git a/test/browser/util/plateau_changeset_tags.js b/test/browser/util/plateau_changeset_tags.js new file mode 100644 index 0000000000..df9900496c --- /dev/null +++ b/test/browser/util/plateau_changeset_tags.js @@ -0,0 +1,63 @@ +describe('utilApplyPlateauSourceTags', () => { + const REF = 'https://wiki.openstreetmap.org/wiki/MLIT_PLATEAU/imports_outline'; + + it('names both the data source and the tool', () => { + const sources = new Set(); + const tags = {}; + Rapid.utilApplyPlateauSourceTags(sources, tags); + + // MLIT_PLATEAU is what the parent import convention asks for (the data's + // origin); RapiD_Plateau_JP identifies the tool that made the edit. + expect([...sources]).to.eql(['MLIT_PLATEAU', 'RapiD_Plateau_JP']); + }); + + it('sets source_ref to the import outline the convention pairs with the source', () => { + const sources = new Set(); + const tags = {}; + Rapid.utilApplyPlateauSourceTags(sources, tags); + expect(tags.source_ref).to.equal(REF); + }); + + it('keeps source values the user typed', () => { + const sources = new Set(['survey']); + const tags = {}; + Rapid.utilApplyPlateauSourceTags(sources, tags); + expect([...sources]).to.include('survey'); + }); + + it('is idempotent — the commit panel re-renders and calls this repeatedly', () => { + const sources = new Set(); + const tags = {}; + Rapid.utilApplyPlateauSourceTags(sources, tags); + Rapid.utilApplyPlateauSourceTags(sources, tags); + Rapid.utilApplyPlateauSourceTags(sources, tags); + + expect([...sources]).to.eql(['MLIT_PLATEAU', 'RapiD_Plateau_JP']); + expect(tags.source_ref).to.equal(REF); + }); + + it('does not clobber a source_ref the user set themselves', () => { + const sources = new Set(); + const tags = { source_ref: 'https://example.com/my-own-note' }; + Rapid.utilApplyPlateauSourceTags(sources, tags); + expect(tags.source_ref).to.equal('https://example.com/my-own-note'); + }); + + // The commit panel recomputes these tags on every render. If the user applies a + // Plateau transfer and then undoes it, our source_ref has to go away too -- + // otherwise the changeset claims a provenance the edit no longer has. + it('clears our own source_ref once Plateau data is no longer used', () => { + const tags = {}; + Rapid.utilApplyPlateauSourceTags(new Set(), tags); + expect(tags.source_ref).to.equal(REF); + + Rapid.utilClearPlateauSourceRef(tags); + expect(tags.source_ref).to.equal(undefined); + }); + + it('leaves a user-set source_ref alone when clearing', () => { + const tags = { source_ref: 'https://example.com/my-own-note' }; + Rapid.utilClearPlateauSourceRef(tags); + expect(tags.source_ref).to.equal('https://example.com/my-own-note'); + }); +});