#6777 Create a site-wide notification mechanism

v1.1.0
closed
Cory Johns
sf-2 (994)
General
Dave Brondsema
2015-08-20
2013-10-18
Cory Johns
No

It's sometimes useful to present some information to all users in a semi-persistent fashion, related to the site as a whole. Create a mechanism to store and present these notifications.

The notifications should be semi-persistent, unlike the existing flash messages. The notifications should be closable; once closed, they should not show up for that user again until a new message is in effect. The notifications should also support a (per message?) configurable number of impressions per user; after the user has seen the message the specified number of times, it should no longer be shown to that user.

Discussion

  • Cory Johns - 2013-10-18
    • status: in-progress --> code-review
     

  • Cory Johns - 2013-10-18

    allura:cj/6777

     

  • Dave Brondsema

    Dave Brondsema - 2013-10-21
    • Milestone: forge-oct-18 --> forge-nov-01
     

  • Dave Brondsema

    Dave Brondsema - 2013-10-21
    • QA: Dave Brondsema
     

  • Dave Brondsema

    Dave Brondsema - 2013-10-21
    • status: code-review --> in-progress
     

  • Dave Brondsema

    Dave Brondsema - 2013-10-21

    The mongo index should be on (deleted, _id) for the current() query to be fastest. And since this will run on every pageview, we need it to be very fast.

    Setting a cookie for each notification's impression count and also when it's been closed seems excessive. That could add up to a lot of cookies. We could have just one cookie per notification. Or probably better would be to store it on user session which is persisted as just a single cookie. And we can keep that small by popping off the old notifications' data. One problem: closing a notification is done all client-side in this implementation. We'd have to make an AJAX call if we stored it in the session. Hybrid could be a cookie for closed notification and impressions stored on the session. Or, assuming we have only one current notification, we could do it all with cookies but just one or two, using the same cookie name all the time and just updating the value to match the current notification id.

    Since there's no obvious administration mechanism for this, we should add to docs/administration.rst explaining how to manually create a notification entry.

     

  • Cory Johns - 2013-10-21

    I assumed we would only have one active notification at a time and that we wouldn't be changing them all that often. I did consider putting it in the user session, but figured that they would fill up the user session faster than the multiple cookies would cause a problem for the browser.

    I can switch it to use a single cookie, though, since the query logic already assumes only one active notification at a time.

     

  • Dave Brondsema

    Dave Brondsema - 2013-10-21

    I agree with your assumptions. Eventually they will accumulate though. And if it's in the session we can pop the old ones off. I guess we could do that with cookies too. Either way, a bit of cleanup I think is appropriate.

     

  • Cory Johns - 2013-10-21
    • status: in-progress --> code-review
     

  • Cory Johns - 2013-10-21

    Rebased and force-pushed to:
    allura:cj/6777

     

  • Dave Brondsema

    Dave Brondsema - 2013-10-22
    • status: code-review --> in-progress
     

  • Dave Brondsema

    Dave Brondsema - 2013-10-22

    allura.tests.functional.test_admin:TestExport.test_access among others fails with:

      vim +20   allura/templates/login.html  # top-level template code
    {% extends g.theme.master %}
  vim +80   allura/templates/jinja_master/master.html  # top-level template code
    {{theme_macros.site_notification()}}
  vim +127  allura/templates/jinja_master/theme_macros.html  # template
    {% set note = g.theme.get_site_notification() %}
  vim +873  allura/lib/plugin.py  # get_site_notification
    note = SiteNotification.current()
  vim +638  allura/model/notification.py  # current
    if not note.active:
AttributeError: 'NoneType' object has no attribute 'active'
     

  • Cory Johns - 2013-10-22

    Fixup pushed (please squash)

     

  • Cory Johns - 2013-10-22
    • status: in-progress --> code-review
     

  • Dave Brondsema

    Dave Brondsema - 2013-10-25
    • status: code-review --> closed
     

Log in to post a comment.