Allow edited messages and interactions to be sent without removing existing embeds

[TapGhoul]

Allows replies to be sent with embeds nulled out in the API, which prevents removing existing embeds under the context of editing an existing message response, both for interactions and standard messages.

This changes existing behavior - now, when not setting any embeds, existing embeds are kept. To clear them, you would call reply.embeds(vec![])

Pairs with serenity-rs/serenity#2968 - this change can be made without it, but together they fix followup messages too. They can be merged separately, though the documentation on the embed() method is considered incorrect until this is merged on Serenity's side.

Fixes #300

[jamesbt365]

This is a breaking change because:

• CreateReply's fields are public and you changed them
• It changes the expected behaviour for users requiring them to make code changes on minor version bumps.

Target next.

Additionally, the current behaviour is apparently more intuitive, and you still have access to the old embeds when handling a ReplyHandle, so its not like the current way is impossible to work with.

poise/src/reply/mod.rs

Lines 107 to 111 in 5750259

	// Clear builder so that adding embeds or attachments won't add on top of
	// the pre-edit items but replace them (which is apparently the more
	// intuitive behavior). Notably, setting the builder to default doesn't
	// mean the entire message is reset to empty: Discord only updates parts
	// of the message that have had a modification specified

The only thing with any mild annoyance is that there isn't a nice way to keep attachments without reuploading them like the serenity builders?

[TapGhoul]

Oop! My bad on the public field breaking change, did this very early in the morning. All good!

The code you highlighted applies to replies, not interactions. Interactions force clear embeds every time, hence the referenced PR in Serenity core too, as this is incorrect on their side as well, which was my focus. However, by force-setting embeds every time, you actually explicitly remove all embeds, unless you supply the full embeds list again - and this is not supplied in the edit/reply builder.

---

As for the embeds, feel free to correct me if I'm wrong as I'm just doing a code walk, not running tests against Poise itself (though I created the pair of PRs out of observed misbehavior, and tested against the API directly) - but from what I understand that's not how that code works, and the comment is outdated.

• To start off with, you construct CreateReply with CreateReply::default() - there is no way to go from a message to a builder that I can find. So no prefill here.
• For all branches (for reply, edit interaction initial and edit interaction followup) Poise first takes in the CreateReply struct, and modify it by applying ephimeral and allow_mentions set as per the command + framework options if not already set. All good thus far.
• Pose then runs the conversion methods (to_prefix_edit(), to_slash_initial_response_edit() and to_slash_followup_response()) with an empty response and tack on what's needed. Fine so far.

Now here's where things go wrong: When generating these, you then always explicitly set .embeds() - on EditMessage, this is where embeds get forcefully cleared. There is no way for the discord API to append new embeds to an existing list of embeds on a message, only set or leave them - I believe there was once some old behavior that did this, but this hasn't been the case in a long time. But your code explicitly removes embeds each time, rather than allowing them to remain.

In the case of interactions, it gets worse - while the same behavior as above applies, serenity itself steps on your toes itself in the case of editing the followup interaction responses (though not the initial one). Even if you prevent this, serenity made the embeds field mandatory by mistake. The serenity-side PR mentioned in my initial comment fixes that, and a similar fix was merged previously for the CreateInteractionResponseData call (now called CreateInteractionResponseMessage) - serenity-rs/serenity#2216.

Poise use this in your own pagination builtin, when pagination occurs on the initial message, if triggered by an interaction such as a slash command. You don't see any issues with yours because you rebuild the embed for every page, but in situations where this is not the case, this breaks.

The test code used to perform this in verifying my claims in this comment, using xh (replace with curl or your favourite HTTP API testing software):

send() {
	xh $1 "https://discord.com/api/v10/$2" Authorization:'Bot <token>' User-Agent:'DiscordBot (<url>, 0.0.1)' ${@:3}
}

# Create the message, take note of the ID
send POST channels/<channelid>/messages content=hi embeds:='[{"title":"hi"}]'
# This will edit the message, but not remove embeds
send PATCH channels/<channelid>/messages/<msgid> content=hello
# This will remove the existing embed, and replace it with the one given here.
send PATCH channels/<channelid>/messages/<msgid> content=hello embeds:='[{"title":"hello"}]'
# This will remove all embeds, and is what Poise does by default
send PATCH channels/<channelid>/messages/<msgid> content=hello embeds:='[]'

[jamesbt365]

The code you highlighted applies to replies, not interactions.

ReplyHandle is a wrapper around the bot's interaction responses and the bot's prefix responses. It is returned from poise's methods on Context and is primarily used to edit or delete responses.

Am I missing anything here? You mention editing the response which ReplyHandle is used for.

[TapGhoul]

When calling handle.edit(), you feed in a builder you construct yourself. It does not give you a pre-configured builder. On build inside edit, it converts it into the underlying Serenity type, and uses the serenity message/interaction methods to fill in the message ID and other context held within the handle's inner (as opposed to Poise managing that itself).

ReplyHandle is fine (though has an outdated comment), ReplyBuilder is the one with issues, and does not get constructed from ReplyHandle directly. It is instead fed into the edit method, already hand-assembled by the user.

[TapGhoul]

Trying to get it properly on poise next. Will reopen once I get it sorted.

[jamesbt365]

Take this as a lesson to stop opening PRs from the main branch of your fork.

[TapGhoul]

I mean, that's the flow github offers up now. Used to not, used to autogenerate a branch called patch-1. I guess something changed. Either way, opened #302.