This module deals with a problem that the Drupal top search keywords are stored to watchdog table, and if you have selected the option for the watchdog not to grow over certain number of records, the statistics will be wrong as older ones will get deleted.

This module creates extra tables in order to hold that data, holding information for both: keywords and full search phrases.

Have searched for module with this functionality however did not find such a module thus wrote this one.

Link to project page:
http://drupal.org/sandbox/centas/1438782

Link to git:
http://drupalcode.org/sandbox/centas/1438782.git/commit/0d34684

Drupal 6

Comments

iler’s picture

Status: Needs review » Needs work

You should make these fixes before the functionality of the module will be tested:
http://ventral.org/pareview/httpgitdrupalorgsandboxcentas1438782git

centas’s picture

Have done all the fixes, had some issues gettin used to git but seem to be better now.

http://ventral.org/pareview/httpgitdrupalorgsandboxcentas1438782git

centas’s picture

Status: Needs work » Needs review
shivachevva’s picture

Status: Needs review » Needs work
StatusFileSize
new2.4 KB

Some coder validations are missing.
Please see the attached file.

centas’s picture

what did you use to check markup?
The ventral.org one seems not to give any errors of markup

centas’s picture

what did you use to check markup?

centas’s picture

Status: Needs work » Needs review
bart.hanssens’s picture

Status: Needs review » Needs work

Hi,

1) I think you could save a few queries in search_statistics_search.

Instead of
"SELECT COUNT(pid) FROM {search_statistics_phrases} WHERE phrase = '%s'"
and then (when phrase is found) a second query
"SELECT pid FROM {search_statistics_phrases} WHERE phrase = '%s'

Perhaps you can just write:
$record = db_fetch_array(db_query("SELECT pid, count FROM {search_statistics_phrases} WHERE phrase = '%s'"));
and check if record is set.

Same remark for keyword search.

2) You may also want to rewrite the db_query("UPDATE...") into a drupal_write_record(...)

3) Reduce the code size by simplifying search_statistics_top_keywords() and search_statistics_top_phrases() : lines between $tablesort and return $output are the same in both functions, so that can be split into a separate function

centas’s picture

Thanks Bart,

will take a look at improving the code.

centas’s picture

Status: Needs work » Needs review

done the changes proposed, plus added a workaround for the search statictis to be updated only once, as hook_search is executed tewice, once per content search and second time per user search.

as well have cleared up the master branch.

centas’s picture

Priority: Normal » Major
patrickd’s picture

I'm sorry for the delay!
as there are currently many applications in queue we need more reviewers,
so think about getting a review bonus and we will come back to your application sooner!

atul.bhosale’s picture

Status: Needs review » Needs work
StatusFileSize
new42.52 KB

Manual review

Wrong search keywords and phrases count

Count get incremented by 1 when user perform search twice

Steps to reproduce

  1. Active theme is Garland
  2. Place search box in left column (Now search with keyword Drupal, assuming content had a word Drupal and site is index)
  3. User is redirected to /search page with search result,
    And in search_statistics_keywords and search_statistics_phrases table drupal is added as phrases and keyword with count
    As user is redirect to /search page now we can see two search boxes

  4. Again enter word drupal to perform search in field which is render in a right content pane (not in search block from left column) count not get incremented. Count only get incremented by 1 when user perform search twice using field which is render in a right content pane.
  5. When user perform search using search block from left column count properly incremented.

Wrong count due to this condition if (!isset($_SESSION['search_statistics_search'])) in code. If this condition is removed count get incremented by 2 which is already highlighted in #10.

centas’s picture

Status: Needs work » Needs review

Have tested it and all seems to work fine.
Tried to contact Atul Bhosale however with no luck.

The idea behind the module is that the module only counts the keywords for node search, possibly the problem with above is that Atul tried seaching for users first?

There is a condition on line #59 That makes sure it is a node seach

if (arg(0) == 'search' && arg(1) == 'node') {
lawik’s picture

Status: Needs review » Needs work

Nice solution to a problem I wasn't familiar with. Seems silly to store that data in the watchdog table.

While checking the features I got only expected results.
I did consider if the tab named "Top search phrases" should be named "Top searches" as "phrases" gives me other connotations.

Automated checks: Drupal Code Sniffer (project drupalcs) gives me three: "Do not use t() in hook_menu()" which should be taken care of. Menu title are automatically t():ed. You'll probably get that using ventral as well.

Hade a look at the code in general. Seems solid. It's not a huge module.

With the t()-fix I'd say this is ready to go.

centas’s picture

Status: Needs work » Needs review

hi lawik,

thanks for your feedback and suggestions. have done and commited the changes now.
I did rename "Top search phrases" to "Top searches" as think that does indeed sound better.

Plus have updated README.txt to clarify that if user will search for users on site it will not be tracked by the module, it will only track the node searches.

atul.bhosale’s picture

Status: Needs review » Needs work

Hi centas ,

Hi there, you have reviewed my module and came back with response:
http://drupal.org/node/1438844#comment-5833882

I tested in in few website and always work fine.
The only thing I can think about that might be causing it is line 59: "if (arg(0) == 'search' && arg(1) == 'node') {", so if you search for users as default and then possibly search for content?

The mmodule counts only keywords for contnet search rather than content and user.

Could you please provide a link to test environment?

Please keep your update within the review thread, rather than email ... this way, others can learn from the discussion as well.

As per your comment in #14, I possibly tried to search user first but in my comment #13 it mention that I tried with word drupal which is already present in content and site is index.

Still I am able to reproduce this issue using steps from #13 comment, also have a look at attached screen shot with #13 comment to be more clear on left column and right content pane

If u want I can set-up demo site for you.

centas’s picture

Hi Atul Bhosale,

I have tested yet again exactly following your step by step instructions and still cannot reproduce the error, every time works exactly as expected.
Did you test the latest commit?
http://drupalcode.org/sandbox/centas/1438782.git/tree/fcb1e8e91eb0473fa9...

If you could provide access to your test environment or set up a demo site where I could take a a look at the error it would really help.

Looking forward to hear from you.

atul.bhosale’s picture

StatusFileSize
new70.5 KB

Hi centas,

yep, I tested with the last commit.

Following is demo site details:
URL : http://www.a-workspace.com/search-statistics/
User name : demouser
Password : demouser123

Also have a look at attached screen shot,

Feel free to revert for further input.

Once done update me I have to close demo site.

centas’s picture

Hi Atul,

Seems like this is a bug, I think I have few ideas on what might be a reson for such behaviour.

Is it possible at all to get an arhive of the demo site? Could you put it to Dropbox for me to download?

atul.bhosale’s picture

Hi Centas,

I will create an archive and share the download link with you at my EOD.

centas’s picture

Hi Atul,

any luck with the archive? :)

atul.bhosale’s picture

StatusFileSize
new1.34 MB
new22.9 KB

Hey Centas,

sorry, deadly forgot to share files.

Login details : drupaladmin/drupaladmin

centas’s picture

Hey Atul,

thanks, I will take a look at it within next few days and will update on the progress.

klausi’s picture

Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.