Opened 17 years ago

Closed 14 years ago

#3995 closed (wontfix)

Give the Message model a type (success/failure/generic)

Reported by: joe4444 Owned by: Tim Graham
Component: Core (Other) Version: dev
Severity: Keywords: auth message
Cc: django@…, ross@…, xerdcire@…, jwm@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The message system is just a bit too light for me. Rather than assume all messages indicate a successful action I think they should also be able to point out errors (unrelated to form validation obviously) or just display generic notices. This patch makes it possible to create 3 types of messages: success (default), failure, and generic. Normal messages will remain the same (always success). For a failure/error or generic message you simply pass an extra argument like so:

    user.message_set.create(message=u'Something went wrong!', category='F')    # failure or error
    user.message_set.create(message=u'This is just a notice', category='G')    # generic message
    user.message_set.create(message=u'It worked!', category='S')    # explicit success
    user.message_set.create(message=u'It worked!')    # implicit success


Now in your template you can distinguish messages by type, and then use CSS to style them differently.

{% if messages %}
  <ul id="messages">
{% for message in messages %}
    <li class="{{ message.get_category_display }}">{{ message }}</li>
{% endfor %}
  </ul>
{% endif %}

Attachments (5)

message_type.diff (880 bytes ) - added by joe4444 17 years ago.
Modify the Message model to categorize messages
message_type.2.diff (1.7 KB ) - added by joe4444 17 years ago.
Same code; appended Message model docstring
message_type.3.diff (1.7 KB ) - added by joe4444 17 years ago.
I think the others only worked for models.py from 0.96
3995.django.contrib.auth.diff (3.8 KB ) - added by Pete Crosier 16 years ago.
Updated messages with a type, with tests and documentation
3995.diff (5.2 KB ) - added by Tim Graham 15 years ago.
updated patch to apply to trunk, added post_syncdb handler to add field

Download all attachments as: .zip

Change History (26)

by joe4444, 17 years ago

Attachment: message_type.diff added

Modify the Message model to categorize messages

comment:1 by joe4444, 17 years ago

Version: SVN0.96

Probably doesn't make a difference, but I patched 0.96 not trunk.

by joe4444, 17 years ago

Attachment: message_type.2.diff added

Same code; appended Message model docstring

comment:2 by joe4444, 17 years ago

Needs documentation: set
Needs tests: set
Patch needs improvement: set

Using a CharField with choices might not be the ideal solution. Any other suggestions?

comment:3 by Simon G. <dev@…>, 17 years ago

Triage Stage: UnreviewedDesign decision needed
Version: 0.96SVN

Interesting idea Joe, please bring it up on Django-Developers to get some discussion going (this is a backwards incompat. issue after all).

comment:4 by Simon G. <dev@…>, 17 years ago

Summary: Give the Message model a typeGive the Message model a type (success/failure/generic)

comment:5 by joe4444, 17 years ago

Yeah I guess you're right. I was thinking since code for creating new messages doesn't have to change then it is backwards compatible. But changing the Message model requires an update on the db table auth_message.

by joe4444, 17 years ago

Attachment: message_type.3.diff added

I think the others only worked for models.py from 0.96

comment:6 by James Bennett, 17 years ago

I'd be -1 on this because of the backwards incompatibility, but there have been various proposals to tie messages into sessions (for messages to anonymous users), and if we ever do that we should revisit this issue.

comment:7 by Jacob, 16 years ago

Triage Stage: Design decision neededAccepted

I think this is basically a good idea, but there needs to be a transition plan. I'd like to see a post-syncdb handler that automatically upgrades the table (if needed), and I'd like the type to just be a string field (instead of choices). And optional, of course.

comment:8 by xian, 16 years ago

I think type as a slugfield makes sense. That way you can use them as css classes and deal with them in the templates a bit easier. And it would be good to be able to get to them separately in the templates {{ messages }} all messages. {{ messages.warning }} just the messages with type='warning'.

comment:9 by anonymous, 16 years ago

Cc: django@… added

comment:10 by anonymous, 16 years ago

Cc: ross@… added

comment:11 by Pete Crosier, 16 years ago

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Pete Crosier
Patch needs improvement: unset

Patch attached as per Jacob's suggestion - just using a string (limited to 30 chars) rather than choices. Ignoring this patch, I think all calls to user.message_set.create need to use (message="blah") rather than just ("blah") - so I think it's backwards compatible (the type is optional in the patch)?

by Pete Crosier, 16 years ago

Updated messages with a type, with tests and documentation

comment:12 by Pete Crosier, 16 years ago

Quick patch update to use "category" rather than "type" attached, thanks to oebfare for pointing out problems and that the patch is indeed backwards incompatible without doing something post-syncdb.

comment:13 by edrex, 15 years ago

IMO this patch is pretty important since many people are hacking around the limitation. The patch looks good, RTBC?

comment:14 by edrex, 15 years ago

a nitpick, "type" seems more standard than "category"

comment:15 by edrex, 15 years ago

Cc: xerdcire@… added

comment:16 by John Morrissey, 15 years ago

Cc: jwm@… added

comment:17 by m@…, 15 years ago

What happened with this? It says "accepted," there's a patch from 2007 that I assume will no longer work, and there's no commentary beyond that. Are we just waiting on a new patch? I know it's almost 1.1 time, should this be brought up on django-dev during 1.2 feature selection, or was there some conscious decision to abandon this change?

in reply to:  17 comment:18 by Karen Tracey, 15 years ago

Replying to m@mihasya.com:

What happened with this? It says "accepted," there's a patch from 2007 that I assume will no longer work, and there's no commentary beyond that. Are we just waiting on a new patch? I know it's almost 1.1 time, should this be brought up on django-dev during 1.2 feature selection, or was there some conscious decision to abandon this change?

The most recent patch does not address Jacob's transition plan requirement mentioned in http://code.djangoproject.com/ticket/3995#comment:7. Any patch that changes the DB schema for a contrib app has to deal with automatically upgrading existing installs, and near as I can see there has been no effort to do that.

comment:19 by Karen Tracey, 15 years ago

Patch needs improvement: set

by Tim Graham, 15 years ago

Attachment: 3995.diff added

updated patch to apply to trunk, added post_syncdb handler to add field

comment:20 by Tim Graham, 15 years ago

Owner: changed from Pete Crosier to Tim Graham

I've brought the old patch up to date and added a first attempt at a post syncdb handler to add the field in the database. Not sure the best way to detect if the field currently exists in the db (just using try/catch currently). Any suggestions on this or any other aspects of the patch appreciated.

comment:21 by Tim Graham, 14 years ago

Resolution: wontfix
Status: newclosed

Seems like this would be very difficult to pull off due to backwards compatibility, plus the new messages framework in 1.2 has message types.

Note: See TracTickets for help on using tickets.
Back to Top