[Merge] lp:~phablet-team/telephony-service/group-chat into lp:telephony-service/staging

Gustavo Pichorim Boiko gustavo.boiko at canonical.com
Thu Oct 20 14:55:26 UTC 2016


Review: Needs Fixing

Some things to check and maybe fix

Diff comments:

> 
> === modified file 'handler/texthandler.cpp'
> --- handler/texthandler.cpp	2016-03-01 18:17:57 +0000
> +++ handler/texthandler.cpp	2016-10-13 18:26:40 +0000
> @@ -101,10 +102,9 @@
>              }
>              if (!found) {
>                  recipientsList << pendingMessage.recipients;
> -            }
> -        }
> -        Q_FOREACH(const QStringList& recipients, recipientsList) {
> -            startChat(recipients, accountId);
> +            }*/
> +            // TODO AVOID CALLING TWICE FOR SAME CHANNEL

Can you please either review and re-enable this block of commented code or remove it completely?

> +            startChat(accountId, pendingMessage.properties);
>          }
>      }
>  }
> @@ -354,20 +406,46 @@
>      }
>  
>      // keep recipient list always sorted to be able to compare
> -    QStringList sortedRecipients = recipients;
> -    sortedRecipients.sort();
> -    PendingMessage pendingMessage = {account->accountId(), sortedRecipients, message, attachments, properties};
> +    PendingMessage pendingMessage = {account->accountId(), message, attachments, properties};
>  
>      if (!account->connected()) {
>          mPendingMessages.append(pendingMessage);
>          return account->accountId();
>      }
>  
> -    QList<Tp::TextChannelPtr> channels = existingChannels(recipients, account->accountId());
> +    QList<Tp::TextChannelPtr> channels = existingChannels(account->accountId(), properties);
>      if (channels.isEmpty()) {
> -        mPendingMessages.append(pendingMessage);
> -        startChat(sortedRecipients, account->accountId());
> -        return account->accountId();
> +        // temporary

Can you double check if this is still really temporary or if ended up being the correct approach here in the end?

> +        switch(properties["chatType"].toUInt()) {
> +        case Tp::HandleTypeNone:
> +        case Tp::HandleTypeContact:
> +            mPendingMessages.append(pendingMessage);
> +            startTextChat(account->account(), pendingMessage.properties);
> +            return account->accountId();
> +        case Tp::HandleTypeRoom: {
> +            channels << startTextChatroom(account->account(), pendingMessage.properties);
> +            qDebug() << "channel returned" << channels.last();
> +       
> +            // multimedia fails if we send the message right away
> +            QTimer *timer = new QTimer(this);
> +            timer->setInterval(3000);
> +            timer->setSingleShot(true);
> +            QObject::connect(timer, &QTimer::timeout, [=]() {
> +                qDebug() << "sending message" << channels.last();
> +                connect(channels.last()->send(buildMessage(pendingMessage)),
> +                        SIGNAL(finished(Tp::PendingOperation*)),
> +                        SLOT(onMessageSent(Tp::PendingOperation*)));
> +
> +                timer->deleteLater();
> +            });
> +            timer->start();
> +            return account->accountId();
> +            break;
> +        }
> +        }
> +
> +        //startChat(account->accountId(), pendingMessage.properties);
> +        //return account->accountId();

if the temporary solution above is not temporary anymore, could you please remove those commented out lines?

>      }
>  
>      connect(channels.last()->send(buildMessage(pendingMessage)),
> 
> === modified file 'tests/handler/handlercontroller.cpp'
> --- tests/handler/handlercontroller.cpp	2015-12-07 16:26:20 +0000
> +++ tests/handler/handlercontroller.cpp	2016-10-13 18:26:40 +0000
> @@ -75,7 +75,11 @@
>  
>  void HandlerController::startChat(const QString &accountId, const QStringList &recipients)
>  {
> -    mHandlerInterface.call("StartChat", accountId, recipients);
> +    // TODO CHANGE SIGNATURE of this method

is this TODO entry still valid?

> +    QVariantMap properties;
> +    properties["participantIds"] = recipients;
> +
> +    mHandlerInterface.call("StartChat", accountId, properties);
>  }
>  
>  void HandlerController::startCall(const QString &number, const QString &accountId)


-- 
https://code.launchpad.net/~phablet-team/telephony-service/group-chat/+merge/308436
Your team Ubuntu Phablet Team is subscribed to branch lp:telephony-service/staging.



More information about the Ubuntu-reviews mailing list