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

Add ability to access XHR object in the callback of trackPageView #13665

Closed
wants to merge 5 commits into from

Conversation

adityasharma7
Copy link

implements #13662

@tsteur
Copy link
Member

tsteur commented Nov 2, 2018

@adityasharma7 be still good to describe your use case to better understand things. And be aware that if image get request is used, the xhr may not be set.

@adityasharma7
Copy link
Author

@tsteur Thanks for your quick response.
As stated on the issue we have another non Matomo system and when we track any page with Matomo some of the data extracted by Piwik.js will also be sent to a different system that may be able to give a quick glimpse about the tracking.
I explored about the getImage() method, it overrides onload so xhr object is there in that case. Though, thanks for pointing that.

@tsteur
Copy link
Member

tsteur commented Nov 4, 2018

Cheers, so it is mainly the request url you are interested in? The PR looks good and can be merged, it's just to understand the use case so I can check if there's maybe a better way to do it that ensures better backwards compatibility in the future etc.

@adityasharma7
Copy link
Author

Yes, exactly only the request url

@tsteur
Copy link
Member

tsteur commented Nov 6, 2018

Could we maybe pass request then? This way it would also work in getImage() method for example.

@tsteur
Copy link
Member

tsteur commented Nov 8, 2018

In Matomo 3.8 we're adding better support for sendBeacon and it can be used by default see https://github.com/matomo-org/matomo/pull/13451/files#diff-1279d666063b65e6d6777f902d11574fR3766 . As there is no XHR object, we need to pass request url instead in the callback so it works for all kind of methods.

@adityasharma7
Copy link
Author

Indeed, request seems a much better solution.

Though I found another way which I am uncertain of a right solution. There is a method getRequest() available with piwikTracker which can be called in the callback. I can get all the parameters except the page title. I digged further, as it is a generic method the request string is initialized with specific parameters for various applications.

Is it the right way? If no I can use the above method till the changes you suggested are available with the release. if yes, we won't need a change.

Looking forward to Matomo 3.8 with https://github.com/matomo-org/matomo/pull/13451/files#diff-1279d666063b65e6d6777f902d11574fR3766 :)

Thanks for sharing that.

One more thing came to my mind what if we make another tracker and use it to send the request to a non matomo server, is it a valid solution?

@tsteur
Copy link
Member

tsteur commented Nov 11, 2018

Calling getRequest would work to get all the parameters (it wouldn't include the idgoal=X though when you track a goal). I just realize it seems there is no getDocumentTitle available which be useful to add. Also realising it would be actually great to pass the tracker instance in the callback as first argument... then optionally we could also pass request variable.

@mattab mattab added this to the 3.9.0 milestone Nov 13, 2018
@mattab mattab added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Nov 13, 2018
@adityasharma7
Copy link
Author

adityasharma7 commented Nov 13, 2018

I just realize it seems there is no getDocumentTitle available which be useful to add.

+1. I will see if I can contribute.

Also realising it would be actually great to pass the tracker instance in the callback as first argument... then optionally we could also pass request variable.

Sounds promising. I will update the code soon

@tsteur
Copy link
Member

tsteur commented Dec 13, 2018

@adityasharma7 I needed the feature now myself so added it here: #13855

Cheers for your PR and the suggestion 👍 and also for letting me better understand why people use the callback 👍

@tsteur tsteur closed this Dec 13, 2018
@tsteur tsteur removed this from the 3.9.0 milestone Dec 13, 2018
@adityasharma7
Copy link
Author

@tsteur That's great!!!!
Thanks for putting your time into it!
Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants