[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