RecentModel: filter on account then regex

This patch tries to simplify the filtering logic.

We first check if there is a selected account, and if not then we
filter everything out. Then we check if there is either no
account set on the CM, or that it matches the selected account.
Finally we see if our filter string is contained in some identifier
of the CM or Person.

Change-Id: Ic00d837bfa224177530a110ac9d09617de5c5d0e
Reviewed-by: Nicolas Jäger <nicolas.jager@savoirfairelinux.com>

src/recentmodel.cpp

@@ -100,7 +100,7 @@ public:
 
     virtual QVariant data(const QModelIndex& index, int role) const override;
 protected:
-    virtual bool filterAcceptsRow ( int source_row, const QModelIndex & source_parent ) const override;
+    virtual bool filterAcceptsRow ( int sourceRow, const QModelIndex & sourceParent ) const override;
 
 };
 
@@ -1143,83 +1143,54 @@ PeopleProxy::PeopleProxy(RecentModel* sourceModel)
 }
 
 bool
-PeopleProxy::filterAcceptsRow(int source_row, const QModelIndex & source_parent) const
+PeopleProxy::filterAcceptsRow(int sourceRow, const QModelIndex & sourceParent) const
 {
-    //we filter only on top nodes
-    if (!source_parent.isValid() && filterRegExp().isEmpty()) {
-        // get the user chosen account
-        auto index_chosen_account = AvailableAccountModel::instance().selectionModel()->currentIndex();
-        auto chosen_account = index_chosen_account.data(static_cast<int>(Account::Role::Object)).value<Account*>();
-
-        // if there is no account selected, show the item.
-        if (not chosen_account)
-            return QSortFilterProxyModel::filterAcceptsRow(source_row, source_parent);
+    auto idxChosenAccount = AvailableAccountModel::instance().selectionModel()->currentIndex();
+    auto chosenAccount = idxChosenAccount.data(static_cast<int>(Account::Role::Object)).value<Account*>();
 
-        auto idx = sourceModel()->index(source_row, 0);
-
-        if (not idx.isValid()) // for example, manages rowCount() calls
-            return QSortFilterProxyModel::filterAcceptsRow(source_row, source_parent);
+    // filter everythin out if there is no account chosen
+    if (not chosenAccount)
+        return false;
 
+    //we filter only on top nodes
+    if (!sourceParent.isValid()) {
+        auto idx = sourceModel()->index(sourceRow, 0);
         auto type = idx.data(static_cast<int>(Ring::Role::ObjectType)).value<Ring::ObjectType>();
         auto object = idx.data(static_cast<int>(Ring::Role::Object));
 
-        if (type == Ring::ObjectType::ContactMethod) {
-            // checks if the associated account is the same that the one selected
-            auto cm = object.value<ContactMethod *>();
-
-            // LRC can create cm without account (typically if the cm was createad but never called)
-            // in this case the cm will be shown for any account.
-            if (not cm->account())
-                return cm;
-
-            return cm->account() == chosen_account;
-
-        } else if (type == Ring::ObjectType::Person) {
-            const auto person_numbers = object.value<Person *>()->phoneNumbers();
-
-            // checks if the Person contains any ContactMethod wich has the same account than the one selected
-            if (chosen_account and \
-                std::any_of(std::begin(person_numbers), std::end(person_numbers),
-                            [&](const ContactMethod* cm) { return cm->account() == chosen_account; })) {
-               return true;
-            }
-
-            // return false if any ContactMethod has a valid account (but none are the selected_account)
-            if (std::any_of(std::begin(person_numbers), std::end(person_numbers),
-                            [&](const ContactMethod* cm) { return cm->account() != nullptr; })) {
-               return false;
-            }
+        Person *person = nullptr;
+        auto filterFunction = [&person, chosenAccount, this] (const ContactMethod* cm) {
+            auto passesFilter = false;
+            // only proceed if there is no account set yet, or if it matches the chosen account
+            if ( !cm->account() or (cm->account() == chosenAccount)) {
+                /* we need to check the Person name as well as any identifier of the
+                 * ContactMethod.
+                 * note: QString::contains() will return true for an empty param string
+                 */
+                passesFilter =
+                    (person and person->formattedName().contains(filterRegExp())) or
+                    cm->uri().full().contains(filterRegExp()) or
+                    cm->registeredName().contains(filterRegExp()) or
+                    cm->primaryName().contains(filterRegExp());
             }
-        // anything else without ContactMethod does not require to be filtered
-        return true;
-
-    }else if (!source_parent.isValid()) {
-        auto idx = sourceModel()->index(source_row, 0);
+            return passesFilter;
+        };
 
         //we want to filter on name and number; note that Person object may have many numbers
-        if (idx.data(static_cast<int>(Ring::Role::Name)).toString().contains(filterRegExp())) {
-            return true;
-        } else {
-            auto type = idx.data(static_cast<int>(Ring::Role::ObjectType)).value<Ring::ObjectType>();
-            auto object = idx.data(static_cast<int>(Ring::Role::Object));
-
         switch (type) {
             case Ring::ObjectType::Person:
             {
-                    auto p = object.value<Person *>();
-                    for (auto cm : p->phoneNumbers()) {
-                        if (cm->uri().full().contains(filterRegExp()))
-                            return true;
-                    }
-                    return false;
+                person = object.value<Person *>();
+                const auto personCMs = person->phoneNumbers();
+
+                return std::any_of(std::begin(personCMs), std::end(personCMs), filterFunction);
             }
-                break;
             case Ring::ObjectType::ContactMethod:
             {
                 auto cm = object.value<ContactMethod *>();
-                    return cm->uri().full().contains(filterRegExp());
+
+                return filterFunction(cm);
             }
-                break;
             // top nodes are only of type Person or ContactMethod
             case Ring::ObjectType::Call:
             case Ring::ObjectType::Media:
@@ -1229,12 +1200,11 @@ PeopleProxy::filterAcceptsRow(int source_row, const QModelIndex & source_parent)
             break;
         }
 
-        }
         return false; // no matches
     }
     //in the case of children, only show if there is more than one unless it is a conference
-    if (static_cast<RecentModel *>(sourceModel())->isConference(source_parent)
-        || sourceModel()->rowCount(source_parent) > 1 )
+    if (static_cast<RecentModel *>(sourceModel())->isConference(sourceParent)
+        || sourceModel()->rowCount(sourceParent) > 1 )
         return true;
     return false;
 }