Page MenuHomePhabricator

mw.thanks.getUserGender should deduplicate requests
Open, LowPublic

Description

When I visit a Flow page with two posts by the same user and I have already thanked both of these posts, I see two API requests for list=users&usprop=gender for the same user.

Stack trace of these API requests:

jQuery.ajaxTransport.send	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:140
jQuery.extend.ajax	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:134
mw.Api.ajax	@	load.php?debug=false&lang=en&modules=ext.echo.base%2Coverlay|ext.echo.overlay.init|ext.eventLogging…:274
mw.Api.get	@	load.php?debug=false&lang=en&modules=ext.echo.base%2Coverlay|ext.echo.overlay.init|ext.eventLogging…:273
mw.thanks.getUserGender	@	load.php?debug=false&lang=en&modules=ext.echo.base%2Coverlay|ext.echo.overlay.init|ext.eventLogging…:81
(anonymous function)	@	load.php?debug=false&lang=en&modules=ext.echo.base%2Coverlay|ext.echo.overlay.init|ext.eventLogging…:236
jQuery.extend.each	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:5
jQuery.fn.jQuery.each	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:2
reloadThankedState	@	load.php?debug=false&lang=en&modules=ext.echo.base%2Coverlay|ext.echo.overlay.init|ext.eventLogging…:236
(anonymous function)	@	load.php?debug=false&lang=en&modules=ext.echo.base%2Coverlay|ext.echo.overlay.init|ext.eventLogging…:237
mw.loader.implement.thanks-button-thanked	@	load.php?debug=false&lang=en&modules=ext.echo.base%2Coverlay|ext.echo.overlay.init|ext.eventLogging…:237
runScript	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:162
mw.loader.checkCssHandles	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:163
execute	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:164
handlePending	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:161
runScript	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:162
mw.loader.checkCssHandles	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:163
execute	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:164
handlePending	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:161
runScript	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:162
mw.loader.checkCssHandles	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:163
execute	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:164
handlePending	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:161
runScript	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:162
mw.loader.checkCssHandles	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:163
execute	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:164
handlePending	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:161
runScript	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:162
mw.loader.checkCssHandles	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:163
execute	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:164
mw.loader.implement	@	load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=wt%2Fyo6SB:170
(anonymous function)	@	load.php?debug=false&lang=en&modules=ext.echo.base%2Coverlay|ext.echo.overlay.init|ext.eventLogging…:312

Event Timeline

Catrope created this task.Jun 4 2015, 12:04 AM
Catrope raised the priority of this task from to Needs Triage.
Catrope updated the task description. (Show Details)
Catrope added a subscriber: Catrope.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 4 2015, 12:04 AM
Catrope set Security to None.
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptJun 4 2015, 12:04 AM

In addition to deduplication (not sending multiple requests for the same user's gender) it would probably also be good if these requests were combined: when there's multiple Flow posts by different authors that we need the gender for, send one request asking about all of these users at once (using ususers=Alice|Bob|Carol) rather than sending separate requests for each user.

Alternatively, we could embed this information in data attributes.

The server loads this info already for thanks-thank/thanks-thanked (for regular revision thanks, but it will have to be for Flow too if we do T101342: Don't make it seem user can re-thank for a Flow post when they already did; render server-side); see generateThankElement. That means we should be able to pull it out of GenderCache into a data attribute with no performance penalty.

In T101340#1336242, @Mattflaschen wrote:

Alternatively, we could embed this information in data attributes.
The server loads this info already for thanks-thank/thanks-thanked (for regular revision thanks, but it will have to be for Flow too if we do T101342: Don't make it seem user can re-thank for a Flow post when they already did; render server-side); see generateThankElement. That means we should be able to pull it out of GenderCache into a data attribute with no performance penalty.

That would help with not sending API requests, yes. Although I suppose if we solved T101342: Don't make it seem user can re-thank for a Flow post when they already did; render server-side then we could do the entire message render on the server and then the client would never have to know about these users' genders at all.

That would help with not sending API requests, yes. Although I suppose if we solved T101342: Don't make it seem user can re-thank for a Flow post when they already did; render server-side then we could do the entire message render on the server and then the client would never have to know about these users' genders at all.

It can pre-render 'Thank' and 'Thanked' on the server (and already does for regular revisions). However, if the user actually does a thank action, it needs to change it to Thanked, which requires the gender data (or pre-rendering the Thanked message, but I think gender data is cleaner).

Catrope triaged this task as Low priority.Jul 22 2015, 11:18 PM
Restricted Application added a project: Growth-Team. · View Herald TranscriptAug 30 2018, 5:17 PM