Page MenuHomePhabricator

CU 2.0: Display data in the Compare tab
Closed, ResolvedPublic2 Estimate Story Points

Description

Goal

This task is for displaying the information we fetch for the Compare tab in T238714: CU 2.0: Fetch information to be displayed in Compare tab.

Acceptance criteria

Mock: https://prtksxna.github.io/wmf-cu-goodplace/compare.html
The information is placed in one table like the one in the Epic ticket:

Things to note:
  • There is a new tab next to Preliminary check for Compare
  • In the initial load, rows are grouped together by username, followed by IP address (for different UAs).
  • If an IP and username looked up are in the same row, make both the IP address and the username bold with darker background.
  • There is a visual indicator to distinguish the usernames and IPs explicitly entered by the user. They are bold with a darker background.
  • The username links to the user-page for the user.
  • The IP address does not link anywhere. Make them black.

Excluded stuff:

  • Include option to sort in the Username, IP address and User-agent column headers. We'll make the sorting actually work in a separate task. - We'll add the sort options in the UI when we do the sorting.
  • There is a button for the (10 from 3 other users) information that would allow the user to click it and add the new rows to the table. We'll add the functionality to actually append the new rows in a follow-up ticket. ! - The button will be done in the follow-up ticket as well.

Details

Related Gerrit Patches:
mediawiki/extensions/CheckUser : masterFix ip css styling on Compare tab in Special:Investigate
mediawiki/extensions/CheckUser : masterAdd compare tab to Special:Investigate

Event Timeline

Niharika created this task.Nov 20 2019, 7:07 PM
Niharika moved this task from Untriaged to Design on the Anti-Harassment board.

@Prtksxna Putting this in your column for the couple mocks needed.

Niharika assigned this task to Prtksxna.Nov 20 2019, 8:10 PM
Niharika triaged this task as Medium priority.
Niharika updated the task description. (Show Details)Nov 21 2019, 5:24 PM
Niharika set the point value for this task to 2.Nov 21 2019, 5:29 PM
Prtksxna added a comment.EditedDec 29 2019, 6:52 PM

The IP column currently has four pieces of information:

  • IP
  • Number of edits by this user using the IP
  • Total number of users using this IP
  • Total number of edits using this IP

Since

  1. the CheckUser might want to sort on some of these fields and
  2. this column is currently overloaded and a bit confusing…

We're going to split this column into four — https://prtksxna.github.io/wmf-cu-prototype/compare.html

I am not happy with the column header labels though, too long.

@Prtksxna or @Niharika is "Edits using this IP" the "Public" IP edits or is it the "Total" edits from that IP (Public and Logged-in Users)?

@Prtksxna or @Niharika is "Edits using this IP" the "Public" IP edits or is it the "Total" edits from that IP (Public and Logged-in Users)?

I think the latter (based on the comments in T238714) . And it should include the the edits made by the current (as in current row) user too. This time, I have tried to make the numbers on the prototype add up correctly to get a feel for it and I think it's making sense. What do you think @Niharika?

Yeah, I think it should be the Total edits as well.

Demian added a comment.EditedJan 9 2020, 1:22 AM

@Niharika for effectively using the checkuser it would be important to also present the subnet/ISP the IP belongs to (related T174553). I've suggested adding this to the design 2 months ago in T237593#5643299, but there was no discussion about it since then. I reckon if this is not done now with the fundamental design work, then it won't be done in the coming years. This is the best time to make that part of the design.

With secondary importance presenting the Tor/VPN/proxy likelihood would be a similar task, the two could be done together.

@Prtksxna I've updated the two patches for the mock-up from 2 months ago that you could merge: a layout fix, a missing </div> and adding "vpn check" to the IP tools.

SQL added a subscriber: SQL.Jan 14 2020, 5:00 PM
Demian added a comment.EditedJan 17 2020, 2:12 AM

@Prtksxna have you looked at the patches I've submitted? Those are fairly trivial, should take long to review. Is there something wrong with it?

Niharika reassigned this task from Prtksxna to dmaza.Jan 24 2020, 8:41 PM
Niharika added a subscriber: dmaza.

@Prtksxna Assigning this to @dmaza since he's working on it already but please add the missing mock (button/icon to add other users from the same IP to the results). Thanks!

dmaza added a comment.Jan 27 2020, 8:49 PM

@Niharika @Prtksxna: It is my understanding that we are going by the task description/acceptance criteria and not by the mocks at https://prtksxna.github.io/wmf-cu-prototype/compare.html. Please confirm and double check that everything we want is in the task description.

dmaza added a comment.Jan 30 2020, 2:19 AM

@Niharika @Prtksxna on the following point from the acceptance criteria There is a visual indicator to distinguish the usernames and IPs explicitly entered by the user.; What happens when the user enters and IP Range? Do we care to mark the ips in that range with the visual indicator too?

Niharika added a comment.EditedFeb 3 2020, 8:44 PM

@Niharika @Prtksxna on the following point from the acceptance criteria There is a visual indicator to distinguish the usernames and IPs explicitly entered by the user.; What happens when the user enters and IP Range? Do we care to mark the ips in that range with the visual indicator too?

The visual indicator @Prtksxna picked in the mocks is that the usernames and IPs entered are in bold with a darker background:


Apples, 1.1.1.2 and Grapes are entered by username in the mock.
For a range, we should do the same to all the IPs in the range.

@Niharika @Prtksxna: It is my understanding that we are going by the task description/acceptance criteria and not by the mocks at https://prtksxna.github.io/wmf-cu-prototype/compare.html. Please confirm and double check that everything we want is in the task description.

The mocks are more up to date.

Demian added a comment.Feb 3 2020, 9:19 PM

@Niharika is this under consideration or will it be?

In T238782#5846160, @AronManning wrote:

@Niharika is this under consideration or will it be?

Are you talking about the IP subnets? It is under consideration once we have the basics in place. I reckon it will be pretty easy to add those in even after the fact. I'm more concerned about getting the existing necessary things right and we can tackle that after. Does that sound okay to you?

Demian added a comment.EditedFeb 3 2020, 9:25 PM

Yes, IP subnets (ISP) and vpn/proxy likelihood. If you have it in the plan, that's perfect. I was a bit concerned there was no reply in months ;-)
Grouping by subnet might be a fundamental design decision, that's worth thinking about in advance if it would be implemented.

Niharika updated the task description. (Show Details)Feb 5 2020, 4:46 PM
Niharika updated the task description. (Show Details)Feb 10 2020, 9:46 PM

Yes, IP subnets (ISP) and vpn/proxy likelihood. If you have it in the plan, that's perfect. I was a bit concerned there was no reply in months ;-)
Grouping by subnet might be a fundamental design decision, that's worth thinking about in advance if it would be implemented.

Grouping by subnet would be nearly the same as sorting, I believe. We are still thinking about how sorting will work so I will bring this up when we talk more about that.

dmaza added a comment.EditedFeb 11 2020, 4:54 AM

@Niharika Usernames and IPs look like they are links. Where are they linking to?

@Niharika Usernames and IPs look like they are links. Where are they linking to?

Oh good question. The username can link to their user page. The IP address should not be a link. No good place to link those too. Will update the task desc.

Niharika updated the task description. (Show Details)Feb 11 2020, 5:51 PM
Niharika updated the task description. (Show Details)Feb 11 2020, 6:46 PM

I have a few questions about the acceptance criteria:

  • There is a new tab next to Preliminary check for Compare (need to come up with a better name though).

Should we make a separate task for coming up with a name?

  • Include option to sort in the Username, IP address and User-agent column headers. We'll make the sorting actually work in a separate task.

It's easier to make the sorting functionality and the visual indicator that sorting is available in the same task. Can we move the visuals out of this one?

  • There is a button for the (10 from 3 other users) information that would allow the user to click it and add the new rows to the table. We'll add the functionality to actually append the new rows in a follow-up ticket. !

Similarly to the sorting, can we put adding a button into the same ticket as making it functional?

Change 571626 had a related patch set uploaded (by Dmaza; owner: Dmaza):
[mediawiki/extensions/CheckUser@master] Add compare tab to Special:Investigate

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

I have a few questions about the acceptance criteria:

  • There is a new tab next to Preliminary check for Compare (need to come up with a better name though).

Should we make a separate task for coming up with a name?

Nah, it's fine for now. If we find this to be confusing during user testing, then I will make a follow-up task.

  • Include option to sort in the Username, IP address and User-agent column headers. We'll make the sorting actually work in a separate task.

It's easier to make the sorting functionality and the visual indicator that sorting is available in the same task. Can we move the visuals out of this one?

Yep. Will update the description.

  • There is a button for the (10 from 3 other users) information that would allow the user to click it and add the new rows to the table. We'll add the functionality to actually append the new rows in a follow-up ticket. !

Similarly to the sorting, can we put adding a button into the same ticket as making it functional?

Yep. Will update the description.

Niharika updated the task description. (Show Details)Feb 14 2020, 6:18 PM
Niharika updated the task description. (Show Details)

Change 571626 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Add compare tab to Special:Investigate

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

  • If an IP and username looked up are in the same row, make both the IP address and the username bold with darker background.

@Niharika Currently, all IP addresses are bold (example below). Should this be changed?

Also, I don't know if it is just my screen, but I don't find the dark background much darker than the other cells in the table.

  • If an IP and username looked up are in the same row, make both the IP address and the username bold with darker background.

@Niharika Currently, all IP addresses are bold (example below). Should this be changed?
Also, I don't know if it is just my screen, but I don't find the dark background much darker than the other cells in the table.

Yep, not all IPs should be bold, just the ones in the initial investigation form. We should fix this in this ticket. Flagging for @dmaza.

For the colors, I think all the colors/design differ from Prateek's mock. We can have a separate ticket to work on that.

dmaza added a comment.Feb 21 2020, 8:14 PM

@Niharika my bad. I understood that they should all be bold since they are not a link anymore. I'll fix it in a follow up patch asap

dmaza added a comment.Feb 21 2020, 9:39 PM

@Niharika, is this what it should look like?

Change 574103 had a related patch set uploaded (by Dmaza; owner: Dmaza):
[mediawiki/extensions/CheckUser@master] Fix ip css styling

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

@Niharika, is this what it should look like?

Yes, thank you. Sorry for the delay.

Yes, thank you. Sorry for the delay.

No problem and thank you!

Change 574103 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Fix ip css styling on Compare tab in Special:Investigate

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

Also, I don't know if it is just my screen, but I don't find the dark background much darker than the other cells in the table.

Eek, this is a problem! While I can see the different grays in that screenshot we need to make sure that the contrast can bee seen on all types of screens. @dom_walden which screen are you using? Are the screenshots below any better?

Base80Base70Base100 (for other cells)
This comment was removed by Prtksxna.

Also, I don't know if it is just my screen, but I don't find the dark background much darker than the other cells in the table.

Eek, this is a problem! While I can see the different grays in that screenshot we need to make sure that the contrast can bee seen on all types of screens. @dom_walden which screen are you using? Are the screenshots below any better?

I believe it is this: https://www.acer.com/ac/en/NZ/content/model/UM.FG6SA.A01

The "Base70" one looks good on my screen, the highlighting being clear. The other two it isn't very clear.

I think I might have quite a low contrast setting. Fiddling about with it, the other two become much clearer. I don't know what we want to do.

  • There is a new tab next to Preliminary check for Compare

Yep.

  • In the initial load, rows are grouped together by username, followed by IP address (for different UAs).

..and then by user agent (in alphabetical order?)

  • If an IP and username looked up are in the same row, make both the IP address and the username bold with darker background.
  • There is a visual indicator to distinguish the usernames and IPs explicitly entered by the user. They are bold with a darker background.

For a search for "User1" and IP "1.2.3.4", it looks something like:

  • The username links to the user-page for the user.

On my local, they link to: http://dev.wiki.local.wmftest.net:8080/w/index.php?title=User:$user&action=edit&redlink=1 when user page does not exist.

Or http://dev.wiki.local.wmftest.net:8080/wiki/User:User4 when user page does exist.

  • The IP address does not link anywhere. Make them black.

Yep.

Version tested: Local vagrant, MediaWiki 1.35.0-alpha (7f637d3), CheckUser 2.5 (8d495b2) 07:52, 25 February 2020

Niharika closed this task as Resolved.Feb 26 2020, 12:39 AM

Perfect, thanks!