Skip to content
This repository has been archived by the owner on Jul 24, 2021. It is now read-only.

Potlach 2 doesn't warn, when corrupting relations #3777

Closed
openstreetmap-trac opened this issue Jul 23, 2021 · 10 comments
Closed

Potlach 2 doesn't warn, when corrupting relations #3777

openstreetmap-trac opened this issue Jul 23, 2021 · 10 comments

Comments

@openstreetmap-trac
Copy link

Reporter: petr.dlouhy[at]email.cz
[Submitted to the original trac issue database at 3.31pm, Saturday, 21st May 2011]

When merging two ways, which one of them is part of a relation and the second isn't, Potlach doesn't warn user. This leads to corruption of the relations

@openstreetmap-trac
Copy link
Author

Author: Richard
[Added to the original trac issue at 10.52am, Sunday, 22nd May 2011]

a) This is not remotely critical.

b) "Corruption" is not remotely the right word. Are the relations still well-formed? Yes. Therefore they are not corrupted.

c) In most cases, Potlatch 2 renders ways that are part of relations (routes, turn restrictions, multipolygons) differently. It is therefore easy for the user to see the effect of their actions. Several years' experience shows me that accidental merges of a relation-ed way with another are almost entirely done in JOSM, which AIUI until the new P2-like MapCSS rendering didn't actually show relation-ed ways differently at all. For example, http://www.openstreetmap.org/browse/changeset/8198740 was mending extensive damage to a route relation by a JOSM user. Generally I have to do this several times a week.

d) If you there is a particular type of relation that is not adequately rendered in P2 and so the user can't see what they've done, please cite it here, rather than coming up with a vague handwavy ticket like this.

e) Popping up warnings ("Do this! Look at that! Do the other") is terrible UI and is generally ignored by most people.

f) Please learn to spell Potlatch, kthxbye.

@openstreetmap-trac
Copy link
Author

Author: woodpeck
[Added to the original trac issue at 11.09am, Sunday, 22nd May 2011]

Could either of you, for the benefit of others reading this, explain what exactly Potlatch does in the case discusssed? Will the new, combined way be part of the relation or will it not? Or does it depend on whichever way was selected first? Properly documenting this would go a long way to avoiding problems, even without any change to the code, I should think.

As for popping up warnings, I think Potlatch has on a number of other occasions already gone a different way - instead of "are you sure you want to delete 12345 nodes", Potlatch would say "deleted 12345 nodes - hit Ctrl-Z to undo" or something. Maybe it is possible to do something similar with way merges in the situation that Peter described.

@openstreetmap-trac
Copy link
Author

Author: Richard
[Added to the original trac issue at 11.35am, Sunday, 22nd May 2011]

The new combined way will be part of the relation. This has always been the case in all versions of Potlatch 1 and 2.

Because Potlatch (1 and 2) generally renders relations, it should be immediately apparent to the user what their action has been. If there is a relation that is not adequately rendered by P2 - and that's certainly possible, because in P2 relation rendering is dictated by the stylesheet rather than being automatic as in P1 - then I'd very much like to hear of it so that I can fix it.

P2 already floats a "check tags" advice bubble after merging ways (the warning is far too verbose and is therefore probably ignored by most users, which needs fixing). That could potentially be "check tags and relations" in these cases, but again, users don't read warnings. Fixing rendering is the first half of the proper way to do it; the second half is to have a friendly "mapping quality" palette (in a much less prescriptive way than the existing validator tools) but that's a long-term aim that will take time to do properly.

@openstreetmap-trac
Copy link
Author

Author: Petr Dlouh
[Added to the original trac issue at 5.15pm, Sunday, 22nd May 2011]

This ticket is based on following commit (although it is not very clear from the commit page, that it do harm): http://www.openstreetmap.org/browse/changeset/8021768

I am not sure, how far you want to go in rendering the relations. In JOSM is routes plugin (http://wiki.openstreetmap.org/wiki/JOSM/Plugins/Routes), which visualizes the hiking/bicycle/other relations (it can even draw parallel lines for more relations on one way).

@openstreetmap-trac
Copy link
Author

Author: Richard
[Added to the original trac issue at 5.18pm, Sunday, 22nd May 2011]

Potlatch 2 renders route relations such as that. It has always done so, without any need for a plugin or alternative stylesheet or whatever.

I don't see any evidence that this is an editor issue. It simply looks like a mapper who made a mistake. We can't completely stop that happening, you know. :)

@openstreetmap-trac
Copy link
Author

Author: Petr Dlouh
[Added to the original trac issue at 6.16pm, Sunday, 22nd May 2011]

He doesn't made a mistake because he was distracted. He made a mistake, because he doesn't know, what he was doing and the editor didn't gave him an advice even if it could.

Why does Potlatch warn if merging two ways with different tags, but does not when merging two ways with different relation membership? Isn't the situation identical?

@openstreetmap-trac
Copy link
Author

Author: Richard
[Added to the original trac issue at 6.28pm, Sunday, 22nd May 2011]

Sure. Like I said above, it could do. The reason why it doesn't yet, I guess, is because neither you nor I nor anyone has fixed it to do so. I'll do that at some point but my willingness to do so is not best helped by trac tickets screaming "critical" and ZOMG CORRUPTION THIS MUST BE FIXED.

But as I keep saying: Potlatch does make the user aware that they have extended the relation membership to a part of the way that was not previously a member. It does this by clearly rendering the way differently. The user is looking at the map so will see this.

After all, should Potlatch pop up an alert saying "You changed the highway type from trunk to primary" every time you do that? Even though the change is reflected in a different rendering? And so on.

@openstreetmap-trac
Copy link
Author

Author: Petr Dlouh
[Added to the original trac issue at 9.16am, Monday, 30th May 2011]

Sorry for my overreaction, but I really think, it is serious problem. Here is another damaging edit from different user: http://www.openstreetmap.org/browse/changeset/8093999

@openstreetmap-trac
Copy link
Author

Author: Richard
[Added to the original trac issue at 7.15pm, Thursday, 16th June 2011]

Fixed in v2.2; it's now included in the "tags merged" pop-up advice.

Perhaps now JOSM can make users aware of merged relations, too, so that JOSM users stop destroying the National Cycle Network. ;)

@openstreetmap-trac
Copy link
Author

Author: Petr Dlouh
[Added to the original trac issue at 10.01am, Friday, 17th June 2011]

Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant