Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hi,
I've added a patch that provides a search blocks for each search page.
Greetz
Tim
Comment | File | Size | Author |
---|---|---|---|
#8 | search_api_page.search_blocks.patch | 10.29 KB | drunken monkey |
#7 | search_api_pages_blocks.patch | 6.8 KB | Anonymous (not verified) |
#2 | search_api_page.search_blocks.patch | 5.32 KB | drunken monkey |
search_api_pages_blocks.patch | 4.73 KB | Anonymous (not verified) | |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #2
drunken monkeyWow, great work, thank you very much!
The patch generally seems to work fine — however, there are a few issues still before I can commit it:
hook_forms()
? I don't see any reason for it, or anything it really does. Also, its implementation is wrong,$args
shouldn't be included in the "callback arguments". Because of this, currently all search forms are "compact".hook_forms()
does, right?)_search_api_page_load_pages
you should just useentity_load
or a newsearch_api_page_load_multiple
function. Static caching should also usedrupal_static
,entity_load
will already do the aching for you and themodule_exists
is unnecessary.Attached is a patch changing a few smaller things. Please also file patches agains the project's root directory, not the contrib/search_api_page directory — that makes them easier to apply.
But thanks again for doing this, it's great to see other people chip in with coding!
Comment #3
Anonymous (not verified) CreditAttribution: Anonymous commentedHi,
The reason I used hook_forms() is to overcome the problem of having multiple forms on the same page. otherwise on submit the forms are not unique and for example form_set_error will highlight all form fields. or the page redirect will for all forms take the same path and search keys.
The forms should indeed be moved to the .module now
The reason i used a helper function with the module_exists :-) but i forgot to add it to the path :-S is, i replaced the for in the hook_menu with the helper function so that you would get an array of the pages.
Thanks for the drupal_static hint, didn't know that one yet.
I also had another question, when creating an search page, you can choose the query type, this seams to give the same results whatever you choose?
Tim.
Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedBy the way, I'll try to rework the patch and post here later today.
Comment #5
drunken monkeyOh, OK, that does make sense. I know that the Form API is pretty poor in that respect, but didn't think of that.
But there still wouldn't be any way the module wouldn't exist when a function in that module's .module file is run, right? Also, I can't really follow you there, I'll look at that in code form …
Because of restrictions of the database backend (that can't deal with phrase queries) this is true for that backend. However, it should have very noticable effects when using the Solr (or any other "professional") backend and doing a search with more than one keyword. (Or, at least, "Single term" should behave noticably different.)
Looking forward to your improved patch! (No rush, though, there's no feature freeze in sight … ;))
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commentedI used it in that way because in the current hook_menu this is the following code:
// During uninstallation, this would lead to a fatal error otherwise.
if (module_exists('search_api_page')) {
......
}
So i didn't want to break that... :-)
I tried it with solr, but it only had like 4 dummy record, and i didn't see any difference there. I will try again with some more realistic and more data, and see if i get different results then.
Greetz
Tim
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedI've reworked the patch, a little less code, also added a
search_api_page_load_multiple
function to get all or multiple search pages by an array of id's, placed the forms in .moduleI've tried to remove the module_exists from the hook_menu, but it indeed fails when trying to uninstall.
Do you know if there is someone already working on a apache solr attachments integration?
Greetz
Tim.
Comment #8
drunken monkeyAs said, the difference is the parsing of multiple keywords. While with "Multiple terms" (and, for most backends, "Direct query" too) a search for »foo bar« will e.g. match in the text "baz bar lol foo", with "Single term" the keywords will be treated as one single phrase query and therefore only match something like "baz foo bar blah".
*_load_multiple()
functions should always follow the same pattern, i.e., receive($ids, $conditions, $reset)
as arguments. Hardcoding it to only returned enabled pages is confusing for a function with that name. For this functionality, you should have named it something likesearch_api_page_load_enabled()
— but for now, I corrected its definition and use and we should just leave it at that. Three uses to get all enabled pages doesn't really warrant an own function, anyways.Yes, of course — sorry, my bad again, completely forgot that, and the use of
module_exists()
inhook_menu()
…No, I don't think so.
Do you know, how attachments are currently implemented? Are they just file fields added via the Field API?
In that case, a data alteration that offers the text content of all file-typed fields would probably the best way to go. But in any case, I would be glad to help, if you want to work on that and need some assistance. Since it will probably need an additional library, we just can't really add it to the core module, even though it would be a great addition. But I'd definitely mention it on the project page.
Back to the issue at hand: Attached is a revised patch that seems to work alright. If you want to change anything, please work from that patch, otherwise I'll just commit it in the next couple of days (or after your response).
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedTested latest patch and everything seems to work, I'm a happy camper :-)
To index the content of files we would have to use apache tika. I'm going to experiment a bit with it.
Greetz
Tim.
Comment #10
drunken monkeyCommitted, thanks!