Added ability to edit message with message builder

[Morazzer]

Added ability to edit messages with the message builder.

Issue: #575

[bassner]

Since it seems like the original author isn't coming back to this (and I need this myself actually), I've changed this to remove the boolean flags and check for the content field to be filled at all.
@KILLEliteMaste Please lmk fi this makes sense for you now. :)

[felldo]

I think we can reuse nearly everything from #send(TextChannel) for this. There are also a lot of things missing like the allowed mentions, flags... See https://discord.com/developers/docs/resources/channel#edit-message-jsonform-params

If for some reason you do not reuse the code from the #send method you should also remove the check to update the message content as it is not possible atm to remove it.

[bassner]

For flags: We currently do not support flags at all, do we? I mean any of those: https://discord.com/developers/docs/resources/channel#message-object-message-flags

So all that's missing is allowed mentions and files/attachments.

you should also remove the check to update the message content

I don't understand this one. I've intentionally added the check. All requests trying to patch in an empty content will either fail (completely empty message won't work) or produce unexpected results (e.g. it'd remove content from a message that also contains an embed although this is not wanted), so we need to exclude it from the request if the content is empty / unset, right? This is important for requests where we e.g. only want to patch in some components:

Message m = ...;
new MessageBuilder().addComponents(...).edit(m);

We do not use Message#toMessageBuilder() since we don't want to change anything else than components in this case, so we create a new one (like you suggested in your complaint about the boolean flags)

[felldo]

All requests trying to patch in an empty content will either fail (completely empty message won't work) or produce unexpected results (e.g. it'd remove content from a message that also contains an embed although this is not wanted), so we need to exclude it from the request if the content is empty / unset, right?

But how else should you be able to edit a message which contains a content and embed to only having an embed?

For flags: We currently do not support flags at all, do we? I mean any of those:

We do: EPHEMERAL for example as this was the only Message flag available back this time we implemented it.

And taking your example

Message m = ...;
new MessageBuilder().addComponents(...).edit(m);

You example assumes that we patch in the new stuff. I was thinking more about we replace the message with the the new MessageBuilder

[bassner]

But how else should you be able to edit a message which contains a content and embed to only having an embed?

Yeah, that is a problem. But we should really avoid to send the entire message in the patch request, even tho we only change the components (for example) ... So how would we do it? We could compare everything against the Message object passed in as parameter to edit() and only send differences ... in theory ...

the only Message flag available back this time we implemented it.

I think you're confusing https://discord.com/developers/docs/interactions/slash-commands#interaction-response-object-interaction-application-command-callback-data-flags with the link I sent above; those are a different set of flags and existed for a long time already when we added support for ephemeral interaction responses, didn't they?
Also, the only thing we can change through the flags field is the SUPPRESS_EMBEDS thing ... everything else will be ignored anyways, so does it matter at all?

I was thinking more about we replace the message with the the new MessageBuilder

Yeah we could do that as well, but it would need to inlcude the difference checking stuff I mentioned above.

[bassner]

Alternatively to the difference chec which would be complex to do, we could track wether or not any of the methods affect content have been called on the MessageBuilder, and send an empty content field if they have been intentionally set to be empty.

Second alternative would be to have a new interface MessageUpdater, similar to ServerUpdater, to make the intention more clear. I agree that having a MessageBuilder with edit would imply that all fields of the builder are applied, even if empty (and thus replace the message).

[felldo]

I think you're confusing...

Prorbably yeah. But since these are 2 different Enums our current implementation for setFlags in InteractionMessageBuilderBase and maybe some other places is wrong as this assumes MessageFlag which is then something different than the Interaction Application Command Callback Data Flags.

I think introducing a message updater is a good idea. This clearly differenciates the MessageBuilder which sends a message to updating a message. And this also allows us to have different updating methods like updateSetFields and replaceMessage without cluttering everything in the MessageBuilder and it makes it clear how you are updating the message.

[bassner]

our current implementation for setFlags ... is wrong

I don't think so. EPHEMERAL is indeed to only flag that can be set when sending interaction responses. Offering anything else would currently be misleading.
We could have another enum to represent the flags of an already sent message, maybe, for example to know wether or not a message has been crossposted or not, but I wouldn't consider that high priority. Might deserve an issue, though. Also, except for the SUPPRESS_EMBEDS flags nothing is updateable, so we could just have a method like Message#suppressEmbeds() to set the flag to true. That would be the same thing as the little "X" that appear on the top right of embeds in the UI.

I think introducing a message updater is a good idea.

I'll create a new pull request for that approach. I'll update this PR. I also like it better than having an .edit() on the builder. I don't know wether or not we should make a difference between "UpdateSetFields" and "ReplaceMessage", though. The first one is how an updater usually works

[felldo]

I don't think so. EPHEMERAL is indeed to only flag that can be set when sending interaction responses. Offering anything else would currently be misleading.

Because exactly of this the current implementation is not correct as MessageFlag also contains i.e. SUPPRESS_EMBEDS. So we have to create a 2nd enum which contains only the valid flags repsonding to an interaction.

The first one is how an updater usually works

Yes, but I think there are actually more use cases for replacing a message than just adding e.g. a component to a message. It's also more clear what the message will actually look like after when replacing it.