Page MenuHomePhabricator

CU 2.0: Add button to add all other users on an IP address to an investigation
Closed, ResolvedPublic2 Estimated Story PointsMar 25 2020

Description

Mock

Active buttonDisabled button (only shown on hover)
Screenshot 2020-02-26 at 10.33.42 AM.png (530×2 px, 185 KB)
Screenshot 2020-02-26 at 10.33.53 AM.png (1×2 px, 433 KB)

Acceptance criteria

  • There is a button underneath every IP address - Show all N users on this IP
  • The button is blue when there are users using this IP that are not already in the table
  • The button is gray if all the users using this IP are in the table already
    • The disabled button is only shown on hovering over the IP cell
    • There is a message on hover telling them why it's disabled - All users on this IP are already in the table
  • On click, all the other users from the same IP are added to the table.

Event Timeline

Niharika triaged this task as Medium priority.Feb 11 2020, 12:52 AM
Niharika created this task.
Niharika renamed this task from CU 2.0: Add button to add all other users on an IP address to CU 2.0: Add button to add all other users on an IP address to an investigation.Feb 11 2020, 12:54 AM
ARamirez_WMF changed the subtype of this task from "Task" to "Deadline".
ARamirez_WMF changed Due Date from Mar 19 2020, 4:00 AM to Mar 25 2020, 4:00 AM.Mar 18 2020, 5:11 PM

Change 581791 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/extensions/CheckUser@master] Add buttons to Special:Investigate for adding extra IP targets

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

Change 581791 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Add buttons to Special:Investigate for adding extra IP targets

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

dom_walden added a subscriber: dom_walden.
  • There is a button underneath every IP address - Show all N users on this IP

Yep, except we don't show the number of users.

  • The button is blue when there are users using this IP that are not already in the table
  • The button is gray if all the users using this IP are in the table already

Button is blue if the IP is not in the investigation, gray if it is. Does not seem to matter whether or not all users on an IP are in the table.

  • The disabled button is only shown on hovering over the IP cell

Yep.

  • There is a message on hover telling them why it's disabled - All users on this IP are already in the table

Yep.

  • On click, all the other users from the same IP are added to the table.

Similarly to T244817#6018033, investigating IP "1.2.3.4" and clicking "Show all users..." for IP "1.2.3.5" is the same as doing a new investigation for both of those IPs.

Similar comments regarding filters (T244817#6018033). If an IP is already in the filters, you can click "Show all users..." for that IP but it doesn't change anything about the results returned, but a log entry is still made for that IP.

  • There is a button underneath every IP address - Show all N users on this IP

Yep, except we don't show the number of users.

Oops, this got missed. We should add the user count to the button.

@dom_walden @Niharika Thanks for pointing this out! I thought we'd discussed leaving out N somewhere, but I'm struggling to find it right now...

The problem is we don't know what N is, and we'd need to do another query for each IP and each user in the results in order to find it out. We'd basically need to do the check for all IPs/users in order to find out how many there are.

It's similar to the problem where we've hit the limit for getting edits, but we don't know which target hit the limit without doing more queries. I'd recommend approaching this in the same way - if we feel it's likely to be important, do another iteration see how it affects performance, before we add the extra queries.

@dom_walden @Niharika Thanks for pointing this out! I thought we'd discussed leaving out N somewhere, but I'm struggling to find it right now...

The problem is we don't know what N is, and we'd need to do another query for each IP and each user in the results in order to find it out. We'd basically need to do the check for all IPs/users in order to find out how many there are.

It's similar to the problem where we've hit the limit for getting edits, but we don't know which target hit the limit without doing more queries. I'd recommend approaching this in the same way - if we feel it's likely to be important, do another iteration see how it affects performance, before we add the extra queries.

Hmm. I'm sorry, I do not recall that conversation. We should get better at documenting slack/meeting decisions. :(

Anyway, I'll close this ticket and open a new one if Prateek and I think it's important to do this. Thanks for explaining!

Hmm. I'm sorry, I do not recall that conversation. We should get better at documenting slack/meeting decisions. :(

Not that all problems are tool problems, but there are tools that make it easy to connect Slack conversations (or email chains) with a task management system so that all conversations about a task can be collated into that task.

It's not something that we will do here but it's something to recognize is possible.

Thanks @Niharika and @aezell - agreed.

(Just so nobody else feels unduly at fault here, another possibility is that I misremembered ever having that conversation!)