To reproduce:
1) Go to http://preview.ideditor.com/master/#background=EsriWorldImagery&disable_features=boundaries&map=19.37/35.99962/-79.30998
2) Try adding an island in the middle of the pond by drawing a closed way / area and then selecting both the pond and new area and pressing C (creating a multipolygon).
3) Try to upload - an error will appear preventing upload.
Oh this is definitely a cacheing issue. We really need to just re-run validation after any operation.
So for this, the issue is that coreDifference doesn't count an entity as modified if only its membership in a relation changed, and so it's not re-validated.
Oh good catch @quincylvania.. this makes a lot of sense.
@bhousel I could fix this but I'm not sure it's performant to call parentRelations during checkEntityID. Any ideas?
@bhousel I could fix this but I'm not sure it's performant to call
parentRelationsduringcheckEntityID. Any ideas?
Here's what I know..
coreDifference should have a way of returning what all is affected by a change - this is different from just returning the literal entity ids that changed.coreDifference#extantIDs might be the place for this.. We call it in the validator, but we need to see what other parts of the iD code call it, and whether it would be safe to return extra entity ids to the caller.. coreDifference#affectedIDs and use that.coreDifference#modified or in checkEntityIDs).. We can't, because difference is also used to assemble the changeset before saving, and we don't want to send things to OSM that haven't actually changed.
coreDifference#extantIDsmight be the place for this.. We call it in the validator, but we need to see what other parts of the iD code call it, and whether it would be safe to return extra entity ids to the caller..
I ended up doing this. I added an option to allow the caller to decide whether they want extantIDs() to also include the ids of modified relation members. Now the validator can know to also expire these IDs from its caches too.
I also closed #3613 by doing almost the same thing, but in complete(). I didn't want to merge the two pieces of code, because they really serve different purposes. complete() returns slightly different set of stuff because it is really for rendering.