Repository: Kraigie/nostrum (elixir)

Issue:
 :GUILD_ROLE_CREATE and :GUILD_ROLE_UPDATE are not including the guild id in the events.
 :GUILD_ROLE_CREATE and :GUILD_ROLE_UPDATE are supposed to include the guild id as part of the event, but they are not. My best **guess** is that something as part of the pluggable guild cache change messed this up. I think the return values of GuildCache.role_create and GuildCache.role_update are supposed to include the guild_id in them after updating the ETS table. 
 
 See [here](https://github.com/Kraigie/nostrum/blob/168367916e2d890a1dcc6e19389ac2dbc9666e37/lib/nostrum/cache/guild_cache/ets.ex#L268)
 
If this is the correct change, id be happy to make a quick PR for it.


Pull request:
Fix GuildCache.ETS not including guild_id in role_create and role_update methods.
I got no issues with the typespecs I changed, but this is the first time I have worked with them before so do double check I got those right.

Closes #376 

Thanks!

Patch:
diff --git a/lib/nostrum/cache/guild_cache.ex b/lib/nostrum/cache/guild_cache.ex
index 8ca0abbd..17530860 100644
--- a/lib/nostrum/cache/guild_cache.ex
+++ b/lib/nostrum/cache/guild_cache.ex
@@ -263,7 +263,7 @@ defmodule Nostrum.Cache.GuildCache do
 
   Return the casted role.
   """
-  @callback role_create(Guild.id(), role :: map()) :: Role.t()
+  @callback role_create(Guild.id(), role :: map()) :: {Guild.id(), new_role :: Role.t()}
 
   @doc """
   Delete the given role on the given guild.
@@ -277,7 +277,8 @@ defmodule Nostrum.Cache.GuildCache do
 
   Return the old role before the update and the updated role.
   """
-  @callback role_update(Guild.id(), role :: map()) :: {old_role :: Role.t(), new_role :: Role.t()}
+  @callback role_update(Guild.id(), role :: map()) ::
+              {Guild.id(), old_role :: Role.t(), new_role :: Role.t()}
 
   @doc """
   Update the voice state of the given guild from upstream data.
diff --git a/lib/nostrum/cache/guild_cache/ets.ex b/lib/nostrum/cache/guild_cache/ets.ex
index dbf4648c..e2e7a680 100644
--- a/lib/nostrum/cache/guild_cache/ets.ex
+++ b/lib/nostrum/cache/guild_cache/ets.ex
@@ -259,13 +259,13 @@ defmodule Nostrum.Cache.GuildCache.ETS do
 
   @doc "Create the given role in the given guild in the cache."
   @impl GuildCache
-  @spec role_create(Guild.id(), map()) :: Role.t()
+  @spec role_create(Guild.id(), map()) :: {Guild.id(), Role.t()}
   def role_create(guild_id, role) do
     [{_id, guild}] = :ets.lookup(@table_name, guild_id)
     {_old, new, new_roles} = upsert(guild.roles, role.id, role, Role)
     new_guild = %{guild | roles: new_roles}
     true = :ets.update_element(@table_name, guild_id, {2, new_guild})
-    new
+    {guild_id, new}
   end
 
   @doc "Delete the given role from the given guild in the cache."
@@ -281,13 +281,13 @@ defmodule Nostrum.Cache.GuildCache.ETS do
 
   @doc "Update the given role in the given guild in the cache."
   @impl GuildCache
-  @spec role_update(Guild.id(), map()) :: {Role.t(), Role.t()}
+  @spec role_update(Guild.id(), map()) :: {Guild.id(), Role.t(), Role.t()}
   def role_update(guild_id, role) do
     [{_id, guild}] = :ets.lookup(@table_name, guild_id)
     {old, new_role, new_roles} = upsert(guild.roles, role.id, role, Role)
     new_guild = %{guild | roles: new_roles}
     true = :ets.update_element(@table_name, guild_id, {2, new_guild})
-    {old, new_role}
+    {guild_id, old, new_role}
   end
 
   @doc "Update guild voice states with the given voice state in the cache."


Test patch:
diff --git a/test/nostrum/cache/guild_cache_test.exs b/test/nostrum/cache/guild_cache_test.exs
index 1567b32e..52146380 100644
--- a/test/nostrum/cache/guild_cache_test.exs
+++ b/test/nostrum/cache/guild_cache_test.exs
@@ -157,15 +157,21 @@ defmodule Nostrum.Cache.GuildCacheTest do
 
         test "role management" do
           # role_create/2
-          expected = Role.to_struct(@test_role)
-          role_id = expected.id
-          assert ^expected = @cache.role_create(@test_guild.id, @test_role)
-          assert {:ok, %Guild{roles: %{^role_id => ^expected}}} = @cache.get(@test_guild.id)
+          expected_guild_id = @test_guild.id
+          expected_role_struct = Role.to_struct(@test_role)
+          expected_return = {expected_guild_id, expected_role_struct}
+          role_id = expected_role_struct.id
+          assert ^expected_return = @cache.role_create(@test_guild.id, @test_role)
+
+          assert {:ok, %Guild{roles: %{^role_id => ^expected_role_struct}}} =
+                   @cache.get(@test_guild.id)
 
           # role_update/2
           updated_payload = Map.put(@test_role, :name, "Higher Sharders")
           new_role = Role.to_struct(updated_payload)
-          {^expected, ^new_role} = @cache.role_update(@test_guild.id, updated_payload)
+
+          {^expected_guild_id, ^expected_role_struct, ^new_role} =
+            @cache.role_update(@test_guild.id, updated_payload)
 
           # role_delete/2
           {_guild_id, ^new_role} = @cache.role_delete(@test_guild.id, @test_role.id)