Move the Approve button to the notification channel set in appsettings.json + minor change regarding the Waiting For Approval message
#6
Conversation
ProgramowanieBot/Modules/Interactions/ButtonInteractions/ApproveInteraction.cs
Outdated
Show resolved
Hide resolved
ProgramowanieBot/Modules/Interactions/ButtonInteractions/ApproveInteraction.cs
Show resolved
Hide resolved
ProgramowanieBot/Modules/Interactions/UserMenuInteractions/ResolveInteraction.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Kuba_Z2 <[email protected]>
…nel ID and the word "post" should only be used when reffering to the database object
…ion to avoid confusion
…ly long code with the method call wherever possible or needed
ProgramowanieBot/Modules/ApplicationCommands/SlashCommands/ResolveCommand.cs
Outdated
Show resolved
Hide resolved
…ix for a previous mistake
Co-authored-by: Kuba_Z2 <[email protected]>
|
Apologies for this mess. I forgot about this PR when I made changes to my fork. This PR is still to be reviewed and merged. |
| await PostsHelper.SendPostResolveMessagesAsync(channelId, Context.User.Id, helper.Id, helper2?.Id, Context.Client.Rest, configuration); | ||
|
|
||
| var isHelper2 = helper2 != null && helper != helper2; | ||
| var user = Context.User; | ||
| return InteractionCallback.Message(new() | ||
| { | ||
| Content = $"**{configuration.Emojis.Success} {(isHelper2 ? string.Format(configuration.Interaction.WaitingForApprovalWith2HelpersResponse, helper, helper2) : string.Format(configuration.Interaction.WaitingForApprovalResponse, helper))}**", | ||
| Components = | ||
| [ | ||
| new ActionRowProperties( | ||
| [ | ||
| new ActionButtonProperties($"approve:{helper.Id}:{helper != user}:{(isHelper2 ? helper2!.Id : null)}:{(isHelper2 ? helper2 != user : null)}", configuration.Interaction.ApproveButtonLabel, ButtonStyle.Success), | ||
| ]), | ||
| ], | ||
| AllowedMentions = AllowedMentionsProperties.None, | ||
| Content = configuration.Emojis.Success, | ||
| Flags = MessageFlags.Ephemeral | ||
| }); | ||
|
|
There was a problem hiding this comment.
Isn't it better to send the message about resolve in the interaction response instead of responding with an emoji and sending a normal message?
You also added an unnecessary blank line at the end.
There was a problem hiding this comment.
One of the main points of this PR, besides moving the Approve button, was to change this exact behaviour that was adopted earlier. I wanted to get rid of the ugly text that said "Original message was deleted" (as seen on the screenshot).
Naturally, when it comes to using a slash command this does not take place, however, I wanted to maintain some sort of consistency and make all the Post Resolve messages regular messages instead of replies or interaction responses.
Also, thank you for your additional feedback regarding my code formatting. I will include that in my next commit.
There was a problem hiding this comment.
As promised, formatting issue fixed with 2eee47c.
There was a problem hiding this comment.
But I think slash commands look good. "Original message was deleted" happens only when using a component of an ephemeral message.
There was a problem hiding this comment.
I am aware of that, but, again, all I wanted was a bit of consistency. Moreover, changing that would require either changing the helper method or not using it, which would require more code while the change wouldn't be neither that noticeable nor that important for the user.
There was a problem hiding this comment.
The whole change won't be noticeable by users at all but I think sending an interaction response with an emoji is not a good idea, it's better to just respond to the interaction with the resolve message. We have more inconsistency with the current approach either.
There was a problem hiding this comment.
So what are you suggesting? Abandoning the helper in this particular case, or adding an exception to the method where if it's a slash command it will use an interaction response rather than a regular message?
There was a problem hiding this comment.
You can create another overload (or method because maybe the current name would be confusing) that is for slash commands. The 2 methods can also share some private helpers to make the code less repetitive.
No description provided.