Option to stop robots inc banners views #22339

jurihahn · 2018-09-23T13:07:08Z

Pull Request for Issue # .

Summary of Changes

Option to stop increment count of banner views if search engine is detected. Count views as before by default for BC.

Testing Instructions

Coder review
OR

Create banner and open page with this banner. Check if banner view count increment.
Set option "Robots Increment Impressions" to "NO"
Reload page with banner. Check if banner impressions count increment. Must be +1.
Change user-agent to anything includes "Googlebot".
Reload page with banner. Check if banner impressions count increment. Must be same as before.
PS: Its can be tested local too. For example in Firefox you can open Developer tools and start Responsive/Mobile view, then add new Device with any user agent.

Expected result

If set option "Robots increment impressions" to "NO", Module should not increment banner views if search engine detected

Actual result

Module increments count of banner impressions always

Documentation Changes Required

No?

Quy · 2018-09-23T13:27:44Z

language/en-GB/en-GB.mod_banners.ini

@@ -26,3 +26,5 @@ MOD_BANNERS_FIELD_TARGET_LABEL="Target"
 MOD_BANNERS_VALUE_STICKYORDERING="Pinned, Ordering"
 MOD_BANNERS_VALUE_STICKYRANDOMISE="Pinned, Randomise"
 MOD_BANNERS_XML_DESCRIPTION="The Banner Module displays the active Banners from the Component."
+MOD_BANNERS_FIELD_ROBOTS_VIEWS_DOIMPRESS_LABEL="Robots increment views"


Sort keys in alpha order.

Quy · 2018-09-23T13:35:50Z

language/en-GB/en-GB.mod_banners.ini

@@ -19,6 +19,8 @@ MOD_BANNERS_FIELD_HEADER_DESC="Text or HTML to display before the group of banne
 MOD_BANNERS_FIELD_HEADER_LABEL="Header Text"
 MOD_BANNERS_FIELD_RANDOMISE_DESC="Randomise the ordering of the banners."
 MOD_BANNERS_FIELD_RANDOMISE_LABEL="Randomise"
+MOD_BANNERS_FIELD_ROBOTS_VIEWS_DOIMPRESS_LABEL="Robots increment views"


Capitalize words Robots Increment Views

Should it be Impressions?

Quy · 2018-09-23T13:40:41Z

language/en-GB/en-GB.mod_banners.ini

-MOD_BANNERS_FIELD_ROBOTS_VIEWS_DOIMPRESS_LABEL="Robots increment views"
-MOD_BANNERS_FIELD_ROBOTS_VIEWS_DOIMPRESS_DESC="Search engines increase the number of banner views"
+MOD_BANNERS_FIELD_ROBOTS_VIEWS_DOIMPRESS_LABEL="Robots Increment Impressions"
+MOD_BANNERS_FIELD_ROBOTS_VIEWS_DOIMPRESS_DESC="Search engines increase the number of banner Impressions"


lower case impressions

SharkyKZ · 2018-09-23T13:45:04Z

language/en-GB/en-GB.mod_banners.ini

@@ -19,6 +19,8 @@ MOD_BANNERS_FIELD_HEADER_DESC="Text or HTML to display before the group of banne
 MOD_BANNERS_FIELD_HEADER_LABEL="Header Text"
 MOD_BANNERS_FIELD_RANDOMISE_DESC="Randomise the ordering of the banners."
 MOD_BANNERS_FIELD_RANDOMISE_LABEL="Randomise"
+MOD_BANNERS_FIELD_ROBOTS_VIEWS_DOIMPRESS_LABEL="Robots Increment Impressions"
+MOD_BANNERS_FIELD_ROBOTS_VIEWS_DOIMPRESS_DESC="Search engines increase the number of banner impressions"


Add period at the end of the sentence.

brianteeman · 2018-09-23T16:48:17Z

Can someone stop to think if this wording accurately describes what it is intended to do. For example there is no mention that this is "excluding search engine generated 'clicks' from the hit count". It just says it excludes robots which is not technically accurate.

Finally I am still not convinced that this is a useful addition to the core of joomla and is better placed as an extension. If this is accepted then we should have the same option everywhere that we record a "hit"

jurihahn · 2018-09-23T17:07:58Z

Sorry, i think this is not the same. For the banners it's more important. If i see statistics of one of my sites:
real user make about 3 000 page views/impressions per day
search engines make about 13 000 !!! And all can be excluded...
If i want sell views/impressions, i can't..

Why can't we make it more accurate? This is only an option...

Make better wording is always welcome.

and maybe i do not understand all what you say, but here is nothing to exclude "click", only views/impressions

jurihahn · 2018-09-23T17:57:21Z

Finally I am still not convinced that this is a useful addition to the core of joomla and is better placed as an extension. If this is accepted then we should have the same option everywhere that we record a "hit"

Really? Why joomla have this in the core? Why we should create new extension for only one option? Joomla banners component is not perfect, but we can make it better.
It is Joomla politic? Make Joomla extensions not good and say "install third party extension they work better".

infograf768 · 2018-09-24T12:39:24Z

The wording is indeed confusing. The feature, in its simplicity, looks OK to me. I see no reason to refuse it.

jurihahn · 2018-09-24T13:02:05Z

The wording is indeed confusing.

I will be glad to change the text to more suitable. Someone will advise more suitable texts?
Maybe like this.
Label: Search engines increase banner impressions count
Description: If set to NO, detected search engines would not increase impressions count of banner.

infograf768 · 2018-09-24T14:28:42Z

I must say I did not test the code and results, though. Therefore it needs testing as I work local.

SharkyKZ · 2018-09-24T14:33:16Z

Regarding usability, I can't think of a scenario where user would want this in one module but not in another. Perhaps this should be set globally in com_banners instead of individually in each banner module?

jurihahn · 2018-09-24T17:39:38Z

I must say I did not test the code and results, though. Therefore it needs testing as I work local.

Its can be tested local too. For example in Firefox you can open Developer tools and start Responsive/Mobile view, then add new Device with any user agent.

Regarding usability, I can't think of a scenario where user would want this in one module but not in another. Perhaps this should be set globally in com_banners instead of individually in each banner module?

yes, I agree.

Quy · 2018-09-24T19:09:57Z

administrator/components/com_banners/config.xml

@@ -32,6 +32,18 @@
 			<option value="1">JYES</option>
 			<option value="0">JNO</option>
 		</field>
+


Remove tabs

Quy · 2018-09-24T19:12:18Z

modules/mod_banners/helper.php

@@ -32,6 +33,7 @@ public static function &getList(&$params)
 		$document = JFactory::getDocument();
 		$app      = JFactory::getApplication();
 		$keywords = explode(',', $document->getMetaData('keywords'));
+		$config = ComponentHelper::getParams('com_banners');


jurihahn · 2018-09-26T15:07:18Z

I have tested this PR with #22368 it show very fine result. ~700 impressions per hour before and ~250 impressions per hour after patch.

Maybe i commit one more "option" to ignore click made by robots by the same way? Or should it be different PR?

infograf768 · 2018-09-26T15:52:12Z

Maybe for the strings:
COM_BANNERS_FIELD_TRACKROBOTSIMPRESSION_LABEL="Impressions by Search Engines"
COM_BANNERS_FIELD_TRACKROBOTSIMPRESSION_DESC="If set to NO, prevents search engines clicks from increasing the number of impressions."

Quy · 2018-09-26T15:56:23Z

Be sure to sort the keys in alpha order.

jurihahn · 2018-09-26T18:27:16Z

Be sure to sort the keys in alpha order.

I don't understand where is it wrong?

jurihahn · 2018-09-26T18:32:56Z

COM_BANNERS_FIELD_TRACKROBOTSIMPRESSION_LABEL="Impressions by Search Engines"

This is good!

COM_BANNERS_FIELD_TRACKROBOTSIMPRESSION_DESC="If set to NO, prevents search engines clicks from increasing the number of impressions."

in this PR is only impressions ignoring, not clicks

Quy · 2018-09-26T19:04:04Z

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

@@ -137,6 +137,8 @@ COM_BANNERS_FIELD_TRACKCLICK_DESC="Record the number of clicks on the banners on
 COM_BANNERS_FIELD_TRACKCLICK_LABEL="Track Clicks"
 COM_BANNERS_FIELD_TRACKIMPRESSION_DESC="Record the impressions (views) of the banners on a daily basis."
 COM_BANNERS_FIELD_TRACKIMPRESSION_LABEL="Track Impressions"
+COM_BANNERS_FIELD_TRACKROBOTSIMPRESSION_LABEL="Impressions by Search Engines"
+COM_BANNERS_FIELD_TRACKROBOTSIMPRESSION_DESC="If set to NO, prevents search engines from increasing the number of impressions."


Move line 141 above line 140.

now I understand)
I just watched the new lines were in the right place )))

==>
"Include search engines in the count of impressions"

==>
"Include search engines in the count of impressions"

thanks, it's done!

jurihahn · 2018-09-27T13:48:54Z

@brianteeman what do you think now?

Quy · 2018-09-27T20:46:17Z

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

@@ -137,7 +137,7 @@ COM_BANNERS_FIELD_TRACKCLICK_DESC="Record the number of clicks on the banners on
 COM_BANNERS_FIELD_TRACKCLICK_LABEL="Track Clicks"
 COM_BANNERS_FIELD_TRACKIMPRESSION_DESC="Record the impressions (views) of the banners on a daily basis."
 COM_BANNERS_FIELD_TRACKIMPRESSION_LABEL="Track Impressions"
-COM_BANNERS_FIELD_TRACKROBOTSIMPRESSION_DESC="If set to NO, prevents search engines from increasing the number of impressions."
+COM_BANNERS_FIELD_TRACKROBOTSIMPRESSION_DESC="Include search engines in the count of impressions"


Add a period.

brianteeman

strings are ok

Quy · 2018-09-30T00:00:38Z

I have tested this item ✅ successfully on 2f36648

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

viocassel · 2018-12-25T09:33:55Z

I have tested this item ✅ successfully on 2f36648

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

Quy · 2018-12-25T15:43:38Z

RTC

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

Juri Hahn added 3 commits September 21, 2018 20:53

do not increment impress/views of banners if robot detected

fcd5772

Option for BC by default

ec54040

Merge branch 'staging' into stop-robots-inc-banners-views

d16463e

jurihahn requested a review from brianteeman as a code owner September 23, 2018 13:07

joomla-cms-bot added Language Change This is for Translators PR-staging labels Sep 23, 2018

Quy reviewed Sep 23, 2018

View reviewed changes

Juri Hahn added 2 commits September 23, 2018 15:29

Sort keys in alpha order

9590a40

quotes for parameter name

7d9320f

Quy reviewed Sep 23, 2018

View reviewed changes

Wording

d053079

Quy reviewed Sep 23, 2018

View reviewed changes

wording

61d61a8

SharkyKZ reviewed Sep 23, 2018

View reviewed changes

wording

61b9c07

jurihahn changed the title ~~Stop robots inc banners views~~ Option to stop robots inc banners views Sep 23, 2018

infograf768 added the Feature label Sep 24, 2018

Juri Hahn added 2 commits September 24, 2018 20:07

Option for ignoring robots moved to com_banners

957e6ef

Merge branch 'staging' into stop-robots-inc-banners-views

6ad9a56

Quy reviewed Sep 24, 2018

View reviewed changes

Codestyle

bfe05c1

Merge branch 'staging' into stop-robots-inc-banners-views

fe0a07c

Better wording

d93afd1

Quy reviewed Sep 26, 2018

View reviewed changes

sort keys

6c90507

wording

177fd6b

Quy reviewed Sep 27, 2018

View reviewed changes

Juri Hahn added 2 commits September 28, 2018 06:50

wording

76271b8

Merge branch 'staging' into stop-robots-inc-banners-views

2f36648

brianteeman approved these changes Sep 28, 2018

View reviewed changes

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

mbabker added this to the Joomla 3.9.2 milestone Dec 27, 2018

mbabker merged commit 63d39cf into joomla:staging Dec 27, 2018

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

tecpromotion mentioned this pull request Dec 31, 2018

Option to stop robots inc banners views #22339 joomlagerman/joomla#663

Closed

jurihahn deleted the stop-robots-inc-banners-views branch March 17, 2019 19:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Option to stop robots inc banners views #22339

Option to stop robots inc banners views #22339

jurihahn commented Sep 23, 2018 •

edited

Quy Sep 23, 2018

Quy Sep 23, 2018

Quy Sep 23, 2018

Quy Sep 23, 2018

SharkyKZ Sep 23, 2018

brianteeman commented Sep 23, 2018 •

edited

jurihahn commented Sep 23, 2018

jurihahn commented Sep 23, 2018

infograf768 commented Sep 24, 2018

jurihahn commented Sep 24, 2018

infograf768 commented Sep 24, 2018

SharkyKZ commented Sep 24, 2018 •

edited

jurihahn commented Sep 24, 2018

Quy Sep 24, 2018

Quy Sep 24, 2018

jurihahn commented Sep 26, 2018

infograf768 commented Sep 26, 2018

Quy commented Sep 26, 2018

jurihahn commented Sep 26, 2018

jurihahn commented Sep 26, 2018

Quy Sep 26, 2018

jurihahn Sep 27, 2018 •

edited

brianteeman Sep 27, 2018

jurihahn Sep 27, 2018

jurihahn commented Sep 27, 2018

Quy Sep 27, 2018

brianteeman left a comment

Quy commented Sep 30, 2018

viocassel commented Dec 25, 2018

Quy commented Dec 25, 2018

Option to stop robots inc banners views #22339

Option to stop robots inc banners views #22339

Conversation

jurihahn commented Sep 23, 2018 • edited

Summary of Changes

Testing Instructions

Expected result

Actual result

Documentation Changes Required

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianteeman commented Sep 23, 2018 • edited

jurihahn commented Sep 23, 2018

jurihahn commented Sep 23, 2018

infograf768 commented Sep 24, 2018

jurihahn commented Sep 24, 2018

infograf768 commented Sep 24, 2018

SharkyKZ commented Sep 24, 2018 • edited

jurihahn commented Sep 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jurihahn commented Sep 26, 2018

infograf768 commented Sep 26, 2018

Quy commented Sep 26, 2018

jurihahn commented Sep 26, 2018

jurihahn commented Sep 26, 2018

Choose a reason for hiding this comment

jurihahn Sep 27, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jurihahn commented Sep 27, 2018

Choose a reason for hiding this comment

brianteeman left a comment

Choose a reason for hiding this comment

Quy commented Sep 30, 2018

viocassel commented Dec 25, 2018

Quy commented Dec 25, 2018

jurihahn commented Sep 23, 2018 •

edited

brianteeman commented Sep 23, 2018 •

edited

SharkyKZ commented Sep 24, 2018 •

edited

jurihahn Sep 27, 2018 •

edited