Change Request Approver & Comment right

Development
,
#1

Hi everyone,

I’m opening this brainstorming to collect a bit of feedback / opinions about how the comments right interact with the approver role in Change Request.

So right now a user who is an approver of a change request always gain the right to comment on that change request. This is enforced by this method whose javadoc is pretty clear about the behaviour: application-changerequest/ChangeRequestRightsManager.java at application-changerequest-1.5 · xwiki-contrib/application-changerequest · GitHub.

This means that even if a user has a comment right denied, they will be able to comment a change request they are approver of: in global comment, diff comment and review comment.
I’ve implemented it like that because my intuition was that the review process automatically involve being able to comment: it doesn’t make much sense if the approver can only accept or refuse the change. They have to explain what should be improved in case they reject it.

Now I have a bit of a second thought about this behaviour: I’ve been requested to ensure that a user who does not have comment right shouldn’t be able to add a review comment. I even opened Loading... before realizing that this behaviour was actually expected currently.

So I’d like to gather other opinions on this before taking a decision, what do you think should be better: to keep current behaviour, or to always comply with the comment right?

Thanks

#2

Are you able to distinguish between:

  • page comments: the comment at the bottom of every page
  • CR comments: the comment added at a given live and a CR

Imo page comments should strictly follow the comment right, and the CR comments should be overridden by CR specific rules.

#3

I would keep rights as simple and predictable as possible. Granting comment right to approvers sounds neither simple nor predictable. I would thus suggest to always comply with the comment right. However, I don’t know much about how rights for change requests are managed so if being an approver already implies rights, it might seem more logical to also imply comment right.

#4

Those concept does not really make sense in CR: comments on the bottom of the page are disabled in CR pages. Those are replaced by the CR “global comment” that you can add on the bottom of the description of the CR. What you call “CR comments” is the diff comment in CR vocabulary. And then you have the review comments.

So there’s distinction but in terms of authorization they’re all checked with the same API I linked in my first post.

#5

Well right now you don’t need to, as it’s automatically “granted”: the API returns true to add comment without checking the right if the user is an approver.

Well there’s no right implication in terms of pure XWiki Right API: being an approver is not strictly a right, so there’s no implied rights.
Now there’s an authorization API in CR and being Approver gives you some power such as the capability to perform a review and to merge in some cases. And also to comment.