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

More virtualization of MatomoTracker class #103

Closed
Demichev opened this issue Apr 21, 2022 · 6 comments
Closed

More virtualization of MatomoTracker class #103

Demichev opened this issue Apr 21, 2022 · 6 comments

Comments

@Demichev
Copy link
Contributor

Summary

May I ask not popular but small feature? We are using MatomoTracker class to emulate user page visits when payment callback is executed. MatomoTracker is poorly virtualized so I overrode the whole sendRequest() method just to replace proxy (getProxy() method is private), add CURLOPT_HTTPHEADER to curl and ['http']['header'] to $stream_options.
It works but each time Matomo is updated I compare code, make changes, etc.
Can you make getProxy() protected and add empty protected methods for curl options/stream_options?

Your Environment

  • Matomo Version: 4.7.1
@bx80
Copy link
Contributor

bx80 commented Apr 22, 2022

Hi @Demichev, I can't see any obvious problem with exposing some protected methods. It might be a while before this gets prioritized though as we've got a back log of issues.

@Demichev
Copy link
Contributor Author

I understand that it will have a very low priority. Just hope that it requires so small changes that it can be done reasonably soon.

@sgiehl
Copy link
Member

sgiehl commented Apr 25, 2022

@Demichev If you have some time to provide a Pull Request with the required changes, we are happy to review and merge it.
Note: the changes needs to be proposed here: https://github.com/matomo-org/matomo-php-tracker

@Demichev
Copy link
Contributor Author

Pull request is sent. Sorry, I am not sure about coding/comments standards for this project.

@Demichev
Copy link
Contributor Author

Any news here?

@justinvelluppillai justinvelluppillai transferred this issue from matomo-org/matomo May 26, 2022
@justinvelluppillai
Copy link

hi @Demichev - thanks for the PR. I have taken a look and also asked @sgiehl to take a look as he has more experience here, so hopefully we can get this merged next week.

@sgiehl sgiehl closed this as completed May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants