[Merge] lp:~tiagosh/telepathy-ofono/fix-mms-groupchat-recipients into lp:telepathy-ofono

[Gustavo Pichorim Boiko]

Review: Needs Fixing

Just one comment, but other than that looks good.

Diff comments:

> === modified file 'connection.cpp'
> --- connection.cpp	2015-08-19 03:05:19 +0000
> +++ connection.cpp	2015-10-02 20:57:38 +0000
> @@ -400,25 +400,32 @@
>      MMSDMessage *msg = new MMSDMessage(path, properties);
>      mServiceMMSList[servicePath].append(msg);
>      if (properties["Status"] ==  "received") {
> -        const QString senderNormalizedNumber = PhoneUtils::normalizePhoneNumber(properties["Sender"].toString());
> +        QString senderNormalizedNumber = PhoneUtils::normalizePhoneNumber(properties["Sender"].toString());
>          QStringList recipientList = properties["Recipients"].toStringList();
> +        QSet<QString> recipients;

Can you just add a comment explaining why a QSet was used in this case and not just a QStringList?
It looks to me as it is there to avoid having repeated elements.

> +        Tp::UIntList initialInviteeHandles;
>          // remove empty strings if any
>          recipientList.removeAll("");
> -        // remove ourselves from the recipient list 
> -        Q_FOREACH(const QString &myNumber, mOfonoSimManager->subscriberNumbers()) {
> -            Q_FOREACH(const QString &remoteNumber, recipientList) {
> -                if (PhoneUtils::comparePhoneNumbers(remoteNumber, myNumber)) {
> -                    recipientList.removeAll(remoteNumber);
> -                    break;
> +        if (recipientList.size() > 1) {
> +            // remove ourselves from the recipient list 
> +            Q_FOREACH(const QString &myNumber, mOfonoSimManager->subscriberNumbers()) {
> +                Q_FOREACH(const QString &remoteNumber, recipientList) {
> +                    if (PhoneUtils::comparePhoneNumbers(remoteNumber, myNumber)) {
> +                        recipientList.removeAll(remoteNumber);
> +                        break;
> +                    }
>                  }
>              }
> -        }
> -        QSet<QString> recipients;
> -        Tp::UIntList initialInviteeHandles;
> -        Q_FOREACH(const QString &recipient, recipientList) {
> -            recipients << PhoneUtils::normalizePhoneNumber(recipient);
> -            initialInviteeHandles << ensureHandle(recipient);
> -        }
> +            Q_FOREACH(const QString &recipient, recipientList) {
> +                recipients << PhoneUtils::normalizePhoneNumber(recipient);
> +                initialInviteeHandles << ensureHandle(recipient);
> +            }
> +        } else if (senderNormalizedNumber.isEmpty() && recipientList.size() == 1) {
> +            // if this is a message coming from the server (no sender), clear the recipient list;
> +            recipientList.clear();
> +            senderNormalizedNumber = "x-ofono-unknown";
> +        }
> +
>          // check if there is an open channel for this number and use it
>          oFonoTextChannel *channel = textChannelForMembers(QStringList() << senderNormalizedNumber << recipients.toList());
>          if (channel) {


-- 
https://code.launchpad.net/~tiagosh/telepathy-ofono/fix-mms-groupchat-recipients/+merge/270834
Your team Ubuntu Phablet Team is subscribed to branch lp:telepathy-ofono.