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

Add notifications to admins about new users #205

Closed
okainov opened this Issue Feb 7, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@okainov
Contributor

okainov commented Feb 7, 2018

Description of problem

Somehow related to #203. When user is being registered and settings.ADMINS are set, the message is being shown. But I would expect some notification from Kiwi to admins as well like "User XXX have been registered and is waiting for activation. Please proceed to admin panel in order to activate the account"

Component (web, API, etc)

web

Version or commit hash (if applicable)

latest

How often reproducible

always

Steps to Reproduce

  1. Modify line 41 in tcms/core/contrib/auth/views.py in order to proceed to "else" branch always
  2. Register new user
  3. Make sure notification with "Following is the admin list" was shown

Actual results

Admins don't receive any notifications

Expected results

Some email should be sent

Additional info

Actually there is one more thing to think about - why is there settings.ADMINS at all? There are already user accounts in Kiwi with admin\superuser rights - cannot you just take all superusers or members of Administrators group and show\notify them instead of using custom setting?

@atodorov

This comment has been minimized.

Member

atodorov commented Feb 7, 2018

But I would expect some notification from Kiwi to admins as well like "User XXX have been registered and is waiting for activation. Please proceed to admin panel in order to activate the account

Good idea, I can even do one better. We can trigger a signal after a user is registered and add a default signal handler to notify admins via email. In addition you can tweak settings and add additional handlers which can perform arbitrary actions. How does this sound ?

@okainov

This comment has been minimized.

Contributor

okainov commented Feb 7, 2018

Sounds good 👍

atodorov added a commit that referenced this issue Feb 14, 2018

Add signal handler to notify admins on new users. Fix #205
- update translation files since we add a new translatable strings

@atodorov atodorov closed this in 5b3e4e2 Feb 14, 2018

@okainov

This comment has been minimized.

Contributor

okainov commented Mar 8, 2018

@atodorov how do you think will it make sense to have the code in settings.py, but maybe commented out? In general, this signals things doesn't seem "well-known" for Django apps, so maybe it would be better to have section like "### Signals connecting" in settings.py so it could be easily seen and uncommented if needed.

@atodorov

This comment has been minimized.

Member

atodorov commented Mar 8, 2018

This is already in the docs.

http://kiwitcms.readthedocs.io/en/latest/configuration.html#augmenting-behavior-with-signal-handlers

points to:
http://kiwitcms.readthedocs.io/en/latest/modules/tcms.signals.html#module-tcms.signals

which has an example of connecting the notification handler in your settings file and a link to the official Django documentation if you need a more complex setup (e.g. using AppConfig).

@okainov

This comment has been minimized.

Contributor

okainov commented Mar 8, 2018

Yes, I saw it, but I think many users either won't read the docs so carefully or will miss this small part. It will be much easier to have section in settings like:

### Signals handling
# //Short description in couple of sentences
# //what is it for in general

## This will enable notifications about new users if uncommented
# user_registered.connect(notify_admins)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment