Roleboard #46

gmcaguilar · 2018-01-24T11:14:03Z

Merge Rolecall v2 to master for publishing

…by_name

…unction

… in the server

…role to an existing one

## To make a commit, type your commit message and press CTRL-ENTER. To cancel ## the commit, close the window. To sign off on the commit, press CTRL-S. ## ## You may also reference or close a GitHub issue with this commit. To do so, ## type `#` followed by the `tab` key. You will be shown a list of issues ## related to the current repo. You may also type `owner/repo#` plus the `tab` ## key to reference an issue in a different GitHub repo. rolecall/UI Specification | 9 ++++++++- rolecall/sample.png | Bin 287260 -> 0 bytes 2 files changed, 8 insertions(+), 1 deletion(-)

…ions

…D strings instead of channel objects

…g entry

…constructed as desired

Chovin · 2018-01-24T12:31:40Z

rolecall/rolecall.py

+                next_key = self.reaction_user_queue.pop()
+                reaction = self.reaction_queue.pop(next_key)
+                await self.process_event(reaction)
+            await asyncio.sleep(0.0001)


can probably wait longer than this 😬

updated ridiculous sleep time to a less ridiculous 0.1 seconds as of commit 34af3cf

Chovin · 2018-01-24T12:33:00Z

rolecall/rolecall.py

@@ -101,73 +46,166 @@ class RoleCall:
    def __init__(self, bot):
        self.bot = bot
        self.settings = dataIO.load_json(SETTINGS_PATH)
+        self.reaction_queue = {}
+        self.reaction_user_queue = set()
+        self.queue_processor_task = bot.loop.create_task(self.queue_processor())


to be nice and tidy, you might want to __unload this task as well

implemented in commit 336c2f1

Chovin · 2018-01-24T12:39:11Z

rolecall/rolecall.py

+        # chosen role board
+        try:
+            await self.post_role(entry)
+        except Exception as e:


be careful with catch-all try/excepts

solved with commit be67606

Chovin · 2018-01-24T12:40:05Z

rolecall/rolecall.py

+        try:
+            await self.create_or_edit_role_channel(server, role_object, role_channel)
+        except Exception as e:
+            print(e)


be careful with catch-all try/excepts.
also, to be nice to your users, you might want to use Red's logger instead of printing the exception directly to the console

Chovin · 2018-01-24T12:54:42Z

rolecall/rolecall.py

-        settings = self.settings[server.id]
+        # retrieve channel mentions in the command message
+        channels = ctx.message.raw_channel_mentions
+        if len(channels) == 2:


this and your raw_role_mentions has incorrect logic.
[p]rolecall add chan1 "test @role1 #chan2 #chan3" role2 :eyes: chan4
will cause the mentions in the message to be used instead of the role and channel in the actual argument

applied correct logic as of 47f7037

Chovin · 2018-01-24T12:55:36Z

rolecall/rolecall.py

+    async def rolecall_add(self, ctx, channel: discord.Channel, 
+                            content_or_message_id: str, role: str, 
+                            emoji: str, 
+                            private_channel: str = None,


once we use this for awhile, let's come up with a simpler more memorable command signature

we definitely should 🤒

Chovin · 2018-01-24T12:59:18Z

rolecall/rolecall.py

+
+        server_id = entry.server.id
+        roleboard_id = entry.roleboard_channel.id
+        message_id = entry.content_or_message_id


for sanity, you should probably add a separate attribute for message id in Entry which is either None or the message id and pull from that.

Chovin · 2018-01-24T13:06:36Z

rolecall/rolecall.py

-    async def roleboard_channel(self, ctx, channel: discord.Channel=None):
-        """Set the roleboard for this server.
+    @rolecall.command(pass_context=True, name="add", no_pm=True)
+    async def rolecall_add(self, ctx, channel: discord.Channel, 


anybody can use this command

fixed with commit 2b9b585

Chovin · 2018-01-24T13:07:25Z

rolecall/rolecall.py


    @commands.group(pass_context=True, no_pm=True)
-    async def roleboard(self, ctx):
-        """change roleboard settings"""
+    async def rolecall(self, ctx):


we should probably add more entry management before we publish.
doesn't need to be in-depth.

Chovin · 2018-01-25T04:16:40Z

@jasperdanan also noticed #48 and #47

Chovin · 2018-01-25T12:55:23Z

rolecall/rolecall.py

@@ -52,7 +53,8 @@ def __init__(self, bot):

    def _record_entry(self, entry: Entry):
        """ record entry to settings file """
-
+        if type(entry.emoji) is discord.Emoji:


please use isinstance instead
https://docs.quantifiedcode.com/python-anti-patterns/readability/do_not_compare_types_use_isinstance.html

used isinstance as of 707ad4b

… classes #47

…message content and they are mistaken to be arguments

…ole to two different emojis

…message already

Chovin · 2018-01-29T09:46:44Z

rolecall/rolecall.py

+        message_id = entry.content_or_message_id
+        keyring = self.settings[server_id][roleboard_id][message_id]
+        for emoji,role in keyring.items():
+            if entry.role.id == role['ROLE_ID']:


do you make sure 1 role can only have 1 entry?

Chovin · 2018-01-29T09:50:41Z

rolecall/rolecall.py

+
+        # check if channel was mentioned
+        if private_channel is None: 
+            pass
        else:


can we do if private_channel is not None: do_things() instead?

I think I prefer it this way instead of using double negatives that may be mildly confusing to some people, or to me when I'm sleepy :3

Chovin · 2018-01-29T09:55:06Z

rolecall/rolecall.py

@@ -333,7 +380,7 @@ def _get_object_by_name(self, otype, server, name, ignore_case=True):
        """ returns object if it exists, otherwise create the object """
        if object_type == "role":               # for roles
            role = discord.utils.get(server.roles, name=object_name)
-            try:                                # try in case role = None
+            try:                                #       try in case role = None


ok?
please don't blindly commit ;)

this is what fixed the spamming problem for me. lol jk

fixed redundant indentation as of 37d82d4

…ct to be None

Chovin · 2018-02-01T04:12:15Z

rolecall/rolecall.py

                role_object = discord.utils.get(server.roles, id=potential_role_id)
-        else:
-            role_object = await self.get_or_create("role", role, server)


whhhhhat?
you're getting or creating a role, but then if it already exists, you're deleting the role and then trying to get it again?

No. Check if the passed argument contains a role id(AKA a role mention) and if it does, delete the role that was created earlier and just get the role associated with the id found

…unctional since the bot would be powerless

…oid discrepancies

Chovin · 2018-03-08T09:18:47Z

rolecall/rolecall.py

+            potential_role_id = nums_found[0]
+            if potential_role_id in [r.id for r in server.roles]:
+                role_object = discord.utils.get(server.roles, id=potential_role_id)
+


this doesn't return anything

Chovin and others added 30 commits October 9, 2017 13:29

get_role_by_name and generalized exceptions for that and get_channel_…

2a006d2

…by_name

Merge branch 'master' into roleboard

5a726ed

made get_or_create method

ee7e78a

added UI Specification and appended missing await keyword for async f…

ea2bbbd

…unction

removed redundant .lower() call

08ffe37

Merge branch 'master' into roleboard

3a77e99

change loop in get_or_create to discord.utils.get

6bab6b4

fix indentation

c8ab930

add a try-except in case role is not found in the list

3c41795

add try except blocks in case user has no privileges to create a role…

d10669e

… in the server

add comments to get_or_create

f760ee4

added create_channel functionality if channel specified does not exist

1803c2e

added description for get_or_create method

aa0051e

added reaction argument

641d8b0

fixed case-sensitive bug for channel names

c92f6e3

implemented check if the user is trying to create a channel or add a …

6328964

…role to an existing one

created make_entry function for constructing roleboard entries

cef629b

fix un-awaited coro function call

2ae8cc2

transferred create channel feature to the 2nd channel argument

3bb1914

added .gitignore for sample screenshots

369ee69

initial entries data structure draft

2c70697

updated default JSON format

fb08ef2

use raw_channel_mentions instead to account for repeated channel ment…

aeb15b8

…ions

use get_channel since raw_channel_mentions return a list of channel I…

256481d

…D strings instead of channel objects

updated description for add command

b02be32

additional updates to add command description

ef29123

removed embed option and added feature that adds a role to an existin…

4e145ec

…g entry

wrote post_role function

fe9f92a

no need to list the roles since it hinders freedom for message to be …

a0b5088

…constructed as desired

Chovin reviewed Jan 24, 2018

View reviewed changes

applied fix for acceptance of both unicode and custom emojis #47

fadf7e9

Chovin reviewed Jan 25, 2018

View reviewed changes

gmcaguilar added 5 commits January 26, 2018 17:24

used isinstance instead for type checking to allow support for parent…

707ad4b

… classes #47

applied fix for the case when roles or channels are mentioned in the …

47f7037

…message content and they are mistaken to be arguments

added fix for spam prevention issue by splitting up the reaction set #48

34af3cf

added a duplicate role prevention feature to avoid linking the same r…

56f59ed

…ole to two different emojis

created isduplicate() to check if emoji or role has been used in the …

5a676e1

…message already

Chovin reviewed Jan 29, 2018

View reviewed changes

gmcaguilar added 4 commits January 31, 2018 08:42

remove redundant .png ignore

c670fcf

remove unused import of threading module

6d7ebd4

fixed accidental comment indentation

37d82d4

fixed incorrect logic on role creation that could cause the role_obje…

fb26927

…ct to be None

Chovin reviewed Feb 1, 2018

View reviewed changes

gmcaguilar added 5 commits February 8, 2018 16:16

added permission checking to avoid creation of entries that are non-f…

62a7e2c

…unctional since the bot would be powerless

created __unload method for cancelling looped task

336c2f1

used specific Exception classes instead of catch-all exceptions to av…

be67606

…oid discrepancies

added check for mod status if command is invoked

2b9b585

create a separate method for checking if a role was mentioned

c11ed06

Chovin requested changes Mar 8, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Roleboard #46

Roleboard #46

gmcaguilar commented Jan 24, 2018

Chovin Jan 24, 2018

gmcaguilar Jan 30, 2018

Chovin Jan 24, 2018

gmcaguilar Feb 8, 2018

Chovin Jan 24, 2018

gmcaguilar Feb 8, 2018

Chovin Jan 24, 2018

Chovin Jan 24, 2018

gmcaguilar Jan 30, 2018

Chovin Jan 24, 2018

gmcaguilar Jan 30, 2018

Chovin Jan 24, 2018

Chovin Jan 24, 2018

gmcaguilar Feb 9, 2018

Chovin Jan 24, 2018

Chovin commented Jan 25, 2018

Chovin Jan 25, 2018

gmcaguilar Jan 30, 2018

Chovin Jan 29, 2018

Chovin Jan 29, 2018

gmcaguilar Jan 30, 2018

Chovin Jan 29, 2018

gmcaguilar Jan 30, 2018

gmcaguilar Jan 30, 2018

Chovin Feb 1, 2018

gmcaguilar Feb 1, 2018

Chovin Mar 8, 2018

Roleboard #46

Are you sure you want to change the base?

Roleboard #46

Conversation

gmcaguilar commented Jan 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chovin commented Jan 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment