I found an issue when using Phabricator audit tool. I am not sure if the issue is an accepted feature, or a bug. This is because the audit system is missing documentation on how a commit is approved or rejected. So in summary either
1. tell me that the current behaviour is correct and document audit behaviour
2. .. that the behaviour I found is not correct, and you should maybe also better document ?
I am currently running version a8be733e5ffebb9c0573d7e932e19c86d8cc5f09 from 2014-12-01 in my environment.
To reproduce the issue, you need to configure some test data
Create a user Bob and another user Patrick
Create a project Database review. Add Bob to the new project Database Review. Bob is the only member of this project.
Create a project Code review. Add Patrick to the new project Code review. Patrick is the only member of this project.
Create a project "my software". Both user have joined the project.
Create a package "package my software in the repository". The owner of the package is project "my software". Path is /trunk/mysoftware
create a package "subsection". The owner of the package is project "my software". Path is /trunk/mysoftware/subsection.
Create a herald rule on the repository, following guidance from the audit page,
1. When a commit is done on the repository
2. Differential revision does not exist
3. Repository is any of xxx
4. any affected package include all of "package my software"
Do :
1. Create a audit request for Project Database review and Code Review.
For this situation, I defined the two package, but I don't think they are nessesary to reproduce the issue.
Case 1 (working). A third user Roger commit to the repository. An audit is created for his commit for Project database review and code review.
In the list of "Project/Package Auditors" there is
1. "package my software"
2. subsection
3. Database review
4. Code review.
Bob approve the commit : project Database review is marked as approved. Package my software and subsection are also marked as approved as user has joined the owner of these package, project mysoftware.
Patrick approve the commit : Code review is marked as approved. Commit is accepted.
Case 2 (not working)
User Bob do a commit. An audit is created for his commit for Project database review and code review.
Patrick approve the commit : Project code review is marked as approved. Package my software and subsection are also marked as approved as user has joined the owner of these package, project mysoftware.
Bob approve it's own commit :
1. Bob is added in a new list of "auditor".
2. project "database review" is still "need attention" (or something like this, don't remember) in the "Project/Package Auditors".
The commit is completly stalled. Using remove does the same thing. Database review is still at need attention. There is no way to accept it completly and it will stay open forever.
So at first I tought that it was a weird bugs, with time I think I understood the rules of the audit :
1. If user bob added a commit
2. and an audit has been created for a project.
3. and bob accept it's commit, even tought he is part of the project database review, Phabricator does not accept his commit. Phabricator NEEDS to have SOMEONE ELSE accept the commit from the group. As the audit is not accepted as database review, phabricator will add the audit as BOB.
4. In this case, Bob is the ONLY member of project database review. Audit is stuck.
Workaround found for right now :
1. Have another user join the project database review. That other user will approve the commit. project database review will be marked as accepted, the other user will not be added to "auditors"
another solution :
1. In the herald rule, split the rule
1.1 add all the generic condition in step 1
1.2 If author is not any of bob, create an audit for database review.
This prevent the problem from happening....
Now, if I am right, this whole thing is more like about there is no documentation to explain how the audit approved condition works.
That might be a good idea !!
Also you could discuss my method here : why did you create a audit for a project you are the sole member of! Because I plan to add/change/remove people from this group later. Also having a group, people understand the name "database review" so they know that only the database review has been done, not the code review or whatever other review there are.
Well.. maybe I would be better to use real user for the time being and forget the group as it's kind of hard to understand how it's working. But in any case, maybe if I configure herald to add a real person as an auditor and not a project, my bob could approve it's own bob commit, but anyway I would still have to add that condition "If author is not any of bob, create an audit for database review." to now lose time on useless things... At this point, I would still not have the issue as I already added that workaround when using the project.
Tought about this ? Documentation issue or issue that was not accounted for ?