diff options
| author | 2020-06-30 23:09:00 +0200 | |
|---|---|---|
| committer | 2020-06-30 23:09:00 +0200 | |
| commit | be4a61fb70c485262d36ca2aabf992f3118abcff (patch) | |
| tree | d34696bfd6b34ef0d71e635328262d64c2132754 | |
| parent | Incidents: append `actioned_by` to webhook username (diff) | |
Incidents: revert latest 2 commits
Decision was made to use embeds to archive incidents instead of
webhooking the raw message. As such, we're reverting the branch
to a state from which the adjustments will be easier to make.
Reverted commits:
* a8d179d9b04f54b20c5e870bcfa85c78c42c8dca
* 6fa8caed037b247a7c194f58a4635de7dae21fd2
| -rw-r--r-- | bot/cogs/moderation/incidents.py | 33 | ||||
| -rw-r--r-- | tests/bot/cogs/moderation/test_incidents.py | 52 | 
2 files changed, 10 insertions, 75 deletions
| diff --git a/bot/cogs/moderation/incidents.py b/bot/cogs/moderation/incidents.py index 72cc4b26c..040f2c0c8 100644 --- a/bot/cogs/moderation/incidents.py +++ b/bot/cogs/moderation/incidents.py @@ -41,30 +41,6 @@ ALLOWED_ROLES: t.Set[int] = {Roles.moderators, Roles.admins, Roles.owners}  ALL_SIGNALS: t.Set[str] = {signal.value for signal in Signal} -def make_username(reported_by: discord.Member, actioned_by: discord.Member, max_length: int = 80) -> str: -    """ -    Create a webhook-friendly username from the names of `reported_by` and `actioned_by`. - -    If the resulting username length exceeds `max_length`, it will be capped at `max_length - 3` -    and have 3 dots appended to the end. The default value is 80, which corresponds to the limit -    Discord imposes on webhook username length. - -    If the value of `max_length` is < 3, ValueError is raised. -    """ -    if max_length < 3: -        raise ValueError(f"Maximum length cannot be less than 3: {max_length=}") - -    username = f"{reported_by.name} | {actioned_by.name}" -    log.trace(f"Generated webhook username: {username} (length: {len(username)})") - -    if len(username) > max_length: -        stop = max_length - 3 -        username = f"{username[:stop]}..." -        log.trace(f"Username capped at {max_length=}: {username}") - -    return username - -  def is_incident(message: discord.Message) -> bool:      """True if `message` qualifies as an incident, False otherwise."""      conditions = ( @@ -172,14 +148,13 @@ class Incidents(Cog):          log.debug("Crawl task finished!") -    async def archive(self, incident: discord.Message, outcome: Signal, actioned_by: discord.Member) -> bool: +    async def archive(self, incident: discord.Message, outcome: Signal) -> bool:          """          Relay `incident` to the #incidents-archive channel.          The following pieces of information are relayed:              * Incident message content (clean, pingless) -            * Incident author name (as webhook username) -            * Name of user who actioned the incident (appended to webhook username) +            * Incident author name (as webhook author)              * Incident author avatar (as webhook avatar)              * Resolution signal (`outcome`) @@ -195,7 +170,7 @@ class Incidents(Cog):              # Now relay the incident              message: discord.Message = await webhook.send(                  content=incident.clean_content,  # Clean content will prevent mentions from pinging -                username=sub_clyde(make_username(incident.author, actioned_by)), +                username=sub_clyde(incident.author.name),                  avatar_url=incident.author.avatar_url,                  wait=True,  # This makes the method return the sent Message object              ) @@ -260,7 +235,7 @@ class Incidents(Cog):              log.debug("Reaction was valid, but no action is currently defined for it")              return -        relay_successful = await self.archive(incident, signal, actioned_by=member) +        relay_successful = await self.archive(incident, signal)          if not relay_successful:              log.trace("Original message will not be deleted as we failed to relay it to the archive")              return diff --git a/tests/bot/cogs/moderation/test_incidents.py b/tests/bot/cogs/moderation/test_incidents.py index a811868e5..2fc9180cf 100644 --- a/tests/bot/cogs/moderation/test_incidents.py +++ b/tests/bot/cogs/moderation/test_incidents.py @@ -68,35 +68,6 @@ mock_404 = discord.NotFound(  ) -class TestMakeUsername(unittest.TestCase): -    """Collection of tests for the `make_username` helper function.""" - -    def test_make_username_raises(self): -        """Raises `ValueError` on `max_length` < 3.""" -        with self.assertRaises(ValueError): -            incidents.make_username(MockMember(), MockMember(), max_length=2) - -    def test_make_username_never_exceed_limit(self): -        """ -        The return string length is always less than or equal to `max_length`. - -        For this test we pass `max_length=10` for convenience. The name of the first -        user (`reported_by`) is always 1 character in length, but we generate names -        for the `actioned_by` user starting at length 1 and up to length 20. - -        Finally, we assert that the output length never exceeded 10 in total. -        """ -        user_a = MockMember(name="A") - -        max_length = 10 -        test_cases = (MockMember(name="B" * n) for n in range(1, 20)) - -        for user_b in test_cases: -            with self.subTest(user_a=user_a, user_b=user_b, max_length=max_length): -                generated_username = incidents.make_username(user_a, user_b, max_length) -                self.assertLessEqual(len(generated_username), max_length) - -  @patch("bot.constants.Channels.incidents", 123)  class TestIsIncident(unittest.TestCase):      """ @@ -307,9 +278,7 @@ class TestArchive(TestIncidents):          propagate out of the method, which is just as important.          """          self.cog_instance.bot.fetch_webhook = AsyncMock(side_effect=mock_404) - -        result = await self.cog_instance.archive(incident=MockMessage(), outcome=MagicMock(), actioned_by=MockMember()) -        self.assertFalse(result) +        self.assertFalse(await self.cog_instance.archive(incident=MockMessage(), outcome=MagicMock()))      async def test_archive_relays_incident(self):          """ @@ -334,18 +303,12 @@ class TestArchive(TestIncidents):              author=MockUser(name="author_name", avatar_url="author_avatar"),              id=123,          ) - -        with patch("bot.cogs.moderation.incidents.make_username", MagicMock(return_value="generated_username")): -            archive_return = await self.cog_instance.archive( -                incident=incident, -                outcome=MagicMock(value="A"), -                actioned_by=MockMember(name="moderator"), -            ) +        archive_return = await self.cog_instance.archive(incident, outcome=MagicMock(value="A"))          # Check that the webhook was dispatched correctly          webhook.send.assert_called_once_with(              content="pingless message", -            username="generated_username", +            username="author_name",              avatar_url="author_avatar",              wait=True,          ) @@ -362,8 +325,7 @@ class TestArchive(TestIncidents):          Discord will reject any webhook with "clyde" in the username field, as it impersonates          the official Clyde bot. Since we do not control what the username will be (the incident -        author name, and actioning moderator names are used), we must ensure the name is cleansed, -        otherwise the relay may fail. +        author name is used), we must ensure the name is cleansed, otherwise the relay may fail.          This test assumes the username is passed as a kwarg. If this test fails, please review          whether the passed argument is being retrieved correctly. @@ -371,11 +333,9 @@ class TestArchive(TestIncidents):          webhook = MockAsyncWebhook()          self.cog_instance.bot.fetch_webhook = AsyncMock(return_value=webhook) -        # The `make_username` helper will return a string with "clyde" in it -        with patch("bot.cogs.moderation.incidents.make_username", MagicMock(return_value="clyde the great")): -            await self.cog_instance.archive(MockMessage(), MagicMock(incidents.Signal), MockMember()) +        message_from_clyde = MockMessage(author=MockUser(name="clyde the great")) +        await self.cog_instance.archive(message_from_clyde, MagicMock(incidents.Signal)) -        # Assert that the "clyde" was never passed to `send`          self.assertNotIn("clyde", webhook.send.call_args.kwargs["username"]) | 
