• 
      

    Add an integration for Trello.

    Review Request #9385 — Created Nov. 17, 2017 and submitted

    Information

    rbintegrations
    master
    f94bd7c...

    Reviewers

    This change adds a new integration for linking to Trello cards from
    review requests. This consists of a few pieces: the integration config
    which connects to a Trello via a developer key, and a review request
    field which allows people to search for and link to multiple cards.

    The UI is currently fully functional, but at a later date I'd like to
    make it look a bit more like the related object selector used in the
    admin UI, with the search box shown above the list of selected items
    (instead of swapping out the entire UI for what looks more like "tags"
    inside the selectize widget).

    • Tested success and error conditions within the integration config UI.
    • Added a bunch of cards and verified that everything loaded correctly.
    • Saved the field in a variety of states.

    Description From Last Updated

    The one thing I realize I don't have enough of a sense of is how the non-Local Site parts will …

    chipx86chipx86

    I think we really need unit tests for these. In particular, anything involving Local Site/configuration/review request access and potential data …

    chipx86chipx86

    F401 'json' imported but unused

    reviewbotreviewbot

    F401 'django.utils.six.moves.urllib.error.HTTPError' imported but unused

    reviewbotreviewbot

    This could easily be one statement.

    chipx86chipx86

    I thought this was a typo of "list" at first, and was leaving a comment about a possible missing variable …

    chipx86chipx86

    Two blank lines.

    chipx86chipx86

    No blank line here.

    chipx86chipx86

    Missing a docstring. Also, so much of this class is the same as the one in Asana. Worth making a …

    chipx86chipx86

    Swap these.

    chipx86chipx86

    Missing period.

    chipx86chipx86

    Should use logger

    chipx86chipx86

    Ordering is wrong.

    chipx86chipx86
    Checks run (1 failed, 1 succeeded)
    flake8 failed.
    JSHint passed.

    flake8

    david
    Review request changed
    Commit:
    7157fa39f2c3f4530610aa0710228a4a54237811
    fbdb77e8c741f8f1e1d10eec557ad5dea503011f

    Checks run (1 failed, 1 succeeded)

    flake8 failed.
    JSHint passed.

    flake8

    david
    chipx86
    1. 
        
    2. Show all issues

      The one thing I realize I don't have enough of a sense of is how the non-Local Site parts will work in RBCommons. Is there any risk of accessing data from Local Sites using the non-Local Site URL here or in Asana, and can arbitrary Local Sites be plugged into the URLs to access state?

    3. Show all issues

      I think we really need unit tests for these. In particular, anything involving Local Site/configuration/review request access and potential data leakage.

    4. Show all issues

      This could easily be one statement.

    5. Show all issues

      I thought this was a typo of "list" at first, and was leaving a comment about a possible missing variable in the template. Can we maybe call this items?

    6. Show all issues

      Two blank lines.

    7. rbintegrations/trello/fields.py (Diff revision 3)
       
       
       
       
      Show all issues

      No blank line here.

    8. rbintegrations/trello/fields.py (Diff revision 3)
       
       
      Show all issues

      Missing a docstring.

      Also, so much of this class is the same as the one in Asana. Worth making a base class? Or is it going to diverge enough where that's not worth it?

      1. I don't know that it's going to diverge but I also don't know that this is enough to justify creating a new module somewhere.

    9. rbintegrations/trello/integration.py (Diff revision 3)
       
       
       
      Show all issues

      Swap these.

    10. rbintegrations/trello/views.py (Diff revision 3)
       
       
      Show all issues

      Missing period.

    11. rbintegrations/trello/views.py (Diff revision 3)
       
       
      Show all issues

      Should use logger

    12. 
        
    david
    chipx86
    1. 
        
    2. rbintegrations/trello/tests.py (Diff revision 4)
       
       
       
       
      Show all issues

      Ordering is wrong.

    3. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (947db9a)