• 
      

    Ensure that e-mails always include absolute URLs for image comments.

    Review Request #7688 — Created Oct. 9, 2015 and submitted

    Information

    Review Board
    release-2.0.x

    Reviewers

    The comment thumbnails (selected regions) for image file attachments are
    included in e-mail notifications for the review, but when using the built-in
    file storage backend, the URLs given to these are relative to the server. This
    change makes it so if the returned URL is relative, we prefix it with the
    server URL.The problem was that when someone was reviewing an image posted it would not provide a server URL.
    I changed image ui, so that when you review an image it now adds the server URL.

    I used the email backend so that I could print emails to the console.
    Then I posted a review with an image attached and posted a review for the image.
    Also, I ran the unit tests and it passed them.

    Description From Last Updated

    These should be sorted in alphabetical order.

    daviddavid

    Please put this line back.

    daviddavid

    So in the case of the built-in file storage, this will work, but if someone is hosting on a service …

    daviddavid

    Col: 80 E501 line too long (89 > 79 characters)

    reviewbotreviewbot
    david
    1. Somehow you deleted the entire reviewboard/attachments/mimetypes.py file. Please undo that.

      1. Barret helped me undo this

    2. reviewboard/reviews/ui/image.py (Diff revision 1)
       
       
       
      Show all issues

      These should be sorted in alphabetical order.

    3. reviewboard/reviews/ui/image.py (Diff revision 1)
       
       
      Show all issues

      Please put this line back.

    4. reviewboard/reviews/ui/image.py (Diff revision 1)
       
       
      Show all issues

      So in the case of the built-in file storage, this will work, but if someone is hosting on a service like S3, crop_image will return an absolute URL to some third-party server.

      We should get the URL, and check to see if it's an absolute URL or a relative path. If it's the latter, then we can wrap it in build_server_url.

    5. 
        
    DA
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/reviews/ui/image.py
          reviewboard/attachments/mimetypes.py
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/reviews/ui/image.py
          reviewboard/attachments/mimetypes.py
      
      
    2. reviewboard/attachments/mimetypes.py (Diff revision 2)
       
       
      Show all issues
      Col: 80
       E501 line too long (89 > 79 characters)
      
    3. 
        
    DA
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/reviews/ui/image.py
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/reviews/ui/image.py
      
      
    2. 
        
    DA
    david
    1. Your summary and description could use some improvement. How about something like this:?

      Ensure that e-mails always include absolute URLs for image comments.
      
      The comment thumbnails (selected regions) for image file attachments are
      included in e-mail notifications for the review, but when using the built-in
      file storage backend, the URLs given to these are relative to the server. This
      change makes it so if the returned URL is relative, we prefix it with the
      server URL.
      
    2. 
        
    DA
    brennie
    1. Ship It!
    2. 
        
    david
    1. Ship It!
    2. 
        
    DA
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-2.0.x (ddfa202)