Skip to content

Preserve universal fields when changing presets - #13125

Open
Hirdyansh-bh wants to merge 6 commits into
openstreetmap:developfrom
Hirdyansh-bh:fix-preset-tag-removal-iD13105
Open

Hirdyansh-bh wants to merge 6 commits into
openstreetmap:developfrom
Hirdyansh-bh:fix-preset-tag-removal-iD13105

Conversation

@Hirdyansh-bh

Copy link
Copy Markdown

When switching between presets (such as from a primary road to a link road), tags associated with universal fields like name were being stripped if the new preset did not explicitly list them in its fields array.

  • Included presetManager.universal() in preserved fields when switching presets
  • Excluded universal fields from oldPresetFieldKeys so they are not treated as obsolete keys to remove
  • Added a regression unit test in test/spec/actions/change_preset.js

Closes #13105

@k-yle k-yle left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR, please see suggestions below for how we could simplify this

Comment thread modules/actions/change_preset.ts Resolved
Comment thread test/spec/actions/change_preset.js Resolved
Comment thread modules/actions/change_preset.ts Resolved
@k-yle k-yle added the bug-release-blocker An important bug - let's get this fixed in the next release! label Nov 5, 2026
@Hirdyansh-bh

Copy link
Copy Markdown
Author

i am working on the fixes and will revert shortly

@Hirdyansh-bh

Copy link
Copy Markdown
Author

@k-yle Thanks for the review! I've reverted the extra filter in change_preset.ts, simplified the test logic by removing the try...catch block, and added a CHANGELOG entry.

@k-yle k-yle left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, this LGTM now. lets wait a bit in case anyone else has opinions

@Hirdyansh-bh

Copy link
Copy Markdown
Author

Sounds good, thanks again for the guidance!

@Hirdyansh-bh

Copy link
Copy Markdown
Author

any updates?

@Hirdyansh-bh

Copy link
Copy Markdown
Author

also it would be really helpful of you if you assign me my next issue on which i can work on

This branch was successfully deployed

1 active (outdated) deployment
pr-preview-13125 — 7e76dbfb Deployed Nov 5, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-release-blocker An important bug - let's get this fixed in the next release!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tag removal logic on changing presets must consider also universal fields

2 participants