Page MenuHomePhabricator

CU 2.0: Sorting in the Compare tab
Closed, ResolvedPublic3 Estimated Story Points

Description

Goal

Users should be able to sort the displayed data in the Compare tab to be able to find patterns more easily.

Acceptance criteria
  • Sorting is possible on IP, username and user-agent. By clicking the column header, the results will sort.
  • Sorting will only work if there is one page worth of data. If there are multiple pages, sorting is not available.

Related Objects

Event Timeline

Niharika triaged this task as Medium priority.Nov 4 2019, 10:21 PM
Niharika removed a project: Epic.
Niharika renamed this task from Add filtering, sorting, pagination(?) to the data table to CU 2.0: Sorting in the Compare tab.Nov 20 2019, 7:41 PM
Niharika updated the task description. (Show Details)

A few thoughts on the implementation:

  1. Less than ca. 100 results should all be sent to the client and sorted client-side for lower latency.
  2. More results could be sorted and paginated server-side. The second page is visited less often, so this would save loading time and bandwidth.
  3. Result pages should be loaded only once and cached in the client. After all pages are loaded (visited), sorting can be done client-side.
  4. The pagination state is: usernames, CU log unique key, CU log timestamp, ordering, starting index, count. Details: T239680#5713699
dbarratt updated the task description. (Show Details)
Niharika removed the point value for this task.
ARamirez_WMF set the point value for this task to 3.Jun 3 2020, 4:44 PM

Change 602121 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/extensions/CheckUser@master] ComparePager: Make the results table sortable if there is only 1 page

https://gerrit.wikimedia.org/r/602121

@Prtksxna @Niharika How important is it that the date column does not sort? Having had a little look into this, we would have to add the unsortable class to that table header cell, and it's a bit messy to access due to the way that the table is built in TablePager::getStartBody.

@Prtksxna @Niharika How important is it that the date column does not sort? Having had a little look into this, we would have to add the unsortable class to that table header cell, and it's a bit messy to access due to the way that the table is built in TablePager::getStartBody.

The date column has ranges so it's a bit unintuitive what the sorting would do. Will it treat the ranges as strings for sorting?

Will it treat the ranges as strings for sorting?

By default yes (though if the user's preferred date format is something like 2020-06-03 then it sorts numerically, which equates to sorting by start date then end date).

We can specify a value to sort the cell by, which can be different from the cell's content - e.g. we could make sure that "3 June 2020" is the sort value, or that the number 2020-06-03 is. Due to the same quirk in the way the table HTML is built, it is actually easier to do this than to make the column unsortable.

Just to clarify, it is technically possible to make the column unsortable - it's just a bit messy.

By default yes (though if the user's preferred date format is something like 2020-06-03 then it sorts numerically, which equates to sorting by start date then end date)
We can specify a value to sort the cell by, which can be different from the cell's content - e.g. we could make sure that "3 June 2020" is the sort value, or that the number 2020-06-03 is. Due to the same quirk in the way the table HTML is built, it is actually easier to do this than to make the column unsortable.

If we could always sort by start date — regardless of the users' date format settings — then we could have the date column be sortable too.

By default yes (though if the user's preferred date format is something like 2020-06-03 then it sorts numerically, which equates to sorting by start date then end date)
We can specify a value to sort the cell by, which can be different from the cell's content - e.g. we could make sure that "3 June 2020" is the sort value, or that the number 2020-06-03 is. Due to the same quirk in the way the table HTML is built, it is actually easier to do this than to make the column unsortable.

If we could always sort by start date — regardless of the users' date format settings — then we could have the date column be sortable too.

+1 to this.

@Tchanders I'm assuming there isn't an easy way to sort by an cell's attributes rather than it's value?

Change 602121 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] ComparePager: Make the results table sortable if there is only 1 page

https://gerrit.wikimedia.org/r/602121

Change 603636 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/extensions/CheckUser@master] ComparePager: Sort date range column by first edit and last edit dates

https://gerrit.wikimedia.org/r/603636

Change 603636 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] ComparePager: Sort date range column by first edit and last edit dates

https://gerrit.wikimedia.org/r/603636

@Tchanders I am a bit confused about what I am seeing when sorting IPv6 addresses. Sorting ascending:

sorting_asc_ipv6.png (275×440 px, 18 KB)

I thought 2a06:f500:1714:e8ac:e97:5d42:de10:989e should be at the bottom.

The "initial" sort order has them in what I think is the correct (ascending) order:

sorting_init_ipv6.png (272×432 px, 7 KB)

IPv4 addresses appear to sort fine ascending:

sorting_ipv4.png (274×344 px, 13 KB)

But, the "initial" order doesn't seem right:

sorting_init_ipv4.png (268×340 px, 13 KB)

(N.B. All the rows in the screenshots above are for the same users.)

@dom_walden Thanks for finding this - it's a bit horrible...

The initial order is determined by the query, which has a bug where it's sorting by the prettified string rather than the hex number: T255312

The ascending and descending orders are determined by the tablesorter, which has special handling for IPv4 addresses, but not IPv6.

Possible solutions:

Change 605330 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/extensions/CheckUser@master] ComparePager: Add sort values to the results for the tablesorter

https://gerrit.wikimedia.org/r/605330

Filed T255694 for the IP address sorting - otherwise, this is done.

(The above patch has been tagged with the new task number instead.)