ehForwarderBot / efb-telegram-master

EFB Telegram Master Channel, a channel for EH Forwarder Bot.
GNU Affero General Public License v3.0
223 stars 79 forks source link

cache replyto chat #69

Closed wolfsilver closed 5 years ago

wolfsilver commented 5 years ago

so no need to replyto every msg

blueset commented 5 years ago

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 commented 5 years ago

usually, we need to reply to a message in a private or group with multi slaves. so the etm can know where to send. this pr cache the destination when you reply to a message. then, if you send a message without replied infomation, it will use the last cached destination.

blueset commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

wolfsilver commented 5 years ago

Yes. It worked fine for me. Maybe using a configuration to set the expire time is better way.

Eana Hufwe notifications@github.com 于2019年9月14日周六 下午12:23写道:

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

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/blueset/efb-telegram-master/pull/69?email_source=notifications&email_token=AASWXYWUDOOC4GDP4COMKLLQJRRMNA5CNFSM4ISOUHN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6WT6TY#issuecomment-531447631, or mute the thread https://github.com/notifications/unsubscribe-auth/AASWXYR6R4QQ7E73S2V34KTQJRRMNANCNFSM4ISOUHNQ .

wolfsilver commented 5 years ago

I have no better practice. So I just take all your advice except https://github.com/blueset/efb-telegram-master/pull/69#discussion_r324426834 Please review again.

blueset commented 5 years ago

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