morebits: api improvements #1038
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
All action=query requests can be done using GET protocol.
…in JSON As JSON support is fairly recent, there are unlikely to be any existing clients. This is the best time we can make this change. All new API usages in JSON should anyway be done with formatversion=2 (phab:T178734 et al)
Amorymeltzer
requested changes
Jul 12, 2020
| @@ -1774,7 +1777,7 @@ Morebits.wiki.api.prototype = { | |||
|
|
|||
| var ajaxparams = $.extend({}, { | |||
| context: this, | |||
| type: 'POST', | |||
| type: this.query.action === 'query' ? 'GET' : 'POST', | |||
| @@ -1721,6 +1721,8 @@ Morebits.wiki.api = function(currentAction, query, onSuccess, statusElement, onE | |||
| } | |||
| if (!query.format) { | |||
| this.query.format = 'xml'; | |||
| } else if (query.format === 'json' && !query.formatversion) { | |||
| this.query.formatversion = '2'; | |||
There was a problem hiding this comment.
Good point about this being a good time. I'm already using it locally, so 👍
morebits.js
Outdated
| @@ -1738,6 +1740,7 @@ Morebits.wiki.api.prototype = { | |||
| statusText: null, // result received from the API, normally "success" or "error" | |||
| errorCode: null, // short text error code, if any, as documented in the MediaWiki API | |||
| errorText: null, // full error description, if any | |||
| tokenRetry: false, | |||
There was a problem hiding this comment.
Looks like you renamed this to badtokenRetry below (or vice-versa)
There was a problem hiding this comment.
Indeed. So what messed me up here was the later realisation that all actions don't use a CSRF token. If we want to be fussy about it, we could do something like what I do in my bot framework (querying the type of token needed as well) but IMO not really necessary.
Easier to pretend that everything is CSRF-based (badtoken error on anything which isn't will be failing as before).
There was a problem hiding this comment.
Indeed, that's roughly where I ended up thinking. It's few and far between atm, so nbd.
Some methods of mw.Api does this as well. Automatically retry (at most once) on badtoken errors for all API calls. With this, there is no need to separately handle badtoken errors in save(), deletePage() and undeletePage()
Action completed status messages are just like all other status lines at the moment. They ought to be emphasised.
Amorymeltzer
added a commit
to Amorymeltzer/twinkle
that referenced
this pull request
Oct 5, 2020
Stems from af13dce/wikimedia-gadgets#1038. Looks like, since `Morebits.wiki.api.getToken` explicitly creates a new construct, `this` in `returnError`'s return ends up going global (`window`). Reported at WT:TW (https://en.wikipedia.org/w/index.php?oldid=982033312#%22Getting_token%22_and_what?)
This was referenced Oct 5, 2020
Closed
Amorymeltzer
added a commit
to Amorymeltzer/twinkle
that referenced
this pull request
Oct 6, 2020
Stems from af13dce/wikimedia-gadgets#1038. Looks like, since `Morebits.wiki.api.getToken` explicitly creates a new construct, `this` in `returnError`'s return ends up going global (`window`). Reported at WT:TW (https://en.wikipedia.org/w/index.php?oldid=982033312#%22Getting_token%22_and_what?)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#1029 seems rather far-fetched for now, so it might still be worth making some improvements to Morebits.wiki.api.