On empty search, show enabled filters to start a search

janusman - May 8, 2009 - 21:25
Project:Apache Solr Search Integration
Version:6.x-2.x-dev
Component:User interface
Category:feature request
Priority:normal
Assigned:Unassigned
Status:closed
Description

This is similar to what the faceted_module does.

This needs #358166: Search for just facet(s) to be solved.

As a proof of concept, I built a patch that extends http://drupal.org/node/358166#comment-1565308 to show facets and their values below the search box from /search/apachesolr_search

See attached screenshot and patch.

AttachmentSize
apachesolr_browseby_test.png81.86 KB
apachesolr_browseby_test.patch7.14 KB

#1

xarbot - May 11, 2009 - 10:55

Sorry, but i have an error while i patch

patching file apachesolr_search.module
Hunk #1 succeeded at 130 (offset 77 lines).
Hunk #2 FAILED at 317.
1 out of 2 hunks FAILED -- saving rejects to file apachesolr_search.module.rej

and then in my website

Fatal error: Cannot redeclare apachesolr_search_menu_alter()

Thanks

Xarbot

#2

janusman - May 11, 2009 - 13:57

@xarbot: I re-checked and the patch applies to a checkout of the latest DRUPAL-6--1 branch... you possibly need to check http://drupal.org/node/204268/cvs-instructions/DRUPAL-6--1 to get the latest code =)

#3

janusman - May 26, 2009 - 14:24

New patch; needs you to apply this patch first: http://drupal.org/node/358166#comment-1627524

AttachmentSize
browse_on_empty_search.patch 3.64 KB

#4

David Lesieur - May 26, 2009 - 17:38

Some suggestions:

  • Use available functions such as apachesolr_search_add_facet_params() to reduce duplication in apachesolr_search_browse().
  • Move the code that wraps and displays the blocks to a theme function.

#5

David Lesieur - May 26, 2009 - 18:41
Status:needs review» needs work

#6

janusman - May 26, 2009 - 21:09
Status:needs work» needs review

Thanks for the input. Here's a new patch. Again, it's dependant on applying this patch first: http://drupal.org/node/358166#comment-1627524

AttachmentSize
457826_6.patch 2.87 KB

#7

Nick_vh - May 28, 2009 - 13:25

The blocks show on my result page instead of the place where I want the blocks.
Isn't it possible to show the blocks at the place they were assigned to?

#8

janusman - May 28, 2009 - 14:23

Weird. Are you using the lates DEV?

Here's my testing site where you can see it yourself. This is the starting "search" page, showing the blocks below the search box:
http://prod68ws.ruv.itesm.mx/dev/6/search/apachesolr_search

#9

Nick_vh - May 28, 2009 - 15:39

Yes the blocks are showing but not on the region I put them on. As seen in your code, the filters always show up on the search form but I want them to show up like the Apache Solr Blocks.

I'll post a screenshot when I'm back!

#10

heacu - May 31, 2009 - 00:07

as regards #7-#9, yes, indeed, this is an issue. below is perhaps a better solution?

inside apachesolr_search_browse(), i comment out the bit which displays the blocks, and then fail to call apachesolr_has_searched(FALSE). this way, your facet blocks will be displayed where you want them instead of below the search form.

the only other catch is that if your facet blocks are set to display only on pages such as search/apachesolr_search/*, they won't show up anymore unless you change this, for example, to search/apachesolr*. apparently even a URL like search/apachesolr_search/?filters=type%3Aperson doesn't match search/apachesolr_search/*

if you use the current search block then when you hit the search/apachesolr_search for the first time that block will say "Current Search N found" with N=the number of documents in your index. users don't conceive of this as a search (but rather a Browse All starting point), so perhaps one might want the Current Search block to be absent when there are no keys or filters.

heacu

apachesolr_static_response_cache($response);
apachesolr_has_searched(TRUE);
$result = "";

// Get blocks for all enabled filters

/*
foreach (apachesolr_get_enabled_facets() as $module => $module_facets) {
foreach($module_facets as $delta => $facet_field) {
if ($delta == "currentsearch") {
continue;
}
$blocks[$facet_field] = (object)module_invoke($module, 'block', 'view', $delta);
}
}

$result = theme('apachesolr_browse_blocks', $blocks);

apachesolr_has_searched(FALSE);
*/

return $result;
}

#11

janusman - June 1, 2009 - 04:02

[I rewrote this reply about 5 times, I hope it makes sense]

I understand the issue now; the patch just shows the facet blocks under the search form (but not as a block itself), and this is colliding with admins' expectations; they have already set up blocks to show in a certain placement in search sessions and were expecting blocks to show there.

It sounds logical, to keep consistency. However it *might* not be what the user expects (IMO offering to filter a search is different from giving the user starting points for a browse/search).

This is still a proof of concept =) I was aiming for a simple patch, and just showing the blocks near the search box was clear and simple (IMO). I was thinking that this patch should not make Drupal behave just like it just did "a search matching everything", because unexpected things might happen : like @heacu mentioned, the current search block might confuse the user... or perhaps even behind-the-scened things like watchdog logging a search when it wasn't a search, etc.

So:
A) Do we just make this patch execute a search matching everything, making the facet blocks will show up in the same place as if conducting a search?
B) Or do we attempt some new layout using a "show all facet blocks together to start browsing" strategy?

I like (B) best; and I admit it could be improved a lot. Some ideas:

  • showing blocks certain order
  • change labels from "Filter by ___" by default, to "Browse by ___". Filter implies there is something being shown to filter.
  • group those facet blocks into a "super" block; the default placement being under the search screen (like the current patch does), but admins could then place it elsewhere, like somewhere on the homepage with a Panel page.

I think it'd be good to offer some screenshots of similar browse interfaces on other sites. I'll work on that...

Meanwhile, any thoughts?

#12

Nick_vh - June 2, 2009 - 13:12

I prefer solution B!

And also willing to help in these implementations. Most of the code is already available, only some checks need to be changed.
- Showing blocks certain order is already implemented if you use the block order from drupal. (If needed you can create a region where you place the apachesolr blocks. I wouldn't add another block sort behavior.
- Dynamic labels, so users choose what labels they want for their content. Easily applied with block settings
- I don't really understand the idea of the super block? I think this is resolved by telling people to add a region (bottom content for example) and place their blocks in that region. You might make a superblock with a more condensed view of the filters?

An easier solution to start with might be adding the possibility to create paths with default search values?

#13

heacu - June 2, 2009 - 17:11

thoughts on A & B:

- the labels definitely need to be dynamically controlled. true, "browse by X" makes most sense at first. but once you've clicked on something and applied an initial filter, then "filter by Y" makes more sense in terms of labeling the potential second filters.
- better i think not to group all facet blocks together automatically. for initial browsing, for example, one might not want to display all the available facets. consider if you have a database of books, an initial browse-by facet by author doesn't make a whole lot of sense, because there could be thousands of authors. but, once you've narrowed it down with a search term, then faceting by author makes sense. this suggests to me that blocks should be small (not super), and that drupal's block management/display system is probably good enough.
- how about a "Browse by..." block that is like the "Current search..." block but for browsing? or... an improvement of the Current Search block that serves double duty, behaving differently when there are filters but no search key.

#14

janusman - June 19, 2009 - 23:29

New patch. This is almost the same except:

  • This labels blocks as "Browse by XXX" instead of "Filter by XXX".
  • Lays the blocks out by their relative weights assigned in the block table, in a 3-column table

See attached screenshot for how it looks. Since #358166: Search for just facet(s) has been comitted, this will now work out of the box.

AttachmentSize
457826-14.patch 3.83 KB
457826-14.png 76.76 KB

#15

Onopoc - June 20, 2009 - 00:03

Wow the "Browse available categories" is impressive. Very handy. It's indeed similar to what was done in Faceted Search module. Thanks.
Subcribing.

#16

robertDouglass - June 20, 2009 - 09:00

Question: Is this still assuming too much about the path? + $query = apachesolr_drupal_query('', '', '', 'search/' . arg(1));

Should we use $_GET['q']?

#17

robertDouglass - June 20, 2009 - 09:03

This should be done before the $blocks array gets sent into the theme:
+ usort($blocks, '_theme_apachesolr_browse_blocks_sort');

I'd suggest calling the function apachesolr_sort_by_weight()

#18

robertDouglass - June 20, 2009 - 09:05

This should all be handled in CSS:
<tr style=vertical-align:top>' : '') . '<td style=width:33%>';

#19

robertDouglass - July 3, 2009 - 21:09
Status:needs review» needs work

I removed the modulus%3 column table in favor of no table (just a list of blocks in divs) because in over half the themes I tried it was broken (fixed width). Better to let the themer override the theme to make three columns. I did some general cleanup including my comments in 16-18. There is a bug, though. If the solr server is unavailable and it goes to search/node, all hell breaks loose. gotta fix that.

AttachmentSize
browse_blocks.patch 3.71 KB

#20

robertDouglass - July 3, 2009 - 21:21

I like this one better - more of an API - makes it so that the browse function can be used by others. Still need a way to prevent it from showing up on search/node, or with a path in the facets that isn't legal.

AttachmentSize
browse_blocks.patch 3.78 KB

#21

JacobSingh - July 4, 2009 - 06:03

Nice! This is looking really good.

A couple items of feedback:

1. Blocks which are not set to be shown, are being shown. This could be good or bad. Someone could very likely have a filter enabled by default, and then just hide it via the blocks page, it would show up again here.

2. Formatting wise, I would like to to do something to pretty it up. As long as there is a theme function, why can't we put them in a tableish structure?

3. Small style point, but what do you think about

drupal_sort_weight() instead of the anonymous function? I don't really care, and you're solution is actually nicer, but just shooting for API consistency.

Either way, it's worth committing now IMO, but a follow up for formatting should be created.

-J

#22

robertDouglass - July 4, 2009 - 08:04

There are some lingering questions in my mind.

1) Why are we doing this in the search results area instead of using the blocks as they are enabled and configured by the admin? If we just did the search and left apachesolr_has_searched(TRUE) then the facet blocks would show up as if a keyword had been used. That might be a nicer behavior overall since it doesn't do anything unexpected. We might also want to give the option "On empty searches, do you want blocks in the normal regions? Or blocks below the search box? Or nothing?
2) If the blocks are showing up under the search results as per this patch, do we want to try to make that configurable in any way? Number of items to show? An include/exclude option per block so that the admin can have fine grained control over which ones show up?
3) Do we want to make the display configurable? In my site there isn't any room for more than one column of blocks. But janusman wanted 3 columns. This seems like a decision we can't make in advance and get right. I think we could add a configuration option to set the columns pretty easily - do we feel it is worth it?

And the known bugs in this patch, just as summary:
1) Blocks are showing that shouldn't
2) The blocks show up on search/node with wrong paths

#23

robertDouglass - July 4, 2009 - 08:09

I think this bit is the wrong approach:

<?php
+  foreach (apachesolr_get_enabled_facets() as $module => $module_facets) {
+    foreach(
$module_facets as $delta => $facet_field) {
?>

We should be using the $response object and iterating over its facets. apachesolr_search_add_facet_params($params, $query) already has all of the logic for what facets are enabled - we need not recreate it. This solves bug #1 from above.

#24

robertDouglass - July 4, 2009 - 09:19

scratch my comment in #23 - the current approach does what it's supposed to. this patch eliminates bug #1 in #22 - empty blocks don't get rendered.

AttachmentSize
browse_blocks.patch 3.92 KB

#25

janusman - July 6, 2009 - 20:37
Status:needs work» reviewed & tested by the community

It works, but theme('block', ...) is wrapping the blocks up with divs with meaningless classnames and ids (e.g. "block-" and "block--").

For instance:

<div class="block block-" id="block--">
  <div class="blockinner">
          <h2 class="title">Browse by XYZ</h2>
        <div class="content">
      <div class="item-list">[...]

The code works, though. =)

I do prefer doing away with the table, seems more "drupalish", and overridable too =)

#26

robertDouglass - July 7, 2009 - 08:54
Status:reviewed & tested by the community» needs work

There's still the bug where blocks show up on search/node with wrong paths.

#27

robertDouglass - July 7, 2009 - 09:02

This will go into the 6-2--0 branch.

#28

robertDouglass - July 13, 2009 - 20:26

Drupal's block loading mechanism is stupid. The only API function is block_list(), which takes $region as it's required parameter. Bleh! It does us no good. But we'd want such an API function here, because the query that gets run inside of it looks like this:

<?php
$result
= db_query(db_rewrite_sql("SELECT DISTINCT b.* FROM {blocks} b
  LEFT JOIN {blocks_roles} r ON b.module = r.module AND b.delta = r.delta
  WHERE b.theme = '%s' AND b.status = 1 AND (r.rid IN ("
. db_placeholders($rids) .") OR r.rid IS NULL)
  ORDER BY b.region, b.weight, b.module"
, 'b', 'bid'), array_merge(array($theme_key), $rids));
?>

In other words, it's more than just calling apachesolr_get_enabled_facets(). :(

I'm going to push forward with the feature but it's going to get a bit uglier. I want to load the blocks right according to the constraints that Drupal normally puts on them.

#29

janusman - July 13, 2009 - 21:07

@robertDouglass: How about a "simple" solution like adding a column of checkboxes to admin/settings/apachesolr/enabled-filters so the user could mark filters available to start a search? That listing could also be weighted (drag-n-drop?)

#30

robertDouglass - July 13, 2009 - 21:15
Status:needs work» needs review

The extra UI isn't a bad idea. I don't have energy for it now, though. I'm thinking about committing this patch and using it to open the 6.2 branch. Sorry the apachesolr_search_browse() function got somewhat ugly. I preserve the important parts of block_list() so that we don't surprise anybody with blocks that aren't supposed to be seen. I also include a line drupal_alter('block', $block); If anybody objects to this I'll take it out, but I patch core (block_list) with the same thing, and it's very useful. It lets other modules alter the $block object before it gets rendered. I'll take it out if someone complains, though. Otherwise it works like the previous patch without the bugs.

AttachmentSize
browse_blocks.patch 6.35 KB

#31

janusman - July 13, 2009 - 23:22
Status:needs review» reviewed & tested by the community

The patch works, thus marking as RTBC.

Just a minor thing...

It kind of threw me off that search/apachesolr_search shows some filters from blocks I do not have assigned to any region... the administrator will certainly get confused as to where they'll show relative to others (what their weight is) if they're all in region "" in admin/build/blocks.

I know the admin should also configure the enabled filters elsewhere if he wants to shut them off entirely (or use the new _alter hook to remove them)... but for clarity's sake I'd suggest not showing facets that are assigned to region "" in admin/build/blocks.

Maybe it's just me? Does it "throw off" anyone else? =)

#32

JacobSingh - July 14, 2009 - 05:11

Yes, I also was confused by that. Unfortunately, the whole block system is screwed up this way.

I feel like showing the enabled filters regardless of placement is preferred though. This also adds some flexibility because if a user wants certain filters to show after a search, and additional ones to show on the "directory" they can do so. It is however silly when they click on one which is not placed, because the block will vanish :)

Best,
Jacob

#33

robertDouglass - July 14, 2009 - 12:21
Version:6.x-1.x-dev» 6.x-2.x-dev
Status:reviewed & tested by the community» fixed

Committing to 6.2

#34

heacu - July 16, 2009 - 23:05

the latest patch forgets to call

apachesolr_modify_query

within apachesolr_search_browse after apachesolr_current_query($query)

this causes unexpected behavior if one's module defines

hook_modify_query

#35

Nick_vh - July 17, 2009 - 08:32

I tested this patch and it's working pretty good. Except it's not this kind of behaviour that I would expect.
Would it be possible to show the blocks (which are generated on the search page now) where they are supposed to be? In their regions?

I searched a bit for this option but couldn't find it rapidly

#36

Nick_vh - July 17, 2009 - 08:37

Also the titles of the blocks are not respected if they are adjusted.
If we look at the block object we see two titles :
$block->subject
$block->title;

What is the preferred way to show the real title? I now have this but I'm sure it's not the most correct option.
if(!empty($block->title)) {
$block->subject = $block->title;
}

#37

robertDouglass - July 17, 2009 - 09:17
Status:fixed» active

Great feedback everybody. I think #34 is correct, which is a bug. #35 should be a pretty simple option to implement. How do we feel about a radio button for the admin that asks:

When visiting the search page before a search term has been submitted,

1. Show no facet blocks
2. Show facet blocks in their normal locations
3. Show facet blocks in the content area

#36 is a sticky issue. Maybe we want to just leave block titles alone and leave it to the administrator to find a single title that always works?

#38

Nick_vh - July 17, 2009 - 17:54

I'm pro for these radio buttons!

#39

janusman - July 20, 2009 - 13:20

I'm also for #37.

#40

janusman - August 28, 2009 - 22:19

New patch, fixed #34, added configurable setting from #37. Don't know what to do about #36 yet.

See screenshot for how each option lays out the blocks in Garland.

AttachmentSize
apachesolr-457826-40.patch 3.38 KB
apachesolr-457826-40.jpg 57.43 KB

#41

janusman - August 28, 2009 - 22:19
Status:active» needs review

Drat! =)

#42

janusman - August 28, 2009 - 22:29

Had different default values in variable_get()s. New patch.

AttachmentSize
apachesolr-457826-42.patch 3.38 KB

#43

janusman - September 2, 2009 - 13:34

Small revision on patch:

Was:

+++ apachesolr_search.module 28 Aug 2009 22:00:14 -0000
@@ -945,6 +956,21 @@
+      'none' => t("Show search box"),
+      'browse' => t("Show search box and enabled filters' blocks under the search box"),
+      'blocks' => t("Show search box and enabled filters' blocks in their configured regions"),
+      'results' => t("Show search box, first page of all available results, and enabled filters' blocks in their configured regions"),

I think the last option should change, for logical arrangement:

+++ apachesolr_search.module 28 Aug 2009 22:00:14 -0000
@@ -945,6 +956,21 @@
+      'none'    => t("Show search box"),
+      'browse'  => t("Show search box and enabled filters' blocks under the search box"),
+      'blocks'  => t("Show search box and enabled filters' blocks in their configured regions"),
+      'results' => t("Show search box, enabled filters' blocks in their configured regions and first page of all available results"),

New patch, still needs review =)

I'm on crack mochaccino. Are you, too?

AttachmentSize
apachesolr-457826-43.patch 3.39 KB

#44

robertDouglass - September 11, 2009 - 11:09
Status:needs review» fixed

Beautiful. Committed.

#45

TwiiK - September 17, 2009 - 12:51

This is a great addition, however with faceted search we used the facets as the main navigation for our sites. Would adding a "Show filter blocks at all times"-option be something that's easy to add here?

I've looked at the module myself, but it's too much and too advanced for me at the moment. :)

#46

robertDouglass - September 17, 2009 - 13:22

@TwiiK That's a known feature request but I'm not sure this configuration page is the place to do it. My plan for facet based navigation is to expose facet blocks as Views via Views 3 and apachesolr_views. This would give you total control over the blocks.

Right now we fight against the block system by taking block visibility into our own hands. This can easily get very messy.

For right now there's no good solution to your problem. But be patient, we'll get there.

#47

System Message - October 1, 2009 - 13:30
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

#48

Onopoc - October 23, 2009 - 16:37

@TwiiK: if you want to test apachesolr_views module Scott has recently added a dev version (6.x-1.x-dev) to the module project page http://drupal.org/project/apachesolr_views

#49

fabdel - October 30, 2009 - 13:50

Subscribe.

I've got the error message :
warning: Invalid argument supplied for foreach() in /**********/sites/all/modules/apachesolr/apachesolr.module on line 1203.

When i go directly to the page without typing any word in the search box.

#50

Jason Ruyle - November 16, 2009 - 18:17

I get this same error message.
Originally we were going to show facet blocks on empty search, but have opted out of this idea. We do not want, nor show, facets on our blank search page, but do get this error.

 
 

Drupal is a registered trademark of Dries Buytaert.