"Catrope" changed the status of MediaWiki.r108187 to "fixme" and commented it.
URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/108187#c29268

Old Status: new
> New Status: fixme

Commit summary for MediaWiki.r108187:

Adding "Show unanswered" filter to feedback dashboard

Catrope's comment:

<pre>
+               $showUnansweredFilter = '<label 
for="fbd-filters-show-unanswered" id="fbd-filters-type-show-unanswered-label" 
class="fbd-filters-label">' . 
+                                                               
$showUnansweredCheckbox . $showUnansweredMsg . '</label>';
</pre>
Why is the checkbox inside the label tag like that, shouldn't it be before it?

<pre>
+                                               $conds['mbfr_user_id'] = 
$wgUser->getId();
</pre>
There is no index on this field at all, please make sure the query is properly 
indexed.

<pre>
+                                       $tableJoin['moodbar_feedback_response'] 
= array( 'LEFT JOIN', 'mbf_id=mbfr_mbf_id' );
+                                       $conds['mbfr_id'] = null; 
</pre>
This isn't a very nice query, it does a full table scan on moodbar_feedback. It 
would be nicer to have a boolean mbf_answered field.

Pro tip: you don't need to specify join conds for INNER JOINs, you can specify 
those implicitly by just adding <code>'mbf_id=mbfr_mbf_id'</code> as a WHERE 
condition.

John's right, the API offers boolean parameters (based on presence/absence). 
You can specify these with <code>'showunanswered' => false,</code> .

_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview

Reply via email to