Merge lp://staging/~sinzui/launchpad/redirect-201 into lp://staging/launchpad
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Curtis Hovey | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 16311 | ||||
Proposed branch: | lp://staging/~sinzui/launchpad/redirect-201 | ||||
Merge into: | lp://staging/launchpad | ||||
Diff against target: |
83 lines (+24/-20) 3 files modified
lib/lp/app/doc/launchpadview.txt (+0/-19) lib/lp/services/webapp/publisher.py (+3/-1) lib/lp/services/webapp/tests/test_publisher.py (+21/-0) |
||||
To merge this branch: | bzr merge lp://staging/~sinzui/launchpad/redirect-201 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Review via email: mp+136198@code.staging.launchpad.net |
Commit message
Do not render html when status is 201.
Description of the change
I'm getting oopses while trying to submit a merge proposal where the
description has a non-ASCII character in it. The character in question
was a Unicode emdash.
The traceback shows that the MP was was created, the error happened when
that page is rendeder. While the unicode error is odd, it is equally
odd that the view is rendering HTML for an ajax view with a status
of 201. The work is wasteful.
-------
RULES
Pre-
* The action is setting 201 and the location so that the ajax method
will issue its own redirect.
* When the HTML for is used, the action sets next_url which sets up
a redirection view and does not render the template.
* Add 201 to the list of statuses that the LaunchpadView does not
render a content.
QA
* Visit https:/
* Enter "This has a mdash — that I can see" in the comment field.
* Submit the MP.
* Verify your browser loads the MP.
LINT
lib/
lib/
lib/
LoC
I have a 16,000 credit this week
TEST
./bin/test -vvc -t LaunchpadView lp.services.
./bin/test -vvc -t launchpadview.txt lp.app.
IMPLEMENTATION
I added unittests for _isRedirected() and how __call__() uses it. Added
201 to the list of statuses that do not render page content. I removed
a doctest that was replaced by a unittest.
lib/
lib/
lib/
Do not render html when status is 201.
The branch looks good.
Trivial thought: it might be nice if the list of status codes that do not trigger a page render were stored in a variable so the tests could iterate over them instead of having to keep the tests in sync with the implementation.