Closed (fixed)
Project:
Apache Solr Search
Version:
6.x-2.x-dev
Component:
User interface
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
8 May 2009 at 21:25 UTC
Updated:
16 Nov 2009 at 18:17 UTC
Jump to comment: Most recent file
Comments
Comment #1
xarbot commentedSorry, 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
Comment #2
janusman commented@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 =)
Comment #3
janusman commentedNew patch; needs you to apply this patch first: http://drupal.org/node/358166#comment-1627524
Comment #4
David Lesieur commentedSome suggestions:
Comment #5
David Lesieur commentedComment #6
janusman commentedThanks for the input. Here's a new patch. Again, it's dependant on applying this patch first: http://drupal.org/node/358166#comment-1627524
Comment #7
nick_vhThe 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?
Comment #8
janusman commentedWeird. 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
Comment #9
nick_vhYes 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!
Comment #10
heacu commentedas 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;
}
Comment #11
janusman commented[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:
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?
Comment #12
nick_vhI 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?
Comment #13
heacu commentedthoughts 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.
Comment #14
janusman commentedNew patch. This is almost the same except:
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.
Comment #15
francewhoaWow the "Browse available categories" is impressive. Very handy. It's indeed similar to what was done in Faceted Search module. Thanks.
Subcribing.
Comment #16
robertdouglass commentedQuestion: Is this still assuming too much about the path? + $query = apachesolr_drupal_query('', '', '', 'search/' . arg(1));
Should we use $_GET['q']?
Comment #17
robertdouglass commentedThis 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()
Comment #18
robertdouglass commentedThis should all be handled in CSS:
<tr style=vertical-align:top>' : '') . '<td style=width:33%>';Comment #19
robertdouglass commentedI 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.
Comment #20
robertdouglass commentedI 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.
Comment #21
JacobSingh commentedNice! 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
Comment #22
robertdouglass commentedThere 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
Comment #23
robertdouglass commentedI think this bit is the wrong approach:
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.
Comment #24
robertdouglass commentedscratch 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.
Comment #25
janusman commentedIt works, but theme('block', ...) is wrapping the blocks up with divs with meaningless classnames and ids (e.g. "block-" and "block--").
For instance:
The code works, though. =)
I do prefer doing away with the table, seems more "drupalish", and overridable too =)
Comment #26
robertdouglass commentedThere's still the bug where blocks show up on search/node with wrong paths.
Comment #27
robertdouglass commentedThis will go into the 6-2--0 branch.
Comment #28
robertdouglass commentedDrupal'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:
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.
Comment #29
janusman commented@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?)
Comment #30
robertdouglass commentedThe 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.
Comment #31
janusman commentedThe 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? =)
Comment #32
JacobSingh commentedYes, 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
Comment #33
robertdouglass commentedCommitting to 6.2
Comment #34
heacu commentedthe 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
Comment #35
nick_vhI 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
Comment #36
nick_vhAlso 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;
}
Comment #37
robertdouglass commentedGreat 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?
Comment #38
nick_vhI'm pro for these radio buttons!
Comment #39
janusman commentedI'm also for #37.
Comment #40
janusman commentedNew 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.
Comment #41
janusman commentedDrat! =)
Comment #42
janusman commentedHad different default values in variable_get()s. New patch.
Comment #43
janusman commentedSmall revision on patch:
Was:
I think the last option should change, for logical arrangement:
New patch, still needs review =)
I'm on
crackmochaccino. Are you, too?Comment #44
robertdouglass commentedBeautiful. Committed.
Comment #45
twiik commentedThis 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. :)
Comment #46
robertdouglass commented@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.
Comment #48
francewhoa@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
Comment #49
fabdel commentedSubscribe.
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.
Comment #50
Jason Ruyle commentedI 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.