From 86a2b550b980361a99314632c3e6e0a112a0d43d Mon Sep 17 00:00:00 2001 From: sco1 Date: Mon, 7 Jan 2019 19:15:22 -0500 Subject: Add role hierarchy comparison check method --- bot/cogs/moderation.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index ac08d3dd4..a15a1b71c 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -1207,6 +1207,19 @@ class Moderation(Scheduler): if User in error.converters: await ctx.send(str(error.errors[0])) + async def respect_role_hierarchy(self, guild: Guild, actor: Member, target: Member) -> bool: + """ + Check if the highest role of the invoking member is less than or equal to the target member + + Implement as a method rather than a check in order to avoid having to reimplement parameter + checks & conversions in a dedicated check decorater + """ + + # Build role hierarchy + role_hierarchy = {role: rank for rank, role in enumerate(reversed(guild.roles))} + + return role_hierarchy[actor.top_role] <= role_hierarchy[target.top_role] + def setup(bot): bot.add_cog(Moderation(bot)) -- cgit v1.2.3 From 646a892348af4cf99fe2b08d6a1688e0aff56d3b Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 9 Jan 2019 16:40:54 -0500 Subject: Add hierarchy check to desired infractions Per #134, only implement for all variations of the ban & kick infractions Update helper method inputs to facilitate a generic logging & context message --- bot/cogs/moderation.py | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index a15a1b71c..057b51bbf 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -125,6 +125,11 @@ class Moderation(Scheduler): :param reason: The reason for the kick. """ + if not self.respect_role_hierarchy(ctx, user, 'kick'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + notified = await self.notify_infraction( user=user, infr_type="Kick", @@ -171,6 +176,11 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ + if not self.respect_role_hierarchy(ctx, user, 'ban'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + notified = await self.notify_infraction( user=user, infr_type="Ban", @@ -327,6 +337,11 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ + if not self.respect_role_hierarchy(ctx, user, 'tempban'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + notified = await self.notify_infraction( user=user, infr_type="Ban", @@ -420,6 +435,11 @@ class Moderation(Scheduler): :param reason: The reason for the kick. """ + if not self.respect_role_hierarchy(ctx, user, 'shadowkick'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + response_object = await post_infraction(ctx, user, type="kick", reason=reason, hidden=True) if response_object is None: return @@ -456,6 +476,11 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ + if not self.respect_role_hierarchy(ctx, user, 'shadowban'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + response_object = await post_infraction(ctx, user, type="ban", reason=reason, hidden=True) if response_object is None: return @@ -581,6 +606,11 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ + if not self.respect_role_hierarchy(ctx, user, 'shadowtempban'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + response_object = await post_infraction(ctx, user, type="ban", reason=reason, duration=duration, hidden=True) if response_object is None: return @@ -1207,18 +1237,29 @@ class Moderation(Scheduler): if User in error.converters: await ctx.send(str(error.errors[0])) - async def respect_role_hierarchy(self, guild: Guild, actor: Member, target: Member) -> bool: + async def respect_role_hierarchy(self, ctx: Context, target: Member, infraction_type: str) -> bool: """ Check if the highest role of the invoking member is less than or equal to the target member + If this check fails, a warning is sent to the invoking ctx + Implement as a method rather than a check in order to avoid having to reimplement parameter checks & conversions in a dedicated check decorater """ # Build role hierarchy - role_hierarchy = {role: rank for rank, role in enumerate(reversed(guild.roles))} + actor = ctx.author + role_hierarchy = {role: rank for rank, role in enumerate(reversed(ctx.guild.roles))} + hierarchy_check = role_hierarchy[actor.top_role] <= role_hierarchy[target.top_role] + + if not hierarchy_check: + log.info( + f"{actor.display_name} ({actor.id}) attempted to {infraction_type} " + f"{target.display_name} ({target.id}), who has a higher top role" + ) + ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") - return role_hierarchy[actor.top_role] <= role_hierarchy[target.top_role] + return hierarchy_check def setup(bot): -- cgit v1.2.3 From 5dd0d53e3ee468c13e44880de8fbd2e733f54667 Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 9 Jan 2019 16:53:49 -0500 Subject: Add missing awaits --- bot/cogs/moderation.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index 057b51bbf..1db7df3dd 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -125,7 +125,7 @@ class Moderation(Scheduler): :param reason: The reason for the kick. """ - if not self.respect_role_hierarchy(ctx, user, 'kick'): + if not await self.respect_role_hierarchy(ctx, user, 'kick'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -176,7 +176,7 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ - if not self.respect_role_hierarchy(ctx, user, 'ban'): + if not await self.respect_role_hierarchy(ctx, user, 'ban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -337,7 +337,7 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ - if not self.respect_role_hierarchy(ctx, user, 'tempban'): + if not await self.respect_role_hierarchy(ctx, user, 'tempban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -435,7 +435,7 @@ class Moderation(Scheduler): :param reason: The reason for the kick. """ - if not self.respect_role_hierarchy(ctx, user, 'shadowkick'): + if not await self.respect_role_hierarchy(ctx, user, 'shadowkick'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -476,7 +476,7 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ - if not self.respect_role_hierarchy(ctx, user, 'shadowban'): + if not await self.respect_role_hierarchy(ctx, user, 'shadowban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -606,7 +606,7 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ - if not self.respect_role_hierarchy(ctx, user, 'shadowtempban'): + if not await self.respect_role_hierarchy(ctx, user, 'shadowtempban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -1257,7 +1257,7 @@ class Moderation(Scheduler): f"{actor.display_name} ({actor.id}) attempted to {infraction_type} " f"{target.display_name} ({target.id}), who has a higher top role" ) - ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") + await ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") return hierarchy_check -- cgit v1.2.3 From d25ac2c3fc1d83a0022f456ee2189507e249e35a Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 9 Jan 2019 16:59:43 -0500 Subject: Get member object for ban infractions to pass to hierarchy check --- bot/cogs/moderation.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index 1db7df3dd..46c10e084 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -176,7 +176,8 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ - if not await self.respect_role_hierarchy(ctx, user, 'ban'): + member = ctx.guild.get_member(user.id) + if not await self.respect_role_hierarchy(ctx, member, 'ban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -337,7 +338,8 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ - if not await self.respect_role_hierarchy(ctx, user, 'tempban'): + member = ctx.guild.get_member(user.id) + if not await self.respect_role_hierarchy(ctx, member, 'tempban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -476,7 +478,8 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ - if not await self.respect_role_hierarchy(ctx, user, 'shadowban'): + member = ctx.guild.get_member(user.id) + if not await self.respect_role_hierarchy(ctx, member, 'shadowban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -606,7 +609,8 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ - if not await self.respect_role_hierarchy(ctx, user, 'shadowtempban'): + member = ctx.guild.get_member(user.id) + if not await self.respect_role_hierarchy(ctx, member, 'shadowtempban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return -- cgit v1.2.3 From 3ed3e2e96ef9e9ab827e331d796f9549bc63455c Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 9 Jan 2019 17:09:57 -0500 Subject: Change logging statement to use author instead of username --- bot/cogs/moderation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index 46c10e084..d9c217b83 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -1258,8 +1258,8 @@ class Moderation(Scheduler): if not hierarchy_check: log.info( - f"{actor.display_name} ({actor.id}) attempted to {infraction_type} " - f"{target.display_name} ({target.id}), who has a higher top role" + f"{actor} ({actor.id}) attempted to {infraction_type} " + f"{target} ({target.id}), who has a higher top role" ) await ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") -- cgit v1.2.3 From d3ec570c49a9b5b4e93a994e63a56b6ee0f17127 Mon Sep 17 00:00:00 2001 From: sco1 Date: Thu, 10 Jan 2019 23:42:47 -0500 Subject: Initial rework of Contributor guidelines (not yet proofread) Add guidance on commit scope & frequency Add guidance on commit messages Beat people over the head with flake8 & precommit hooks Add logging level definition Remove the random linebreaks Minor wordsmithing, formatting changes --- CONTRIBUTING.md | 62 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 36152fc5d..f0371f9f5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,41 +1,45 @@ -# Contributing to one of our projects +# Contributing to one of our projects1 -Our projects are open-source, and are deployed as commits are pushed to the `master` branch on each repository. -We've created a set of guidelines here in order to keep everything clean and in working order. Please note that -contributions may be rejected on the basis of a contributor failing to follow the guidelines. +Our projects are open-source and are deployed as commits are pushed to the `master` branch on each repository, so we've created a set of guidelines in order to keep everything clean and in working order. + +Note that contributions may be rejected on the basis of a contributor failing to follow these guidelines. ## Rules 1. **No force-pushes** or modifying the Git history in any way. -1. If you have direct access to the repository, **create a branch for your changes** and create a merge request for that branch. - If not, fork it and work on a separate branch there. - * Some repositories require this and will reject any direct pushes to `master`. Make this a habit! -1. If someone is working on a merge request, **do not open your own merge request for the same task**. Instead, leave some comments - on the existing merge request. Communication is key, and there's no point in two separate implementations of the same thing. - * One option is to fork the other contributor's repository, and submit your changes to their branch with your - own merge request. If you do this, we suggest following these guidelines when interacting with their repository - as well. +1. If you have direct access to the repository, **create a branch for your changes** and create a merge request for that branch. If not, create a branch on a fork of the repository and create a merge request from there. + * It's common practice for a repository to reject direct pushes to `master`, so make branching a habit! +1. **Make great commits**2. A well structured git log is key to a project's maintainability; it efficiently provides insight into when and *why* things were done for future maintainers of the project. + * Commits should be as narrow in scope as possible. Commits that span hundreds of lines across multiple unrelated functions and/or files are very hard for maintainers to follow. After about a week they'll probably be hard for you to follow too. + * Commit messages should succintly *what* and *why* the changes were made. + * Try to avoid making minor commits for fixing typos or linting errors. Since you've already set up a pre-commit hook to run `flake8` before a commit, you shouldn't be committing linting issues anyway. +1. **Avoid frequent pushes to the main repository**. Our test build pipelines are triggered every time a push to the repository is made. Where possible, try to batch your commits until you've finished working for that session, or collaborators need your commits to continue their work. This also provides you the opportunity to amend commits for minor changes rather than having to commit them on their own because you've already pushed. +1. If someone is working on a merge request, **do not open your own merge request for the same task**. Instead, collaborate with the author(s) of the existing merge request. Communication is key, and there's no point in two separate implementations of the same thing. + * One option is to fork the other contributor's repository and submit your changes to their branch with your own merge request. We suggest following these guidelines when interacting with their repository as well. 1. **Adhere to the prevailing code style**, which we enforce using [flake8](http://flake8.pycqa.org/en/latest/index.html). - * Additionally, run `flake8` against your code before you push it. Your commit will be rejected by the build server - if it fails to lint. -1. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully - chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the - work has already been done for you! -1. **Work as a team** and cooperate where possible. Keep things friendly, and help each other out - these are shared - projects, and nobody likes to have their feet trodden on. -1. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server - does not. With this trust comes responsibility - do not release any information you have learned as a result of - your contributor position. We are very strict about announcing things at specific times, and many staff members - will not appreciate a disruption of the announcement schedule. - -Above all, the needs of our community should come before the wants of an individual. Work together, build solutions to -problems and try to do so in a way that people can learn from easily. Abuse of our trust may result in the loss of your Contributor role, especially in relation to Rule 7. + * Run `flake8` against your code **before** you push it. Your commit will be rejected by the build server if it fails to lint. + * [Git Hooks](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks) are a powerful tool that can be a daunting to set up. Fortunately, [`pre-commit`](https://github.com/pre-commit/pre-commit) abstracts this process away from you and is provided as a dev dependency for this project. Run `pipenv run precommit` when you set up the project and you'll never have to worry about breaking the build for linting errors. +1. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the work has already been done for you! +1. **Work as a team** and collaborate whereever possible. Keep things friendly and help each other out - these are shared projects and nobody likes to have their feet trodden on. +1. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server does not. With this trust comes responsibility - do not release any information you have learned as a result of your contributor position. We are very strict about announcing things at specific times, and many staff members will not appreciate a disruption of the announcement schedule. + +Above all, the needs of our community should come before the wants of an individual. Work together, build solutions to problems and try to do so in a way that people can learn from easily. Abuse of our trust may result in the loss of your Contributor role, especially in relation to Rule 7. ## Changes to this arrangement -All projects evolve over time, and this contribution guide is no different. This document may also be subject to pull -requests or changes by contributors, where you believe you have something valuable to add or change. +All projects evolve over time, and this contribution guide is no different. This document may also be subject to pull requests or changes by contributors, where you believe you have something valuable to add or change. + +## Supplemental Information +### Logging levels +The project currently defines [`logging`] levels as follows: +* **TRACE:** Use this for tracing every step of a complex process. That way we can see which step of the process failed. Err on the side of verbose. +* **INFO:** Something completely ordinary happened. Like a cog loading during startup. +* **DEBUG:** Someone is interacting with the application, and the application is behaving as expected. +* **WARNING:** Someone is interacting with the application in an unexpected way or the application is responding in an unexpected way, but without causing an error. +* **ERROR:** An error that affects the specific part that is being interacted with +* **CRITICAL:** An error that affects the whole application. ## Footnotes -This document was inspired by the [Glowstone contribution guidelines](https://github.com/GlowstoneMC/Glowstone/blob/dev/docs/CONTRIBUTING.md). +1. This document was inspired by the [Glowstone contribution guidelines](https://github.com/GlowstoneMC/Glowstone/blob/dev/docs/CONTRIBUTING.md). +2. A more in-depth guide to writing great commit messages can be found in Chris Beam's [*How to Write a Git Commit Message*](https://chris.beams.io/posts/git-commit/) \ No newline at end of file -- cgit v1.2.3 From 15c749cc3bd33bf548d57f2cf0676dbf1bf25195 Mon Sep 17 00:00:00 2001 From: sco1 Date: Fri, 11 Jan 2019 16:39:57 -0500 Subject: Proofreading pass on Contributor doc --- CONTRIBUTING.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f0371f9f5..4cc8883d6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,4 +1,4 @@ -# Contributing to one of our projects1 +# Contributing to one of our projects Our projects are open-source and are deployed as commits are pushed to the `master` branch on each repository, so we've created a set of guidelines in order to keep everything clean and in working order. @@ -9,17 +9,18 @@ Note that contributions may be rejected on the basis of a contributor failing to 1. **No force-pushes** or modifying the Git history in any way. 1. If you have direct access to the repository, **create a branch for your changes** and create a merge request for that branch. If not, create a branch on a fork of the repository and create a merge request from there. * It's common practice for a repository to reject direct pushes to `master`, so make branching a habit! -1. **Make great commits**2. A well structured git log is key to a project's maintainability; it efficiently provides insight into when and *why* things were done for future maintainers of the project. +1. **Adhere to the prevailing code style**, which we enforce using [flake8](http://flake8.pycqa.org/en/latest/index.html). + * Run `flake8` against your code **before** you push it. Your commit will be rejected by the build server if it fails to lint. + * [Git Hooks](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks) are a powerful tool that can be a daunting to set up. Fortunately, [`pre-commit`](https://github.com/pre-commit/pre-commit) abstracts this process away from you and is provided as a dev dependency for this project. Run `pipenv run precommit` when setting up the project and you'll never have to worry about breaking the build for linting errors. +1. **Make great commits**. A well structured git log is key to a project's maintainability; it efficiently provides insight into when and *why* things were done for future maintainers of the project. * Commits should be as narrow in scope as possible. Commits that span hundreds of lines across multiple unrelated functions and/or files are very hard for maintainers to follow. After about a week they'll probably be hard for you to follow too. - * Commit messages should succintly *what* and *why* the changes were made. * Try to avoid making minor commits for fixing typos or linting errors. Since you've already set up a pre-commit hook to run `flake8` before a commit, you shouldn't be committing linting issues anyway. -1. **Avoid frequent pushes to the main repository**. Our test build pipelines are triggered every time a push to the repository is made. Where possible, try to batch your commits until you've finished working for that session, or collaborators need your commits to continue their work. This also provides you the opportunity to amend commits for minor changes rather than having to commit them on their own because you've already pushed. + * A more in-depth guide to writing great commit messages can be found in Chris Beam's [*How to Write a Git Commit Message*](https://chris.beams.io/posts/git-commit/) +1. **Avoid frequent pushes to the main repository**. This goes for PRs opened against your fork as well. Our test build pipelines are triggered every time a push to the repository (or PR) is made. Try to batch your commits until you've finished working for that session, or you've reached a point where collaborators need your commits to continue their own work. This also provides you the opportunity to amend commits for minor changes rather than having to commit them on their own because you've already pushed. + * This includes merging master into your branch. Try to leave merging from master for after your PR passes review; a maintainer will bring your PR up to date before merging. Exceptions to this include: resolving merge conflicts, needing something that was pushed to master for your branch, or something was pushed to master that could potentionally affect the functionality of what you're writing. +1. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the work has already been done for you! 1. If someone is working on a merge request, **do not open your own merge request for the same task**. Instead, collaborate with the author(s) of the existing merge request. Communication is key, and there's no point in two separate implementations of the same thing. * One option is to fork the other contributor's repository and submit your changes to their branch with your own merge request. We suggest following these guidelines when interacting with their repository as well. -1. **Adhere to the prevailing code style**, which we enforce using [flake8](http://flake8.pycqa.org/en/latest/index.html). - * Run `flake8` against your code **before** you push it. Your commit will be rejected by the build server if it fails to lint. - * [Git Hooks](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks) are a powerful tool that can be a daunting to set up. Fortunately, [`pre-commit`](https://github.com/pre-commit/pre-commit) abstracts this process away from you and is provided as a dev dependency for this project. Run `pipenv run precommit` when you set up the project and you'll never have to worry about breaking the build for linting errors. -1. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the work has already been done for you! 1. **Work as a team** and collaborate whereever possible. Keep things friendly and help each other out - these are shared projects and nobody likes to have their feet trodden on. 1. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server does not. With this trust comes responsibility - do not release any information you have learned as a result of your contributor position. We are very strict about announcing things at specific times, and many staff members will not appreciate a disruption of the announcement schedule. @@ -27,11 +28,11 @@ Above all, the needs of our community should come before the wants of an individ ## Changes to this arrangement -All projects evolve over time, and this contribution guide is no different. This document may also be subject to pull requests or changes by contributors, where you believe you have something valuable to add or change. +All projects evolve over time, and this contribution guide is no different. This document is open to pull requests or changes by contributors. If you believe you have something valuable to add or change, please don't hesitate to do so in a PR. ## Supplemental Information ### Logging levels -The project currently defines [`logging`] levels as follows: +The project currently defines [`logging`](https://docs.python.org/3/library/logging.html) levels as follows: * **TRACE:** Use this for tracing every step of a complex process. That way we can see which step of the process failed. Err on the side of verbose. * **INFO:** Something completely ordinary happened. Like a cog loading during startup. * **DEBUG:** Someone is interacting with the application, and the application is behaving as expected. @@ -41,5 +42,4 @@ The project currently defines [`logging`] levels as follows: ## Footnotes -1. This document was inspired by the [Glowstone contribution guidelines](https://github.com/GlowstoneMC/Glowstone/blob/dev/docs/CONTRIBUTING.md). -2. A more in-depth guide to writing great commit messages can be found in Chris Beam's [*How to Write a Git Commit Message*](https://chris.beams.io/posts/git-commit/) \ No newline at end of file +This document was inspired by the [Glowstone contribution guidelines](https://github.com/GlowstoneMC/Glowstone/blob/dev/docs/CONTRIBUTING.md). \ No newline at end of file -- cgit v1.2.3 From 6f2f726885f92ce757cc7b604a209048c1da10e0 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 12 Jan 2019 13:24:12 -0500 Subject: Update sentence flow for clarity and to prevent dizziness Co-Authored-By: sco1 --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4cc8883d6..45f97a7fd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,6 +1,6 @@ # Contributing to one of our projects -Our projects are open-source and are deployed as commits are pushed to the `master` branch on each repository, so we've created a set of guidelines in order to keep everything clean and in working order. +Our projects are open-source and are automatically deployed whenever commits are pushed to the `master` branch on each repository, so we've created a set of guidelines in order to keep everything clean and in working order. Note that contributions may be rejected on the basis of a contributor failing to follow these guidelines. @@ -42,4 +42,4 @@ The project currently defines [`logging`](https://docs.python.org/3/library/logg ## Footnotes -This document was inspired by the [Glowstone contribution guidelines](https://github.com/GlowstoneMC/Glowstone/blob/dev/docs/CONTRIBUTING.md). \ No newline at end of file +This document was inspired by the [Glowstone contribution guidelines](https://github.com/GlowstoneMC/Glowstone/blob/dev/docs/CONTRIBUTING.md). -- cgit v1.2.3 From 702a5908edfa1cf16ab6f01a7fe6189b7e83d72b Mon Sep 17 00:00:00 2001 From: sco1 Date: Sat, 12 Jan 2019 13:25:32 -0500 Subject: Clarify trace logging level as PyDis-implemented --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 45f97a7fd..ae7afd8f7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -33,7 +33,7 @@ All projects evolve over time, and this contribution guide is no different. This ## Supplemental Information ### Logging levels The project currently defines [`logging`](https://docs.python.org/3/library/logging.html) levels as follows: -* **TRACE:** Use this for tracing every step of a complex process. That way we can see which step of the process failed. Err on the side of verbose. +* **TRACE:** Use this for tracing every step of a complex process. That way we can see which step of the process failed. Err on the side of verbose. **Note:** This is a PyDis-implemented logging level. * **INFO:** Something completely ordinary happened. Like a cog loading during startup. * **DEBUG:** Someone is interacting with the application, and the application is behaving as expected. * **WARNING:** Someone is interacting with the application in an unexpected way or the application is responding in an unexpected way, but without causing an error. -- cgit v1.2.3 From f5823b0c3c8c6265857254e305a788e006158f0e Mon Sep 17 00:00:00 2001 From: sco1 Date: Sun, 13 Jan 2019 11:41:25 -0500 Subject: Enumerate items Markdown is made for humans to read and only coincidentally for computers to render. If it's supposed to be an unordered list, use - or *. If it's supposed to be ordered, enumerate it. --- CONTRIBUTING.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ae7afd8f7..aba61959f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,22 +7,22 @@ Note that contributions may be rejected on the basis of a contributor failing to ## Rules 1. **No force-pushes** or modifying the Git history in any way. -1. If you have direct access to the repository, **create a branch for your changes** and create a merge request for that branch. If not, create a branch on a fork of the repository and create a merge request from there. +3. If you have direct access to the repository, **create a branch for your changes** and create a merge request for that branch. If not, create a branch on a fork of the repository and create a merge request from there. * It's common practice for a repository to reject direct pushes to `master`, so make branching a habit! -1. **Adhere to the prevailing code style**, which we enforce using [flake8](http://flake8.pycqa.org/en/latest/index.html). +5. **Adhere to the prevailing code style**, which we enforce using [flake8](http://flake8.pycqa.org/en/latest/index.html). * Run `flake8` against your code **before** you push it. Your commit will be rejected by the build server if it fails to lint. * [Git Hooks](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks) are a powerful tool that can be a daunting to set up. Fortunately, [`pre-commit`](https://github.com/pre-commit/pre-commit) abstracts this process away from you and is provided as a dev dependency for this project. Run `pipenv run precommit` when setting up the project and you'll never have to worry about breaking the build for linting errors. -1. **Make great commits**. A well structured git log is key to a project's maintainability; it efficiently provides insight into when and *why* things were done for future maintainers of the project. +7. **Make great commits**. A well structured git log is key to a project's maintainability; it efficiently provides insight into when and *why* things were done for future maintainers of the project. * Commits should be as narrow in scope as possible. Commits that span hundreds of lines across multiple unrelated functions and/or files are very hard for maintainers to follow. After about a week they'll probably be hard for you to follow too. * Try to avoid making minor commits for fixing typos or linting errors. Since you've already set up a pre-commit hook to run `flake8` before a commit, you shouldn't be committing linting issues anyway. * A more in-depth guide to writing great commit messages can be found in Chris Beam's [*How to Write a Git Commit Message*](https://chris.beams.io/posts/git-commit/) -1. **Avoid frequent pushes to the main repository**. This goes for PRs opened against your fork as well. Our test build pipelines are triggered every time a push to the repository (or PR) is made. Try to batch your commits until you've finished working for that session, or you've reached a point where collaborators need your commits to continue their own work. This also provides you the opportunity to amend commits for minor changes rather than having to commit them on their own because you've already pushed. +9. **Avoid frequent pushes to the main repository**. This goes for PRs opened against your fork as well. Our test build pipelines are triggered every time a push to the repository (or PR) is made. Try to batch your commits until you've finished working for that session, or you've reached a point where collaborators need your commits to continue their own work. This also provides you the opportunity to amend commits for minor changes rather than having to commit them on their own because you've already pushed. * This includes merging master into your branch. Try to leave merging from master for after your PR passes review; a maintainer will bring your PR up to date before merging. Exceptions to this include: resolving merge conflicts, needing something that was pushed to master for your branch, or something was pushed to master that could potentionally affect the functionality of what you're writing. -1. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the work has already been done for you! -1. If someone is working on a merge request, **do not open your own merge request for the same task**. Instead, collaborate with the author(s) of the existing merge request. Communication is key, and there's no point in two separate implementations of the same thing. +11. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the work has already been done for you! +13. If someone is working on a merge request, **do not open your own merge request for the same task**. Instead, collaborate with the author(s) of the existing merge request. Communication is key, and there's no point in two separate implementations of the same thing. * One option is to fork the other contributor's repository and submit your changes to their branch with your own merge request. We suggest following these guidelines when interacting with their repository as well. -1. **Work as a team** and collaborate whereever possible. Keep things friendly and help each other out - these are shared projects and nobody likes to have their feet trodden on. -1. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server does not. With this trust comes responsibility - do not release any information you have learned as a result of your contributor position. We are very strict about announcing things at specific times, and many staff members will not appreciate a disruption of the announcement schedule. +15. **Work as a team** and collaborate whereever possible. Keep things friendly and help each other out - these are shared projects and nobody likes to have their feet trodden on. +17. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server does not. With this trust comes responsibility - do not release any information you have learned as a result of your contributor position. We are very strict about announcing things at specific times, and many staff members will not appreciate a disruption of the announcement schedule. Above all, the needs of our community should come before the wants of an individual. Work together, build solutions to problems and try to do so in a way that people can learn from easily. Abuse of our trust may result in the loss of your Contributor role, especially in relation to Rule 7. -- cgit v1.2.3 From d2efc786b4cb7c38a38930f538e576af5ff840da Mon Sep 17 00:00:00 2001 From: sco1 Date: Sun, 13 Jan 2019 11:43:08 -0500 Subject: Change MR to PR, reorder log levels --- CONTRIBUTING.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aba61959f..24376905b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,7 +7,7 @@ Note that contributions may be rejected on the basis of a contributor failing to ## Rules 1. **No force-pushes** or modifying the Git history in any way. -3. If you have direct access to the repository, **create a branch for your changes** and create a merge request for that branch. If not, create a branch on a fork of the repository and create a merge request from there. +3. If you have direct access to the repository, **create a branch for your changes** and create a pull request for that branch. If not, create a branch on a fork of the repository and create a pull request from there. * It's common practice for a repository to reject direct pushes to `master`, so make branching a habit! 5. **Adhere to the prevailing code style**, which we enforce using [flake8](http://flake8.pycqa.org/en/latest/index.html). * Run `flake8` against your code **before** you push it. Your commit will be rejected by the build server if it fails to lint. @@ -19,8 +19,8 @@ Note that contributions may be rejected on the basis of a contributor failing to 9. **Avoid frequent pushes to the main repository**. This goes for PRs opened against your fork as well. Our test build pipelines are triggered every time a push to the repository (or PR) is made. Try to batch your commits until you've finished working for that session, or you've reached a point where collaborators need your commits to continue their own work. This also provides you the opportunity to amend commits for minor changes rather than having to commit them on their own because you've already pushed. * This includes merging master into your branch. Try to leave merging from master for after your PR passes review; a maintainer will bring your PR up to date before merging. Exceptions to this include: resolving merge conflicts, needing something that was pushed to master for your branch, or something was pushed to master that could potentionally affect the functionality of what you're writing. 11. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the work has already been done for you! -13. If someone is working on a merge request, **do not open your own merge request for the same task**. Instead, collaborate with the author(s) of the existing merge request. Communication is key, and there's no point in two separate implementations of the same thing. - * One option is to fork the other contributor's repository and submit your changes to their branch with your own merge request. We suggest following these guidelines when interacting with their repository as well. +13. If someone is working on a pull request, **do not open your own pull request for the same task**. Instead, collaborate with the author(s) of the existing pull request. Communication is key, and there's no point in two separate implementations of the same thing. + * One option is to fork the other contributor's repository and submit your changes to their branch with your own pull request. We suggest following these guidelines when interacting with their repository as well. 15. **Work as a team** and collaborate whereever possible. Keep things friendly and help each other out - these are shared projects and nobody likes to have their feet trodden on. 17. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server does not. With this trust comes responsibility - do not release any information you have learned as a result of your contributor position. We are very strict about announcing things at specific times, and many staff members will not appreciate a disruption of the announcement schedule. @@ -34,8 +34,8 @@ All projects evolve over time, and this contribution guide is no different. This ### Logging levels The project currently defines [`logging`](https://docs.python.org/3/library/logging.html) levels as follows: * **TRACE:** Use this for tracing every step of a complex process. That way we can see which step of the process failed. Err on the side of verbose. **Note:** This is a PyDis-implemented logging level. -* **INFO:** Something completely ordinary happened. Like a cog loading during startup. * **DEBUG:** Someone is interacting with the application, and the application is behaving as expected. +* **INFO:** Something completely ordinary happened. Like a cog loading during startup. * **WARNING:** Someone is interacting with the application in an unexpected way or the application is responding in an unexpected way, but without causing an error. * **ERROR:** An error that affects the specific part that is being interacted with * **CRITICAL:** An error that affects the whole application. -- cgit v1.2.3 From e83b86cd06595c4f1cea8c2ce2a2e2535b33505d Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 14 Jan 2019 12:36:28 +0100 Subject: Separating the potential helper watch and bb watch by adding a separate nominate-command --- bot/cogs/alias.py | 2 +- bot/cogs/bigbrother.py | 165 +++++++++++++++++++++++++++++++++++++------------ bot/constants.py | 1 + config-default.yml | 2 + 4 files changed, 128 insertions(+), 42 deletions(-) diff --git a/bot/cogs/alias.py b/bot/cogs/alias.py index 0b848c773..0e6b3a7c6 100644 --- a/bot/cogs/alias.py +++ b/bot/cogs/alias.py @@ -71,7 +71,7 @@ class Alias: @command(name="watch", hidden=True) async def bigbrother_watch_alias( - self, ctx, user: User, *, reason: str + self, ctx: Context, user: User, *, reason: str ): """ Alias for invoking bigbrother watch user [text_channel]. diff --git a/bot/cogs/bigbrother.py b/bot/cogs/bigbrother.py index 70916cd7b..289bc47aa 100644 --- a/bot/cogs/bigbrother.py +++ b/bot/cogs/bigbrother.py @@ -3,23 +3,30 @@ import logging import re from collections import defaultdict, deque from time import strptime, struct_time -from typing import List, Union +from typing import List, NamedTuple, Optional, Union from aiohttp import ClientError from discord import Color, Embed, Guild, Member, Message, TextChannel, User -from discord.ext.commands import Bot, Context, group +from discord.ext.commands import Bot, Context, command, group from bot.constants import BigBrother as BigBrotherConfig, Channels, Emojis, Guild as GuildConfig, Keys, Roles, URLs from bot.decorators import with_role from bot.pagination import LinePaginator from bot.utils import messages from bot.utils.moderation import post_infraction +from bot.utils.time import parse_rfc1123, time_since log = logging.getLogger(__name__) URL_RE = re.compile(r"(https?://[^\s]+)") +class WatchInformation(NamedTuple): + reason: str + actor_id: Optional[int] + inserted_at: Optional[str] + + class BigBrother: """User monitoring to assist with moderation.""" @@ -66,7 +73,7 @@ class BigBrother: data = await response.json() self.update_cache(data) - async def get_watch_reason(self, user_id: int) -> str: + async def get_watch_information(self, user_id: int) -> WatchInformation: """ Fetches and returns the latest watch reason for a user using the infraction API """ re_bb_watch = rf"^{self.infraction_watch_prefix}" @@ -84,16 +91,26 @@ class BigBrother: infraction_list = await response.json() except ClientError: log.exception(f"Failed to retrieve bb watch reason for {user_id}.") - return "(error retrieving bb reason)" + return WatchInformation(reason="(error retrieving bb reason)", actor_id=None, inserted_at=None) if infraction_list: + # Get the latest watch reason latest_reason_infraction = max(infraction_list, key=self._parse_infraction_time) + + # Get the actor of the watch/nominate action + actor_id = int(latest_reason_infraction["actor"]["user_id"]) + + # Get the date the watch was set + date = latest_reason_infraction["inserted_at"] + + # Get the latest reason without the prefix latest_reason = latest_reason_infraction['reason'][len(self.infraction_watch_prefix):] + log.trace(f"The latest bb watch reason for {user_id}: {latest_reason}") - return latest_reason + return WatchInformation(reason=latest_reason, actor_id=actor_id, inserted_at=date) - log.trace(f"No bb watch reason found for {user_id}; returning default string") - return "(no reason specified)" + log.trace(f"No bb watch reason found for {user_id}; returning defaults") + return WatchInformation(reason="(no reason specified)", actor_id=None, inserted_at=None) @staticmethod def _parse_infraction_time(infraction: str) -> struct_time: @@ -182,15 +199,38 @@ class BigBrother: if message.author.id != last_user or message.channel.id != last_channel or msg_count > limit: # Retrieve watch reason from API if it's not already in the cache if message.author.id not in self.watch_reasons: - log.trace(f"No watch reason for {message.author.id} found in cache; retrieving from API") - user_watch_reason = await self.get_watch_reason(message.author.id) - self.watch_reasons[message.author.id] = user_watch_reason + log.trace(f"No watch information for {message.author.id} found in cache; retrieving from API") + user_watch_information = await self.get_watch_information(message.author.id) + self.watch_reasons[message.author.id] = user_watch_information self.last_log = [message.author.id, message.channel.id, 0] + # Get reason, actor, inserted_at + reason, actor_id, inserted_at = self.watch_reasons[message.author.id] + + # Setting up the default author_field + author_field = message.author.nick or message.author.name + + # When we're dealing with a talent-pool header, add nomination info to the author field + if destination == self.bot.get_channel(Channels.talent_pool): + log.trace("We're sending a header to the talent-pool; let's add nomination info") + # If a reason was provided, both should be known + if actor_id and inserted_at: + # Parse actor name + guild: GuildConfig = self.bot.get_guild(GuildConfig.id) + actor_as_member = guild.get_member(actor_id) + actor = actor_as_member.nick or actor_as_member.name + + # Get time delta since insertion + date_time = parse_rfc1123(inserted_at).replace(tzinfo=None) + time_delta = time_since(date_time, precision="minutes", max_units=1) + + # Adding nomination info to author_field + author_field = f"{author_field} (nominated {time_delta} by {actor})" + embed = Embed(description=f"{message.author.mention} in [#{message.channel.name}]({message.jump_url})") - embed.set_author(name=message.author.nick or message.author.name, icon_url=message.author.avatar_url) - embed.set_footer(text=f"Watch reason: {self.watch_reasons[message.author.id]}") + embed.set_author(name=author_field, icon_url=message.author.avatar_url) + embed.set_footer(text=f"Reason: {reason}") await destination.send(embed=embed) @staticmethod @@ -217,6 +257,39 @@ class BigBrother: await messages.send_attachments(message, destination) + async def _watch_user(self, ctx: Context, user: User, reason: str, channel_id: int): + post_data = { + 'user_id': str(user.id), + 'channel_id': str(channel_id) + } + + async with self.bot.http_session.post( + URLs.site_bigbrother_api, + headers=self.HEADERS, + json=post_data + ) as response: + if response.status == 204: + if channel_id == Channels.talent_pool: + await ctx.send(f":ok_hand: added {user} to the <#{channel_id}>!") + else: + await ctx.send(f":ok_hand: will now relay messages sent by {user} in <#{channel_id}>") + + channel = self.bot.get_channel(channel_id) + if channel is None: + log.error( + f"could not update internal cache, failed to find a channel with ID {channel_id}" + ) + else: + self.watched_users[user.id] = channel + + # Add a note (shadow warning) with the reason for watching + reason = f"{self.infraction_watch_prefix}{reason}" + await post_infraction(ctx, user, type="warning", reason=reason, hidden=True) + else: + data = await response.json() + error_reason = data.get('error_message', "no message provided") + await ctx.send(f":x: the API returned an error: {error_reason}") + @group(name='bigbrother', aliases=('bb',), invoke_without_command=True) @with_role(Roles.owner, Roles.admin, Roles.moderator) async def bigbrother_group(self, ctx: Context): @@ -274,35 +347,7 @@ class BigBrother: channel_id = Channels.big_brother_logs - post_data = { - 'user_id': str(user.id), - 'channel_id': str(channel_id) - } - - async with self.bot.http_session.post( - URLs.site_bigbrother_api, - headers=self.HEADERS, - json=post_data - ) as response: - if response.status == 204: - await ctx.send(f":ok_hand: will now relay messages sent by {user} in <#{channel_id}>") - - channel = self.bot.get_channel(channel_id) - if channel is None: - log.error( - f"could not update internal cache, failed to find a channel with ID {channel_id}" - ) - else: - self.watched_users[user.id] = channel - self.watch_reasons[user.id] = reason - - # Add a note (shadow warning) with the reason for watching - reason = f"{self.infraction_watch_prefix}{reason}" - await post_infraction(ctx, user, type="warning", reason=reason, hidden=True) - else: - data = await response.json() - error_reason = data.get('error_message', "no message provided") - await ctx.send(f":x: the API returned an error: {error_reason}") + await self._watch_user(ctx, user, reason, channel_id) @bigbrother_group.command(name='unwatch', aliases=('uw',)) @with_role(Roles.owner, Roles.admin, Roles.moderator) @@ -328,7 +373,45 @@ class BigBrother: reason = data.get('error_message', "no message provided") await ctx.send(f":x: the API returned an error: {reason}") + @bigbrother_group.command(name='nominate', aliases=('n',)) + @with_role(Roles.owner, Roles.admin, Roles.moderator) + async def nominate_command(self, ctx: Context, user: User, *, reason: str): + """ + Nominates a user for the helper role by adding them to the talent-pool channel + + A `reason` for watching is required, which is added for the user to be watched as a + note (aka: shadow warning) + """ + + # Note: This function is called from HelperNomination.nominate_command so that the + # !nominate command does not show up under "BigBrother" in the help embed, but under + # the header HelperNomination for users with the helper role. + + channel_id = Channels.talent_pool + + await self._watch_user(ctx, user, reason, channel_id) + + +class HelperNomination: + def __init__(self, bot): + self.bot = bot + + @command(name='nominate', aliases=('n',)) + @with_role(Roles.owner, Roles.admin, Roles.moderator, Roles.helpers) + async def nominate_command(self, ctx: Context, user: User, *, reason: str): + """ + Nominates a user for the helper role by adding them to the talent-pool channel + + A `reason` for the nomination is required and will be added as a note to + the user's records. + """ + + cmd = self.bot.get_command("bigbrother nominate") + + await ctx.invoke(cmd, user, reason=reason) + def setup(bot: Bot): bot.add_cog(BigBrother(bot)) + bot.add_cog(HelperNomination(bot)) log.info("Cog loaded: BigBrother") diff --git a/bot/constants.py b/bot/constants.py index be713cef2..d823da81e 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -352,6 +352,7 @@ class Channels(metaclass=YAMLGetter): off_topic_3: int python: int reddit: int + talent_pool: int verification: int diff --git a/config-default.yml b/config-default.yml index bb49a46e1..d7f2efdb4 100644 --- a/config-default.yml +++ b/config-default.yml @@ -114,6 +114,7 @@ guild: python: 267624335836053506 reddit: 458224812528238616 staff_lounge: &STAFF_LOUNGE 464905259261755392 + talent_pool: &TALENT_POOL 534321732593647616 verification: 352442727016693763 ignored: [*ADMINS, *MESSAGE_LOG, *MODLOG] @@ -202,6 +203,7 @@ filter: - *BBLOGS - *STAFF_LOUNGE - *DEVTEST + - *TALENT_POOL role_whitelist: - *ADMIN_ROLE -- cgit v1.2.3 From 0f501deddfc1ec5bf82b5bfdc1dc07dfd63b1a8e Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 14 Jan 2019 12:58:45 +0100 Subject: Updating docstring for 'bigbrother nominate' command so it's equal to 'nominate' --- bot/cogs/bigbrother.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/bigbrother.py b/bot/cogs/bigbrother.py index 289bc47aa..b68785161 100644 --- a/bot/cogs/bigbrother.py +++ b/bot/cogs/bigbrother.py @@ -379,8 +379,8 @@ class BigBrother: """ Nominates a user for the helper role by adding them to the talent-pool channel - A `reason` for watching is required, which is added for the user to be watched as a - note (aka: shadow warning) + A `reason` for the nomination is required and will be added as a note to + the user's records. """ # Note: This function is called from HelperNomination.nominate_command so that the -- cgit v1.2.3 From 24b3c7533f13b904bff335297f46bd4bc930f507 Mon Sep 17 00:00:00 2001 From: sco1 Date: Mon, 14 Jan 2019 13:33:10 -0500 Subject: Change enumeration increment --- CONTRIBUTING.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 24376905b..1cfc8f5c5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,22 +7,22 @@ Note that contributions may be rejected on the basis of a contributor failing to ## Rules 1. **No force-pushes** or modifying the Git history in any way. -3. If you have direct access to the repository, **create a branch for your changes** and create a pull request for that branch. If not, create a branch on a fork of the repository and create a pull request from there. +2. If you have direct access to the repository, **create a branch for your changes** and create a pull request for that branch. If not, create a branch on a fork of the repository and create a pull request from there. * It's common practice for a repository to reject direct pushes to `master`, so make branching a habit! -5. **Adhere to the prevailing code style**, which we enforce using [flake8](http://flake8.pycqa.org/en/latest/index.html). +3. **Adhere to the prevailing code style**, which we enforce using [flake8](http://flake8.pycqa.org/en/latest/index.html). * Run `flake8` against your code **before** you push it. Your commit will be rejected by the build server if it fails to lint. * [Git Hooks](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks) are a powerful tool that can be a daunting to set up. Fortunately, [`pre-commit`](https://github.com/pre-commit/pre-commit) abstracts this process away from you and is provided as a dev dependency for this project. Run `pipenv run precommit` when setting up the project and you'll never have to worry about breaking the build for linting errors. -7. **Make great commits**. A well structured git log is key to a project's maintainability; it efficiently provides insight into when and *why* things were done for future maintainers of the project. +4. **Make great commits**. A well structured git log is key to a project's maintainability; it efficiently provides insight into when and *why* things were done for future maintainers of the project. * Commits should be as narrow in scope as possible. Commits that span hundreds of lines across multiple unrelated functions and/or files are very hard for maintainers to follow. After about a week they'll probably be hard for you to follow too. * Try to avoid making minor commits for fixing typos or linting errors. Since you've already set up a pre-commit hook to run `flake8` before a commit, you shouldn't be committing linting issues anyway. * A more in-depth guide to writing great commit messages can be found in Chris Beam's [*How to Write a Git Commit Message*](https://chris.beams.io/posts/git-commit/) -9. **Avoid frequent pushes to the main repository**. This goes for PRs opened against your fork as well. Our test build pipelines are triggered every time a push to the repository (or PR) is made. Try to batch your commits until you've finished working for that session, or you've reached a point where collaborators need your commits to continue their own work. This also provides you the opportunity to amend commits for minor changes rather than having to commit them on their own because you've already pushed. +5. **Avoid frequent pushes to the main repository**. This goes for PRs opened against your fork as well. Our test build pipelines are triggered every time a push to the repository (or PR) is made. Try to batch your commits until you've finished working for that session, or you've reached a point where collaborators need your commits to continue their own work. This also provides you the opportunity to amend commits for minor changes rather than having to commit them on their own because you've already pushed. * This includes merging master into your branch. Try to leave merging from master for after your PR passes review; a maintainer will bring your PR up to date before merging. Exceptions to this include: resolving merge conflicts, needing something that was pushed to master for your branch, or something was pushed to master that could potentionally affect the functionality of what you're writing. -11. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the work has already been done for you! -13. If someone is working on a pull request, **do not open your own pull request for the same task**. Instead, collaborate with the author(s) of the existing pull request. Communication is key, and there's no point in two separate implementations of the same thing. +6. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the work has already been done for you! +7. If someone is working on a pull request, **do not open your own pull request for the same task**. Instead, collaborate with the author(s) of the existing pull request. Communication is key, and there's no point in two separate implementations of the same thing. * One option is to fork the other contributor's repository and submit your changes to their branch with your own pull request. We suggest following these guidelines when interacting with their repository as well. -15. **Work as a team** and collaborate whereever possible. Keep things friendly and help each other out - these are shared projects and nobody likes to have their feet trodden on. -17. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server does not. With this trust comes responsibility - do not release any information you have learned as a result of your contributor position. We are very strict about announcing things at specific times, and many staff members will not appreciate a disruption of the announcement schedule. +7. **Work as a team** and collaborate whereever possible. Keep things friendly and help each other out - these are shared projects and nobody likes to have their feet trodden on. +8. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server does not. With this trust comes responsibility - do not release any information you have learned as a result of your contributor position. We are very strict about announcing things at specific times, and many staff members will not appreciate a disruption of the announcement schedule. Above all, the needs of our community should come before the wants of an individual. Work together, build solutions to problems and try to do so in a way that people can learn from easily. Abuse of our trust may result in the loss of your Contributor role, especially in relation to Rule 7. -- cgit v1.2.3 From 6bfe55e092ff97ffd1d1ce7c8a7e498f66893da5 Mon Sep 17 00:00:00 2001 From: sco1 Date: Mon, 14 Jan 2019 13:35:54 -0500 Subject: Fix number duplication --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1cfc8f5c5..cade82366 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,8 +21,8 @@ Note that contributions may be rejected on the basis of a contributor failing to 6. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the work has already been done for you! 7. If someone is working on a pull request, **do not open your own pull request for the same task**. Instead, collaborate with the author(s) of the existing pull request. Communication is key, and there's no point in two separate implementations of the same thing. * One option is to fork the other contributor's repository and submit your changes to their branch with your own pull request. We suggest following these guidelines when interacting with their repository as well. -7. **Work as a team** and collaborate whereever possible. Keep things friendly and help each other out - these are shared projects and nobody likes to have their feet trodden on. -8. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server does not. With this trust comes responsibility - do not release any information you have learned as a result of your contributor position. We are very strict about announcing things at specific times, and many staff members will not appreciate a disruption of the announcement schedule. +8. **Work as a team** and collaborate whereever possible. Keep things friendly and help each other out - these are shared projects and nobody likes to have their feet trodden on. +9. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server does not. With this trust comes responsibility - do not release any information you have learned as a result of your contributor position. We are very strict about announcing things at specific times, and many staff members will not appreciate a disruption of the announcement schedule. Above all, the needs of our community should come before the wants of an individual. Work together, build solutions to problems and try to do so in a way that people can learn from easily. Abuse of our trust may result in the loss of your Contributor role, especially in relation to Rule 7. -- cgit v1.2.3 From efce6be44154920a79f580e5e0c2f4721d0d0944 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 16 Jan 2019 13:18:00 +0100 Subject: Changing incorrect type hinting for _parse_infraction_time and updating its docstring --- bot/cogs/bigbrother.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bot/cogs/bigbrother.py b/bot/cogs/bigbrother.py index b68785161..f07289985 100644 --- a/bot/cogs/bigbrother.py +++ b/bot/cogs/bigbrother.py @@ -113,8 +113,12 @@ class BigBrother: return WatchInformation(reason="(no reason specified)", actor_id=None, inserted_at=None) @staticmethod - def _parse_infraction_time(infraction: str) -> struct_time: - """Takes RFC1123 date_time string and returns time object for sorting purposes""" + def _parse_infraction_time(infraction: dict) -> struct_time: + """ + Helper function that retrieves the insertion time from the infraction dictionary, + converts the retrieved RFC1123 date_time string to a time object, and returns it + so infractions can be sorted by their insertion time. + """ date_string = infraction["inserted_at"] return strptime(date_string, "%a, %d %b %Y %H:%M:%S %Z") -- cgit v1.2.3 From f39ee73d4d585e1531834b0a6b562a87aa70b970 Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 16 Jan 2019 20:17:52 -0500 Subject: Add developer wiki links --- CONTRIBUTING.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cade82366..e05655af1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,6 +31,11 @@ Above all, the needs of our community should come before the wants of an individ All projects evolve over time, and this contribution guide is no different. This document is open to pull requests or changes by contributors. If you believe you have something valuable to add or change, please don't hesitate to do so in a PR. ## Supplemental Information +### Developer Environment +A working environment for the [PyDis site](https://github.com/python-discord/site) is required to develop the bot. Instructions for setting up environments for both the site and the bot can be found on the PyDis Wiki: + * [Site](https://wiki.pythondiscord.com/wiki/contributing/project/site) + * [Bot](https://wiki.pythondiscord.com/wiki/contributing/project/bot) + ### Logging levels The project currently defines [`logging`](https://docs.python.org/3/library/logging.html) levels as follows: * **TRACE:** Use this for tracing every step of a complex process. That way we can see which step of the process failed. Err on the side of verbose. **Note:** This is a PyDis-implemented logging level. -- cgit v1.2.3 From 6d3fbf07efa0768ad32ef9319dac73a3b1e840cc Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 16 Jan 2019 20:55:58 -0500 Subject: Simplify role check logic --- bot/cogs/moderation.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index d9c217b83..64c76ae8a 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -1243,7 +1243,7 @@ class Moderation(Scheduler): async def respect_role_hierarchy(self, ctx: Context, target: Member, infraction_type: str) -> bool: """ - Check if the highest role of the invoking member is less than or equal to the target member + Check if the highest role of the invoking member is greater than that of the target member If this check fails, a warning is sent to the invoking ctx @@ -1251,19 +1251,18 @@ class Moderation(Scheduler): checks & conversions in a dedicated check decorater """ - # Build role hierarchy actor = ctx.author - role_hierarchy = {role: rank for rank, role in enumerate(reversed(ctx.guild.roles))} - hierarchy_check = role_hierarchy[actor.top_role] <= role_hierarchy[target.top_role] - - if not hierarchy_check: + target_is_lower = target.top_role < actor.top_role + if not target_is_lower: log.info( f"{actor} ({actor.id}) attempted to {infraction_type} " - f"{target} ({target.id}), who has a higher top role" + f"{target} ({target.id}), who has an equal or higher top role" + ) + await ctx.send( + f":x: {actor.mention}, you may not {infraction_type} someone with an equal or higher top role" ) - await ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") - return hierarchy_check + return target_is_lower def setup(bot): -- cgit v1.2.3