• 
      

    Display a pop-up bubble for updates/reviews on review requests.

    Review Request #900 — Created June 23, 2009 and submitted

    Information

    Review Board SVN (deprecated)
    trunk

    Reviewers

    Display a pop-up bubble for updates/reviews on review requests.
    
    Today, in order to see that there are updates to a review request, you
    have to either keep reloading the review request, reloading the
    dashboard, or checking e-mail. These are not very efficient methods.
    
    This change introduces a gmail-style popup bubble that appears when
    there are new updates to a review request (and specifically indicates if
    there's a new diff), or new reviews/replies. We check every 5 minutes.
    The user can choose to ignore the notification or reload the page.
    
    The logic for this is implemented in a reviewrequests/<id>/last-update/
    API call that can return the last update to a review request and who
    made that update. This can also be useful outside of this change for
    third-party integration.
    
    When on the diff viewer page, we only look for updates to diffs. The idea
    is that the user is interested at that point in the diff, and would want to
    know if they should stop reviewing the current version. Other changes, such
    as replies, are not as interesting at that point.
    Tested this on the review request and diff viewer pages. Screenshot pages don't have this functionality, as I don't think it's really as important on that page (though this could be implemented if need be).
    
    I loaded the pages and then updated various things. The review request last_updated field, uploaded a new diff, reviewed using another account, and replied. In each case I got the bubbles I expected.
    
    On the diff viewer page, the only thing that showed updates were the diff updates.
    chipx86
    GR
    1. 
        
    2. /trunk/reviewboard/htdocs/media/rb/js/reviews.js (Diff revision 1)
       
       
       
       
       
       
       
      it's usually faster and more readable to create complete html from a string rather than creating static html from elements on the fly. Readers must guess what's the resulting html should be.
      1. Yes but we need to hold on to these elements, so either we build it like this, or we build as a string and then try to grab the right elements out of it. Could do that, sure, but this is a one-time operation per page and there's not much here, so speed is not an issue.
      2. agree
    3. 
        
    david
    1. Most of this code looks good but I'd like to hear a little more about testing and see some screenshots.
      
      What I'd like to hear/see most is what happens as multiple notifications queue up.
      1. Not sure what testing you want to hear about beyond what I wrote.
        
        There's only one notification ever shown at a time. Most recent one is displayed, as we want to show the most recent update.
        
        I'll attach screenshots.
    2. 
        
    chipx86
    chipx86
    Review request changed
    david
    1. Looks good.
    2.