This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/webapi/resources.py
    reviewboard/reviews/templatetags/reviewtags.py
  Ignored Files:

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

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

I think it is better to prepare a variable's value first then return the variable, instead of having a if statement within return. 

Can a specific user be deleted? I doubt that. 

*Should* always succeed, but that doesn't mean it will. Always check and log.

Nitpick, but comments should always end with a period. (And be valid sentences.)

This should be using comment.get_review(), which we also call below. We should only fetch that once.

I feel we're putting too much business logic now in the template tag. Ideally, this should all be determined in a can_change_issue_status() function in the BaseComment model.

We're determining a bool and then later converting to a string. Just do an if statement here and set interactive to be 'true' or 'false' from that.

You're doing this twice.

This logic is different from the above logic.

Don't check this stuff here, or compute comment_creator, or any of that. Instead, use the aforementioned new function.

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/webapi/resources.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/reviews/models.py
  Ignored Files:

Col: 16
 E128 continuation line under-indented for visual indent

Col: 16
 E128 continuation line under-indented for visual indent

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/webapi/resources.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/reviews/models.py
  Ignored Files:

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/webapi/tests.py
    reviewboard/webapi/resources.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/reviews/models.py
  Ignored Files:

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

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

Docstrings must follow this convention:

    """One-line summary

    Multi-line description.
    """

or:

    """One-line summary"""

Line must be < 80 chars.

This information should be explained in the docstring.

A tip that Django won't tell you about but will save a query is to not compare against things like review_request.submitter. Instead, compare the IDs ('pk' variable). So:

    if user.pk == self.get_review().user_id:


On top of this, the users_allowed_change trick is cute, but inefficient. We'll fetch both objects up-front, meaning we introduce 2 SQL queries, always. Instead, we should check each one individually, so that optimistically, we only have one query.

We should start the check with the user that is most likely going to be making the change. For most uses out there, this is going to be the review request's submitter, so check that first.

The first two checks are free, and the third can have up to 3 queries (for review, review request, and submitter). We should do the first two checks first, and if they fail, bail.

Not a fan of this syntax. It's a bit weird to read, and it makes it harder to add any logic to that without awkward wrapping. IN general, I prefer a standard if/else instead.

Now in this case, the fact that this was computing "true" and "false" was wrong. That means this code knows exactly how it's going to be used in the template (as a JavaScript value). Instead, this should pass the bool, and the template should be converting to the right value.

"a user"

This is a new test.

Trailing period.

Also a new test.

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/webapi/tests.py
    reviewboard/webapi/resources.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/reviews/models.py
  Ignored Files:
    reviewboard/templates/reviews/comment_issue.html

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Looking good, but I want to go for more consistency with the new tests and our existing ones. While your new ones are very descriptive, we don't really need that level of descriptiveness in the test strings.

What I'd recommend is having all the extra precondition/postcondition explanation be comments at the top of the function. That keeps the test output more scannable, and yet makes it more clear what's expected in the test functions when they're read (if something fails, for instance).

Too much copy/paste. Let's limit the check to where we're putting the boolean. (It's okay for that to exceed 80 chars.)

We don't usually have a whole post-condition description, since that makes the test output extremely long. I'd say just do "... with trying to change an unpublished issue"

"... with owner trying to change a published issue"

This is a new test.

"... with changing an issue and no permission"

Similar comments to above.

In fact, at this point, all my previous comments repeat.

I've gone through and created an updated version of this change at https://reviews.reviewboard.org/r/4380/