Remove custom ordering from ReadingLists extension
Closed, ResolvedPublic

Description

Custom sort was problematic, especially with the current performance restrictions, and not really necessary as apps can just sort on the client side. That means it is not possible to sync sort settings or custom sort orders between devices, which has been deemed acceptable.

  • remove sort tables from the database
  • remove readinglistorder MW API module
  • remove /lists/order and /lists/{id}/order RESTBase endpoints
  • add a sort query parameter to /lists/ for sorting non-deleted lists by list name (ascending) vs. sorting by last changed date (descending)
  • add a similar parameter to /lists/{id}/entries/ (name/date, name sorts by title only)
  • add corresponding sort parameters to the readinglists and readinglistentries endpoint
  • make sure DB schema supports performant sorting
Tgr created this task.Nov 13 2017, 8:25 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Tgr updated the task description. (Show Details)Nov 13 2017, 8:29 PM

Some answers…

No reverse sorting is necessary

Date sorting should be descending and name sorting should be ascending

Both lists and entries should have sorting by title and date

Use the page title for sorting page entries (ignore the project)

Tgr updated the task description. (Show Details)Nov 14 2017, 3:23 AM

@Pchelolo @mobrovac is there a convention for sort parameters in RESTBase? Would it look like /lists/?sort=name or /lists/byname/ or /lists/sortby/name/? From an implementation point of view the first seems like the least hassle (no need to create extra spec nodes) and seems like the most RESTish too (since different orderings are still representations of the same resource).

@Tgr we've never had the need for sorting so we've never settled on a convention. I personally like the /lists/?sort=name more then others.

@Tgr we've never had the need for sorting so we've never settled on a convention. I personally like the /lists/?sort=name more then others.

+1 on this from me too. Sorting does not represent a new resource.

Change 391736 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/ReadingLists@master] Remove custom ordering

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

Tgr updated the task description. (Show Details)Nov 16 2017, 12:29 AM

Change 391736 merged by jenkins-bot:
[mediawiki/extensions/ReadingLists@master] Remove custom ordering

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

Tgr added a comment.Nov 28 2017, 2:52 AM

@Dbrant @JoeWalsh the current implementation just stores titles / names as UTF-8 byte strings (which is how MediaWiki stores text in general), which means there is no way to do human-friendly sorting (e.g. if you have titles foo1, Foo2 and foo3, those will be sorted as Foo2, foo1, foo3). Do you think that's a problem? Proper collation is a complex business (and depends on the language to some extent, and we can't let the database handle it since, unlike normal MediaWiki tables, it is not sharded by user language) but case-insensitivity in itself does not seem that hard (but would require using some DB options which we haven't used so far on the cluster). Do you think that is important?

(Also, there is no normalization of titles to uppercase-first at the moment since not all wikis do that (specifically, wiktionaries don't) and I did not make any assumptions about which projects list entries can refer to.)

Change 393925 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/ReadingLists@master] [WIP] Sort lists and entries by name and last updated timestamp

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

Change 393925 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/ReadingLists@master] Sort lists and entries by name and last updated timestamp

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

Change 393925 merged by jenkins-bot:
[mediawiki/extensions/ReadingLists@master] Sort lists and entries by name and last updated timestamp

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

Tgr closed this task as Resolved.Dec 5 2017, 10:56 PM
Tgr claimed this task.

Change 396108 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/ReadingLists@wmf/1.31.0-wmf.10] Sort lists and entries by name and last updated timestamp

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

Change 396116 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/ReadingLists@wmf/1.31.0-wmf.11] Sort lists and entries by name and last updated timestamp

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

Change 396108 merged by jenkins-bot:
[mediawiki/extensions/ReadingLists@wmf/1.31.0-wmf.10] Sort lists and entries by name and last updated timestamp

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

Change 396116 merged by jenkins-bot:
[mediawiki/extensions/ReadingLists@wmf/1.31.0-wmf.11] Sort lists and entries by name and last updated timestamp

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