[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