Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No reply if permission denied #330

Merged
merged 16 commits into from
Aug 12, 2022

Conversation

aconitumnapellus
Copy link
Contributor

@aconitumnapellus aconitumnapellus commented Aug 5, 2022

Hi!

This is a little "feature" ive made for myself because the "permission denied" message annoyed me in channels where certain commands were not allowed (For example, i dont want the "cats" command to throw a "permission denied" in "#salespitches" when someone accidentally types "cats")

This PR contains:

  • a "silence_fail_msg" parameter that allows to silence the "permission denied" reply (Default is to reply with "permission denied")
  • Unit tests for the allowed_channels feature and the "silence_fail_msg" parameter
  • Instead of using host networking in the podman-compose file, i opted for port exposing, as host networking does not seem to work correctly. [1]
  • A bump for pytype, as the version mentioned in the requirements does not exist anymore?
    • My version of python was too new for the version of pytype, thats why it did not "exist". Ive reverted the commit that bumped pytype.

[1]

podman create --name=mattermost-bot-test --label io.podman.compose.config-hash=123 --label io.podman.compose.project=integration_tests --label io.podman.compose.version=0.0.1 --label com.docker.compose.project=integration_tests --label com.docker.compose.project.working_dir=/home/selen/github/mmpy_bot/tests/integration_tests --label com.docker.compose.project.config_files=tests/integration_tests/docker-compose.yml --label com.docker.compose.container-number=1 --label com.docker.compose.service=app --network host --net integration_tests_default --network-alias app --add-host dockerhost:127.0.0.1 integration_tests_app ./mm/docker-entry.sh
Error: cannot set multiple networks without bridge network mode, selected mode host: invalid argument
exit code: 125

@attzonko
Copy link
Owner

attzonko commented Aug 5, 2022

I hate to sound too nit-picky but the no_reply name seems very broad and general, and the intention is to specifically silence an authorization failure. Further no_reply being a negative makes it a bit awkward when checking for example: if self.no_reply is False: and the more awkward double negative if not self.no_reply:

I would like to propose you change it to something more specific like silence_fail_msg or disable_fail_reply or some other combination. The conditionals can sound a bit more natural like if not self.silence_fail_msg:

@attzonko
Copy link
Owner

attzonko commented Aug 5, 2022

Thank you for adding tests for this new feature 👍🏼

@codeclimate
Copy link

codeclimate bot commented Aug 8, 2022

Code Climate has analyzed commit 6b94ada and detected 0 issues on this pull request.

View more on Code Climate.

@attzonko attzonko merged commit 9394470 into attzonko:main Aug 12, 2022
@attzonko
Copy link
Owner

Thanks for contributing @aconitumnapellus

@aconitumnapellus aconitumnapellus deleted the permission_denied_no_reply branch August 12, 2022 10:58
unode pushed a commit to unode/mmpy_bot that referenced this pull request Feb 9, 2023
Adds capability to silence error message from bot when using commands in disallowed channel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants