-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
QA: Only display multiplier details if not 1.0
#48
Comments
TESTshould not display any multiplier info to a normal bounty hunter with default multiplier settings |
/assign |
Tips:
|
/unassign |
You have been unassigned from the bounty @EresDev |
TESTIf the multiplier is not 1.0 then show the multiplier number, reason, and total. |
/multiplier 0.7 @web4er "testing" |
Successfully changed the payout multiplier for @web4er to 0.7. The reason provided is "testing". |
/assign |
Tips:
|
/unassign |
You have been unassigned from the bounty @web4er |
TESTIf the multiplier is 1.0 and a reason is set, show all the multiplier details, but don't show the total. |
/multiplier 1 @web4er "testing again" |
Successfully changed the payout multiplier for @web4er to 1. The reason provided is "testing again". |
/assign |
Tips:
|
TESTdon't show any multiplier info when message is set as an empty string and multiplier is 1 This basically brings back the default mode, when nothing was done to a hunter's multiplier. |
/multiplier 1 @web4er "" |
Successfully changed the payout multiplier for @web4er to 1. The reason is not provided. |
/unassign |
You have been unassigned from the bounty @web4er |
/assign |
Tips:
|
/unassign |
You have been unassigned from the bounty @web4er |
I'm curious about this one. You should be able to set your multiplier and not add an empty string, for example, /multiplier @pavlovcik 1 Should be the correct way to not set a reason. Technically because you set an empty string, shouldn't the reason be an empty string instead of null? In which case shouldn't the reason render? I actually do like this result, where the bot understands that an empty string means null but still just curious about it. |
Yes, you can skip the reason from the command and it will work as it was working before. Writing some tests for this below. I don't know if it was intentional, but one thing I didn't like in the code was, appending an empty space to the reason. This was before my changes:
This creates confusion in the code because when you provide "" as a reason, it stores it as " " (notice the space). And they evaluate to different booleans. ""==false and " "==true when it is checked in different places in the code. With my change "" will be treated as "no reason provided". If you want to have an empty reason, you can explicitly provide it with a space " " or maybe something even more explicit "N/A" or "Not Applicable" when you don't want to have a reason. |
/multiplier @web4er 1 |
Successfully changed the payout multiplier for @web4er to 1. The reason is not provided. |
It is unlikely to be intentional. Perhaps we should fix it! By the way, can you send me a message on Telegram? We could use your attention to detail to help with some QA efforts! |
The pull request with this QA fixes it. |
Steps for each test
The text was updated successfully, but these errors were encountered: