Add a new integration for I Done This.

Review Request #8776 - Created Feb. 22, 2017 and updated

Michael Udaltsov
rbintegrations
master
12e3ce0...
rbintegrations

I Done This is a service that provides daily status reports with a list of
tasks which are done, planned, or blocked. This integration allows posting
'done' entries to I Done This teams on behalf of users when they publish,
change, or close a review request, and when they publish reviews or replies.

Each posted message includes a description of what the user did, along with
details such as the review request id, summary, and url. Reviewer groups are
also added as #tags for tracking on the I Done This website. The messages are
currently hard-coded, but could become configurable in another change.

The I Done This API gives control over individual user accounts, so each user
has to provide their personal API token on their Review Board account page.
The token is validated via the API before getting encrypted and saved in the
user settings. The list of the user's teams is retrieved from the API the first
time the user needs to post, and is cached to avoid duplicate requests. The
cache expires in 24 hours, or can be deleted manually by re-saving the token.

The administrator has to add an integration configuration for each I Done This
team that posts are allowed to, along with conditions to filter which review
request activity gets posted. Multiple configurations can specify the same team
to allow alternative sets of conditions, but only one message will be posted
to each unique team.

The configuration method is not ideal, since the admin can't verify that the
team is correctly set up without using an API token of a user on that team.
Users also don't have access to the team configurations to choose or even see
which teams or review requests get posted. However, the overall goal is to keep
user settings to a minimum, and let he admin control the configuration. There
may be ways in the future to improve the user experience, for example by adding
UI notifications from the integration when posting to I Done This.

Unit tests have been added to verify all the posted message types, various
reasons for not posting, posts with multiple configurations and team ids,
functionality of the user and admin forms, and team id requests with caching.

Unit tests pass, with full coverage except icon_static_urls().

Manually tested adding integration configurations, setting the user API token,
form validation errors, and posting all message types to I Done This.

Loading file attachments...

  • 2
  • 21
  • 3
  • 26
Description From Last Updated
You may want to replace - with _ in group.name because idt doesn't like tags with dashes (but underscores work ... Barret Rennie Barret Rennie
Why not just use, e.g.: import re #... re.sub(r'\s{2,}', ' ', entry) You can compile the regex with re.compile so ... Barret Rennie Barret Rennie
Review Bot
David Trowbridge
Michael Udaltsov
Review request changed

Change Summary:

Addressed review comments.
Moved integration_mgr.clear_all_configs_cache() to base class for test cases - https://reviews.reviewboard.org/r/8915/
Updated some form test cases to pass data in the constructor and call form.full_clean() to verify that the appropriate clean_<field> method is called.

Commit:

-fd602194eca270644f6d58a1e61c594974805b2f
+12e3ce01ac26185b641a21046d9f69c09b00ad36

Diff:

Revision 2 (+2783)

Show changes

Checks run (2 succeeded, 1 failed with error)

JSHint passed.
PEP8 Style Checker internal error.
Pyflakes passed.
Barret Rennie
  1. 
      
  2. rbintegrations/idonethis/forms.py (Diff revision 2)
     
     

    File-level docstring.

  3. rbintegrations/idonethis/forms.py (Diff revision 2)
     
     

    <b> is deprecated. Use <strong> instead.

  4. rbintegrations/idonethis/forms.py (Diff revision 2)
     
     

    The fieldset label should be marked for translation.

  5. rbintegrations/idonethis/forms.py (Diff revision 2)
     
     

    Here too.

  6. rbintegrations/idonethis/forms.py (Diff revision 2)
     
     

    Maybe use <p> over linebreaks? Also, IDK if we use self-closing tags (they are deprecated in the HTML spec)

    1. Turns out I can just use \n and the fieldset template will turn them into paragraphs. I think I originally only tried it on the field help text, where the template doesn't split paragraphs.

      I've changed the self-closing <br /> to regular <br> on the user account page.

  7. rbintegrations/idonethis/integration.py (Diff revision 2)
     
     

    Missing file-level docstring.

  8. rbintegrations/idonethis/integration.py (Diff revision 2)
     
     

    Must inherit from object.

    1. Moved to a separate module instead of a class.

  9. rbintegrations/idonethis/integration.py (Diff revision 2)
     
     
     
     

    unicode.format is slow compared to using %-based interpolation (and we also don't really use the former elsewhere). You can change these to use:

    format_strings = {
        REPLY_PUBLISHED:
            'Replied to review request %(request_id)s: '
            '%(summary)s %(url)s %(group_tags)s',
    
        #...
    }
    

    and then interpolate with:

    format_strings[entry] % {
        'group_tags': group_tags,
        'num_issues': num_issues,
        'request_id': review_request.display_id,
        'summary': review_request.summary,
        'url': url,
    }
    
    1. After looking more into possible string formatting, both .format() and % have issues with being able to properly validate custom format strings. I've decided to use string.Template instead, which provides very limited named placeholder replacement. It works better for admin-specified format strings.

  10. rbintegrations/idonethis/integration.py (Diff revision 2)
     
     

    You may want to replace - with _ in group.name because idt doesn't like tags with dashes (but underscores work for some reason :/)

    1. Good catch!
      Are there any other special characters allowed in group names that I may need to worry about?

  11. rbintegrations/idonethis/integration.py (Diff revision 2)
     
     

    Why not just use, e.g.:

    import re
    #...
    
    re.sub(r'\s{2,}', ' ', entry)
    

    You can compile the regex with re.compile so we only have to parse the regex once.

    I'm not sure which is more expensive

    1. I was mostly going for simplicity and avoiding additional imports, but I could change to re. The preformance probably doesn't matter since urlopen will be slower than any string operations.

  12. rbintegrations/idonethis/integration.py (Diff revision 2)
     
     
     

    Coalesce into elif

  13. rbintegrations/idonethis/pages.py (Diff revision 2)
     
     

    File-level docstring.

  14. rbintegrations/idonethis/tests.py (Diff revision 2)
     
     

    File-level docstring.

  15. rbintegrations/idonethis/utils.py (Diff revision 2)
     
     

    File-level docstring.

Loading...