Add in ability to give negative points. #13 #22
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixed leaderboard errors. Removed user tagging in leaderboards.
Fixing colors indexing error.
Added username validation. Fixed leaderboard truncation.
Fixed username validate.
Adding functions/methods to prevent the bot from sending PM's. Currently working for InMemoryStorage
Added PM Preferences methods to AzureStorage(storage) class
Added methods to RedisStorage to make use of the PM Preference setting. (left commented out)
Preparing to work new issue
-Added negative points to list of accepted commands/commands_with_target -Added logic to _extract_count for negative_emoji
-Added neg_points methods to the base storage class -Coded out neg_points methods in the InMemoryStorage class -Making lint happy
- Added logic to determine whether :NEGATIVE-EMOJI: or :EMOJI: was passed. -get_user_neg_points_remaining(user_id) coded out - 'allneg' command handling - determine whether give or giveneg was implied
- Added `kind` to 'get points remaining' functions/methods - Added handling for give negative points commands
It's unnecessary to check for NEG_POINTS == 'ALLOW'. If NEG_POINTS != 'ALLOW', the NEGATIVE_EMOJI, and NEGATIVE_POINTS environment variables won't be loaded so any check against one of those variables should return false
MattPitlyk
reviewed
Nov 19, 2017
| @@ -19,25 +19,33 @@ | |||
| BOT_ID = os.environ.get("BOT_ID") | |||
| EMOJI = os.environ.get('EMOJI') | |||
| POINTS = os.environ.get('POINTS') | |||
| SELF_POINTS = os.environ.get('SELF_POINTS', "DISALLOW") | |||
| SELF_POINTS = os.environ.get('SELF_POINTS', "ALLOW") | |||
There was a problem hiding this comment.
I think the default should be DISALLOW.
MattPitlyk
reviewed
Nov 19, 2017
| @@ -19,25 +19,33 @@ | |||
| BOT_ID = os.environ.get("BOT_ID") | |||
| EMOJI = os.environ.get('EMOJI') | |||
| POINTS = os.environ.get('POINTS') | |||
| SELF_POINTS = os.environ.get('SELF_POINTS', "DISALLOW") | |||
| SELF_POINTS = os.environ.get('SELF_POINTS', "ALLOW") | |||
| NEG_POINTS = os.environ.get('NEG_POINTS', "ALLOW") | |||
There was a problem hiding this comment.
I think the default should be DISALLOW.
There was a problem hiding this comment.
Agreed. I must have forgotten to switch those off when I was done testing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added NEG_POINTS = 'ALLOW' / 'DISALLOW' environment variable.
If set to allow, it will expect the NEGATIVE_EMOJI, and NEGATIVE_POINTS environment variables.
Added _extract_type to FireballMessage which will return EMOJI or NEGATIVE_EMOJI
Added kind to check_user_points_remaining functions ( check_user_points_remaining(user_id, num, kind==EMOJI) ). Instead of coding another function to check negative points. Can we do things this way, or make separate functions for everything?
Added command handling to the bot
Work out leaderboard issues
Add negative points support to check_points bot commands
Add functionality to Azure/Redis storage
Other things I didn't think of
Basically hoping for a code review to make sure I'm headed in the right direction