Add a CanCanCan ability to see redacted versions #4553
Conversation
AntonKhorev
force-pushed
the
show-unredacted-ability
branch
from
bfaa37e to
d66f064
Compare
AntonKhorev
force-pushed
the
show-unredacted-ability
branch
3 times, most recently
from
62b6445 to
cafd4a0
Compare
AntonKhorev
changed the title
AntonKhorev
added
the
refactor
AntonKhorev
force-pushed
the
show-unredacted-ability
branch
from
cafd4a0 to
012e752
Compare
| @@ -54,6 +54,7 @@ def initialize(user) | |||
| can [:index, :create, :destroy], UserMute | |||
|
|
|||
| if user.moderator? | |||
| can :show_redactions, [Node, Way, Relation, OldNode, OldWay, OldRelation] | |||
There was a problem hiding this comment.
Redactions are only applicable to Old{Node, Way, Relation}, i.e. we cannot redact the currently active version. I didn't quite get why we need to have "Node, Way, Relation" in the list here.
There was a problem hiding this comment.
Yes, I've seen this commit. I found the old code a bit clearer, because to me the concept of redactions makes only sense when talking about Old* elements.
There was a problem hiding this comment.
Checking current_user&.moderator? could be clearer only if you know that moderators can view redacted information.
There was a problem hiding this comment.
Tried replacing @feature with things like "Old#{@type.classify}".constantize, looks worse.
I don't see anything wrong with being able to "show_redactions" of "Node" (= "show_redactions" of some versions of "Node").
There was a problem hiding this comment.
"Node" (as in app/models/node.rb) does not have a redaction_id and does not belong to a redaction (unlike app/models/old_node.rb).
Also, "Node" has no concept of "some versions", it's always the latest version of an object only, which by definition cannot be redacted. Only the "Old*" objects have the full element history, including the latest version.
So maybe I'm a bit confused here, because my definition of Node and OldNode is based on the model definitions.
There was a problem hiding this comment.
"Node" has no concept of "some versions", it's always the latest version ...
Yet we link to its history from its page. How can we do that if "Node" is only the latest version?
There was a problem hiding this comment.
Well, to link to the history, you would only need the element type, object id and the current version of the object (=latest version). Based on this info, you could generate a link to the latest version, and also to the first version, if the current version is not 1.
The actual history page data would then be provided by the old* objects:
I found it helpful to check the local PostgreSQL db and compare current_nodes and nodes table contents (representing Node and OldNode models).
AntonKhorev
force-pushed
the
show-unredacted-ability
branch
from
012e752 to
89201a3
Compare
Thinking about https://github.com/openstreetmap/openstreetmap-website/pull/4324/files#r1501833550:
deny_accessis the ability exception handler, why not then throw the exception? Even better if cancancan throws it after making checks inability.rb. The problem is, the ability to see redacted versions doesn't correspond to any controller method. But we can define this ability anyway and tell cancancan to check it withauthorize!when required.