From 65b745a1dcbe361cb7f298105383b827b94c23d6 Mon Sep 17 00:00:00 2001
From: Arsentiy Milchakov <milcars@mapswithme.com>
Date: Thu, 28 May 2020 14:24:29 +0300
Subject: [PATCH] [layers][statistics] review fixes

---
 map/traffic_manager.cpp        | 43 ++++++++++++++++++++++------------
 map/traffic_manager.hpp        |  2 ++
 map/transit/transit_reader.cpp | 30 +++++++++++++++---------
 map/transit/transit_reader.hpp |  2 ++
 4 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/map/traffic_manager.cpp b/map/traffic_manager.cpp
index 8bb7ec7f72..9cf8c3d44c 100644
--- a/map/traffic_manager.cpp
+++ b/map/traffic_manager.cpp
@@ -97,6 +97,7 @@ void TrafficManager::SetEnabled(bool enabled)
     Clear();
     ChangeState(enabled ? TrafficState::Enabled : TrafficState::Disabled);
     m_trackFirstSchemeData = enabled;
+    m_lastTrackedStatus = {};
   }
 
   m_drapeEngine.SafeCall(&df::DrapeEngine::EnableTraffic, enabled);
@@ -513,7 +514,6 @@ void TrafficManager::UpdateState()
       mwmVersions.insert(mwmId.GetInfo()->GetVersion());
   }
 
-  auto const previousState = m_state.load();
   if (networkError || maxPassedTime >= kNetworkErrorTimeout)
     ChangeState(TrafficState::NetworkError);
   else if (waiting)
@@ -529,8 +529,7 @@ void TrafficManager::UpdateState()
   else
     ChangeState(TrafficState::Enabled);
 
-  if (previousState != m_state)
-    TrackStatistics(mwmVersions);
+  TrackStatistics(mwmVersions);
 }
 
 void TrafficManager::ChangeState(TrafficState newState)
@@ -579,23 +578,37 @@ void TrafficManager::SetSimplifiedColorScheme(bool simplified)
 
 void TrafficManager::TrackStatistics(std::set<int64_t> const & mwmVersions)
 {
-  if (m_trackFirstSchemeData)
+  if (!m_trackFirstSchemeData)
+    return;
+
+  std::optional<LayersStatistics::Status> statisticStatus;
+  if (m_state == TrafficState::Enabled)
   {
-    if (m_state == TrafficState::Enabled)
+    if (mwmVersions.empty())
+    {
+      statisticStatus = LayersStatistics::Status::Unavailable;
+    }
+    else
     {
       m_trackFirstSchemeData = false;
-      m_statistics.LogActivate(LayersStatistics::Status::Success, mwmVersions);
-    }
-    else if (m_state == TrafficState::NetworkError)
-    {
-      m_statistics.LogActivate(LayersStatistics::Status::Error, mwmVersions);
-    }
-    else if (m_state == TrafficState::NoData || m_state == TrafficState::ExpiredData ||
-             m_state == TrafficState::ExpiredApp)
-    {
-      m_statistics.LogActivate(LayersStatistics::Status::Unavailable, mwmVersions);
+      statisticStatus = LayersStatistics::Status::Success;
     }
   }
+  else if (m_state == TrafficState::NetworkError)
+  {
+    statisticStatus = LayersStatistics::Status::Error;
+  }
+  else if (m_state == TrafficState::NoData || m_state == TrafficState::ExpiredData ||
+           m_state == TrafficState::ExpiredApp)
+  {
+    statisticStatus = LayersStatistics::Status::Unavailable;
+  }
+
+  if (!statisticStatus || m_lastTrackedStatus == statisticStatus)
+    return;
+
+  m_lastTrackedStatus = statisticStatus;
+  m_statistics.LogActivate(*statisticStatus, mwmVersions);
 }
 
 std::string DebugPrint(TrafficManager::TrafficState state)
diff --git a/map/traffic_manager.hpp b/map/traffic_manager.hpp
index 768d6846e5..a6061c736f 100644
--- a/map/traffic_manager.hpp
+++ b/map/traffic_manager.hpp
@@ -24,6 +24,7 @@
 #include <cstdint>
 #include <map>
 #include <mutex>
+#include <optional>
 #include <set>
 #include <string>
 #include <utility>
@@ -190,6 +191,7 @@ private:
   threads::SimpleThread m_thread;
 
   bool m_trackFirstSchemeData = false;
+  std::optional<LayersStatistics::Status> m_lastTrackedStatus;
   LayersStatistics m_statistics;
 };
 
diff --git a/map/transit/transit_reader.cpp b/map/transit/transit_reader.cpp
index 06a2e696d5..9204c7b0de 100644
--- a/map/transit/transit_reader.cpp
+++ b/map/transit/transit_reader.cpp
@@ -184,6 +184,7 @@ void TransitReadManager::EnableTransitSchemeMode(bool enable)
     return;
   m_isSchemeMode = enable;
   m_trackFirstSchemeData = enable;
+  m_lastTrackedStatus = {};
 
   m_drapeEngine.SafeCall(&df::DrapeEngine::EnableTransitScheme, enable);
 
@@ -436,17 +437,24 @@ void TransitReadManager::ChangeState(TransitSchemeState newState)
 
 void TransitReadManager::TrackStatistics(std::set<int64_t> const & mwmVersions)
 {
-  if (m_trackFirstSchemeData)
+  if (!m_trackFirstSchemeData)
+    return;
+
+  LayersStatistics::Status statisticStatus;
+  if (m_state == TransitSchemeState::Enabled && !mwmVersions.empty())
   {
-    if (m_state == TransitSchemeState::Enabled)
-    {
-      eye::Eye::Event::LayerShown(eye::Layer::Type::PublicTransport);
-      m_trackFirstSchemeData = false;
-      m_statistics.LogActivate(LayersStatistics::Status::Success, mwmVersions);
-    }
-    else
-    {
-      m_statistics.LogActivate(LayersStatistics::Status::Unavailable, mwmVersions);
-    }
+    eye::Eye::Event::LayerShown(eye::Layer::Type::PublicTransport);
+    m_trackFirstSchemeData = false;
+    statisticStatus = LayersStatistics::Status::Success;
   }
+  else
+  {
+    statisticStatus = LayersStatistics::Status::Unavailable;
+  }
+
+  if (statisticStatus == m_lastTrackedStatus)
+    return;
+
+  m_lastTrackedStatus = statisticStatus;
+  m_statistics.LogActivate(statisticStatus, mwmVersions);
 }
diff --git a/map/transit/transit_reader.hpp b/map/transit/transit_reader.hpp
index 983c620260..322d0bfbb8 100644
--- a/map/transit/transit_reader.hpp
+++ b/map/transit/transit_reader.hpp
@@ -156,6 +156,8 @@ private:
   bool m_isSchemeMode = false;
   bool m_isSchemeModeBlocked = false;
   std::pair<ScreenBase, bool> m_currentModelView = {ScreenBase(), false /* initialized */};
+
   bool m_trackFirstSchemeData = false;
+  std::optional<LayersStatistics::Status> m_lastTrackedStatus;
   LayersStatistics m_statistics;
 };