Report at admin/reports/apachesolr doesn't always display data from the current default solr server [#1631702]

In the implementation of hook_menu, the menu item for the admin/reports/apachesolr looks like this:

$env_id = apachesolr_default_environment();
$items['admin/reports/apachesolr'] = array(
  'title'              => 'Apache Solr search index',
  'description'        => 'Information about the contents of the index the server',
  'page callback'      => 'apachesolr_index_report',
  'page arguments'     => array($env_id),
  'access arguments'   => array('access site reports'),
  'file'               => 'apachesolr.admin.inc',
);

which means that the $env_id is kinda hard-coded to the default solr env at the time menu cache was flushed. If the default env is changed, this page will still display data of the previous default env until the menu cache is flushed again.

And there is no way to display data from the other (non-default) environments.

I think this URL should make use of an argument from the URL. And fall-back to the default env inside the page callback, to make sure that the correct default env is used.

Comment	File	Size	Author
#12	1631702-12.patch	6.74 KB	nick_vh
#11	report_at_admin-1631702-11.patch	5.81 KB	killua99
#9	1631702-9.patch	5.78 KB	nick_vh
#8	1631702-8.patch	4.07 KB	nick_vh

Comments

Comment #1

khaled.zaidan commented 13 June 2012 at 09:05

Here's quick patch to fix this, there might still be room for improvement, but at least here's something that works:

Index: apachesolr.admin.inc
===================================================================
--- apachesolr.admin.inc
+++ apachesolr.admin.inc
@@ -510,7 +510,11 @@
   return $output;
 }
 
-function apachesolr_index_report($env_id) {
+function apachesolr_index_report($env_id = NULL) {
+  if (!$env_id) {
+    $env_id = apachesolr_default_environment();
+  }
+
   try {
     $solr = apachesolr_get_solr($env_id);
     $solr->clearCache();

Index: apachesolr.module
===================================================================
--- apachesolr.module
+++ apachesolr.module
@@ -152,12 +152,11 @@
     'file'               => 'apachesolr.admin.inc',
     'type'               => MENU_CALLBACK,
   );
-  $env_id = apachesolr_default_environment();
   $items['admin/reports/apachesolr'] = array(
     'title'              => 'Apache Solr search index',
     'description'        => 'Information about the contents of the index the server',
     'page callback'      => 'apachesolr_index_report',
-    'page arguments'     => array($env_id),
+    'page arguments'     => array(3),
     'access arguments'   => array('access site reports'),
     'file'               => 'apachesolr.admin.inc',
   );

Comment #2

khaled.zaidan commented 13 June 2012 at 09:13

It might be cool also, in the top of the page, to add a link for each of the currently available environments. That way you can easily view the details of each environment without having to type its machine name in the URL.

Comment #3

nick_vh

he/him

Ghent

commented 13 June 2012 at 09:49

This is indeed something we should improve and we did not expect that the different environments would be picked up so quickly
I'll take a look at you patch during the Drupal Dev Days! Feel free to keep improving your patch over the next coming hours/days.

If you go to the environments page, you can already click on index and it will take you to a page with information about that environment. The basic front page is meant to only show the default one. We should do something similar for the reports I think?

Comment #4

nick_vh

he/him

Ghent

commented 14 June 2012 at 15:03

Category:

bug

» feature

Feature request, let's get to RC1 first

Comment #5

pwolanin commented 14 June 2012 at 16:42

Right, we should fix the hook menu bug - I don't think the approach here is optimal.

Comment #6

khaled.zaidan commented 17 June 2012 at 15:29

Category:

feature

» bug

Hi Nick,

My main concern is the bug in hook_menu. Sure the suggested feature of having a link for each environment is an extra featured, and can definitely wait.

But the main issue itself (having the default environment fused into the menu item definition) is a bug, not a feature request. I think it is important and simple to fix.

(Changing category back to 'bug report')