session-based messages

FlashMiddleware? and flash context processor

I definitely don't like the name, but the idea is interesting. It does seem like messages naturally fit the session (maybe moreso than the users)

This has to go to a design decision, I recommend you bring this up on the dev group.

I'm pretty sure this is a duplicate of a request that was filed a while back for the same thing (messages tied to sessions so they'd work with non-authenticated users), but I can't find it and I don't think the other one had a patch.

Here's my take on it, with tests and docs.

I would really replace the current Message-system with this one. Besides from beeing more generic it fixes a race-condition, that could happen when two users surf with the same account and both receive messages. Should be rare, but is fixed with this patch.

Two users shouldn't be using the same account though :)

Two people may often share an account. For many applications there is no need to create a unique user for every person who will use the system, one superuser and one manager is often sufficient for many small projects.

I'd love to see messages attached to sessions instead of users, although it doesn't necesarily need to *replace* the user messages functionality which could also be useful (setting messages offline to be received at next login).

I also thought this issue had another ticket (with a patch) already.

Nice patch, Chris. Two things I wanted to bring up though...

1. Looks like your patch is impacted by #4729. As a result, if the message is set and the session is persisted, the message will never be deleted by get_and_delete_messages.

2. I wonder if the use of a LazyMessage style object might be helpful. This would ensure that the messages aren't actually purged until they are viewed.

Thx Ben. Yep, looks like it does rely on #4729 - that's a bit less straightforward than it actually looks however (see my comment there).

Regarding a LazyMessage, perhaps, but that's a full change of functionality. Things like that help leave tickets in DDR for much longer. Hmm, it does actually sound quite useful. Damn... :)

My apologies for the LazyMessage suggestion ;-)

I quickly threw together a context processor that uses lazy evaluation for both (or either) Session and User messages... I ended-up breaking it out of contrib (into my own project) though since it doesn't fit nicely in either contrib.sessions or contrib.auth.

If there's interest I can break it apart to handle only session messages (visitor_messages as you've named them) and post it here... but I don't have the time to write the necessary tests right now.

I sat down and had a good think about the LazyMessage idea and I'm not sure that it is actually necessary. You don't have to use the context processor, you could use the django.core.context_processors.request context processor and access the messages directly:

{% for message in request.session.get_and_delete_messages %}...{% endfor %}

So we're (thankfully) back to the only design decision left being whether the original idea itself is actually core-worthy...

The visitor_messages.patch works for me.

One thing: ""The session message system provides a simple way to queue messages for

anonymous site visitors. To associate messages with users in the user database,

use the authentication message framework_."""

I would say: ""The session message system provides a simple way to queue messages for

all (anonymous or authenticated) site visitors. If you want to associate messages

with users in the user database ..."""

Update to visitor_messages patch to use session backends

Updated patch to new session implementation and included the suggested documentation change

There is a bug in visitor_messages_r6373.patch

old:

    def get_and_delete_messages(self):  
        return self.pop(self.MESSAGES_NAME, [])

new:

    def get_and_delete_messages(self):
        msgs = self.pop(self.MESSAGES_NAME, [])
        if msgs:
            self.modified = True  
        return msgs

I think it would be nice to have a loglevel.

KNOWN_LOGLEVELS=["info", "error"]

class Message(object):
    def __init__(self, message, loglevel="info"):
        self.message=message
        assert loglevel in KNOWN_LOGLEVEL
        self.loglevel=loglevel
    def __unicode__(self):
        return self.message

def create_message(self, message, loglevel="info"):
    ...
    messages.append(Message(message, loglevel))
    ...

Hello Thomas,

The change you mention (self.modified = True) is not a problem with this patch... this patch just reveals the problem.

The recent update of the session framework broke the functionality of pop. Please see #4729 for more information.

As for the loglevels, I'm -1. I think that this was discussed on the django-dev list for the User-based message framework... you might want to check there. If not, you should bring it up. I think that it's important to keep both the Session message framework and User message framework functionality the same.

Updated patch for revision 6596

Updated patch for revision 6596

-1 for loglevels

Marking this as Accepted based on discussion at the 12/1/2007 sprint in Lawrence. I'm claiming it for now, and will work on it over the coming week or so.

We need to come up with an interface that gracefully allows use of both auth messages and sessions messages. I have some ideas on how to do this and will write them up here if I don't get around to implementing them in the next week or two.

Hawkeye, wanna email me (@gmail) to discuss this?

Rewritten version, new core context processor which grabs messages from both user and session

Ok, here's a rewritten version which matches the requirements which hawkeye discussed with me via email.

If some people want to have a play with it and report back, that'd be swell :)

Note that if you want the admin messages to work still and you have specified a TEMPLATE_CONTEXT_PROCESSORS in your own settings module, you'll need to ensure you add the new context processor to it.

This will need to be mentioned in BackwardsIncompatibleChanges.

Patch looks good overall.

Three quick points:

1. The documentation seems to focus on authenticated versus anonymous. I see this as synchronous (session) versus asynchronous (user) messaging.
2. You import memoize and lazy into django.core.context_processors, but it doesn't look like you're using either.
3. There currently aren't any tests for the LazyMessages class. I'd feel more comfortable if they were there.

Replying to __hawkeye__:

Patch looks good overall. Three quick points: 1. The documentation seems to focus on authenticated versus anonymous. I see this as synchronous (session) versus asynchronous (user) messaging.

It uses the terms "user messages" and "session messages". I'm not sure that calling this synchronous/asynchronous would make it any clearer.

The line "The session message system provides a simple way to queue messages for all (anonymous or authenticated) site visitors" is just there to explain synchronosity (my imaginary word of the day).

2. You import memoize and lazy into django.core.context_processors, but it doesn't look like you're using either.

Indeed I'm not. I was going to use them but in the end just wrote the LazyMessages class, due to the fact that lazy() isn't useful for non-string classes.

3. There currently aren't any tests for the LazyMessages class. I'd feel more comfortable if they were there.

Sure... now I just have to figure out where to stick them.

More tests

removed unnecessary imports

Ok, new tests in now. This patch also adds a new empty tests/middleware/models.py file, since those tests (which do pass) weren't actually being run due to the lack of this file.

Slight documentation change.

Made a slight documentation change, but this patch looks good to me. Patch applies cleanly, passes tests and works in my limited testing.

Thanks, SmileyChris?, for this latest patch.

I have been considering the fact that this is quite a big incompatible change (if you have overridden the context processors setting, you'll have to add the messages processor in to retain your admin message functionality).

I was thinking that perhaps the admin processor should check to see if messages has been turned on, and if not then continue treating the (user) messages like it currently does. This would be tricky to document and I'm not sure how much I'm allowed to "rock the boat" with this change, so I'll wait for a committer comment before worrying about adding that (it wouldn't be too difficult, anyway)

A couple of stylistic issues (all in the LazyMessages class):

1. Subclass from django.utils.encoding.StrAndUnicode so that you get a __str__ method.
2. Probably better form to call iter(self.messages), rather than calling self.messages.__iter__() directly. Use the interface, not the implementation (seems to be the python-dev-preferred approach, too). I prefer the way you've implemented __len__ to the __iter__ implementation, in other words.
3. Blank lines are needed between the methods in that class.

I don't have a strong opinion about the move to the context processor at the moment, although you're correct that it's of some concern. Adding code that tries to work with both old- and new-style is something we generally avoid doing, because in the long-term it's just bloat and there's never a good moment to remove it (particularly if 1.0 was released in the interim, so you might as well do it up-front). It's hard to make things fail with an exception if somebody forgets, too. I feel that this is a bit like the old "compulsory middleware" stuff: it's a bit of an oxymoron. If we really need that processor, which we do because admin needs it, then it shouldn't be in any way optional. I haven't wrapped my brain around why it can't be called all the time yet.

My secret hope is that another maintainer will adopt this, since this isn't too high on my priority list, but it looks like it's getting towards some sort of completion point, so we should keep it on the radar.

Now with better backwards compatibility

Replying to mtredinnick:

A couple of stylistic issues (all in the LazyMessages class):

All addressed - thanks for the django.utils.encoding.StrAndUnicode tip.

I don't have a strong opinion about the move to the context processor at the moment, although you're correct that it's of some concern.

I have now made the auth processor work like it did before (in as much as it adds a messages variable) and changed the messages processor to not be enabled by default.

So now we have better backwards compatibility and everyone can hold hands and dance around together.

identical to previous, except has empty init.py and models.py files for tests

identical to previous, except has empty init.py and models.py files for tests (no, really this time)

implement __getitem__ too, some people are using stuff like {{ messages.0 }}

Removing 1.0-beta tag. It might make it in or it might not, but it's not on the "maybe" list and there's enough stuff to do that we have to guard that milestone setting carefully.

Spoke with Malcolm... looks like there's hesitation to do this currently (and somewhat in general).

hawkeye,

That is really disappointing to hear. What is the source of hesitation (beyond being a non-priority for 1.0)? Personally, I think it's pretty obvious that this is a necessary feature. (I mean, really it's just a better implementation of the currently existing message passing system.)

I think this feature is quite useful in every communication with anon users of your website: contact form (sent contact info sucessfully), newsletter (thanks for joining newletter), ...

Why don't commit this? it isn't a huge refactoring.

Updated messages.7.diff to avoid rejects with django.core.context_processors.py

Just chiming in to say that I find this really useful, and hope it does make it in after 1.0's done and dusted.

To me it makes no sense to store messages with a short lifetime in user.message_set. user.message_set should be reserved for system or administrator messages to the user. The session is the right place to store feedback about the current session for the user.

What if, for example, the same user is logged in from two different browsers and completing two different tasks, simultaneously? When using user.message_set to store feedback for the user, the messages will be distributed on a first come first served basis, with no regard for what session actually generated what feedback. It's bad to get in the habit of using user.message_set for messages like "Article updated successfully," or other messages that really have no context outside the current session.

For these reasons, I think this patch erroneously conflates user messages and session messages.

FYI I took ideas from the patch and separated it into middleware: http://www.djangosnippets.org/snippets/1002/

Also added optional type for messages so you render them differently (e.g. http://traviscline.com/blog/2008/08/23/django-middleware-session-backed-messaging/)

Moving back from "ready for checkin", because I don't think it is yet. I suspect the new lazy() from r8120 is probably more reusable here. The duplicate lazy stuff makes me nervous.

I don't like the backwards compatibility this brings to the table. This is probably something that's relatively orthogonal to the existing stuff. I'm also not wild about it being in core, rather than a contrib app, since whilst it's useful sometimes and meets the "a reasonably common pattern", it's hardly impossible to use Django without it.

Right now, it's really suffering from not having any core maintainer having fallen in love with the idea and/or implementation enough. For me, a clean implementation that was low-impact on other pieces of code and backwards compatible and optional (part of contrib) would be more palatable. I haven't really seen much in the way of comment from any other core committer. I realise a fair bit of work has gone into this, but it doesn't feel like it's there yet. Solve the backwards incompatibility problem (not breaking existing admin, particularly) and work out how it could be part of contrib somewhere (contrib/auth maybe?) and it will have a better chance or proceeding. The fact that it impacts contrib/sessions also makes me wonder if the implementation is really correct. It's really being baked into the code in a few places. We must be able to do better than that.

Replying to mtredinnick:

Moving back from "ready for checkin", because I don't think it is yet. I suspect the new lazy() from r8120 is probably more reusable here. The duplicate lazy stuff makes me nervous.

That's fine - back when I started lazy() didn't do a very good job.

I don't like the backwards compatibility this brings to the table. This is probably something that's relatively orthogonal to the existing stuff. I'm also not wild about it being in core, rather than a contrib app, since whilst it's useful sometimes and meets the "a reasonably common pattern", it's hardly impossible to use Django without it.

It's only "in core" as much as the auth context processor is. And that is only required if you want messages and aren't using the auth context processor. I guess it could have it's own contrib app but it would only consist of a context processor!

Solve the backwards incompatibility problem (not breaking existing admin, particularly)

What backwards incompatibility? It won't break existing admin.

... and work out how it could be part of contrib somewhere (contrib/auth maybe?) and it will have a better chance or proceeding. The fact that it impacts contrib/sessions also makes me wonder if the implementation is really correct.

The whole point is that this is a session message system. It seems obvious to bake it into contrib/sessions to me.

The context processor just provides a single message context variable containing both (existing) user messages and (new) session messages. This keeps backwards compatibility with the current message system (and admin). However, as it has been noted by tobias, the admin is currently doing the wrong thing and should really be updated to using a session based messaging system.

Patch updated to r9698.

I've updated SmileyChris' messages.7.diff to trunk as of now (messages-r8347.diff was missing three files).

Maybe it's a good idea to take a look at Django-flash, an open source app that provides an implementation that works pretty much like the Rails one:

http://djangoflash.destaquenet.com/

I'm the Django-flash's lead developer, and I'd be more than happy to see it bundled with Django.

Replying to anonymous:

I just posted to the mailing list without seeing that you've commented here: http://groups.google.com/group/django-developers/browse_thread/thread/1ad495e30f946b2c/f6501a9e2a66e1d0

I've been using django-flash and I like it a lot. Maybe an actual Django patch with flash as a contrib app and proper Django documentation and such would be helpful in getting more support for this implementation?

I realise now that my efforts towards integrating with current user messages in a backwards-compatible manor is a waste of time.

Since that was the only reason to have the code here I have now started a separate app to do session/cookie based user notifications. This app supersedes the patches in this ticket (and would be dead easy to convert to a contrib app if this is ever wanted - which I think it should be, as it is a good example for something which there should be one right way [for most cases] to do).

You can find it here: http://code.google.com/p/django-notify/

I haven't done tests yet, but am willing to accept contributions ;)

Django-flash codebase is quite stable and it is fully documented and tested (all possible use cases are tested both in unit and integration tests), so I think the code is fine right now. I obviously don't know how many people are actually using it in their projects, but I know some of them who are quite happy with it. And I am too, since I created django-flash to scratch my own itch!

If you guys think django-flash would be a nice addition to Contrib (I think it is!), just get the code and use it the way you want, seriously. The project is currently licensed under LGPL, but I don't think it is a problem. (if it is, I can just dual license the project.)

As I said before, I'd be more than happy to see django-flash bundled with Django. Unfortunately, I will not send patches and work on a Django-style documentation before I get the green light.

The problem is that django-flash doesn't really encapsulate the requirements for a standard session based messaging system since it doesn't enforce a single way of using messages, so there is no way for third party apps to provide messages in a consistent manor.

This doesn't mean that I don't think it is a well developed or useful application, I'm just saying that I don't think on its own it works as a notification system.

As an unimportant personal aside, I don't like the name "flash".

I know what you mean. Although django-flash fulfills its purpose, this "flash" thing is probably just a subset of a full-fledged notification system.

Now I ask you: what exactly should a notification system do? Depending on the context, a notification system should do different things, at different levels of abstraction. So unless you define what a notification system should do, such term as "notification system" is just too general and creates more confusion than it helps.

I think we are missing the point here. The "flash" thing is part of Rails since the old days, and it continues to prove its usefulness. It is also general enough to cover the common cases. I might be biased, but I personally don't know anybody who dislikes the Rails' "flash" thing or thinks it's not great the way it is.

And, by the way, I don't like the name "flash" either, but that name happened to be used by several other web frameworks and became the standard way to describe such context or scope. So I think we should stick with that name and make everybody's life easier.

Oops, wasn't logged in before (in comment 80).

Actually, I'd say flash is a superset of the notification system that's being discussed in this ticket. All we're after is an alternative to storing messages in the User - so what we want is to encapsulate this behaviour, but in a standard way. The only frequently requested addition is the ability to have messages which can have different HTML classes.

Oh, and another request from several parties was to allow these messages without causing a hit on the database: django-notify comes with an alternate cookie storage backend you can use for this.

I'm going to take a stab at converting django-flash + django-notify into something that looks like a contrib app with docs and tests. Let me know if you'd like to help! I think sticking with the "flash" name, although it sucks, will end up being a good thing.

leah: Personally I wouldn't bother the conversion until the issue is discussed a bit more in django-dev - and that discussion would best wait until after 1.1

But if you still need some help after then, I'd be more than happy. Let's just get a solution!

Ditto on helping out. I realize that django-flash may be closely following a convention with which I'm not familiar, but I don't like it for two reasons:

# It doesn't have an obvious, standard method for storing 2+ separate messages # If the second request generates a redirect (or is another view that doesn't print session messages), and that view doesn't know about django-flash, you'll lose your messages

For these reasons I like django-notify better. It provides a standard way to store/retrieve multiple messages, and those messages aren't erased until they're actually printed in the template.

At least, that's my limited understanding of it. Just my two cents...

Replying to tobias:

# It doesn't have an obvious, standard method for storing 2+ separate messages

Just use Python! There's nothing more "obvious and standard" than a plain List to store several messages under a single key:

request.flash['message'] = ['Message one', 'Message two']

I like this approach because we are being explicit that there's multiple messages under the same key. (Remember the Zen of Python, which says "Explicit is better than implicit.")

# If the second request generates a redirect (or is another view that doesn't print session messages), and that view doesn't know about django-flash, you'll lose your messages

If you want flash messages to survive a redirect:

def view1(request):
    request.flash['message'] = 'My message'
    return HttpRedirectResponse(reverse(view2))

def view2(request):
    request.keep() # Keeps all messages stored inside the flash
    request.keep('message1', 'message2') # Only keeps the given messages
    return HttpRedirectResponse(reverse(final_view))

I personally think it's a feature, not a bug or something like that for the same reason: "Explicit is better than implicit".

Another reason is that we avoid propagating undesired messages by accident, since we might store stuff inside the flash to be retrieved by other views, not templates. It's rare, but it's possible.

Ooops, fixing some typos in the code:

def view1(request):
    request.flash['message1'] = 'My message'
    return HttpRedirectResponse(reverse(view2))

def view2(request):
    request.flash.keep() # Keeps all messages stored inside the flash
    request.flash.keep('message1', 'message2') # Only keeps the given messages
    return HttpRedirectResponse(reverse(final_view))

• flash['message'] = ['1', '2'] is too flexible. You might have a project with one pluggable app that just sets flash['message'] = 'message', another that does flash['message'].append('message'), and yet another that does flash['message'] = ('message 1', 'message 2'). And if you want to append, who's responsible for creating the list in the first place? As far as I can tell from the docs, django-flash doesn't propose a convention for doing this. Maybe it's just me, but notifications.add('message') seems much more explicit...

• request.keep is all well and good for your own tightly integrated code, but what if you're dealing with a view in a pluggable app that you can't modify, and it doesn't know about django-flash?

• WRT propagating undesired messages, as far as I can tell django-notify (and I believe at least one of the patches on this ticket) only destroys messages when iterated (from a template OR a view). I'm not exactly clear about your concern but would that address it?

I tried to sum up some of my points about what would comprise a good contrib app here: http://www.caktusgroup.com/blog/2009/06/19/towards-a-standard-for-django-session-messages/

Replying to tobias:

* flash['message'] = ['1', '2'] is too flexible. You might have a project with one pluggable app that just sets flash['message'] = 'message', another that does flash['message'].append('message'), and yet another that does flash['message'] = ('message 1', 'message 2'). And if you want to append, who's responsible for creating the list in the first place? As far as I can tell from the docs, django-flash doesn't propose a convention for doing this. Maybe it's just me, but notifications.add('message') seems much more explicit...

Storing several messages under the same key is not the rule, but the exception, at least for the projects I work on. But once you treat the flash as a regular dictionary (in fact their interface are very similar), things become easier to grasp. But, for the record, I have no problem with the ".add('message')" thing.

* request.keep is all well and good for your own tightly integrated code, but what if you're dealing with a view in a pluggable app that you can't modify, and it doesn't know about django-flash?

This is a very good point. I haven't thought about this until now. I'll try to come up with something.

* WRT propagating undesired messages, as far as I can tell django-notify (and I believe at least one of the patches on this ticket) only destroys messages when iterated (from a template OR a view). I'm not exactly clear about your concern but would that address it?

Yes it does, if that's really the case. The only thing I don't like about it is the idea of getting my messages removed when doing something like "request.flashmessage?" in my views.

Replying to daniel_martins:

Storing several messages under the same key is not the rule, but the exception, at least for the projects I work on. But once you treat the flash as a regular dictionary (in fact their interface are very similar), things become easier to grasp. But, for the record, I have no problem with the ".add('message')" thing.

It has nothing to do with being easier to grasp. The issue is that django-flash does not propose a singular, standard structure for creating and retrieving 2+ messages. The very fact that it's a dictionary leaves too much as an "exercise for the reader."

It has nothing to do with being easier to grasp. The issue is that django-flash does not propose a singular, standard structure for creating and retrieving 2+ messages. The very fact that it's a dictionary leaves too much as an "exercise for the reader."

Without having actually tried django-flash: How so? If it's a dictionary, then the interface is pretty clear: It's the same interface as the standard python dict, possibly with some goodies on top of it. How does it matter if you use [] or update() to add messages?

I'm chiming in here because I would really like to see multiple **types** of messages supported, e.g. "error", "notice" etc. Using a dict seems fitting then.

Multiple message types sounds good to me. A dict probably makes sense behind the scenes, but I think it's exposing too much to make that the default interface for creating/retrieving messages.

Guys, might I suggest a different venue for this discussion, trac is a pretty bad place for it, as it's not as easily searchable, and it's visible to a smaller cut of individuals.

Replying to Travis Cline:

FYI I took ideas from the patch and separated it into middleware: http://www.djangosnippets.org/snippets/1002/ Also added optional type for messages so you render them differently (e.g. http://traviscline.com/blog/2008/08/23/django-middleware-session-backed-messaging/)

seems to be the same idea as 11617: Enhance User Message System

django-notify has a combo/fallback backend now:

http://code.google.com/p/django-notify/issues/detail?id=5

per Alex's request I started a conversation on django-developers here:

http://groups.google.com/group/django-developers/browse_thread/thread/eba93088c649022b

and a wiki page here:

http://code.djangoproject.com/wiki/SessionMessages

Leah, do you mind if I claim this ticket? Are you still working on it? Thanks -tobias

Here you go! Nice wiki page summary.

development branch for proposed messages app is here: http://bitbucket.org/tobias.mcnulty/django-contrib-messages/

diff showing changes in django-contrib-messages branch as of changeset 9f54c0f8719c

diff showing changes in django-contrib-messages branch as of changeset e4da706e1152

changes in django-contrib-messages branch as of rev 6399c12d1773

final patch from django-contrib-messages branch

(In [11804]) Fixed #4604 - Configurable message passing system, supporting anonymous users

This deprecates User.message_set in favour of a configurable messaging system, with backends provided for cookie storage, session storage and backward compatibility.

Many thanks to Tobias McNulty? for the bulk of the work here, with contributions from Chris Beaven (SmileyChris?) and lots of code review from Russell Keith-Magee, and input from many others. Also credit to the authors of various messaging systems for Django whose ideas may have been pinched :-)

Took me a minute to find the docs, so here they are if anyone else is looking: http://docs.djangoproject.com/en/dev/ref/contrib/messages/