Posted by cpliakas on January 8, 2012 at 1:09am
4 followers
| Project: | Apache Solr Search Integration |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | cpliakas |
| Status: | needs work |
Issue Summary
Based on the discussion at #1391850: Fatal error when Solr server is not available on non-search pages that are showing facet blocks starting at comment #6, the administrator should select which search page facets link to when they are displayed on non-search pages. As mentioned by pwolanin in the referenced thread:
I think conceptually it would make more sense to link to a specific search page, then we'd always know where the facet links go.
This is the approach that should be taken as opposed to guessing the path by the ApacheSolrFacetapiAdapter::getSearchPath() method which is how the current implementation works.
Comments
#1
The attached patch is my first attempt at adding the suggested functionality.
To summarize, the code finds all paths that are associated with the environment and exposes them as radio buttons. If there are no search pages associated with an environment, the settings are not displayed. A wrapper function was added to get the path from the selected search page while checking the sanity of the value. If it is not sane, for example the administrator moves the page to another environment, then the facets are not displayed. I also grouped the settings under the "apachesolr_variables" element to simplify the
apachesolr_search_facetapi_realm_settings_form_submit()function.#2
#3
Fixed some comments.
#4
Is this a duplicate from #1394136: Link non-search page facets to a search page rather than environment ? I can close the other one if you like?
#5
Search fail. Since this one has patch history, I will close the other one.
#6
Changing status for the testbot, testing it out :-)
#7
#8
This patch is working fine but I am not sure if the facet configuration is the right place to add this setting. Just brainstorming here.
What if we add this setting to the search pages advanced configuration. Say : Add facets from this search page also on .
This way you have a context of search environment and path directly and I think the location where the setting is would be easier to find for novice users?
#9
Thanks for testing, and brainstorming is good!
The reason why I added it under the facet configuration page is because architecturally facet configurations are tied to environments. To me it would be more confusing to split facet configurations out between the environment-specific administrative page and search page-specific administrative pages. The way the UI works in the attached patch the user can say, "This is my facet configuration, these are the non-search pages that I want my facets to display on, and this is the search page I want them to link to." all on the same administrative page. Otherwise they would have to configure the facets, click on the search pages tab, click on the search page, then expand the advanced settings tab. To me that is too much clicking required to configure settings that are deeply coupled, and it would also hide the functionality which based on my issue queue is a fairly common request.
Another issue I see is that we are only checking to display facets related to the default environment. I still don't really agree with this and think we should be checking all environments, bu regardless this could cause some confusion where settings are active on search pages that don't belong to the default environment. This is more of a minor issue because we could hide the configuration for pages tied to non-default environments, however I also think that it would be confusing that the non-search page settings are active on some search pages but not on others.
Just my $0.02,
Chris
#10
The facet blocks have to link to a search page, so I think the approach proposed by Nick is the best option conceptually.
I agree you might have to go elsewhere to configure the facets, but that's already true, and we let you switch a search page to use a different environment (meaning different facets) already.
#11
I still strongly disagree with this approach from a UX standpoint. If you can only link the facets to one search page, then why are we providing this option on every search page? How do we specify which search page's configuration is active? I'll re-roll a patch taking the approach in #8 if that is consensus, but I do strongly urge that we walk through a site builder's use case to make sure we aren't introducing too much confusion.
#12
The problem arises when a search page switches from environment.
When this happens you loose all context of what should happen with a certain facet block of a specific environment & a specific search page.
If we stick with facet configuration you'd likely have to never change environment because you would loose those settings?
On the other hand, I do see potential problems when someone tries to redirect a certain block to two different search pages.
Search page 1: show blocks on /randomurl
Search page 2: show blocks on /randomurl
The blocks of that environment will probably have to pick the first available search page to redirect to?
#13
Well, you could change the environment, but you would have to select another search page that facets would link to otherwise they would no longer display. There really isn't a good way around this other than displaying a message after changing an environment of the selected page saying "Hey... go here to choose another search page.".
This is the part that I have the most problems with from a UX standpoint. Users will think that it is a valid option to have facets on page /foo link to page 1 and facets on page /bar link to page 2. This isn't possible given that facets and all of the non-search page settings are tied to an environment. This limitation is made clear and there is no confusion with the UI in #3 since it hits you over the head that you can only link to one search page.
I guess we could rearchitect the solution to store the settings per-search page as opposed to per-environment in order to accomodate this? The down side is that we would have to loop over all search page configs in hook_init() and stop on the first one we encounter. Not really a big fan of that, though, because it seems like there is some guessing happening which could lead to inconsistency.
#14
Hmm
You do make some valid points. One of my other concerns is that maybe the configuration of the non-search page facets is very hidden somehow. It'd would take my some time to find it under the list of facets. Would there be another sensible place to put this? Maybe even in the top of that page?
#15
I'm not sure where that alternate location would be, but I am definitely open to exploring this. I want to take a step back so I can think about this more objectively. Like we discussed offline there is no magic bullet here, each option has its drawbacks.
#16
Oh, and also this would create confusion between search environments and search pages. Event though there are not linked with each other in a hard way.
Proposed by pwolanin in chat might be the most straight forward solution. And I'm not claiming it is the best but we have to roll Beta13 out asap :-)
This would affect only the default search environment and it would have one search page to fall back on in non-search pages. Kind of the 'default' fallback search page. This could be a setting similar to the 'default' setting for a search environment, only 1 search page is handled or *prioritized*
The configuration of the paths that should show the facets could stay in the facets configuration imho because it would be confusing to add this to the search pages.
Not much really has to change to the current setup, only the "default" search page should be defined. If we need more improvements we can handle this at a later moment.
Agreed?
In short : singleton goto search page for any kind of block on a non-search page.
#17
So, this is a patch demonstrating what we've been suggesting
#18
Bit cleaner, without the stupid "correction" to the default env :-)
#19
This version of the patch adds a (default) to the list of search pages and also moves the form to the top of the facet configuration so it becomes more visible. I feel that all-in-all this is the best of both worlds
#20
Moves the option of the default search page a bit higher and sets it in all cases, not only for facetapi
#21
Committed this to roll Beta14. Please continue on the current code if changes need to apply
#22
Small update
#23
#24
Committed this small bugfix
#25
Backported this and committed patch #20 and #22 to 6.x-3.x
#26
Changing back to 7.x-1.x
#27
Closing this since it is a bit confusing what the next steps should be.Whenever needed we'll open a new issue for improvements.Thanks for all the effort#28
*Not closing* Patch 26 needs review! I was mistaken
#29
The last submitted patch, 1397526-25.patch, failed testing.