[ios] Bookmark category visibility is now correctly restored on load

- Fixed bookmark tests
- Bookmarks categories should be explicitly saved now after modification

4 files changed, 21 insertions, 23 deletions

diff --git a/map/bookmark.cpp b/map/bookmark.cpp
index 5d7b12a5c0..546a87f68a 100644
--- a/map/bookmark.cpp
+++ b/map/bookmark.cpp
@@ -21,16 +21,11 @@ void BookmarkCategory::AddBookmarkImpl(Bookmark const & bm, double scale)
   p->SetScale(scale);
 
   m_bookmarks.push_back(p);
-
-  // Turn on visibility, so user can see added/replaced bookmark on a map
-  if (!IsVisible())
-    SetVisible(true);
 }
 
 void BookmarkCategory::AddBookmark(Bookmark const & bm, double scale)
 {
   AddBookmarkImpl(bm, scale);
-  VERIFY ( SaveToKMLFile(), () );
 }
 
 void BookmarkCategory::ReplaceBookmark(size_t index, Bookmark const & bm, double scale)
@@ -45,12 +40,6 @@ void BookmarkCategory::ReplaceBookmark(size_t index, Bookmark const & bm, double
     m_bookmarks[index] = p;
 
     delete old;
-
-    // Turn on visibility, so user can see added/replaced bookmark on a map
-    if (!IsVisible())
-      SetVisible(true);
-
-    VERIFY ( SaveToKMLFile(), () );
   }
   else
     LOG(LWARNING, ("Trying to replace non-existing bookmark"));
@@ -73,8 +62,6 @@ void BookmarkCategory::DeleteBookmark(size_t index)
   {
     delete m_bookmarks[index];
     m_bookmarks.erase(m_bookmarks.begin() + index);
-
-    VERIFY ( SaveToKMLFile(), () );
   }
   else
   {
@@ -476,6 +463,7 @@ bool BookmarkCategory::SaveToKMLFile()
   }
   catch (std::exception const & e)
   {
+    LOG(LWARNING, ("Exception while saving bookmarks:", e.what()));
   }
 
   LOG(LWARNING, ("Can't save bookmarks category", m_name, "to file", m_file));
diff --git a/map/framework.cpp b/map/framework.cpp
index 90a49ed7d8..a0ab414f4f 100644
--- a/map/framework.cpp
+++ b/map/framework.cpp
@@ -363,7 +363,7 @@ void Framework::LoadBookmarks()
   }
 }
 
-void Framework::AddBookmark(string const & category, Bookmark const & bm)
+BookmarkCategory * Framework::AddBookmark(string const & category, Bookmark const & bm)
 {
   // Get global non-rotated viewport rect and calculate viewport scale level.
   double const scale = scales::GetScaleLevelD(
@@ -385,7 +385,7 @@ void Framework::AddBookmark(string const & category, Bookmark const & bm)
       if (category == cat->GetName())
       {
         cat->ReplaceBookmark(static_cast<size_t>(index), bm, scale);
-        return;
+        return cat;
       }
       else
       {
@@ -397,6 +397,7 @@ void Framework::AddBookmark(string const & category, Bookmark const & bm)
 
   BookmarkCategory * cat = GetBmCategory(category);
   cat->AddBookmark(bm, scale);
+  return cat;
 }
 
 namespace
diff --git a/map/framework.hpp b/map/framework.hpp
index 6ff0dcb5ca..6138a0eb7c 100644
--- a/map/framework.hpp
+++ b/map/framework.hpp
@@ -187,7 +187,8 @@ public:
   /// Scans and loads all kml files with bookmarks in WritableDir
   void LoadBookmarks();
 
-  void AddBookmark(string const & category, Bookmark const & bm);
+  // Always returns existing or newly created bookmark category
+  BookmarkCategory * AddBookmark(string const & category, Bookmark const & bm);
   inline size_t GetBmCategoriesCount() const { return m_bookmarks.size(); }
   /// @returns 0 if category is not found
   BookmarkCategory * GetBmCategory(size_t index) const;
diff --git a/map/map_tests/bookmarks_test.cpp b/map/map_tests/bookmarks_test.cpp
index 1b995f134d..c30f596c56 100644
--- a/map/map_tests/bookmarks_test.cpp
+++ b/map/map_tests/bookmarks_test.cpp
@@ -188,9 +188,13 @@ UNIT_TEST(Bookmarks_Getting)
   // This is not correct because Framework::OnSize doesn't work until SetRenderPolicy is called.
   //TEST(m2::AlmostEqual(m2::PointD(400, 200), pixC), (pixC));
 
-  fm.AddBookmark("cat1", Bookmark(m2::PointD(38, 20), "1", "placemark-red"));
-  fm.AddBookmark("cat2", Bookmark(m2::PointD(41, 20), "2", "placemark-red"));
-  fm.AddBookmark("cat3", Bookmark(m2::PointD(41, 40), "3", "placemark-red"));
+  BookmarkCategory const * c1 = fm.AddBookmark("cat1", Bookmark(m2::PointD(38, 20), "1", "placemark-red"));
+  BookmarkCategory const * c2 = fm.AddBookmark("cat2", Bookmark(m2::PointD(41, 20), "2", "placemark-red"));
+  BookmarkCategory const * c3 = fm.AddBookmark("cat3", Bookmark(m2::PointD(41, 40), "3", "placemark-red"));
+
+  TEST_NOT_EQUAL(c1, c2, ());
+  TEST_NOT_EQUAL(c2, c3, ());
+  TEST_NOT_EQUAL(c1, c3, ());
 
   (void)fm.GetBmCategory("notExistingCat");
   TEST_EQUAL(fm.GetBmCategoriesCount(), 4, ());
@@ -215,7 +219,9 @@ UNIT_TEST(Bookmarks_Getting)
   TEST_EQUAL(bm->GetType(), "placemark-red", ());
 
   // This one should replace previous bookmark
-  fm.AddBookmark("cat3", Bookmark(m2::PointD(41, 40), "4", "placemark-blue"));
+  BookmarkCategory const * c33 = fm.AddBookmark("cat3", Bookmark(m2::PointD(41, 40), "4", "placemark-blue"));
+
+  TEST_EQUAL(c33, c3, ());
 
   res = fm.GetBookmark(fm.GtoP(m2::PointD(41, 40)), 1.0);
   TEST(IsValid(res), ());
@@ -312,7 +318,7 @@ UNIT_TEST(Bookmarks_AddingMoving)
   m2::PointD const globalPoint = m2::PointD(40, 20);
   m2::PointD const pixelPoint = fm.GtoP(globalPoint);
 
-  fm.AddBookmark(categoryOne, Bookmark(globalPoint, "name", "placemark-red"));
+  BookmarkCategory const * c1 = fm.AddBookmark(categoryOne, Bookmark(globalPoint, "name", "placemark-red"));
   BookmarkAndCategory res = fm.GetBookmark(pixelPoint, 1.0);
   TEST(IsValid(res), ());
   TEST_EQUAL(res.second, 0, ());
@@ -320,7 +326,8 @@ UNIT_TEST(Bookmarks_AddingMoving)
   TEST_EQUAL(fm.GetBmCategory(res.first)->GetName(), categoryOne, ());
 
   // Edit the name and type of bookmark
-  fm.AddBookmark(categoryOne, Bookmark(globalPoint, "name2", "placemark-blue"));
+  BookmarkCategory const * c11 = fm.AddBookmark(categoryOne, Bookmark(globalPoint, "name2", "placemark-blue"));
+  TEST_EQUAL(c1, c11, ());
   res = fm.GetBookmark(pixelPoint, 1.0);
   TEST(IsValid(res), ());
   TEST_EQUAL(res.second, 0, ());
@@ -331,7 +338,8 @@ UNIT_TEST(Bookmarks_AddingMoving)
   TEST_EQUAL(pBm->GetType(), "placemark-blue", ());
 
   // Edit name, type and category of bookmark
-  fm.AddBookmark(categoryTwo, Bookmark(globalPoint, "name3", "placemark-green"));
+  BookmarkCategory const * c2 = fm.AddBookmark(categoryTwo, Bookmark(globalPoint, "name3", "placemark-green"));
+  TEST_NOT_EQUAL(c1, c2, ());
   TEST_EQUAL(fm.GetBmCategoriesCount(), 2, ());
   res = fm.GetBookmark(pixelPoint, 1.0);
   TEST(IsValid(res), ());