[Merge] lp:~phablet-team/history-service/simplify_participant_filtering into lp:history-service/staging

Roberto Mier Escandón  roberto.escandon at canonical.com
Thu Nov 24 09:25:59 UTC 2016


Review: Approve

Much more clear now. Tested and works ok (nothing is broken :))
Just remove a not needed comment in code (see below)


Diff comments:

> === modified file 'Ubuntu/History/historymodel.cpp'
> --- Ubuntu/History/historymodel.cpp	2016-11-09 17:42:27 +0000
> +++ Ubuntu/History/historymodel.cpp	2016-11-24 01:51:38 +0000
> @@ -94,40 +94,29 @@
>          break;
>      case ParticipantsRole: {
>          // FIXME: reimplement in a cleaner way

maybe that // FIXME is already fixed :)

> -        History::Participants participants = History::Participants::fromVariantList(properties[History::FieldParticipants].toList());
> +        History::Participants participants = History::Participants::fromVariantList(properties[History::FieldParticipants].toList())
> +                                             .filterByState(History::ParticipantStateRegular);
>          if (mMatchContacts) {
>              QVariantList finalParticipantsList;
>              QVariantList participantsInfo = History::ContactMatcher::instance()->contactInfo(properties[History::FieldAccountId].toString(),
> -                                                                      participants.identifiers());
> -            int count = 0;
> -            Q_FOREACH(const QVariant &participantInfo, participantsInfo) {
> -                QVariantMap newMap = participantInfo.toMap();
> -                if (participants.at(count).state() != History::ParticipantStateRegular) {
> -                   count++;
> -                   continue;
> -                }
> -                newMap[History::FieldParticipantState] = participants.at(count).state();
> -                newMap[History::FieldParticipantRoles] = participants.at(count++).roles();
> +                                                                                             participants.identifiers());
> +            for (int i = 0; i < participantsInfo.count(); ++i) {
> +                QVariantMap newMap = participantsInfo[i].toMap();
> +                History::Participant participant = participants[i];
> +                newMap[History::FieldParticipantState] = participant.state();
> +                newMap[History::FieldParticipantRoles] = participant.roles();
>                  finalParticipantsList << newMap;
>              }
>              result = finalParticipantsList;
>          } else {
>              //FIXME: handle contact changes
> -            result = properties[History::FieldParticipants];
> +            result = participants.identifiers();
>          }
>          break;
>      }
>      case ParticipantsRemotePendingRole: {
> -        // FIXME: reimplement in a cleaner way
> -        QStringList identifiers;
> -        History::Participants participants;
> -        // filter remote pending participants
> -        Q_FOREACH(const History::Participant &participant, History::Participants::fromVariantList(properties[History::FieldParticipants].toList())) {
> -            if (participant.state() == History::ParticipantStateRemotePending) {
> -                participants << participant;
> -            }
> -        }
> -
> +        History::Participants participants = History::Participants::fromVariantList(properties[History::FieldParticipants].toList())
> +                                             .filterByState(History::ParticipantStateRemotePending);
>          if (mMatchContacts) {
>              QVariantList finalParticipantsList;
>              QVariantList participantsInfo = History::ContactMatcher::instance()->contactInfo(properties[History::FieldAccountId].toString(),


-- 
https://code.launchpad.net/~phablet-team/history-service/simplify_participant_filtering/+merge/311674
Your team Ubuntu Phablet Team is subscribed to branch lp:history-service/staging.



More information about the Ubuntu-reviews mailing list