Repository: hikari-py/hikari
Pull request #1378 (2022-11-27 17:48:27)

Deprecate delete_message_days for delete_message_seconds in ban methods
### Summary
This PR implements the `delete_message_seconds` parameter for PartialGuild.ban and Member.ban, and begins the deprecation process for the `delete_message_days` parameter.

### 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.


null-domain: I think I have a lot of your suggestions/comments already implemented in my local version. I've been a little busy since I last looked at this, but I'll take another look at this PR this weekend and discuss the rest of my changes with you in Discord :)

Patch:
diff --git a/changes/1378.deprecation.md b/changes/1378.deprecation.md
new file mode 100644
index 00000000..e5e59085
--- /dev/null
+++ b/changes/1378.deprecation.md
@@ -0,0 +1,1 @@
+Deprecate the `delete_message_days` parameter for PartialGuild.ban and Member.ban.
diff --git a/changes/1378.feature.md b/changes/1378.feature.md
new file mode 100644
index 00000000..2eb4cf57
--- /dev/null
+++ b/changes/1378.feature.md
@@ -0,0 +1,2 @@
+Allow specifying the `delete_message_seconds` parameter for PartialGuild.ban and Member.ban.
+- This parameter can be specified as either an int, a float, or a datetime.timedelta object.
diff --git a/hikari/api/rest.py b/hikari/api/rest.py
index 4bb207ce..553040fb 100644
--- a/hikari/api/rest.py
+++ b/hikari/api/rest.py
@@ -5926,9 +5926,10 @@ async def ban_user(
         user: snowflakes.SnowflakeishOr[users.PartialUser],
         *,
         delete_message_days: undefined.UndefinedOr[int] = undefined.UNDEFINED,
+        delete_message_seconds: undefined.UndefinedOr[time.Intervalish] = undefined.UNDEFINED,
         reason: undefined.UndefinedOr[str] = undefined.UNDEFINED,
     ) -> None:
-        """Ban a member from a guild.
+        """Ban the given user from this guild.
 
         Parameters
         ----------
@@ -5944,6 +5945,13 @@ async def ban_user(
         delete_message_days : hikari.undefined.UndefinedOr[int]
             If provided, the number of days to delete messages for.
             This must be between 0 and 7.
+
+            .. deprecated:: 2.0.0.dev114
+                Use `delete_message_seconds` instead.
+        delete_message_seconds : hikari.undefined.UndefinedNoneOr[hikari.internal.time.Intervalish]
+            If provided, the number of seconds to delete messages for.
+            This can be represented as either an int/float between 0 and 604800 (7 days), or
+            a `datetime.timedelta` object.
         reason : hikari.undefined.UndefinedOr[str]
             If provided, the reason that will be recorded in the audit logs.
             Maximum of 512 characters.
@@ -5980,6 +5988,7 @@ async def ban_member(
         user: snowflakes.SnowflakeishOr[users.PartialUser],
         *,
         delete_message_days: undefined.UndefinedOr[int] = undefined.UNDEFINED,
+        delete_message_seconds: undefined.UndefinedOr[time.Intervalish] = undefined.UNDEFINED,
         reason: undefined.UndefinedOr[str] = undefined.UNDEFINED,
     ) -> None:
         """Alias of `ban_user`."""
diff --git a/hikari/guilds.py b/hikari/guilds.py
index 0d52d910..88a059cd 100644
--- a/hikari/guilds.py
+++ b/hikari/guilds.py
@@ -66,6 +66,7 @@
 from hikari import urls
 from hikari import users
 from hikari.internal import attr_extensions
+from hikari.internal import deprecation
 from hikari.internal import enums
 from hikari.internal import routes
 from hikari.internal import time
@@ -691,6 +692,7 @@ async def ban(
         self,
         *,
         delete_message_days: undefined.UndefinedOr[int] = undefined.UNDEFINED,
+        delete_message_seconds: undefined.UndefinedOr[time.Intervalish] = undefined.UNDEFINED,
         reason: undefined.UndefinedOr[str] = undefined.UNDEFINED,
     ) -> None:
         """Ban this member from this guild.
@@ -700,6 +702,13 @@ async def ban(
         delete_message_days : hikari.undefined.UndefinedOr[int]
             If provided, the number of days to delete messages for.
             This must be between 0 and 7.
+
+            .. deprecated:: 2.0.0.dev114
+                Use `delete_message_seconds` instead.
+        delete_message_seconds : hikari.undefined.UndefinedNoneOr[hikari.internal.time.Intervalish]
+            If provided, the number of seconds to delete messages for.
+            This can be represented as either an int/float between 0 and 604800 (7 days), or
+            a `datetime.timedelta` object.
         reason : hikari.undefined.UndefinedOr[str]
             If provided, the reason that will be recorded in the audit logs.
             Maximum of 512 characters.
@@ -728,8 +737,21 @@ async def ban(
         hikari.errors.InternalServerError
             If an internal error occurs on Discord while handling the request.
         """
+        if delete_message_days is not undefined.UNDEFINED:
+            deprecation.warn_deprecated(
+                "delete_message_days",
+                removal_version="2.0.0.dev116",
+                additional_info="'delete_message_seconds' should be used instead.",
+            )
+            if delete_message_seconds is not undefined.UNDEFINED:
+                raise ValueError(
+                    "You may only specify one of 'delete_message_days' or 'delete_message_seconds', not both"
+                )
+
+            delete_message_seconds = delete_message_days * 24 * 60**2
+
         await self.user.app.rest.ban_user(
-            self.guild_id, self.user.id, delete_message_days=delete_message_days, reason=reason
+            self.guild_id, self.user.id, delete_message_seconds=delete_message_seconds, reason=reason
         )
 
     async def unban(
@@ -1451,6 +1473,7 @@ async def ban(
         user: snowflakes.SnowflakeishOr[users.PartialUser],
         *,
         delete_message_days: undefined.UndefinedOr[int] = undefined.UNDEFINED,
+        delete_message_seconds: undefined.UndefinedOr[time.Intervalish] = undefined.UNDEFINED,
         reason: undefined.UndefinedOr[str] = undefined.UNDEFINED,
     ) -> None:
         """Ban the given user from this guild.
@@ -1465,6 +1488,13 @@ async def ban(
         delete_message_days : hikari.undefined.UndefinedOr[int]
             If provided, the number of days to delete messages for.
             This must be between 0 and 7.
+
+            .. deprecated:: 2.0.0.dev114
+                Use `delete_message_seconds` instead.
+        delete_message_seconds : hikari.undefined.UndefinedNoneOr[hikari.internal.time.Intervalish]
+            If provided, the number of seconds to delete messages for.
+            This can be represented as either an int/float between 0 and 604800 (7 days), or
+            a `datetime.timedelta` object.
         reason : hikari.undefined.UndefinedOr[str]
             If provided, the reason that will be recorded in the audit logs.
             Maximum of 512 characters.
@@ -1493,7 +1523,20 @@ async def ban(
         hikari.errors.InternalServerError
             If an internal error occurs on Discord while handling the request.
         """
-        await self.app.rest.ban_user(self.id, user, delete_message_days=delete_message_days, reason=reason)
+        if delete_message_days is not undefined.UNDEFINED:
+            deprecation.warn_deprecated(
+                "delete_message_days",
+                removal_version="2.0.0.dev116",
+                additional_info="'delete_message_seconds' should be used instead.",
+            )
+            if delete_message_seconds is not undefined.UNDEFINED:
+                raise ValueError(
+                    "You may only specify one of 'delete_message_days' or 'delete_message_seconds', not both"
+                )
+
+            delete_message_seconds = delete_message_days * 24 * 60**2
+
+        await self.app.rest.ban_user(self.id, user, delete_message_seconds=delete_message_seconds, reason=reason)
 
     async def unban(
         self,
diff --git a/hikari/impl/rest.py b/hikari/impl/rest.py
index de291884..2d3ad1be 100644
--- a/hikari/impl/rest.py
+++ b/hikari/impl/rest.py
@@ -2934,10 +2934,27 @@ async def ban_user(
         user: snowflakes.SnowflakeishOr[users.PartialUser],
         *,
         delete_message_days: undefined.UndefinedOr[int] = undefined.UNDEFINED,
+        delete_message_seconds: undefined.UndefinedOr[time.Intervalish] = undefined.UNDEFINED,
         reason: undefined.UndefinedOr[str] = undefined.UNDEFINED,
     ) -> None:
+        if delete_message_days is not undefined.UNDEFINED:
+            deprecation.warn_deprecated(
+                "delete_message_days",
+                removal_version="2.0.0.dev116",
+                additional_info="'delete_message_seconds' should be used instead.",
+            )
+            if delete_message_seconds:
+                raise ValueError(
+                    "You may only specify one of 'delete_message_days' or 'delete_message_seconds', not both"
+                )
+
+            delete_message_seconds = delete_message_days * 24 * 60**2
+
+        if isinstance(delete_message_seconds, datetime.timedelta):
+            delete_message_seconds = delete_message_seconds.total_seconds()
+
         body = data_binding.JSONObjectBuilder()
-        body.put("delete_message_days", delete_message_days)
+        body.put("delete_message_seconds", delete_message_seconds)
         route = routes.PUT_GUILD_BAN.compile(guild=guild, user=user)
         await self._request(route, json=body, reason=reason)
 
@@ -2947,9 +2964,26 @@ def ban_member(
         user: snowflakes.SnowflakeishOr[users.PartialUser],
         *,
         delete_message_days: undefined.UndefinedOr[int] = undefined.UNDEFINED,
+        delete_message_seconds: undefined.UndefinedOr[time.Intervalish] = undefined.UNDEFINED,
         reason: undefined.UndefinedOr[str] = undefined.UNDEFINED,
     ) -> typing.Coroutine[typing.Any, typing.Any, None]:
-        return self.ban_user(guild, user, delete_message_days=delete_message_days, reason=reason)
+        if delete_message_days is not undefined.UNDEFINED:
+            deprecation.warn_deprecated(
+                "delete_message_days",
+                removal_version="2.0.0.dev116",
+                additional_info="'delete_message_seconds' should be used instead.",
+            )
+            if delete_message_seconds:
+                raise ValueError(
+                    "You may only specify one of 'delete_message_days' or 'delete_message_seconds', not both"
+                )
+
+            delete_message_seconds = delete_message_days * 24 * 60**2
+
+        if isinstance(delete_message_seconds, datetime.timedelta):
+            delete_message_seconds = delete_message_seconds.total_seconds()
+
+        return self.ban_user(guild, user, delete_message_seconds=delete_message_seconds, reason=reason)
 
     async def unban_user(
         self,


Test patch:
diff --git a/tests/hikari/impl/test_event_manager_base.py b/tests/hikari/impl/test_event_manager_base.py
index ad124c02..b4f6859e 100644
--- a/tests/hikari/impl/test_event_manager_base.py
+++ b/tests/hikari/impl/test_event_manager_base.py
@@ -23,6 +23,7 @@
 
 import asyncio
 import contextlib
+import gc
 import logging
 import sys
 import typing
@@ -215,6 +216,7 @@ def test___del___for_active_stream(self):
 
         with unittest.TestCase().assertLogs("hikari.event_manager", level=logging.WARNING) as logging_watcher:
             del streamer
+            gc.collect()  # Force a GC collection
 
         assert logging_watcher.output == [
             "WARNING:hikari.event_manager:active 'Event' streamer fell out of scope before being closed"
diff --git a/tests/hikari/impl/test_rest.py b/tests/hikari/impl/test_rest.py
index 9d914ac0..6e7c6dc0 100644
--- a/tests/hikari/impl/test_rest.py
+++ b/tests/hikari/impl/test_rest.py
@@ -1123,10 +1123,25 @@ def test_ban_member(self, rest_client):
         mock_ban_user = mock.Mock()
         rest_client.ban_user = mock_ban_user
 
-        result = rest_client.ban_member(43123, 54123, delete_message_days=6, reason="wowowowo")
+        result = rest_client.ban_member(43123, 54123, delete_message_seconds=518400, reason="wowowowo")
 
         assert result is mock_ban_user.return_value
-        mock_ban_user.assert_called_once_with(43123, 54123, delete_message_days=6, reason="wowowowo")
+        mock_ban_user.assert_called_once_with(43123, 54123, delete_message_seconds=518400, reason="wowowowo")
+
+    def test_ban_member_deprecated_delete_message_days(self, rest_client):
+        mock_ban_user = mock.Mock()
+        rest_client.ban_user = mock_ban_user
+
+        result = rest_client.ban_member(43123, 54123, delete_message_days=1, reason="wowowowo")
+
+        assert result is mock_ban_user.return_value
+        mock_ban_user.assert_called_once_with(43123, 54123, delete_message_seconds=86400, reason="wowowowo")
+
+    def test_ban_member_deprecated_delete_message_days_when_both_passed(self, rest_client):
+        with pytest.raises(
+            ValueError, match="You may only specify one of 'delete_message_days' or 'delete_message_seconds', not both"
+        ):
+            rest_client.ban_member(43123, 54123, delete_message_days=1, delete_message_seconds=1, reason="wowowowo")
 
     def test_unban_member(self, rest_client):
         mock_unban_user = mock.Mock()
@@ -4825,13 +4840,32 @@ async def test_kick_user(self, rest_client):
 
     async def test_ban_user(self, rest_client):
         expected_route = routes.PUT_GUILD_BAN.compile(guild=123, user=456)
-        expected_json = {"delete_message_days": 7}
+        expected_json = {"delete_message_seconds": 604800}
+        rest_client._request = mock.AsyncMock()
+
+        await rest_client.ban_user(
+            StubModel(123), StubModel(456), delete_message_seconds=604800, reason="because i can"
+        )
+
+        rest_client._request.assert_awaited_once_with(expected_route, json=expected_json, reason="because i can")
+
+    async def test_ban_user_deprecated_delete_message_days(self, rest_client):
+        expected_route = routes.PUT_GUILD_BAN.compile(guild=123, user=456)
+        expected_json = {"delete_message_seconds": 86400}
         rest_client._request = mock.AsyncMock()
 
-        await rest_client.ban_user(StubModel(123), StubModel(456), delete_message_days=7, reason="because i can")
+        await rest_client.ban_user(StubModel(123), StubModel(456), delete_message_days=1, reason="because i can")
 
         rest_client._request.assert_awaited_once_with(expected_route, json=expected_json, reason="because i can")
 
+    async def test_ban_user_deprecated_delete_message_days_when_both_passed(self, rest_client):
+        with pytest.raises(
+            ValueError, match="You may only specify one of 'delete_message_days' or 'delete_message_seconds', not both"
+        ):
+            await rest_client.ban_user(
+                43123, 54123, delete_message_seconds=1, delete_message_days=1, reason="because i can"
+            )
+
     async def test_unban_user(self, rest_client):
         expected_route = routes.DELETE_GUILD_BAN.compile(guild=123, user=456)
         rest_client._request = mock.AsyncMock()
diff --git a/tests/hikari/test_guilds.py b/tests/hikari/test_guilds.py
index de4d3db4..3f15ecef 100644
--- a/tests/hikari/test_guilds.py
+++ b/tests/hikari/test_guilds.py
@@ -388,9 +388,24 @@ async def test_fetch_roles(self, model):
     async def test_ban(self, model):
         model.app.rest.ban_user = mock.AsyncMock()
 
-        await model.ban(delete_message_days=10, reason="bored")
+        await model.ban(delete_message_seconds=600, reason="bored")
 
-        model.app.rest.ban_user.assert_awaited_once_with(456, 123, delete_message_days=10, reason="bored")
+        model.app.rest.ban_user.assert_awaited_once_with(456, 123, delete_message_seconds=600, reason="bored")
+
+    @pytest.mark.asyncio()
+    async def test_ban_deprecated_delete_message_days(self, model):
+        model.app.rest.ban_user = mock.AsyncMock()
+
+        await model.ban(delete_message_days=1, reason="bored")
+
+        model.app.rest.ban_user.assert_awaited_once_with(456, 123, delete_message_seconds=86400, reason="bored")
+
+    @pytest.mark.asyncio()
+    async def test_ban_deprecated_delete_message_days_when_both_passed(self, model):
+        with pytest.raises(
+            ValueError, match="You may only specify one of 'delete_message_days' or 'delete_message_seconds', not both"
+        ):
+            await model.ban(delete_message_days=1, delete_message_seconds=1, reason="bored")
 
     @pytest.mark.asyncio()
     async def test_unban(self, model):
@@ -624,9 +639,25 @@ async def test_kick(self, model):
     @pytest.mark.asyncio()
     async def test_ban(self, model):
         model.app.rest.ban_user = mock.AsyncMock()
-        await model.ban(4321, delete_message_days=10, reason="Go away!")
 
-        model.app.rest.ban_user.assert_awaited_once_with(90210, 4321, delete_message_days=10, reason="Go away!")
+        await model.ban(4321, delete_message_seconds=864000, reason="Go away!")
+
+        model.app.rest.ban_user.assert_awaited_once_with(90210, 4321, delete_message_seconds=864000, reason="Go away!")
+
+    @pytest.mark.asyncio()
+    async def test_ban_deprecated_delete_message_days(self, model):
+        model.app.rest.ban_user = mock.AsyncMock()
+
+        await model.ban(4321, delete_message_days=1, reason="Go away!")
+
+        model.app.rest.ban_user.assert_awaited_once_with(90210, 4321, delete_message_seconds=86400, reason="Go away!")
+
+    @pytest.mark.asyncio()
+    async def test_ban_deprecated_delete_message_days_when_both_passed(self, model):
+        with pytest.raises(
+            ValueError, match="You may only specify one of 'delete_message_days' or 'delete_message_seconds', not both"
+        ):
+            await model.ban(4321, delete_message_days=1, delete_message_seconds=1, reason="Go away!")
 
     @pytest.mark.asyncio()
     async def test_unban(self, model):