-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
robchurch@svn.wikimedia.org wrote:
Move all watchlist editing into WatchlistEditor class, integrating "raw editing" from ExportWatchlist extension, per Brion. Spiffed up the existing editor and "clear" interface; all three editors better integrated from a UI perspective.
Looks good!
$titles = $this->extractTitles( $request->getText( 'titles' ) );$this->clearWatchlist( $user );$this->watchTitles( $titles, $user );$user->invalidateCache();
This raw edit save will reset the last-seen timestamps for all watched pages, which could hurt a bit for people using email notification.
Doing an array diff and only adding/removing the changed items should work cleaner for that case, plus save some DB activity when making small changes to a large watchlist.
[snip]
$rows[] = array('wl_user' => $user->getId(),'wl_namespace' => ( $title->getNamespace() & ~1 ),'wl_title' => $title->getDBkey(),'wl_notificationtimestamp' => null,);$rows[] = array('wl_user' => $user->getId(),'wl_namespace' => ( $title->getNamespace() | 1 ),'wl_title' => $title->getDBkey(),'wl_notificationtimestamp' => null,);
This low-level code makes some surprises in corner cases. :)
If you pass a Special: or Media: title, it accepts them and treats them as an article-talk pair. Interwiki titles are also accepted, treated as normal pages.
Probably these unwatchable titles should get stripped out of the stream, either here or at the high-level UI end.
I'd also recommend moving the low-level DB routines over to WatchedItem, which IIRC already has some batch-change functions.
}- }
- /**
* Show the standard watchlist editing form** @param OutputPage $output* @param User $user*/- private function showNormalForm( $output, $user ) {
if( ( $count = $this->showItemCount( $output, $user ) ) > 0 ) {$self = SpecialPage::getTitleFor( 'Watchlist' );$form = Xml::openElement( 'form', array( 'method' => 'post','action' => $self->getLocalUrl( 'action=edit' ) ) );$form .= Xml::hidden( 'token', $user->editToken( 'watchlistedit' ) );$form .= '<fieldset><legend>' . wfMsgHtml( 'watchlistedit-normal-legend' ) . '</legend>';$form .= wfMsgExt( 'watchlistedit-normal-explain', 'parse' );foreach( $this->getWatchlist( $user ) as $namespace => $pages ) {$form .= '<h2>' . $this->getNamespaceHeading( $namespace ) . '</h2>';$form .= '<ul>';foreach( $pages as $dbkey => $redirect ) {$title = Title::makeTitleSafe( $namespace, $dbkey );$form .= $this->buildRemoveLine( $title, $redirect, $user->getSkin() );
The makeTitleSafe() reminds me of the possibility of invalid entries in the watchlist table; those may not roundtrip nicely through the raw edit form. Just something to consider... automatic cleanup (for instance when namespaces change in weird ways) might be appropriate.
+'watchlistedit-normal-explain' => 'Titles on your watchlist are shown below. To remove a title, check
- the box next to it, and click Remove Titles. You can also [[Special:Watchlist/raw|edit the raw list]],
- or [[Special:Watchlist/clear|remove all titles]].',
The mode links here feel buried in the UI text to me; it's not obvious to me that it's there. It might be interesting to try a more tab-like link here, maybe just simple links like on Special:Allmessages
- -- brion vibber (brion @ wikimedia.org)