Merge lp:~thumper/launchpad/fix-inline-diff-encoding into lp:launchpad
Proposed by
Tim Penhey
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Michael Nelson | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | not available | ||||
Proposed branch: | lp:~thumper/launchpad/fix-inline-diff-encoding | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
175 lines 5 files modified
lib/lp/code/browser/codereviewcomment.py (+34/-2) lib/lp/code/browser/tests/test_branchmergeproposal.py (+37/-0) lib/lp/code/mail/codereviewcomment.py (+5/-3) lib/lp/code/mail/tests/test_codereviewcomment.py (+3/-2) lib/lp/code/templates/codereviewcomment-body.pt (+2/-2) |
||||
To merge this branch: | bzr merge lp:~thumper/launchpad/fix-inline-diff-encoding | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michael Nelson (community) | release-critical | Approve | |
Michael Hudson-Doyle | Approve | ||
Review via email:
|
Commit message
The inline diffs are now decoded appropriately and rendered as a diff.
To post a comment you must log in.
= Summary =
Fix bug 414852. This is due to the diff text of the emailed attachments not
being decoded correctly.
== Proposed fix ==
Decorate the attachment and decode the text that will be rendered.
== Implementation details ==
First added a test that replicates the current oops in_attachment_ renders) , although this found another error where
(test_unicode_
the attachments that were being emailed out were not being encoded correctly.
The production emailer probably handles this, but the test one doesn't like
non-ascii. This was fixed in the code review comment mailer. Instead of
directly adding message parts to be added as attachments, it now calls the
controller addAttachment method which does optimistic encoding of the
attachment.
Then changed the view to use the DiffAttachment class as it decodes the diff
text, and changed the template to use that. Added the other test to confirm
the decoding.
Finally changed the rendering of the diff from fmt:nice_pre to fmt:diff.
== Tests ==
TestCommentAtta chmentRendering
== Demo and Q/A ==
make harness is your friend. You can copy the test case example and look at
it in the web browser.
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files: code/mail/ codereviewcomme nt.py code/templates/ codereviewcomme nt-body. pt code/browser/ tests/test_ branchmergeprop osal.py code/browser/ codereviewcomme nt.py
lib/lp/
lib/lp/
lib/lp/
lib/lp/
== Pylint notices ==
lib/lp/ code/browser/ codereviewcomme nt.py interface' (No module named
19: [F0401] Unable to import 'lazr.delegates' (No module named delegates)
20: [F0401] Unable to import 'lazr.restful.
restful)