Osmwithspace/MapComplete
1
0
Fork 0
forked from MapComplete/MapComplete
Code Pull requests Activity

Feat: remove 'noteImportLayer', they are not used enough for the big performance and maintenance impact they have

Browse source
This commit is contained in:
Pieter Vander Vennet 2024-10-12 12:57:09 +02:00
parent 48dc03b1e6
commit e35df654d7
10 changed files with 9 additions and 351 deletions
Download patch file Download diff file Expand all files Collapse all files

1
scripts/generateDocs.ts
View file

@ -459,7 +459,6 @@ export class GenerateDocs extends Script {
const allLayers = AllSharedLayers.getSharedLayersConfigs()
const layersToShow = theme.layers.filter(
(l) =>
!l.id.startsWith("note_import_") &&
l.id !== "favourite" &&
Constants.added_by_default.indexOf(<any>l.id) < 0
)

3
scripts/generateLayerOverview.ts
View file

@ -304,9 +304,6 @@ class LayerOverviewUtils extends Script {
if(sharedLayers.has(l.id)){
continue
}
if(l.id.startsWith("note_import")){
continue
}
LayerOverviewUtils.mergeKeywords(keywords, this.layerKeywords(l))
}

1
src/Logic/Actors/NoElementsInViewDetector.ts
View file

@ -16,7 +16,6 @@ export default class NoElementsInViewDetector {
const minZoom = Math.min(
...themeViewState.layout.layers
.filter((l) => Constants.priviliged_layers.indexOf(<any>l.id) < 0)
.filter((l) => !l.id.startsWith("note_import"))
.map((l) => l.minzoom)
)
const mapProperties = themeViewState.mapProperties

3
src/Logic/FeatureSource/TiledFeatureSource/SummaryTileSource.ts
View file

@ -24,8 +24,7 @@ export class SummaryTileSourceRewriter implements FeatureSource {
) {
this.filteredLayers = Array.from(filteredLayers.values()).filter(
(l) =>
Constants.priviliged_layers.indexOf(<any>l.layerDef.id) < 0 &&
!l.layerDef.id.startsWith("note_import")
Constants.priviliged_layers.indexOf(<any>l.layerDef.id) < 0
)
this._summarySource = summarySource
filteredLayers.forEach((v) => {

209
src/Models/ThemeConfig/Conversion/CreateNoteImportLayer.ts
View file

@ -1,209 +0,0 @@
import { Conversion } from "./Conversion"
import LayerConfig from "../LayerConfig"
import { LayerConfigJson } from "../Json/LayerConfigJson"
import Translations from "../../../UI/i18n/Translations"
import { Translation, TypedTranslation } from "../../../UI/i18n/Translation"
import { ConversionContext } from "./ConversionContext"
export default class CreateNoteImportLayer extends Conversion<LayerConfigJson, LayerConfigJson> {
/**
* A closed note is included if it is less then 'n'-days closed
* @private
*/
private readonly _includeClosedNotesDays: number
constructor(includeClosedNotesDays = 0) {
super(
[
"Advanced conversion which deducts a layer showing all notes that are 'importable' (i.e. a note that contains a link to some MapComplete theme, with hash '#import').",
"The import buttons and matches will be based on the presets of the given theme",
].join("\n\n"),
[],
"CreateNoteImportLayer"
)
this._includeClosedNotesDays = includeClosedNotesDays
}
convert(layerJson: LayerConfigJson, _: ConversionContext): LayerConfigJson {
const t = Translations.t.importLayer
/**
* The note itself will contain `tags=k=v;k=v;k=v;...
* This must be matched with a regex.
* This is a simple JSON-object as how it'll be put into the layerConfigJson directly
*/
const isShownIfAny: any[] = []
const layer = new LayerConfig(layerJson, "while constructing a note-import layer")
for (const preset of layer.presets) {
const mustMatchAll = []
for (const tag of preset.tags) {
const key = tag.key
const value = tag.value
const condition = "_tags~(^|.*;)" + key + "=" + value + "($|;.*)"
mustMatchAll.push(condition)
}
isShownIfAny.push({ and: mustMatchAll })
}
const title = layer.presets[0].title
const importButton = {}
{
const translations = trs(t.importButton, {
layerId: layer.id,
title: layer.presets[0].title,
})
for (const key in translations) {
if (key !== "_context") {
importButton[key] = "{" + translations[key] + "}"
} else {
importButton[key] = translations[key]
}
}
}
function embed(prefix, translation: Translation, postfix) {
const result = {}
for (const language in translation.translations) {
result[language] = prefix + translation.translations[language] + postfix
}
result["_context"] = translation.context
return result
}
function tr(translation: Translation) {
return { ...translation.translations, _context: translation.context }
}
function trs<T>(translation: TypedTranslation<T>, subs: T): Record<string, string> {
return { ...translation.Subs(subs).translations, _context: translation.context }
}
return {
id: "note_import_" + layer.id,
// By disabling the name, the import-layers won't pollute the filter view "name": t.layerName.Subs({title: layer.title.render}).translations,
description: trs(t.description, { title: layer.title.render }),
source: {
osmTags: {
and: ["id~[0-9]+", "comment_url~.*notes/[0-9]*/comment.json"],
},
geoJson:
"https://api.openstreetmap.org/api/0.6/notes.json?limit=10000&closed=" +
this._includeClosedNotesDays +
"&bbox={x_min},{y_min},{x_max},{y_max}",
geoJsonZoomLevel: 10,
},
/* We need to set 'pass_all_features'
There are probably many note_import-layers, and we don't want the first one to gobble up all notes and then discard them...
*/
passAllFeatures: true,
minzoom: Math.min(12, layerJson.minzoom - 2),
title: {
render: trs(t.popupTitle, { title }),
},
calculatedTags: [
"_first_comment=get(feat)('comments')[0]?.text?.toLowerCase() ?? ''",
"_trigger_index=(() => {const lines = feat.properties['_first_comment']?.split('\\n') ?? []; const matchesMapCompleteURL = lines.map(l => l.match(\".*https://mapcomplete.\\(org|osm.be\\)/\\([a-zA-Z_-]+\\)\\(.html\\)?.*#import\")); const matchedIndexes = matchesMapCompleteURL.map((doesMatch, i) => [doesMatch !== null, i]).filter(v => v[0]).map(v => v[1]); return matchedIndexes[0] })()",
"_comments_count=get(feat)('comments').length",
"_intro=(() => {const lines = get(feat)('comments')[0].text.split('\\n'); lines.splice(get(feat)('_trigger_index')-1, lines.length); return lines.filter(l => l !== '').join('<br/>');})()",
"_tags=(() => {let lines = get(feat)('comments')[0].text.split('\\n').map(l => l.trim()); lines.splice(0, get(feat)('_trigger_index') + 1); lines = lines.filter(l => l != ''); return lines.join(';');})()",
],
isShown: {
and: ["_trigger_index~*", { or: isShownIfAny }],
},
titleIcons: [
{
render: "<a href='https://openstreetmap.org/note/{id}' target='_blank'><img src='./assets/svg/osm-logo-us.svg'></a>",
},
],
tagRenderings: [
{
id: "Intro",
render: "{_intro}",
},
{
id: "conversation",
render: "{visualize_note_comments(comments,1)}",
condition: "_comments_count>1",
},
{
id: "import",
render: importButton,
condition: "closed_at=",
},
{
id: "close_note_",
render: embed(
"{close_note(",
t.notFound.Subs({ title }),
", ./assets/svg/close.svg, id, This feature does not exist, 18)}"
),
condition: "closed_at=",
},
{
id: "close_note_mapped",
render: embed(
"{close_note(",
t.alreadyMapped.Subs({ title }),
", ./assets/svg/duplicate.svg, id, Already mapped, 18)}"
),
condition: "closed_at=",
},
{
id: "handled",
render: tr(t.importHandled),
condition: "closed_at~*",
},
{
id: "comment",
render: "{add_note_comment()}",
},
{
id: "add_image",
render: "{add_image_to_note()}",
},
{
id: "nearby_images_note",
render: tr(t.nearbyImagesIntro),
},
{
id: "all_tags_note",
render: "{all_tags()}",
metacondition: {
or: [
"__featureSwitchIsDebugging=true",
"mapcomplete-show_tags=full",
"mapcomplete-show_debug=yes",
],
},
},
],
pointRendering: [
{
location: ["point"],
marker: [
{
icon: "circle",
color: "#fff",
},
{
icon: {
render: "help",
mappings: [
{
if: { or: ["closed_at~*", "_imported=yes"] },
then: "checkmark",
},
],
},
color: "#00",
},
],
iconSize: "40,40",
anchor: "center",
},
],
allowMove: false,
}
}
}

79
src/Models/ThemeConfig/Conversion/PrepareTheme.ts
View file

@ -1,27 +1,15 @@
import {
Concat,
Conversion,
DesugaringContext,
DesugaringStep,
Each,
Fuse,
On,
Pass,
SetDefault,
} from "./Conversion"
import { Concat, Conversion, DesugaringContext, DesugaringStep, Each, Fuse, On, Pass, SetDefault } from "./Conversion"
import { LayoutConfigJson } from "../Json/LayoutConfigJson"
import { PrepareLayer } from "./PrepareLayer"
import { LayerConfigJson } from "../Json/LayerConfigJson"
import { Utils } from "../../../Utils"
import Constants from "../../Constants"
import CreateNoteImportLayer from "./CreateNoteImportLayer"
import LayerConfig from "../LayerConfig"
import { TagRenderingConfigJson } from "../Json/TagRenderingConfigJson"
import DependencyCalculator from "../DependencyCalculator"
import { AddContextToTranslations } from "./AddContextToTranslations"
import ValidationUtils from "./ValidationUtils"
import { ConversionContext } from "./ConversionContext"
import { PrevalidateTheme } from "./Validation"
class SubstituteLayer extends Conversion<string | LayerConfigJson, LayerConfigJson[]> {
private readonly _state: DesugaringContext
@ -40,7 +28,7 @@ class SubstituteLayer extends Conversion<string | LayerConfigJson, LayerConfigJs
function reportNotFound(name: string) {
const knownLayers = Array.from(state.sharedLayers.keys())
const withDistance = knownLayers.map((lname) => [
const withDistance:[string,number][] = knownLayers.map((lname) => [
lname,
Utils.levenshteinDistance(name, lname),
])
@ -221,68 +209,6 @@ class AddDefaultLayers extends DesugaringStep<LayoutConfigJson> {
}
}
class AddImportLayers extends DesugaringStep<LayoutConfigJson> {
constructor() {
super(
"For every layer in the 'layers'-list, create a new layer which'll import notes. (Note that priviliged layers and layers which have a geojson-source set are ignored)",
["layers"],
"AddImportLayers",
)
}
convert(json: LayoutConfigJson, context: ConversionContext): LayoutConfigJson {
if (!(json.enableNoteImports ?? true)) {
context.info(
"Not creating a note import layers for theme " + json.id + " as they are disabled",
)
return json
}
json = { ...json }
const allLayers: LayerConfigJson[] = <LayerConfigJson[]>json.layers
json.layers = [...json.layers]
const creator = new CreateNoteImportLayer()
for (let i1 = 0; i1 < allLayers.length; i1++) {
const layer = allLayers[i1]
if (layer.source === undefined) {
// Priviliged layers are skipped
continue
}
if (layer.source["geoJson"] !== undefined) {
// Layer which don't get their data from OSM are skipped
continue
}
if (layer.title === undefined || layer.name === undefined) {
// Anonymous layers and layers without popup are skipped
continue
}
if (layer.presets === undefined || layer.presets.length == 0) {
// A preset is needed to be able to generate a new point
continue
}
try {
const importLayerResult = creator.convert(
layer,
context.inOperation(this.name).enter(i1),
)
if (importLayerResult !== undefined) {
json.layers.push(importLayerResult)
}
} catch (e) {
console.error("Error", e)
context.err("Could not generate an import-layer for " + layer.id + " due to " + e)
}
}
return json
}
}
class AddContextToTranslationsInLayout extends DesugaringStep<LayoutConfigJson> {
constructor() {
super(
@ -589,7 +515,6 @@ class PostvalidateTheme extends DesugaringStep<LayoutConfigJson> {
for (const l of json.layers) {
const layer = <LayerConfigJson>l
const basedOn = <string>layer["_basedOn"]
const basedOnDef = this._state.sharedLayers.get(basedOn)
if (!basedOn) {
continue
}

14
src/Models/ThemeConfig/LayoutConfig.ts
View file

@ -23,6 +23,7 @@ export class MinimalLayoutInformation {
keywords?: Record<string, string[]>
layers: string[]
}
/**
* Minimal information about a theme
**/
@ -38,7 +39,6 @@ export class LayoutInformation {
}
export default class LayoutConfig implements LayoutInformation {
public static readonly defaultSocialImage = "assets/SocialImage.png"
public readonly id: string
@ -195,7 +195,7 @@ export default class LayoutConfig implements LayoutInformation {
icon: "./assets/svg/pop-out.svg",
href: "https://{basepath}/{theme}.html?lat={lat}&lon={lon}&z={zoom}&language={language}",
newTab: true,
requirements: ["iframe", "no-welcome-message"],
requirements: ["iframe", "no-welcome-message"]
},
context + ".extraLink"
)
@ -296,12 +296,7 @@ export default class LayoutConfig implements LayoutInformation {
}
untranslated
.get(ln)
.push(
translation.context.replace(
/^note_import_[a-zA-Z0-9_]*/,
"note_import"
)
)
.push(translation.context)
}
})
},
@ -315,6 +310,7 @@ export default class LayoutConfig implements LayoutInformation {
return { untranslated, total }
}
public getMatchingLayer(tags: Record<string, string>): LayerConfig | undefined {
if (tags === undefined) {
return undefined
@ -348,7 +344,7 @@ export default class LayoutConfig implements LayoutInformation {
// The 'favourite'-layer contains pretty much all images as it bundles all layers, so we exclude it
const jsonNoFavourites = {
...json,
layers: json.layers.filter((l) => l["id"] !== "favourite"),
layers: json.layers.filter((l) => l["id"] !== "favourite")
}
const usedImages = jsonNoFavourites._usedImages
usedImages.sort()

6
src/Models/ThemeViewState.ts
View file

@ -737,11 +737,7 @@ export default class ThemeViewState implements SpecialVisualizationState {
/**
* MaxZoom for the summary layer
*/
const normalLayers = this.layout.layers.filter(
(l) =>
Constants.priviliged_layers.indexOf(<any>l.id) < 0 &&
!l.id.startsWith("note_import"),
)
const normalLayers = this.layout.layers.filter(l => l.isNormal())
const maxzoom = Math.min(...normalLayers.map((l) => l.minzoom))

3
src/UI/BigComponents/ShareScreen.svelte
View file

@ -60,9 +60,6 @@
if (flayer.layerDef.filterIsSameAs) {
continue
}
if (id.indexOf("note_import") >= 0) {
continue
}
if (Constants.added_by_default.indexOf(<any>id) >= 0) {
continue
}

41
test/Models/ThemeConfig/Conversion/CreateNoteImportLayer.spec.ts
View file

@ -1,41 +0,0 @@
import { Utils } from "../../../../src/Utils"
import { DesugaringContext } from "../../../../src/Models/ThemeConfig/Conversion/Conversion"
import { LayerConfigJson } from "../../../../src/Models/ThemeConfig/Json/LayerConfigJson"
import { TagRenderingConfigJson } from "../../../../src/Models/ThemeConfig/Json/TagRenderingConfigJson"
import { PrepareLayer } from "../../../../src/Models/ThemeConfig/Conversion/PrepareLayer"
import * as bookcases from "../../../../assets/layers/public_bookcase/public_bookcase.json"
import CreateNoteImportLayer from "../../../../src/Models/ThemeConfig/Conversion/CreateNoteImportLayer"
import { describe, expect, it } from "vitest"
import { QuestionableTagRenderingConfigJson } from "../../../../src/Models/ThemeConfig/Json/QuestionableTagRenderingConfigJson"
import { ConversionContext } from "../../../../src/Models/ThemeConfig/Conversion/ConversionContext"
describe("CreateNoteImportLayer", () => {
it("should generate a layerconfig", () => {
const desugaringState: DesugaringContext = {
sharedLayers: new Map<string, LayerConfigJson>(),
tagRenderings: new Map<string, QuestionableTagRenderingConfigJson>(),
}
const layerPrepare = new PrepareLayer(desugaringState)
const layer = layerPrepare.convertStrict(
<any>bookcases,
ConversionContext.test("parse bookcases")
)
const generator = new CreateNoteImportLayer()
const generatedLayer: LayerConfigJson = generator.convertStrict(
layer,
ConversionContext.test("convert")
)
expect(generatedLayer.isShown["and"][1].or[0].and[0]).toEqual(
"_tags~(^|.*;)amenity=public_bookcase($|;.*)"
)
// "Zoomlevel is to high"
expect(generatedLayer.minzoom <= layer.minzoom).toBe(true)
let renderings = Utils.NoNull(
Utils.NoNull(
generatedLayer.tagRenderings.map((tr) => (<TagRenderingConfigJson>tr).render)
).map((render) => render["en"])
)
// "no import button found"
expect(renderings.some((r) => r.indexOf("import_button") > 0)).toBe(true)
})
})