`!remind edit` bugs and potential reminder enhancements

[TizzySaurus]

trafficstars

Further to discussion with @HassanAbouelela in #dev-contrib, I'm creating this issue to outline some potential changes to the !remind command that can be made.

Bugs:

• !remind edit doesn't validate that the reminder was actually changed before sending the API call and sending success message. Resolving this would mean having to GET the reminder being edited so that the appropriate fields can be compared.
• !remind edit content doesn't support using a referenced message, but !remind new does.

Enhancements:

• Update !remind list to actually mention the reminder's mentions instead of displaying the .name attribute, since it's in an embed (won't ping) and will be more explicit.
• Add support for deleting multiple reminders at once (means changing the typehint from int to Greedy[int] and then iterating).

@HassanAbouelela suggested that PRs should ideally be limited in the amount of things they add, so I'd do a separate PR for each of the bugs, and then both enhancements in 1 PR since the first is a one-line change.

[HassanAbouelela]

Doing the enhancements together sounds fine 👍

I have a couple concerns wrt the implementation of the other two.

Bug 1. I don't know if it's worth it to do a second API call, when the current behavior doesn't seem to be a problem. I don't mind either way though

Bug 2. Not sure this is a bug, more of a new feature. Off the top of my head, I don't believe there's a one-to-one relationship between a message and a reminder. You can create multiple reminders to one message, and you can have multiple users with multiple reminders to the same message. You can have reminders with no messages. How would replying to a message tie it to the reminder?

[TizzySaurus]

Bug 1. I don't know if it's worth it to do a second API call, when the current behavior doesn't seem to be a problem. I don't mind either way though

It's not really a "problem", more a QOL thing. It just seems weird to have an edit command that doesn't actually verify if it was edited. If we decide it's not worth due to the API call then that's fine with me.

How would replying to a message tie it to the reminder? The same way it does for !remind new, i.e. use the referenced message's content as the reminder content, or if there's no content, it'll instead set the content to "See referenced message".

If we want different behaviour, then !remind new should be updated to reflect the new behaviour as well.

[HassanAbouelela]

I had Tizzy explain to me on discord. The proposed command in bug 2 is remind edit content <reminder ID>, then have the body of the reply become the new body of the reminder. I don't think this is intuitive behavior, but we do use it in other places in our bots so it could become expected I suppose. I won't stop it, I'm mostly indifferent.

[TizzySaurus]

After further discussion on discord, I've decided that Bug 1 is fine as-is, and not really a big deal.

The two enhancements have been implemented in #2232, and Bug 2 has been implemented in #2233. Once both of these have been merged, this can be closed.

[shtlrs]

@HassanAbouelela We forgot to close this. (I'm going through the list of bugs we have & noticed it)

[HassanAbouelela]

Thanks, it's been closed