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.

Sign up for GitHub

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

Jump to bottom

Update to iD v2.21.0 #3561

Merged
merged 1 commit into from
Jun 8, 2022
Merged

Update to iD v2.21.0 #3561

merged 1 commit into from
Jun 8, 2022

Conversation

tyrasd
Copy link
Member

@tyrasd tyrasd commented Jun 3, 2022 •
edited
Loading

The following major changes required some adaptions of the iD integration code in this repo:

  • Drop legacy support for Internet Explorer 11 (#8811)
    • Vendorfile – use non-legacy source file for iD (which works in modern browsers, but not IE11)
  • Use OAuth2 for authentication with OSM API (#9134, thanks @bhousel and @dakotabenjamin)
    • CONFIGURE.md & config/settings.yml – updated the installation instructions.
    • app/assets/javascripts/edit/id.js.erb & app/views/site/_id.html.erb – simplify the check whether iD is configured
    • app/assets/javascripts/id.js & app/views/site/id.html.erb – upgrade to OAuth 2 authentication
    • test/controllers/site_controller_test.rb – remove some unnecessary setup/cleanup code

⚠️ existing deployments need to reconfigure the authentication for iD (see instructions)! For the main iD deployments on osm.org, I've created new OAuth 2 Client IDs: Ee1wWJ6UlpERbF6BfTNOpwn0R8k_06mvMXdDUkeHMgw (for api06.dev.osm.org) and 0tmNTmd0Jo1dQp4AUmMBLtGiD9YpMuXzHefitcuVStc (for osm.org).

Further changes from the changelog:

✅ Validation

  • Handle indoor features like buildings when checking for crossing ways (#8944)

🐛 Bugfixes

  • Fix rendering of KeepRight issues (#8963)
  • Fix KeepRight warnings showing up as "Unknown" issues (#8925)
  • Fix ⌥W keyboard shortcut not working on MacOS in certain system languages / keyboard layouts (e.g. Spanish) (#8905)
  • Render closed ways tagged as public_transport=platform, waterway=dam or highway=elevator as areas (#8985)
  • Fix a bug which caused validations to not take effect in certain situations (#9021, thanks @mbrzakovic)
  • Properly escape currently logged-in user's user name (#9097, thanks @jleedev)

🚀 Presets

  • Optimize order of values in dropdowns of access fields (#8945)
  • Use value of vehicle tag as placeholder value of access fields for motor_vehicle and bicycle
  • Render golf features tee, fairway, rough and green in green color and using a grass pattern (#8927)
  • Tweak preset-matching to penalize non-searchable presets when matching OSM objects to presets
  • Do not overwrite existing *=no tags by a preset's addTags
  • Imply access=no in access field of highway=construction objects (#9102)
  • Don't show non-language tag-suffixes in multilingual name field (#9124, thanks @wcedmisten)
  • Render horse riding centers like farmyards (#9118)
  • Support searching presets by their aliases (#6139)
  • Allow searching presets by their tag (key=value) (#8869)

Other

  • Redact more API tokens from custom imagery sources in changeset metadata tags (#8976, thanks @k-yle)
  • New Bing imagery API key and limit tiles vintage API requests (#9133, thanks @mbrzakovic)

🔨 Development

  • Switch build system to esbuild for much faster builds (#8774, thanks @mbrzakovic and @bhousel)
  • Upgrade dependencies: maki to v7.1, fontawesome to v6.1, d3 to v7.4, node-diff to v3.1, mocha to v9.2, svg-sprite to v1.5.4, marked to v4.0, temaki to v5.1, mapillary-js to v4.1

@tyrasd
Update to iD v2.21.0
c55d346 
major changes in this release:
* dropped support for Internet Explorer 11
* switched authentication to OAuth 2

for further changes please refer to https://github.com/openstreetmap/iD/blob/release/CHANGELOG.md#2210
tomhughes
tomhughes reviewed Jun 6, 2022
View reviewed changes
CONFIGURE.md
* 'Modify the map'
* 'Read private GPS traces'
* 'Upload GPS traces'
* 'Modify notes'
Copy link
Member

Choose a reason for hiding this comment

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

Does it actually need the two new permissions that it didn't have before?

Copy link
Member Author

@tyrasd tyrasd Jun 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

In 2.21.0, iD requests the write_prefs scope (see https://github.com/openstreetmap/iD/blob/v2.21.0/modules/services/osm.js#L23). AFAICS it is not (yet) used anywhere obvious. (//cc @bhousel: Did I overlooking something here?) I was however planning to make use of it in the not so far future to preserve some settings and/or editor states. That's why I choose to just keep it in.

write_gpx is currently not used by iD (and also not requested during authentication), but I thought about adding a functionality to upload traces from within iD at some point in the future.

I assumed that for a typical user setting up this repo, it would be less trouble to allow all potentially/eventually used permissions right now, as this would prevent hard to debug issues when the scopes would be used by iD at some point in the future.

Let me know if you prefer to handle it differently. I can also prepare a patch release which doesn't request write_prefs.

Copy link
Member

Choose a reason for hiding this comment

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

I'm happy to add them. I was just trying to check what was going on so I knew what to do when creating applications for production.

Copy link
Member

Choose a reason for hiding this comment

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

AFAICS it is not (yet) used anywhere obvious. (//cc @bhousel: Did I overlooking something here?) I was however planning to make use of it in the not so far future to preserve some settings and/or editor states. That's why I choose to just keep it in.

Yes write_prefs is not used currently, but it's been something we've considering for a long time and might use eventually: openstreetmap/iD#3002

@tomhughes
Copy link
Member

There was no need to create IDs on the dev and production servers - the operations team will take care of that.

@tyrasd
Copy link
Member Author

tyrasd commented Jun 8, 2022 •
edited
Loading

There was no need to create IDs on the dev and production servers - the operations team will take care of that.

Ok.

I had created those IDs for the non-integrated standalone version(s) of iD (e.g. ideditor.netlify.app/) already (see https://github.com/openstreetmap/iD/blob/v2.21.0/index.html#L46-L54). Do you prefer that I replace these with IDs/secrets provided by the operations team as well or is it fine to keep them as is?

@tomhughes
Copy link
Member

I had created those IDs for the non-integrated standalone version(s) of iD (e.g. ideditor.netlify.app/) already (see https://github.com/openstreetmap/iD/blob/v2.21.0/index.html#L46-L54). Do you prefer that I replace these with IDs/secrets provided by the operations team as well or is it fine to keep them as is?

I know nothing about that but I'd say do whatever you've been doing which I guess is having your own ID for it.

I was just saying that I'll create an application on the production site for deployment, and dev is actually done automatically by chef.

@tyrasd
Copy link
Member Author

tyrasd commented Jun 8, 2022 •
edited
Loading

having your own ID for it.

that works for me.

@tomhughes tomhughes merged commit a566589 into openstreetmap:master Jun 8, 2022
@tyrasd tyrasd deleted the iD-2.21.0 branch June 8, 2022 17:30
@1ec5 1ec5 mentioned this pull request Jun 15, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomhughes tomhughes tomhughes left review comments

@bhousel bhousel bhousel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tyrasd @tomhughes @bhousel