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
Allow use of GeoLite2-ASN database as ISP database #12874
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, some system tests appear to be failing now.
Unrelated to this PR, but noticed in https://github.com/matomo-org/matomo/blob/3.x-dev/plugins/GeoIp2/GeoIp2.php#L26, we're checking if the current location provider is an instance of the |
Tested locally, works, just need to fix the tests. |
@diosmosis fixed the check and updated the tests. |
Hey, this be available in the next release, right? |
@mackuba this should be in 3.5.1, so should be in the latest release. |
@diosmosis ah, right, I've looked in the code on the server and the changes are there. Is it necessary to enable the Provider plugin to see ISPs on the location reports page? I've dug into the code a little bit, and it looks like without the Provider plugin the |
Looks like the plugin has to be installed for the column to be in the table and for the provider to be looked up: https://github.com/matomo-org/matomo/blob/3.x-dev/plugins/UserCountry/Columns/Provider.php#L31. I think it also needs to be enabled in order to display the report. The reverse DNS logic shouldn't execute if the provider information is obtained via GeoIP2 (I believe). I think otherwise you have to be using a Maxmind GeoIP implementation and the ASN database has to be named 'GeoLite2-ASN.mmdb' or 'GeoIP2-ISP.mmdb'. |
Great, thanks! |
* Adds possibility to use GeoLite2-ASN database as ISP database * Adds some tests for GeoIP2 files * fix tests * fix check for GeoIp2 provider
fixes #12871