Tag filter#1879
Conversation
alisa911
left a comment
alisa911
left a comment
There was a problem hiding this comment.
The goal was "one filtering logic". In practice only the predicates (shouldDisplayKey, isKeyToSkip, getPoiAdditionalType) were moved into the shared AdditionalInfoBundle, while the loop that assembles "which tags to show" was duplicated: one copy in SearchService.checkTagsVisibility, a second in Android's AmenityUIHelper.buildInternal, and a third (partial) one in the web fallback fixTagsKeysFallback plus the extra work in groupLocalizedTags. Three sources of truth instead of one.
Move the whole logic into a method in AdditionalInfoBundle. The web should receive a ready to display list of tags, and there should be no groupLocalizedTags or other such logic on the web. The web should only build the UI, which icon, a list or a tag, and so on.
| AMENITY_PREFIX + SUBTYPE, | ||
| ]; | ||
|
|
||
| const HIDDEN_EXTENSIONS_POI = [ |
There was a problem hiding this comment.
Rename HIDDEN_EXTENSIONS_POI to web tags, and HIDDEN_EXTENSIONS should not exist, it duplicates the server logic
| tagObj = addWikidataTags(key, value, tagObj); | ||
| } | ||
| res.push(tagObj); | ||
| const poiNameTags = {}; |
There was a problem hiding this comment.
Unclear name remainingTags remaining after what, filterTagsByVisibility does not filter, the fallback with duplication is not needed, pick one thing either mainTag or mainEntry, const SVG is a magic constant
| const arr = tag.key.split(':'); | ||
| tag.key = arr[0]; | ||
| tag.lang = arr[1]; | ||
| async function buildTagObj(key, value, lang, ctx, subtypeTag) { |
There was a problem hiding this comment.
buildTagObj has a large number of arguments, use {}
| tagObj.needLinks = false; | ||
| tagObj.textPrefix = value; | ||
| break; | ||
| case OPENING_HOURS: |
There was a problem hiding this comment.
If you touched the switch in buildTagObj, then rewrite it properly
| } | ||
| } | ||
|
|
||
| res = res.filter((t) => !t.key.startsWith(WEB_PREFIX)); |
There was a problem hiding this comment.
Why did you remove the WEB_PREFIX filter?
| } | ||
|
|
||
| function fixTagsKeys(tags) { | ||
| async function filterTagsByVisibility(tags) { |
There was a problem hiding this comment.
The server returns an untyped map where a value is either a string or a nested localizations object. With no schema, the client has to rebuild what the server already computed: groupLocalizedTags re-splits the keys, re-picks the main language and rebuilds otherLangs. That double interpretation is where the bugs came from, and if the server changes the shape the web breaks silently.
Fix: return a typed, render ready list like { key, value, lang, otherLangs } already grouped, so the client only renders it.
|
This violates map/CLAUDE.md |
There was a problem hiding this comment.
Pull request overview
This PR addresses tag rendering in the OsmAnd Web map “WPT/Favorites” info block by introducing a server-backed visibility filter for tags (related to osmandapp/OsmAnd#24809), and refactoring tag-building to better handle localized variants and POI name tags.
Changes:
- Add a
/search/get-tags-visibilityPOST call to filter/normalize tag visibility before rendering. - Refactor tag construction into helpers (
buildTagObj, grouping localized tags, handling POI name + other languages). - Add client-side filtering of certain web-only/internal keys before visibility processing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( | ||
| key.startsWith(ALT_NAME) || | ||
| HIDDEN_EXTENSIONS_POI.includes(key) || | ||
| key.startsWith(ORIGINAL_ICON) || | ||
| key === SVG | ||
| ) { |
osmandapp/OsmAnd#24809