Page MenuHomePhabricator

Audit status is none on commit views of Audit if no Auditors specifically assigned
Closed, InvalidPublic

Description

Hi,

We're using a Herald rule to trigger an audit on all commits as described here.

The problem is in the Audit tool in the commits section (/audit/view/commits/), the commits that are Audit Required but haven't had any comments appear as Audit Status: None.
Commits with comments correctly appear as Audit Required.
(see attached audit-none.png for an example of this)

If you look at the commit in Diffusion, you'll see that the status is Audit Required as it should be.
(see audit-none-is-required.png for an example of this with rPUP6304 from the first screenshot)

Finally, this causes issues with the filter, as filtering by Open will only show those commits that have been acted on, rather than correctly list all the commits with Audit Required.
(see audit-none-open.png for this example. Notice that rPUP6304 is missing, but rPUP4725 is shown)

It looks like it might be only listing as Audit Required if there is a particular Auditor assigned, rather than a general Project.
If this is desired behaviour, perhaps include Audit Required: None in the Open filter.

Repro steps:

  1. Set up audit flow as per link above
  2. Make a commit and trigger an audit
  3. Notice that audit appears in Audit list
  4. Click through to Diffusion for commit and notice Audit is listed as Audit Required
  5. Go to Audit and select Commits->All (url /audit/view/commits/)
  6. Notice that commit is listed as Audit Status: None
  7. Comment on audit (without Accept/Raise Concern)
  8. Notice that commit is now listed as Audit Required.

Event Timeline

zorfling attached 3 file(s): Restricted File, Restricted File, Restricted File.
zorfling added a subscriber: zorfling.

I've just noticed I can display this behaviour here on http://secure.phabricator.com

See list:
https://secure.phabricator.com/audit/view/commits/
Notice rPd1daf80d0d03 is Audit Status: None

Click through and notice it's Audit Required by Herald Rule
https://secure.phabricator.com/rPd1daf80d0d03f6ea2ae4b0cadb635072d57d46e9

Notice it's not listed in the open filter
https://secure.phabricator.com/audit/view/commits/?status=open

I recall that we're actually using two sources for audit status. One status in the commit, and one in the actual audit.

https://secure.phabricator.com/diffusion/P/browse/master/src/applications/audit/constants/PhabricatorAuditStatusConstants.php
https://secure.phabricator.com/diffusion/P/browse/master/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php

The filters and constants are set alright, we probably miss to sync the commit status at some point (likely during the Herald update). I'll take a closer look.

epriestley triaged this task as Normal priority.Apr 9 2013, 5:10 PM

This definitely looks like a bug to me. I don't immediately see what's causing the issue from spending 30 seconds glancing at the code, though -- let me know what you figure out, @AnhNhan

chad renamed this task from Audit status is none on commit views of Audit if no Auditors specifically assigned. to Audit status is none on commit views of Audit if no Auditors specifically assigned.Jul 3 2015, 3:59 AM
chad changed the visibility from "All Users" to "Public (No Login Required)".

This is 100 years old and I'm no longer sure what it's describing or how to reproduce it. Feel free to file a new issue if this is still a problem. Note that Audit is churning a lot right now (see T10978) so the behavior may change in the near term. This may also be a "viewer audit status" vs "commit overall audit status" issue in some sense (T9482 / T6024).

Yep @epriestley this one looks fixed.

Finally, this causes issues with the filter, as filtering by Open will only show those commits that have been acted on

Open Audits filter is now listing untouched audits correctly.