Page MenuHomePhabricator

New #lastedit parserfunction
Closed, DeclinedPublic

Description

Author: Winbots

Description:
Patch for new parserfunction.

It would be useful, especially since StatusBot on enwiki was blocked, to have a #lastedit parserfunction which would accept a username as input and return a standard timestamp of the last edit by that username. I have included a patch file which should do this.


Version: unspecified
Severity: enhancement

attachment mediawiki.parserfunction.lastedit.diff ignored as obsolete

Details

Reference
bz14384

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 21 2014, 10:13 PM
bzimport added a project: MediaWiki-Parser.
bzimport set Reference to bz14384.
bzimport added a subscriber: Unknown Object (MLST).

ayg wrote:

+ wfGetDB( DB_SLAVE, 'contributions' );

You don't need the contributions server specifically here. Any slave will do.

+ array( 'rev_user_text' => $s ),

You want to trim $s, and normalize spaces to underscores, and maybe other stuff.

+ array( 'ORDER_BY' => 'rev_timestamp', 'LIMIT' => 1 )

Shouldn't that be 'ORDER BY', not 'ORDER_BY'? I don't think the latter works. And don't you want 'rev_timestamp DESC'? Also, LIMIT is redundant if you're doing selectField.

This parser function needs to be marked expensive. If we want still more such functions . . . pretty soon we'll be getting 100 extra queries every page view guaranteed. Sigh.

Was this patch tested? It doesn't seem as though it would work correctly.

soxred93 wrote:

Actually, the status of statubot is unknown (no pun intended). I'm still trying to negotiate with the admins a possible solution.

Winbots wrote:

(In reply to comment #1)

+ wfGetDB( DB_SLAVE, 'contributions' );

You don't need the contributions server specifically here. Any slave will do.

+ array( 'rev_user_text' => $s ),

You want to trim $s, and normalize spaces to underscores, and maybe other
stuff.

+ array( 'ORDER_BY' => 'rev_timestamp', 'LIMIT' => 1 )

Shouldn't that be 'ORDER BY', not 'ORDER_BY'? I don't think the latter works.
And don't you want 'rev_timestamp DESC'? Also, LIMIT is redundant if you're
doing selectField.

This parser function needs to be marked expensive. If we want still more such
functions . . . pretty soon we'll be getting 100 extra queries every page view
guaranteed. Sigh.

Was this patch tested? It doesn't seem as though it would work correctly.

No, it wasn't tested. I didn't have access to a webserver when I was writing it (on my laptop). Also, this was the first time using the database classes in code modifications. Yes, you are correct in all instances.

Is this somehow useful for creating of wiki content (either articles or community)? If it's just for bots, maybe trying of API would be better.

Winbots wrote:

(In reply to comment #4)

Is this somehow useful for creating of wiki content (either articles or
community)? If it's just for bots, maybe trying of API would be better.

Yes. It isn't really related to bots at all, except that there was a bot updating hundreds of status pages which contained this information that was transcluded. Brion blocked that bot for using too many resources. I think a parser function is a lot more efficient than having a bot query the api every x minutes and update several pages, which would then be transcluded. On enwiki, people directly transclude these pages onto their user/talk pages or though templates that transclude them. Also, a few lists of users transclude the pages to show status of the users in the list.

Winbots wrote:

Fixed, tested patch.

I have attached a new fixed, tested patch. :)

attachment mediawiki.parserfunction.lastedit.2.diff ignored as obsolete

Will the #lastedit parser function use the same limits that #ifexist and {{PAGESINCATEGORY}} use?

Winbots wrote:

Better patch.

Attached a better patch, which, I think, takes care of the expensive functions.

attachment mediawiki.parserfunction.lastedit.3.diff ignored as obsolete

ayg wrote:

(In reply to comment #7)

Will the #lastedit parser function use the same limits that #ifexist and
{{PAGESINCATEGORY}} use?

It will if it's committed.

+ $parser->setFunctionHook( 'lastedit', array( CLASS, 'lastedit' ), SFH_NO_HASH );
...
+ 'lastedit' => array( 0, '#LASTEDIT:' ),

This looks wrong. What's that hash doing here? Is this supposed to be {{lastedit:}} or {{#lastedit:}}? (It should be the former.)

+ $user = User::newFromName( $s );
+ $s = $user->getName();

This will cause a fatal error if the provided name is invalid, e.g., {{lastedit:<>}}.

+ $ts = $db->selectField(
+ 'revision',
+ 'rev_timestamp',
+ array( 'rev_user_text' => $s ),
+ METHOD,
+ array( 'ORDER BY' => 'rev_timestamp DESC', 'LIMIT' => 1 )
+ );

You're still using LIMIT here, it's unnecessary.

I don't think I want to check this in without first asking Brion what our policy should be on this kind of parser function. Perhaps all expensive parser functions should be moved to the ParserFunctions extension, to begin with. Eventually some framework for batching them will be needed, if they keep on multiplying, and it's not clear that constructing such a framework would be the best use of development resources.

Winbots wrote:

Even better patch.

This patch fixes Simetrical's problems listed above, except the first issue, which brion said should stay the same.

Attached:

I suggest if you want to know the last edit timestamp of a given user, you use JavaScript with the API. Saving it into an article using a parser function will mean that it won't update properly, due to the cache.

If you want an online status indicator on user pages, please write it as an extension. That way, it'll have optimal performance, it'll be running on secure, reliable servers, the code will be reviewed, and it can be shared with other MediaWiki sites.