tristan/kdeconnect-kde
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[Desktop] Update conversation list when a new message arrives

Browse source 
Summary:
When a new message is delivered, the conversation list should update by changing the preview text and re-sorting the conversations

Bonus bug discovered and fixed:  previously, when the conversations list was being populated, it made a request for the first message in every conversation. This would be fine if the conversationdbusinterface pulled from local cache. However, this actually triggers a request to the phone for *every* conversation.

This should be handled differently in conversationdbusinterface's requestConversation as well, but that's a project for a later day (TODO comments added)

Test Plan:
 - Launch SMS app
 - Verify conversations list appears
 - Verify lack of massive stream of debug output indicating lots of messages for the wrong conversation are being received
 - Verify that opening a particular conversation shows the messages after a short delay while the backend fetches the content from the phone
 - Verify that receiving a new message into an existing conversation updates the conversation list

Reviewers: #kde_connect, nicolasfella

Reviewed By: #kde_connect, nicolasfella

Subscribers: nicolasfella, apol, kdeconnect

Tags: #kde_connect

Differential Revision: https://phabricator.kde.org/D15608
This commit is contained in:
Simon Redman 2018-10-07 21:46:25 -06:00
parent ab33cce5a5
commit a7db3ab5e1
6 changed files with 88 additions and 129 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
plugins/sms/conversationsdbusinterface.cpp
View file

@ -43,9 +43,25 @@ ConversationsDbusInterface::~ConversationsDbusInterface()
{
}
QStringList ConversationsDbusInterface::activeConversations()
QVariantList ConversationsDbusInterface::activeConversations()
{
return m_conversations.keys();
QList<QVariant> toReturn;
toReturn.reserve(m_conversations.size());
for (auto it = m_conversations.cbegin(); it != m_conversations.cend(); ++it) {
const auto& conversation = it.value().values();
if (conversation.isEmpty()) {
// This should really never happen because we create a conversation at the same time
// as adding a message, but better safe than sorry
qCWarning(KDECONNECT_CONVERSATIONS)
<< "Conversation with ID" << it.key() << "is unexpectedly empty";
break;
}
const QVariantMap& message = (*conversation.crbegin()).toVariant();
toReturn.append(message);
}
return toReturn;
}
void ConversationsDbusInterface::requestConversation(const QString& conversationID, int start, int end)
@ -57,13 +73,15 @@ void ConversationsDbusInterface::requestConversation(const QString& conversation
qCWarning(KDECONNECT_CONVERSATIONS) << "Got a conversationID for a conversation with no messages!" << conversationID;
}
// TODO: Check local cache before requesting new messages
// TODO: Make Android interface capable of requesting small window of messages
m_smsInterface.requestConversation(conversationID);
// Messages are sorted in ascending order of keys, meaning the front of the list has the oldest
// messages (smallest timestamp number)
// Therefore, return the end of the list first (most recent messages)
int i = start;
for(auto it = messagesList.crbegin(); it != messagesList.crend(); ++it) {
for(auto it = messagesList.crbegin() + start; it != messagesList.crend(); ++it) {
Q_EMIT conversationMessageReceived(it->toVariant(), i);
i++;
if (i >= end) {
@ -88,7 +106,7 @@ void ConversationsDbusInterface::addMessage(const ConversationMessage &message)
// Tell the world about what just happened
if (newConversation) {
Q_EMIT conversationCreated(threadId);
Q_EMIT conversationCreated(message.toVariant());
} else {
Q_EMIT conversationUpdated(message.toVariant());
}

9
plugins/sms/conversationsdbusinterface.h
View file

@ -54,9 +54,12 @@ public:
public Q_SLOTS:
/**
* Return a list of the threadID for all valid conversations
* Return a list of the first message in every conversation
*
* Note that the return value is a list of QVariants, which in turn have a value of
* QVariantMap created from each message
*/
QStringList activeConversations();
QVariantList activeConversations();
void requestConversation(const QString &conversationID, int start, int end);
@ -71,7 +74,7 @@ public Q_SLOTS:
void requestAllConversationThreads();
Q_SIGNALS:
Q_SCRIPTABLE void conversationCreated(const QString& threadID);
Q_SCRIPTABLE void conversationCreated(const QVariantMap& msg);
Q_SCRIPTABLE void conversationRemoved(const QString& threadID);
Q_SCRIPTABLE void conversationUpdated(const QVariantMap& msg) const;
Q_SCRIPTABLE void conversationMessageReceived(const QVariantMap& msg, int pos) const;

66
smsapp/conversationlistmodel.cpp
View file

@ -23,6 +23,7 @@
#include <QLoggingCategory>
#include "interfaces/conversationmessage.h"
#include "interfaces/dbusinterfaces.h"
Q_LOGGING_CATEGORY(KDECONNECT_SMS_CONVERSATIONS_LIST_MODEL, "kdeconnect.sms.conversations_list")
@ -30,7 +31,7 @@ ConversationListModel::ConversationListModel(QObject* parent)
: QStandardItemModel(parent)
, m_conversationsInterface(nullptr)
{
qCCritical(KDECONNECT_SMS_CONVERSATIONS_LIST_MODEL) << "Constructing" << this;
qCDebug(KDECONNECT_SMS_CONVERSATIONS_LIST_MODEL) << "Constructing" << this;
auto roles = roleNames();
roles.insert(FromMeRole, "fromMe");
roles.insert(AddressRole, "address");
@ -48,19 +49,31 @@ ConversationListModel::~ConversationListModel()
void ConversationListModel::setDeviceId(const QString& deviceId)
{
qCCritical(KDECONNECT_SMS_CONVERSATIONS_LIST_MODEL) << "setDeviceId" << deviceId << "of" << this;
if (deviceId == m_deviceId)
if (deviceId == m_deviceId) {
return;
}
qCDebug(KDECONNECT_SMS_CONVERSATIONS_LIST_MODEL) << "setDeviceId" << deviceId << "of" << this;
if (m_conversationsInterface) {
disconnect(m_conversationsInterface, SIGNAL(conversationCreated(QString)), this, SLOT(handleCreatedConversation(QString)));
disconnect(m_conversationsInterface, SIGNAL(conversationCreated(QVariantMap)), this, SLOT(handleCreatedConversation(QVariantMap)));
disconnect(m_conversationsInterface, SIGNAL(conversationUpdated(QVariantMap)), this, SLOT(handleConversationUpdated(QVariantMap)));
delete m_conversationsInterface;
m_conversationsInterface = nullptr;
}
m_deviceId = deviceId;
// This method still gets called *with a valid deviceID* when the device is not connected while the component is setting up
// Detect that case and don't do anything.
DeviceDbusInterface device(deviceId);
if (!(device.isValid() && device.isReachable())) {
return;
}
m_conversationsInterface = new DeviceConversationsDbusInterface(deviceId, this);
connect(m_conversationsInterface, SIGNAL(conversationCreated(QString)), this, SLOT(handleCreatedConversation(QString)));
connect(m_conversationsInterface, SIGNAL(conversationMessageReceived(QVariantMap,int)), this, SLOT(createRowFromMessage(QVariantMap,int)));
connect(m_conversationsInterface, SIGNAL(conversationCreated(QVariantMap)), this, SLOT(handleCreatedConversation(QVariantMap)));
connect(m_conversationsInterface, SIGNAL(conversationUpdated(QVariantMap)), this, SLOT(handleConversationUpdated(QVariantMap)));
prepareConversationsList();
m_conversationsInterface->requestAllConversationThreads();
@ -68,20 +81,31 @@ void ConversationListModel::setDeviceId(const QString& deviceId)
void ConversationListModel::prepareConversationsList()
{
if (!m_conversationsInterface->isValid()) {
qCWarning(KDECONNECT_SMS_CONVERSATIONS_LIST_MODEL) << "Tried to prepareConversationsList with an invalid interface!";
return;
}
QDBusPendingReply<QVariantList> validThreadIDsReply = m_conversationsInterface->activeConversations();
QDBusPendingReply<QStringList> validThreadIDsReply = m_conversationsInterface->activeConversations();
setWhenAvailable(validThreadIDsReply, [this](const QStringList& convs) {
clear();
for (const QString& conversationId : convs) {
handleCreatedConversation(conversationId);
setWhenAvailable(validThreadIDsReply, [this](const QVariantList& convs) {
clear(); // If we clear before we receive the reply, there might be a (several second) visual gap!
for (const QVariant& headMessage : convs) {
QDBusArgument data = headMessage.value<QDBusArgument>();
QVariantMap message;
data >> message;
handleCreatedConversation(message);
}
}, this);
}
void ConversationListModel::handleCreatedConversation(const QString& conversationId)
void ConversationListModel::handleCreatedConversation(const QVariantMap& msg)
{
m_conversationsInterface->requestConversation(conversationId, 0, 1);
createRowFromMessage(msg);
}
void ConversationListModel::handleConversationUpdated(const QVariantMap& msg)
{
createRowFromMessage(msg);
}
void ConversationListModel::printDBusError(const QDBusError& error)
@ -99,11 +123,8 @@ QStandardItem * ConversationListModel::conversationForThreadId(qint32 threadId)
return nullptr;
}
void ConversationListModel::createRowFromMessage(const QVariantMap& msg, int row)
void ConversationListModel::createRowFromMessage(const QVariantMap& msg)
{
if (row != 0)
return;
const ConversationMessage message(msg);
if (message.type() == -1) {
// The Android side currently hacks in -1 if something weird comes up
@ -127,10 +148,19 @@ void ConversationListModel::createRowFromMessage(const QVariantMap& msg, int row
}
item->setData(message.threadID(), ConversationIdRole);
}
// Update the message if the data is newer
// This will be true if a conversation receives a new message, but false when the user
// does something to trigger past conversation history loading
bool oldDateExists;
qint64 oldDate = item->data(DateRole).toLongLong(&oldDateExists);
if (!oldDateExists || message.date() >= oldDate) {
// If there was no old data or incoming data is newer, update the record
item->setData(message.address(), AddressRole);
item->setData(message.type() == ConversationMessage::MessageTypeSent, FromMeRole);
item->setData(message.body(), Qt::ToolTipRole);
item->setData(message.date(), DateRole);
}
if (toadd)
appendRow(item);

5
smsapp/conversationlistmodel.h
View file

@ -87,8 +87,9 @@ public:
void setDeviceId(const QString &/*deviceId*/);
public Q_SLOTS:
void handleCreatedConversation(const QString& conversationId);
void createRowFromMessage(const QVariantMap& message, int row);
void handleCreatedConversation(const QVariantMap& msg);
void handleConversationUpdated(const QVariantMap& msg);
void createRowFromMessage(const QVariantMap& message);
void printDBusError(const QDBusError& error);
private:

6
smsapp/conversationmodel.cpp
View file

@ -61,7 +61,11 @@ void ConversationModel::setDeviceId(const QString& deviceId)
return;
qCDebug(KDECONNECT_SMS_CONVERSATION_MODEL) << "setDeviceId" << "of" << this;
if (m_conversationsInterface) delete m_conversationsInterface;
if (m_conversationsInterface) {
disconnect(m_conversationsInterface, SIGNAL(conversationMessageReceived(QVariantMap, int)), this, SLOT(createRowFromMessage(QVariantMap, int)));
disconnect(m_conversationsInterface, SIGNAL(conversationUpdated(QVariantMap)), this, SLOT(handleConversationUpdate(QVariantMap)));
delete m_conversationsInterface;
}
m_deviceId = deviceId;

97
smsapp/qml/ContactList.qml
View file

@ -1,97 +0,0 @@
/*
* This file is part of KDE Telepathy Chat
*
* Copyright (C) 2015 Aleix Pol Gonzalez <aleixpol@blue-systems.com>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
import QtQuick 2.5
import QtQuick.Controls 2.1
import QtQuick.Layouts 1.1
import org.kde.people 1.0
import org.kde.plasma.core 2.0 as Core
import org.kde.kirigami 2.2 as Kirigami
import org.kde.kdeconnect 1.0
Kirigami.ScrollablePage
{
Component {
id: chatView
ConversationDisplay {}
}
ListView {
id: view
currentIndex: 0
model: PersonsSortFilterProxyModel {
requiredProperties: ["phoneNumber"]
sortRole: Qt.DisplayRole
sortCaseSensitivity: Qt.CaseInsensitive
sourceModel: PersonsModel {}
}
header: TextField {
id: filter
placeholderText: i18n("Filter...")
width: parent.width
onTextChanged: {
view.model.filterRegExp = new RegExp(filter.text)
view.currentIndex = 0
}
Keys.onUpPressed: view.currentIndex = Math.max(view.currentIndex-1, 0)
Keys.onDownPressed: view.currentIndex = Math.min(view.currentIndex+1, view.count-1)
onAccepted: {
view.currentItem.startChat()
}
Shortcut {
sequence: "Ctrl+F"
onActivated: filter.forceActiveFocus()
}
}
delegate: Kirigami.BasicListItem
{
hoverEnabled: true
readonly property var person: PersonData {
personUri: model.personUri
}
label: display
icon: decoration
function startChat() {
applicationWindow().pageStack.push(chatView, { person: person.person, device: Qt.binding(function() {return devicesCombo.device })})
}
onClicked: { startChat(); }
}
}
footer: ComboBox {
id: devicesCombo
readonly property QtObject device: currentIndex>=0 ? model.data(model.index(currentIndex, 0), DevicesModel.DeviceRole) : null
enabled: count > 0
displayText: enabled ? undefined : i18n("No devices available")
model: DevicesSortProxyModel {
//TODO: make it possible to sort only if they can do sms
sourceModel: DevicesModel { displayFilter: DevicesModel.Paired | DevicesModel.Reachable }
onRowsInserted: if (devicesCombo.currentIndex < 0) {
devicesCombo.currentIndex = 0
}
}
textRole: "display"
}
}