Page MenuHomePhabricator

Update API query continuation usage in Gather for future change of default
Closed, ResolvedPublic

Description

In the not too distant future, the default for continuation for API action=query will be changing from the current "raw" format to a more user-friendly format, see here for the most recent announcement of this planned change.

This extension appears to be using query-continue, and so will probably be affected by this change. If you wish to continue using the "raw" format, specify the 'rawcontinue' parameter in your API queries. To be ready for the new format (documented here), specify the 'continue' parameter with the empty string as a value in the initial query.

Event Timeline

Anomie created this task.Apr 22 2015, 3:06 PM
Anomie updated the task description. (Show Details)
Anomie raised the priority of this task from to Needs Triage.
Anomie added projects: MediaWiki-API, Gather.
Anomie moved this task to Non-core-API stuff on the MediaWiki-API board.
Anomie added a subscriber: Anomie.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 22 2015, 3:06 PM
Anomie triaged this task as High priority.Jun 2 2015, 8:42 PM

I've set the date for the breaking change: the plan is to merge this for the end of June, which if I counted correctly will be 1.26wmf12.

Legoktm renamed this task from Update API query continuation usage for future change of default to Update API query continuation usage in Gather for future change of default.Jun 22 2015, 9:56 PM
Legoktm set Security to None.
Jhernandez added a subscriber: Jhernandez.

Pulled to the sprint, we'll prioritize it soon.

Ping @phuedx @Jdlrobson ^

Are we good on this? I recall seeing continue at least on the ui php code? What about the api stuff?

phuedx added a comment.EditedJun 24 2015, 1:17 PM

There are two instances of 'query-continue' in the Gather codebase: in the Collection model and the CollectionsApi class. In both cases, the query-continue section of the API result is returned to the client. However, in the former case, it's done immediately after handling the continue section of the API result, so we could simply remove the first instance.

Hi @phuedx, @Jhernandez, I was hoping to work on this as a way of learning more about the API. I'll submit a patch based on what @phuedx said above if that's okay. :)
Thanks.

@NiharikaKohli: Sure. I think that this is now high priority for the Readership Web team, given that the change is coming soon. Do you have any idea when you'll be able to work on it?

@phuedx, I could do this today. To be clear, the second instance of 'query-continue' in CollectionApi.js should stay in place and only the first one to be removed?
Also, how can I test it once I've made the necessary changes?

Tgr added a subscriber: Tgr.Jun 24 2015, 2:46 PM

Needs to be fixed this week.

There is at least one call, I forgot where exactly, that needs to use the old continuation.

phuedx added a comment.EditedJun 24 2015, 2:46 PM

To be clear, the second instance of 'query-continue' in CollectionApi.js should stay in place and only the first one to be removed?

Nope. They'll both have to be changed. The first case is much simpler than the second (I think).

The broadest way to test your changes will be to create a handful of large collections and then prove that the pagination and infinite scrolling still works as expected. @Jhernandez: Is this correct?

Tgr added a comment.Jun 24 2015, 3:34 PM
In T96864#1396471, @Tgr wrote:

There is at least one call, I forgot where exactly, that needs to use the old continuation.

I thought I saw some code a props API was called with a generator and you had to make sure to iterate the generator results but not the props results, to make sure you don't get the same page multiple times. (That's only possible with the old continuation syntax.) I can't find it now though so I am probably confusing things.

Awesome, thanks @NiharikaKohli.

Regarding https://github.com/wikimedia/mediawiki-extensions-Gather/blob/master/includes/models/Collection.php#L269 we can remove it or set the rawcontinue I think to get the old behavior supported. In this case I think removing it is just fine, since it is a young extension and I don't think there are urls on the interwebs linking to query-continue type params.

In the case of https://github.com/wikimedia/mediawiki-extensions-Gather/blob/master/resources/ext.gather.api/CollectionsApi.js#L197 it should just be update it to the new format.

For testing it install the Extension:Gather and dependencies. For seeing and creating collections go to the mobile version of the wiki (link at the bottom of the page Mobile view) and click on the top left menu icon, go to Settings and activate Beta.

After that you should see a Collections menu item where you will be able to create new collections. Also if you visit an article while in beta you should see a call to action for adding it to a collection and clicking on the star should prompt you with the add-to-collection modal.

For testing pagination in the case of Collection.php it is the pages of a collection. You should create a collection and add a bunch of items to it, the limit is 50 per page I think. (personally I find the $limit and lower it to 2 and test 2 with item pages).

For testing pagination in the case of CollectionsApi.js it is the list of collections of a user (http://en.m.wikipedia.beta.wmflabs.org/wiki/Special:Gather/by/Jhernandez) or the active ones (http://en.m.wikipedia.beta.wmflabs.org/wiki/Special:Gather/all/recent). You should create a bunch of collections and add a bunch 4 items or more to each. (personally here I'd find the $limit in php and the limit sent in JS and lower it to 2 and test 2 collections pages).

If it is too confusing please reach back and we'll figure a way to communicate more!

Awesome, thanks @NiharikaKohli.

Regarding https://github.com/wikimedia/mediawiki-extensions-Gather/blob/master/includes/models/Collection.php#L269 we can remove it or set the rawcontinue I think to get the old behavior supported. In this case I think removing it is just fine, since it is a young extension and I don't think there are urls on the interwebs linking to query-continue type params.

In the case of https://github.com/wikimedia/mediawiki-extensions-Gather/blob/master/resources/ext.gather.api/CollectionsApi.js#L197 it should just be update it to the new format.

For testing it install the Extension:Gather and dependencies. For seeing and creating collections go to the mobile version of the wiki (link at the bottom of the page Mobile view) and click on the top left menu icon, go to Settings and activate Beta.

After that you should see a Collections menu item where you will be able to create new collections. Also if you visit an article while in beta you should see a call to action for adding it to a collection and clicking on the star should prompt you with the add-to-collection modal.

For testing pagination in the case of Collection.php it is the pages of a collection. You should create a collection and add a bunch of items to it, the limit is 50 per page I think. (personally I find the $limit and lower it to 2 and test 2 with item pages).

For testing pagination in the case of CollectionsApi.js it is the list of collections of a user (http://en.m.wikipedia.beta.wmflabs.org/wiki/Special:Gather/by/Jhernandez) or the active ones (http://en.m.wikipedia.beta.wmflabs.org/wiki/Special:Gather/all/recent). You should create a bunch of collections and add a bunch 4 items or more to each. (personally here I'd find the $limit in php and the limit sent in JS and lower it to 2 and test 2 collections pages).

If it is too confusing please reach back and we'll figure a way to communicate more!

Thanks @Jhernandez! Very helpful. I'll try to get a patch in within 24 hours.

No continuation seems to be broken in Gather on latest mediawiki master... we use query-continue in two places, in one of which we test for continue first. It might just be a case of cleaning that up.

Jdlrobson assigned this task to Niharika.

I've assigned you! Good luck @NiharikaKohli :)

Change 220749 had a related patch set uploaded (by Niharika29):
[WIP] Fix query-continue in Gather

https://gerrit.wikimedia.org/r/220749

@Jdlrobson, @phuedx, I submitted a patch. Pagination for collections list isn't working. The moment I try to scroll, all collections appear at once. I saw something about "infinite scrolling" above. Does the collections list exhibit infinite scrolling?

@NiharikaKohli: Yes. If you've got JavaScript enabled, then you won't see the collections pagination buttons. Instead you'll see more collections load as you scroll. Simply disable JavaScript to stop this from happening.

@phuedx, In that case the patch works as expected. I'll remove the WIP. Up for review.

Change 220749 merged by jenkins-bot:
Fix query-continue in Gather

https://gerrit.wikimedia.org/r/220749

Verifed as part of T103974.

phuedx closed this task as Resolved.Jun 26 2015, 7:09 PM
phuedx moved this task from Ready for Signoff to Done on the Reading-Web-Sprint-50-The-X-Files board.

Obviously we will keep an eye on this next week…

Change 222196 had a related patch set uploaded (by Jdlrobson):
Allow user to add to more than 50 collections from page

https://gerrit.wikimedia.org/r/222196

Change 222196 merged by jenkins-bot:
Allow user to add to more than 50 collections from page

https://gerrit.wikimedia.org/r/222196