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:1 by Caleb Davis, 13 years ago

I am wondering if this would help -
`http://pypi.python.org/pypi/FlashMessage/0.2.0 <http://pypi.python.org/pypi/FlashMessage/0.2.0>`_

EDIT: there is also this -
`http://blog.ianbicking.org/2008/12/17/javascript-status-message-display/ <http://blog.ianbicking.org/2008/12/17/javascript-status-message-display/>`_



comment:2 by Caleb Davis, 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 Christopher Allan Webber, 13 years ago

Owner: set to Caleb Davis
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 Caleb Davis, 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 Caleb Davis, 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 Caleb Davis, 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 Christopher Allan Webber, 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 Caleb Davis, 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 Christopher Allan Webber, 13 years ago

Thanks Caleb! I'll hopefully look at this and merge it tonight.
Looking good from a brief scan!



comment:10 by Christopher Allan Webber, 13 years ago

Status: NewClosed
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 Will Kahn-Greene, 13 years ago

The original url for this bug was http://bugs.foocorp.net/issues/368 .

Note: See TracTickets for help on using tickets.