r89617 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r89616‎ | r89617 | r89618 >
Date:23:46, 6 June 2011
Author:reedy
Status:resolved (Comments)
Tags:
Comment:
* (bug 28695) If there are no results for an abuse filter log, can you put "No matching items in log" or something similar, as we do for usual logs


Think this is right, not verified
Modified paths:
  • /trunk/extensions/AbuseFilter/AbuseFilter.i18n.php (modified) (history)
  • /trunk/extensions/AbuseFilter/special/SpecialAbuseLog.php (modified) (history)

Diff [purge]

Index: trunk/extensions/AbuseFilter/special/SpecialAbuseLog.php
——@@ -203,9 +203,13 @@
204204
205205 $pager = new AbuseLogPager( $this, $conds );
206206
207 - $wgOut->addHTML( $pager->getNavigationBar() .
208 - Xml::tags( 'ul', null, $pager->getBody() ) .
209 - $pager->getNavigationBar() );
 207+ if( $pager->getResult()->numRows() !== 0 ) {
 208+ $wgOut->addHTML( $pager->getNavigationBar() .
 209+ Xml::tags( 'ul', null, $pager->getBody() ) .
 210+ $pager->getNavigationBar() );
 211+ } else {
 212+ $wgOut->addWikiMsg( 'abusefilter-log-noresults' );
 213+ }
210214 }
211215
212216 function showDetails( $id ) {
Index: trunk/extensions/AbuseFilter/AbuseFilter.i18n.php
——@@ -275,7 +275,7 @@
276276 'abusefilter-edit-builder-funcs-lcase' => 'To lower case (lcase)',
277277 'abusefilter-edit-builder-funcs-ccnorm' => 'Normalise confusable characters (ccnorm)',
278278 'abusefilter-edit-builder-funcs-rmdoubles' => 'Remove double-characters (rmdoubles)',
279 - 'abusefilter-edit-builder-funcs-specialratio' => 'Special characters / total characters (specialratio)',
 279+ 'abusefilter-edit-builder-funcs-specabuialratio' => 'Special characters / total characters (specialratio)',
280280 'abusefilter-edit-builder-funcs-norm' => 'Normalise (norm)',
281281 'abusefilter-edit-builder-funcs-count' => 'Number of times string X appears in string Y (count)',
282282 'abusefilter-edit-builder-funcs-rcount' => 'Number of times regex X appears in string Y (rcount)',
——@@ -451,6 +451,7 @@
452452 'abusefilter-log-header' => "This log shows a summary of changes made to filters.
453453 For full details, see [[Special:AbuseFilter/history|the list]] of recent filter changes.",
454454 'abusefilter-log-entry-modify' => 'modified $1 ($2)',
 455+ 'abusefilter-log-noresults' => 'No results',
455456
456457 // Diffs
457458 'abusefilter-diff-title' => 'Differences between versions',

Follow-up revisions

RevisionCommit summaryAuthorDate
r89635Followup r89617, remove spurious letters from different message keyreedy09:09, 7 June 2011
r91428Followup r89617...reedy22:00, 4 July 2011
r91431Followup r89617, r91428 call $pager->doQuery explicitally so we get a set res...reedy00:09, 5 July 2011

Comments

#Comment by Gurch (talk | contribs)   03:37, 7 June 2011

- 'abusefilter-edit-builder-funcs-specialratio' => 'Special characters / total characters (specialratio)', + 'abusefilter-edit-builder-funcs-specabuialratio' => 'Special characters / total characters (specialratio)',


was that intentional?

#Comment by SPQRobin (talk | contribs)   23:56, 3 July 2011

I get: Fatal error: Call to a member function numRows() on a non-object in C:\wamp\www\mw\extensions\AbuseFilter\special\SpecialAbuseLog.php on line 206

#Comment by Reedy (talk | contribs)   22:00, 4 July 2011

Can you give r91428 a try and see if it fixes it for you? Thanks!

#Comment by SPQRobin (talk | contribs)   23:52, 4 July 2011

Hmm, now it gives "no results" even if there are hits.

#Comment by Reedy (talk | contribs)   00:04, 5 July 2011

Pager Sucks. 

	function getBody() {
		if ( !$this->mQueryDone ) {
			$this->doQuery();
		}
		# Don't use any extra rows returned by the query
		$numRows = min( $this->mResult->numRows(), $this->mLimit );


Can you try 

		$pager = new AbuseLogPager( $this, $conds );
		$result = $pager->getResult();
		if( $result && $result->numRows() !== 0 ) {

swapped with 

		$pager = new AbuseLogPager( $this, $conds );
		$pager->doQuery();
		$result = $pager->getResult();
		if( $result && $result->numRows() !== 0 ) {

If that works, I'll explicitly mark the doQuery method in Pager as public

#Comment by SPQRobin (talk | contribs)   00:07, 5 July 2011

Yeah that works :)

#Comment by SPQRobin (talk | contribs)   00:05, 5 July 2011

It appears IndexPager->getResult() and IndexPager->mResult are always empty. Strange..

We might use if( $pager->getBody() )

#Comment by Reedy (talk | contribs)   00:07, 5 July 2011

It needs the hit on $pager->doQuery() to do 

		$this->mResult = $this->reallyDoQuery(
			$this->mOffset,
			$queryLimit,
			$descending
		);

So that's when it gets defined, and hence, and then will work. See my comment above =)

Status & tagging log