• 
      

    Add some features and UI polish for Webhooks.

    Review Request #6391 — Created Oct. 1, 2014 and submitted

    Information

    Review Board
    master
    bffb925...

    Reviewers

    Now that we have support for webhooks in Review Board, it's time to make
    them completely awesome and flexible, without being annoyingly hard to
    use.

    Previously, our webhooks were global across repositories and were forced
    into pre-built JSON payloads. This is pretty par-for-the-course for
    webhook implementations, but a few implementations out there had some
    nice features that were worth bringing in.

    Now, webhooks can be tied to one or more repositories, can be set to
    apply only for events not matching a repository (such as review requests
    that are only used for file uploads), or can be set to always fire
    regardless of whether there's a repository.

    They can be set to output JSON, XML, or form data payloads (in which
    case there will be a payload=<encoded form data> sent to the server.

    Payloads are also no longer limited to what we generate. Users can
    toggle an option for specifying a custom payload, which gives them a
    CodeMirror widget they can edit. This supports the Django template
    format, allowing the use of template tags, with the original payload
    provided as the variable context. Several scary tags (such as include,
    load, etc.) are disabled, and RequestContext is not used, keeping this
    safe.

    The webhook UI has been updated to provide a bit more guidance, especially
    with the new options.

    There were a few changes under the hood as well. Instead of a
    X-ReviewBoard-Signature, we now use the more standard X-Hub-Signature
    (followed by GitHub and others). We also provide a proper User-Agent.

    Added unit tests for all the conditions.

    Played around with creating some webhooks.

    Database evolved successfully.


    Description From Last Updated

    "Without repositories" is kind of weird. How about "...not associated with a repository (file attachments only)"?

    daviddavid

    This should link to documentation on what to put in here.

    daviddavid

    local variable 'target1' is assigned to but never used

    reviewbotreviewbot

    local variable 'target2' is assigned to but never used

    reviewbotreviewbot

    local variable 'target1' is assigned to but never used

    reviewbotreviewbot

    local variable 'target3' is assigned to but never used

    reviewbotreviewbot

    local variable 'target3' is assigned to but never used

    reviewbotreviewbot

    local variable 'target2' is assigned to but never used

    reviewbotreviewbot

    local variable 'target3' is assigned to but never used

    reviewbotreviewbot

    'Q' imported but unused

    reviewbotreviewbot

    'Template' imported but unused

    reviewbotreviewbot

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

    reviewbotreviewbot

    Can you use the word "not" instead of "blocks"? I was initially kind of confused because I read it as …

    daviddavid
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/notifications/evolutions/webhooktarget_extra_state.py
          reviewboard/notifications/models.py
          reviewboard/notifications/tests.py
          reviewboard/notifications/admin.py
          reviewboard/notifications/evolutions/__init__.py
          reviewboard/staticbundles.py
          reviewboard/notifications/webhooks.py
          reviewboard/notifications/forms.py
      
      Ignored Files:
          reviewboard/static/rb/js/webhooksAdmin/views/webhookFormView.js
          reviewboard/static/rb/css/pages/admin.less
          reviewboard/templates/admin/notifications/webhooktarget/change_form.html
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/notifications/evolutions/webhooktarget_extra_state.py
          reviewboard/notifications/models.py
          reviewboard/notifications/tests.py
          reviewboard/notifications/admin.py
          reviewboard/notifications/evolutions/__init__.py
          reviewboard/staticbundles.py
          reviewboard/notifications/webhooks.py
          reviewboard/notifications/forms.py
      
      Ignored Files:
          reviewboard/static/rb/js/webhooksAdmin/views/webhookFormView.js
          reviewboard/static/rb/css/pages/admin.less
          reviewboard/templates/admin/notifications/webhooktarget/change_form.html
      
      
    2. reviewboard/notifications/tests.py (Diff revision 1)
       
       
      Show all issues
       local variable 'target1' is assigned to but never used
      
    3. reviewboard/notifications/tests.py (Diff revision 1)
       
       
      Show all issues
       local variable 'target2' is assigned to but never used
      
    4. reviewboard/notifications/tests.py (Diff revision 1)
       
       
      Show all issues
       local variable 'target1' is assigned to but never used
      
    5. reviewboard/notifications/tests.py (Diff revision 1)
       
       
      Show all issues
       local variable 'target3' is assigned to but never used
      
    6. reviewboard/notifications/tests.py (Diff revision 1)
       
       
      Show all issues
       local variable 'target3' is assigned to but never used
      
    7. reviewboard/notifications/tests.py (Diff revision 1)
       
       
      Show all issues
       local variable 'target2' is assigned to but never used
      
    8. reviewboard/notifications/tests.py (Diff revision 1)
       
       
      Show all issues
       local variable 'target3' is assigned to but never used
      
    9. reviewboard/notifications/webhooks.py (Diff revision 1)
       
       
      Show all issues
       'Q' imported but unused
      
    10. reviewboard/notifications/webhooks.py (Diff revision 1)
       
       
      Show all issues
       'Template' imported but unused
      
    11. reviewboard/notifications/webhooks.py (Diff revision 1)
       
       
      Show all issues
      Col: 80
       E501 line too long (80 > 79 characters)
      
    12. 
        
    chipx86
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/notifications/evolutions/webhooktarget_extra_state.py
          reviewboard/notifications/models.py
          reviewboard/notifications/tests.py
          reviewboard/notifications/admin.py
          reviewboard/notifications/evolutions/__init__.py
          reviewboard/staticbundles.py
          reviewboard/notifications/webhooks.py
          reviewboard/notifications/forms.py
      
      Ignored Files:
          reviewboard/static/rb/js/webhooksAdmin/views/webhookFormView.js
          reviewboard/static/rb/css/pages/admin.less
          reviewboard/templates/admin/notifications/webhooktarget/change_form.html
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/notifications/evolutions/webhooktarget_extra_state.py
          reviewboard/notifications/models.py
          reviewboard/notifications/tests.py
          reviewboard/notifications/admin.py
          reviewboard/notifications/evolutions/__init__.py
          reviewboard/staticbundles.py
          reviewboard/notifications/webhooks.py
          reviewboard/notifications/forms.py
      
      Ignored Files:
          reviewboard/static/rb/js/webhooksAdmin/views/webhookFormView.js
          reviewboard/static/rb/css/pages/admin.less
          reviewboard/templates/admin/notifications/webhooktarget/change_form.html
      
      
    2. 
        
    david
    1. 
        
    2. Show all issues

      "Without repositories" is kind of weird. How about "...not associated with a repository (file attachments only)"?

    3. Show all issues

      This should link to documentation on what to put in here.

      1. Agreed. First we need to write documentation though. I'll track this in Asana for 2.1.

    4. reviewboard/notifications/webhooks.py (Diff revision 2)
       
       
      Show all issues

      Can you use the word "not" instead of "blocks"? I was initially kind of confused because I read it as "block tags"

      1. Yeah, that's confusing. Can do.

    5. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/notifications/evolutions/webhooktarget_extra_state.py
          reviewboard/notifications/models.py
          reviewboard/notifications/tests.py
          reviewboard/notifications/admin.py
          reviewboard/notifications/evolutions/__init__.py
          reviewboard/staticbundles.py
          reviewboard/notifications/webhooks.py
          reviewboard/notifications/forms.py
      
      Ignored Files:
          reviewboard/static/rb/js/webhooksAdmin/views/webhookFormView.js
          reviewboard/static/rb/css/pages/admin.less
          reviewboard/templates/admin/notifications/webhooktarget/change_form.html
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/notifications/evolutions/webhooktarget_extra_state.py
          reviewboard/notifications/models.py
          reviewboard/notifications/tests.py
          reviewboard/notifications/admin.py
          reviewboard/notifications/evolutions/__init__.py
          reviewboard/staticbundles.py
          reviewboard/notifications/webhooks.py
          reviewboard/notifications/forms.py
      
      Ignored Files:
          reviewboard/static/rb/js/webhooksAdmin/views/webhookFormView.js
          reviewboard/static/rb/css/pages/admin.less
          reviewboard/templates/admin/notifications/webhooktarget/change_form.html
      
      
    2. 
        
    david
    1. Ship It!

    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (2668e0e)