Page MenuHomePhabricator
Create Task
Maniphest T3943

Able to assign oneself as reviewer despite setting differential.allow-self-accept to false
Closed, InvalidPublic
Actions

Description

Differential.allow-self-accept is false in my phabricator:

differential.allow-self-accept
Customized
Allows users to accept their own revisions.
Current Value: false

However, a user reported that he was able to assign himself as the reviewer for his own revision. He has no option to accept it though i.e. he does not see Accept Revision; he sees Abandon Revision but that's not what he wants.

I followed his steps and was able to assign myself as the reviewer for my own revision as well

click on Create Diff
upload a file
click on Create Diff
click on Continue
enter title for revision
assign myself as reviewer
click on save

Event Timeline

hlau created this task.Oct 11 2013, 8:07 PM
hlau raised the priority of this task from to Needs Triage.
hlau updated the task description. (Show Details)
hlau added a subscriber: hlau.
asherkin added a subscriber: asherkin.Oct 11 2013, 10:02 PM

Abandon Revision is a standard action for authors, the reviewer ones are are accept and request changes - if he isn't seeing those the option is being respected.

hlau added a comment.Oct 11 2013, 10:22 PM

I don't see the logic. I can make myself the reviewer for my own revision but as a reviewer, I cannot accept the revision. So I am only allowed to abandon my revision even if I am happy with my code changes?

asherkin added a comment.Oct 11 2013, 10:23 PM

Right, because you've disabled being able to accept your own revisions by setting differential.allow-self-accept to false.

asherkin added a comment.Oct 11 2013, 10:25 PM

Clarification: if you set differential.allow-self-accept to true, it sounds like you'll get exactly what you want - where people can accept diffs they submitted.

hlau added a comment.Oct 11 2013, 11:09 PM

I set it to false which stops a user from accepting his own revision. This works fine. But it should also stop a user from making himself the reviewer. This does not work. If we allow a user to be his own reviewer but not allow him to accept his own revision, then his only choice is abandoning his review. This does not make sense.

epriestley added a subscriber: epriestley.Oct 12 2013, 11:48 PM

The bug is that the user is able to be their own reviewer. I'm not sure how the user managed to do this. I get this error when trying from the web UI:

Screen_Shot_2013-10-12_at_4.46.43_PM.png (1×1 px, 161 KB)

I get this error when trying from the CLI:

>>> orbital ~/repos/git-working-copy $ arc diff --create
Linting...
No lint engine configured for this project.
Running unit tests...
No tests to run.
Exception
ERR-CONDUIT-CORE: The owner of a revision may not be a reviewer.
(Run with --trace for a full exception trace.)
>>> orbital ~/repos/git-working-copy $

Do you know how the user managed to do this? Can you reproduce it?

then his only choice is abandoning his review

The user has the same choices they would have if they didn't set any reviewers: they can add other reviewers. They should also be able to remove themselves as a reviewer.

hlau added a comment.Oct 14 2013, 6:56 PM

My task description gave details on the steps that the user managed to assign himself the reviewer of his own revision. I was able to follow the same steps to recreate the problem.

epriestley added a comment.Oct 14 2013, 7:03 PM

When I follow those steps, I get this:

Screen_Shot_2013-10-14_at_12.02.02_PM.png (1×1 px, 157 KB)

epriestley added a comment.Oct 14 2013, 7:04 PM

Can you create a revision on this install with yourself as a reviewer?

btrahan closed this task as Invalid.Feb 7 2014, 8:45 PM
btrahan claimed this task.

We can't reproduce this so I blame ghosts or possibly some old version of Phabricator. If you can reproduce this on this install please do re-open. :D

Phabricator · Built by Phacility