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 IP2Location to recommended geolocation providers and fix a possib… #12711

Conversation

MichaelRoosz
Copy link
Contributor

Possible bug fix: As I understand it, the "$currentProviderId ==" code part is a possible typo and should not be there.

Changed: Added IP2Location as a recommended geolocation provider. It is a actively maintained plugin that provides good location data.

@fdellwing
Copy link
Contributor

Please see #12699

@sgiehl
Copy link
Member

sgiehl commented Apr 21, 2018

@MichaelHeerklotz thanks for the PR. The "fix" is already included in #12699.
I'm not sure if we should add third party providers as recommended "hard-coded" within Matomo.
But maybe we could make it possible to define additional recommended providers in config?
@mattab do you think it would make sense to define recommended providers in config.php using DI? If so I could add those changes to #12699

@mattab
Copy link
Member

mattab commented Apr 23, 2018

The goal of the system check is to point out potential problems with the GeoIp providers that we offer by default. If the user has taken decision to use a Geolocation provider from the Marketplace (such as IP2Location), then the system check should be green. It's up to the third-party geolocation service to mention any limitation and problems on their page. Therefore, I suggest to automatically mark as Green the system diagnostic, when a third party location system is used.

@sgiehl could you maybe make this change in core so the system check works for all third party geolocation providers?

@mattab mattab added this to the 3.5.0 milestone Apr 23, 2018
@mattab mattab closed this Apr 23, 2018
@mattab
Copy link
Member

mattab commented Apr 23, 2018

Thanks for the suggestion @MichaelHeerklotz
@sgiehl let's make the change to accept all third party geolocation providers in a separate PR

@sgiehl
Copy link
Member

sgiehl commented Apr 23, 2018

@mattab I will include it in #12699 to avoid later marge conflicts as I already changed some stuff there...

@MichaelRoosz MichaelRoosz deleted the update_GeolocationDiagnostic branch October 29, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants