Allow the hiding of changeset tags

[woodpeck]

This pull request establishes a simple mechanism for moderators to hide/unhide changset tags. All tags of a changeset are hidden, or none are hidden; there is no per-tag granularity. Tags are only hidden from the web interface, and not from the API.
This is meant to be used in cases where people upload data with offensive changeset comments, or where advertising is placed in the changeset comment.

[tomhughes]

I'm really not sure about this - if we allow changeset tags to be hidden then why not node, way and relation tags?

I'm really not sure I want to start down the road of complicating the main geodata with things like this.

[tomhughes]

In any case there doesn't seem to be a migration to add the tags_hidden field?

[woodpeck]

I have added the migration. -- You are right that there is a conceptual inconsistency when you look at it from the "data primitives" side. But if you look at things from the "stuff that we publish on our web site" perspective, then any and all node, way, or relation tags can already be hidden by redacting the objects in question. There's no way to redact a changeset, so once inacceptable content has been added to changeset tags there's no way (except running an update command on the raw database) to suppress this being published through the web site. That is the itch being scratched with this PR.

It might be possible to re-phrase this whole thing as "redacting changesets" to be more in sync with the other data primitives, however with changesets being final and not versioned, this would also introduce a couple funny effects.

[Zverik]

I don't see any permissions code. Would anyone be able to hide/unhide changeset tags?

[pnorman]

stray whitespace

[gravitystorm]

@Zverik is correct - this PR would allow anyone to make the changes. We run a "default permit" permissions system currently (but see #1626 for me wanting to change this). So the PR can't be merged as-is.

I also think it's a good idea to align the terminology, so that we are "redacting" changeset tags. It has implications for dumps, replication and the API too, since if the tags are being redacted they are presumably being done so for a good reason!

@woodpeck what was your reasoning for this only affecting the website and not the API? First-things-first, or something more fundamental?

[woodpeck]

Uh, I had completely forgotten that I ever wrote this, and I can't believe I didn't somehow add an "if moderator" check but reviewing the changes now it seems I indeed haven't. Sorry for that.

What I wanted to achieve with this was a really lightweight way of "not blasting out advertising through our web site". I view any kind of redaction as a heavy-handed meddling with our data that leads to inconsistencies with the history (someone might once have taken a snapshot of our data and now even our historic data doesn't fit the snapshot any more), and would reserve that for really important cases. For a mere advertising slogan, suppressing its publication on the web page seemed the better alternative; it's not something so evil that it must be hidden from everyone even when accessing the API.

What I wanted was cosmetics, whereas redaction is surgery.

I don't have strong feelings either way. I can see how my lightweight approach might seem unsatisfactory from a data model perspective because it introduces a hitherto unknown "this exists but isn't shown on the web page" state. What I submitted was simply the best I could come up in the short time I had allowed myself.

[pnorman]

I'm really not sure about this - if we allow changeset tags to be hidden then why not node, way and relation tags?

We do need to treat changesets differently, because we can't go back and edit the tags belonging to someone else at a later date. With a normal object with advertising text, we can just edit it to a new version, and the only trace of the text remains in the history.

I'm not sure this way is the best, but we shouldn't expect the same approach as nodes, ways, and relations.

[gravitystorm]

I've reviewed this PR again today, and for a few reasons, I don't think it's yet suitable for merging:

• There's no tests included.
• There should be consistency between what's available through the website and through the API.
• The permissions checks should be done via the abilities system, rather than hardcoded roles.
• Redaction vs simple hiding of data needs to be decided, and work through the implications for dumps.

The first two should be fairly straightforward to fix, so I'd be happy to see a fresh PR with the above items addressed. For the fourth one, it might be worth discussing in an issue before coding.