Page MenuHomePhabricator

Max results should not be hard-coded in CheckUser
Closed, ResolvedPublic

Description

Currently, the "5000" row limit to results is hard-coded in several places; this includes both in interface messages ("checkuser-too-many") and in the logic of the code (SpecialCheckUser::doIPEditRequest())

It should abstracted out as a config variable, or at least as a constant defined in the class.

Event Timeline

Huji created this task.Jul 9 2020, 11:25 PM

@ST47 I would encourage you to fix this first, before fixing the "get edits" issue.

ST47 claimed this task.Jul 10 2020, 6:11 PM

Change 611399 had a related patch set uploaded (by Gerrit Patch Uploader; owner: [[mw:User:ST47]]):
[mediawiki/extensions/CheckUser@master] T257641: Create a config variable for the max result count

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

ST47 added a comment.Jul 10 2020, 7:36 PM

@Huji I think I fixed the lint errors with this patch, but go ahead and make any other changes that are needed

ST47 added a comment.Jul 10 2020, 11:24 PM

@Huji, I don't know if I agree with your comment here. Prior to this change, if someone called doUserIPsDBRequest(.., .., 200), then the LIMIT would be 200. I don't know when that would ever happen - since it isn't possible to specify a limit in Special:CheckUser - but the current patch set behaves in the same way as the original code.

Huji added a comment.Jul 11 2020, 12:21 AM

The error is not yours; it is a Technical-Debt from before. The code assumed that the $limit value passed to different functions would be different (5000 for some, 5001 for others) and never documented anywhere either. While you are fixing the issue at hand, we could also address this technical debt too. But it is a not "must do" kind of thing; more a "nice to have".

I am going to review you and likely accept your patch in a few minutes.

ST47 added a comment.Jul 11 2020, 2:19 AM

Thanks, not sure why I didn't notice that in my own testing. I have set the limit to 7 edits in my localsettings.php, and get edits for user and get edits for IP are both working for me now.

Huji closed this task as Resolved.Jul 11 2020, 12:20 PM
Huji added a comment.EditedJul 11 2020, 12:31 PM

Thank you @ST47!

I second that! I think we are just one step away from ST47 setting up git review on their machine and becoming a lifelong developer :)