From e07f2db550f1b881a92a25a420289b1a704256c6 Mon Sep 17 00:00:00 2001 From: rrandomsky Date: Fri, 20 Jan 2023 11:17:44 +0100 Subject: [PATCH] [MOS-864] Fix for country code in new contact based on deleted one Fix for scenario when contact with some number with/without country code was deleted, and new contact with same number but without/with country code is added and then new contact have same prefix as deleted one. Now new contact will have number exacly like provided. --- module-db/Interface/ContactRecord.cpp | 72 ++++++--- module-db/Interface/ContactRecord.hpp | 11 ++ module-db/Tables/ContactsTable.cpp | 2 +- module-db/tests/ContactsRecord_tests.cpp | 164 ++++++++++++++++++++ module-services/service-db/DBServiceAPI.cpp | 6 +- pure_changelog.md | 3 +- 6 files changed, 229 insertions(+), 29 deletions(-) diff --git a/module-db/Interface/ContactRecord.cpp b/module-db/Interface/ContactRecord.cpp index d15bf18271416514223446ef3157e7c4823caa7b..6a1002c66d73ebbd8b6c239b92dab96da52cba4d 100644 --- a/module-db/Interface/ContactRecord.cpp +++ b/module-db/Interface/ContactRecord.cpp @@ -47,6 +47,13 @@ auto ContactRecordInterface::Add(ContactRecord &rec) -> bool if (!numbersIDs.has_value()) { return false; } + + auto oldNumberIDs = splitNumberIDs(numbersIDs.value()); + auto newNumbers = rec.numbers; + if (!changeNumberRecordInPlaceIfCountryCodeIsOnlyDifferent(oldNumberIDs, newNumbers)) { + return false; + } + if (!rec.isTemporary()) { const auto nameID = addOrUpdateName(contactID, DB_ID_NONE, rec); if (!nameID.has_value()) { @@ -124,29 +131,9 @@ auto ContactRecordInterface::Update(const ContactRecord &rec) -> bool } auto oldNumberIDs = splitNumberIDs(contact.numbersID); - - /* Changing number table record in place if new number is same as old number but with/without country code */ - auto numberMatcher = buildNumberMatcher(NumberMatcherPageSize); - for (const auto oldNumberID : oldNumberIDs) { // pick one of the old number for this contactID (from DB) - auto numberRecord = contactDB->number.getById(oldNumberID); - utils::PhoneNumber oldPhoneNumber(numberRecord.numberUser, numberRecord.numbere164); - for (const auto &newNumberID : rec.numbers) { // pick one of the new number from &rec - utils::PhoneNumber newPhoneNumber(newNumberID.number); - // if DB have not such a new number already and if one of this have country code and other doesn't - if (!numberMatcher.bestMatch(newPhoneNumber, utils::PhoneNumber::Match::EXACT).has_value() && - (newPhoneNumber.match(oldPhoneNumber) == utils::PhoneNumber::Match::POSSIBLE) && - ((!oldPhoneNumber.isValid() && newPhoneNumber.isValid()) || - (oldPhoneNumber.isValid() && !newPhoneNumber.isValid()))) { - // which means that only country code is to add or remove (change of country code is not supported here) - // then change old number record in number table to the new number - auto oldNumberRecordToUpdate = contactDB->number.getById(oldNumberID); - oldNumberRecordToUpdate.numberUser = newPhoneNumber.get(); - oldNumberRecordToUpdate.numbere164 = newPhoneNumber.toE164(); - if (!contactDB->number.update(oldNumberRecordToUpdate)) { - return false; - } - } - } + auto newNumbers = rec.numbers; + if (!changeNumberRecordInPlaceIfCountryCodeIsOnlyDifferent(oldNumberIDs, newNumbers)) { + return false; } auto newNumbersIDs = getNumbersIDs(contact.ID, rec, utils::PhoneNumber::Match::EXACT); @@ -1559,3 +1546,42 @@ auto ContactRecordInterface::verifyTemporary(ContactRecord &record) -> bool return isTemporary; } + +auto ContactRecordInterface::changeNumberRecordInPlaceIfCountryCodeIsOnlyDifferent( + const std::vector &oldNumberIDs, std::vector &newNumbers) -> bool +{ + if (oldNumberIDs.empty() || newNumbers.empty()) { + LOG_ERROR("Failed to change number record in place if country code is only different. " + "Empty input data."); + return false; + } + for (const auto id : oldNumberIDs) { + if (id == 0) + LOG_WARN("Number ID == 0"); + } + + auto numberMatcher = buildNumberMatcher(NumberMatcherPageSize); + for (const auto oldNumberID : oldNumberIDs) { // pick one of the old number for this contactID (from DB) + auto numberRecord = contactDB->number.getById(oldNumberID); + utils::PhoneNumber oldPhoneNumber(numberRecord.numberUser, numberRecord.numbere164); + for (const auto &newNumberID : newNumbers) { // pick one of the new number from newNumbers + utils::PhoneNumber newPhoneNumber(newNumberID.number); + // if DB have not such a new number already and if one of this have country code and other doesn't + if (!numberMatcher.bestMatch(newPhoneNumber, utils::PhoneNumber::Match::EXACT).has_value() && + (newPhoneNumber.match(oldPhoneNumber) == utils::PhoneNumber::Match::POSSIBLE) && + ((!oldPhoneNumber.isValid() && newPhoneNumber.isValid()) || + (oldPhoneNumber.isValid() && !newPhoneNumber.isValid()))) { + // which means that only country code is to add or remove (change of country code is not supported here) + // then change old number record in number table to the new number + numberRecord.numberUser = newPhoneNumber.get(); + numberRecord.numbere164 = newPhoneNumber.toE164(); + if (!contactDB->number.update(numberRecord)) { + LOG_ERROR("Failed to change number record in place if country code is only different. " + "Number update failed."); + return false; + } + } + } + } + return true; +} diff --git a/module-db/Interface/ContactRecord.hpp b/module-db/Interface/ContactRecord.hpp index 0ffe5ea73ea83efc49fc5c32cc5668445428ff26..abed2db4c519fc0f56e7f955efa230bc82dd8975 100644 --- a/module-db/Interface/ContactRecord.hpp +++ b/module-db/Interface/ContactRecord.hpp @@ -301,4 +301,15 @@ class ContactRecordInterface : public RecordInterface std::optional; auto addOrUpdateRingtone(std::uint32_t contactID, std::uint32_t ringtoneID, const ContactRecord &contact) -> std::optional; + + /** + * @brief Changing number table record in place if new number is same as old number but with/without country code + * + * @param oldNumberIDs vector of old number IDs in contact_number table which can be changed in place + * only if in newNumbers is same number but with/without country code + * @param newNumbers vector of numbers to which we want to change the old ones + * @return true if operation have no error + */ + auto changeNumberRecordInPlaceIfCountryCodeIsOnlyDifferent(const std::vector &oldNumberIDs, + std::vector &newNumbers) -> bool; }; diff --git a/module-db/Tables/ContactsTable.cpp b/module-db/Tables/ContactsTable.cpp index 1fe951ae01630e0c9f52a059088583b872063c26..48dc80a8ae5ba40096da92faa14ee5006aba297d 100644 --- a/module-db/Tables/ContactsTable.cpp +++ b/module-db/Tables/ContactsTable.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include "ContactsTable.hpp" diff --git a/module-db/tests/ContactsRecord_tests.cpp b/module-db/tests/ContactsRecord_tests.cpp index 3565628b326d50dcfe044c1e1f670b77e21493d5..f25153570e3b609b7b28dc78b27956a83a621838 100644 --- a/module-db/tests/ContactsRecord_tests.cpp +++ b/module-db/tests/ContactsRecord_tests.cpp @@ -760,3 +760,167 @@ TEST_CASE("Check if new contact record exists in DB as a temporary contact") REQUIRE(records.verifyTemporary(noTemporaryContactRecord) == true); } } + +TEST_CASE("Check replacement of number in place in db when only different is having of country code") +{ + db::tests::DatabaseUnderTest contactsDb{"contacts.db", db::tests::getPurePhoneScriptsPath()}; + + // Preparation of DB initial state + auto records = ContactRecordInterface(&contactsDb.get()); + ContactRecord testContactRecord; + + std::array numbers = {{{"500600100"}, {"500600200"}}}; + std::array numbersPL = {{{"+48500600100"}, {"+48500600200"}}}; // Poland country code + std::array numbersFR = {{{"+33500600100"}, {"+33500600200"}}}; // France country code + + SECTION("Temporary contact exists but new one with same number have country code") + { + // Number of record in number table should stay the same + constexpr int32_t NUMBER_IN_RECORD_DURING_REPLACEMENT = 1; + + // Adding of temporary contact without country code + ContactRecord temporaryContactRecord; + temporaryContactRecord.numbers = std::vector({ + ContactRecord::Number(numbers[0], std::string(""), ContactNumberType::HOME), + }); + temporaryContactRecord.addToGroup(contactsDb.get().groups.temporaryId()); + + REQUIRE(records.Add(temporaryContactRecord)); + REQUIRE(contactsDb.get().number.count() == NUMBER_IN_RECORD_DURING_REPLACEMENT); + + auto validationRecord = records.GetByID(1); // There aren't new normal contact + REQUIRE(validationRecord.ID == DB_ID_NONE); + REQUIRE(validationRecord.numbers.empty()); + + validationRecord = records.GetByIdWithTemporary(1); // There are new temporary contact + REQUIRE(validationRecord.numbers.size() == 1); + REQUIRE(validationRecord.numbers[0].number.getEntered() == numbers[0]); + + // Adding of normal contact witch same number than previous temporary but with country code + ContactRecord noTemporaryContactRecord; + noTemporaryContactRecord.primaryName = "PrimaryNameNoTemporary"; + noTemporaryContactRecord.alternativeName = "AlternativeNameNoTemporary"; + noTemporaryContactRecord.numbers = std::vector({ + ContactRecord::Number(numbersPL[0], std::string(""), ContactNumberType::HOME), + }); + + REQUIRE(records.verifyTemporary(noTemporaryContactRecord) == true); + + REQUIRE(records.Add(noTemporaryContactRecord)); + REQUIRE(contactsDb.get().number.count() == NUMBER_IN_RECORD_DURING_REPLACEMENT); // That was replacement in + // place + + validationRecord = records.GetByID(1); + REQUIRE(validationRecord.numbers.size() == 0); // Now first contact is temporary + REQUIRE(validationRecord.ID == DB_ID_NONE); + REQUIRE(validationRecord.numbers.empty()); + + validationRecord = records.GetByID(2); + REQUIRE(validationRecord.numbers.size() == 1); + REQUIRE(validationRecord.numbers[0].number.getEntered() == numbersPL[0]); + } + + SECTION("Adding contact without country code after deleting one with same number but with country code") + { + // Scenario from MOS-864: + // After deleting contact with prefix: e.g. +48 512 345 678, there was an issue when + // new contact with similar number but without country code "512 345 678" was added. + // Then new contact had number with prefix: "+48 512 345 678" instead of provided number by user + + // Number of record in number table should stay the same + constexpr int32_t NUMBER_IN_RECORD_DURING_REPLACEMENT = 1; + constexpr int32_t FIRST_CONTACT_ID = 1; + + // Adding of normal contact with country code + ContactRecord noTemporaryContactRecord; + noTemporaryContactRecord.primaryName = "PrimaryNameNoTemporary"; + noTemporaryContactRecord.alternativeName = "AlternativeNameNoTemporary"; + noTemporaryContactRecord.numbers = std::vector({ + ContactRecord::Number(numbersPL[0], std::string(""), ContactNumberType::HOME), + }); + + REQUIRE(records.Add(noTemporaryContactRecord)); + REQUIRE(contactsDb.get().number.count() == NUMBER_IN_RECORD_DURING_REPLACEMENT); + + auto validationRecord = records.GetByID(FIRST_CONTACT_ID); + REQUIRE(validationRecord.numbers.size() == 1); + REQUIRE(validationRecord.numbers[0].number.getEntered() == numbersPL[0]); // with country code + + // Remove contact + REQUIRE(records.RemoveByID(FIRST_CONTACT_ID)); + + validationRecord = records.GetByID(FIRST_CONTACT_ID); + REQUIRE(validationRecord.numbers.size() == 0); // Now first contact is temporary + REQUIRE(validationRecord.ID == DB_ID_NONE); + REQUIRE(validationRecord.numbers.empty()); + + // Add new contact with same number as previously deleted but without country code + ContactRecord NewNoTemporaryContactRecord; + NewNoTemporaryContactRecord.primaryName = "PrimaryNameNoTemporaryOther"; + NewNoTemporaryContactRecord.alternativeName = "AlternativeNameNoTemporaryOther"; + NewNoTemporaryContactRecord.numbers = std::vector({ + ContactRecord::Number(numbers[0], std::string(""), ContactNumberType::HOME), + }); + + REQUIRE(records.Add(NewNoTemporaryContactRecord)); + REQUIRE(contactsDb.get().number.count() == NUMBER_IN_RECORD_DURING_REPLACEMENT); + + validationRecord = records.GetByID(FIRST_CONTACT_ID + 1); // New contact + REQUIRE(validationRecord.numbers.size() == 1); + REQUIRE(validationRecord.numbers[0].number.getEntered() == numbers[0]); // without country code + } + + SECTION("Adding contact with country code after deleting one with same number but with different country code") + { + // Number of record in number table should NOT stay the same - no replacement in place + constexpr int32_t FIRST_CONTACT_ID = 1; + constexpr int32_t SECOND_NEW_CONTACT_ID = 2; + + // Adding of normal contact with country code + ContactRecord noTemporaryContactRecordPL; + noTemporaryContactRecordPL.primaryName = "PrimaryNameNoTemporary"; + noTemporaryContactRecordPL.alternativeName = "AlternativeNameNoTemporary"; + noTemporaryContactRecordPL.numbers = std::vector({ + ContactRecord::Number(numbersPL[0], std::string(""), ContactNumberType::HOME), + }); + + REQUIRE(records.Add(noTemporaryContactRecordPL)); + REQUIRE(contactsDb.get().number.count() == 1); + + auto validationRecord = records.GetByID(FIRST_CONTACT_ID); + REQUIRE(validationRecord.numbers.size() == 1); + REQUIRE(validationRecord.numbers[0].number.getEntered() == numbersPL[0]); // with country code + + // Remove contact + REQUIRE(records.RemoveByID(FIRST_CONTACT_ID)); + + validationRecord = records.GetByID(FIRST_CONTACT_ID); + REQUIRE(validationRecord.numbers.size() == 0); // Now first contact is temporary + REQUIRE(validationRecord.ID == DB_ID_NONE); + REQUIRE(validationRecord.numbers.empty()); + + // Add new contact with same number as previously deleted but without country code + ContactRecord NewNoTemporaryContactRecordFR; + NewNoTemporaryContactRecordFR.primaryName = "PrimaryNameNoTemporaryOther"; + NewNoTemporaryContactRecordFR.alternativeName = "AlternativeNameNoTemporaryOther"; + NewNoTemporaryContactRecordFR.numbers = std::vector({ + ContactRecord::Number(numbersFR[0], std::string(""), ContactNumberType::HOME), + }); + + REQUIRE(records.Add(NewNoTemporaryContactRecordFR)); + REQUIRE(contactsDb.get().number.count() == 2); // New number record - there are record for PL and FR number + + validationRecord = records.GetByID(FIRST_CONTACT_ID); + REQUIRE(validationRecord.numbers.size() == 0); // Now first contact is temporary + REQUIRE(validationRecord.ID == DB_ID_NONE); + REQUIRE(validationRecord.numbers.empty()); + + validationRecord = records.GetByID(SECOND_NEW_CONTACT_ID); // New contact + REQUIRE(validationRecord.numbers.size() == 1); + REQUIRE(validationRecord.numbers[0].number.getEntered() == numbersFR[0]); // without country code + + // Check number table + REQUIRE(contactsDb.get().number.getById(FIRST_CONTACT_ID).numberUser == numbersPL[0]); + REQUIRE(contactsDb.get().number.getById(SECOND_NEW_CONTACT_ID).numberUser == numbersFR[0]); + } +} diff --git a/module-services/service-db/DBServiceAPI.cpp b/module-services/service-db/DBServiceAPI.cpp index a93e7e664d46ae63169583c0b16b5e14e52c591c..1487f0ae034696bbfde7965f0ecc9af7d38ab4ee 100644 --- a/module-services/service-db/DBServiceAPI.cpp +++ b/module-services/service-db/DBServiceAPI.cpp @@ -102,7 +102,7 @@ auto DBServiceAPI::MatchContactByNumberID(sys::Service *serv, std::uint32_t numb auto DBServiceAPI::NumberByID(sys::Service *serv, std::uint32_t numberID) -> utils::PhoneNumber::View { - auto msg = std::make_unique(numberID); + auto msg = std::make_unique(numberID); const auto [status, response] = DBServiceAPI::GetQueryWithReply( serv, db::Interface::Name::Contact, std::move(msg), constants::DefaultTimeoutInMs); if (status != sys::ReturnCodes::Success || !response) { @@ -168,8 +168,8 @@ auto DBServiceAPI::verifyContact(sys::Service *serv, const ContactRecord &rec) } if (rec.numbers.size() > 1 && rec.numbers[1].number.getEntered().size() > 0) { - auto retPhone2 = MatchContactByPhoneNumber(serv, rec.numbers[1].number); - if (retPhone2 && retPhone2->ID != rec.ID) { + auto retPhone2 = MatchContactByPhoneNumber(serv, rec.numbers[1].number, rec.ID); + if (retPhone2) { if (retPhone2->isTemporary()) { return ContactVerificationResult::temporaryContactExists; } diff --git a/pure_changelog.md b/pure_changelog.md index 16bed15fa08e55742f19e8d49dc457f85cde87e0..d59d1e4c5dfbfdda7acf11b75e81d269cc3b16e8 100644 --- a/pure_changelog.md +++ b/pure_changelog.md @@ -51,8 +51,7 @@ * Fixed inability to enter text in search field after clicking right arrow * Fixed displaying wrong screen after contact removal * Fixed improper font weight used in navbar - -### Added +* Fix for country code in new contact based on deleted one ## [1.5.0 2022-12-20]