Repository: hikari-py/hikari
Pull request #1195 (2022-06-26 05:41:02)

Rename `edit_permission_overwrites` to `edit_permission_overwrite`
### Summary
<!-- Small summary of the merge request -->
**Deprecation**. Renames `edit_permission_overwrites` to `edit_permission_overwrite`.

### Checklist
<!-- Make sure to tick all the following boxes by putting an `x` in between (like this `[x]`) -->
- [x] I have run `nox` and all the pipelines have passed.
- [x] I have made unittests according to the code I have added/modified/deleted.

### Related issues
<!--
To mention an issue use `#issue-id` and to mention a merge request use `!merge-request-id`
To close/fix an issue use `Close #issue-id` or `Fix #issue-id` (depending on the merge request)
-->
https://discord.com/channels/574921006817476608/700378161526997003/979493994092449802


parafoxia: Pretty sure "overwrites" is correct here. The phrase "edit permission overwrite" makes no sense.
Lunarmagpie: > Pretty sure "overwrites" is correct here. The phrase "edit permission overwrite" makes no sense.

The discord API docs also refers to it as "permission overwrites"
GoogleGenius: > Pretty sure "overwrites" is correct here. The phrase "edit permission overwrite" makes no sense.

Hmm I thought and agreed with davfsa that "overwrite" made more sense because it edits a specific entry if I understand the method correctly.
GoogleGenius: > > Pretty sure "overwrites" is correct here. The phrase "edit permission overwrite" makes no sense.
> 
> The discord API docs also refers to it as "permission overwrites"

This form is used throughout hikari as well but it's this specific method that edits only a single entry I believe for an overwrite?
GoogleGenius: I'll add the fragment later by the way.
davfsa: > Pretty sure "overwrites" is correct here. The phrase "edit permission overwrite" makes no sense.

There is currently an inconsistency in the lib with this name, with one method calling it overwrites and another one overwrite. Since you are editing a specific permission overwrite, I think the singular is more fitting.

This was fact a todo of mine that seems to have been snipped, lol
GoogleGenius: > This was fact a todo of mine that seems to have been snipped, lol

Heh it was, I just thought I could do it anyway haha. 
GoogleGenius: @davfsa Should the tests should use the old name or new name?
GoogleGenius: Nvm about review, this is failing CI, let me fix.
Jonxslays: > @davfsa Should the tests should use the old name or new name?

The old method deprecated method and the new method should both be tested up until the deprecated method is removed. 
GoogleGenius: > > @davfsa Should the tests should use the old name or new name?
> 
> The old deprecated method and the new method should both be tested up until the deprecated method is removed.

What about methods that use that method? They can just be updated with the new name and the test changed right?
parafoxia: > > Pretty sure "overwrites" is correct here. The phrase "edit permission overwrite" makes no sense.
> 
> There is currently an inconsistency in the lib with this name, with one method calling it overwrites and another one overwrite. Since you are editing a specific permission overwrite, I think the singular is more fitting.
> 
> This was fact a todo of mine that seems to have been snipped, lol

Don't you have the ability to edit multiple overwrites with the same method though?
GoogleGenius: > > > Pretty sure "overwrites" is correct here. The phrase "edit permission overwrite" makes no sense.
> > 
> > 
> > There is currently an inconsistency in the lib with this name, with one method calling it overwrites and another one overwrite. Since you are editing a specific permission overwrite, I think the singular is more fitting.
> > This was fact a todo of mine that seems to have been snipped, lol
> 
> Don't you have the ability to edit multiple overwrites with the same method though?

Hmm, now I'm a bit conflicted here. It makes sense as singular when you look at it from an "overwrite entry" but if you're editing multiple permissions, you can use the bitwise `|` operator on the `hikari.Permissions` enum. Then it would be `edit_overwrite_permissions` rather.
GoogleGenius: I'm also looking at how discord.js and discord.py named this and utterly confused 😕 
davfsa: A permission overwrite is the singular object that consists of a target, what permissions are allowed and which ones are denied.

Since this endpoint only edits a specific one, then `edit_permission_overwrite` makes more sense than `edit_permission_overwrites`, as you are only editing one.

I currently can't check and I don't know it of the top of my head, but the method to bulk ediy them is a different one @parafoxia 
parafoxia: > A permission overwrite is the singular object that consists of a target, what permissions are allowed and which ones are denied.
> 
> Since this endpoint only edits a specific one, then `edit_permission_overwrite` makes more sense than `edit_permission_overwrites`, as you are only editing one.
> 
> I currently can't check and I don't know it of the top of my head, but the method to bulk ediy them is a different one @parafoxia 

Right yeah, I'm seeing what you mean. I'd always taken a "permission overwrite" to be an overwrite of a specific permission, rather than all permissions for a given entity.
parafoxia: @GoogleGenius I've spotted a typo which might as well be fixed in this PR. The docs (existing and updated) say "vale" instead of "value" in the allowed and denied fields.
GoogleGenius: > @GoogleGenius I've spotted a typo which might as well be fixed in this PR. The docs (existing and updated) say "vale" instead of "value" in the allowed and denied fields.

Thanks, will fix.

Patch:
diff --git a/changes/1195.deprecation.md b/changes/1195.deprecation.md
new file mode 100644
index 00000000..4624ce05
--- /dev/null
+++ b/changes/1195.deprecation.md
@@ -0,0 +1,1 @@
+`RESTClient.edit_permission_overwrites` renamed to `RESTClient.edit_permission_overwrite`
diff --git a/hikari/api/rest.py b/hikari/api/rest.py
index 9e074b12..28b5e4a0 100644
--- a/hikari/api/rest.py
+++ b/hikari/api/rest.py
@@ -505,7 +505,7 @@ async def edit_voice_state(
 
     @typing.overload
     @abc.abstractmethod
-    async def edit_permission_overwrites(
+    async def edit_permission_overwrite(
         self,
         channel: snowflakes.SnowflakeishOr[channels_.GuildChannel],
         target: typing.Union[channels_.PermissionOverwrite, users.PartialUser, guilds.PartialRole],
@@ -518,7 +518,7 @@ async def edit_permission_overwrites(
 
     @typing.overload
     @abc.abstractmethod
-    async def edit_permission_overwrites(
+    async def edit_permission_overwrite(
         self,
         channel: snowflakes.SnowflakeishOr[channels_.GuildChannel],
         target: snowflakes.Snowflakeish,
@@ -531,7 +531,7 @@ async def edit_permission_overwrites(
         """Edit permissions for a given entity ID and type."""
 
     @abc.abstractmethod
-    async def edit_permission_overwrites(
+    async def edit_permission_overwrite(
         self,
         channel: snowflakes.SnowflakeishOr[channels_.GuildChannel],
         target: typing.Union[
@@ -560,9 +560,9 @@ async def edit_permission_overwrites(
             If provided, the type of the target to update. If unset, will attempt to get
             the type from `target`.
         allow : hikari.undefined.UndefinedOr[hikari.permissions.Permissions]
-            If provided, the new vale of all allowed permissions.
+            If provided, the new value of all allowed permissions.
         deny : hikari.undefined.UndefinedOr[hikari.permissions.Permissions]
-            If provided, the new vale of all disallowed permissions.
+            If provided, the new value of all disallowed permissions.
         reason : hikari.undefined.UndefinedOr[builtins.str]
             If provided, the reason that will be recorded in the audit logs.
             Maximum of 512 characters.
diff --git a/hikari/channels.py b/hikari/channels.py
index af769dce..dde06869 100644
--- a/hikari/channels.py
+++ b/hikari/channels.py
@@ -1035,9 +1035,9 @@ async def edit_overwrite(
             If provided, the type of the target to update. If unset, will attempt to get
             the type from `target`.
         allow : hikari.undefined.UndefinedOr[hikari.permissions.Permissions]
-            If provided, the new vale of all allowed permissions.
+            If provided, the new value of all allowed permissions.
         deny : hikari.undefined.UndefinedOr[hikari.permissions.Permissions]
-            If provided, the new vale of all disallowed permissions.
+            If provided, the new value of all disallowed permissions.
         reason : hikari.undefined.UndefinedOr[builtins.str]
             If provided, the reason that will be recorded in the audit logs.
             Maximum of 512 characters.
@@ -1074,11 +1074,9 @@ async def edit_overwrite(
             assert not isinstance(
                 target, int
             ), "Cannot determine the type of the target to update. Try specifying 'target_type' manually."
-            return await self.app.rest.edit_permission_overwrites(
-                self.id, target, allow=allow, deny=deny, reason=reason
-            )
+            return await self.app.rest.edit_permission_overwrite(self.id, target, allow=allow, deny=deny, reason=reason)
 
-        return await self.app.rest.edit_permission_overwrites(
+        return await self.app.rest.edit_permission_overwrite(
             self.id, typing.cast(int, target), target_type=target_type, allow=allow, deny=deny, reason=reason
         )
 
diff --git a/hikari/impl/rest.py b/hikari/impl/rest.py
index fac326f6..20ff9441 100644
--- a/hikari/impl/rest.py
+++ b/hikari/impl/rest.py
@@ -1046,7 +1046,7 @@ async def edit_voice_state(
         body.put("suppress", suppress)
         await self._request(route, json=body)
 
-    async def edit_permission_overwrites(
+    async def edit_permission_overwrite(
         self,
         channel: snowflakes.SnowflakeishOr[channels_.GuildChannel],
         target: typing.Union[
@@ -1078,6 +1078,28 @@ async def edit_permission_overwrites(
         body.put("deny", deny)
         await self._request(route, json=body, reason=reason)
 
+    @deprecation.deprecated("2.0.0.dev110", "edit_permission_overwrite")
+    async def edit_permission_overwrites(
+        self,
+        channel: snowflakes.SnowflakeishOr[channels_.GuildChannel],
+        target: typing.Union[
+            snowflakes.Snowflakeish, users.PartialUser, guilds.PartialRole, channels_.PermissionOverwrite
+        ],
+        *,
+        target_type: undefined.UndefinedOr[typing.Union[channels_.PermissionOverwriteType, int]] = undefined.UNDEFINED,
+        allow: undefined.UndefinedOr[permissions_.Permissions] = undefined.UNDEFINED,
+        deny: undefined.UndefinedOr[permissions_.Permissions] = undefined.UNDEFINED,
+        reason: undefined.UndefinedOr[str] = undefined.UNDEFINED,
+    ) -> None:
+        """Edit permissions for a specific entity in the given guild channel.
+
+        .. deprecated:: 2.0.0.dev110
+            Use `RESTClient.edit_permission_overwrite` instead.
+        """
+        await self.edit_permission_overwrite(
+            channel, target, target_type=target_type, allow=allow, deny=deny, reason=reason
+        )
+
     async def delete_permission_overwrite(
         self,
         channel: snowflakes.SnowflakeishOr[channels_.GuildChannel],


Test patch:
diff --git a/tests/hikari/impl/test_rest.py b/tests/hikari/impl/test_rest.py
index be1e20ae..83abb268 100644
--- a/tests/hikari/impl/test_rest.py
+++ b/tests/hikari/impl/test_rest.py
@@ -2158,13 +2158,13 @@ async def test_edit_voice_state_without_optional_arguments(self, rest_client):
         assert result is None
         rest_client._request.assert_awaited_once_with(expected_route, json={"channel_id": "321"})
 
-    async def test_edit_permission_overwrites(self, rest_client):
+    async def test_edit_permission_overwrite(self, rest_client):
         target = StubModel(456)
         expected_route = routes.PUT_CHANNEL_PERMISSIONS.compile(channel=123, overwrite=456)
         rest_client._request = mock.AsyncMock()
         expected_json = {"type": 1, "allow": 4, "deny": 1}
 
-        await rest_client.edit_permission_overwrites(
+        await rest_client.edit_permission_overwrite(
             StubModel(123),
             target,
             target_type=channels.PermissionOverwriteType.MEMBER,
@@ -2174,6 +2174,26 @@ async def test_edit_permission_overwrites(self, rest_client):
         )
         rest_client._request.assert_awaited_once_with(expected_route, json=expected_json, reason="cause why not :)")
 
+    async def test_edit_permission_overwrites(self, rest_client):
+        with mock.patch.object(rest_client, "edit_permission_overwrite") as edit_permission_overwrite:
+            await rest_client.edit_permission_overwrites(
+                StubModel(123),
+                StubModel(456),
+                target_type=channels.PermissionOverwriteType.MEMBER,
+                allow=permissions.Permissions.BAN_MEMBERS,
+                deny=permissions.Permissions.CREATE_INSTANT_INVITE,
+                reason="cause why not :)",
+            )
+
+        edit_permission_overwrite.assert_awaited_once_with(
+            StubModel(123),
+            StubModel(456),
+            target_type=channels.PermissionOverwriteType.MEMBER,
+            allow=permissions.Permissions.BAN_MEMBERS,
+            deny=permissions.Permissions.CREATE_INSTANT_INVITE,
+            reason="cause why not :)",
+        )
+
     @pytest.mark.parametrize(
         ("target", "expected_type"),
         [
@@ -2218,17 +2238,17 @@ async def test_edit_permission_overwrites(self, rest_client):
             ),
         ],
     )
-    async def test_edit_permission_overwrites_when_target_undefined(self, rest_client, target, expected_type):
+    async def test_edit_permission_overwrite_when_target_undefined(self, rest_client, target, expected_type):
         expected_route = routes.PUT_CHANNEL_PERMISSIONS.compile(channel=123, overwrite=456)
         rest_client._request = mock.AsyncMock()
         expected_json = {"type": expected_type}
 
-        await rest_client.edit_permission_overwrites(StubModel(123), target)
+        await rest_client.edit_permission_overwrite(StubModel(123), target)
         rest_client._request.assert_awaited_once_with(expected_route, json=expected_json, reason=undefined.UNDEFINED)
 
-    async def test_edit_permission_overwrites_when_cant_determine_target_type(self, rest_client):
+    async def test_edit_permission_overwrite_when_cant_determine_target_type(self, rest_client):
         with pytest.raises(TypeError):
-            await rest_client.edit_permission_overwrites(StubModel(123), StubModel(123))
+            await rest_client.edit_permission_overwrite(StubModel(123), StubModel(123))
 
     async def test_delete_permission_overwrite(self, rest_client):
         expected_route = routes.DELETE_CHANNEL_PERMISSIONS.compile(channel=123, overwrite=456)
diff --git a/tests/hikari/test_channels.py b/tests/hikari/test_channels.py
index 4a4b226e..7121eac1 100644
--- a/tests/hikari/test_channels.py
+++ b/tests/hikari/test_channels.py
@@ -340,7 +340,7 @@ def test_mention_property(self, model):
 
     @pytest.mark.asyncio()
     async def test_edit_overwrite(self, model):
-        model.app.rest.edit_permission_overwrites = mock.AsyncMock()
+        model.app.rest.edit_permission_overwrite = mock.AsyncMock()
         user = mock.Mock(users.PartialUser)
         await model.edit_overwrite(
             333,
@@ -350,7 +350,7 @@ async def test_edit_overwrite(self, model):
             reason="vrooom vroom",
         )
 
-        model.app.rest.edit_permission_overwrites.assert_called_once_with(
+        model.app.rest.edit_permission_overwrite.assert_called_once_with(
             69420,
             333,
             target_type=user,
@@ -361,13 +361,13 @@ async def test_edit_overwrite(self, model):
 
     @pytest.mark.asyncio()
     async def test_edit_overwrite_target_type_none(self, model):
-        model.app.rest.edit_permission_overwrites = mock.AsyncMock()
+        model.app.rest.edit_permission_overwrite = mock.AsyncMock()
         user = mock.Mock(users.PartialUser)
         await model.edit_overwrite(
             user, allow=permissions.Permissions.BAN_MEMBERS, deny=permissions.Permissions.CONNECT, reason="vrooom vroom"
         )
 
-        model.app.rest.edit_permission_overwrites.assert_called_once_with(
+        model.app.rest.edit_permission_overwrite.assert_called_once_with(
             69420,
             user,
             allow=permissions.Permissions.BAN_MEMBERS,