Enhancements to diff section of review request emails.
Review Request #8237 — Created June 11, 2016 and submitted
This adds three new/modified features to the diff section of review request emails:
- The diff section is now omitted for review requests which do not actually contain diffs. This has the effect of also preventing the View Diff link, which for the no diffs case was previously a link to a page that did not exist.
- Update View Diff link to point to the specific version of the diff that is relevant at the time that the email is produced, rather than always referencing the most recent version of the diff. The benefit here is that the link will take you to the diff version that is pertinent relative to the email, even if subsequent diffs have been posted.
- Add Show Changes link to provide a shortcut to the relevant interdiff. This mimics the links available in a "Review request changed" element in the main review request UI when an updated diff is posted. Correspondingly, the link is included in emails for all requests where an updated diff has been posted, and the diff revision is greater than one.
Manually tested all new features and examined resulting emails.
- For item 1, tested review requests both with and without diffs.
- For item 2, tested review request with multiple diff revisions and verified that View Diff link corresponded to the appropriate specific revision number.
- For item 3, tested that Show Changes link did not appear for diff revision 1, but did appear for subsequent revisions and had the correct interdiff range.
| Description | From | Last Updated |
|---|---|---|
|
Can you post screenshots of this? You can use the preview_review_request_email view at http://localhost:8080/r/<id>/preview-email/html/ and http://localhost:8080/r/<id>/preview-email/text/. |
 brennie
|
|
|
Same here. |
brennie
|
|
|
The with statement should be above the for loop above so that we're not binding the same variable for each … |
brennie
|
-
Posting this to gauge interest in these changes. While I think all three are worthwhile, I have listed them in what I believe is the order of importance. I haven't done a lot with the Django template language, so I'd welcome feedback on that front. Also, I don't know that I've used the most optimal approach for determining if a review request contains a diff.
There is also one open issue in my mind that I would appreciate guidance on. The View Diff link is unconditionally included in emails. I've followed that approach for Show Changes as well (assuming we are at diff revision >1 where interdiffs actually come into play). But, I think it might make more sense to only include Show Changes when
rev > 1 and changes.diff-- that is when the diff has in fact changed. Thoughts?
-
-
Can you post screenshots of this?
You can use the
preview_review_request_emailview athttp://localhost:8080/r/<id>/preview-email/html/andhttp://localhost:8080/r/<id>/preview-email/text/. -
-
The
withstatement should be above theforloop above so that we're not binding the same variable for each filediff.
- Change Summary:
-
Rebase onto release-2.5.x.
- Branch:
-
masterrelease-2.5.x
- Commit:
-
e269a45f6bcc6d5c355cfa21ea727fbc2937cb9431b62a304e49120d0b01c5f33c4b02b6312c9a31
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/notifications/review_request_email.html reviewboard/templates/notifications/review_request_email.txt Tool: Pyflakes Ignored Files: reviewboard/templates/notifications/review_request_email.html reviewboard/templates/notifications/review_request_email.txt
- Change Summary:
-
Only include Show Changes link in emails when a new diff is posted.
- Description:
-
This adds three new/modified features to the diff section of review request emails:
- The diff section is now omitted for review requests which do not actually contain diffs. This has the effect of also preventing the View Diff link, which for the no diffs case was previously a link to a page that did not exist.
- Update View Diff link to point to the specific version of the diff that is relevant at the time that the email is produced, rather than always referencing the most recent version of the diff. The benefit here is that the link will take you to the diff version that is pertinent relative to the email, even if subsequent diffs have been posted.
~ - Add Show Changes link to provide a shortcut to the relevant interdiff. This mimics the links available in a "Review request changed" element in the main review request UI when an updated diff is posted. The link is included for all diff revisions beyond the first.
~ - Add Show Changes link to provide a shortcut to the relevant interdiff. This mimics the links available in a "Review request changed" element in the main review request UI when an updated diff is posted. Correspondingly, the link is included in emails for all requests where an updated diff has been posted, and the diff revision is greater than one.
- Commit:
-
31b62a304e49120d0b01c5f33c4b02b6312c9a314eaa5f1f3cc21345824de2fcbd0247938c9ad1d9
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/notifications/review_request_email.html reviewboard/templates/notifications/review_request_email.txt Tool: Pyflakes Ignored Files: reviewboard/templates/notifications/review_request_email.html reviewboard/templates/notifications/review_request_email.txt
Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/notifications/review_request_email.html reviewboard/templates/notifications/review_request_email.txt Tool: Pyflakes Ignored Files: reviewboard/templates/notifications/review_request_email.html reviewboard/templates/notifications/review_request_email.txt