-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add wikidata asynchronously based on wikipedia #2732
Conversation
57cf840
to
bdc73e7
Compare
bdc73e7
to
e2512ab
Compare
Cool, looks like a good start... I'm mostly concerned about |
Indeed, it is possible to cause |
@bhousel, f31f992 looks a bit weird, but the idea is that we update the entity the normal way (same as the synchronous update) if the same entity was selected as before. That way the UI gets updated too. Otherwise, we only update the underlying model. I used Network Link Conditioner to verify that the race condition is gone. |
if (selectedIDs && selectedIDs.length === 1 && selectedIDs[0] === entity.id) { | ||
event.change({wikidata: qid}); | ||
} else { | ||
context.entity(entity.id).tags.wikidata = qid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not going to claim that my approach is correct :) but I created an action and then had the context 'perform' it. See https://github.com/strava/iD/blob/master/js/id/operations/slide.js#L103
This looks like it edits the context/graph data directly, which, as has been said above, messes with the immutability of the context. (This feature makes it easy to do undo/redo as you can just swap contexts).
I was assuming that calling the perform on the context took care of serializing the edits and updating the history (it's seemed to work in practice). The worry is the context/graph may have changed in between the time you clicked the button and when the request came back.
I hope this is helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, @paulmach yeah your code is the right way to do things, and is safer because you use iD.ui.Loading
to stop the user from editing while waiting for the callback.
It would be even more bulletproof if you do
iD.ui.Loading(context).blocking(true).message('Sliding')
because without the .blocking(true)
the user can just click and make the dialog go away. I was able to slide a really long twisty way, dismiss the dialog, and delete the way, then the callback happened and threw an "entity xxx not found" error. Kind of a tricky sequence, but a user haphazardly clicking around the radial menu could do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bhousel I've updated!!
What's the status on this? |
Nobody is working on this currently.. |
Asynchronous tag updating was really tricky to get right (and adding test coverage was even trickier) but I think this will work ok. Code here for the curious. merged as |
Thank you so much, @bhousel! |
From the OSM end, if I look at an object or the changeset that a wikidata tag was added in, how do I know whether the wikidata tag was added manually by the user (and presumably checked beforehand) or whether it was derived automatically? Is something like "source:wikidata=automatically added by iD based on wikipedia" going to be added? Is there going to be any kind of duplicate checking / warning? We've had examples in the past where people add wikipedia links to "category of thing A" (e.g. "McDonalds") to an "example of thing A" (a specific fast food joint) in OSM. Obviously there are many examples of valid duplicate links to wikidata (e.g. the various bits of the M42 motorway, which is taginfo's most common wikidata tag), so it'd be difficult to do. |
It's probably safe to assume that if you see both
You can open an issue for discussion of this if you feel it's important.. Putting
I can't think of any way to prevent this. People can always add wrong things in the wikipedia tag. We can't duplicate check against all of osm, only the tiles that we've already downloaded, so that makes a check here not very useful - and like you said, sometimes it's an ok thing to do. |
As a first step towards #2680, when the user enters a Wikipedia article title using the Wikipedia preset field, pair the resulting
wikipedia
tag with awikidata
tag. The query is asynchronous using JSONP, because neither wikidata.org nor wqt.wmflabs.org supports XmlHttpRequests from any hosted iD instance.Also tracked in Phabricator as part of the Wikimanía 2015 Hackathon.
/cc @pigsonthewing @bhousel