Use server time rather than client time wherever possible #741

siddharthvp · 2019-10-29T19:12:59Z

Largely addresses #6. Per #6 (comment), the server time is available to us whenever we are in a .load() callback, via the starttimestamp attribute in load API output which is stored in Morebits.wiki.page as ctx.loadTime, publicly exposed via .getLoadTime().

In case of RFD, the curtimestamp parameter is added to the API request while finding the target of the redirect, so that the server time is available while determining the log page.

The only remaining usages of client clock is in:

an3 - not important to fix as it is only used for deciding what revisions to retrieve from which user can select reverts.
in determining daily log page name for CFD, FFD and TFD. The code that does is sitting outside of any load callback, because unlike in AfD, tagging of the page is occurring after the discussion is posted. Maybe the order can be changed in a future Great XfD De-Duplication Project.

siddharthvp · 2019-10-30T17:13:10Z

Note: the starttimestamp parameter in load API output is added by intoken=edit (it adds the token and this timestamp). If we remove that or replace that by meta=tokens in the future, the current timestamp will need to be explicitly requested using the curtimestamp param.

Amorymeltzer · 2019-12-23T22:10:50Z

I've yet to thoroughly review this, but it all looks very reasonable; I'm surprised we've gone so long without wanting getLoadTime!

Note: the starttimestamp parameter in load API output is added by intoken=edit (it adds the token and this timestamp). If we remove that or replace that by meta=tokens in the future, the current timestamp will need to be explicitly requested using the curtimestamp param.

Could you add a comment to this effect? #615 can be our open ticket, but a comment in the code would be nice. I'll try to get to this in the new year, sorry for the lag.

Amorymeltzer

Magnifique! Two requests for updated comments, but otherwise 💯

Amorymeltzer · 2020-01-06T20:55:10Z

modules/twinkleprod.js

@@ -241,8 +241,7 @@ Twinkle.prod.callbacks = {

 		// Alert if article is at least three days old, not in Category:Living people, and BLPPROD is selected
 		if (params.blp) {
-			var now = new Date().toISOString();
-			var timeDiff = (new Date(now) - new Date(params.creation)) / 1000 / 60 / 60 / 24; // days from milliseconds
+			var timeDiff = (new Date(pageobj.getLoadTime()).getTime() - new Date(params.creation).getTime()) / 1000 / 60 / 60 / 24; // days from milliseconds


🎉 Yes, much better!

Amorymeltzer · 2020-01-06T21:01:15Z

modules/twinklexfd.js

@@ -1242,20 +1243,22 @@ Twinkle.xfd.callbacks = {
 		// This is a closure for the callback from the above API request, which gets the target of the redirect
 		findTargetCallback: function(callback) {
 			return function(apiobj) {
-				var xmlDoc = apiobj.responseXML;
-				var target = $(xmlDoc).find('redirects r').first().attr('to');
+				var $xmlDoc = $(apiobj.responseXML);


Just noting that elsewhere in xfd we don't use the jquery naming convention; this is the only one that actually uses it for more than convenience, so it's reasonable to do here.

modules/twinklexfd.js

morebits.js

…t load

Largely addresses wikimedia-gadgets#6. Per wikimedia-gadgets#6 (comment), the server time is available to us whenever we are in a `.load()` callback, via the starttimestamp attribute in load API output which is stored in Morebits.wiki.page as ctx.loadTime, publicly exposed via `.getLoadTime()`.

The curtimestamp parameter is added to the API request while finding the target of the redirect, so that the server time is available while determining the log page.

siddharthvp · 2020-01-07T09:47:49Z

Looks like my git directory on windows has went bust (weird permission denied errors). Will push the latest changes when later when I log into ubuntu.

Amorymeltzer

🏆

1. Since wikimedia-gadgets#741 (specifically, 3e21765), RfD was using a returned `curtimestamp` to generate the log page. That query was only being done if the page wasn't a soft redirect, however, leading to NaN/undefined issues. Closes wikimedia-gadgets#861 by falling back to the previous client clock method. 2. wikimedia-gadgets#527 (44c0e42) added the ability to notify a redirect's target, but returns an error if the title is invalid, such as with soft redirects via {{wi}}. This now skips the notification in such cases.

…tokens Deprecated ages ago (see https://www.mediawiki.org/wiki/MediaWiki_1.24#API_changes and https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/153110/). Closes wikimedia-gadgets#615. Use `curtimestamp` on `.load` calls now that we no longer use `intoken`. As noted in wikimedia-gadgets#741, `starttimestamp` was only available because we were still using the outdated `intoken` method. With that gone, we need to explicitly provide `query.curtimestamp`, thus replicating the loading timestamp that we can feed it into `starttimestamp` and avoid edit conflicts. Also replaced `intoken` in `twinklefluff.js` and removed the undelete kludge from wikimedia-gadgets#616. Some references to token names in error messages are kept for ease of debugging.

1. Since wikimedia-gadgets#741 (specifically, 3e21765), RfD was using a returned `curtimestamp` to generate the log page. That query was only being done if the page wasn't a soft redirect, however, leading to NaN/undefined issues. Closes wikimedia-gadgets#861 by falling back to the previous client clock method. 2. wikimedia-gadgets#527 (44c0e42) added the ability to notify a redirect's target, but returns an error if the title is invalid, such as with soft redirects via {{wi}}. This now skips the notification in such cases.

…tokens Deprecated ages ago (see https://www.mediawiki.org/wiki/MediaWiki_1.24#API_changes and https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/153110/). Closes wikimedia-gadgets#615. Use `curtimestamp` on `.load` calls now that we no longer use `intoken`. As noted in wikimedia-gadgets#741, `starttimestamp` was only available because we were still using the outdated `intoken` method. With that gone, we need to explicitly provide `query.curtimestamp`, thus replicating the loading timestamp that we can feed it into `starttimestamp` and avoid edit conflicts. Also replaced `intoken` in `twinklefluff.js` and removed the undelete kludge from wikimedia-gadgets#616. Some references to token names in error messages are kept for ease of debugging.

siddharthvp force-pushed the noclientclock branch from 7ddf55e to d235e6f Compare November 8, 2019 17:56

Amorymeltzer added this to the January 2020 update milestone Nov 12, 2019

Amorymeltzer added the Module: morebits The morebits.js library label Nov 20, 2019

Amorymeltzer requested changes Jan 6, 2020

View reviewed changes

siddharthvp added 3 commits January 7, 2020 15:04

morebits: wiki.page: add getter function to retrieve timestamp of las…

77242b9

…t load

xfd: avoid client clock for RFD log page determination

3e21765

The curtimestamp parameter is added to the API request while finding the target of the redirect, so that the server time is available while determining the log page.

siddharthvp force-pushed the noclientclock branch from d235e6f to 3e21765 Compare January 7, 2020 10:07

Amorymeltzer added Feature request Module: block Module: prod Module: speedy Module: warn Module: xfd labels Jan 7, 2020

Amorymeltzer approved these changes Jan 7, 2020

View reviewed changes

Amorymeltzer merged commit 48e8ea6 into wikimedia-gadgets:master Jan 7, 2020

Amorymeltzer mentioned this pull request Feb 21, 2020

xfd: Fix two bugs when RfDing soft redirects #862

Merged

Amorymeltzer mentioned this pull request Mar 14, 2020

morebits: Use csrftoken obtained via meta=tokens #888

Merged

Amorymeltzer mentioned this pull request Sep 18, 2020

xfd: Remove final uses of client clock #1132

Merged

siddharthvp deleted the noclientclock branch October 22, 2020 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use server time rather than client time wherever possible #741

Use server time rather than client time wherever possible #741

siddharthvp commented Oct 29, 2019 •

edited

siddharthvp commented Oct 30, 2019

Amorymeltzer commented Dec 23, 2019

Amorymeltzer left a comment

Amorymeltzer Jan 6, 2020

Amorymeltzer Jan 6, 2020

siddharthvp commented Jan 7, 2020

Amorymeltzer left a comment

Use server time rather than client time wherever possible #741

Use server time rather than client time wherever possible #741

Conversation

siddharthvp commented Oct 29, 2019 • edited

siddharthvp commented Oct 30, 2019

Amorymeltzer commented Dec 23, 2019

Amorymeltzer left a comment

Choose a reason for hiding this comment

Amorymeltzer Jan 6, 2020

Choose a reason for hiding this comment

Amorymeltzer Jan 6, 2020

Choose a reason for hiding this comment

siddharthvp commented Jan 7, 2020

Amorymeltzer left a comment

Choose a reason for hiding this comment

siddharthvp commented Oct 29, 2019 •

edited