[Merge] lp:~tiagosh/telephony-service/send_image_im into lp:telephony-service

Gustavo Pichorim Boiko gustavo.boiko at canonical.com
Tue Nov 10 17:25:03 UTC 2015


Review: Needs Fixing

One problem found.

Diff comments:

> 
> === modified file 'handler/texthandler.cpp'
> --- handler/texthandler.cpp	2015-11-10 17:05:35 +0000
> +++ handler/texthandler.cpp	2015-11-10 17:05:35 +0000
> @@ -348,72 +356,24 @@
>          return;
>      }
>      QString accountId = account->accountId();
> -    QStringList recipients;
>      mChannels.append(channel);
>  
>      // check for pending messages for this channel
> -    if (!mPendingMessages.contains(accountId) && 
> -        !mPendingMMSs.contains(accountId) &&
> -        !mPendingSilentMessages.contains(accountId)) {
> +    if (mPendingMessages.isEmpty()) {
>          return;
>      }
>  
> -    Q_FOREACH(const Tp::ContactPtr &channelContact, channel->groupContacts(false)) {
> -        recipients << channelContact->id();
> -    }
> -
> -    QMap<QStringList, QStringList> &pendingMessages = mPendingMessages[accountId];
> -    QMap<QStringList, QStringList>::iterator it = pendingMessages.begin();
> -    while (it != pendingMessages.end()) {
> -        bool found = false;
> -        Q_FOREACH(const Tp::TextChannelPtr &existingChannel, existingChannels(it.key(), accountId)) {
> -            if (existingChannel == channel) {
> -                Q_FOREACH(const QString &message, it.value()) {
> -                    sendMessage(recipients, message, accountId);
> -                }
> -                it = pendingMessages.erase(it);
> -                found = true;
> -                break;
> -            }
> -        }
> -        if (!found) {
> -            ++it;
> -        }
> -    }
> -    QMap<QStringList, QList<AttachmentList>> &pendingMMSs = mPendingMMSs[accountId];
> -    QMap<QStringList, QList<AttachmentList>>::iterator it2 = pendingMMSs.begin();
> -    while (it2 != pendingMMSs.end()) {
> -        bool found = false;
> -        Q_FOREACH(const Tp::TextChannelPtr &existingChannel, existingChannels(it2.key(), accountId)) {
> -            if (existingChannel == channel) {
> -                Q_FOREACH(const AttachmentList &attachments, it2.value()) {
> -                    sendMMS(recipients, attachments, accountId);
> -                }
> -                it2 = pendingMMSs.erase(it2);
> -                found = true;
> -                break;
> -            }
> -        }
> -        if (!found) {
> -            ++it2;
> -        }
> -    }
> -    QMap<QStringList, QStringList> &pendingSilentMessages = mPendingSilentMessages[accountId];
> -    QMap<QStringList, QStringList>::iterator it3 = pendingSilentMessages.begin();
> -    while (it3 != pendingSilentMessages.end()) {
> -        bool found = false;
> -        Q_FOREACH(const Tp::TextChannelPtr &existingChannel, existingChannels(it3.key(), accountId)) {
> -            if (existingChannel == channel) {
> -                Q_FOREACH(const QString &message, it3.value()) {
> -                    sendSilentMessage(recipients, message, accountId);
> -                }
> -                it3 = pendingSilentMessages.erase(it3);
> -                found = true;
> -                break;
> -            }
> -        }
> -        if (!found) {
> -            ++it3;
> +    QList<PendingMessage>::iterator it = mPendingMessages.begin();
> +    while (it != mPendingMessages.end()) {
> +        Q_FOREACH(const Tp::TextChannelPtr &existingChannel, existingChannels(it->recipients, it->accountId)) {
> +            if (existingChannel == channel) {
> +                // FIXME: we can't trust recipients for group chats in regular IM accounts
> +                sendMessage(it->accountId, it->recipients, it->message, it->attachments, it->properties);
> +                it = mPendingMessages.erase(it);
> +                break;
> +             } else {
> +                ++it;

This should go outside the Q_FOREACH loop.

> +             }
>          }
>      }
>  }


-- 
https://code.launchpad.net/~tiagosh/telephony-service/send_image_im/+merge/267581
Your team Ubuntu Phablet Team is subscribed to branch lp:telephony-service.



More information about the Ubuntu-reviews mailing list