[4.0] com_finder: add search statistics by Hackwar · Pull Request #20681 · joomla/joomla-cms

Hackwar · 2018-06-06T21:28:40Z

Smart Search right now has no own logging, but instead uses the crappy logging of com_search, which itself is from Mambo times... The logging of com_search only logs the search term and the number of times that it was searched for and while there is an idea that it should log the statistics separately for both com_search and com_finder, it can't even distinguish between the two...

Anyway, this PR is based on issue #17437 and for now immitates the logging of com_search. In the component configuration I renamed the option to enable the logging to something that is a bit more descriptive. Then I added a new table for the logging data and in that table we now store the search term, the query object, the number of times this specific search was executed (including the filter settings), the number of given results for this and an md5sum to compare searches easily.
In the backend we have a new view, copied over from com_search mainly and updated to com_finder, that displays the statistics. At the top we have a button to reset these stats.

This PR implements the statistics that are part of com_search in com_finder, so that we can drop com_search and I would call this PR ready for now. In a second step, I would like to improve the FinderIndexerQuery class and the display of the stats to also display the filters of that search query and the language. But to be honest, I don't know how to display this nicely right now and I wanted to get this PR out there. So that would be part of another PR.

@alikon Can you check if I did the Postgres queries right? I didn't test those and just modeled them after the other queries that we have so far.

This removes a blocker for #20637

brianteeman · 2018-06-08T11:27:18Z

administrator/language/en-GB/en-GB.com_finder.ini

 COM_FINDER_ITEM_X_ONLY="%s Only"
 COM_FINDER_ITEMS="Content"
+COM_FINDER_LOGGING_DISABLED="Gathering statistics disabled. Enable it in the Options."
+COM_FINDER_LOGGING_ENABLED="Gathering statistics enabled"


This should have a period at the end - however I am not sure we even need to display this notice

Fixed. Since search logging could be pretty resource intensive, it doesn't seem like the worst idea to have that notice. We have this in the current com_search statistics, too. However I'm open for change requests. Should I remove this notice?

…4finder_log # Conflicts: # administrator/components/com_finder/sql/install.postgresql.sql # administrator/components/com_finder/sql/uninstall.mysql.sql # administrator/components/com_finder/sql/uninstall.postgresql.sql

carlitorweb · 2018-06-18T18:32:08Z

I got this on the frontend:

Warning: count(): Parameter must be an array or an object that implements Countable in ..components\com_finder\View\Search\HtmlView.php on line 163

carlitorweb · 2018-06-18T19:42:33Z

administrator/components/com_finder/View/Searches/HtmlView.php

+	{
+		$canDo = $this->canDo;
+
+		ToolbarHelper::title(\JText::_('COM_FINDER_MANAGER_SEARCHES'), 'search');


I see you use JText for all strings. Why not use the new Text class?

It honestly doesn't matter either way right now if namespaced or aliased global classes are used.

Yes I know, that is why I ask. Thank for answer

Hackwar · 2018-06-18T20:32:15Z

This is mainly code copied from current com_search. Is it required that all of this is updated to the latest code or can we keep it the way it is right now and merge this?

carlitorweb · 2018-06-18T20:38:09Z

I know you wrote, you will work more in this in other PR, but you can update the code with this move, and then work more easily in the update you want to do. No problem, if you prefer to wait.

Hackwar · 2018-06-18T20:52:36Z

@carlitorweb Please make sure that you have the latest version of 4.0-dev before applying this PR. It seems as if you don't have the latest updates that I did to com_finder lately.

carlitorweb · 2018-06-18T21:03:35Z

Yes, I have the last version of 4.0-dev branch. No problem with that.
After i applied the last change, i got this:
Argument 1 passed to Joomla\Component\Finder\Site\Helper\FinderHelper::logSearch() must be an instance of Joomla\Component\Finder\Site\Helper\FinderIndexerQuery, instance of FinderIndexerQuery given, called in ..components\com_finder\View\Search\HtmlView.php on line 163

carlitorweb · 2018-06-18T21:25:25Z

By the way, now I have the option working well, in the admin side. No more the message I had before. Sorry for this, was a mistake when I merge you PR

Hackwar · 2018-06-19T17:28:52Z

Sorry, a typo-fix was not commited. Fixed that one. Please test again. 😉

carlitorweb · 2018-06-19T21:07:07Z

I still got:
Warning: count(): Parameter must be an array or an object that implements Countable in ..components\com_finder\View\Search\HtmlView.php on line 163

The statistic works fine. But as you can see, the Results column only show 1, because the warning I put you above.

carlitorweb · 2018-06-19T21:09:24Z

Also, every time you enter in the Statistics view the message Notice - Gathering statistics enabled show up. I suggest to you if the option is On no show this message, is unnecessary in my opinion

Quy · 2018-06-19T22:52:16Z

administrator/components/com_finder/forms/filter_searches.xml

+		<field
+			name="search"
+			type="text"
+			label="COM_FINDER_SEARCH_IN_PHRASE"


Missing language string. label and description have the same constant.

Hackwar · 2018-06-20T07:37:08Z

@carlitorweb Since the statistics gathering can potentially create a lot of entries in the table, there should be a notice either way, both when it is on and when it is off. That was at least the reasoning for this in the logging in com_search.

@Quy fixed.

…4finder_log

…inder_log

Hackwar · 2018-06-20T19:15:54Z

@carlitorweb I can not confirm that error message. Please don't use the pulltester for this.

carlitorweb · 2018-06-20T19:19:58Z

Thank @Hackwar . Yes, I just end the test without patchtester and no get the warning. Sorry for boring with this, I just start to get the idea when use or not the patchtester

carlitorweb · 2018-06-20T19:50:59Z

I have tested this item ✅ successfully on 8ed03d7

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20681.}

Quy · 2018-06-21T02:34:44Z

Fix year in filename 4.0.0-2918-06-06.sql

Quy · 2018-06-21T02:38:55Z

administrator/components/com_finder/Model/SearchesModel.php

+/**
+ * Methods supporting a list of search terms.
+ *
+ * @since  __DEPLOYED_VERSION__


__DEPLOY_VERSION__

Quy · 2018-06-21T02:39:08Z

administrator/components/com_finder/Model/SearchesModel.php

+	 * @param   MVCFactoryInterface  $factory  The factory.
+	 *
+	 * @see     \Joomla\CMS\MVC\Model\BaseDatabaseModel
+	 * @since   4.0


__DEPLOY_VERSION__ Repeat below.

Quy · 2018-06-21T03:02:13Z

components/com_finder/View/Search/HtmlView.php


 		// Log the search
-		SearchHelper::logSearch($this->query->input, 'com_finder');
+		FinderHelper::logSearch($this->query, count($this->total));


$this->total is an integer thus the warning message below:

Warning: count(): Parameter must be an array or an object that implements Countable in \components\com_finder\View\Search\HtmlView.php on line 163

…4finder_log

Hackwar · 2018-06-21T08:17:58Z

@Quy fixed the bugs.

carlitorweb · 2018-06-26T17:57:28Z

Then, my local environment no was so crazy when I had this warning ;) .

carlitorweb · 2018-06-26T19:41:56Z

All work ok now.

carlitorweb · 2018-06-26T19:43:39Z

I have tested this item ✅ successfully on 780205e

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20681.}

…4finder_log # Conflicts: # components/com_finder/View/Search/HtmlView.php

TobsBobs · 2018-07-07T15:50:02Z

TobsBobs · 2018-07-07T15:50:38Z

I have tested this item ✅ successfully on 7eeab91

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20681.}

ghost · 2018-07-07T17:14:51Z

@carlitorweb can you please retest?

carlitorweb · 2018-07-07T17:16:23Z

I have tested this item ✅ successfully on 7eeab91

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20681.}

ghost · 2018-07-07T17:27:49Z

Ready to Commit after two successful tests.

csthomas · 2018-07-23T15:35:10Z

administrator/components/com_admin/sql/updates/mysql/4.0.0-2018-06-06.sql

+  `hits` INT(11) NOT NULL DEFAULT '1',
+  `results` INT(11) NOT NULL DEFAULT '0',
+  UNIQUE INDEX `md5sum` (`md5sum`),
+  INDEX `searchterm` (`searchterm`)


Index has to be limited to the first 191 characters. See issue #21235

com_finder: add search statistics

edf1cd3

Hackwar requested review from brianteeman and mbabker as code owners June 6, 2018 21:28

joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Jun 6, 2018

This was referenced Jun 6, 2018

[4.0] Removing com_search #20637

Merged

Smart Search results are displayed in the wrong place #17437

Closed

Comments and codestyle

24b6b01

brianteeman reviewed Jun 8, 2018

View reviewed changes

Hackwar added 2 commits June 12, 2018 11:49

Adding . in language string

ff57a95

carlitorweb reviewed Jun 18, 2018

View reviewed changes

Fixing logging empty searches

79441d5

Typo

add104e

Merge branch '4.0-dev' into j4finder_log

736d005

Quy reviewed Jun 19, 2018

View reviewed changes

Hackwar added 2 commits June 20, 2018 09:32

Adding different codes for label and description

a25c0e5

Adding language strings

82ae14a

Hackwar added 2 commits June 20, 2018 14:02

Merge branch '4.0-dev' of https://github.com/joomla/joomla-cms into j…

c8558c4

…4finder_log

Merge branch 'j4finder_log' of github.com:Hackwar/joomla-cms into j4f…

8ed03d7

…inder_log

Quy reviewed Jun 21, 2018

View reviewed changes

Hackwar added 2 commits June 21, 2018 10:12

Merge branch '4.0-dev' of https://github.com/joomla/joomla-cms into j…

5f7e4ff

…4finder_log

Fixing total count, fixing comments, fixing filename

9e1f71a

Merge branch '4.0-dev' into j4finder_log

780205e

Hackwar added 3 commits July 1, 2018 18:55

Merge branch '4.0-dev' into j4finder_log

3c7781a

Merge branch '4.0-dev' into j4finder_log

9f617e9

Merge branch '4.0-dev' of https://github.com/joomla/joomla-cms into j…

7eeab91

…4finder_log # Conflicts: # components/com_finder/View/Search/HtmlView.php

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 7, 2018

wilsonge merged commit f41772c into joomla:4.0-dev Jul 7, 2018

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 7, 2018

Hackwar mentioned this pull request Jul 7, 2018

[4.0] Reworking result layout of com_finder #20572

Merged

zero-24 added this to the Joomla 4.0 milestone Jul 7, 2018

csthomas reviewed Jul 23, 2018

View reviewed changes

infograf768 mentioned this pull request Jul 24, 2018

[4.0] Alpha 4 Installation problem Index column size too large. The maximum column size is 767 bytes. #21235

Closed

Hackwar deleted the j4finder_log branch April 27, 2019 08:11

Uh oh!

Comments

Conversation

Hackwar commented Jun 6, 2018 • edited by joomla-cms-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlitorweb commented Jun 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hackwar commented Jun 18, 2018

Uh oh!

carlitorweb commented Jun 18, 2018

Uh oh!

Hackwar commented Jun 18, 2018

Uh oh!

carlitorweb commented Jun 18, 2018

Uh oh!

carlitorweb commented Jun 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hackwar commented Jun 19, 2018

Uh oh!

carlitorweb commented Jun 19, 2018

Uh oh!

carlitorweb commented Jun 19, 2018

Uh oh!

Quy Jun 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hackwar commented Jun 20, 2018

Uh oh!

Hackwar commented Jun 20, 2018

Uh oh!

carlitorweb commented Jun 20, 2018

Uh oh!

carlitorweb commented Jun 20, 2018

Uh oh!

Quy commented Jun 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Quy Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hackwar commented Jun 21, 2018

Uh oh!

carlitorweb commented Jun 26, 2018

Uh oh!

carlitorweb commented Jun 26, 2018

Uh oh!

carlitorweb commented Jun 26, 2018

Uh oh!

TobsBobs commented Jul 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobsBobs commented Jul 7, 2018

Uh oh!

ghost commented Jul 7, 2018

Uh oh!

carlitorweb commented Jul 7, 2018

Uh oh!

ghost commented Jul 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Hackwar commented Jun 6, 2018 •

edited by joomla-cms-bot

Loading

carlitorweb commented Jun 18, 2018 •

edited

Loading

carlitorweb commented Jun 18, 2018 •

edited

Loading

Quy Jun 19, 2018 •

edited

Loading

Quy Jun 21, 2018 •

edited

Loading

TobsBobs commented Jul 7, 2018 •

edited

Loading