Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.20.0 tag validation issue #8615

Closed
cicku opened this issue Aug 1, 2021 · 3 comments · Fixed by #8618
Closed

2.20.0 tag validation issue #8615

cicku opened this issue Aug 1, 2021 · 3 comments · Fixed by #8618
Assignees
Milestone

Comments

@cicku
Copy link
Contributor

cicku commented Aug 1, 2021

When adding Internet access tags to a hotel brand:

Before
image

After
image

@UKChris-osm
Copy link

@bhousel is this an NSI issue?

@bhousel
Copy link
Member

bhousel commented Aug 2, 2021

@bhousel is this an NSI issue?

My guess is that there are 2 competing tags for internet_access and the NSI entry for "Fairfield Inn and Suites" has a different one than whatever iD's Hotel preset wants to set it to.

I'll look into this and change it on the NSI side if that's what it is.

@bhousel
Copy link
Member

bhousel commented Aug 2, 2021

Ok this issue is a bit more interesting than I initially thought.

internet_access=yes is being considered a "primary" or "toplevel" tag. (sort of like amenity=yes or emergency=yes)
The reason this tag is being considered this way is because we have these "LinkNYC" internet access points in NSI.

(This is another situation like what we originally saw in testing #8305 where it would try to remove emergency=yes from hospitals for the same reason)

I'll add some special code to preserve internet_access tags, since they can go on almost anything.

bhousel added a commit that referenced this issue Aug 2, 2021
We'll only _replace_ the tag value if this tag is the toplevel/defining tag for the matched item (`k`)
(closes #8615)
@mbrzakovic mbrzakovic added this to the 2.20.1 milestone Aug 2, 2021
@bhousel bhousel self-assigned this Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants