Page MenuHomePhabricator

Problems in the "Commits in this Package that Need Attention" view
Closed, ResolvedPublic

Description

After the changes in audit (we are using it heavily) we have different problems with it:

  1. In the "Needs Audit" query I now also see my own commits which should not be in my audit list, correct?
  1. in the package view, if I click on "Commits in this Package that Need Attention", the following error is displayed: "Unknown audit status 'open'! Valid statuses are: audit-status-any, audit-status-open, audit-status-concern, audit-status-accepted, audit-status-partial." See also screenshot.

Could you please have a look?

Thank you!

Event Timeline

alexnb assigned this task to epriestley.
alexnb updated the task description. (Show Details)
alexnb added projects: Audit, Restricted Project.
alexnb added subscribers: gou1, joshuaspence, epriestley and 4 others.

audit_package_problems.PNG (804×1 px, 47 KB)

Also, the status icon is wrong.

For the first half of this, see T9430 / D14013 / T9372.

Second half is just a bug.

I'm not immediately able to reproduce that status icon issue, what does the icon say when you hover over it?

Oh, never mind -- pretty sure I figured it out.

epriestley, thanks for your quick reply! I should have searched before entering the task, because T9430 which you mention cover exactly the pains we experience.

Hovering over the (?) icon brings the "Not Applicable" tooltip .

D14189 should fix the button.

The icon is somewhat entwined with the T9430 / T9372 mess. We're showing an icon corresponding to the first audit you have authority over on the commit, or the first audit if no such audit exists. In this UI, you reasonably expect us to show the commit's overall state instead.

However, communicating the commit's audit status with respect to the viewer is still valuable, and I'm hesitant to just swap this behavior locally without considering the larger picture. Particularly, whatever comes of T9372 may inform icon handling and object vs viewer state communication here.

All of this is likely fairly straightforward, I just don't have an approach that I like in mind for T9372 yet.

eadler added a project: Restricted Project.Jan 11 2016, 9:40 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.May 13 2016, 9:58 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jun 1 2016, 10:39 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jul 4 2016, 9:18 PM

I'm going to switch this view to show overall commit audit status, not weird semi-viewer audit status. At one point it wasn't clear how heavily we were going to try to skew views to account for viewer status, but bucketing changes seem to have reduced the need for this and I think extending the Differential model to Audit now makes sense. See also T9430 for some context.