Improves Geolocation diagnostics (#16780)

5 files changed, 83 insertions, 23 deletions

diff --git a/plugins/UserCountry/Controller.php b/plugins/UserCountry/Controller.php
index b3f23886cc..c3d84522aa 100644
--- a/plugins/UserCountry/Controller.php
+++ b/plugins/UserCountry/Controller.php
@@ -11,6 +11,7 @@ namespace Piwik\Plugins\UserCountry;
 use Exception;
 use Piwik\Common;
 use Piwik\IP;
+use Piwik\Notification;
 use Piwik\Piwik;
 use Piwik\Plugin\Manager;
 use Piwik\Plugins\GeoIp2\LocationProvider\GeoIp2;
@@ -45,6 +46,35 @@ class Controller extends \Piwik\Plugin\ControllerAdmin
         $view->thisIP = IP::getIpFromHeader();
         $view->hasGeoIp2Provider = Manager::getInstance()->isPluginActivated('GeoIp2');
 
+        if (!LocationProvider::getCurrentProvider()) {
+            $provider = LocationProvider::getProviderById(LocationProvider::getCurrentProviderId());
+
+            if ($provider) {
+                $message = Piwik::translate('UserCountry_GeolocationProviderBroken', '<strong>' . $provider->getInfo()['title'] . '</strong>');
+            } else {
+                $message = Piwik::translate('UserCountry_GeolocationProviderUnavailable', '<strong>' . LocationProvider::getCurrentProviderId() . '</strong>');
+            }
+
+            $notification          = new Notification($message);
+            $notification->context = Notification::CONTEXT_ERROR;
+            $notification->raw     = true;
+            Notification\Manager::notify('UserCountry_GeoLocationProviderBroken', $notification);
+        } else {
+            $isWorking = LocationProvider::getCurrentProvider()->isWorking();
+            if (true !== $isWorking) {
+                $message = Piwik::translate('UserCountry_GeolocationProviderBroken', '<strong>' . LocationProvider::getCurrentProvider()->getInfo()['title'] . '</strong>');
+
+                if ($isWorking) {
+                    $message .= '<br /><br />' . $isWorking;
+                }
+
+                $notification          = new Notification($message);
+                $notification->context = Notification::CONTEXT_ERROR;
+                $notification->raw     = true;
+                Notification\Manager::notify('UserCountry_GeoLocationProviderBroken', $notification);
+            }
+        }
+
         // check if there is a working provider (that isn't the default one)
         $isThereWorkingProvider = false;
         foreach ($allProviderInfo as $id => $provider) {
diff --git a/plugins/UserCountry/Diagnostic/GeolocationDiagnostic.php b/plugins/UserCountry/Diagnostic/GeolocationDiagnostic.php
index ddb72291b0..00fcb59dab 100644
--- a/plugins/UserCountry/Diagnostic/GeolocationDiagnostic.php
+++ b/plugins/UserCountry/Diagnostic/GeolocationDiagnostic.php
@@ -8,10 +8,9 @@
 namespace Piwik\Plugins\UserCountry\Diagnostic;
 
 use Piwik\Config;
-use Piwik\Plugin\Manager;
+use Piwik\Piwik;
 use Piwik\Plugins\Diagnostics\Diagnostic\Diagnostic;
 use Piwik\Plugins\Diagnostics\Diagnostic\DiagnosticResult;
-use Piwik\Plugins\GeoIp2\LocationProvider\GeoIp2;
 use Piwik\Plugins\UserCountry\LocationProvider;
 use Piwik\Translation\Translator;
 
@@ -32,9 +31,9 @@ class GeolocationDiagnostic implements Diagnostic
 
     public function execute()
     {
-        $isPiwikInstalling = !Config::getInstance()->existsLocalConfig();
-        if ($isPiwikInstalling) {
-            // Skip the diagnostic if Piwik is being installed
+        $isMatomoInstalling = !Config::getInstance()->existsLocalConfig();
+        if ($isMatomoInstalling) {
+            // Skip the diagnostic if Matomo is being installed
             return array();
         }
 
@@ -42,28 +41,36 @@ class GeolocationDiagnostic implements Diagnostic
 
         $currentProviderId = LocationProvider::getCurrentProviderId();
         $allProviders = LocationProvider::getAllProviderInfo();
-        $isNotRecommendedProvider = in_array($currentProviderId, array(
-            LocationProvider\DefaultProvider::ID,
-            GeoIp2\ServerModule::ID));
-        $isProviderInstalled = (isset($allProviders[$currentProviderId]['status']) && $allProviders[$currentProviderId]['status'] == LocationProvider::INSTALLED);
 
-        if (!$isNotRecommendedProvider && $isProviderInstalled) {
-            return array(DiagnosticResult::singleResult($label, DiagnosticResult::STATUS_OK));
+        $providerStatus = $allProviders[$currentProviderId]['status'] ?? LocationProvider::NOT_INSTALLED;
+
+        $providerWarning = $allProviders[$currentProviderId]['usageWarning'] ?? null;
+        $statusMessage = $allProviders[$currentProviderId]['statusMessage'] ?? null;
+
+        if ($providerStatus === LocationProvider::BROKEN) {
+            $message = Piwik::translate('UserCountry_GeolocationProviderBroken', '<strong>' . $allProviders[$currentProviderId]['title'] . '</strong>');
+            if ($statusMessage) {
+                $message .= '<br /><br />' . $statusMessage;
+            }
+            return [DiagnosticResult::singleResult($label, DiagnosticResult::STATUS_ERROR, $message)];
+        }
+
+        if ($providerStatus === LocationProvider::NOT_INSTALLED) {
+            $provider = $allProviders[$currentProviderId] ?? null;
+
+            if ($provider) {
+                $message = Piwik::translate('UserCountry_GeolocationProviderBroken', '<strong>' . $allProviders[$currentProviderId]['title'] . '</strong>');
+            } else {
+                $message = Piwik::translate('UserCountry_GeolocationProviderUnavailable', '<strong>' . LocationProvider::getCurrentProviderId() . '</strong>');
+            }
+
+            return [DiagnosticResult::singleResult($label, DiagnosticResult::STATUS_ERROR, $message)];
         }
 
-        if ($isProviderInstalled) {
-            $comment = $this->translator->translate('GeoIp2_GeoIPLocationProviderNotRecommended') . ' ';
-            $message = 'GeoIp2_LocationProviderDesc_ServerModule2';
-            $comment .= $this->translator->translate($message, array(
-                '<a href="https://matomo.org/docs/geo-locate/" rel="noreferrer noopener" target="_blank">', '', '', '</a>'
-            ));
-        } else {
-            $comment = $this->translator->translate('UserCountry_DefaultLocationProviderDesc1') . ' ';
-            $comment .= $this->translator->translate('UserCountry_DefaultLocationProviderDesc2', array(
-                '<a href="https://matomo.org/docs/geo-locate/" rel="noreferrer noopener" target="_blank">', '', '', '</a>'
-            ));
+        if (!empty($providerWarning)) {
+            return [DiagnosticResult::singleResult($label, DiagnosticResult::STATUS_WARNING, $providerWarning)];
         }
 
-        return array(DiagnosticResult::singleResult($label, DiagnosticResult::STATUS_WARNING, $comment));
+        return [DiagnosticResult::singleResult($label, DiagnosticResult::STATUS_OK)];
     }
 }
diff --git a/plugins/UserCountry/LocationProvider.php b/plugins/UserCountry/LocationProvider.php
index 43cf3557d3..53af2f7447 100644
--- a/plugins/UserCountry/LocationProvider.php
+++ b/plugins/UserCountry/LocationProvider.php
@@ -179,6 +179,16 @@ abstract class LocationProvider
     }
 
     /**
+     * Returns a message that should be shown as diagnostics warning if provider is used
+     *
+     * @return null|string
+     */
+    public function getUsageWarning(): ?string
+    {
+        return null;
+    }
+
+    /**
      * Returns every available provider instance.
      *
      * @return LocationProvider[]
@@ -299,6 +309,7 @@ abstract class LocationProvider
             $info['statusMessage'] = $statusMessage;
             $info['location'] = $location;
             $info['isVisible'] = $provider->isVisible();
+            $info['usageWarning'] = $provider->getUsageWarning();
 
             $allInfo[$info['order']] = $info;
         }
diff --git a/plugins/UserCountry/LocationProvider/DefaultProvider.php b/plugins/UserCountry/LocationProvider/DefaultProvider.php
index 76004243fb..2abfe0098a 100644
--- a/plugins/UserCountry/LocationProvider/DefaultProvider.php
+++ b/plugins/UserCountry/LocationProvider/DefaultProvider.php
@@ -109,5 +109,15 @@ class DefaultProvider extends LocationProvider
             . '</a></p>';
         return array('id' => self::ID, 'title' => self::TITLE, 'description' => $desc, 'order' => 1);
     }
+
+    public function getUsageWarning(): ?string
+    {
+        $comment = Piwik::translate('UserCountry_DefaultLocationProviderDesc1') . ' ';
+        $comment .= Piwik::translate('UserCountry_DefaultLocationProviderDesc2', array(
+            '<a href="https://matomo.org/docs/geo-locate/" rel="noreferrer noopener" target="_blank">', '', '', '</a>'
+        ));
+
+        return $comment;
+    }
 }
 
diff --git a/plugins/UserCountry/lang/en.json b/plugins/UserCountry/lang/en.json
index 4c927f7345..d645375bec 100644
--- a/plugins/UserCountry/lang/en.json
+++ b/plugins/UserCountry/lang/en.json
@@ -21,6 +21,8 @@
         "GeoIPDocumentationSuffix": "In order to see data for this report, you must setup GeoIP in the Geolocation admin tab. The commercial %1$sMaxmind%2$s GeoIP databases are more accurate than the free ones. To see how accurate they are, click %3$shere%4$s.",
         "Geolocation": "Geolocation",
         "GeolocationPageDesc": "On this page you can change how Matomo determines visitor locations.",
+        "GeolocationProviderBroken": "The configured geolocation provider %1$s is broken. Please fix the provider or configure another one to get geolocation working again.",
+        "GeolocationProviderUnavailable": "The configured geolocation provider %1$s is not available anymore, please configure another one.",
         "getCityDocumentation": "This report shows the cities your visitors were in when they accessed your website.",
         "getContinentDocumentation": "This report shows which continent your visitors were in when they accessed your website.",
         "getCountryDocumentation": "This report shows which country your visitors were in when they accessed your website.",