Would be nice if this module took steps to not include the SQL in the search index. You could write some code or refer people to http://www.lullabot.com/articles/hiding-content-drupals-search-system or http://drupal.org/project/search_restrict.

Not sure if apachesolr needs different treatment to be excluded from index.

CommentFileSizeAuthor
#3 customreports-lots-of-stuff-2.patch15.06 KBpobster
#1 customreports-lots-of-stuff.patch14.3 KBpobster

Comments

pobster’s picture

pobster commented
Priority: Normal » Major
Status: Active » Needs review
StatusFileSize
new customreports-lots-of-stuff.patch14.3 KB

Okay so I've addressed a few niggles I have with this module;

1. Don't use t() in hook_menu.
2. Naming conflict with hook_access.
3. Missing t() in admin form item.
4. Old D5.x parameters in _submit callback.
5. Removed t() from node_type creation (it can't refer back, it's stored in the db).
6. Exclude node-type from search index/ advanced search form.
7. Removed header() in favour of drupal_set_header().
8. Minor punctuation and other changes… See diff!

The _clean_quotes() function still concerns me though, if you have a query with both single and double quotes - surely this will break it? …But that's another issue...

Thanks,

Pobster

pobster’s picture

pobster commented

Balls… Note that the doxygen tip for _customreports_alter_advanced_search_form() is incorrect - apologies nobody is perfect! Won't post another patch, just pointing this out.

Thanks,

Pobster

pobster’s picture

pobster commented
StatusFileSize
new customreports-lots-of-stuff-2.patch15.06 KB

Okay, you can ignore this if you wish… It's an old string CSV parser that I found on php.net a couple of years ago, I've reused it in dozens of projects! It's pretty tidy… Unfortunately I've completely forgotten who wrote it, so can't give any credit...

Thanks,

Pobster
edit: Note that it includes the patch from above, and also addresses the incorrect doxygen tip.

a_c_m’s picture

a_c_m commented

Will try to review this, this weekend at drupal dev days.