Opened 13 years ago
Last modified 13 years ago
#83 closed defect (FIXED)
Messaging framework
Reported by: | Christopher Allan Webber | Owned by: | Caleb Davis |
---|---|---|---|
Priority: | minor | Milestone: | 0.0.3 |
Component: | programming | Keywords: | |
Cc: | Parent Tickets: |
Description
There's been some discussion of a messaging framework in `http://bugs.foocorp.net/issues/366#note-5 <http://bugs.foocorp.net/issues/366#note-5>`_ and `http://bugs.foocorp.net/issues/314#note-1 <http://bugs.foocorp.net/issues/314#note-1>`_ It would be good if we had such a system. Simply queueing messages in the session and then iterating through them in the interface is probably good enough. We could have 3 simple types of message: - warning message (red) - notification neutral message (yellow) - happy success message (green) We could iterate through the messages at the top of the document before we show the rest of the page. We could maybe also provide a way with javascript to be able to "close" the messages though that's lower priority for this ticket. See also: `http://schendje.fedorapeople.org/temp/temp3.png <http://schendje.fedorapeople.org/temp/temp3.png>`_ `http://bugs.foocorp.net/attachments/48/temp.png <http://bugs.foocorp.net/attachments/48/temp.png>`_
Change History (11)
comment:2 by , 13 years ago
shoot, I had to re-install my OS tonight, and lost the suggested code snippets from IRC. Please paste the conversation, or throw out a few lines for me to build from. I think I could do this one with a little kick-start.
comment:3 by , 13 years ago
Owner: | set to |
---|
So I think several steps should happen for this ticket: - make a new module, mediagoblin/messages.py - add global variables; this is mostly so we can have a canonical list of message types - DEBUG = 'debug' - INFO = 'info' - SUCCESS = 'success' - WARNING = 'warning' - ERROR = 'error' - Add a function in this module for adding a message, messages.add\_message(request, level, text). Something like: def add\_message(request, level, text): messages = request.session.setdefault('messages', []) messages.append({'level': level, 'text': text}) request.session.save() - Add a function messages.fetch\_messages(request, clear=True) Something like: :: def fetch_messages(request, clear_from_session=True): messages = request.session.get('messages') if messages and clear_from_session: # Save that we removed the messages from the session request.session['messages'] = [] request.session.save() return messages - Add the fetch\_messages function to the template environment globals so that function is always available. See: `http://jinja.pocoo.org/docs/api/#jinja2.Environment.globals <http://jinja.pocoo.org/docs/api/#jinja2.Environment.globals>`_ and wherever the environment is set up in util.py - Add to the templates/mediagoblin/utils/messages.html which includes some kind of macro that fetches any new messages that exist and displays them. It should maybe make a .. raw:: html <ul id="messages"> and each message is a .. raw:: html <li> that has the class set to the level name. (Or this could be just a template that you {% include %} instead? Up to you, really.) - Add tests Does this make sense? Thanks for working on this!
comment:4 by , 13 years ago
Adjusting to the flexible namespace in python is taking some time. I just learned in IRC something about jinja2 and want to record it here for myself. I am paraphrasing: When you render a template in jinja2 it does something like :: this_template.render({'myvar': 'thisvalue'}) then you can access that in the template. :: My variable is {{ myvar }} would render :: My variable is thisvalue Similarly you can pass in functions to the context, so you could use that inside the template macro. But you don't want every function ever to have to pass in that method, since theroretically all sorts of templates might use that method without even thinking about it. So a solution is in util.get\_jinja\_env right before :: if exists(locale): do this: :: template_env.globals['fetch_messages'] = messages.fetch_messages
comment:5 by , 13 years ago
Also, the tests to implement should, at a minimum: - see if you can add messages and they show up in the request.session - see if you get out that list while doing the fetch method - that it clears things if that value set
comment:6 by , 13 years ago
okay, so after much gnashing of teeth and ritual sacrifice I have a first pass at this here - [https://gitorious.org/\ :sub:`cfdv/mediagoblin/cfdvs-mediagoblin/commit/2264670357754ae546ed69e3c825da8860b372aa](https://gitorious.org/`\ cfdv/mediagoblin/cfdvs-mediagoblin/commit/2264670357754ae546ed69e3c825da8860b372aa) I like the global template context abstraction between the templates and the messaging module, now that I understand [more of] it. :) I decided to {% include %} the messages in base.html because that seemed the easiest thing to do. Let me know if using a macro or putting the include somewhere else would make more sense. Also, I'm using :: {% set foo = bar(request) %} which I haven't seen anywhere else in the codebase, so I'm worried that's not quite the best thing to do. I still need to do testing, but I would appreciate any feedback on what I have so far.
comment:7 by , 13 years ago
Looking great! :D One comment: :: template_env.globals['add_message'] = messages.add_message is not needed as the only thing templates need is to fetch messages... adding messages should be done by views.
comment:8 by , 13 years ago
well whaddya-know?! :: assert status(368) == closed [https://gitorious.org/\ :sub:`cfdv/mediagoblin/cfdvs-mediagoblin/commits/f368\_msg\_framework](https://gitorious.org/`\ cfdv/mediagoblin/cfdvs-mediagoblin/commits/f368\_msg\_framework)
comment:9 by , 13 years ago
Thanks Caleb! I'll hopefully look at this and merge it tonight. Looking good from a brief scan!
comment:10 by , 13 years ago
Status: | New → Closed |
---|
Merged! Great job Caleb! I also added some styling, as you can see here, for each message type: `http://dustycloud.org/tmp/mediagoblin\_messages\_rainbow.png <http://dustycloud.org/tmp/mediagoblin_messages_rainbow.png>`_
comment:11 by , 13 years ago
The original url for this bug was http://bugs.foocorp.net/issues/368 .
Note:
See TracTickets
for help on using tickets.