Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent call page view event with mt() #6782

Open
wants to merge 5 commits into
base: staging
from

Conversation

4 participants
@kuzmany
Copy link
Contributor

kuzmany commented Oct 25, 2018

Please be sure you are submitting this against the staging branch.

Q A
Bug fix?
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

We can call mt() multiple times. At the moment Mautic support just send/pageview event.
Standard call with tracking code

mt('send', 'pageview');

But you can call it more time on page. If you call mt(); Mautic send page hit,
What is issue for me If we want use mt() for our own events. For example mt('send', 'click');

Steps to reproduce the bug:

  1. Add tracking code to to page
  2. Load page. Verify If tracking code is loaded properly
  3. Then open browser console and call mt('send', 'pageview'); . Page hit is fired again
  4. Then call mt(); and page hit is fired again

image

Steps to test this PR:

  1. Repeat all steps.
  2. Just mt('send', 'pageview'); should call page hit in browser console

@kuzmany kuzmany added this to the 2.15.0 milestone Oct 25, 2018

@kuzmany kuzmany added this to To do in Testing 2.15.0 Oct 25, 2018

@kuzmany kuzmany changed the title Prevent call mt() with required Prevent call page hit with mt() Oct 25, 2018

@kuzmany kuzmany changed the title Prevent call page hit with mt() Prevent call page view event with mt() Oct 27, 2018

@dongilbert

This comment has been minimized.

Copy link
Member

dongilbert commented Oct 30, 2018

This is a good problem to fix - thanks @kuzmany!

I'd like to see though a function that can perform this check with parameters. Something like:

MauticJS.ensureEventContext = function(event, context0, context1) { // 
    return event.hasOwnProperty('detail')
        && event.detail[0] === context0
        && event.detail[1] === context1;
};

That can be added to the base MauticJS object that in the listener here: https://github.com/mautic/mautic/blob/staging/app/bundles/CoreBundle/EventListener/BuildJsSubscriber.php#L77

That can then be utilized as:

if (MauticJS.ensureEventContext(e, 'send', 'pageview')) {
    m.sendPageview(e.detail);
}

@kuzmany kuzmany added WIP and removed Ready To Test labels Nov 2, 2018

@kuzmany kuzmany added Ready To Test and removed WIP labels Nov 4, 2018

@Woeler Woeler modified the milestones: 2.15.0, 2.15.1 Dec 5, 2018

@Woeler Woeler removed this from To do in Testing 2.15.0 Dec 5, 2018

@heathdutton heathdutton added this to Code Review (2 required) in Mautic 2 Dec 6, 2018

@npracht npracht moved this from Code Review (2 required) to Ready to Test (first time) in Mautic 2 Jan 3, 2019

@alanhartless alanhartless added this to Needs Testing in 2.15.1 Jan 14, 2019

found an issue

Mautic 2 automation moved this from Ready to Test (first time) to Changes Requested / Review Jan 30, 2019

@alanhartless

This comment has been minimized.

Copy link
Contributor

alanhartless commented Jan 30, 2019

@kuzmany I did some tests with this. I found that the first mt('send', 'pageview'); works. But if I call it again in the console, nothing happens. Where in staging, it fires the event again. Is that intended? I think that's a must have for single page websites.

To reproduce, simply try calling mt('send', 'pageview'); in your browser's console a few times and look at that ajax requests made.

This PR:

In staging:

@alanhartless alanhartless moved this from Needs Testing to Discussion in 2.15.1 Jan 30, 2019

@kuzmany

This comment has been minimized.

Copy link
Contributor Author

kuzmany commented Jan 30, 2019

@alanhartless Now should work. Tested on my test eniveroment

image

@kuzmany kuzmany moved this from Changes Requested / Review to Ready to Test (first time) in Mautic 2 Jan 30, 2019

@npracht npracht moved this from Discussion to Needs Testing in 2.15.1 Feb 6, 2019

@alanhartless alanhartless removed this from Needs Testing in 2.15.1 Mar 11, 2019

@alanhartless alanhartless modified the milestones: 2.15.1, 2.16.0 Mar 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.