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

Revert "Notify the owner on common mis-configurations" #291

Closed
wants to merge 1 commit into from

Conversation

anarcat
Copy link

@anarcat anarcat commented Feb 13, 2020

This causes unneeded spam to the owner when we block access to a user
through the web interface.

This reverts commit 03d0af9.

This causes unneeded spam to the owner when we block access to a user
through the web interface.

This reverts commit 03d0af9.
@anarcat
Copy link
Author

anarcat commented Feb 13, 2020

Opened a discussion about this in the forum in https://forum.bestpractical.com/t/rt-4-4-too-noisy-with-denied-users/34749 and filed it as a bug in debian in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=951272

@cbrandtbuffalo
Copy link
Member

Hi @anarcat , thanks for submitting a PR. In this case the code is working as intended, so I wouldn't categorize it as a bug. The reason we send email in this case is a common request we had from users is for admins to see when a legitimate user was being blocked due to rights.

I'm sorry it's created too much email for your case, but there is another good solution. One of the goals of the refactoring was to make email handling more plugable. In this case, you could write your own mail plugin that essentially copies the default but removes the unwanted owner email and maybe even the response to the spammer. You can put your new plugin first and it should run before the default. If it returns true, additional processing will stop. More info here:

https://docs.bestpractical.com/rt/4.4.4/extending/mail_plugins.html#CheckACL

@anarcat
Copy link
Author

anarcat commented Feb 14, 2020

@cbrandtbuffalo that's a good idea, thanks! I just worry about changing the default, why isn't that check simply a separate plugin that we could disable if we trust our admins? it's the way it used to work before, isn't it?

i worry that if i write a third-party plugin, i will have to maintain it forever and chase the RT API in the future...

after all, that plugin was separate before, is there any reason it was merged in Default.pm?

thanks for the response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants