From 27444c4d8cf1b59a879f928ebe5d92179ef8e7e9 Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Thu, 26 May 2016 18:06:31 +0300 Subject: [PATCH 1/3] Make features as deleted when place doesn't exist is sent. --- drape_frontend/apply_feature_functors.cpp | 9 +++-- drape_frontend/apply_feature_functors.hpp | 2 +- drape_frontend/poi_symbol_shape.cpp | 4 +- drape_frontend/shape_view_params.hpp | 2 +- indexer/index.hpp | 4 +- indexer/osm_editor.cpp | 38 +++++++++++++++++-- indexer/osm_editor.hpp | 7 ++++ .../Classes/Editor/MWMEditorViewController.mm | 3 +- search/mwm_context.cpp | 4 +- 9 files changed, 58 insertions(+), 15 deletions(-) diff --git a/drape_frontend/apply_feature_functors.cpp b/drape_frontend/apply_feature_functors.cpp index 5689b85fd4..1c23b108e7 100644 --- a/drape_frontend/apply_feature_functors.cpp +++ b/drape_frontend/apply_feature_functors.cpp @@ -269,7 +269,7 @@ ApplyPointFeature::ApplyPointFeature(TInsertShapeFn const & insertShape, Feature , m_hasPoint(false) , m_hasArea(false) , m_createdByEditor(false) - , m_deletedInEditor(false) + , m_obsoleteInEditor(false) , m_symbolDepth(dp::minDepth) , m_circleDepth(dp::minDepth) , m_symbolRule(nullptr) @@ -279,10 +279,11 @@ ApplyPointFeature::ApplyPointFeature(TInsertShapeFn const & insertShape, Feature void ApplyPointFeature::operator()(m2::PointD const & point, bool hasArea) { + auto const & editor = osm::Editor::Instance(); m_hasPoint = true; m_hasArea = hasArea; - m_createdByEditor = osm::Editor::IsCreatedFeature(m_id); - m_deletedInEditor = false; //TODO: implement + m_createdByEditor = editor.GetFeatureStatus(m_id) == osm::Editor::FeatureStatus::Created; + m_obsoleteInEditor = editor.GetFeatureStatus(m_id) == osm::Editor::FeatureStatus::Obsolete; m_centerPoint = point; } @@ -378,7 +379,7 @@ void ApplyPointFeature::Finish() params.m_posZ = m_posZ; params.m_hasArea = m_hasArea; params.m_createdByEditor = m_createdByEditor; - params.m_deletedInEditor = m_deletedInEditor; + params.m_obsoleteInEditor = m_obsoleteInEditor; m_insertShape(make_unique_dp(m_centerPoint, params)); } } diff --git a/drape_frontend/apply_feature_functors.hpp b/drape_frontend/apply_feature_functors.hpp index ba7ae590da..b2edc079bd 100644 --- a/drape_frontend/apply_feature_functors.hpp +++ b/drape_frontend/apply_feature_functors.hpp @@ -80,7 +80,7 @@ private: bool m_hasPoint; bool m_hasArea; bool m_createdByEditor; - bool m_deletedInEditor; + bool m_obsoleteInEditor; double m_symbolDepth; double m_circleDepth; SymbolRuleProto const * m_symbolRule; diff --git a/drape_frontend/poi_symbol_shape.cpp b/drape_frontend/poi_symbol_shape.cpp index 0ef9b12d41..4e796950bb 100644 --- a/drape_frontend/poi_symbol_shape.cpp +++ b/drape_frontend/poi_symbol_shape.cpp @@ -117,7 +117,7 @@ void PoiSymbolShape::Draw(ref_ptr batcher, ref_ptrSetPivotZ(m_params.m_posZ); handle->SetExtendingSize(m_params.m_extendingSize); - if (m_params.m_deletedInEditor) + if (m_params.m_obsoleteInEditor) { dp::TextureManager::ColorRegion maskColorRegion; textures->GetColorRegion(kDeletedColorMask, maskColorRegion); @@ -134,7 +134,7 @@ uint64_t PoiSymbolShape::GetOverlayPriority() const // Set up maximum priority for shapes which created by user in the editor. if (m_params.m_createdByEditor) return dp::kPriorityMaskAll; - + // Set up minimal priority for shapes which belong to areas. if (m_params.m_hasArea) return 0; diff --git a/drape_frontend/shape_view_params.hpp b/drape_frontend/shape_view_params.hpp index 56c791bc6b..be8eea5fd8 100644 --- a/drape_frontend/shape_view_params.hpp +++ b/drape_frontend/shape_view_params.hpp @@ -29,7 +29,7 @@ struct PoiSymbolViewParams : CommonViewParams float m_posZ = 0.0f; bool m_hasArea = false; bool m_createdByEditor = false; - bool m_deletedInEditor = false; + bool m_obsoleteInEditor = false; }; struct CircleViewParams : CommonViewParams diff --git a/indexer/index.hpp b/indexer/index.hpp index 7e4fa77153..eb850cafd1 100644 --- a/indexer/index.hpp +++ b/indexer/index.hpp @@ -116,7 +116,9 @@ private: FeatureType feature; switch (m_editor.GetFeatureStatus(mwmID, index)) { - case osm::Editor::FeatureStatus::Deleted: return; + case osm::Editor::FeatureStatus::Deleted: + case osm::Editor::FeatureStatus::Obsolete: + return; case osm::Editor::FeatureStatus::Modified: VERIFY(m_editor.GetEditedFeature(mwmID, index, feature), ()); m_f(feature); diff --git a/indexer/osm_editor.cpp b/indexer/osm_editor.cpp index 9d7a9ee6f0..fa646a62b5 100644 --- a/indexer/osm_editor.cpp +++ b/indexer/osm_editor.cpp @@ -55,6 +55,7 @@ constexpr char const * kXmlMwmNode = "mwm"; constexpr char const * kDeleteSection = "delete"; constexpr char const * kModifySection = "modify"; constexpr char const * kCreateSection = "create"; +constexpr char const * kObsoleteSection = "obsolete"; /// We store edited streets in OSM-compatible way. constexpr char const * kAddrStreetTag = "addr:street"; @@ -165,13 +166,14 @@ void Editor::LoadMapEdits() } } - array, 3> const sections = + array, 4> const sections = {{ {FeatureStatus::Deleted, kDeleteSection}, {FeatureStatus::Modified, kModifySection}, + {FeatureStatus::Obsolete, kObsoleteSection}, {FeatureStatus::Created, kCreateSection} }}; - int deleted = 0, modified = 0, created = 0; + int deleted = 0, obsolete = 0, modified = 0, created = 0; bool needRewriteEdits = false; @@ -230,6 +232,7 @@ void Editor::LoadMapEdits() { case FeatureStatus::Deleted: ++deleted; break; case FeatureStatus::Modified: ++modified; break; + case FeatureStatus::Obsolete: ++obsolete; break; case FeatureStatus::Created: ++created; break; case FeatureStatus::Untouched: ASSERT(false, ()); continue; } @@ -253,7 +256,8 @@ void Editor::LoadMapEdits() // Save edits with new indexes and mwm version to avoid another migration on next startup. if (needRewriteEdits) Save(GetEditorFilePath()); - LOG(LINFO, ("Loaded", modified, "modified,", created, "created and", deleted, "deleted features.")); + LOG(LINFO, ("Loaded", modified, "modified,", + created, "created,", deleted, "deleted and", obsolete, "obsolete features.")); } bool Editor::Save(string const & fullFilePath) const @@ -280,6 +284,7 @@ bool Editor::Save(string const & fullFilePath) const xml_node deleted = mwmNode.append_child(kDeleteSection); xml_node modified = mwmNode.append_child(kModifySection); xml_node created = mwmNode.append_child(kCreateSection); + xml_node obsolete = mwmNode.append_child(kObsoleteSection); for (auto const & index : mwm.second) { FeatureTypeInfo const & fti = index.second; @@ -303,6 +308,7 @@ bool Editor::Save(string const & fullFilePath) const case FeatureStatus::Deleted: VERIFY(xf.AttachToParentNode(deleted), ()); break; case FeatureStatus::Modified: VERIFY(xf.AttachToParentNode(modified), ()); break; case FeatureStatus::Created: VERIFY(xf.AttachToParentNode(created), ()); break; + case FeatureStatus::Obsolete: VERIFY(xf.AttachToParentNode(obsolete), ()); break; case FeatureStatus::Untouched: CHECK(false, ("Not edited features shouldn't be here.")); } } @@ -404,6 +410,8 @@ Editor::SaveResult Editor::SaveEditedFeature(EditableMapObject const & emo) FeatureTypeInfo fti; auto const featureStatus = GetFeatureStatus(fid.m_mwmId, fid.m_index); + ASSERT_NOT_EQUAL(featureStatus, FeatureStatus::Obsolete, ("Obsolete feature cannot be modified")); + bool const wasCreatedByUser = IsCreatedFeature(fid); if (wasCreatedByUser && featureStatus == FeatureStatus::Untouched) { @@ -571,6 +579,9 @@ EditableProperties Editor::GetEditableProperties(FeatureType const & feature) co if (!version::IsSingleMwm(feature.GetID().m_mwmId.GetInfo()->m_version.GetVersion())) return {}; + if (GetFeatureStatus(feature.GetID()) == FeatureStatus::Obsolete) + return {}; + // TODO(mgsergio): Check if feature is in the area where editing is disabled in the config. auto editableProperties = GetEditablePropertiesForTypes(feature::TypesHolder(feature)); @@ -675,7 +686,7 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset switch (fti.m_status) { case FeatureStatus::Untouched: CHECK(false, ("It's impossible.")); continue; - + case FeatureStatus::Obsolete: continue; // Obsolete features will be deleted by OSMers. case FeatureStatus::Created: { XMLFeature feature = fti.m_feature.ToXML(true); @@ -893,6 +904,23 @@ void Editor::Invalidate() m_invalidateFn(); } +void Editor::MarkFeatureAsObsolete(FeatureID const & fid) +{ + auto const featureStatus = GetFeatureStatus(fid); + ASSERT(featureStatus == FeatureStatus::Untouched || + featureStatus == FeatureStatus::Modified, + ("Created or deleted features can't be obsolete")); + + auto & fti = m_features[fid.m_mwmId][fid.m_index]; + // If a feature was modified we can drop all changes since it's now obsolete. + fti.m_feature = *m_getOriginalFeatureFn(fid); + fti.m_status = FeatureStatus::Obsolete; + fti.m_modificationTimestamp = time(nullptr); + + Save(GetEditorFilePath()); + Invalidate(); +} + Editor::Stats Editor::GetStats() const { Stats stats; @@ -966,6 +994,7 @@ void Editor::CreateNote(ms::LatLon const & latLon, FeatureID const & fid, sstr << kPlaceDoesNotExistMessage; if (!note.empty()) sstr << " User comments: \"" << note << '\"'; + MarkFeatureAsObsolete(fid); break; case NoteProblemType::General: sstr << note; @@ -988,6 +1017,7 @@ string DebugPrint(Editor::FeatureStatus fs) { case Editor::FeatureStatus::Untouched: return "Untouched"; case Editor::FeatureStatus::Deleted: return "Deleted"; + case Editor::FeatureStatus::Obsolete: return "Obsolete"; case Editor::FeatureStatus::Modified: return "Modified"; case Editor::FeatureStatus::Created: return "Created"; }; diff --git a/indexer/osm_editor.hpp b/indexer/osm_editor.hpp index 8098ea077d..e3d3070493 100644 --- a/indexer/osm_editor.hpp +++ b/indexer/osm_editor.hpp @@ -48,6 +48,7 @@ public: { Untouched, Deleted, + Obsolete, // The feature is obsolete when is marked for deletion via note. Modified, Created }; @@ -157,6 +158,9 @@ public: }; Stats GetStats() const; + // Don't use this function to determine if a feature in editor was created. + // Use GetFeatureStatus(fid) instead. This function is used when a feature is + // not yet saved and we have to know if it was modified or created. static bool IsCreatedFeature(FeatureID const & fid); private: @@ -167,6 +171,9 @@ private: /// Notify framework that something has changed and should be redisplayed. void Invalidate(); + // Saves a feature in internal storage with FeatureStatus::Obsolete status. + void MarkFeatureAsObsolete(FeatureID const & fid); + FeatureID GenerateNewFeatureId(MwmSet::MwmId const & id); EditableProperties GetEditablePropertiesForTypes(feature::TypesHolder const & types) const; diff --git a/iphone/Maps/Classes/Editor/MWMEditorViewController.mm b/iphone/Maps/Classes/Editor/MWMEditorViewController.mm index 415b73a0db..da9e2f5de4 100644 --- a/iphone/Maps/Classes/Editor/MWMEditorViewController.mm +++ b/iphone/Maps/Classes/Editor/MWMEditorViewController.mm @@ -670,6 +670,7 @@ void registerCellsForTableView(vector const & cells, UITab case osm::Editor::FeatureStatus::Untouched: return L(@"editor_place_doesnt_exist"); case osm::Editor::FeatureStatus::Deleted: + case osm::Editor::FeatureStatus::Obsolete: // TODO(Vlad): Either make a valid button or disable it. NSAssert(false, @"Incorrect feature status!"); return L(@"editor_place_doesnt_exist"); case osm::Editor::FeatureStatus::Modified: @@ -893,7 +894,7 @@ void registerCellsForTableView(vector const & cells, UITab - (void)tryToChangeInvalidStateForCell:(MWMEditorTextTableViewCell *)cell { [self.tableView beginUpdates]; - + NSIndexPath * indexPath = [self.tableView indexPathForCell:cell]; [self.invalidCells removeObject:indexPath]; diff --git a/search/mwm_context.cpp b/search/mwm_context.cpp index b89575d420..d9a066b127 100644 --- a/search/mwm_context.cpp +++ b/search/mwm_context.cpp @@ -21,7 +21,9 @@ bool MwmContext::GetFeature(uint32_t index, FeatureType & ft) const { switch (GetEditedStatus(index)) { - case osm::Editor::FeatureStatus::Deleted: return false; + case osm::Editor::FeatureStatus::Deleted: + case osm::Editor::FeatureStatus::Obsolete: + return false; case osm::Editor::FeatureStatus::Modified: case osm::Editor::FeatureStatus::Created: VERIFY(osm::Editor::Instance().GetEditedFeature(GetId(), index, ft), ()); From 47e1a0e317d6ab28e32f4a295f9b9af7697681b1 Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Thu, 2 Jun 2016 22:19:12 +0300 Subject: [PATCH 2/3] Adjust GUI code. Close PP after object is makred as obsolete. --- .../jni/com/mapswithme/maps/editor/Editor.cpp | 10 +++++----- .../com/mapswithme/maps/editor/Editor.java | 7 ++++--- .../maps/editor/EditorFragment.java | 4 ++++ .../maps/widget/placepage/PlacePageView.java | 3 ++- .../Classes/Editor/MWMEditorViewController.mm | 4 ++-- iphone/Maps/Classes/MWMPlacePageEntity.mm | 4 +++- map/framework.cpp | 20 +++++++++++++++---- map/framework.hpp | 3 +++ 8 files changed, 39 insertions(+), 16 deletions(-) diff --git a/android/jni/com/mapswithme/maps/editor/Editor.cpp b/android/jni/com/mapswithme/maps/editor/Editor.cpp index 6a718dc077..bb5091bb1c 100644 --- a/android/jni/com/mapswithme/maps/editor/Editor.cpp +++ b/android/jni/com/mapswithme/maps/editor/Editor.cpp @@ -405,16 +405,16 @@ Java_com_mapswithme_maps_editor_Editor_nativeCreateMapObject(JNIEnv *, jclass, j JNIEXPORT void JNICALL Java_com_mapswithme_maps_editor_Editor_nativeCreateNote(JNIEnv * env, jclass clazz, jstring text) { - Editor::Instance().CreateNote(g_editableMapObject.GetLatLon(), g_editableMapObject.GetID(), - osm::Editor::NoteProblemType::General, jni::ToNativeString(env, text)); + g_framework->NativeFramework()->CreateNote(g_editableMapObject.GetLatLon(), g_editableMapObject.GetID(), + osm::Editor::NoteProblemType::General, jni::ToNativeString(env, text)); } // static void nativePlaceDoesNotExist(String comment); JNIEXPORT void JNICALL Java_com_mapswithme_maps_editor_Editor_nativePlaceDoesNotExist(JNIEnv * env, jclass clazz, jstring comment) { - Editor::Instance().CreateNote(g_editableMapObject.GetLatLon(), g_editableMapObject.GetID(), - osm::Editor::NoteProblemType::PlaceDoesNotExist, jni::ToNativeString(env, comment)); + g_framework->NativeFramework()->CreateNote(g_editableMapObject.GetLatLon(), g_editableMapObject.GetID(), + osm::Editor::NoteProblemType::PlaceDoesNotExist, jni::ToNativeString(env, comment)); } JNIEXPORT void JNICALL @@ -528,7 +528,7 @@ Java_com_mapswithme_maps_editor_Editor_nativeGetCategory(JNIEnv * env, jclass cl JNIEXPORT jint JNICALL Java_com_mapswithme_maps_editor_Editor_nativeGetMapObjectStatus(JNIEnv * env, jclass clazz) { - return static_cast(osm::Editor::Instance().GetFeatureStatus(g_editableMapObject.GetID().m_mwmId, g_editableMapObject.GetID().m_index)); + return static_cast(osm::Editor::Instance().GetFeatureStatus(g_editableMapObject.GetID())); } JNIEXPORT jboolean JNICALL diff --git a/android/src/com/mapswithme/maps/editor/Editor.java b/android/src/com/mapswithme/maps/editor/Editor.java index 0144da4526..d724312227 100644 --- a/android/src/com/mapswithme/maps/editor/Editor.java +++ b/android/src/com/mapswithme/maps/editor/Editor.java @@ -26,13 +26,14 @@ public final class Editor { // Should correspond to core osm::FeatureStatus. @Retention(RetentionPolicy.SOURCE) - @IntDef({UNTOUCHED, DELETED, MODIFIED, CREATED}) + @IntDef({UNTOUCHED, DELETED, OBSOLETE, MODIFIED, CREATED}) public @interface FeatureStatus {} public static final int UNTOUCHED = 0; public static final int DELETED = 1; - public static final int MODIFIED = 2; - public static final int CREATED = 3; + public static final int OBSOLETE = 2; + public static final int MODIFIED = 3; + public static final int CREATED = 4; private static final AppBackgroundTracker.OnTransitionListener sOsmUploader = new AppBackgroundTracker.OnTransitionListener() { diff --git a/android/src/com/mapswithme/maps/editor/EditorFragment.java b/android/src/com/mapswithme/maps/editor/EditorFragment.java index eeac05af1e..f7f252e9fa 100644 --- a/android/src/com/mapswithme/maps/editor/EditorFragment.java +++ b/android/src/com/mapswithme/maps/editor/EditorFragment.java @@ -431,6 +431,8 @@ public class EditorFragment extends BaseMwmFragment implements View.OnClickListe break; case Editor.DELETED: throw new IllegalStateException("Can't delete already deleted feature."); + case Editor.OBSOLETE: + throw new IllegalStateException("Obsolete objects cannot be reverted now."); } } @@ -455,6 +457,8 @@ public class EditorFragment extends BaseMwmFragment implements View.OnClickListe break; case Editor.DELETED: throw new IllegalStateException("Can't delete already deleted feature."); + case Editor.OBSOLETE: + throw new IllegalStateException("Obsolete objects cannot be reverted now."); } } diff --git a/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java b/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java index acf3a44a57..0ba8861ad0 100644 --- a/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java +++ b/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java @@ -687,7 +687,8 @@ public class PlacePageView extends RelativeLayout implements View.OnClickListene else { UiUtils.showIf(Editor.nativeIsFeatureEditable(), mEditPlace); - UiUtils.showIf(Framework.nativeIsActiveObjectABuilding(), mAddOrganisation); + UiUtils.showIf(Framework.nativeIsActiveObjectABuilding() && + Editor.nativeGetMapObjectStatus() != Editor.OBSOLETE, mAddOrganisation); UiUtils.showIf(Framework.nativeCanAddPlaceFromPlacePage(), mAddPlace); } diff --git a/iphone/Maps/Classes/Editor/MWMEditorViewController.mm b/iphone/Maps/Classes/Editor/MWMEditorViewController.mm index da9e2f5de4..3942660615 100644 --- a/iphone/Maps/Classes/Editor/MWMEditorViewController.mm +++ b/iphone/Maps/Classes/Editor/MWMEditorViewController.mm @@ -309,7 +309,7 @@ void registerCellsForTableView(vector const & cells, UITab noteInfo[kStatLat] = @(latLon.lat); noteInfo[kStatLon] = @(latLon.lon); [Statistics logEvent:kStatEditorProblemReport withParameters:noteInfo]; - osm::Editor::Instance().CreateNote(latLon, featureID, osm::Editor::NoteProblemType::General ,self.note.UTF8String); + f.CreateNote(latLon, featureID, osm::Editor::NoteProblemType::General, self.note.UTF8String); } switch (f.SaveEditedMapObject(m_mapObject)) @@ -997,7 +997,7 @@ void registerCellsForTableView(vector const & cells, UITab kStatEditorMWMVersion : @(fid.GetMwmVersion()), kStatProblem : @(osm::Editor::kPlaceDoesNotExistMessage), kStatLat : @(latLon.lat), kStatLon : @(latLon.lon)}]; - osm::Editor::Instance().CreateNote(latLon, fid, osm::Editor::NoteProblemType::PlaceDoesNotExist, additional); + GetFramework().CreateNote(latLon, fid, osm::Editor::NoteProblemType::PlaceDoesNotExist, additional); [self backTap]; [self showDropDown]; }]; diff --git a/iphone/Maps/Classes/MWMPlacePageEntity.mm b/iphone/Maps/Classes/MWMPlacePageEntity.mm index f5480d7c78..945bbfa53f 100644 --- a/iphone/Maps/Classes/MWMPlacePageEntity.mm +++ b/iphone/Maps/Classes/MWMPlacePageEntity.mm @@ -6,6 +6,8 @@ #include "Framework.h" +#include "indexer/osm_editor.hpp" + #include "platform/measurement_utils.hpp" #include "platform/mwm_version.hpp" @@ -146,7 +148,7 @@ void initFieldsMap(BOOL isBooking) - (NSString *)getCellValue:(MWMPlacePageCellType)cellType { auto const s = MapsAppDelegate.theApp.mapViewController.controlsManager.navigationState; - BOOL const editOrAddAreAvailable = version::IsSingleMwm(GetFramework().Storage().GetCurrentDataVersion()) && s == MWMNavigationDashboardStateHidden && !self.isBooking; + BOOL const editOrAddAreAvailable = version::IsSingleMwm(GetFramework().Storage().GetCurrentDataVersion()) && s == MWMNavigationDashboardStateHidden && m_info.IsEditable(); switch (cellType) { case MWMPlacePageCellTypeName: diff --git a/map/framework.cpp b/map/framework.cpp index 2fd2cacd9a..e6caa75420 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -733,17 +733,21 @@ void Framework::FillPointInfo(m2::PointD const & mercator, string const & custom void Framework::FillInfoFromFeatureType(FeatureType const & ft, place_page::Info & info) const { + auto const featureStatus = osm::Editor::Instance().GetFeatureStatus(ft.GetID()); + ASSERT_NOT_EQUAL(featureStatus, osm::Editor::FeatureStatus::Deleted, + ("Deleted features cannot be selected from UI")); info.SetFromFeatureType(ft); - info.m_isEditable = osm::Editor::Instance().GetEditableProperties(ft).IsEditable(); - info.m_localizedWifiString = m_stringsBundle.GetString("wifi"); - info.m_localizedRatingString = m_stringsBundle.GetString("place_page_booking_rating"); - if (ftypes::IsAddressObjectChecker::Instance()(ft)) info.m_address = GetAddressInfoAtPoint(feature::GetCenter(ft)).FormatHouseAndStreet(); if (ftypes::IsBookingChecker::Instance()(ft)) info.m_isSponsoredHotel = true; + + info.m_isEditable = featureStatus != osm::Editor::FeatureStatus::Obsolete && + !info.IsSponsoredHotel(); + info.m_localizedWifiString = m_stringsBundle.GetString("wifi"); + info.m_localizedRatingString = m_stringsBundle.GetString("place_page_booking_rating"); } void Framework::FillApiMarkInfo(ApiMarkPoint const & api, place_page::Info & info) const @@ -2902,3 +2906,11 @@ bool Framework::RollBackChanges(FeatureID const & fid) } return rolledBack; } + +void Framework::CreateNote(ms::LatLon const & latLon, FeatureID const & fid, + osm::Editor::NoteProblemType const type, string const & note) +{ + osm::Editor::Instance().CreateNote(latLon, fid, type, note); + if (type == osm::Editor::NoteProblemType::PlaceDoesNotExist) + DeactivateMapSelection(true); +} diff --git a/map/framework.hpp b/map/framework.hpp index 1e8a4c2dea..08aa65f130 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -654,6 +654,9 @@ public: void DeleteFeature(FeatureID const & fid) const; osm::NewFeatureCategories GetEditorCategories() const; bool RollBackChanges(FeatureID const & fid); + void CreateNote(ms::LatLon const & latLon, FeatureID const & fid, + osm::Editor::NoteProblemType const type, string const & note); + //@} private: From ce64dac110b8d0e1994b2f46f5bbd5e10142fd67 Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Fri, 3 Jun 2016 13:52:14 +0300 Subject: [PATCH 3/3] Code review. --- .../com/mapswithme/maps/editor/EditorFragment.java | 4 ++-- drape_frontend/apply_feature_functors.cpp | 5 +++-- editor/editor_tests/user_stats_test.cpp | 8 +------- indexer/osm_editor.cpp | 6 +++--- map/framework.cpp | 12 ++++++------ 5 files changed, 15 insertions(+), 20 deletions(-) diff --git a/android/src/com/mapswithme/maps/editor/EditorFragment.java b/android/src/com/mapswithme/maps/editor/EditorFragment.java index f7f252e9fa..19d94ee29b 100644 --- a/android/src/com/mapswithme/maps/editor/EditorFragment.java +++ b/android/src/com/mapswithme/maps/editor/EditorFragment.java @@ -432,7 +432,7 @@ public class EditorFragment extends BaseMwmFragment implements View.OnClickListe case Editor.DELETED: throw new IllegalStateException("Can't delete already deleted feature."); case Editor.OBSOLETE: - throw new IllegalStateException("Obsolete objects cannot be reverted now."); + throw new IllegalStateException("Obsolete objects cannot be reverted."); } } @@ -458,7 +458,7 @@ public class EditorFragment extends BaseMwmFragment implements View.OnClickListe case Editor.DELETED: throw new IllegalStateException("Can't delete already deleted feature."); case Editor.OBSOLETE: - throw new IllegalStateException("Obsolete objects cannot be reverted now."); + throw new IllegalStateException("Obsolete objects cannot be reverted."); } } diff --git a/drape_frontend/apply_feature_functors.cpp b/drape_frontend/apply_feature_functors.cpp index 1c23b108e7..c18d3dd892 100644 --- a/drape_frontend/apply_feature_functors.cpp +++ b/drape_frontend/apply_feature_functors.cpp @@ -282,8 +282,9 @@ void ApplyPointFeature::operator()(m2::PointD const & point, bool hasArea) auto const & editor = osm::Editor::Instance(); m_hasPoint = true; m_hasArea = hasArea; - m_createdByEditor = editor.GetFeatureStatus(m_id) == osm::Editor::FeatureStatus::Created; - m_obsoleteInEditor = editor.GetFeatureStatus(m_id) == osm::Editor::FeatureStatus::Obsolete; + auto const featureStatus = editor.GetFeatureStatus(m_id); + m_createdByEditor = featureStatus == osm::Editor::FeatureStatus::Created; + m_obsoleteInEditor = featureStatus == osm::Editor::FeatureStatus::Obsolete; m_centerPoint = point; } diff --git a/editor/editor_tests/user_stats_test.cpp b/editor/editor_tests/user_stats_test.cpp index d1e43b1ff1..e21e4d724d 100644 --- a/editor/editor_tests/user_stats_test.cpp +++ b/editor/editor_tests/user_stats_test.cpp @@ -9,7 +9,7 @@ namespace editor namespace { auto constexpr kEditorTestDir = "editor-tests"; -auto constexpr kUserName = "Vladimir BI"; +auto constexpr kUserName = "Vladimir BI"; // TODO(mgsergio, Zverik): Make a test user account. UNIT_TEST(UserStatsLoader_Smoke) { @@ -31,9 +31,6 @@ UNIT_TEST(UserStatsLoader_Smoke) int32_t rank, changesCount; TEST(userStats.GetRank(rank), ()); TEST(userStats.GetChangesCount(changesCount), ()); - - TEST_GREATER_OR_EQUAL(rank, 5800, ()); - TEST_EQUAL(changesCount, 2, ()); } // This test checks if user stats info was stored in setting. @@ -48,9 +45,6 @@ UNIT_TEST(UserStatsLoader_Smoke) int32_t rank, changesCount; TEST(userStats.GetRank(rank), ()); TEST(userStats.GetChangesCount(changesCount), ()); - - TEST_GREATER_OR_EQUAL(rank, 5800, ()); - TEST_EQUAL(changesCount, 2, ()); } } } // namespace diff --git a/indexer/osm_editor.cpp b/indexer/osm_editor.cpp index fa646a62b5..e7c21dc6a2 100644 --- a/indexer/osm_editor.cpp +++ b/indexer/osm_editor.cpp @@ -410,7 +410,7 @@ Editor::SaveResult Editor::SaveEditedFeature(EditableMapObject const & emo) FeatureTypeInfo fti; auto const featureStatus = GetFeatureStatus(fid.m_mwmId, fid.m_index); - ASSERT_NOT_EQUAL(featureStatus, FeatureStatus::Obsolete, ("Obsolete feature cannot be modified")); + ASSERT_NOT_EQUAL(featureStatus, FeatureStatus::Obsolete, ("Obsolete feature cannot be modified.")); bool const wasCreatedByUser = IsCreatedFeature(fid); if (wasCreatedByUser && featureStatus == FeatureStatus::Untouched) @@ -420,7 +420,7 @@ Editor::SaveResult Editor::SaveEditedFeature(EditableMapObject const & emo) } else { - ASSERT_NOT_EQUAL(featureStatus, FeatureStatus::Deleted, ("Unexpected feature status")); + ASSERT_NOT_EQUAL(featureStatus, FeatureStatus::Deleted, ("Unexpected feature status.")); fti.m_feature = featureStatus == FeatureStatus::Untouched ? *m_getOriginalFeatureFn(fid) @@ -909,7 +909,7 @@ void Editor::MarkFeatureAsObsolete(FeatureID const & fid) auto const featureStatus = GetFeatureStatus(fid); ASSERT(featureStatus == FeatureStatus::Untouched || featureStatus == FeatureStatus::Modified, - ("Created or deleted features can't be obsolete")); + ("Only untouched and modified features can be made obsolete")); auto & fti = m_features[fid.m_mwmId][fid.m_index]; // If a feature was modified we can drop all changes since it's now obsolete. diff --git a/map/framework.cpp b/map/framework.cpp index e6caa75420..def6156fb8 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -735,7 +735,7 @@ void Framework::FillInfoFromFeatureType(FeatureType const & ft, place_page::Info { auto const featureStatus = osm::Editor::Instance().GetFeatureStatus(ft.GetID()); ASSERT_NOT_EQUAL(featureStatus, osm::Editor::FeatureStatus::Deleted, - ("Deleted features cannot be selected from UI")); + ("Deleted features cannot be selected from UI.")); info.SetFromFeatureType(ft); if (ftypes::IsAddressObjectChecker::Instance()(ft)) @@ -1757,7 +1757,7 @@ bool Framework::ShowMapForURL(string const & url) if (result != FAILED) { // Always hide current map selection. - DeactivateMapSelection(true); + DeactivateMapSelection(true /* notifyUI */); // set viewport and stop follow mode if any StopLocationFollow(); @@ -2000,7 +2000,7 @@ void Framework::OnTapEvent(df::TapInfo const & tapInfo) alohalytics::Stats::Instance().LogEvent(somethingWasAlreadySelected ? "$DelectMapObject" : "$EmptyTapOnMap"); // UI is always notified even if empty map is tapped, // because empty tap event switches on/off full screen map view mode. - DeactivateMapSelection(true); + DeactivateMapSelection(true /* notifyUI */); } } @@ -2900,7 +2900,7 @@ bool Framework::RollBackChanges(FeatureID const & fid) if (rolledBack) { if (status == osm::Editor::FeatureStatus::Created) - DeactivateMapSelection(true); + DeactivateMapSelection(true /* notifyUI */); else UpdatePlacePageInfoForCurrentSelection(); } @@ -2908,9 +2908,9 @@ bool Framework::RollBackChanges(FeatureID const & fid) } void Framework::CreateNote(ms::LatLon const & latLon, FeatureID const & fid, - osm::Editor::NoteProblemType const type, string const & note) + osm::Editor::NoteProblemType const type, string const & note) { osm::Editor::Instance().CreateNote(latLon, fid, type, note); if (type == osm::Editor::NoteProblemType::PlaceDoesNotExist) - DeactivateMapSelection(true); + DeactivateMapSelection(true /* notifyUI */); }