Page MenuHomePhabricator

Make Audit tables sortable
Closed, ResolvedPublic

Description

It would be great if the Audit tables were sortable like the PhabricatorSortTableExample.
The audits come up in an odd order (depending when they were committed vs pushed for example), and it would be nice to be able to sort by date.

I've attached a screenshot of the strange order.

Thanks.

Related Objects

Event Timeline

zorfling attached 1 file(s): Restricted File.
zorfling added a subscriber: zorfling.
animecyc added a subscriber: animecyc.

What do you think of this? Also would it be acceptable to order based on the content on page rather than all data. If we try and order based on all the data we have issues with auditor names that get parsed after the audit rows are retrieved.

I'm also not sure if the situation described in T2832 might cause issues for sorting?

Thanks for looking at this guys.

T2832 describes a UI problem which results from the current sort order -- at a minimum, we should either make the sort order group all audits for the same commit together or drop that UI rule.

I don't think we should impose a per-page order only (e.g., SortTable or any other client sorting which just rearranges the visible data without rearranging the data overall), since I think it will be very confusing for users. When a page is ordered alphabetically, and the last record us "ugrant", you assume the next page is going to have "v-z" and that there are only a couple of pages.

A global commit ordering is probably a better default than the current "when the thing happened to be inserted" ordering. A disadvantage is that recent activity on older commits does not rise to the top, but this is probably not much of a disadvantage overall.

The repository_auditrequest table is on the same database as the repository_commit table, so it's fine to just JOIN them to impose a sort order. However, repository_commit has no key which is usable for this query. To achieve reasonable performance, it's likely necessary to denormalize the epoch column from the repository_commit table into a new commitEpoch column on the repository_auditrequest table:

  • Add the column;
  • add a key on it;
  • make the code populate it when creating an audit request;
  • add a migration script to backpopulate all the existing audits (this can be a single UPDATE, no need to loop over the records in PHP).

Not a ton of work, but probably more than an hour's worth.

It might be worthwhile to fix T1768 first, too.

chad changed the visibility from "All Users" to "Public (No Login Required)".Jul 3 2015, 4:01 AM
chad added a subscriber: chad.

FWIW, these are no longer tables. I presume the solution is offering additional order by options in ApplicationSearch.

epriestley claimed this task.

They now sort by date and have some options.