Display jump url to message invoking an infraction in infraction log #2372
Conversation
vivekashok1221
requested review from
Den4200,
jb3,
ks129 and
mbaruh
as code owners
shtlrs
left a comment
shtlrs
left a comment
There was a problem hiding this comment.
A couple of nit changes here and there but I haven't tested this yet.
wookie184
added
a: frontend
vivekashok1221
force-pushed
the
vivek/jump-url-infr-log
branch
from
ac3fdc9 to
c6af8bf
Compare
vivekashok1221
added
s: needs review
| is_in_category(ctx.channel, category) | ||
| for category in (Categories.modmail, Categories.appeals, Categories.appeals2) | ||
| ): | ||
| jump_url = "Infraction issued in a ModMail channel." |
There was a problem hiding this comment.
Either something has a jump link or it doesn't, storing a string in here too means we can't have a validator on the db that it's a valid link.
IMO we should have a validator on site that requires it to be formatted as a link, or null.
There was a problem hiding this comment.
I could be misremembering but Ive read somewhere (django docs or stackoverflow) that using blank strings is recommended by django ORM. I can look for the link later.
There was a problem hiding this comment.
Maybe it's just a convention.
There was a problem hiding this comment.
And regarding the validator, I could use URLField instead of CharField because it has URL validation.
There was a problem hiding this comment.
A URLField that can be blank/null sounds good to me
| if jump_url == "": | ||
| # Infraction was issued prior to jump urls being stored in the database. | ||
| jump_url = "N/A" | ||
| elif "discord.com" in jump_url: | ||
| jump_url = f"[Click here.]({jump_url})" | ||
| # Else, infraction was issued in ModMail category. |
There was a problem hiding this comment.
This would be simplified if we went with the "it's either a link or null"
There was a problem hiding this comment.
Yeah, I did think of that.
|
Have we considered a URL button for the embed? Or would that be too big? |
Personally, I don't think it's necessary. I'm open to opinions though. |
|
That would make two buttons at most in the same message when listing infractions. Then we lose context of which infraction does that button refer to, no? Which might become confusing. |
Previously displayed "Infraction issued in modmail" for infractions in ModMail.
vivekashok1221
added
s: WIP
|
I will fix the tests as soon as I get some time but I'm not sure why the tests didn't fail during my earlier pushes. Might be random and may even disappear on re-running the tests- nevertheless, will try fixing. Passes locally btw $ poetry run task test
=============================== test session starts ================================
platform linux -- Python 3.10.9, pytest-7.2.0, pluggy-1.0.0
rootdir: /home/vivek/Programs/python/contrib/pydis/bot, configfile: pyproject.toml
plugins: subtests-0.9.0, xdist-3.0.2, cov-4.0.0
gw0 [473] / gw1 [473] / gw2 [473] / gw3 [473] / gw4 [473] / gw5 [473] / gw6 ok / gw7gw0 [473] / gw1 [473] / gw2 [473] / gw3 [473] / gw4 [473] / gw5 [473] / gw6 [473] / gw0 [473] / gw1 [473] / gw2 [473] / gw3 [473] / gw4 [473] / gw5 [473] / gw6 [473] / gw7 [473]
............................................................................ [ 16%]
............................................................................ [ 32%]
............................................................................ [ 48%]
............................................................................ [ 64%]
............................................................s............... [ 80%]
............................................................................ [ 96%]
................. [100%]
========================= 472 passed, 1 skipped in 20.26s ==========================I'll retry after updating my branch. |
vivekashok1221
added
the
s: needs review
Closes #2329 .
Related site issue: python-discord/site#801
Related site PR: python-discord/site#824
screenshots:
Infraction log:
#mod-logs embed:
