Add event type filters and custom message templates for I Done This.

Review Request #8961 — Created Oct. 18, 2017 and updated


This change provides some customization for messages posted to I Done This.

Event types can be filtered so that only some kinds of activity get posted,
while others do not. For example, the "Reply published" event can be disabled
to reduce noise in I Done This from every single reply getting published.

The posted messages can also be customized independently for every event type.
An empty message causes a fallback to the default format string, which is shown
below the text field for reference. Parameters such as the review request ID,
summary, and URL are substituted into placeholder strings, or omitted from the
final message. The syntax for placeholders is from string.Template, which is
limited and safer compared to other kinds of string formatting.

New unit tests were added to verify the added functionality, and some existing
tests were updated to test individual event type filtering.

All unit tests pass.

Manually tested event type filtering. Choosing "All events" posts all activity
as before. Choosing a subset of event types only posts those events.

Also specified some custom message template strings and verified that they get
formatted and posted to I Done This, or cause appropriate validation errors.

Loading file attachments...

  • 0
  • 0
  • 22
  • 0
  • 22
Description From Last Updated
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.


  2. I don't know that the word hash clarifies anything.

    1. You're right, that's an implementation detail (in the API they are called hash_id). This really applies to the first review request, so I'll change it there.

  3. rbintegrations/idonethis/ (Diff revision 1)

    Do we want to wrap the template strings with <code> e.g. <dt><code>...</code></dt> ?

    1. Sure, it looks better that way. I also changed from dl to table to lay things out horizontally for clarity.

  1. These are pretty much all just trivial style issues.

  2. rbintegrations/idonethis/ (Diff revision 2)

    You could just have entry_type=None in the arguments list instead of using kwargs.pop here.

  3. rbintegrations/idonethis/ (Diff revision 2)

    Needs the return type (unicode)

  4. rbintegrations/idonethis/ (Diff revision 2)

    Add a blank line after this.

  5. rbintegrations/idonethis/ (Diff revision 2)

    Instead of str(e), this should use six.text_type(e). You can get that by adding from django.utils import six in the imports.

  6. rbintegrations/idonethis/ (Diff revision 2)

    str -> six.text_type. Also add a blank line after this.

  7. rbintegrations/idonethis/ (Diff revision 2)

    Can we rewrap this a bit?

    return [
  8. rbintegrations/idonethis/ (Diff revision 2)

    This can be dedented 4 spaces (there's only ever one return value).

  9. rbintegrations/idonethis/ (Diff revision 2)

    Add a blank line after this.

  10. rbintegrations/idonethis/ (Diff revision 2)

    Add a trailing comma on this line?

  11. rbintegrations/idonethis/ (Diff revision 2)

    Add a blank line after this.

  12. rbintegrations/idonethis/ (Diff revision 2)

    Add a blank line after this.

  13. This should be in the imperative mood ("Render the view").

    This also should have a:

        This object, for chaining.
    1. Some older files in Review Board also need to be updated :)
      I was using reviewboard/static/rb/js/admin/views/webhookFormView.js as a template for this functionality.

  14. If #id_events is a child of #content-main form (which I believe it should be), you can use this.$ for a more efficient query.

  1. One little nit left.

  2. rbintegrations/idonethis/ (Diff revision 3)

    This coment should probably go above the if key.startswith line.

Review request changed

Checks run (2 succeeded)

flake8 passed.
JSHint passed.