Page MenuHomePhabricator

Figure out if the Echo API with format=html is being used
Closed, ResolvedPublic

Description

A change[1] to Special:Notifications to add a "mark as read" button breaks the API with notformat=html, which was deprecated on 2016-02-04[2].

Figure out if it is being used at all or if we can remove it for good.

[1] https://gerrit.wikimedia.org/r/#/c/276256
[2] https://gerrit.wikimedia.org/r/#/c/264718

Event Timeline

SBisson created this task.Mar 22 2016, 8:16 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 22 2016, 8:16 PM
Legoktm updated the task description. (Show Details)Mar 22 2016, 9:20 PM

It seems to be used still:

grep "notformat=html" /a/mw-log/api.log reveals very few requests, but there's usually a whole bunch of requests at once.
I've found these 2 types of requests:

1/ A whole bunch of requests to a load of different wikis in just a couple of seconds of time. All of the requests being:

uselang=uk action=query format=json meta=notifications notprop=list notformat=html notlimit=15 notalertunreadfirst= notmessagecontinue=

I'm fairly certain that this kind comes from crosswatch, which still seems to use notformat=html:
https://git.wikimedia.org/blob/labs%2Ftools%2Fcrosswatch.git/master/backend%2Fcelery%2Ftasks.py#L192

2/ A bunch of consecutive requests to the same wiki (frwiki, but may have been a coincidence in my sample), with the same params, all of them being spread out more (e.g. 11:57:43, 11:57:54, 11:58:04, 12:04:04, ...)
All of those requests being:

action=query format=xml meta=notifications notformat=html

I have no idea where those come from.

Since the param has been deprecated for awhile now, I would plan to kill it soon.
Let's notify crosswatch & plan to get a patch removing support for notformat=html merged soon.

FYI, there already is a patch to remove these: https://gerrit.wikimedia.org/r/#/c/263789/
Needs to be rebased though.

matthiasmullie added a comment.EditedMar 30 2016, 11:46 AM

Also note that MobileFrontend is still using &notformat=flyout, which is also deprecated.

Strike that, I was looking at old code.

Let's notify crosswatch & plan to get a patch removing support for notformat=html merged soon.

Did anyone notify them?

Mattflaschen-WMF added a comment.EditedApr 5 2016, 1:21 AM

Let's notify crosswatch & plan to get a patch removing support for notformat=html merged soon.

https://gerrit.wikimedia.org/r/#/c/276256/ also breaks 'special'.

Only 'html' is deprecated, not 'special'. 'special' is basically exactly the same thing AFAICT just with a less-super-generic name, and the current code explicitly tells people to use it instead of 'html'.

We also need to deprecate 'special', right? Do we also want to deprecate 'special'? If so, we should do that and provide a deprecation period if we think it's being used.

I'll grep through the API log overnight (it's 100 GB):

grep "notformat=special" /a/mw-log/api.log

In the meantime, it's hacky, but you could enableOOUI from the API in the 'special' path.

format=html was used to feed the frontend the same HTML to render "read more" notifications in Special:Notifications.
format=special now serves the same purpose, but with PresentationModel instead of the old formatters (and format=html is now a shim for this until we remove it)

Unless/until we duplicate Special:Notifications HTML rendering code on the frontend, we'll have to keep this around.
But API params i18n now explicitly says this HTML may change at any point: https://phabricator.wikimedia.org/diffusion/ECHO/browse/master/i18n/en.json;0518d8e15f83d4ce0a2c6dca7495269d2c77ff09$228

In T130661#2179567, @Mattflaschen wrote:

Let's notify crosswatch & plan to get a patch removing support for notformat=html merged soon.

Did anyone notify them?

I don't think so. I didn't :p
Just filed T131829: Stop using notformat=html with notifications API

format=html was used to feed the frontend the same HTML to render "read more" notifications in Special:Notifications.
format=special now serves the same purpose, but with PresentationModel instead of the old formatters (and format=html is now a shim for this until we remove it)
Unless/until we duplicate Special:Notifications HTML rendering code on the frontend, we'll have to keep this around.

We're going to do T129169: Change current no-JS Special:Notifications to use core Pager (no-JS) (which will remove the simple AJAX behavior (I feel like I saw a patch removing that JS around already, but I can't find it)) and T129363: Pagination for the Notification Page (JS). I don't know if the second will use 'special' or use the existing JS rendering path used by the flyout.

In the meantime, we can not break 'special' (my grep found 31 uses, all of which seem to be from the special page, BTW).

SBisson closed this task as Resolved.May 16 2016, 2:26 PM
SBisson claimed this task.

We've found that format=html is indeed used according to the logs.

The patch that was threatening to break it was fixed.