- A contact with whom we are not already in a conversation
- An arbitrary phone number
Details
- Reviewers
albertvaka nicolasfella turx - Group Reviewers
KDE Connect
Diff Detail
- Repository
- R224 KDE Connect
- Lint
Lint Skipped - Unit
Unit Tests Skipped
There are some changes in this patch that don't belong here. You probably need to do git rebase master to fix it
plugins/systemvolume/systemvolumeplugin-pulse.cpp | ||
---|---|---|
77 ↗ | (On Diff #46401) | This slipped in by accident. Please remove it |
Let us know if you have any trouble with the rebase. If you did your work on the master branch, you should be able to just do git pull --rebase. If you did your work on a different branch, first change to master, then do git pull, then go back to your branch and do git rebase master
This is good so far. Thank you!
Two small bugs which come from the same source:
If I type the phone number of a contact with whom I do not have a conversation, that gets matched to the contact (nice job!)
Since the filter box has their phone number typed in, the contact is not visible, so you get the error: "TypeError: Cannot call method 'startChat' of null"
There is also an error when you type the phone number for a contact with whom there is already a conversation
However, I think that is more of a bug that was already in the search box. The search box should allow filtering by address as well as by contact name. That way, these situations could never occur.
plugins/sms/conversationsdbusinterface.cpp | ||
---|---|---|
125 | No need to keep commented-out code | |
127 | You pass the conversationID here, which you have hard-coded (in the QML application) to 0. That means that the phone tries to send a message to the phone number "0" which is probably not what was intended. Did you mean message.address()? | |
smsapp/conversationlistmodel.cpp | ||
170–185 | This is a good idea! However, since it is code that was copy-pasted from another method ("createRowFromMessage") could you make a helper function which can be used by both of those? For instance, maybe something like "createItemFromMessage" which returns QStandardItem*. That method gets used directly by "createRowFromMessage". This method ("createRowFromAddress") would construct the dummy message with the dummy "[New Conversation]" body and timestamp, then call the same helper | |
183 | @apol How do we make this translate-able? | |
184 | Do you need to specify this? What happens if you leave it out? | |
smsapp/qml/ConversationList.qml | ||
68 | Is this used for anything? | |
85 | This should be set to 0 instead of 1 |
There is also an error when you type the phone number for a contact with whom there is already a conversation
Something that might help when considering address vs thread_id, is to notice that if you receive an SMS on Android from any contact (ie. Mom/Dad) who share a phone number, Google/Android will just pick one and the messages will go there.
In other words, it may no even be possible to have two different thread_id's for the same address, so the thread_id is really only necessary for fetching messages. In the GSConnect UI, new incoming messages (whether for a new or existing conversation) are "routed" by address, not by thread_id, and I haven't found a situation where that breaks yet.
Oh. Hm. All of our SMS app works on thread_id, with the assumption that they would be unique, or at least the desktop GUI would match the phone GUI. If I'm understanding what you're saying correctly, we're likely going to hit a wall at some point. In any case, fixing that is outside of the scope of this current patch.
smsapp/conversationlistmodel.cpp | ||
---|---|---|
183 | i18n("Text") |
All of our SMS app works on thread_id, with the assumption that they would be unique...
Oh, I believe they are unique, but they are *also* unique to phone number. So there will never be a address with more than one thread_id; any messages received from or sent to that address will always appear in the same thread_id.
So what I mean is, when starting a "new" conversation, you can watch for new messages from the target address and catch the thread_id (probably even from your "sent confirmation" packet).
This can be re-opened when we want to look at this feature again, but I'm closing for now