Skip to content

Incident archive msg improvements#2031

Merged
wookie184 merged 6 commits into
mainfrom
incident-archive-msg-improvements
Aug 18, 2022
Merged

Incident archive msg improvements#2031
wookie184 merged 6 commits into
mainfrom
incident-archive-msg-improvements

Conversation

@TizzySaurus
Copy link
Copy Markdown
Contributor

Closes #1059

Incident archive messages now show when the incident was made and no longer displays when the incident was actioned as it's the same as the message timestamp.

Also removed some redundant escapes in the discord message link regex.

@TizzySaurus TizzySaurus added a: frontend Related to output and formatting a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 3 - low Low Priority t: enhancement Changes or improvements to existing features s: needs review Author is waiting for someone to review and approve labels Dec 29, 2021
@TizzySaurus TizzySaurus force-pushed the incident-archive-msg-improvements branch from c86c2a1 to 1e0c0cf Compare January 1, 2022 16:18
@Xithrius Xithrius requested review from bast0006 and kosayoda and removed request for Den4200 and ks129 January 2, 2022 20:12
Copy link
Copy Markdown
Contributor

@kosayoda kosayoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of the proposed UI, but I think it can be improved. Thoughts?

Suggestion:
image

  • Simplified time of report (less clutter, straight to the point)
  • Relative action time in footer (per @MarkKoz here)
  • Absolute action time as footer timestamp
crude suggestion diff
diff --git a/bot/exts/moderation/incidents.py b/bot/exts/moderation/incidents.py
index bd9e5b88..f1ad6a59 100644
--- a/bot/exts/moderation/incidents.py
+++ b/bot/exts/moderation/incidents.py
@@ -1,18 +1,20 @@
 import asyncio
 import re
+from datetime import datetime, timezone
 from enum import Enum
 from typing import Optional
 
 import discord
 from async_rediscache import RedisCache
 from discord.ext.commands import Cog, Context, MessageConverter, MessageNotFound
+from dateutil.relativedelta import relativedelta
 
 from bot.bot import Bot
 from bot.constants import Channels, Colours, Emojis, Guild, Roles, Webhooks
 from bot.log import get_logger
 from bot.utils import scheduling
 from bot.utils.messages import format_user, sub_clyde
-from bot.utils.time import TimestampFormats, discord_timestamp
+from bot.utils.time import TimestampFormats, discord_timestamp, humanize_delta
 
 log = get_logger(__name__)
 
@@ -90,17 +92,19 @@ async def make_embed(incident: discord.Message, outcome: Signal, actioned_by: di
     """
     log.trace(f"Creating embed for {incident.id=}")
 
+    relative_action_time = humanize_delta(
+        relativedelta(datetime.now(timezone.utc), incident.created_at), max_units=1
+    )
     if outcome is Signal.ACTIONED:
         colour = Colours.soft_green
-        footer = f"Actioned by {actioned_by}"
+        footer = f"Actioned by {actioned_by} after {relative_action_time}"
     else:
         colour = Colours.soft_red
-        footer = f"Rejected by {actioned_by}"
+        footer = f"Rejected by {actioned_by} after {relative_action_time}"
 
-    day_timestamp = discord_timestamp(incident.created_at, TimestampFormats.DATE)
-    time_timestamp = discord_timestamp(incident.created_at, TimestampFormats.TIME)
+    datetime_timestamp = discord_timestamp(incident.created_at, TimestampFormats.DATE_TIME)
     relative_timestamp = discord_timestamp(incident.created_at, TimestampFormats.RELATIVE)
-    reported_on_msg = f"__*Reported {day_timestamp} at {time_timestamp} ({relative_timestamp}).*__"
+    reported_on_msg = f"Reported {datetime_timestamp} ({relative_timestamp})."
 
     # If the description will be too long (>4096 total characters), truncate the incident content
     if len(incident.content) > (allowed_content_chars := 4096-len(reported_on_msg)-2):  # -2 for the newlines
@@ -110,6 +114,7 @@ async def make_embed(incident: discord.Message, outcome: Signal, actioned_by: di
 
     embed = discord.Embed(
         description=description,
+        timestamp=datetime.utcnow(),
         colour=colour,
     )
     embed.set_footer(text=footer, icon_url=actioned_by.display_avatar.url)
cc issue creator @kwzrd

@TizzySaurus
Copy link
Copy Markdown
Contributor Author

TizzySaurus commented Jan 2, 2022

Relative action time in footer (per @MarkKoz here)

I discussed this with one of the core devs (I believe in a mod channel) and we decided we don't want this statistic, as it's often not particularly accurate (people forgetting to action, or an incident taking a long time to resolve due to a ModMail thread etc.) and causes extra pressure on Moderators to action incidents sooner, which isn't wanted.

Absolute action time as footer timestamp

For this I refer back to this PR's description: "... and no longer displays when the incident was actioned as it's the same as the incident-archive message timestamp." This isn't needed.

EDIT: I suppose there is a case here; when multiple reports by the same user are actioned. In this scenario, only the first will have the timestamp. You can know it was actioned within a few minutes of the previous; is this accurate enough or do we want to re-add the timestamp for precision?

Simplified time of report (less clutter, straight to the point)

I do agree with this point; it definitely looks much nicer so will change.

@kosayoda
Copy link
Copy Markdown
Contributor

kosayoda commented Jan 2, 2022

we decided we don't want this statistic

Sure, makes sense.

when multiple reports by the same user are actioned.

In compact mode, if I want to see the date an incident was actioned, I would have to scroll to the first incident of that day, so adding the timestamp to the footer helps. Although, in practice the action time shouldn't be too distant from the report time, so the information is technically in the embed already.

@TizzySaurus
Copy link
Copy Markdown
Contributor Author

TizzySaurus commented Jan 2, 2022

Although, in practice the action time shouldn't be too distant from the report time, so the information is technically in the embed already.

I mean there are multiple instances where it's several hours after, simply because we're unfortunately not a 24/7 taskforce. Then there's the odd case of it being days later (due to being delayed by a ModMail thread etc.).

I think supported with your compact mode comment, that's enough rationale for re-adding the timestamp.

Comment thread bot/exts/moderation/incidents.py Outdated
@Bluenix2 Bluenix2 added s: waiting for author Waiting for author to address a review or respond to a comment and removed s: needs review Author is waiting for someone to review and approve labels Mar 1, 2022
@Xithrius Xithrius added up for grabs Available for anyone to work on and removed s: waiting for author Waiting for author to address a review or respond to a comment labels May 27, 2022
@Xithrius Xithrius removed the up for grabs Available for anyone to work on label Jul 12, 2022
- Use the more concise DATETIME timestamp instead of both a DATE and a TIME timestamp.

- Remove underline from the "Reported ..." section at the bottom of the embed.

- Re-add time of action/rejection timestamp to footer of embed.
@TizzySaurus TizzySaurus requested review from Bluenix2 and kosayoda July 14, 2022 21:23
@TizzySaurus TizzySaurus added the review: do not merge The PR can be reviewed but cannot be merged now label Jul 14, 2022
@TizzySaurus
Copy link
Copy Markdown
Contributor Author

This latest commit is untested as I'm on holiday, so have added the "do not merge" tag to make sure it doesn't get merged before confirming it all works as planned.

@TizzySaurus TizzySaurus added the s: needs review Author is waiting for someone to review and approve label Jul 14, 2022
@TizzySaurus TizzySaurus marked this pull request as draft July 15, 2022 21:49
@ChrisLovering ChrisLovering removed the s: needs review Author is waiting for someone to review and approve label Jul 17, 2022
@TizzySaurus
Copy link
Copy Markdown
Contributor Author

Have tested and everything works as expected:

image

As such, this is now ready for review 😄

@TizzySaurus TizzySaurus marked this pull request as ready for review July 23, 2022 07:25
@TizzySaurus TizzySaurus added s: needs review Author is waiting for someone to review and approve and removed review: do not merge The PR can be reviewed but cannot be merged now labels Jul 23, 2022
@wookie184 wookie184 self-requested a review August 13, 2022 16:12
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Seems like there are some errors in the tests that need fixing though

Comment thread bot/exts/moderation/incidents.py
@TizzySaurus
Copy link
Copy Markdown
Contributor Author

TizzySaurus commented Aug 17, 2022

LGTM. Seems like there are some errors in the tests that need fixing though

Turns out I forgot to push the commit where I fixed the tests 😅
Have done so now 👍

Copy link
Copy Markdown
Contributor

@shenanigansd shenanigansd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wookie184 wookie184 enabled auto-merge August 18, 2022 13:57
@wookie184 wookie184 merged commit fb8d587 into main Aug 18, 2022
@wookie184 wookie184 deleted the incident-archive-msg-improvements branch August 18, 2022 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: frontend Related to output and formatting a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 3 - low Low Priority s: needs review Author is waiting for someone to review and approve t: enhancement Changes or improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incidents: show report datetime in archive embed

7 participants