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

Sending an image with a caption in telegram then editing the caption causes the image to get sent again #41

Closed
Chris-Bitler opened this issue Oct 11, 2017 · 12 comments
Labels
improvement Improves on something that already exists research Research and more data is needed to make a decision Telegram Issues relating to Telegram bridge
Milestone

Comments

@Chris-Bitler
Copy link
Contributor

If you edit the caption on an image on telegram it will re-upload the image and send it to the irc chat again after the caption is edited:
image
image

@Chris-Bitler
Copy link
Contributor Author

@jflory7 @robbyoconnor Is this intended or something we should fix? It might make more sense to send a message saying that the title was updated without reuploading the image.

@robbyoconnor
Copy link
Contributor

robbyoconnor commented Oct 11, 2017 via email

@jwflory
Copy link
Member

jwflory commented Oct 12, 2017

@Chris-Bitler: This behavior comes from the functionality either @thenaterhood or @repkam09 added for edited messages. Normally for a message, if it's edited, a new copy is sent as a new line to IRC. The edited message functionality is just resending the message, which in this case, is a copy of the image.

I'm more curious if we can retrieve the subtitle / caption and insert that for an image. If we could include the caption, then I'd actually prefer for it to resend the full message with the picture. Anyone know if this is possible?

@robbyoconnor
Copy link
Contributor

robbyoconnor commented Oct 12, 2017 via email

@Chris-Bitler
Copy link
Contributor Author

@jflory7 It is possible to get the caption and it actually already does, that's why it resends the image. Look at the title above the image in irc in the single quotes ('). It was @ dan first, then I edited it to be @ schneidy, which updated the title on the image. Also, in this case it's not just resending the image, it is completely re-uploading it to imgur (which means it's going through the entire code flow for photo messages again). That not be an issue, but it doesn't seem intended

@jwflory jwflory added the improvement Improves on something that already exists label Feb 16, 2018
@jwflory
Copy link
Member

jwflory commented Mar 14, 2018

Looking back on this, is there a way to distinguish regular messages versus edit messages? If we can fistinguish messages that are edits, I think we can figure out a way to extract only a caption from an edit message.

If not, I think we should close issue out as wontfix, since I think this will be intended behavior (however inconvenient).

@jwflory jwflory added this to the v1.2 milestone Mar 14, 2018
@jwflory jwflory modified the milestones: v1.2, v1.2.1 Oct 14, 2018
@jwflory jwflory modified the milestones: v1.2.2, v1.3 Nov 24, 2018
@jwflory jwflory added help wanted Anyone is welcome to help us with this! good first issue Good for newcomers labels Nov 24, 2018
@xforever1313
Copy link
Member

A potential approach:

Now that we have the TgHandlers split out, we could add a new function to each of those classes. At the moment those classes only have the constructor and the RelayMessage function. We could add an additional function called RelayEditedMessage that we can hook into TgEventListener (and by extension, add "edited" events).

We could add new events to TgEventListener called "edited_message", "edited_photo", "edited_document", etc, and have each TgHandler class subscribe to those events by calling a new RelayEditedMessage function. TgEventListener has ParseMessage, we can add ParseEditedMessage (which #110 takes care of for us).

On an edited message:

  • TgMessageHandler prepends the edit with the edit prefix (from Add [EDITED] prefix for edited messages on irc. #110), and sends the message
  • TgJoinHandler behaves the same (probably not even applicable).
  • TgPartHandler behaves the same (probably not even applicable).
  • TgErrorHandler behaves the same.
  • TgPhotoHandler, TgDocumentHandler, TgStickerHandler can prepend the message with the edit prefix, and I honestly see no reason why we couldn't just repost the link.
  • TgImgurHandler can prepend the messaged with the edit prefix, but not upload to imgur.

@jwflory jwflory added priority:crit and removed good first issue Good for newcomers help wanted Anyone is welcome to help us with this! labels Feb 2, 2019
@jwflory
Copy link
Member

jwflory commented Feb 2, 2019

Discussed in 2019-02-02 RITlug developers' meeting. This issue is targeted for the v1.3 milestone, estimated to release on March 2nd, 2019.


@nic-hartley determined a non-admin user is able to edit a message for two days max. Thus, we could create a cache of media messages from two days and keep track of what Imgur URLs were generated for messages sent over those two days. When a user edits a media message caption, uploading the image to Imgur could be skipped and we reuse an existing URL. For admin users editing messages older than two days, then re-uploading the image is acceptable.

@nic-hartley volunteered to work on this one, but it will likely fit in towards the end of the sprint (after #44). We'll revisit at the next developers' meeting on Saturday, Feb. 9th.

Helpful resource: Telegram Bot API, editMediaMessage

@jwflory
Copy link
Member

jwflory commented Mar 3, 2019

This ticket is pushed to the backlog. It didn't make the change window for v1.3. We decided to hold off from including it in v1.4 because while annoying, it doesn't crucially disrupt the functioning of Teleirc.

@jwflory jwflory removed this from the v1.3 milestone Mar 3, 2019
@jwflory jwflory added this to the v2.x.x milestone Feb 16, 2020
@jwflory jwflory added the duplicate This issue or pull request already exists label Feb 27, 2020
@jwflory jwflory removed this from the v2.x.x milestone Feb 27, 2020
@jwflory
Copy link
Member

jwflory commented Feb 27, 2020

We are rewriting the photo handler in Go for the v2.0.0 rewrite (#163). #195 is a new issue to discuss the photo handler and I left a comment with a note about not uploading the image twice.

Given we are re-architecting the photo handler for v2.0.0 anyways, I don't think we will back-port a fix for v1.x.x releases. I'm closing this issue as a duplicate and we can drive further discussion in #195. 🎬

@jwflory jwflory closed this as completed Feb 27, 2020
@jwflory jwflory added Telegram Issues relating to Telegram bridge research Research and more data is needed to make a decision and removed duplicate This issue or pull request already exists labels Apr 3, 2020
@jwflory jwflory added this to the v2.x.x milestone Apr 3, 2020
@jwflory
Copy link
Member

jwflory commented Apr 3, 2020

Reopening per discussion in #195.


v2.0.0 will include the image handler, but caching uploaded images to avoid re-uploading the same image will be handled in a v2.x.x release.

We will have to introduce some sort of state by implementing this. I don't think it is essential for the state to be persistent, so we could use something like Redis or SQLite for this. Preferably something that has no end-user setup required and will mostly "just work." Persistence across restarts is nice but not essential.

Basically there are two, maybe three, pieces of data we will want to cache together from Telegram:

  • Telegram message ID / timestamp
  • Imgur URL for an uploaded image
  • Image caption text (maybe not though?)

I think a local database stored in a flatfile is sufficient for this. It would be persistent across restarts this way so long as the directory contents are preserved across restarts (might not be true for containers). Adding a configurable path for this flatfile is probably ideal, but default can be in present working directory. Back in my Minecraft plugin days, SQLite was pretty popular for stuff like this but I think that was because of excellent library support. Some more research is needed on this issue for how we handle it and where this cached data ultimately lives.

@jwflory jwflory reopened this Apr 3, 2020
@Tjzabel
Copy link
Member

Tjzabel commented Jul 30, 2021

So, for whatever reason, version 2.0+ does not have this issue anymore. Closing as fixed during version migration 🎉

@Tjzabel Tjzabel closed this as completed Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves on something that already exists research Research and more data is needed to make a decision Telegram Issues relating to Telegram bridge
Projects
None yet
Development

No branches or pull requests

5 participants