Fix: force a space between value and denomination (e.g. 20 mph), add integration test for this

This commit is contained in:
Pieter Vander Vennet 2025-09-06 22:12:16 +02:00
parent 946439f8c3
commit 258cbe0802
4 changed files with 43 additions and 29 deletions

View file

@ -13,7 +13,7 @@ type Brand<B> = { [__is_optimized]: B }
*/ */
export type OptimizedTag = Brand<TagsFilter> export type OptimizedTag = Brand<TagsFilter>
export type UploadableTag = Tag | SubstitutingTag | And export type UploadableTag = Tag | SubstitutingTag | And<UploadableTag>
/** /**
* Not nested * Not nested
*/ */

View file

@ -328,22 +328,6 @@ export class TagUtils {
return keyValues return keyValues
} }
/**
* Flattens an 'uploadableTag' and replaces all 'SubstitutingTags' into normal tags
*/
static FlattenAnd(tagFilters: UploadableTag, currentProperties: Record<string, string>): Tag[] {
const tags: Tag[] = []
tagFilters.visit((tf: UploadableTag) => {
if (tf instanceof Tag) {
tags.push(tf)
}
if (tf instanceof SubstitutingTag) {
tags.push(tf.asTag(currentProperties))
}
})
return tags
}
static optimzeJson(json: TagConfigJson): TagConfigJson | boolean { static optimzeJson(json: TagConfigJson): TagConfigJson | boolean {
const optimized = TagUtils.Tag(json).optimize() const optimized = TagUtils.Tag(json).optimize()
if (optimized === true || optimized === false) { if (optimized === true || optimized === false) {
@ -1024,4 +1008,14 @@ export class TagUtils {
} }
return tag return tag
} }
static flattenAnd(tg: UploadableTag | UploadableTag[]): (SubstitutingTag | Tag)[] {
if (Array.isArray(tg)) {
return tg.flatMap(t => TagUtils.flattenAnd(t))
}
if (tg["and"] || tg instanceof And) {
return tg["and"].flatMap(tg => TagUtils.flattenAnd(tg))
}
return [tg]
}
} }

View file

@ -5,10 +5,7 @@ import { TagUtils } from "../../Logic/Tags/TagUtils"
import { And } from "../../Logic/Tags/And" import { And } from "../../Logic/Tags/And"
import { Utils } from "../../Utils" import { Utils } from "../../Utils"
import { Tag } from "../../Logic/Tags/Tag" import { Tag } from "../../Logic/Tags/Tag"
import { import { MappingConfigJson, QuestionableTagRenderingConfigJson } from "./Json/QuestionableTagRenderingConfigJson"
MappingConfigJson,
QuestionableTagRenderingConfigJson,
} from "./Json/QuestionableTagRenderingConfigJson"
import Validators, { ValidatorType } from "../../UI/InputElement/Validators" import Validators, { ValidatorType } from "../../UI/InputElement/Validators"
import { TagRenderingConfigJson } from "./Json/TagRenderingConfigJson" import { TagRenderingConfigJson } from "./Json/TagRenderingConfigJson"
import { RegexTag } from "../../Logic/Tags/RegexTag" import { RegexTag } from "../../Logic/Tags/RegexTag"
@ -23,6 +20,7 @@ import ComparingTag from "../../Logic/Tags/ComparingTag"
import { Unit } from "../Unit" import { Unit } from "../Unit"
import { Lists } from "../../Utils/Lists" import { Lists } from "../../Utils/Lists"
import { IsOnline } from "../../Logic/Web/IsOnline" import { IsOnline } from "../../Logic/Web/IsOnline"
import SubstitutingTag from "../../Logic/Tags/SubstitutingTag"
export interface Mapping { export interface Mapping {
readonly if: UploadableTag readonly if: UploadableTag
@ -803,7 +801,7 @@ export default class TagRenderingConfig {
multiSelectedMapping: boolean[] | undefined, multiSelectedMapping: boolean[] | undefined,
currentProperties: Record<string, string>, currentProperties: Record<string, string>,
unit?: Unit unit?: Unit
): UploadableTag[] { ): (Tag | SubstitutingTag)[] {
if (typeof freeformValue === "string") { if (typeof freeformValue === "string") {
freeformValue = freeformValue?.trim() freeformValue = freeformValue?.trim()
} }
@ -820,7 +818,8 @@ export default class TagRenderingConfig {
valueNoUnit, valueNoUnit,
() => currentProperties["_country"] () => currentProperties["_country"]
) )
freeformValue = formatted + denom.canonical // In general, we want a space between the amount and the unit
freeformValue = formatted + " " + denom.canonical
} else { } else {
freeformValue = validator.reformat( freeformValue = validator.reformat(
freeformValue, freeformValue,
@ -865,7 +864,7 @@ export default class TagRenderingConfig {
const freeformOnly = { [this.freeform.key]: freeformValue } const freeformOnly = { [this.freeform.key]: freeformValue }
const matchingMapping = this.mappings?.find((m) => m.if.matchesProperties(freeformOnly)) const matchingMapping = this.mappings?.find((m) => m.if.matchesProperties(freeformOnly))
if (matchingMapping) { if (matchingMapping) {
return [matchingMapping.if, ...(matchingMapping.addExtraTags ?? [])] return [...TagUtils.flattenAnd(matchingMapping.if), ...(matchingMapping.addExtraTags ?? [])]
} }
// Either no mappings, or this is a radio-button selected freeform value // Either no mappings, or this is a radio-button selected freeform value
const tag = [ const tag = [
@ -877,7 +876,7 @@ export default class TagRenderingConfig {
return undefined return undefined
} }
return tag return TagUtils.flattenAnd(tag)
} }
if (this.multiAnswer) { if (this.multiAnswer) {
@ -907,7 +906,7 @@ export default class TagRenderingConfig {
) { ) {
return undefined return undefined
} }
return and return TagUtils.flattenAnd(and)
} }
// Is at least one mapping shown in the answer? // Is at least one mapping shown in the answer?
@ -927,11 +926,11 @@ export default class TagRenderingConfig {
if (useFreeform) { if (useFreeform) {
return [ return [
new Tag(this.freeform.key, freeformValue), new Tag(this.freeform.key, freeformValue),
...(this.freeform.addExtraTags ?? []), ...(TagUtils.flattenAnd(this.freeform.addExtraTags) ?? [])
] ]
} else if (singleSelectedMapping !== undefined) { } else if (singleSelectedMapping !== undefined) {
return [ return [
this.mappings[singleSelectedMapping].if, ...TagUtils.flattenAnd(this.mappings[singleSelectedMapping].if),
...(this.mappings[singleSelectedMapping].addExtraTags ?? []), ...(this.mappings[singleSelectedMapping].addExtraTags ?? []),
] ]
} else { } else {
@ -1188,7 +1187,7 @@ export class TagRenderingConfigUtils {
console.log("Could not download the NSI: ", extraMappingsErr["error"]) console.log("Could not download the NSI: ", extraMappingsErr["error"])
return config return config
} }
const extraMappings = extraMappingsErr?.success const extraMappings = extraMappingsErr?.["success"]
if(extraMappings === undefined){ if(extraMappings === undefined){
if(!IsOnline.isOnline.data){ if(!IsOnline.isOnline.data){
// The 'extraMappings' will still attempt to download the NSI - it might be in the service worker's cache // The 'extraMappings' will still attempt to download the NSI - it might be in the service worker's cache

View file

@ -0,0 +1,21 @@
import { describe, it } from "vitest"
import LayerConfig from "../../src/Models/ThemeConfig/LayerConfig"
import maxspeed_layer from "../../public/assets/generated/layers/maxspeed.json"
import { LayerConfigJson } from "../../src/Models/ThemeConfig/Json/LayerConfigJson"
import { expect } from "chai"
import { Tag } from "../../src/Logic/Tags/Tag"
describe("Integration_unit_has_space", () => {
it("maxspeed should produce '20 mph' and not '20mph'", () => {
const maxspeed = new LayerConfig(
<LayerConfigJson><any>maxspeed_layer, "integration_test"
)
const maxspeedQuestion = maxspeed.tagRenderings.find(tr => tr.id === "maxspeed-maxspeed")
const unit = maxspeed.units.find(unit => unit.appliesToKeys.has("maxspeed"))
const spec = maxspeedQuestion.constructChangeSpecification("20", undefined, undefined, {
"_country": "gb"
}, unit).find(t => t["key"] === "maxspeed")
expect(spec.asJson()).to.eq(new Tag("maxspeed", "20 mph").asJson())
})
})