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

Added setting to add a message after logout #104

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pythad
Copy link

@pythad pythad commented Dec 21, 2017

It's a common use case to provide a message after logout because of inactivity. This PR implements this feature.

@@ -11,6 +11,7 @@

from datetime import datetime, timedelta

from django.contrib import messages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not check that this app is installed ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been in Django core for quite a bit. Not sure for how long, but it was added to Git repo in 2009. It's definitely 1.8+ (django-session-security supported versions). So no, there is no need to check if this app has been installed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if an app isn't using contrib.messages? Does the call fail silently? Error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default django.contrib.messages is included into INSTALLED_APPS. So unless a developer deleted the package from INSTALLED_APPS everything will work. If there is no django.contrib.messages in INSTALLED_APPS an exception will be raised. But the default value of LOGOUT_MESSAGE is None, this means that by default we even don't need django.contrib.messages. And if a user will want to set LOGOUT_MESSAGE he will read the docs and see that it depends on django.contrib.messages. So the only scenario the exeption will be raised is when a user will specify LOGOUT_MESSAGE and delete django.contrib.messages from INSTALLED_APPS.

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.

3 participants