Build 20191009.6 succeeded
Requested by
GitHub
Duration
00:01:37
Build pipeline
Bot
1 messages · Page 36 of 1
Build 20191009.6 succeeded
GitHub
00:01:37
Bot
This test is a bit fragile in the sense that it relies on the active validation logic preceding the validation for allowing expiration or being hidden. But I think I can live with that.
Build 20191009.7 succeeded
GitHub
00:02:20
Bot
How come a UniqueValidator wasn't used instead? Supposedly a ModelSerializer does this:
It will automatically generate validators for the serializer, such as unique_together validators.
I wonder, does it support generating validators for unique constraints on the model?
Since this and the following test cases have no assertions, it means you're relying on IntegrityError not being raised? If it was raised (like any other exception), it'd fail the test, right? Sorry if that has an obvious answer, just making sure here.
The nickname is supposed to be for the user, not the actor. To be clear, the user is the one which received the infraction. The actor is the one who gave out the infraction. You can see that the actor is already mentioned so there's no need to show their nickname anyway.
Also, I think it'd be better to replace the user's name with a nickname if it's available rather than adding it on a separate line.
Ok, because I muted myself during tests I didn't see the difference
I don't believe this is consistent with our code style but honestly it currently looks better than what it's "supposed" to be:
self.assertEqual(
response.json(),
{
'active': [f'{infraction_type} infractions cannot be active.']
}
)
It wouldn't be as bad if the dict was on one line:
self.assertEqual(
response.json(),
{'active': [f'{infraction_type} infractions cannot be active.']}
)
Oh, so I'm not sure if this should be changed or not. Thoughts?
I will change this to a more explicit variant. I've looked around for how to properly test a assertNotRaises situation, since there's no built-in way to do that with unittest, but I'm finding conflicting answers.
Some advise to just run it, like I've done here, and rely on the test resulting in Error if an exception occurs, but other argue, and I think I agree with that, that observing an exception where non should occur should result in a test Failure instead.
One solution wou...
Shouldn't it still do something? Can Django already figure out how to, for example, change a field back to True from False on its own?
The reason we don't need to do anything is because we can't reverse the state of the actual data; we simply don't know what the previous value were. Reversal is necessary if the forward function of RunPython also changed the model itself, since you then have to restore the model to the state prior to the migration.
So, we can't undo the data migration and we don't have to undo structural changes, since we made none.
Minor efficiency thing: I think the prefetch could be done after the gte 2 filter and the distinct()
Well it's just a migration I guess this sort of stuff doesn't even matter
It's a simple change, but I wonder if it will make a difference in the SQL query that's going to be made. The actual query will not be run until after all the methods are chained together and we're actually trying to read from query in the original function.
Build 20191009.8 succeeded
GitHub
00:02:15
Bot
Build 20191009.9 succeeded
GitHub
00:01:58
Bot
These two lines seem to be indented a bit too far.
This dictionary doesn't seem to be used anywhere else.
Why is this a MigrationsTestCase? The tests for the factory aren't concerned with what values are allowed for fields but rather just that if a given value is equal to the resulting value. Is it just so future migrations don't break this test (e.g. in the future a check constraint for a kick would be added that it must always be hidden, so line 105 would fail)?
Why use .update() over cls.created_infractions[1] = InfractionFactory.create(...)?
Do you think using strings as keys instead of IDs would improve readability, or is it already pretty clear from the test case names which infractions are being used?
Do all these filter by infraction ID too? Or is that not necessary?
Is it just so future migrations don't break this test (e.g. in the future a check constraint for a kick would be added that it must always be hidden, so line 105 would fail)?
Yes, it's to pin these tests to the point in the migration history we are testing. For our non-migration tests, we always update the tests to reflect the current (i.e., latest) states of the models, but that would not make sense for something that is used specifically to generate data according to an older model.
...
Good point. Adaptation from an earlier version and it was late.
I think strings like "user 1" would be clearer, so I'm going to change that.
Yes, they do. The infraction object returned by the InfractionFactory will include the id (and all the other fields of the created Infraction object).
MigrationsTestCase does not itself have any tests. Would it be feasible to come up with tests for that? Is it even worth it?I was thinking something along the lines of "no_infractions", "one_active_note", etc.
I disliked scrolling back and forth in the module to see what the infractions are supposed to be but I understand the objects need to be created in the hook rather than in the test cases (which would be more ideal for readability).
In this case, I want to use InfractionFactory to specifically generate data that would violate the UniqueTogether constraint introduced in a later migration.
That makes perfect sense, I did not catch this use myself.
we simply don't know what the previous value were.
Oh, right... that seems quite obvious now 😄
I don't know if we can assume that tokens are a constant length, per some discussions found on the Discord API server. Looking at the test cases they generated in the PR, it looks like only the final field varies, so we should be able to just vary its with and use the simpler regex.
That sounds good. Even using the more limited character set should make the regex a lot less fragile.
Looks like the only remaining action on this is to reduce the spacing of the embed.
If there is only this left to do, I could work on it.
Build 20191009.10 failed
GitHub
00:01:26
Bot
Build 20191009.11 failed
GitHub
00:01:26
Bot
I've updated the tasks to use the time parameter and combined them into one, however in upgrading discord.py to the latest (v1.3a) version it breaks the linter which seems to want to lint discord.py source, too...
This line will never get reached, as passing in no extensions will raise commands.MissingRequiredArgument first (which does a good enough job of telling you arguments are wrong)
Sorry that's my bad I was using the alias
This made me realise the other two commands are missing this check.
319cf13 Show help when ext load/unload are invoked with... - MarkKoz
Build 20191009.12 succeeded
GitHub
00:01:18
Bot
Can be set to get_user() instead of None to avoid doing x or y later. Keep in mind get_user() also returns None so it'd be good to set it to an empty string in such case to avoid displaying "None" as the username.
I am not sure but it's quite likely that if a user is not in the guild then they wont be in the cache either and thus get_user() won't find them. Could use fetch_user() instead, downside being that makes an API call. However, I think that's OK. Anyone else have thoughts on that?
Another idea for this one is to dispatch a cog_ready event inside add_cog. Right now most of the cogs use the on_ready listener to do any async stuff (or not) that needs doing when loading cogs, however, this fails if:
on_ready has completed.Postgres backup completed!
Resolves some small QoL moderation issues. See commit messages for details.
Build 20191010.1 succeeded
GitHub
00:01:27
Bot
@mathsman5133 With #488 practically done, I don't think this is needed. Also, I feel __init__ is basically the same as the proposed cog_ready would be anyway.
lgtm, but needs to be adjusted for the conflict created by #462
Build 20191010.2 succeeded
GitHub
00:01:41
Bot
I had a good think about this and sensible ways of dishing up some of the current files into subfolders.
(Names might need a change)
Automod: All things that trigger bot messages/reactions without any command - namely antispam.py, the on_message and related parts of bot.py, filtering.py, and token_remover.py. Maybe there's more but that's where I'd start
General commands for all to use (that aren't bot, server or site info): doc.py, free.py, reddit.py, `reminders.py...
This sounds good. You can work on it. I think @scragly may have had some ideas for how to improve it too.
Sure 👍 the difference being cog_ready could be a coro so fetching webhooks and the like would be simple, but seems like that PR does the same job
Closes #354.
Client_ID and Secret have to be created by an admin (https://www.reddit.com/prefs/apps).
The User-Agent string has to be modified with proper values.
Further commits are going to be needed as this PR w...
Build 20191010.3 failed
GitHub
00:01:02
Bot
I think using the <@id> is the best method because it always works and resolves and the issues!
Build 20191010.4 succeeded
GitHub
00:01:12
Bot
I think this issued is fixed, or at least I did achieve to reproduce it, either by using !helps or !tags commands.
Using json.dumps with an indent of 2 or 4 will look better and more readable because it puts newlines after brackets.
This makes more sense even though it does the same thing
for field_name in ('embeds', 'attachments'):
The closing parenthesis should be on a new line.
It strikes me that there is some redundancy in this code in terms of loops. The loop inside the else can be used instead of the messages_content comprehension e.g.
posted = False
async for message in messages:
if message.content == PERIODIC_PING:
posted = True
...
if not posted:
...send(PERIODIC_PING)
Note that in this case the history iterator would not need to be flattened.
I think an approach which would be better and still app...
Postgres backup completed!
Closes #465
Currently, there is nothing stopping very similar names being added to the rotation, eg. kosa and kosaa.
To solve this, a check is added that rejects names that have a close match using difflib.get_close_matches with a cutoff of 0.8.

A command !otn forceadd or !otn fa was added to override this check and add the name to the rotation...
Build 20191011.1 succeeded
GitHub
00:01:25
Bot
Closes #499
Currently, the #user-log channel provides no indication that a member has left the server due to being banned or temporarily banned, only if they voluntarily leave the server.
This is fixed by unignoring the member_ban event in the Infractions cog, and rerouting the log embed to #user-log in the ModLog cog.
Build 20191011.2 succeeded
GitHub
00:02:04
Bot
I think everything that's still relevant can go in #217.
Build 20191011.3 succeeded
Sebastiaan Zeeff
00:01:26
Bot
From #236:
Some cogs use the cog_check method to add a check to every command in the cog. The cog_check method is the predicate itself, which poses a problem if we were to use has_any_role, which is decorator check.
One solution would be to nab has_any_role from discordpy, place it in decorators.py, and split the predicate and check.
That way we get the...
After consideration, I think the current feedback is fine as it is to give confirmation on the actual channel name. Feel free to reopen the issue to discuss this topic further.
Hi! Is anyone working on this? I'd like to, take care of it if it's open. I already threw together a quick prototype to make sure I understood how to implement this - I should be able to submit a PR pretty quickly.
Build 20191011.4 succeeded
Sebastiaan Zeeff
00:01:18
Bot
Nobody is working on this, feel free to have a go at it!
6bd2ea4 Allow viewing log entries in the Django Admin. - jchristgit
87105f2 Pluralize properly. - jchristgit
6fa19e0 Use multi-column output from Django. - jchristgit
8309c94 Group fieldsets. - jchristgit
This PR adds log entries to the Django Admin (under the API section) to allow viewing them there.
The default overview displays log entries in chronologically descending order...

... and individual log entries can be inspected by clicking on them, with editing disabled to reduce clutter ...

In Safari it's black and white

195d66b Remove old import. - jchristgit
woops, forgot to run flake8 after moving to the django builtin stuff
Build 20191011.2 succeeded
GitHub
00:01:43
Site
shouldn't this subclass, call super(), and then strip the email, in case allauth does something useful here?
It's nice that you document this, but I think it would be better to just provide a link to the Django documentation for these three paragraphs
I think showing both the module and the logger name is redundant in most cases though technically they don't have to be the same name. I suppose it's fine to include it. In any case, the description for the module field is incorrect since clearly that is not a fully qualified name.
Does the list support filtering?
The kwarg is a bit hard to figure out without reading the function, would be nice to
self._apply_groups(user, account, deletion=True)
Allauth actually does nothing at all here, just return data. I figured this would be better for future work, since you can't actually tell Allauth which class to use - although I guess I could replace the entire class? I feel like that'd break things, though. I dunno.
What do you think?
I agree with you, @lemonsaurus asked me to document it though.
c55ff9a Clarify _apply_groups kwarg (Thanks, @jchristgit) - gdude2002
Looking at the .remove call above, can't we have these as generators?
Django evaluates this as a list by itself:
def add(self, *objs, bulk=True):
self._remove_prefetched_objects()
objs = list(objs)
# ...
via django/db/models/fields/related_descriptors.py
I think it's some kind of Apple thing, since these were generated by a website that I've used for a bunch of sites with no issue. it probably works.
Build 20191011.3 failed
GitHub
00:01:41
Site
Not fully convinced you need the if conditions here. I don't think it hurts to always run this and keep the code simpler
@jchristgit This Safari Pinned Tab icon must be a monochrome and transparent SVG, according to https://realfavicongenerator.net/. It offers several ways to convert the uploaded picture to a fitting format, including to upload a separate transparent PNG and convert it to a solid "shadow", which is what I did.
Probably that caused these small dot artifacts around the shape where we'd have a drop shadow in the original. Maybe there could have been a better approach by preparing a different ico...
There is a link to the Django docs, too. This is fine.
This looks pretty solid so I will have to resort to notpicking :(
The role mapping model is just about the cleanest way I have seen to accomplish this. I think in my last implementation of this I just hardcoded that, but a nice admin frontend for it is pretty brilliant.
That said, if the role mappings update, shouldn't the signal handler fire?
this should use assertEqual(..., 0)
this should use assertEqual(..., 0)
I'm not gonna comment on each, but you know the drill
Yeah, that makes sense.
Build 20191011.4 succeeded
Leon Sandøy
00:03:13
Site
can't we use type__in here instead?
.filter(type__in=('note', 'warning', 'kick'))
I am probably missing something, but what's the difference between Q and no Q?
isn't a docstring without method body valid on its own without pass?
The __str__ of discord.Member will already return the same thing.
The __str__ of discord.Member will already return the same thing.
await self.bot.api_client.post('bot/off-topic-channel-names', params={'name': name})
6d9cb1a Change pipeline testrunner to xmlrunner - SebastiaanZ
Build 20191011.5 succeeded
Sebastiaan Zeeff
00:01:20
Bot
As far as I can tell, there are a couple reasons to do this:
Those aren't necessarily real concerns for the time being, I guess. I do still need to get the URL from the provider though, and this is the official way Allauth provides to do it.
f6db7cc Simplify signals.py as per @jchristgit's review - gdude2002
404a9a7 Clean up signal tests as per @jchristgit's review - gdude2002
In any case, the description for the module field is incorrect since clearly that is not a fully qualified name.
Yeah, my original assumption was that it was, but for some reason getting the full module out of logging seems to be pretty hard. I can correct that in a separate PR.
Good catch on the missing signals. I'm leaving a short TODO list in this comment so I don't forget:
pre_save and check against object already in DB if possible, otherwise try to disallow updates)Build 20191011.5 succeeded
GitHub
00:03:06
Site
Are the comments about the token format really accurate? If the first part is indeed base64, the character set should be [a-zA-Z0-9+=\\]. However, in practice, I have not seen +=\. It states the middle is an integer but I have definitely seen both letters and numbers in there as well as a dash.
Build 20191011.6 failed
GitHub
00:01:35
Site
This has proven to be not the case.

Nothing. I changed my approach, copy-pasted it, and did not think about it properly. I'm working on an update of this PR, so I'll this here as well.
Build 20191011.7 succeeded
GitHub
00:01:37
Site
f158422 Allow filtering through metadata, and searching... - jchristgit
Yes, we can. I'll change that as well.
Does the list support filtering?
Now it does. I (well, Django) added filtering by app / timestamp / level and searching by message in f158422.

https://github.com/python-discord/site/pull/277 was closed. So this is blocked until that change is made.
@Akarys42 did you at least open an issue on the site repo so that someone else can make this change if you don't intend to make it yourself?
a6e4f85 Implement test cases suggested by @MarkKoz. - jchristgit
Agree about parse_rfc1123. I've added test cases for the others. The negative max_units one found an interesting case.
I think we should raise ValueError on max_units <= 0. What do you think?
Sleeping for negative seconds will just be gobbled by Python itself, as asyncio.sleep will just yield control to the event loop on sleep(n <= 0)
I disagree, since it doesn't split up long text into multiple lines. This makes the embed very cumbersome to read.
Compare:

Just tried this out. Filtering works well.
My concern now: can the ability to delete log entries be removed from the django admin? I feel like we would never want to do that.
Build 20191011.6 failed
GitHub
00:01:07
Bot
I think it's fine, too.
This PR is out of date with master and has some conflicts that need to be resolved. @lemonsaurus also rightly noted that we should rather have a solution that's robust instead of one that uses hard coded links for edge cases. Hard coded links will need to be maintained by hand and makes me wonder if more edge cases are going to come up in the future.
I didn’t opened a issue because I want to do it myself, I’m with my family this weekend, so I will probably do it next week is it okay for you.
I agree with @avayert
815f2b7 Rename the "cogs" extension & cog to "extensions" - MarkKoz
4f75160 Add enum for extension actions - MarkKoz
6cdda6a Simplify extension discovery using pkgutil - MarkKoz
f7109cc Replace with_role decorator with a cog_check - MarkKoz
19ad439 Add a generic method to manage loading/unloadin... - MarkKoz
yeah, I prefer this, too.
Is there a way to get the best of both worlds? Or too complicated?
ee18908 Remove transparent margin in logo_discord.svg's - ByteCommander
Also, a nice feature to have is the ability to link to source code on our repo based on the line number and module name (and/oror link in the tracebacks). Perhaps that can be saved for another PR though.
Connected!
Build 20191011.7 succeeded
GitHub
00:03:21
Bot
Build 20191011.8 succeeded
GitHub
00:01:38
Bot
Build 20191011.9 succeeded
GitHub
00:01:49
Bot
ee71c2c Prepare cogs on cog init & wait for bot ready flag - Jens
8a8ab19 Specify names of "prepare_cog" methods - Jens
ab5d972 Add missing awaits and call bot as attribut - Jens
b57f5c0 Merge remote-tracking branch 'origin/master' in... - MarkKoz
45631e9 Merge pull request #488 from K4cePhoenix/Refact... - jchristgit
The logo_discord.svg and .min.svg files had a transparent margin around the logo, instead of being cropped as closely to the content as possible. This made them unsuitable for favicon generation.
I removed this extra space using Inkscape and minified the result again using https://www.svgminify.com/
All PNGs which were generated from the original SVGs had no margin, so they remain as they are.
I think this looks pretty clean. Good PR.
We have something for this in bot/utils/time.py but it only works for deltas. Maybe you can cast to a delta here and use that function instead?
await role.edit(mentionable=False, reason="Automatic role lock - timeout.")
Build 20191011.10 succeeded
GitHub
00:02:48
Bot
Connected!
I think we will probably eventually write a real frontend for this instead of using the Django admin, and that would be a good place to put nice to have features like advanced filtering, links to relevant source code, maybe even for restricting deletion.
This should probably be a classmethod, so you can call it from the codeblock cog without passing the class as the first argument, whilst still having access to the other methods.
can a user DOS the bot by spamming this command and making it get stuck in ratelimits?
why not zero-width space to preserve the triple quotes in a readable way?
Disagree about <@id> being the best method. It doesn't always work, as Discord's clients will often only load online users into the cache, which means that these will display as something like the following:

This is especially true on mobile.
platform should probably be something like python3, I don't think docker makes a lot of sense here.
Also, the versioning doesn't have lots of value. I think we should export an environment variable like BUILD_COMMIT_SHA in the build process via git and use that here instead.
What's the actual implication of this? The cog doesn't work?
Okay yes, let's do this.
ee71c2c Prepare cogs on cog init & wait for bot ready flag - Jens
815f2b7 Rename the "cogs" extension & cog to "extensions" - MarkKoz
4f75160 Add enum for extension actions - MarkKoz
6cdda6a Simplify extension discovery using pkgutil - MarkKoz
f7109cc Replace with_role decorator with a cog_check - MarkKoz
Build 20191011.13 succeeded
GitHub
00:01:21
Bot
91a8813 Fix #346: display infraction count after giving... - MarkKoz
e120013 Resolve #357: show ban reason and bb watch stat... - MarkKoz
e118d22 Resolve #458: support exact timestamps as args ... - MarkKoz
184a546 Merge branch 'master' into moderation-tweaks - lemonsaurus
d29be57 Merge branch 'master' into moderation-tweaks - lemonsaurus
Connected!
Build 20191011.11 succeeded
GitHub
00:02:54
Bot
Build 20191011.14 succeeded
Leon Sandøy
00:02:46
Bot
Connected!
b3aa11b Deny LogEntry deletion. - jchristgit
My concern now: can the ability to delete log entries be removed from the django admin? I feel like we would never want to do that.
Done.
9b38f42 Migrate nominations to new Nomination model - SebastiaanZ
7136018 Add favicons; workaround for gitattributes - ByteCommander
9c43ef1 Merge pull request #279 from python-discord/fav... - lemonsaurus
5c1c863 Merge pull request #269 from python-discord/mig... - jchristgit
a185a25 Merge branch 'master' into simple-admin-log-ent... - jchristgit
Build 20191011.8 failed
GitHub
00:01:16
Site
The merge introduced a conflicting migration that needs to be fixed:
django.core.management.base.CommandError: Conflicting migrations detected; multiple leaf nodes in the migration graph: (0044_migrate_nominations_from_infraction_to_nomination_model, 0044_add_plural_name_for_log_entry in api).```
879d807 Improve formatting for has_delete_permission,... - jchristgit
673cc3b Resolve migration merge conflicts. - jchristgit
Build 20191011.9 failed
GitHub
00:01:11
Site
Build 20191011.10 succeeded
GitHub
00:01:46
Site
Sorry. Now I noticed the "add log entry" button which I think should also be removed. I could have sworn that button wasn't there before!
9424ae0 Deny manual LogEntry creation. - jchristgit
I don't mind it at all, no worries
Removed in 9424ae0
f
I was already wondering where that change went...
Amended to the latest commit
Build 20191011.12 succeeded
GitHub
00:01:38
Site
Build 20191011.15 succeeded
GitHub
00:01:25
Bot
Build 20191011.13 succeeded
GitHub
00:03:04
Site
aa26342 Bump PostgreSQL in compose. - jchristgit
The commit message contains instructions to upgrade, but here it is for "doppelt hält besser" again, since GitHub nicely pasted it in here:
This might break local PostgreSQL containers with the following error:
postgres_1 | 2019-10-11 20:01:48.772 UTC [1] FATAL: database files are incompatible with server
postgres_1 | 2019-10-11 20:01:48.772 UTC [1] DETAIL: The data directory was initialized by PostgreSQL version 11, which is not compatible with this version 12.0.
You wil...
Build 20191011.15 succeeded
GitHub
00:01:40
Site
I think so, but I don't mind the pass being there to more explicitly convey that it does nothing.
org-issue-137 talks about how we should make a slow mode for multi-message projects.
Slow mode is engaged and set to a 6-hour cooldown.
We need to implement a standardized format system to replace this with.
The format should be like this
Title:
GitHub:
Description:
I think we should raise ValueError on max_units <= 0. What do you think?
If it exhibits strange behaviour then yes, just do that. Do you think that when it's too high it should also be raised, for consistency? Or could take it further and also do it for the precision.
Because then if you try to copy paste it's not immediately obvious why the code blocks don't work
This is what the C# server does for example:

Yeah, most likely; this didn't occur to me. It should probably be gated behind a cooldown, and I don't know what the actual ratelimit is but some people said it's 1/1. How strict should the cooldown be?
In retrospect I kinda like Source: better then Github:
not sure that Main contributor fits, since show your projects is suppose to be done with the main maintainer.
I agree. It should also be last so it looks better next to the embed.
org-issue-141 talks about how certain members are confused about the red X that is added to embeds.
Like the one that !eval uses

The consensus is that we change it for a nice trashcan 🗑
Ideally we'd have a custom emote that resembles a trash can, but looks nicer than twemoji's.
I was thinking something like this with simpler lines would look good
![]()
yeah, I was also thinking something like that @Numerlor
d6bedd1 Make textual changes to testing guide - SebastiaanZ
I feel like without more than anecdotal evidence that +=/ do not occur, it would be safer to just include the full base64 character set.
I'm also not comfortable assuming the length of the substrings will be constant, neither right now and especially not in the future.
So, for that reason, I think we should just go ahead and merge this as is.
ee71c2c Prepare cogs on cog init & wait for bot ready flag - Jens
815f2b7 Rename the "cogs" extension & cog to "extensions" - MarkKoz
4f75160 Add enum for extension actions - MarkKoz
6cdda6a Simplify extension discovery using pkgutil - MarkKoz
f7109cc Replace with_role decorator with a cog_check - MarkKoz
Build 20191011.16 succeeded
GitHub
00:01:29
Bot
Connected!
Build 20191011.17 succeeded
Leon Sandøy
00:02:51
Bot
<@&267628507062992896> WARNING: Unable to get DEFCON settings!
Add emojis indicative of success when you use !eval.



The <p> tags are meant to be for paragraphs. You have paragraphs present but instead use the <p> tag as a content wrap and split paragraphs with <br/><br/>.
I'd suggest using the following structure:
<p>
We're a large community focused around the Python programming language.
We believe anyone can learn to code, and are very dedicated to helping
novice developers take their first steps into the world of programming. We also...
9b38f42 Migrate nominations to new Nomination model - SebastiaanZ
d872264 disable usage of pyuwsgi on windows due to inco... - jos-b
7136018 Add favicons; workaround for gitattributes - ByteCommander
6bd2ea4 Allow viewing log entries in the Django Admin. - jchristgit
87105f2 Pluralize properly. - jchristgit
Currently, when a user posts a message with far to many lines (usually a long code block between a couple messages), the user will receive a temporary mute from the bot. I propose that we change that behavior slightly. We keep the check for too many lines, however we add in the check for formatting that the bot also uses to inform them about the Discord formatting.
By doing this, we can instead point users to the hosted hastebin rather than punishing them for not knowing about it.
953195e Signals: Handle (and test) mapping updates/dele... - gdude2002
Build 20191011.16 succeeded
GitHub
00:01:32
Site
fb6d75d Update favicons (rounded corners, other improve... - ByteCommander
Build 20191011.18 succeeded
GitHub
00:01:35
Site
1ced4fc Update the landing page text. - lemonsaurus
0c43428 Address code review from Scragly and Mark. - lemonsaurus
4c18a63 Merge branch 'master' into tizzysaurus_landing_... - lemonsaurus
c44b5a7 Make the homepage paragraphier. - lemonsaurus
7dee86f Merge branch 'tizzysaurus_landing_page_text' of... - lemonsaurus
1ced4fc Update the landing page text. - lemonsaurus
0c43428 Address code review from Scragly and Mark. - lemonsaurus
4c18a63 Merge branch 'master' into tizzysaurus_landing_... - lemonsaurus
c44b5a7 Make the homepage paragraphier. - lemonsaurus
7dee86f Merge branch 'tizzysaurus_landing_page_text' of... - lemonsaurus
Favicon updates according to the discussions in the server.
Related to python-discord/branding#31
Alright, I think that's all of your review addressed, @jchristgit!
Build 20191011.20 succeeded
GitHub
00:01:29
Site
Build 20191011.21 succeeded
GitHub
00:01:24
Site
Build 20191011.19 succeeded
Leon Sandøy
00:03:00
Site
We recently decided to migrate the bot's test suite to the unittest framework. One of the main reasons for that decision was that one of our other core repositories, the site repository, uses the test tools Django provides, which are based on unittest. Since we want to have some consistency between our repositories, we decided that it would be better to use unittest for the bot as well.
This draft pull request is the start of that migration and will be our main way of keeping track...
Build 20191011.18 succeeded
GitHub
00:01:17
Bot
Tested. This works like a charm, and it looks great at every size. Excellent work, @ByteCommander
9b38f42 Migrate nominations to new Nomination model - SebastiaanZ
1ced4fc Update the landing page text. - lemonsaurus
0c43428 Address code review from Scragly and Mark. - lemonsaurus
4c18a63 Merge branch 'master' into tizzysaurus_landing_... - lemonsaurus
c44b5a7 Make the homepage paragraphier. - lemonsaurus
Build 20191011.22 succeeded
GitHub
00:01:35
Site
very, IMO. I don't see a big need for regular users to run this on a constant basis
but maybe the others have a better idea
453dd22 Bump the site PostgreSQL version to 12. - jchristgit
Build 20191011.19 succeeded
GitHub
00:01:15
Bot
Build 20191011.23 succeeded
Leon Sandøy
00:03:53
Site
ee71c2c Prepare cogs on cog init & wait for bot ready flag - Jens
815f2b7 Rename the "cogs" extension & cog to "extensions" - MarkKoz
4f75160 Add enum for extension actions - MarkKoz
6cdda6a Simplify extension discovery using pkgutil - MarkKoz
f7109cc Replace with_role decorator with a cog_check - MarkKoz
7625d2a
I don't think we should raise for high values as that doesn't cause any odd behaviour.
ee18908 Remove transparent margin in logo_discord.svg's - ByteCommander
46f725a Remove transparent margins from logo_solo.svgs too - ByteCommander
793af61 Add logo_solo_flat as variant of solo without d... - ByteCommander
56695ff Make logo_solo_flat.svg square enough for favic... - ByteCommander
aab71e6 Replace old favicons with realfavicongenerator ... - ByteCommander
Build 20191011.20 succeeded
GitHub
00:01:19
Bot
9b38f42 Migrate nominations to new Nomination model - SebastiaanZ
1ced4fc Update the landing page text. - lemonsaurus
0c43428 Address code review from Scragly and Mark. - lemonsaurus
4c18a63 Merge branch 'master' into tizzysaurus_landing_... - lemonsaurus
c44b5a7 Make the homepage paragraphier. - lemonsaurus
that sounds like a good idea
Build 20191011.21 succeeded
GitHub
00:01:18
Bot
Build 20191011.24 succeeded
GitHub
00:01:29
Site
Build 20191011.25 succeeded
GitHub
00:02:58
Site
I think ⚠️ is better for the third one since it's yellow rather than red. More distinct from the :x: that way.
1ced4fc Update the landing page text. - lemonsaurus
0c43428 Address code review from Scragly and Mark. - lemonsaurus
4c18a63 Merge branch 'master' into tizzysaurus_landing_... - lemonsaurus
c44b5a7 Make the homepage paragraphier. - lemonsaurus
7dee86f Merge branch 'tizzysaurus_landing_page_text' of... - lemonsaurus
Build 20191011.26 succeeded
GitHub
00:01:31
Site
Build 20191011.23 succeeded
GitHub
00:01:23
Bot
Build 20191011.27 succeeded
GitHub
00:03:02
Site
Build 20191011.22 succeeded
GitHub
00:02:40
Bot
Connected!
Build 20191011.24 succeeded
GitHub
00:01:16
Bot
Build 20191011.26 succeeded
GitHub
00:01:21
Bot
Build 20191011.25 succeeded
GitHub
00:02:49
Bot
Connected!
Connected!
Build 20191011.27 succeeded
GitHub
00:02:51
Bot
Nobody noticed it in the review either...
382c512 Bump PostgreSQL in compose. - jchristgit
578ea35 Merge pull request #283 from python-discord/fav... - SebastiaanZ
aa4a5dc Merge branch 'master' into upgrade-postgresql-t... - SebastiaanZ
d1d0d0c Merge pull request #282 from python-discord/upg... - SebastiaanZ
d1d968d Merge branch 'master' into favicons - ByteCommander
Build 20191011.29 succeeded
GitHub
00:01:26
Site
redirect_output has been adjusted to run the delete_invocation inside a task as the help command will wait for that to run before sending the help or doing anything else.
pagination has been adjusted to support deleting the paginated message if cleanup is True, and an optional description that is present through all pages of pagination.
The help command has been refactored to subclass commands.HelpCommand. This means that it now supports methods such as `ctx.send_help(ct...
Build 20191012.1 succeeded
GitHub
00:01:12
Bot
Build 20191012.2 succeeded
GitHub
00:01:31
Bot
Build 20191012.3 succeeded
GitHub
00:01:20
Bot
I should add that while the current help is case-insensitive, this one is not. Currently getting the cog help for the Reddit or Free cogs is impossible. While not an issue for these since they only have 1 group command, if we were to have a Cog with more than 1 command it would be awkward.

The bot tells you you should try either of these, but you can't actually reach the se...
Thanks for taking the time to clean up some of the other modules.
Postgres backup completed!
Reddits API rules used android as an example for platform, so I deemed docker to be fitting. We could also be more descriptive and change it to something like docker-python or maybe even docker-python/aiohttp.
Using BUILD_COMMIT_SHA as version string sounds good to me.
Yes pretty much. Without client id or secret, we can not get an acces token and therefore not use Reddit API.
Perhaps this should be changed to something more descriptive like No client credentials were provided. Cannot use Reddit API and unload the cog?
Yeah, that sounds like a good idea.
Seems to focus on the same issue as the second part of #439
b5e1155 Move the sync cog tests to `unittest. - jchristgit
In addition, there are plenty of files in bot that currently do not have any testing at all.
I'm gonna open PRs for the remaining tests to be moved - and I'll get back to adding new tests afterwards.
42abcc5 Move tests.test_resources to unittest. - jchristgit
e1c4f08 Move tests.test_pagination to unittest. - jchristgit
4f22550 Move the rules.attachments module tests to `u... - jchristgit
6d3af7c Move the antispam cog tests to unittest. - jchristgit
63fad8b Move the security cog tests to unittest. - jchristgit
8656973 Move the token_remover cog tests to unittest. - jchristgit
Build 20191012.4 succeeded
GitHub
00:01:30
Bot
Disagree about <@id> being the best method. It doesn't always work, as Discord's clients will often only load online users into the cache, which means that these will display as something like the following:
This is especially true on mobile.
Yes I though even if the user was not in the cache clicking on the mention would load it, unfortunately it's not true. I wil...
After some more tests, to use fetch_user I have to turn infraction_to_string into a async function which tons of errors. Do you have any idea to resolve them?
<@&267628507062992896> WARNING: Unable to get DEFCON settings!
Build 20191012.5 succeeded
Joseph Banks
00:02:58
Bot
Connected!
I can't think of anything great right now but I'll put it on the backburner in case I have a revelation
Build 20191012.6 succeeded
GitHub
00:01:26
Bot
I don't really understand why actor is searched in a different way than user, so I leaved it the way it was, but instead of doing this:
actor_id = infraction["actor"]
guild = self.bot.get_guild(constants.Guild.id)
actor = guild.get_member(actor_id)
I think this would be better:
actor_id = infraction["actor"]
actor = self.bot.get_user(actor_id)
I would be also more consistent.
@jchristgit no more use of <@id>
@jchristgit no more use of <@id>
Using .mention is the same as building <@id> yourself though: https://github.com/Rapptz/discord.py/blob/c7a1f5e6e9f18fff3128d67b6d098a4ab842fcb7/discord/user.py#L204-L207
The only exception is that you're also fetching the user now which can make this command very slow due to the rate limiting that endpoint has set. get_user loads from cache.
I think the most simple approach to this is to:
Using
.mentionis the same as building<@id>yourself though: https://github.com/Rapptz/discord.py/blob/c7a1f5e6e9f18fff3128d67b6d098a4ab842fcb7/discord/user.py#L204-L207
Damn, how could I forget? I said it myself...
Also I am pretty sure that
actor.mentionrelies on it.
The only exception is that you're also fetching the user now which can make this command very slow due to the rate limiting that endpoint has set.
get_userloads from cache.
Fetching only if the user i...
Closes #274
current output:

What was proposed in the channel:

My proposal:

I left the list to make clearer the fact that the output...
Build 20191012.7 succeeded
GitHub
00:01:28
Bot
Howdy! This PR is intended to address issue #471 (auto-deleting messages with potentially malicious file attachments). After discussing with @lemonsaurus, it became clear that a new Cog would make for a cleaner implementation, instead of a new rule within the AntiSpam Cog. I'll be happy to elaborate on those reasons if anyone would like further details.
Here's about the simplest implementation I could conceive, using the suggestions made in the issue's notes. Some points for further disc...
Build 20191012.8 failed
GitHub
00:01:36
Bot
Build 20191012.9 succeeded
GitHub
00:01:14
Bot
Postgres backup completed!
Build 20191013.1 succeeded
GitHub
00:01:17
Bot
I added a custom case-check for when the help command runs the wolfram custom cooldown check, so that it won't update/add to the cooldown counter, while still returning the true cooldown status. Before, if you ran !help enough, it would start spitting out cooldown error messages, too.
There is some code redundancy here. I would rather make only on for.
for attachement in message.attachments:
#if python_file add to detected_pyfile
#if not in whitelist add to rejected_attachments
Are you sure one message can have several attachements?
You can use
if rejected_attachments:
....
instead, because an empty list is False
Hey!
Thank you for your time! I did a few remarks on the form of your code, but I'm a small contributor, so you can wait for a more in-depth review.
if there can be several attachements, why use a if else structure?
Just because I got a similar review comment, could this utilise the utils.time.humanize_time method, if you cast inactive to a timedelta?
Could this be shortened to a single line?
7768616 Add subTest + move test_resource to resources s... - SebastiaanZ
562ede8 Move test_attachments.py to tests/bot/rules dir - SebastiaanZ
4f22550 Move the rules.attachments module tests to u... - jchristgit [562ede8](https://github.com/python-discord/bot/commit/562ede819308929900e3e0c6a41ae61ca32abab6) Move test_attachments.py to tests/bot/rules dir - SebastiaanZ [86e942d`](https://github.com/python-discord/bot/commit/86e942deeb4e04766d25861b4de9f2d96e232f3a) Merge branch 'rules-attachment-tests-unittest' ... - SebastiaanZ
There are some mock helpers set-up for Bot and Context in tests.helpers that follow the specifications of the objects they're mocking (using the spec parameter of Mock/MagicMock) and have some commonly used attributes already set up (like the author attribute for the Context mock with a Member mock that follows the discord.Member spec and send for the Context mock with an AsyncMock).
Simplest way to do this I think is not to worry about mentions at all, and display the infos as raw text. I will modify it tonight.
Build 20191013.2 succeeded
Sebastiaan Zeeff
00:01:43
Bot
Hey, @kraktus, I was wondering, are you also a member of our Discord community? What's your handle on there?
Testing seems to suggest that you're correct, they cannot. I misunderstood. Thanks!
Good feedback @kraktus, thank you! As for the message deletion, that begins on line 39, directly above the def setup()... function.
d26eba1 Show matched word and location in watchlist embed - kosayoda
Closes #374
This pull request adds the word triggered from the watchlist and a short snippet of the message that contains the trigger to the embed sent to #mod-alerts whenever the filter catches a naughty message.
Rationale from the issue:
Without knowing the trigger word, reading the original content of the message to figure out what it was is unnecessarily difficult. In order to see if moderation action is required, we have to first find what the word was and where it is in t...
Build 20191013.3 succeeded
GitHub
00:01:25
Bot
I didn't want to clutter things up with too much response text, and deemed the pastebin advice to be the more important of the two. Just a personal judgment call, I'll be happy to revise if requested by staff.
Good feedback @kraktus, thank you! As for the message deletion, that begins on line 39, directly above the
def setup()...function.
Yep I did not see it because I reviewed the first commit :)
612994a Use MockBot and MockContext. - jchristgit
hello no i just wanted to reply to the comment not start a review tyvm
@SebastiaanZ I changed it to take the same username as here: kraktus :)
Build 20191013.4 succeeded
GitHub
00:01:26
Bot
Updated, should be finished now.
88c2579 Check partially hidden words against the wordlist - AnonGuy
Build 20191013.5 failed
GitHub
00:01:02
Bot
Build 20191013.6 succeeded
GitHub
00:01:23
Bot
Build 20191013.7 succeeded
GitHub
00:01:35
Bot
Well that's not what MarkKoz says:
Also, I think it'd be better to replace the user's name with a nickname if it's available rather than adding it on a separate line.
I agree with him because adding the nickname to the username will longer too much the line
Well, depends on what the staff deems to be useful information. I was thinking both the nickname and discriminator are not needed because the ID will always be shown anyway.
I think error handling would be needed for the API calls for two reasons:
NotFound is probably raised when a user deletes their accountHTTPException could be raised but it could happen...It doesn't look like discord.py will cache the user returned from the API call which isn't ideal. Could we cache it ourselves.
With the caching and error handling, perhaps it's becoming more effort than it's worth. But I think moderators, the ones going to use this command, should be the ones to comment on whether they'd be OK with not displaying names if not in the cache.
This is why we used to have this. I guess we deemed it no longer necessary since we did not foresee having to use non-PyPI releases anymore.
And I don't know if it's OK to move to 1.3a. Hopefully someone more familiar with how things are going with that version's development can comment.
It would be even more robust to check if authentication succeeds rather than just if the credentials are present. In any case, I agree that unloading the extension should be done.
Probably a good idea to create a mock helper for messages and use mocks with specs for some of the attributes here (like the author and channel).
Don't think so because it uses full names for units instead of single letters.
However, looking over the time-related code here, it could be improved. The delta object could be appended instead of just the seconds to avoid having to do the redundant math below to get the minutes and hours.
For some reason your picture looks worse than the original proposal. I am pretty sure it's just a difference in fonts. Yours has a lot less space between lines which worsens legibility to the point of me almost not wanting it to be formatted it that way.
This is a bit weird cause it could just be match = triggered[0]. Maybe you did it that way because the name triggered is misleading. In that case, it could just be renamed to something like match anyway.
How about return match[0] here? Would have to explicitly check triggered is not False though (perhaps better to return None instead of False in that case).
Messages can definitely have multiple attachments. Though the suggestion of using a single for loop is still a good suggestion.
I think the more conventional syntax for lists should be used as it's more consistent with the rest of the config.
Why is this done? As an alias?
This wasn't defined in constants.py.
Postgres backup completed!
Sure. FWIW, I've had a (400 guild) bot running on latest versions of v1.3a for months now with absolutely no issues. I don't believe (and just had a quick look at commit history) that any breaking changes have been made so far. Instead, small additions (new methods or not breaking), bugfixes and doc changes have been made.
Yeah, please avoid changing dependencies without first discussing it, especially when you're going to entirely change the release source.
For example, changing things to use v1.3a requires all our docker images and our CI to use git for installing, which we've purposely avoided. It would greatly impact our build times for CI/Docker Images thanks to pipenv version resolution and git installation taking significant extra time.
Slipping this into a PR would also result in our docker image...
Unnecessary major dependency source change that will negatively impact CI/CD.
Unnecessary major dependency source change that will negatively impact CI/CD.
I ran pipfile lock/update and it seemed to update other deps in the Pipfile.lock. Do I need to revert these too?
If you've not actually changed any deps you can just reset the Pipfile and Pipfile.lock to avoid any edge cases, but for reference, updating within scope of the existing pins are usually less of a concern and are typically done anytime a new dep is added anyhow.
Very high quality PR, but I do have a few requests before I can approve.
Again, this docstring could stand to be improved, especially considering the complexity of this setup method.
What are we setting up? Why?
This class name is too generic. We might have other signal listener classes other places in the site, so it would probably be good if we make this class name more specific. something like AllauthSignalListener would be fine.
Besides, SignalListener sounds a bit like a Django internal, and it feels like it should take arguments that configure which signals to listen to, or something.
Don't we lint for docstrings on functions like this?
Either way I think there should be a docstring.
Perfect. Thanks for that, and sorry for the issues, taken note for future 👍
Should I resolve the conflicts?
You can yep, it's up to you how you want to do it though, either by adding a new resolution commit or by doing a rebase to maybe take advantage of cleaning up history on this branch while also applying changes on top of current origin master without conflict.
We don't lint for docstrings on private methods - just public ones - but I'm happy to add one.
Do you mean, like, so they're not all on separate lines?
Not sure what you mean by lining up here either
Connected!
no no, just the inline comments.
created=None, # Not realistic, but we don't use it
using=None, # Again, we don't use it
thing() # this
with.fire() # looks
pull_chain() # messy
thing() # this
with.fire() # looks
pull_chain() # clean
I kind of just pre-emptively did this to make sure garbage collection didn't screw us over, but I can check.
Works fine without that, yep.
Build 20191014.1 succeeded
GitHub
00:02:20
Site
d52c5cb Improve homepage flex responsiveness - ByteCommander
382c512 Bump PostgreSQL in compose. - jchristgit
fb6d75d Update favicons (rounded corners, other improve... - ByteCommander
20b757e Merge branch 'master' into fix-home-responsive - lemonsaurus
0613919 Merge pull request #280 from python-discord/fix... - lemonsaurus
Build 20191014.2 succeeded
GitHub
00:01:54
Site
That seems reasonable to me.
I think we should factor out the mocking of those Discord "Model" that we use a lot and typically require the same or a very similar set-up. Since it does mean we'd basically have to maintain and test a small discord.py mocking library, I'd say we should restrict it those those base models though. I think that with a set of Member, Guild, Role, Message, TextChannel, Context, Bot, and Role we've covered a lot of things that we typically usr.
0dabafc Move test_security to tests.bot.cogs - SebastiaanZ
Absolutely sexy. LGTM
Build 20191014.1 succeeded
Sebastiaan Zeeff
00:01:46
Bot
Very high quality PR, but I do have a few requests before I can approve.
99bc330 Initial Allauth dependency and settings/urls - gdude2002
8cfff31 Remove accidental static files commit; fix Allauth - gdude2002
b83f1b2 Re-add erroneously removed static files - gdude2002
01c0598 Disable Allauth email verification - gdude2002
99263f6 Add model to map Discord roles to Django permis... - gdude2002
Build 20191014.3 succeeded
GitHub
00:04:04
Site
9de24c0 Migrate test_constants to unittest - SebastiaanZ
Migrates test_constants.py to the unittest framework. As with the pytest version, it does not yet test container types.
Note: There is a small, but important logical difference with the pytest version. The pytest version skips the rest of the section once the first container item was encountered within a section. This version isolates the inner iteration over section items using self.subTest, making sure we still check the rest of the items in the section.
db37eca Wiki: Permissions hotfix - gdude2002
This is a quick hotfix for a problem we noticed after Allauth was implemented, which allows any user to edit the wiki.
Build 20191014.4 failed
GitHub
00:01:24
Site
Build 20191014.5 succeeded
GitHub
00:02:10
Site
Yes, to clean up its use later on - a.filename.lower().endswith(tuple(AntiMalwareConfig.whitelist)) just looked ugly and unclear to me. Also I think I originally intended to use the whitelist more than once for some reason.
What do you prefer?
Build 20191014.6 succeeded
GitHub
00:03:51
Site
Okay. Can you help me understand why? Not trying to be argumentative, just trying to improve. Is the single loop preferred because iterating twice is simply more expensive? I considered that but thought the readability was better in this case, and that the size of the lists being so small would make it a trivial performance hit.
Is this more like what you'd want to see instead?
rejected_attachments = list()
detected_pyfile = list()
for attachment in message.attachments:
...
This is a small PR in order to bring the wiki back into spec post-Allauth.
Unfortunately, Django-Wiki has insecure defaults for people that have any kind of user account, and assumes that everyone that isn't anonymous must have some kind of editor permission. We've rectified the basics in #285 already at the settings.py level, this just fixes a few frontend issues.
Build 20191014.7 succeeded
GitHub
00:02:19
Site
I return the match because I still need to access the the match start and end positions to get the Location: information, here:
surroundings = m.string[max(m.start() - 10, 0): m.end() + 10]
I do suppose I can rename triggered to match, since they're functionally the same thing and would provide clarity in certain areas. What do you think?
2134ee4 Wiki: Show breadcrumb without menus for users t... - gdude2002
Build 20191014.8 succeeded
GitHub
00:01:49
Site
True enough, it does a bit, but the focus on this one is more about how the moderation triggers work. Although I agree they could be condensed into one.
I've added a 2 per 3 minutes ratelimit with staff bypasses. I chose 2 because there is a subcommand that they might want to compare to the parent invokation and letting them do that immediately seemed not-too-bad. 3 minutes was arbitrary.
Build 20191014.2 succeeded
GitHub
00:01:38
Bot
Closing Reason: Duplicate of #439
Connected!
Build 20191014.3 succeeded
GitHub
00:03:07
Bot
@MarkKoz @jchristgit I'm going to add a couple more specific mocks and tests, but I'll resolve this during the merge.
I'll go ahead and take care of this one. Already have a couple of ideas on how I want to address it.
well yeah, it's twice as expensive if you iterate the same list twice. But I also think it has better readability when you avoid the list comprehensions. Consider for example this line:
len([a for a in message.attachments if a.filename.lower().endswith('.py')]) > 0
You'd read this sequentially by saying to yourself "okay, so it's the length of, uh, a list that for every attachment in message.attachment but only if the filename ends with py, a list of items that match this co...
I was expecting flake8 to nag here, but I guess there's no lint for it.
Via PEP 8, the first argument to a classmethod should be named cls
One more thing. Aside from that LGTM
Build 20191014.9 succeeded
GitHub
00:03:48
Site
Build 20191014.10 succeeded
GitHub
00:03:17
Site
Build 20191014.4 succeeded
GitHub
00:02:57
Bot
Connected!
my memory fails me what I meant originally, but since we require 100% coverage anyways, this is not needed.
Oh, my bad, I didn't realise there was a relevant issue for the shields when I pushed that. I should have checked, sorry!
The same checks need to be present for message edits as well.
e4e01cd Add more specialized Mocks to tests.helpers - SebastiaanZ
57432d5 Merge branch 'unittest-migration' of github.com... - SebastiaanZ
Do this means this should be left alone for now?
8a83d68 Move the sync cog tests to unittest. - jchristgit
42abcc5 Move tests.test_resources to unittest. - jchristgit
e1c4f08 Move tests.test_pagination to unittest. - jchristgit
4f22550 Move the rules.attachments module tests to u... - jchristgit [6d3af7c](https://github.com/python-discord/bot/commit/6d3af7ccbc8a0ff0d9657c562707281a3a7c2ee2) Move the antispamcog tests tounittest`. - jchristgit
Uhm, I messed up the git history... But, I've merged this correctly on the command line, so I'm just going to close this. Sorry for that. (Please don't hurt me.)
20f1ced Remove empty tests.cogs folder - SebastiaanZ
Build 20191014.5 succeeded
Sebastiaan Zeeff
00:01:21
Bot
Since we're suggesting using a normal for loop, I don't see this alias as useful any more.
Build 20191014.6 succeeded
GitHub
00:01:48
Bot
0159a60 Create the !mention command - mathsman5133
ade137d revert back tests.cogs.test_information - mathsman5133
f1522ad revert back tests.cogs.test_information. I go... - mathsman5133
5eda443 add requested changes for review - mathsman5133
08ef78f properly send message with asyncio.run - mathsman5133
Build 20191014.7 succeeded
GitHub
00:03:03
Bot
Connected!
#491 Implemented a command to automatically unlock a given role by it's ID or name, and automatically re-lock after it's been used.
#441 Implemented a role info command.
This issue is to discuss some refinements I had in mind involving these two commands:
role command (from #441) to a command grouproles alias to role command groupmention command to sub-command of rolemention command name to unlock (!role unlock)lock comm...Build 20191014.8 succeeded
GitHub
00:01:22
Bot
Build 20191015.1 succeeded
GitHub
00:01:19
Bot
Well that was a trek and a half, but got there in the end. The last thing we need is an actual webhook ID (I've just chucked 123456789 in there for the moment)
The Discord client will not be able to resolve the name of a mentioned user if the user is not in the cache so it ends up just displaying the ID. Furthermore, the user ID can be copied by right clicking a mention. Therefore, the text in parenthesis should be the user's nickname as plain text instead of their ID.
This will need to account for working with the proxy user object which only has an ID - no name or nickname. In such case, don't display anything in addition to the mention; I don'...
Would anyone miss this functionality?
0753cd6 Implement tox to allow for local testing across... - sco1
Postgres backup completed!
We're still discussing whether or not we should actually remove this internally.
I'm not sure why, but the old help-embed allows for more vertical space than the proposed help-embed and I'm not sure I like that:
Current output:

As you can see, it fits 8 individual commands from three separate cogs.
Output of the proposed rewrite
620607373828030464)365960823622991872)473092532147060736)Since this restriction should hold for...
We have discussed this with the moderation team and no one seems to be opposed to removing the "actor ping".
To still get explicit feedback about DMs failing to be delivered to the user, we would like to add an emoji to the infraction confirmation message in a similar style to the "swooshing" envelope we use to signal that a DM was successfully sent:
...
The emoji has been added as :failmail: (ID of 633660039931887616) to the emoji server.
Build 20191015.4 succeeded
GitHub
00:01:48
Bot
As it is right now, we are unable to get a rule directly using the alias !rules, which returns a generic rules embed.

The only way right now is to use !site rule, but that returns rules as zero-indexed.

A few amendments have to be made.
Details:
I can't remember the last time we've seen a slack invite being spammed in our server. Do we still want to add a specific filter for it?
There's no harm keeping it and I see no benefit in removing it.
We don't have one, I believe.
Oh, my mistake then. If it's not present then it's probably unlikely something to worry about.
Okay, I'm going to close this for now, then.
This is ready for review. It's pretty massive, but a decent amount of the test files have already been reviewed.
For context:
This is the first phase of getting a nice test coverage for the bot, but there's still a long way to go.
Most of the specific test files have been migrated "as-is", without a lot of changes to their internal testing logic. If you feel that certain tests can be enhanced, feel free to make note of that, but we may postpone the implementation of those enhancem...
Closes: #536
!rule <num> and !rules <num> besides !site rule <num>
Build 20191015.5 succeeded
GitHub
00:01:47
Bot
Build 20191015.6 succeeded
GitHub
00:01:42
Bot
2a1b017 Add check to !otn add to prevent too similar na... - kosayoda
fedf5bb Add !otn forceadd command. - kosayoda
d05bf26 Utilize str of discord.Member in logging an... - kosayoda
645f8f7 Merge branch 'master' into off-topic-check - kosayoda
61bbe0c Merge branch 'master' into off-topic-check - kosayoda
Connected!
Build 20191015.7 succeeded
GitHub
00:03:53
Bot
I will add a exception catch to the fetch_user function. I don't change the display output for now, waiting for a mod opinion.

This is a loser screenshot. We can see that the channels's mention are not aligned, but I don't understand why, according to the code it should be the case.
will be done, thanks
@sco1 Hi, I am sorry but I am not sure about what you mean. Do you say the bot should also checks if a message contains a token every time a message is edited?
With how the symbols are currently loaded to the inventories dict, earlier entries get overwritten by newer ones.
This can have negative impact, most notably when loading the 2to3 tool which overrides things like print, zip or input and returns mostly useless docs to them.
It could be solved by never overwriting existing dict entries, or by defining a list of modules that won't override existing dict entries, to keep cases where it's better to not do it (object as an example)
Sec...
Build 20191015.8 failed
GitHub
00:01:18
Bot
Postgres backup completed!
When you disable defcon the text message in #mod-log is DEFCON enabled it should say DEFCON disabled
It works like this in the #defcon channel.
Current message in #mod-log

but a few months ago you can see this behaviour.
Message from 08.08.2019 in #mod-log

co...
I would like to work on this.
I was planning to get to it if it's approved
On Wed, Oct 16, 2019, 11:32 Atul mishra notifications@github.com wrote:
I would like to work on this.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/python-discord/bot/issues/538?email_source=notifications&email_token=AGFP55DQLGAXVJQN25NOQOLQO3NTLA5CNFSM4JBDE2F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBL2QUA#issuecomment-542615632,
...
This is a tracking issue, just for an overview of the functionality needed and a note that I'm working on it.
Now that Allauth is implemented and users can login with Discord, we need to provide a way for users to manage their accounts. The proposal is simple: A user settings page.
This page should include the following, ideally:
If you can add more details on this, I would like to pick this.
I would like to work on this. Thanks!
Build 20191016.1 failed
GitHub
00:01:22
Bot
Sounds good to me, feel free to start work on it @Numerlor
@atmishra I've assigned you to this task, so feel free to start work on it. If you need any assistance with dev environment setup, feel free to ask questions in the #meta channel in the discord server.
@atmishra It appears you've not yet contributed to our org before, so welcome. I've accepted you to work on another task for now, so I'm going to leave this one open for others until that's complete.
2a1b017 Add check to !otn add to prevent too similar na... - kosayoda
fedf5bb Add !otn forceadd command. - kosayoda
d05bf26 Utilize str of discord.Member in logging an... - kosayoda
645f8f7 Merge branch 'master' into off-topic-check - kosayoda
61bbe0c Merge branch 'master' into off-topic-check - kosayoda
Build 20191016.2 succeeded
GitHub
00:01:45
Bot
Build 20191016.3 succeeded
GitHub
00:03:42
Bot
Connected!
Currently, when we deliver an infraction, it will show the infraction total in the bot's response.

This is a cool feature, but should not happen in public channels. So let's do something about that.
Infraction total can show up in the following channels:
#admins - ID: 365960823622991872
#mod-spam - ID: 620607373828030464
#mods - ID: 305126844661760000
#helpers - ID: 38547...
If both a rejected and a py file are found then you can break out of this loop.
Does it really have to be a tuple or can it just be any sequence? Presumably it is was list before you converted it to a tuple.
It would make more sense for this to be a boolean.
If I were to use the list of symbols/doc module names to specify which can overwrite and which cannot, where should I put it? Skimming through it, it looks like a dozen or so will be needed
Hmm, I think I see what you're saying. If that's desired, couldn't it be slightly simpler - if a .py file is found, it can break, since we are prioritizing the message sent for .py files?
Oddly enough, it does have to be a tuple or str: https://docs.python.org/3/library/stdtypes.html?highlight=endswith#str.endswith
Build 20191016.4 succeeded
GitHub
00:01:38
Bot
Yeah, that is a good point. It'd make more sense to have the .py check come first. But if another extension is found, it shouldn't break because maybe another attachment later will be a .py file.
So as I mentioned, this check should come first to make things a bit more efficient. You'd also have to rework the if statements to be like:
`` py
if detected_pyfile:
...
elif rejected_attachments:
...
Initial triage is taken care of, going to close this meta-ish issue in favor of some more specific ones.
Python 3.8 needs to be added to our Azure CI.
This is dependent on Microsoft adding 3.8 to their hosted Ubuntu agent: https://github.com/microsoft/azure-pipelines-image-generation/issues/1317
With Python 3.8's updates to ast adding native type comment support, we should be able to lean on the built-in module over typed-ast for 3.8 onward.
Importing and installation is handled in c78a5d55f725972d1389c1eed8eda1fe58918eb3, but our type comment tests are now failing in Python 3.8 for reason(s) TBD.
Python 3.8 introduces positional-only arguments, which are handled explicitly in the AST as their own argument type so they won't be checked for type annotations.
Since this will add a new error code, this will likely mean we'll bump to v1.2.0 instead of v1.1.1
Three of our linting plugins are having issues with Python 3.8 and need to be updated before 3.8 can be supported:
flake8-docstringsflake8-import-orderflake8-tidy-importsThis needs validation, but I believe that the coverage report currently being provided by the CI is only for whatever test environment runs the tests last. While not an immediate issue for the current codebase, as Python 3.8-specific logic starts to be added to the codebase the coverage reported by a single test environment will decrease.
To remedy this, the coverage report should be generated from the combination of reports across all of the test environments.
This will likely be easie...
what if I just return the file name for the help command? I will not hard code the file link tho, I can get it by the code.
Can you confirm these are not already present/relatively up-to-date?
python -m pip install --upgrade pip
pip install setuptools wheel
Tangentially related to #43 and #47, and a bit of thinking out loud: it might be easier in the long run to reconfigure the Azure pipeline to utilize the tox configuration we’ve generating for local dev
Tox should be able to accomplish pretty much all of the same tasks, and it would make life a bit easier not having to synchronize the tox and azure configurations so the user isn’t getting surprised when they get to CI.
Postgres backup completed!
Thanks! for assigning it to me. I will start working on it.
Build 20191017.1 succeeded
GitHub
00:01:40
Bot
Python 3.7.4
pip 19.2.3 from /opt/hostedtoolcache/Python/3.7.4/x64/lib/python3.7/site-packages/pip (python 3.7)
Package Version
---------- -------
pip 19.2.3
setuptools 40.8.0
WARNING: You are using pip version 19.2.3, however version 19.3 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
No wheel, setuptools is from February. pip is relatively up to date. I don't see a problem with ensuring that these are up to date...
I'm not sure how to implement it quite the way you've shown without either violating DRY or adding another conditional:
if detected_pyfile or rejected_attachments:
# Send warning message and delete offending message, etc.
but I have implemented the re-ordering as you've requested to skip the one extra conditional evaluation in the cases where a .py file is attached. I hope I understood you correctly.
Build 20191017.2 succeeded
GitHub
00:01:35
Bot
That isn't what I meant. Sorry for not being clear. Pseudocode:
for msg in attachments:
if py_file:
detected_pyfile = True
break
if not whitelisted:
rejected_attachments = True
# Don't break! Since the py message takes priority,
# we should keep checking attachments to see if maybe
# a later attachment will be a py file.
if detected_pyfile:
print("Please use paste service..")
elif rejected_attachments:
pr...
Okay, good point, I'll get the second break out of there.
Build 20191017.3 succeeded
GitHub
00:01:38
Bot
Build 20191017.1 succeeded
GitHub
00:04:04
Site
The conditions for sending out the message still needs to be addressed.
Never mind actually, it's fine too. It's just a different way of doing what I had in mind and I didn't recognise that initially.
07fbef5 Remove flake8-type-annotations - sco1
dad0150 Remove flake8-type-annotations - sco1
Build 20191017.1 succeeded
GitHub
00:01:49
Django Simple Bulma
Build 20191017.2 succeeded
GitHub
00:01:47
Django Simple Bulma
Tested this locally and it works, even with multiple attachments (sent from phone). One concern is that moderators are not exempt from this functionality but I'll let them have the say on whether they should be.
b0f1f7e Add 3.8 to tox config, update tests for typed-a... - sco1
fa138e8 Add positional argument parsing - sco1
47700b0 Add version-specific parsing of src file into AST - sco1
5de947b Update tox configuration to combine coverage re... - sco1
da59f8c Fix incorrect type comment flag in checker init - sco1
8c2871f Make it easier for user to search for tags - ikuyarihS
Applying the algorithm for Needles and Haystack to find and match tag in tags, for example:

This only applies to searching tag_name with more than 3 in length, and at least 80% of its letters are found, from left to right.
There are 3 levels of checking, stop at first found:
Build 20191018.1 succeeded
GitHub
00:01:27
Bot
Postgres backup completed!
I've changed this logic for the UniqueTogetherValidator with a custom message. It works well, although it gives slightly less specific feedback:
'non_field_errors': ['This user already has an active infraction of this type.']
However, since this is generated for a specific infraction payload, that should not really be an issue and the message is unambiguous in the context of the query.
I've removed it in my latest commit (not pushed atm, but soon)
Yes, that can't really be helped unless we want to rollback, add data, apply the migration in each test case. That would increase the running time of the tests by a lot, though. This test is already on the heavy side (although we're talking one to two seconds in the pipeline, which is acceptable in my opinion).
I'll use sensible strings as keys to make it clearer, I think that's the best we can do here.
- The infractions tests don't strike me as very DRY, though many other tests for the site are probably also guilty of that. Would it be feasible to come up with some functions to make it more DRY, at least for the new infraction tests you've written?
(I still have to look into this.)
- While your tests are already thorough, I noticed that
MigrationsTestCasedoes not itself have any tests. Would it be feasible to come up with tests for that? Is it even worth it?
I'm not quite su...
Postgres backup completed!
I think an enum would be nicer if this is staying (see other comment).
Given that the action is always a PUT request, this may as well just be done here and take parameters for the days and enable/disable.
Perhaps then you could also get rid of action_type and deduce it from the aforementioned arguments. Going with the assumption that the days would never be updated to 0, could "updated" be when enabled = True and days = 0? An alternative is to keep action_type and not have an enabled parameter since I think it's easy to deduce that value from the ...
Build 20191019.1 succeeded
GitHub
00:01:51
Django Crispy Bulma
Build 20191019.1 succeeded
GitHub
00:01:34
Bot
That's some very good suggestions, now that I inspected the actions closer, it is very possible to move all the puts there, and to accept only days and the bool for `'enabled''. Thank you.
71b4084 Add a jump-url field in the reminder model - Akarys42
Build 20191019.2 succeeded
GitHub
00:01:36
Bot
Closes: #535
This PR add an extra check in_channel_check(ctx, *constants.MODERATION_CHANNELS) to cog_check. This conditon will pass only if the channel belongs to the list of moderation channels.
Build 20191019.3 failed
GitHub
00:01:21
Bot
Hi, I have raised a PR for this ticket https://github.com/python-discord/bot/pull/543
This is my first PR so I am not sure how to test this. I tried to do the local setup using docker, but its failing.
Hello and thank you for contributing, are you in the Discord server? We'll be very glad to help you get your local setup working, since manual testing is important for Discord bots.
Build 20191019.4 succeeded
GitHub
00:01:36
Bot
Since the project is growing, it would be nice to have an index of every object in the repo.It is compatible with Github Pages, so they can be enabled in one click, no extra configuration required.
I have just finished working on my docstring documentation generator handsdown and think that it might be useful for Discord Bot as well.
Thank you! Yes I am on the server, my username is atmishra#0512
Build 20191019.5 succeeded
GitHub
00:01:33
Bot
Build 20191019.6 succeeded
GitHub
00:01:31
Bot
Hi, python-discord/bot#466 requires to add a jump_url field to the reminder model, so here it is!
Here it is: python-discord/site#288
This PR add the jump_url field to the reminder model : python-discord/site#288. This PR need to be merged first.
Build 20191019.1 succeeded
GitHub
00:02:21
Site
Hello! This would be best started as a discussion in the issue we have open for updating the dependencies (#25), or even the PR we have open for updating the dependencies (#26).
Thanks!
I reverted a change here and I probably forgot to re-add the line break.
9938623 Pluralize "infractions" as necessary. - jchristgit
Build 20191019.7 succeeded
GitHub
00:01:30
Bot
9b38f42 Migrate nominations to new Nomination model - SebastiaanZ
1ced4fc Update the landing page text. - lemonsaurus
99bc330 Initial Allauth dependency and settings/urls - gdude2002
8cfff31 Remove accidental static files commit; fix Allauth - gdude2002
b83f1b2 Re-add erroneously removed static files - gdude2002
48a78de Do not display an expiry for notes or warnings. - jchristgit
Build 20191019.2 succeeded
GitHub
00:02:08
Site
Build 20191019.8 succeeded
GitHub
00:01:37
Bot
Connected!
Build 20191019.9 succeeded
GitHub
00:03:19
Bot
Build 20191019.10 failed
GitHub
00:01:20
Bot
sorry for taking so long, this looks great!
I can make the payload more dependent on the type, but it would mean a logically more complex test.
I forgot about this. I still need to fix this.
I've changed it to the proposed solution.
So you still ended up having to manually specify the validator. It did not generate it on its own?
This test name is confusing, perhaps unsurprisingly.
I don't really feel like this test is necessary to be honest.
This docstring seems to be missing a word.
It's necessary since we include tests in the coverage report and otherwise the except-block for the test we're testing here will never be run.
We may be able to declare an exception to the coverage requirement for those specific lines, but I wrote to test to make sure it actually works like I intended it to, since it does have custom test logic.
13083b3 Fix indentation and missing word in docstring - SebastiaanZ
Build 20191019.3 succeeded
GitHub
00:02:09
Site
Oh right. I am not used to coverage being enabled for tests. I still think the name could be better by mentioning that it is specifically testing the exception being caught, but the name is already getting quite long 🤔 .
As far as I can tell, it does not automatically add a validator for the constraint we've added to the model. If I remove our "custom" validator, the serializer doesn't seem to catch double active infractions and we'll get an IntegrityError (fail late in the database) instead.
Maybe not that much more complex. I'm wondering how it'd look if the expiration and hidden args could be put in the tuple alongside the types.
That's fine then. I looked at the source and it seems to only work if unique_together is used.