From b755c62377ad3356ecec302e3d77f7d83b9993ad Mon Sep 17 00:00:00 2001
From: Andrei <b1naryth1ef@gmail.com>
Date: Sun, 13 Aug 2017 15:53:52 -0700
Subject: [PATCH] [guild] rework role update interface

Previously to update a role you would update attributes of the role
object, and then call save. This was very much an anti-pattern when
compared to the way things work elsewhere, and had the following
annoyances attached to it;

- Updating attributes of the object actually smashes state, we aren't
guarenteed that the user will call save nor that the API will accept our
save, and thus attributes on the object (which could be shared within
the state module) are smashed and innaccurate.
- Guild.update_role is effectively useless in this case, we can only
ever pass a role object in. This makes partial updates impossible, and
forces us to match the OO interface to use it.

The new style follows along with how we do things elsewhere, there is
still likely some additional work that can be done here to allow passing
in the role, but for now we'll just call this a deprecation
---
 disco/api/client.py  | 19 +++++++++++++++++--
 disco/types/guild.py | 18 +++++++-----------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/disco/api/client.py b/disco/api/client.py
index efeacb8..f5c8fe4 100644
--- a/disco/api/client.py
+++ b/disco/api/client.py
@@ -389,11 +389,26 @@ class APIClient(LoggingClass):
         r = self.http(Routes.GUILDS_ROLES_MODIFY_BATCH, dict(guild=guild), json=roles, headers=_reason_header(reason))
         return Role.create_map(self.client, r.json(), guild_id=guild)
 
-    def guilds_roles_modify(self, guild, role, reason=None, **kwargs):
+    def guilds_roles_modify(self, guild, role,
+            name=None,
+            hoist=None,
+            color=None,
+            permissions=None,
+            position=None,
+            mentionable=None,
+            reason=None):
+
         r = self.http(
             Routes.GUILDS_ROLES_MODIFY,
             dict(guild=guild, role=role),
-            json=kwargs,
+            json=optional(
+                name=name,
+                hoist=hoist,
+                color=color,
+                permissions=permissions,
+                position=position,
+                mentionable=mentionable,
+            ),
             headers=_reason_header(reason))
         return Role.create(self.client, r.json(), guild_id=guild)
 
diff --git a/disco/types/guild.py b/disco/types/guild.py
index a19adc0..f29b729 100644
--- a/disco/types/guild.py
+++ b/disco/types/guild.py
@@ -116,8 +116,8 @@ class Role(SlottedModel):
     def delete(self):
         self.guild.delete_role(self)
 
-    def save(self):
-        self.guild.update_role(self)
+    def update(self, *args, **kwargs):
+        self.guild.update_role(self, *args, **kwargs)
 
     @property
     def mention(self):
@@ -410,15 +410,11 @@ class Guild(SlottedModel, Permissible):
         """
         self.client.api.guilds_roles_delete(self.id, to_snowflake(role))
 
-    def update_role(self, role):
-        return self.client.api.guilds_roles_modify(self.id, role.id, **{
-            'name': role.name,
-            'permissions': role.permissions.value,
-            'position': role.position,
-            'color': role.color,
-            'hoist': role.hoist,
-            'mentionable': role.mentionable,
-        })
+    def update_role(self, role, **kwargs):
+        if 'permissions' in kwargs and isinstance(kwargs['permissions'], PermissionValue):
+            kwargs['permissions'] = kwargs['permissions'].value
+
+        return self.client.api.guilds_roles_modify(self.id, to_snowflake(role), **kwargs)
 
     def sync(self):
         if self.synced: