the-draupnir-project / Draupnir

A Matrix moderation bot
https://the-draupnir-project.github.io/draupnir-documentation/
Academic Free License v3.0
68 stars 13 forks source link

Missing Reason Prompt from command handler. #441

Open FSG-Cat opened 1 month ago

FSG-Cat commented 1 month ago

image

The command Draupnir Ban !draupnir ban @evil1234:evil.com should return a prompt for Reason And policy list. As the inspect element edited picture shows it only returns a Policy list selection.

Please note that the red section is essentially the stuff i removed via inspect element. Its there and renders perfectly fine.

Whats missing is the whole section about reason selection.

Quick reasoning about labels. L4 due to well most users use the command handler but its not manditory P3 if your on mobile this is quite severe like its not completely unusable but calling it outrageous yes. T5 well the command hanlder exists to be mobile friendly and well that key usecase is completely impaired by this flaw.

Edit: Replicated on following version strings. v2.0.0-beta.3,v2.0.0-beta.4

Gnuxie commented 1 month ago

@FSG-Cat So, when changed the code to promptForAccept https://github.com/the-draupnir-project/Draupnir/commit/fb8f76906b6847fb2eed1c631b0ab8b1f39de39c (to allow prompts to live indefinitely and not time out), we broke some old behavior.

The old behaviour existed on shakey ground to start with but the problem was that the ban command has arguments like this:

ban <entity> <list> [...reason], that's to say the list is required but can be prompted if ignored and the reason is entirely optional. The old behavior was that we prompted for the reason too when list was omitted, but that's not really sound, because how can we know that a command that omits list doesn't also want to omit the reason?

So how do we get around this problem?

One option is to change the prompt behavior so that optional arguments that have a prompt are prompted for in all cases where the argument is missing. However, this would mean that you can't make a ban with no reason anymore, which could be frustrating to some users (but if it's really needed we can add an option flag for that)? I'm not sure.

I've not thought about other solutions yet.

Gnuxie commented 1 month ago

Cat says why not just add "no reason supplied" to the default prompts