r81619 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r81618‎ | r81619 | r81620 >
Date:02:35, 7 February 2011
Author:brion
Status:ok (Comments)
Tags:
Comment:
* (bug 18011) Special:FileDuplicateSearch UI fixes and remote file repo support

Fixes:
- now accepts file titles with or without File: prefix instead of demanding you must remove it
- moved result summary line from bottom to top
- added a line telling you no file was found if it wasn't found
- now pulls the reference file's SHA-1 via FileRepo, so can give it a remote file (eg from Commons)
- now pulls duplicate files via the main RepoGroup instead of querying image table manually, so turns up remote duplicates
- dropped the QueryPage standard paging header/footer; file dupe lists are usually very short and it's not worth copying the infrastructure

To make this work, I switched the special page class from using the standard QueryPage paths to doing the query and list itself; QueryPage is currently very tightly tied in to database queries, and doesn't provide a very clean way to drop in an alternative way of looking stuff up (say an API query or just getting a big array you've gotten from somewhere). If that gets improved, this page should be cleaned up to use more of the QueryPage infrastructure again so it's pretty and doesn't scare people coming in to maintain it.

Localization changes:
- added fileduplicatesearch-noresults message
- changed fileduplicatesearch-summary in English master to remove the supplementary line about taking out the 'File:' prefix
Modified paths:
  • /trunk/phase3/includes/specials/SpecialFileDuplicateSearch.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialFileDuplicateSearch.php
@@ -29,8 +29,13 @@
3030 * @ingroup SpecialPage
3131 */
3232 class FileDuplicateSearchPage extends QueryPage {
33 - protected $hash, $filename;
 33+ protected $hash = '', $filename = '';
3434
 35+ /**
 36+ * @var File $file selected reference file, if present
 37+ */
 38+ protected $file = null;
 39+
3540 function __construct( $name = 'FileDuplicateSearch' ) {
3641 parent::__construct( $name );
3742 }
@@ -43,6 +48,35 @@
4449 return array( 'filename' => $this->filename );
4550 }
4651
 52+ /**
 53+ * Fetch dupes from all connected file repositories.
 54+ *
 55+ * @return Array of File objects
 56+ */
 57+ function getDupes() {
 58+ return RepoGroup::singleton()->findBySha1( $this->hash );
 59+ }
 60+
 61+ /**
 62+ *
 63+ * @param Array of File objects $dupes
 64+ */
 65+ function showList( $dupes ) {
 66+ global $wgUser, $wgOut;
 67+ $skin = $wgUser->getSkin();
 68+
 69+ $html = array();
 70+ $html[] = $this->openList( 0 );
 71+
 72+ foreach ( $dupes as $key => $dupe ) {
 73+ $line = $this->formatResult( $skin, $dupe );
 74+ $html[] = "<li>" . $line . "</li>";
 75+ }
 76+ $html[] = $this->closeList();
 77+
 78+ $wgOut->addHtml( implode( "\n", $html ) );
 79+ }
 80+
4781 function getQueryInfo() {
4882 return array(
4983 'tables' => array( 'image' ),
@@ -63,11 +97,11 @@
6498 $this->outputHeader();
6599
66100 $this->filename = isset( $par ) ? $par : $wgRequest->getText( 'filename' );
 101+ $this->file = null;
67102 $this->hash = '';
68 - $title = Title::makeTitleSafe( NS_FILE, $this->filename );
 103+ $title = Title::newFromText( $this->filename, NS_FILE );
69104 if( $title && $title->getText() != '' ) {
70 - $dbr = wfGetDB( DB_SLAVE );
71 - $this->hash = $dbr->selectField( 'image', 'img_sha1', array( 'img_name' => $title->getDBkey() ), __METHOD__ );
 105+ $this->file = wfFindFile( $title );
72106 }
73107
74108 # Create the input form
@@ -82,11 +116,20 @@
83117 Xml::closeElement( 'form' )
84118 );
85119
 120+ if( $this->file ) {
 121+ $this->hash = $this->file->getSha1();
 122+ } else {
 123+ $wgOut->wrapWikiMsg(
 124+ "<p class='mw-fileduplicatesearch-noresults'>\n$1\n</p>",
 125+ array( 'fileduplicatesearch-noresults', wfEscapeWikiText( $this->filename ) )
 126+ );
 127+ }
 128+
86129 if( $this->hash != '' ) {
87130 $align = $wgContLang->alignEnd();
88131
89132 # Show a thumbnail of the file
90 - $img = wfFindFile( $title );
 133+ $img = $this->file;
91134 if ( $img ) {
92135 $thumb = $img->transform( array( 'width' => 120, 'height' => 120 ) );
93136 if( $thumb ) {
@@ -102,36 +145,46 @@
103146 }
104147 }
105148
106 - parent::execute( $par );
 149+ $dupes = $this->getDupes();
 150+ $numRows = count( $dupes );
107151
108152 # Show a short summary
109 - if( $this->numRows == 1 ) {
 153+ if( $numRows == 1 ) {
110154 $wgOut->wrapWikiMsg(
111155 "<p class='mw-fileduplicatesearch-result-1'>\n$1\n</p>",
112 - array( 'fileduplicatesearch-result-1', $this->filename )
 156+ array( 'fileduplicatesearch-result-1', wfEscapeWikiText( $this->filename ) )
113157 );
114 - } elseif ( $this->numRows > 1 ) {
 158+ } elseif ( $numRows ) {
115159 $wgOut->wrapWikiMsg(
116160 "<p class='mw-fileduplicatesearch-result-n'>\n$1\n</p>",
117 - array( 'fileduplicatesearch-result-n', $this->filename,
118 - $wgLang->formatNum( $this->numRows - 1 ) )
 161+ array( 'fileduplicatesearch-result-n', wfEscapeWikiText( $this->filename ),
 162+ $wgLang->formatNum( $numRows - 1 ) )
119163 );
120164 }
 165+
 166+ $this->showList( $dupes );
121167 }
122168 }
123169
 170+ /**
 171+ *
 172+ * @param Skin $skin
 173+ * @param File $result
 174+ * @return string
 175+ */
124176 function formatResult( $skin, $result ) {
125177 global $wgContLang, $wgLang;
126178
127 - $nt = Title::makeTitle( NS_FILE, $result->title );
 179+ $nt = $result->getTitle();
128180 $text = $wgContLang->convert( $nt->getText() );
129181 $plink = $skin->link(
130182 Title::newFromText( $nt->getPrefixedText() ),
131183 $text
132184 );
133185
134 - $user = $skin->link( Title::makeTitle( NS_USER, $result->img_user_text ), $result->img_user_text );
135 - $time = $wgLang->timeanddate( $result->img_timestamp );
 186+ $userText = $result->getUser( 'text' );
 187+ $user = $skin->link( Title::makeTitle( NS_USER, $userText ), $userText );
 188+ $time = $wgLang->timeanddate( $result->getTimestamp() );
136189
137190 return "$plink . . $user . . $time";
138191 }
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -4318,15 +4318,14 @@
43194319
43204320 # Special:FileDuplicateSearch
43214321 'fileduplicatesearch' => 'Search for duplicate files',
4322 -'fileduplicatesearch-summary' => 'Search for duplicate files based on hash values.
4323 -
4324 -Enter the filename without the "{{ns:file}}:" prefix.',
 4322+'fileduplicatesearch-summary' => 'Search for duplicate files based on hash values.',
43254323 'fileduplicatesearch-legend' => 'Search for a duplicate',
43264324 'fileduplicatesearch-filename' => 'Filename:',
43274325 'fileduplicatesearch-submit' => 'Search',
43284326 'fileduplicatesearch-info' => '$1 × $2 pixel<br />File size: $3<br />MIME type: $4',
43294327 'fileduplicatesearch-result-1' => 'The file "$1" has no identical duplication.',
43304328 'fileduplicatesearch-result-n' => 'The file "$1" has {{PLURAL:$2|1 identical duplication|$2 identical duplications}}.',
 4329+'fileduplicatesearch-noresults' => 'No file named "$1" found.',
43314330
43324331 # Special:SpecialPages
43334332 'specialpages' => 'Special pages',

Follow-up revisions

RevisionCommit summaryAuthorDate
r81621Follow-up r81619: Add new message key to maintenance fileraymond07:32, 7 February 2011

Comments

#Comment by Bryan (talk | contribs)   15:40, 7 February 2011

Niiice

QueryPage is one of those horrible interfaces that mixes presentation with querying. (Pager is another one)

#Comment by Duplicatebug (talk | contribs)   21:18, 11 February 2011

Let the api (prop=duplicatefiles) also find dupes from repos. Thanks.

#Comment by Duplicatebug (talk | contribs)   19:33, 19 February 2011

Status & tagging log