Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache replyto chat #69

Merged
merged 11 commits into from Sep 15, 2019
Merged

cache replyto chat #69

merged 11 commits into from Sep 15, 2019

Conversation

wolfsilver
Copy link
Contributor

so no need to replyto every msg

so no need to replyto every msg
@blueset
Copy link
Member

blueset commented Aug 31, 2019

Hi, can you further elaborate on what this PR changes? It'd be great if you can update the README file if this PR involves changes in user interaction.

Thanks!

@wolfsilver
Copy link
Contributor Author

wolfsilver commented Aug 31, 2019 via email

@blueset
Copy link
Member

blueset commented Sep 1, 2019

Thanks for the explanation.

Several other users has also been suggesting similar features trying to avoid quote reply when sending messages to non-linked or multi-linked chats. However, I always have this concern where this kind of logic could lead to unintended behavior, especially when a lot messages has been received between two of such messages sent to Telegram.

For example, with the logic you have suggested:

In a chat thread directly with bot:                               Sent to   

+--------------------------------------------------------------+            
|                                                              |            
| ETM Bot                                                      |            
| +-----------------------------+                              |            
| | Chat A:                     |                              |            
| | Lorem ipsum dolor sit amet. |                              |            
| +-----------------------------+                              |            
|    ^                                                     You |            
|    |    (quote-reply to)    +------------------------------+ |            
|    +------------------------| Consectetur adipiscing elit. | |  Chat A    
|                             +------------------------------+ |            
| ETM Bot                                                      |            
| +--------------------------------+                           |            
| | Chat A:                        |                           |            
| | Nullam hendrerit ullamcorper.  |                           |            
| +--------------------------------+                           |            
|                                                          You |            
|                                   +------------------------+ |            
|                                   | Finibus, etiam ut sem. | |  Chat A    
|                                   +------------------------+ |            
| ETM Bot                                                      |            
| +-------------------------------+                            |            
| | Chat A:                       |                            |            
| | Vitae augue feugiat eleifend. |                            |            
| +-------------------------------+                            |            
|                                                              |            
|                                                              |            
.                                                              .            
.                     < Few hours later >                      .            
.                                                              .            
|                                                              |            
|                                                              |            
|                                                              |            
|  ETM Bot                                                     |            
|  +---------------------------+                               |            
|  | Chat B:                   |                               |            
|  | Nunc vestibulum interdum! |                               |            
|  +---------------------------+--------+                      |            
|  | Chat C:                            |                      |            
|  | Non scelerisque quam venenatis in. |                      |            
|  +-----------------------------------++                      |            
|  | Chat C:                           |                       |            
|  | Nunc hendrerit nunc eu justo      |                       |            
|  | auctor, at porttitor metus orane. |                       |            
|  +-------------------------------+---+                       |            
|  | Chat C:                       |                           |            
|  | Etiam pulvinar neque sit amet |                           |            
|  | libero eleifend tincidunt.    |                           |            
|  +-------------------------------+                           |            
|                                                          You |            
|                                    +-----------------------+ |            
|                                    | [Something intended   | |  Chat A    
|                                    | to reply to Chat C.]  | |            
|                                    +-----------------------+ |            
|                                                              |            
+--------------------------------------------------------------+            

After a longer period of time, the user might just forget who they last replied. And when they got a long chunk of messages from another chat, they could either simply assume the next message they sent would be to that new chat, or they could simply forget to do the "quote-reply" action.

In either case, the new message would be delivered to unintended recipient and cause unnecessary problem once sent. Furthermore, ETM's default behavior doesn't include a delivery receipt for most messages (including the new message mentioned above). The user wouldn't event notice that the message is sent to the wrong chat.


Some other users have suggested that messages with no indicated recipient should be sent to the chat where last message in that Telegram thread is from. This would also prone to misdelivery where messages from multiple chats are coming continuously to this Telegram thread. See the char below for details.

Sequence chart
+----------+        +----------+       +----------+         +----------+
|  Slave   |        |   ETM    |       | Telegram |         |   User   |
| channel  |        |          |       |  Server  |         |          |
+----+-----+        +----+-----+       +-----+----+         +----+-----+
     | Message from      |                   |                   |      
     | chat A            |                   |                   |      
     +------------------->                   |                   |      
     |                   | Send message      |                   |      
     |                   | from chat A       |                   |      
     |                   +------------------->                   |      
     | Message from      |                   | Receive message   |      
     | chat B            |                   | from chat A       |      
     +------------------->                   +------------------->      
     |                   | Send message      |                   |      
     |                   | from chat B       | Send message X    |      
     |                   +-------------------> (intended to      |      
     |                   |                   | send to chat A)   |      
     |                   | Send message X    <-------------------+      
     |                   <-------------------+ Receive message   |      
     | Send message X    |                   | from chat B       |      
     | to chat B         |                   +------------------->      
     <-------------------+                   |                   |      
     |                   |                   |                   |      
     |                   |                   |                   |      

If you have better suggestions or further question on this problem, feel free to leave a comment below, or better if you would have another PR.

@wolfsilver
Copy link
Contributor Author

At the beginning, I had thought about this question.
My solution is below, but not finished.(I'm not good at python)
Set a timer to delete the cache after 10 minites or half an hour.
Or set a timestamp when replied to message. Check if it has expired.
This will cover most of the situations. Because if it is not a temporary chat, we will link it to a single group.

@catbaron0
Copy link

Agree with @blueset . I don't think it's a good idea to cache replyto chat, at least not in a way with no explicit notification about which chat the user is replying to. Actually, I think the approach proposed by @wolfsilver that expires a cache after a short period of time, 'cauze it may make user even more confusing. I think we should keep the status consistent without user's operation.

If we really need this feature to reply to a chat instantly with no need to create a new group to link to, maybe we need a new commands to check the chat that the user want to reply to.

In this case, we could cache the last replied chat as this pr does, and users could check the chat they are to reply by typing something like /replyto. This may help solve the problem that

After a longer period of time, the user might just forget who they last replied

@blueset
Copy link
Member

blueset commented Sep 2, 2019

Even with a check command like /replyto, users would simply forget to use that to check recipients, as they would forget to do quote-reply for messages.

If we would take the timeout version of the "cached reply-to" strategy proposed here by @wolfsilver, it's probably better to notify the user about what ETM is going to do on this chat every time it starts to use/change the cache, like:

All messages sent here without recipient indication in the next {timeout} minutes will be sent to "{chat.long_name}" from "{chat.module_name}" until you indicate a new recipient.

However, I would still consider this as an arguable feature, and would prioritize more on other existing proposals, unit tests and stable version releases. As usual, if there are better implementations, I'll be more than grateful to give the PR a review.

@wolfsilver
Copy link
Contributor Author

wolfsilver commented Sep 4, 2019

How about this.

In a chat thread directly with bot:                               Sent to   |     

+--------------------------------------------------------------+            |                   
|                                                              |            |                   
| ETM Bot                                                      |            |                   
| +-----------------------------+                              |            |                   
| | Chat A:                     |                              |            |                   
| | Lorem ipsum dolor sit amet. |                              |            |                   
| +-----------------------------+                              |            |                   
|    ^                                                     You |            |                   
|    |    (quote-reply to)    +------------------------------+ |            |                   
|    +------------------------| Consectetur adipiscing elit. | |  Chat A    |       cache[bot] = A            
|                             +------------------------------+ |            |                   
| ETM Bot                                                      |            |                   
| +--------------------------------+                           |            |                   
| | Chat A:                        |                           |            |                   
| | Nullam hendrerit ullamcorper.  |                           |            |                   
| +--------------------------------+                           |            |                   
|                                                          You |            |                   
|                                   +------------------------+ |            |                   
|                                   | Finibus, etiam ut sem. | |  Chat A    |       reply to cache[bot] (A)           
|                                   +------------------------+ |            |                   
| ETM Bot                                                      |            |                   
| +-------------------------------+                            |            |                   
| | Chat A:                       |                            |            |                   
| | Vitae augue feugiat eleifend. |                            |            |                   
| +-------------------------------+                            |            |                   
|                                                              |            |                   
|                                                              |            |                   
.                                                              .            |                   
.                     < Few hours later >                      .            |       cache[bot] expire after an hour           
.                                                              .            |                   
|                                                              |            |                   
|                                                              |            |                   
|                                                              |            |                   
|  ETM Bot                                                     |            |                   
|  +---------------------------+                               |            |                   
|  | Chat B:                   |                               |            |                   
|  | Nunc vestibulum interdum! |                               |            |                   
|  +---------------------------+--------+                      |            |                   
|  | Chat C:                            |                      |            |                   
|  | Non scelerisque quam venenatis in. |                      |            |                   
|  +-----------------------------------++                      |            |                   
|  | Chat C:                           |                       |            |                   
|  | Nunc hendrerit nunc eu justo      |                       |            |                   
|  | auctor, at porttitor metus orane. |                       |            |                   
|  +-------------------------------+---+                       |            |                   
|  | Chat C:                       |                           |            |                   
|  | Etiam pulvinar neque sit amet |                           |            |                   
|  | libero eleifend tincidunt.    |                           |            |                   
|  +-------------------------------+                           |            |                   
|                                                          You |            |                   
|                                    +-----------------------+ |            |                   
|                                    | [Something intended   | |    None    |       cache[bot] is None, so report an error MS01.            
|                                    | to reply to Chat C.]  | |            |                   
|                                    +-----------------------+ |            |                   
| ETM Bot                                                      |            |                   
| +--------------------------------------------+               |            |                   
| | bot:                                       |               |            |                   
| | Error: No recipient specified.             |               |            |                   
| | Please reply to a previous message. (MS01) |               |            |                   
| +--------------------------------------------+               |            |                      
|                                                              |            |      
|                                                              |            |      
|                                                              |            |      
|    ^                                                     You |            |                   
|    |    (quote-reply to C)  +------------------------------+ |            |                   
|    +------------------------| Consectetur adipiscing elit. | |  Chat C    |       cache[bot] = C         
|                             +------------------------------+ |            |                    
| ETM Bot                                                      |            |                   
| +--------------------------------+                           |            |                   
| | Chat C:                        |                           |            |                   
| | Nullam hendrerit ullamcorper.  |                           |            |                   
| +--------------------------------+                           |            |                   
|                                                          You |            |                   
|                                   +------------------------+ |            |                   
|                                   | Finibus, etiam ut sem. | |  Chat C    |       reply to cache[bot] (C)           
|                                   +------------------------+ |            |                   
| ETM Bot                                                      |            |                   
| +-------------------------------+                            |            |                   
| | Chat C:                       |                            |            |                   
| | Vitae augue feugiat eleifend. |                            |            |                   
| +-------------------------------+                            |            |                   
|                                                              |            |                    
| ETM Bot                                                      |            |                   
| +-------------------------------+                            |            |                   
| | Chat B:                       |                            |            |        cache[bot] != B, remove cache[bot]       
| | Vitae augue feugiat eleifend. |                            |            |                   
| +-------------------------------+                            |            |                     
|                                                          You |            |                   
|                                    +-----------------------+ |            |                   
|                                    | [Something intended   | |    None    |       cache[bot] is None, so report an error MS01.            
|                                    | to reply to Chat C.]  | |            |                   
|                                    +-----------------------+ |            |                   
| ETM Bot                                                      |            |                   
| +--------------------------------------------+               |            |                   
| | bot:                                       |               |            |                   
| | Error: No recipient specified.             |               |            |                   
| | Please reply to a previous message. (MS01) |               |            |                   
| +--------------------------------------------+               |            |                
|                                                              |            |                      
+--------------------------------------------------------------+            |                   

cache expired after an hour.
if chatting with C, then B send a message. Remove the cache.

The original intention of this pr is to solve the temporary chat group that maybe not used after hours or one day. It's not for long term situations.

@blueset
Copy link
Member

blueset commented Sep 14, 2019

This looks like a better strategy so far. Does your current code work like that?

@wolfsilver
Copy link
Contributor Author

wolfsilver commented Sep 14, 2019 via email

Copy link
Member

@blueset blueset left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Brilliant work in general! 🎉

I've left some comments in the code. Please take a look and see if you have any doubt or question on them.

Also, I happened to encounter some similar code online while searching for the meaning of some comments in code. In case that you have taken any code from other sources, please take care if that is fine copyright-wise. Don't forget to attribute the original author and/or license if required.

efb_telegram_master/cache.py Outdated Show resolved Hide resolved
efb_telegram_master/cache.py Outdated Show resolved Hide resolved
efb_telegram_master/cache.py Show resolved Hide resolved
efb_telegram_master/cache.py Outdated Show resolved Hide resolved
efb_telegram_master/cache.py Outdated Show resolved Hide resolved
efb_telegram_master/cache.py Outdated Show resolved Hide resolved
efb_telegram_master/cache.py Outdated Show resolved Hide resolved
efb_telegram_master/cache.py Outdated Show resolved Hide resolved
efb_telegram_master/master_message.py Outdated Show resolved Hide resolved
efb_telegram_master/slave_message.py Outdated Show resolved Hide resolved
wolfsilver and others added 9 commits September 15, 2019 09:21
Co-Authored-By: Eana Hufwe <ilove@1a23.com>
Co-Authored-By: Eana Hufwe <ilove@1a23.com>
Co-Authored-By: Eana Hufwe <ilove@1a23.com>
Co-Authored-By: Eana Hufwe <ilove@1a23.com>
Co-Authored-By: Eana Hufwe <ilove@1a23.com>
Co-Authored-By: Eana Hufwe <ilove@1a23.com>
Co-Authored-By: Eana Hufwe <ilove@1a23.com>
Co-Authored-By: Eana Hufwe <ilove@1a23.com>
@wolfsilver
Copy link
Contributor Author

I have no better practice. So I just take all your advice except #69 (comment)
Please review again.

@blueset
Copy link
Member

blueset commented Sep 15, 2019

Thanks for the effort! I'll merge this now.

@blueset blueset merged commit e438c7f into ehForwarderBot:master Sep 15, 2019
@wolfsilver wolfsilver deleted the chatcache branch September 15, 2019 03:19
YouXam pushed a commit to YouXam/efb-telegram-master that referenced this pull request Apr 13, 2023
* cache replyto chat

so no need to replyto every msg

* Mod modify cache replied chat logic

* Update efb_telegram_master/cache.py

Co-Authored-By: Eana Hufwe <ilove@1a23.com>

* Update efb_telegram_master/cache.py

Co-Authored-By: Eana Hufwe <ilove@1a23.com>

* Update efb_telegram_master/cache.py

Co-Authored-By: Eana Hufwe <ilove@1a23.com>

* Update efb_telegram_master/cache.py

Co-Authored-By: Eana Hufwe <ilove@1a23.com>

* Update efb_telegram_master/cache.py

Co-Authored-By: Eana Hufwe <ilove@1a23.com>

* Update efb_telegram_master/cache.py

Co-Authored-By: Eana Hufwe <ilove@1a23.com>

* Update efb_telegram_master/slave_message.py

Co-Authored-By: Eana Hufwe <ilove@1a23.com>

* Update efb_telegram_master/cache.py

Co-Authored-By: Eana Hufwe <ilove@1a23.com>

* fix code review problem
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants