Add a cog to manage tracking a user's alt accounts #2845
Conversation
ChrisLovering
requested review from
mbaruh,
Den4200,
ks129 and
jb3
as code owners
ChrisLovering
added
t: feature
ChrisLovering
force-pushed
the
feat/1353/alt-users
branch
2 times, most recently
from
f915536
to
1152d3c
Compare
ChrisLovering
force-pushed
the
feat/1353/alt-users
branch
3 times, most recently
from
e3de335
to
67b2423
Compare
bot/exts/moderation/alts.py
Outdated
| return str(error.response_json) | ||
| return error.response_text | ||
|
|
||
| async def alts_to_string(self, alts: list[dict[str: str | int | list[int]]]) -> list[str]: |
There was a problem hiding this comment.
I always think it's better to have a local pydantic model that explains the structure, instead of a lot of nested types likes this
There was a problem hiding this comment.
I was going to leave it as list[dict] but thought I might as well expand it further. Didn't want to make a full model since site might change and I don't want ot have to maintain it in two places
There was a problem hiding this comment.
Why would you have to maintain it in 2 places ?
Reading this as it is is "painful" and one would need to squeeze their toughts in order to understand what's going on, and it isn't pleasant. Giving it structure would be more readable/intuitive/understandable.
There was a problem hiding this comment.
I think that the current type isn't very useful ,str | int | list[int] makes the usages below technically incorrect since whenever accessing a value they should handle it being any of those types.
I would be find with any of these, since they're all "correct", it just depends what you consider in-scope for the PR given our typing elsewhere with infractions isn't great either afaik:
list[dict[str, typing.Any]]orlist[dict]- Use a
TypedDict - Use a Pydantic model
There was a problem hiding this comment.
Simplified the type annotation in a380d2e (option 1)
ChrisLovering
force-pushed
the
feat/1353/alt-users
branch
from
67b2423
to
49dac82
Compare
ChrisLovering
force-pushed
the
feat/1353/alt-users
branch
from
49dac82
to
eba30d5
Compare
bot/exts/moderation/alts.py
Outdated
| return str(error.response_json) | ||
| return error.response_text | ||
|
|
||
| async def alts_to_string(self, alts: list[dict[str: str | int | list[int]]]) -> list[str]: |
There was a problem hiding this comment.
Why would you have to maintain it in 2 places ?
Reading this as it is is "painful" and one would need to squeeze their toughts in order to understand what's going on, and it isn't pleasant. Giving it structure would be more readable/intuitive/understandable.
bot/exts/info/information.py
Outdated
| @@ -337,6 +338,12 @@ async def create_user_embed(self, ctx: Context, user: MemberOrUser, passed_as_me | |||
|
|
|||
| return embed | |||
|
|
|||
| async def user_alt_count(self, user: MemberOrUser) -> tuple[str, int | str]: | |||
| """Get the number oif alts for the given member.""" | |||
There was a problem hiding this comment.
itty bitty typo oif -> of
There was a problem hiding this comment.
I have corrected my accent in 98eea89
wookie184
added
s: waiting for author
ChrisLovering
force-pushed
the
feat/1353/alt-users
branch
from
eba30d5
to
98eea89
Compare
wookie184
removed
the
s: waiting for author
ChrisLovering
force-pushed
the
feat/1353/alt-users
branch
from
98eea89
to
daeb4bd
Compare
Important note
This must be merged after python-discord/site#1171
Summary
This allows moderators, in mod channels, to add, remove & list a user's alt accounts.
When searching for infractions by user id, the number of al accounts is also shown, if there are any.
Screenshot
Wording can be discussed...
