• 
      

    Fix some issues with avatar services

    Review Request #8257 — Created June 22, 2016 and submitted

    Information

    Review Board
    release-3.0.x
    81b327e...

    Reviewers

    The avatar services code has been cleaned up. Docstrings have been
    touched up so that they now document all parameters and do not document
    unnecessary ones. The RB.ReviewReplyEditorView now renders the avatar
    with a srcset= attribute instead of the data-at2x attribute, which
    is no longer used. Finally, a new template tag, {% avatar_urls %}, has
    been introduced to serialize all avatar URLs for a given user and
    requsested size to a Javascript object. This prevents KeyErrors from
    being raised when avatar services only provide 1x resolution avatars,
    as well as provides all additional resolutions that they can provide.
    (Previously, we were hardcoding 1x and 2x resolutions.)

    • Able to use an avatar service (the file upload avatar service, in this
      case) that only provides 1x resolution avatars.
    • Ran unit tests.
    Description From Last Updated

    Did you test this? It seems odd to me to be returning this given that this is an inclusion tag.

    daviddavid

    Hmm, actually. Is it really worth rendering a whole new template for this instead of just returning json.dumps(urls) as a …

    daviddavid

    'six' imported but unused

    reviewbotreviewbot

    'escapejs' imported but unused

    reviewbotreviewbot

    Shouldn't this be from django.utils import six?

    daviddavid

    This looks like it will have an extra closing brace in the javascript and an extra > in the HTML.

    daviddavid
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
          reviewboard/templates/avatars/urls.js
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
          reviewboard/templates/avatars/urls.js
      
      
    2. 
        
    david
    1. 
        
    2. Show all issues

      Did you test this? It seems odd to me to be returning this given that this is an inclusion tag.

      1. Oops I did not test this case.

    3. 
        
    brennie
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/templatetags/tests.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
          reviewboard/templates/avatars/urls.js
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/templatetags/tests.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
          reviewboard/templates/avatars/urls.js
      
      
    2. 
        
    david
    1. 
        
    2. Show all issues

      Hmm, actually. Is it really worth rendering a whole new template for this instead of just returning json.dumps(urls) as a string?

    3. 
        
    brennie
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/templatetags/tests.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/templatetags/tests.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
      
      
    2. Show all issues
       'six' imported but unused
      
    3. Show all issues
       'escapejs' imported but unused
      
    4. 
        
    brennie
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/templatetags/tests.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/templatetags/tests.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
      
      
    2. 
        
    david
    1. 
        
    2. reviewboard/avatars/templatetags/avatars.py (Diff revision 4)
       
       
       
      Show all issues

      Shouldn't this be from django.utils import six?

    3. Show all issues

      This looks like it will have an extra closing brace in the javascript and an extra > in the HTML.

    4. 
        
    brennie
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/templatetags/tests.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/avatars/templatetags/avatars.py
          reviewboard/avatars/templatetags/tests.py
          reviewboard/avatars/settings.py
      
      Ignored Files:
          reviewboard/static/rb/js/views/reviewReplyEditorView.js
          reviewboard/templates/base.html
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    brennie
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-3.0.x (d15c7aa)