Skip to content

Allow forwarding of media, to media channels #1266

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
+45 −16
Open

Allow forwarding of media, to media channels #1266

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d20f984
Allow forwarding of media, to media channels
billpapat Jun 28, 2025
2742573
fix: Grace period & clean up
billpapat Jun 29, 2025
a21f294
Embed only when !content.isEmpty()
billpapat Jul 1, 2025
869da01
I reversed the code (removed the timer and the id list) until I find a
billpapat Jul 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 55 additions & 5 deletions ...ion/src/main/java/org/togetherjava/tjbot/features/mediaonly/MediaOnlyChannelListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import net.dv8tion.jda.api.entities.Message;
import net.dv8tion.jda.api.entities.MessageEmbed;
import net.dv8tion.jda.api.entities.MessageType;
import net.dv8tion.jda.api.entities.messages.MessageSnapshot;
import net.dv8tion.jda.api.events.message.MessageReceivedEvent;
import net.dv8tion.jda.api.requests.RestAction;
import net.dv8tion.jda.api.utils.messages.MessageCreateBuilder;
Expand All @@ -13,6 +14,7 @@
import org.togetherjava.tjbot.features.MessageReceiverAdapter;

import java.awt.Color;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;

Expand All @@ -25,6 +27,13 @@
*/
public final class MediaOnlyChannelListener extends MessageReceiverAdapter {

private static final Pattern MEDIA_URL_PATTERN = Pattern.compile(
".*https?://\\S+\\.(png|jpe?g|gif|bmp|webp|mp4|mov|avi|webm|mp3|wav|ogg|youtube\\.com/|youtu\\.com|imgur\\.com/).*",
Pattern.CASE_INSENSITIVE);

private final ConcurrentHashMap<Long, Long> lastValidForwardedMediaMessageTime =
new ConcurrentHashMap<>();

/**
* Creates a MediaOnlyChannelListener to receive all message sent in MediaOnly channel.
*
Expand All @@ -45,19 +54,60 @@ public void onMessageReceived(MessageReceivedEvent event) {
return;
}

if (messageHasNoMediaAttached(message)) {
message.delete().flatMap(any -> dmUser(message)).queue(any -> {
}, failure -> tempNotifyUserInChannel(message));
long userId = event.getAuthor().getIdLong();

boolean isForwardedWithMedia =
!message.getMessageSnapshots().isEmpty() && !messageHasNoMediaAttached(message);

if (isForwardedWithMedia) {
lastValidForwardedMediaMessageTime.put(userId, System.currentTimeMillis());
return;
}

boolean isNormalMediaUpload =
message.getMessageSnapshots().isEmpty() && !messageHasNoMediaAttached(message);
if (isNormalMediaUpload) {
return;
}

Long lastForwardedMediaTime = lastValidForwardedMediaMessageTime.get(userId);
long gracePeriodMillis = TimeUnit.SECONDS.toMillis(1);

if (lastForwardedMediaTime != null
&& (System.currentTimeMillis() - lastForwardedMediaTime) <= gracePeriodMillis) {
lastValidForwardedMediaMessageTime.remove(userId);
return;
}

message.delete().queue(deleteSuccess -> dmUser(message).queue(dmSuccess -> {
}, dmFailure -> tempNotifyUserInChannel(message)),
deleteFailure -> tempNotifyUserInChannel(message));
}

private boolean messageHasNoMediaAttached(Message message) {
return message.getAttachments().isEmpty() && message.getEmbeds().isEmpty()
&& !message.getContentRaw().contains("http");
if (!message.getAttachments().isEmpty() || !message.getEmbeds().isEmpty()
|| MEDIA_URL_PATTERN.matcher(message.getContentRaw()).matches()) {
return false;
}

if (!message.getMessageSnapshots().isEmpty()) {
for (MessageSnapshot snapshot : message.getMessageSnapshots()) {
if (!snapshot.getAttachments().isEmpty() || !snapshot.getEmbeds().isEmpty()
|| MEDIA_URL_PATTERN.matcher(snapshot.getContentRaw()).matches()) {
return false;
}
}
return true;
}

return true;
Copy link
Member

Choose a reason for hiding this comment

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

im not happy with how this logic is currently structured.

the main structure should be:

if (!is there any media?) {
  delete it
} else {
  forward it
}

the details of detecting media should be in a helper method. and the details of that cache handling as well.

i also dont quite get the point of the cache thing. what is it used for? and how does the UX look like if they run into it?
for the cache topic u should use high level classes like Duration and Instant instead.

Copy link
Author

Choose a reason for hiding this comment

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

Semi-done!

Copy link
Author

@billpapat billpapat Jun 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The bot previously failed to recognize forwarded media such as images, voice messages, and GIFs, processing them as plain text and subsequently deleting them. The updated version's handling of such media is illustrated in the accompanying images.

image

If the forwarded message contains no media (i.e., it's plain text), it will be automatically deleted, and the user will be notified as usual.

image

Copy link
Member

Choose a reason for hiding this comment

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

okay. and what is the role of the grace period thing now?

Copy link
Author

@billpapat billpapat Jul 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Imagine a user tries to forward a media file (like an image or video) to a "media-only" Discord channel. Discord might process upcoming (potential) additional comment added by the sender as message without content (text).

Without a grace period: If the bot immediately checks after the forwarded message, the added comment would be incorrectly deleted even though the user did forward an image or video to post media.

image
Discord's forwarding system allows for the addition of extra comment (text) that is being sent separately than the embed, meaning it will be deleted by the bot for no-media (we do not want forwarded media without context). That is what the grace period is for, waits 1 second after forwarded media (that have passed the messageHasNoMediaAttached) to allow for the comment to not be deleted.

Copy link
Member

Choose a reason for hiding this comment

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

im having a super hard time following ur explanation. it sounds like ur saying there are multiple messages involved. when a user sends an image its a single message isnt it.
my advice is to please work more with screenshots or gifs/videos. make it visual and it will be easy to understand 👍

}

private MessageCreateData createNotificationMessage(Message message) {
String originalMessageContent = message.getContentRaw();
if (originalMessageContent.trim().isEmpty()) {
originalMessageContent = "(Original message had no visible text content)";
Copy link
Member

Choose a reason for hiding this comment

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

that text is written with a developer as target audience, not the user. id rewrite it as See image: or similar.

Copy link
Author

Choose a reason for hiding this comment

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

Not sure what you mean by that. It checks for .isEmpty() meaning I cannot return "See image:" Since there is not one

Copy link
Member

Choose a reason for hiding this comment

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

what i mean is that if this is a user facing text, its not written in a user friendly way.

}

MessageEmbed originalMessageEmbed =
new EmbedBuilder().setDescription(originalMessageContent)
Expand Down
Loading