~aleteoryx/muditaos

e07f2db550f1b881a92a25a420289b1a704256c6 — rrandomsky 2 years ago a56e8db
[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.
M module-db/Interface/ContactRecord.cpp => module-db/Interface/ContactRecord.cpp +49 -23
@@ 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<std::uint32_t> &oldNumberIDs, std::vector<ContactRecord::Number> &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;
}

M module-db/Interface/ContactRecord.hpp => module-db/Interface/ContactRecord.hpp +11 -0
@@ 301,4 301,15 @@ class ContactRecordInterface : public RecordInterface<ContactRecord, ContactReco
        -> std::optional<std::uint32_t>;
    auto addOrUpdateRingtone(std::uint32_t contactID, std::uint32_t ringtoneID, const ContactRecord &contact)
        -> std::optional<std::uint32_t>;

    /**
     * @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<std::uint32_t> &oldNumberIDs,
                                                               std::vector<ContactRecord::Number> &newNumbers) -> bool;
};

M module-db/Tables/ContactsTable.cpp => module-db/Tables/ContactsTable.cpp +1 -1
@@ 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"

M module-db/tests/ContactsRecord_tests.cpp => module-db/tests/ContactsRecord_tests.cpp +164 -0
@@ 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> contactsDb{"contacts.db", db::tests::getPurePhoneScriptsPath()};

    // Preparation of DB initial state
    auto records = ContactRecordInterface(&contactsDb.get());
    ContactRecord testContactRecord;

    std::array<std::string, 2> numbers   = {{{"500600100"}, {"500600200"}}};
    std::array<std::string, 2> numbersPL = {{{"+48500600100"}, {"+48500600200"}}}; // Poland country code
    std::array<std::string, 2> 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>({
            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>({
            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>({
            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>({
            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>({
            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>({
            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]);
    }
}

M module-services/service-db/DBServiceAPI.cpp => module-services/service-db/DBServiceAPI.cpp +3 -3
@@ 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<db::query::NumberGetByID>(numberID);
    auto msg                      = std::make_unique<db::query::NumberGetByID>(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;
            }

M pure_changelog.md => pure_changelog.md +1 -2
@@ 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]