Page MenuHomePhabricator

Consolidate CheckUser front-end modules
Closed, ResolvedPublic

Description

With the introduction of Special:Investigate, several new modules were also introduced. The extension currently has the following modules:

ext.checkUser
ext.checkUser.investigate
ext.checkUser.investigateblock
ext.checkUser.investigateblock.styles
ext.checkUser.investigate.styles
ext.guidedTour.tour.checkuserinvestigate
ext.guidedTour.tour.checkuserinvestigateform

This contravenes the effort to keep the number of distinct modules low: https://www.mediawiki.org/wiki/Wikimedia_Performance_Team/Page_load_performance#Size_of_scripts - particularly since there is unlikely to be a functional requirement for all of these to be distinct.

We should consolidate these modules where possible.

Event Timeline

@Ladsgroup - I've been told you may be already working on this? (No worries if not of course, just wanted to check.)

It's in my todo list but haven't started working on it yet. If you want to pick it up, I would be more than happy to at least review it :)

Niharika triaged this task as Medium priority.Sep 23 2020, 5:25 AM

Change 630329 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/CheckUser@master] Merge modules of Investigate and InvestigateBlock special pages

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

I added this that merges two modules ^ I didn't have time to fully test all of its aspects, please take a look. Regarding the GuidedTours, I'm not sure it would give much value specially since CUs are seasoned editors (maybe we can remove it later?) but my personal (biased towards performance) opinion is to remove the GT modules altogether. I assume we will drop Special:CU one day too. Please 🥺

Thanks for the patch @Ladsgroup.

As for the GuidedTours, there was a conscious product/design decision to add them - @Prtksxna, @cwylo or @Niharika might have more details about why they were added.

@Ladsgroup We introduced some radical changes in Special:Investigate that we found users did not discover on their own. For example you can enter multiple usernames or IPs at the same time in the lookup form. Many users missed this in our usability test. To make them aware of the new features we decided to add GuidedTour. We can remove it three weeks after we have launched the new feature everywhere. Does that sound okay?

@Ladsgroup We introduced some radical changes in Special:Investigate that we found users did not discover on their own. For example you can enter multiple usernames or IPs at the same time in the lookup form. Many users missed this in our usability test. To make them aware of the new features we decided to add GuidedTour. We can remove it three weeks after we have launched the new feature everywhere. Does that sound okay?

That sounds amazing. Thank you!

Majavah renamed this task from Consolidate CheckUser font-end modules to Consolidate CheckUser front-end modules.Oct 1 2020, 5:11 PM

@Ladsgroup Do you think it's worth documenting the performance issue with GuidedTour on the extension page: https://www.mediawiki.org/wiki/Extension:GuidedTour ? Could help teams in the future when weighing up whether to add a tour. Pinging also Growth-Team, who are listed as the maintainers (though they may no longer be!)

We can remove it three weeks after we have launched the new feature everywhere. Does that sound okay?

I think we should keep the GuidedTour even after people have transitioned to the new tool. There will be new people who encounter the Special:Investigate (once they get the CU right) and the tour would help familiarize them to the tool.

If you really want to keep GT modules, there are some ways you can at least merge them to one module. From @Krinkle:

  • one way would be to have the tour code go into one big module, like messageposter. you'd have an extension registry attribute like "GuidedTourModule", and then the guidedtour extension takes those, merges them, and registers them as one file module. This would mean 'scripts', no packageFile module since those cannot easily be merged as simple PHP arrays. see MessagePosterModule for an example of that
  • Another way would be to expand the configuration to take a module name and property name. This would be much simpler, but only solves the problem if the extension isn't perf-sensitive and has another module you can throw the code into. E.g. for CheckUser, you could throw the code in a random module, and then tell guided tour to get it from require('ext.checkUser.stuff').tourA with some JSON declaration
  • Similar to #2, we could invert the way GT works and make individual extensions responsible for loading the tours instead of making GT do it. E.g. centralnotice special page or hooks would queue what they need, and their own code would load and/or construct whatever they need to do, like you would for any other JS on the page.
  • Solve T225842 first, and make these private modules, automatically merged by RL into one "ext.guidedTours.tours" bundle

All of these ^ require work on GT extension itself. I'm not sure you want to do it.

For this ticket, I would suggest to ignore GuidedTour and focus on consolidating the other modules only.

Unfortunately, GuidedTour currently has a hard-requirement that necessitates shipping code in an inefficient manner. The above was result from a brainstorm on how (someone) could pick up GuidedTour in the future and make it perform to current standards. This is important in the big picture and imho a prerequsite for any new tours. However, for CheckUser specifically, I cannot quantify any individual tour as being problematic. It is not. So long as they are intended to be phased out within a few months, I wouldn't worry about it. We can spend that effort elsewhere for more impact :)

Change 630329 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Merge modules of CheckUser, Investigate and InvestigateBlock special pages

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

Tchanders claimed this task.